Hi Aymeric, tldr: We only have about week to upload the fix, so I'd prefer to merge what I reviewed (where you used a patch) right now and upload ASAP. Aymeric Agon-Rambosson <aymeric.agon@yandex.com> writes: > Le dimanche 22 janvier 2023 à 17:31, Nicholas D Steeves > <sten@debian.org> a écrit : > >> It would be nice to see some of the above text in the patch >> header >> and/or Debian changelog (it's not relevant for the upstream PR >> as far as >> I can tell) > > I don't think it is relevant to this specific patch either, and it > could change in the future. I think this a generic piece of > knowledge that we have to keep in mind for all our packages. I > remember enquiring about it and later explaining it a couple of > times on #debian-emacsen, for instance. Maybe the wiki would be > the good place to store that kind of information ? > I agree, the wiki is a good place to centralise information. Linking to that article can then be used to avoid needing to reexplaining things; it also prevents gating when you link to it from package that needs a workaround--this is especially important for a package that needs an active human maintainer! Another possibility might be to eventually provide additional documentation in dh-elpa. [snip] > I sincerely hope that in a real-world yasnippet usage, our users > do not override a primitive as important as `buffer-list' so > recklessly. > Fair point haha! > The bug lies most definitely in the interaction of the native > compiler and the override of the primitive `buffer-list' > implemented by the macro `yas-with-overriden-buffer-list' used by > the three failing tests. This macro is used only for testing, and > is not shipped to our users. > > Advising primitives was frowned upon even before native > compilation, but now it should be avoided at all costs. As far as > I know, the main valid use case for doing it anyway are tests that > aim to give themselves a reproducible environment, so mostly > package maintainers are concerned by this. > It sounds like you're proposing a policy such as this: If an upstream package has advised primitives, in the test file (or in test functions) *for the purposes of testing*, and the package FTBFS, then the Debian package should add these advised primitives to 'native-comp-never-optimize-functions' to prevent FTBFS. Any other instanced of advised of primitives is an upstream bug. Of course it's also worth considering that if the test environment has been advised to such an extent that it no longer represents the package-as-used-by-a-user on a Debian system, then tests run in such an environment have poor functional (and qualitative) value. > According to me, these tests do in fact trigger a difficult to > reproduce bug in emacs related to native compilation and primitive > redefinition. Our users should be fine as long as they do not override > this primitive, or if they do, they also include it to > `native-comp-never-optimize-functions'. > A reliable way to reproduce a difficult to reproduce bug is valuable! :) >> Go ahead and finalise the changelog >> with a commit message that mentions the version >> (0.14.0+git20200603.5cbdbf0d-2) as well as the >> distribution/suite >> (unstable)--in other words, it's just about ready to sponsor. > > If you still think this fix is valid, I can go ahead. However, I > will not implement this fix with a patch, I'll use either d/rules > (like I did with projectile) or more probably d/elpa-test (that > allows arbitrary elisp evaluation before the running of the test). > The "not with a patch" alternative is valid but I worry it may be insufficient, so I would need to see what you're proposing...and we're short on time...for the record, here's my criteria: 1. Don't make Debian-specific fixes (make them upstream, and make the same changes in the Debian package), except for Debian-specific issues 2. Provide the URL where fix was forwarded 3. It's better if no further action needs to be taken when rebasing the Debian package on a future (fixed) upstream version 4. Optionally provide a link to upstream bug (in this case, it sounds like that would be an Emacs bug). In the absence of this, it would be nice to see it as a TODO item, because we're supposed to be helping upstream work towards a more robust future. 5. Don't normalise the bug or take on technical debt--I feel like a simple use of d/elpa-test would probably do this, because this file is usually used for permanent Debian-specific CI integration type problems...also, if it's truly an Emacs then I think that dh-elpa is the place to implement the workaround. >> Also, please forward that patch. > > I'll open a PR and link the various issues and PRs you and David > contributed to. > Thanks! That will fulfil #1 of the above. >> As an aside, I seem to remember that >> native-comp-never-optimize-functions >> is supposed to be renamed sooner or later > > Do you remember where you read that ? > Sorry, I can't remember, other than that someone had already done the work, and that person was writing about the rename as if it had already been merged. It might be floating around someone's feature branch somewhere, or on a 'for-next'-type staging branch. > I'll let you tell me whether you still think this fix is a good > idea, and I'll act accordingly. > Yes to the patch, and I'm not sure about the nascent alternative. Regards, Nicholas
Attachment:
signature.asc
Description: PGP signature