Re: [boost] [gsoc] Boost.Process done

Subject: Re: [boost] [gsoc] Boost.Process done
From: Jeff Flinn (TriumphSprint2000_at_[hidden])
Date: 2010-09-01 09:04:34

Stewart, Robert wrote:
> Boris Schaeling wrote:
>> On Tue, 31 Aug 2010 14:48:43 +0200, Ilya Sokolov wrote:
>>
>>>> In general Boost.Process depends on these header files
>>>> from other Boost C++ libraries:
>>>>
>>>> boost/algorithm/string/predicate.hpp
>>> I don't think its use is justified.
> 
> You should clearly state your grounds for the objection.
> 
>> Quickly checked what is used where: find_executable_in_path() uses
>> boost::algorithm::ends_with() and boost::algorithm::iends_with().
>>
>>>> boost/filesystem/path.hpp
>>>> boost/lexical_cast.hpp
>>> ditto
>> find_executable_in_path() throws
>> boost::filesystem::filesystem_error() if a program isn't found in
>> PATH.  boost::lexical_cast is used by named_pipe to convert the
>> UUID (from Boost.Uuid) to a std::string.
> 
> That code from the included headers is used is, indeed, justification for their inclusion.  Any argument against that functionality should include an alternative implementation of the required functionality and a justification for choosing the alternatives.
> 
>> Regarding the header files above: I wonder whether it makes
>> sense to get rid of actual helpful functions like
>> boost::algorithm::ends_with() only to remove a dependency on
>> another Boost library. I understand the case about
>> Boost.Filesystem which requires to be built. But it's a kind
>> of strange goal for Boost libraries to reuse as little code
>> as possible from other Boost libraries?
> 
> I agree.  This has reared its head before.  While gratuitous and superfluous dependencies should be avoided, the whole point of Boost is to provide ready made, well tested libraries of reusable code.  If one Boost library cannot use another, why would we expect anyone else to use Boost libraries?
> 
> A dependency on Boost.Filesystem is reasonable to avoid, if possible, because it means Boost.Process could no longer be considered header only.  However, filesystem_error seems a perfectly logical choice and, I presume, it would be expected by anyone already using Boost.Filesystem.  More significantly, there was some mention of using boost::filesystem::path, IIRC, so the dependency exists independent of that error type.

I would prefer that filesystem::path be used rather than strings that 
represent filenames. This would negate the usage of boost.algorithm 
since it's usage is replicating fundamental filesystem functionality.

Most of the code in 'find_executable_in_path' could be replaced with a 
few lines of portable filesystem code.

At a minimum, 'find_executable_in_path' should be pulled out of 
operations.hpp and placed in it's own header/cpp. Alternatively 
filesystem could provide this function. Searching for an executable 
differs little from searching the environment path for a dll, or 
searching a list of directories for includes/libs.

Jeff