Hi! On Fri, 2024-07-26 at 13:20:28 +0200, Nicolas Boulenguez wrote: > > Hmm, at this point I'm starting to ponder whether to revert the > > optimization commit for the Makefile fragment files, because this > > is starting to feel like too much breakage, and then the fragment > > code is becoming too hard to debug, or even test. > > This sentence seems a bit unfair. The new implementation has come > with new regression tests, and does not increase the source > complexity.
Oh, it was not my intention to disparage your work, I'm also glad you have been helping out with these regressions, and to be very clear I have no problem in general with implementation bugs or regressions which happen to all of us, as long as I get to get these fixed quickly (timelines depending on the severity of course, etc). My comment was more about make(1) being a very brittle language to use for general programming, and the fact that to get good performance it needs such a maze of nested eval processing, the multiple variable origins, etc, which IMO does add complexity, cognitive if nothing else (and of course the previous code was already not trivial). > > I've not yet looked into it, Nicolas if you can have a look please, > > otherwise I might do the revert and another upload later today or so. > > I will investigate, but without much hope. The difference is probably > caused by things like conflicting CFLAGS on the command line or in the > environment, from debian/rules or dpkg-buildpackage, for ./configure > or make, possibly kept by ./configure for make despite a now > conflicting environment… Thanks for looking into it, in any case! If I end up reverting, I'd not necessarily consider this final, and I'd be fine reintroducing the changes, once the problem is understood, etc, and ideally once we can test for this, or if it's deemed a problem then after an orderly transition has been planned, or perhaps as an additional implementation to choose from based on the dpkg-build-api, so that maintainers can opt-in to the new behavior and watch for issues, etc. > This mess is probably one of the reasons why > DEB_CFLAGS_MAINT_APPEND was introduced and CFLAGS+= deprecated in both > the environment and debian/rules. I don't recall the exact context for these, and I don't think it's worth digging that up in email/irc-logs, but I'd say these were added just to provide a declarative (ish) way to get output flags already as desired w/o needing to post-process them, and to match the dpkg-buildflags config options, and globally settable environment variables to override all build flags that people were probably starting to misuse from within packaging. > The fact that the previous lazy evaluation mechanism, in which the > $(evals VAR=$(VAR)) trick is already present, did what you expect in > some contexts does not make CFLAGS+= a supported interface, and your > code could break in other contexts. > > Replacing > CFLAGS+= foo > with > DEB_CFLAGS_MAINT_APPEND := foo > in the broken packages seems more fruitful to me, and I would prefer > to help with that. > > That would most probably fix this bug, does not hurt if the > optimization is reverted, and will actively help if dpkg-buildpackage > ever becomes the main entry point for package builds. The problem though is, that this is something that used to work, and it is currently a silent breakage (the worst kind), which _could_ (but might not) affect hundreds of packages. A not very exhaustive query on codesearch.d.n shows that just for CFLAGS this could indeed be a potential problem for hundreds of packages. And I'm not very comfortable with this kind of silent and subtle breakage at this scale, when that can end up with either miscompilation, reduced security or features or similar. Thanks, Guillem
