CVS: boost/tools/build/v2/build generators.jam, 1.100, 1.101 targets.jam, 1.197, 1.198

Rene Rivera <grafikrobot <at> gmail.com> writes:

> 
> Vladimir Prus wrote:
> > On 8/2/06, Rene Rivera <grafik <at>
users.sourceforge.net> wrote:
> 
> >> Skip targets that can't be generated like
BBv1 does.
> > 
> > Just as a check -- did you run V2 testsuite before
and after this 
> > change, to
> > verify it causes no regressions?
> 
> No, but that was because I didn't have the time. I'll
get to that this 
> weekend.

Hmm, generally it's better to run regression tests *before*
checkin. That's 
what I do, at the very least. 

> 
> > Index: generators.jam
> >> +    # Indicates if this generator can handle
building the given target
> >> +    # combination.
> > 
> > I think this comment is not sufficient.
> 
> It seems as sufficient as some of the other commented
methods in that 
> class. 

Are you saying those other comments are not sufficient too?
That most likely 
true, but that does not mean we should not try to improve
commenting for new 
checkings. Those other comments were probably written years
ago, when I did not
realize how really important comments are.

> It says exactly what it means and nothing else. Perhaps
comments 
> need to be added to where can-build is called to
explain what effect it 
> has there.
> 
> > How returning false from this rule
> > is different from
> > returning nothing from the 'run' rule?
> 
> I have no clue. But then again the run rule doesn't
say what happens 
> when it returns nothing.

I can fix that part; but that does not change the fact that
'can-build' is 
under-commented.

> 
> > I think there are some differences,
> > but the comment does not say what they are.
> 
> Perhaps... What do you think are the differences?

Well, returning nothing from 'run' method means the
generator failed.
If all possible generators fail, then building a targets
failed and an error 
is produced. As for 'can-build' returning false, you know
better, but it seem 
to be some "half-failed" situation.

