[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: build profile syntax ideas



Hi Guillem,

Quoting Guillem Jover (2013-12-04 08:48:15)
> Ok, here's the reworked patch I've got locally which will be included in
> today's upload (after I wake up), and the diff against yours.

Thanks a lot for working on this! :D

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.

> I've changed the environment variable and the field contents to be whitespace
> separated, the former because it's easier to handle from makefiles, the
> latter to match the Architecture field. I've pluralized their names too.

Thanks!

> 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,

I spent quite some time on it because it's indeed quite confusing.

It turns out that we have different opinions of how multiple activated profiles
with mixed positive and negative restrictions should be handled.

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.

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.

> 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.

With an implementation that allows to mix positive and negative restrictions in
the form of a logical disjunction, it is up to the maintainer which form she
prefers.

On the other hand, it will probably be very seldom that one needs to mix
positive and negative restrictions and thus it might be more clear to forbid
mixing of positive and negative restrictions as it is already the case for
architectures.

Maybe you can look at my patch again and evaluate the profile_is_concerned
function I wrote with having in mind that I thought about the restriction list
as a disjunction instead of "the first match wins"?

> 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.

That or Built-With-Profiles and Build-Profiles-Used as suggested by Raphaël
Hertzog in another follow up of this email would surely work well.

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.

> In any case given that this is currently periferal to normal packaging, and
> that the syntax is not yet accepted by the Debian infrastructure, I'm open to
> further refinements during the next weeks after 1.17.2 hits unstable, if need
> be.

I didnt test your patch in practice (probably wookey can do this quicker than
I?) but it looks great! Thanks for working on it!

cheers, josch


Reply to: