Re: [Boost-build] Custom generator problem

Subject: Re: [Boost-build] Custom generator problem
From: Vladimir Prus (ghost_at_[hidden])
Date: 2009-11-26 04:17:02

On Thursday 26 November 2009 11:45:21 Johan Nilsson wrote:

> Vladimir Prus wrote:
> > On Wednesday 25 November 2009 16:48:22 Johan Nilsson wrote:
> >
> 
> [...]
> 
> >>
> >> I'm attaching the real, updated, generator to this posting. Even
> >> though you won't be able to actully test it without having other
> >> stuff installed, maybe you can find out what the problem is by
> >> browsing the code. This is the the output when adding an alias for
> >> the test targets (paths snipped to protect the innocent):
> >
> > local main-xml-target = [ new file-target $(name) : NUNIT_XML :
> > $(project) : $(main-target-action) ] ; main-targets += [
> >         virtual-target.register $(main-xml-target) ] ; ...
> > local copy-xml-action = [ new action $(main-xml-target) : common.copy
> > : $(xml-target-ps) ] ;
> >
> > At this point 'main-xml-target' is *still* the target constructed by
> > 'new'. Although you have
> > called virtual-target.register passing the current value of
> > 'main-xml-target', you have stored
> > the result in 'main-targets', and 'main-xml-target' is what it was.
> >
> > Does rewriting this as:
> >
> > local main-xml-target = [ new file-target $(name) : NUNIT_XML :
> >            $(project) : $(main-target-action) ] ; main-xml-target =
> > [ virtual-target.register $(main-xml-target) ] ;
> > main-targets += $(main-xml-target) ;
> >         ...
> > local copy-xml-action = [ new action $(main-xml-target) : common.copy
> > : $(xml-target-ps) ] ;
> >
> > Improve things?
> 
> Of course. How extremely embarrasing! I've spent so much time going through
> the generated-targets method to find out the problem, and even when
> Boost.Build tells me exactly what target is duplicated I wasn't able to
> parse that.
> 
> I guess a fresh pair of eyes is what's necessary sometimes. Many thanks!
> 
> Some further questions and suggestions while I'm into extending Boost.Build:
> 
> - I use "virtual-target.from-file" in order to generate the "source targets"
> for the common.copy invocation, but for those I do not use
> virtual-target.register. Neither did you in your fixed version of the first
> sample project. Is this ok because these "targets" are only used as sources?

It's OK -- because virtual-target.from-file tries hard to always return
the same virtual target for a given file. 

In fact, we probably should have virtual-target.create-file-target that calls
'new file-target' and then passes result via register. I don't quite remember
why I did not do this yet -- maybe because if you have an action that produces
more than one targets, you have to first create all of them, and then register.

> - I also use the targets from generated-targets method's "sources" argument
> without registering those. Is it generally safe to do so - can I rely on
> them referring to registered targets before my generator is called?

Yes. It's expected that targets are registered by a generator that creates them,
so when you get result of 'generated-targets' it's done.

> - Again, same goes for the targets named using the <library-file> property
> (this is an artifact of the dotnet generator, which change sources of type
> SHARED_LIB to become a "<library-file>" property instead).

Same here. In general, any dependency properties passed to generators are
expected to be already registered.
 
> - It is a bit confusing at times when "sources" are "targets". Would it be
> possible to have a more neutral name?

It surely would be possible -- except I don't know what better naming to use.
SCons uses 'node' -- is that better? Or 'input' and 'output' targets?

> - It would likely be a good idea to emphasize and exemplify the need for
> registering the targets before use even inside e.g. the "generated-targets"
> method in the documentation.

Added to my docs todo list.

> - The generator's "generated-targets" method does not seem to have to return
> all of the targets, only the top-level ones. The other ones are still
> generated as they are dependencies of the main target(s). Is it ok to only
> return the top-level targets, as long as all are registered?

Yes. It's not only OK, but it's the preferred behaviour. In general, when a metatarget,
such as 'a' declared below:

        exe a : a.cpp ;

is built with specific properties, the 'generate' method of the corresponding
'basic-target' instance should return only 'top-level' targets. And for that,
each generator should also only return the top-level targets.

Doing otherwise will have two drawbacks:

- Dependents of the targets will get a pile of targets they don't care about,
  and spend time (albeit small) before deciding to drop them
- In a few contexts, the extra targets will be used, e.g.

      exe a : a.cpp ;
      install dist : a ;

  You don't really want 'a' to return a.obj -- as you don't want to install
  it.

> - Do you have any other comments on the generator code itself? I'd be happy
> for suggestions for improvements.

I do not have any knowledge of how NUnit works, but as far as Boost.Build is
concerned, I did not find anything wrong there. You might want to use the
options module instead of this:

        local p = [ MATCH --nunit-path=(.*) : [ modules.peek : ARGV ] ] ;

as this will allow you to put

        option nunit-path : whatever ;

in any config file. This is a pure convenience matter -- what you have
is fine.

- Volodya