Re: [jamboost] Boost jam patch to improve locating source directories

From: Vladimir Prus (ghost_at_[hidden])
Date: 2005-05-06 08:03:28

Hi Craig,
> I would like to submit the following patch, which was
> developed by my colleague Alexander Kabaev <kan_at_[hidden]>.
>
> We use this patch to Boost Build, in order to help us
> specifiy the directories where source files are located
> on a per-project basis. I have attached the patch, and
> a testcase, where the source files for a project are
> in multiple directories.
>
> This basically achieves for me what I would have
> used VPATH in GNU make for.
>
> Thanks for your consideration.

As I've told to you off-list, the patch is mostly fine. I've just run
the tests with it and all of them pass. Below are some minor issues:

> --- /home/crodrigu/boost-build/build/targets.jamšššššššš2005-03-20 
14:00:43.000000000 -0500
> +++ /usr/snapshots/akabaev/boost-build/build/targets.jamšššššššš2005-04-15 
20:26:03.838527248 -0400

> š š š# Returns true if the referred file really exists;
> š š šrule exists ( )
> š š š{
> - š š š šlocal location = [ path.root $(self.name)
> - š š š š š[ $(self.project).get source-location ] ] ; š š š š
> - š š š šreturn [ CHECK_IF_FILE [ path.native $(location) ] ] ;
> - š š} š š
> - š š
> + š š š šreturn [ CHECK_IF_FILE [ path.native [ location ] ] ] ;
> + š š}
> +
......
> +ššššššš š šfor local src-dir in $(source-location)
> + š š š š š š{
> + š š š š š š š šif ! $(self.file-location)
> + š š š š š š š š{
> + š š š š š š š š š šlocal location = [ path.root $(self.name) 
$(src-dir) ] ;
> + š š š š š š š š š šif [ CHECK_IF_FILE [ path.native$(location) ] ]
> + š š š š š š š š š š{
> + š š š š š š š š š š š š self.file-location = $(location) ;
> +ššššššššššššššš š š}
> + š š š š š š š š}
> + š š š š š š}
> + š š š š š šif ! $(self.file-location)
> + š š š š š š{
> + š š š š š š š š self.file-location = [ path.root $(self.name)
> + š š š š š š š š š š $(source-location[1]) ] ;
> + š š š š š š}

What's the point of this last 'if'. If we've tried to find the file, and 
failed, then we failed. Why return a value that's known to contain no such 
file?

Also, the 'exists' rule first calls 'location' and then calls CHECK_IF_FILE 
again. Why just not make 'location' return nothing when file is not found, 
and rewrite exists as:

rule exists ( )
{
return [ location ] ;
}

> diff 
-ur /home/crodrigu/boost-build/build/virtual-target.jam /usr/snapshots/akabaev/boost-build/build/virtual-target.jam
> --- /home/crodrigu/boost-build/build/virtual-target.jamš2005-03-20 
14:00:43.000000000 -0500
> +++ /usr/snapshots/akabaev/boost-build/build/virtual-target.jamš2005-04-15 
20:34:57.312426888 -0400
> @@ -525,10 +525,14 @@
> š š š š š}
> š š š š šelse
> š š š š š{
> + š š š š š šlocal source-location = ;
> + š š š š š šfor local source-dir in [ $(self.project).get source-location ]
> + š š š š š š{
> +šššššššššššššššsource-location += [ path.native $(source-dir) ] ;
> +ššššššš š š}
> š š š š š š š# This is a source file.
> - š š š š š šSEARCH on $(target) =
> - š š š š š š š[ path.native [ $(self.project).get source-location ] ] ;
> - š š š š} š š š š
> + š š š š š šSEARCH on $(target) = $(source-location) ;
> + š š š š}
> š š š}

Given that 'exists'/'location' above already computed the exact location of 
the target, maybe it's better to store it as a member and just use:

SEARCH on $(target) = $(self.location) ;

Would save some time searching.

Another detail. I'd be happy to commit the patch, but if you'd like it to work 
despite all the refactorings I make in future ;-), here's a simple recipe:

1. Take the test project you've sent and copy it to the "test" directory. 
2. Run 

python load_dir.py $the_project_dir > vpath.py

3. Edit vpath.py and modify the list of binaries that are expected to be 
created (at the end of the file)
4. Modify the copyright message.

That done, vpath functionality will be always tested.

Thanks,
Volodya