Re: build profile syntax ideas

[Guillem Jover <guillem@debian.org>]

Hey!

[ Undusting, before it gets lost in my postponed mail box, along other
  old corpses there. ]

On Wed, 2013-12-04 at 14:47:38 +0100, Johannes Schauer wrote:
> I see you attributed me as the author of the patch but please note that as
> noted in the email in which I submitted the patch, it was based on yet another
> patch by wookey and Patrick McDermott (pehjota).
>
> While I do not care about receiving attribution for this patch I do not know
> about the others.

Ah, thanks, I had forgotten the initial patch by Patrick, it was not
clear to me if Wookey had done any code change from the one you picked up,
besides hosting the patch, but included him anyway given your comment.

> Quoting Guillem Jover (2013-12-04 08:48:15)
> > I've also changed the restrictions logic to match the arch one, because the
> > one proposed here didn't make sense to me in some cases,

Just to clarify, because the wording here might have been unfortunate,
what I meant was that observing the function as if it was a black box,
giving it inputs, the outputs didn't make sense to me in some cases,
and make sense as in “this outcome is not good”, not as in “not
understood”.

The code and the logic are way more beautiful in your proposal,
unfortunately I think they have a major drawback.

> In your test cases I see that for "dep3 <!profile.stage1 profile.notest>" with
> profiles stage1 and notest both activated, dep3 does not get picked.

Right (although I rearranged it so that it gets picked now, and I guess
that could be considered cheating :), I should probably add another case
demonstrating it being dropped.

> I thought of the list of restrictions as a logical disjunction, meaning it
> evaluates to true if any of its terms evaluates to true. Thus I thought that
> dep3 would be included with profiles stage1 and notest both be activated, as
> you can see in the initial patch I submitted.  The reason being that while the
> first term "!profile.stage1" is surely not met, the second term
> "profile.notest" is.  Since it's a disjunction, as long as one of the
> conditions is true, the result is true and dep3 should be picked.
> 
> From that perspective, your choice does not make sense. But maybe this is
> exactly why it should not be allowed to mix positive and negative restrictions.
> Especially because there is an alternative way to express them (see further
> down).
> 
> Your choice does make sense if you do not treat the expression as a disjunction
> but instead process them as "the first match wins" as it is already done for
> architectures. This makes it important in which order profiles are given. I
> dont think that the order should matter.
> 
> The "the first match wins" approach in my eyes makes sense if you only have
> negative or only have positive terms. But not if you can mix them.

Sure. The issue with the disjunction is that adding a new negated
restriction is not safe anymore, in the common case of building with
just one profile, and no mixed positive/negative restrictions. What
tripped me, was the dep1 in the 2/4 and 3/4 cases.

Say, you don't want a specific dependency on profile.stage1, you have:

  dep1 <!profile.stage1>

it gets reduced fine if only profile.stage1 is specified, but then if
you decide you don't want it either on profile.other, then:

  dep1 <!profile.stage1 !profile.other>

will make dep1 suddenly start appearing on profile.stage1.

This means, that adding negations to an existing negated restriction is
not safe any longer, because it has side effects, and to get the old
behaviour you need to know what other restrictions are in place for
each dependency where profile.stage1 appears so that they can be
specified as part of the build. This seems highly undesirable to me.

If this analysis is wrong, I'd like to hear, or if you have a solution
to that specific problem, I'd gladly revert to a nicer implementation
that could support also positive/negative restrictions nicely.

> > the problem though comes from supporting positive and negative profiles in
> > the same restriction, while supporting multiple active profiles. In general
> > I'd advise against mixing positive and negative profiles, though, because it
> > can produce confusing results.
> 
> Correct. Mixing positive and negative restrictions and at the same time
> specifying multiple profiles at once can be confusing.
> 
> Alternatively (with my interpretation of the list of restrictions),
> 
> 	foo:any (>=1.0) <profile.blub profile.bla !profile.bar>
> 
> would translate to
> 
> 	foo:any (>=1.0) <profile.blub>, foo:any (>=1.0) <profile.bla>, foo:any (>=1.0) <!profile.bar>" 
> 
> which is quite repetitive and we wanted to minimize the amount of required
> repetition in such cases.
> 
> Also, with your interpretation of how multiple activated profiles with mixed
> positive and negative restrictions should be handled, the expansion above would
> not be valid. Indeed there would not exist a valid expansion because there is
> no "the first match wins" in dependency resolution otherwise.  This is also why
> I though that the idea of treating the list of restrictions as a disjunction
> would make more sense.

There's no valid representation of a conjunction in neither of the designs
either, which could be useful in some rare cases I guess.

> > There's a thing I'm not entirely sold on yet, and will probably rethink it
> > once I wake up, that's the name of the output field, currently
> > Build-Profiles, but I'm pondering on Built-For-Profiles for example.

> We decided to name it Build-Profiles as well because the Architecture field in
> binary packages is also not called Built-for-Architecture and we wanted to keep
> some parallels to how Architectures work. But either is fine.

Well the difference is that the Architecture is a property of the
built package, the profiles are a property of the build environment.
But in any case, the problem with “build” is that it's already a pretty
overloaded term in dpkg-dev, we have things the host/build(/target)
convention, then there's build used for build environment stuff, like
build-flags and similar, extending that seems it might create more
confusion down the line.

Thanks,
Guillem