> 
> >> -rule find-viable-generators ( target-type :
property-set )
> >> +rule find-viable-generators ( target-type :
property-set : sources * )
> >> {
> >>     local key = $(target-type).$(property-set)
;
> >>     local l = $(.fv.$(key)) ;
> >>     if ! $(l)
> > 
> > 
> > You've added new parameter by you still key the
.fv. catch on
> > target-type/property-set.
> > It means that if two invocation differ by
'source' the second one will 
> > reuse
> > results of the first.
> 
> OK, didn't notice there was a cache being use. May I
suggest better 
> names for such variable. Perhaps something that has the
word cache in 
> it, like "viable-generators-cache". I'll
go fix the cache key problem 
> though 

I'm not sure about longer name -- won't that increase
memory consumption of 
bjam?

> 
> >>  <at>  <at>  -733,8 +735,11 
<at>  <at> 
> >>         for local p in $(all-property-sets)
> >>         {
> >>             local r = [ generate-really $(p) ]
;
> >> -            usage-requirements = [
$(usage-requirements).add $(r[1]) ] ;
> >> -            result += $(r[2-]) ;
> >> +            if $(r)
> >> +            {
> >> +                usage-requirements = [
$(usage-requirements).add $(r[1])
> >> ] ;
> >> +                result += $(r[2-]) ;
> >> +            }
> >>         }
> > 
> > So, this code allows a dependency to emit a
warning, produce nothing, and
> > still go on. 
> 
> Yea, seems that way.
> 
> > It seems to be that if any dependency fails to
build then
> > dependent's build should be aborted immediately.
> 
> If by "aborted" you mean that it should
print a warning and skip 
> building the dependent's target? Then yes. How can a
target be made to skip?

You need to modify abstract-target.generate so that it can
return 'skipped' 
return value, and adjust all callers to act accordingly on
'skipped' value. 
That's gonna be a lot of work, though.

> 
> > I still don't understand why *failure* to build a
target should become a
> > warning.
> 
> Because skipping unbuildable targets has been the
expected behavior of 
> Boost.Build from the start. 

Well, we don't know if that's the behaviour users really
want -- it was not 
documented like that, and nobody ever complained about
either V1 behaviour or 
V2 behaviour.

> When users ask for something to get built it 
> should build as much as it can and only reports
problems at the end. But 
> that's not really an argument, but an excuse. The real
reason is that we 
> don't want to force artificial barriers on building
what we think are 
> incorrect configurations. That just frustrates new
users who might get 
> fed up with an uncooperative build system. And it
frustrates experienced 
> users because it will prevent them from doing things
that they know will 
> work.

That's an argument for disabling runtime-link=static check.
We can discuss 
that decision, but your change does something different. It
seem to make V2 
ignore all problems with building targets. So if no
generators at all can't be 
found, you still get just warning. And dependents are still
built. I don't 
think such hiding of problems is good. Imagine that V2 will
do 30 min build of 
a large project, and then report about a problem, that will
indicate the build 
properties are wrong, and you need yet another 30 min build.


> > In
> > general, if you ask generators.generate to produce
target of type WHATEVER,
> > you expect it to do just that, or fail.
> 
> Returning nothing is a failure. It's just not
aborting, without letting 
> you handle the error, as the code previously operated.

Hmm, I now recall that generators.generate are already
supposed to return
empty list on failure.

> 
> > If you add a third option -- build
> > nothing -- you either need to use a special return
code,
> 
> Returning and empty list, i.e. nothing, is a special
return code. It 
> means the call was unable to satisfy your request.
> 
> > or check that all
> > callers will properly react to such
"nothing" return.
> 
> Yes.
> 

Hmm, I start to think what you did could be done simpler.
You can modify gcc 
linking generators, so that for runtime-link=static it
returns nothing. Then, 
you can modify typed-targets so that when generators return
nothing, warning 
is emitted instead of error (probably, just for Boost).
Then, you won't need 
new generators method, as well as some of the changes you
made.

This approach would simplify the code; though I still don't
agree with target 
skipping approach.

Also, the problem with Python generators is likely to be
problem present 
before your changes -- it should be checking that
generators.generate return 
something.


- Volodya







_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost-build

Vladimir Prus wrote:
> On a second though, it looks like 'can-build'
returning false make 
> generator fail during find-viable-generators call,
instead of failing
> in 'run' method. So, it's a mechanism for making
generator fail
> earlier. I'm not sure if we need this second
mechanism, given that
> 'run' can return nothing already.

If that's true, then why did you initially use the
"generate-targets" 
rule instead of the "run" rule to implement the
link-runtime=static 
restriction?


-- 
-- Grafik - Don't Assume Anything
-- Redshift Software, Inc. - http://redshift-software
.com
-- rrivera/acm.org - grafik/redshift-software.com
-- 102708583/icq - grafikrobot/aim - grafikrobot/yahoo
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost-build

Vladimir Prus wrote:
> Rene Rivera <grafikrobot <at> gmail.com>
writes:
> 
>> Vladimir Prus wrote:
>>> On 8/2/06, Rene Rivera <grafik <at>
users.sourceforge.net> wrote:
>>>> Skip targets that can't be generated like
BBv1 does.
>>> Just as a check -- did you run V2 testsuite
before and after this 
>>> change, to
>>> verify it causes no regressions?
>> No, but that was because I didn't have the time.
I'll get to that this 
>> weekend.
> 
> Hmm, generally it's better to run regression tests
*before* checkin. That's 
> what I do, at the very least. 

Running the tests spews a bunch of errors, mostly
complaining about 
toolsets not configured. Even though I have 5 different
toolsets 
configured and I specified a toolset to use on the
invocation.

Does this work on Windows?

It's hard to pay attention to a test system that seems
broken 

-- 
-- Grafik - Don't Assume Anything
-- Redshift Software, Inc. - http://redshift-software
.com
-- rrivera/acm.org - grafik/redshift-software.com
-- 102708583/icq - grafikrobot/aim - grafikrobot/yahoo
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost-build

Vladimir Prus wrote:
> Rene Rivera <grafikrobot <at> gmail.com>
writes:
>> Vladimir Prus wrote:
>>> On 8/2/06, Rene Rivera <grafik <at>
users.sourceforge.net> wrote:

> Are you saying those other comments are not sufficient
too?

Yep.

> That most likely 
> true, but that does not mean we should not try to
improve commenting for new 
> checkings. Those other comments were probably written
years ago, when I did not
> realize how really important comments are.

Sure, hopefully things will get better. But my changes show
how someone 
relatively new can do things that might be superfluous or
wrong.

>> I have no clue. But then again the run rule
doesn't say what happens 
>> when it returns nothing.
> 
> I can fix that part; but that does not change the fact
that 'can-build' is 
> under-commented.

I changed the gcc generator to use run and removed the
can-build. And I 
added a small comment to generator.run that might help the
next person 
who runs into this. You might want to tweak the comment with
a better 
explanation.

>> OK, didn't notice there was a cache being use. May
I suggest better 
>> names for such variable. Perhaps something that has
the word cache in 
>> it, like "viable-generators-cache".
I'll go fix the cache key problem 
>> though 
> 
> I'm not sure about longer name -- won't that increase
memory consumption of 
> bjam?

Yes it would, but the values are likely to take up much more
room. And I 
would think that understandable programs are more important.
Perhaps 
just a comment indicating it's creating a cache would be
enough.

>> If by "aborted" you mean that it should
print a warning and skip 
>> building the dependent's target? Then yes. How can
a target be made to skip?
> 
> You need to modify abstract-target.generate so that it
can return 'skipped' 
> return value, and adjust all callers to act accordingly
on 'skipped' value. 
> That's gonna be a lot of work, though.

Would having it return a special
"skipped-target", that can never be 
selected, work?

>>> I still don't understand why *failure* to
build a target should become a
>>> warning.
>> Because skipping unbuildable targets has been the
expected behavior of 
>> Boost.Build from the start. 
> 
> Well, we don't know if that's the behaviour users
really want -- it was not 
> documented like that, and nobody ever complained about
either V1 behaviour or 
> V2 behaviour.

Well in BBv1 not much was documented  But I do
have some vague 
memories of discussions about this.

>> When users ask for something to get built it 
>> should build as much as it can and only reports
problems at the end. But 
>> that's not really an argument, but an excuse. The
real reason is that we 
>> don't want to force artificial barriers on
building what we think are 
>> incorrect configurations. That just frustrates new
users who might get 
>> fed up with an uncooperative build system. And it
frustrates experienced 
>> users because it will prevent them from doing
things that they know will 
>> work.
> 
> That's an argument for disabling runtime-link=static
check.

Yes it is, which would make it behave like BBv1.

> We can discuss 
> that decision, but your change does something
different. It seem to make V2 
> ignore all problems with building targets. So if no
generators at all can't be 
> found, you still get just warning.

That is fixed. I accidentally removed the EXIT in that case.

> And dependents are still built. I don't 
> think such hiding of problems is good. Imagine that V2
will do 30 min build of 
> a large project, and then report about a problem, that
will indicate the build 
> properties are wrong, and you need yet another 30 min
build.

Well it currently reports problems at the start. But yes
building 
dependents is a problem.

> Hmm, I start to think what you did could be done
simpler. You can modify gcc 
> linking generators, so that for runtime-link=static it
returns nothing. Then, 
> you can modify typed-targets so that when generators
return nothing, warning 
> is emitted instead of error (probably, just for Boost).
Then, you won't need 
> new generators method, as well as some of the changes
you made.
> 
> This approach would simplify the code; though I still
don't agree with target 
> skipping approach.

Hm, I wonder if an easier solution is to have the gcc
toolset add a 
"<build>no" to the properties. Is that
possible? It would seem to solve 
all the problems of skipping targets and dependents.



-- 
-- Grafik - Don't Assume Anything
-- Redshift Software, Inc. - http://redshift-software
.com
-- rrivera/acm.org - grafik/redshift-software.com
-- 102708583/icq - grafikrobot/aim - grafikrobot/yahoo
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost-build

Rene Rivera wrote:
> Vladimir Prus wrote:
>> Rene Rivera <grafikrobot <at>
gmail.com> writes:
>>> Vladimir Prus wrote:

>> Hmm, I start to think what you did could be done
simpler. You can modify gcc 
>> linking generators, so that for runtime-link=static
it returns nothing. Then, 
>> you can modify typed-targets so that when
generators return nothing, warning 
>> is emitted instead of error (probably, just for
Boost). Then, you won't need 
>> new generators method, as well as some of the
changes you made.
>>
>> This approach would simplify the code; though I
still don't agree with target 
>> skipping approach.
> 
> Hm, I wonder if an easier solution is to have the gcc
toolset add a 
> "<build>no" to the properties. Is
that possible? It would seem to solve 
> all the problems of skipping targets and dependents.

Pinging since I never got a response on this one.


-- 
-- Grafik - Don't Assume Anything
-- Redshift Software, Inc. - http://redshift-software
.com
-- rrivera/acm.org - grafik/redshift-software.com
-- 102708583/icq - grafikrobot/aim - grafikrobot/yahoo

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost-build

On Monday 07 August 2006 10:00, Rene Rivera wrote:

> > Hmm, I start to think what you did could be done
simpler. You can modify
> > gcc linking generators, so that for
runtime-link=static it returns
> > nothing. Then, you can modify typed-targets so
that when generators
> > return nothing, warning is emitted instead of
error (probably, just for
> > Boost). Then, you won't need new generators
method, as well as some of
> > the changes you made.
> >
> > This approach would simplify the code; though I
still don't agree with
> > target skipping approach.
>
> Hm, I wonder if an easier solution is to have the gcc
toolset add a
> "<build>no" to the properties. Is
that possible? It would seem to solve
> all the problems of skipping targets and dependents.

I'm not sure what you mean. We can add

   
<toolset>gcc,<link>shared,<runtime-link>st
atic:<build>no

to top-level requirements, so that if such combination
won't be ever built.

- Volodya


-- 
Vladimir Prus
http://vladimir_pru
s.blogspot.com
Boost.Build V2: http://boost.org/boost-
build2
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost-build

Hm, almost missed your reply :-(

Vladimir Prus wrote:
> On Monday 07 August 2006 10:00, Rene Rivera wrote:
> 
>>> Hmm, I start to think what you did could be
done simpler. You can modify
>>> gcc linking generators, so that for
runtime-link=static it returns
>>> nothing. Then, you can modify typed-targets so
that when generators
>>> return nothing, warning is emitted instead of
error (probably, just for
>>> Boost). Then, you won't need new generators
method, as well as some of
>>> the changes you made.
>>>
>>> This approach would simplify the code; though I
still don't agree with
>>> target skipping approach.
>> Hm, I wonder if an easier solution is to have the
gcc toolset add a
>> "<build>no" to the properties. Is
that possible? It would seem to solve
>> all the problems of skipping targets and
dependents.
> 
> I'm not sure what you mean. We can add
> 
>    
<toolset>gcc,<link>shared,<runtime-link>st
atic:<build>no
> 
> to top-level requirements, so that if such combination
won't be ever built.

Well kindof, what I meant is can we make the toolset itself
add that to 
targets. There seem to be a variety of time when the toolset
itself 
needs to impose such constraints on what gets built. I did
this in BBv1 
with the toolset::requirements rule. And I thought that was
the overall 
intent of the custom gcc generator. So the overall question:

How do we impose build requirements from a toolset?


-- 
-- Grafik - Don't Assume Anything
-- Redshift Software, Inc. - http://redshift-software
.com
-- rrivera/acm.org - grafik/redshift-software.com
-- 102708583/icq - grafikrobot/aim - grafikrobot/yahoo

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost-build