meson-python
meson-python copied to clipboard
mesonbuild
ENH: do not include uncommitted changes in the sdist
Including uncommitted changes in the sdist was implemented in #58 after the discussion in #53. However, the current behavior for which uncommitted changes to files under version control are included but not other files, is an hard to justify surprising half measure.
After careful analysis, it has been determined that none of the use cases for this feature is still valid. Removing it makes the implementation easier, and the behavior less surprising and easier to document an explain.
Thanks. Quick comment: could you update the sdist docs in docs/explanations/design-old.rst for this change, and mention add_dist_script there?
design-old.rst is not linked anywhere, thus it is not published. I thought we keep it only as base for some future documentation work. This is why I haven't updated it. I'm also working on some documentation updates, but the job got bigger than anticipated and it is not finished yet.
design-old.rstis not linked anywhere, thus it is not published. I thought we keep it only as base for some future documentation work
You're right. My bad, that's what I get for trying to squeeze in a quick comment before signing off. I saw the removal of the add_dist_script comment, then looked for a mention in the docs, didn't find it, then looked for .gitattributes. Our only mention of that is in design-old.rst. Looks like we should revive some of that content and link to https://mesonbuild.com/Creating-releases.html
Should we merge this and leave the documentation updates to a separate PR?
I'm okay with doing the docs in a follow-up. This is ready to go, right? If so, I need to do some testing with numpy, scipy, & co.
Some typos: s/changer/changes/ in the PR title and commit summary, and s/an hard/a hard/ and s/document an explain/document and explain/ in the description.
@dnicolodi I'd like to merge this for the next release after doing some final testing (likely tomorrow). Would you mind resolving the conflict and fixing the commit message typos pointed out by @QuLogic?
@rgommers I fixed the ownership metadata in the sdist archive to be root:root (not only to use the 0 uid and gid as it was implicitly done before). This follows what git archive does. I left the permissions unchanged: we use the same permissions exported by git archive. It seems a valid choice to me. I added another commit that makes the sdist byte-for-byte reproducible in most cases, see the longish comments in the code. I think it is a worthwhile goal and it relatively easy to achieve. Finally, I promoted and expanded the little documentation about sdist we had in the unpublished document to its own section in the manual. Please have a look.
That sounds great! Thanks for improving the docs too. I'll try and revisit this in the morning.
I'm very surprised that this happens, especially by the path that two different paths are used. I don't see how that can happen. Let me see if I can reproduce it.
Actually, we have tests where the project name passed to Meson's project() and the Python distribution name differ, and these work just fine. I really don't see how what you see could happen. Also, the code that adjusts the paths didn't change compared to what you tested previously. Are you sure that the test is correct?
I can reproduce it building an sdist for numpy. But I really cannot get how it can happen.
All the paths that are not correctly rewritten are longer than 100 characters. It looks like a Python bug to me. Investigating further.
Yup. I'm pretty sure it is a Python tarfile module bug: when the name filed in the TarInfo object is set, and the object has an associated extended pax header, the path property in the pax extended header is not reset. If the new name is shorter than what requires the path to be stored in the extended pax header, the path in the pax header is not updated when the archive member is serialized. When the header is read back, the path in the extended path header is read, instead of the shorter one in the regular header. The work around is simple. I'll implement it with a test tomorrow.
Indeed. Pushed now. I think I was waiting to file a bug for CPython to be able to reference it in the code comment and commit message, but I haven't submitted it yet. I don't think it is important to reference it here, and I can add a reference later. Pushed now.