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 -- To UNSUBSCRIBE, email to debian-dpkg-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20131204134738.2752.60436@hoothoot