Re: Spec comments

From: David Abrahams (gclbb-jamboost_at_[hidden])
Date: 2003-06-23 08:48:21

Vladimir Prus <ghost_at_[hidden]> writes:

> Context for the list: some time ago Ali wrote a proposal for improved V2 
> architecture. After some off-list discussion we did not decide anything,
> except that we should keep all discussions on list. So I'm posting an attempt 
> on analysis the spec. The spec itself can be found at
>
> http://boost.sf.net/boost-build2/architecture.txt
> http://boost.sf.net/boost-build2/architecture.pdf
>
> but note that it's not yet finished.
>
> First, here's my analysis of the way we process properties currently. We start 
> with main-target.generate call. 
>
> 1. Inside main target
> 1) apply-default-build is called
> 2) property-adjuster is called
> 3) select-alternatives is called which selects an instance of
> basic-target, which 'generate' method is called
>
> 2. Inside basic-target
> 1. refined-properties is called, which
> 1. Calls 'property-set.refine'
> 2. Calls 'property-set.evaluate-conditionals'
> 3. Calls 'property-set.run-actions'
> 2. All dependency properties are found and generated.
> 3. All sources are generated
> 4. Usage requirements propagated from dependencies in steps
> 2 and 3 are added to the property set used for building
> 5. The 'construct' method is called
> 6. Usage requirements that should be retuned are determined
> 1. Conditionals in usage requirements are evaluated
> 2. Dependency properties in usage requirements are generated
> 3. Usage requirements from 2) are added
>
> In Ali's proposal:
> 
> - 1.2 and 1.3 are omitted
> - 2.1 is basically retained
> - 2.2 and 2.3 are handled in one step, because sources are considered as
> property <source>
> - 2.4 is handled implicitly by 'addProperty' method called everywhere
> - 2.6 is omitted.
>
> My comments:
>
> 0. I don't really like the idea that conditional properties are evaluated in 
> context accumulated after evaluation of previous conditional properties.
> IOW, meaning of conditional properties depends on order. On Jabber, Dave said
> there was some use case which asked for order-dependent semantic, but could 
> not remember that use case.

Yeah, part of the problem is that the time between when Ali and I
had the discussion and when the spec got posted was so long that I
forgot much of the background thinking and rationale.

I think it's worth thinking of the move in this context: the idea was
to try to represent main target alternatives as conditional
properties. If alternatives themselves can currently have differing
conditional properites, there's no way to make this representational
collapse without losing expressivity.

Of course, I'm now convinced that trying to do that collapse causes
some real implementation difficulty, and we ought to keep alternatives
if it's possible to fit them into Ali's proposal without too much
trouble, so contextual evaluation may not be so important.

> 1. The 'property-adjuster' is present in current code for a reason -- to make 
> 'direct_request_test.py' work. I agree that it's somewhat complex, but I 
> don't know a simpler solution.

Again, because of long delays I've forgotten rationale, but we
considered this carefully. My vague memory is that one of the
following is true:

a. we convinced ourselves that direct_request_test.py actually
*shouldn't* work, and that using the property adjustment could just
as easily lead to unintuitive behavior in several cases (which of
course I've forgotten - Ali came up with them)

b. the way the process is structured actually does handle that
case.

> 2. select-alternatives is just omitted and no concrete replacement
> is proposed. In some list discussion, it was said that it's still
> will be necessary to select only one element from a group --- which
> makes me wonder if 'select-alternatives' is still needed.

That was an effect of the knowledge that alternatives can be
represented as conditional properties. As you've pointed out though,
just because they *can* doesn't mean they *should*.

> 3. The 'addProperty' method in the proposal adds properties to 
> this->"usage-requirements" --- but it cannot do that. Main target can be 
> generated with different properties and different usage requirements
> can be generated. 

Yeah, that looks like a problem.

> I suspect that fixing this problem will make usage
> requirements processing similiar to the current one.

It seems to me that the fix is to append to t->"usage_requirements"
instead. Isn't that simple enough? After all, usage requirements
are only taken from already-realized targets elsewhere. A few lines
up:

Target t = findAndRealize( "property"->value );
for p in t->"usage-requirements"
...

> 4. The current code does something complex with sources. 

You say that like it's a *good* think <wink>

> It groups all virtual target generated from the same main target
> dependency together. This allows to issue a warning about unused
> source only if no virtual target from a single main target was
> consumed.

Can you give a concrete example, please?

> 5. How initial value of 'usage-requirements' is converted into value
> that is returned is not specificed. In particular, it's not stated
> what is done with conditional properties and dependency properties.

In which step of the pseudocode, please?

> 6. It was suggested long ago that library sources are treated
> intelligently. I.e. library which is source of another statically
> linked library would automatically become usage requirement. Is it
> possible with the proposed algorithm? How?

I think we need to make progress on the other issues before we tackle
this one.

-- 
Dave Abrahams
Boost Consulting
www.boost-consulting.com