| clarkb | now there is a new problem with pip install -e and pip install having apparently different behaviors when it comes to writing out the consoel scripts | 00:01 |
|---|---|---|
| clarkb | oh maybe it is because I cannot spell | 00:08 |
| Clark[m] | Ok I need to stop for the day but I think the issue is pyproject.toml goes through a different path for console scripts so the pbr files aren't used? | 00:38 |
| Clark[m] | After fixing my issues the install -e also fails | 00:38 |
| fungi | yeah, creating the console scripts is pip's responsibility (they're not packed inside sdists and wheels) | 14:20 |
| fungi | so it's a question of making pip notice, i guess | 14:21 |
| fungi | i think it's expecting a [project.scripts] table in pyproject.toml | 14:22 |
| fungi | the footnote at https://setuptools.pypa.io/en/latest/userguide/pyproject_config.html#id3 looks relevant. i think that means it might prefer a [tool.setuptools.dynamic.entry-points] table instead with a console_scripts subtable? | 14:27 |
| fungi | looks like pep 621 is where project.scripts was standardized | 14:38 |
| fungi | fwiw, we may need to duplicate some values between setup.cfg and pyproject.toml to make this work, at least until we reach the point where we can drop setup.cfg/setup.py entirely | 14:40 |
| fungi | though maybe tool.setuptools.dynamic.entry-points is the backdoor to letting pbr fill the console_scripts metadata instead of relying on the project.scripts table? | 14:44 |
| fungi | yeah, in the project where i'm using pbr with a project.scripts table in pyproject.toml, setuptools puts an entry_points.txt in the wheel containing a [console_scripts] block with the mapping of script name to module:function, which pip then uses to add the scripts at installation | 14:49 |
| fungi | so it does at least work that way, though not having to duplicate it between setup.cfg and pyproject.toml would be great | 14:49 |
| fungi | part of the problem, i think, is the notion that the existence of pyproject.toml in a project means most of its metadata is statically defined in the file, and in order to dynamically feed it to setuptools (or any other backend) with a plugin you need to basically state in pyproject.toml that it will be "dynamic" (i need to do that for the version in mine, for example, because pbr | 14:54 |
| fungi | will be determining that rather than it being a fixed string in pyproject.toml) | 14:54 |
| fungi | something like... | 14:55 |
| fungi | [project] | 14:55 |
| fungi | dynamic = ["console_scripts","version"] | 14:55 |
| fungi | (and anything else that's going to live exclusively in setup.cfg or get generated by pbr on the fly at build time) | 14:56 |
| fungi | looks setuptools as of 75.6.0 should also have started emitting warnings about ignoring them when not flagged as dynamic per https://github.com/pypa/setuptools/pull/4012/files | 15:00 |
| fungi | i think the expectation in the python packaging ecosystem is that, if your project is going to have both a pyproject.toml and old-style (setup.py, setup.cfg, requirements.txt...) files, you should duplicate as much as possible into pyproject.toml and then mark the rest as dynamic | 15:02 |
| fungi | in the case of my project, i really only need dynamic = ["version"] in order to allow pbr to generate version strings (otherwise tools are supposed to expect a hard-coded project.version string in pyproject.toml) | 15:04 |
| Clark[m] | fungi: to be clear pip writes out console scripts for us but they are not using the PBR content for those scripts. You think marking console scripts as dynamic will allow pbr/setup tools to write them instead? | 15:24 |
| fungi | how does pbr's version of the scripts differ? | 15:24 |
| Clark[m] | The difference isn't huge. I think in the before times PBR was cutting out pkg_resources slowness | 15:25 |
| fungi | and no, what *should* matter at least is if the package metadata in your sdist and/or wheel have the right console_scripts list | 15:25 |
| Clark[m] | Today they are much more similar | 15:25 |
| fungi | since console_scripts is what pip is looking at to create the scripts | 15:26 |
| Clark[m] | Right but we explicitly don't want pip to create anything for us | 15:26 |
| Clark[m] | It's doing that now and doing it "wrong" | 15:26 |
| fungi | i don't think setuptools/pbr even can create those scripts? where in the wheel would they end up? | 15:26 |
| fungi | unless you mean exclusively in the editable case, maybe that's where setuptools/pbr are on the hook for making the script files | 15:27 |
| Clark[m] | I don't know but it works in testing with setup.py stuff today before I started changing stuff | 15:27 |
| Clark[m] | It works for both editable and not editable installs prior to my changes | 15:27 |
| fungi | when i was testing using pbr with pyproject.toml files in the past, i basically created sdists and wheels both with and without, and then diffed the resulting metadata file contents | 15:28 |
| Clark[m] | I'm going to push an update to testing that adds pip instead of replacing setup.py as a base change and we can compare/build from there | 15:29 |
| fungi | setuptools/pbr don't get executed at install time, only at build time. the distinction between build and install is a little fuzzy when "installing" a source tree or sdist, but for wheels there is a pretty clear separation (and modern pip tries to enforce that by building a wheel and then installing the wheel) | 15:29 |
| fungi | so all the data pip is going to be acting on in the wheel install case has to be contained inside the wheel, and there is nowhere in a wheel that i'm aware of to record full entrypoint script contents (only the mapping of name to function) | 15:31 |
| clarkb | I see so this may be a consequence of changing from setup.py to pip (because setuptools isn't available on python3.12 by default) and not a switch from setup.cfg and setup.py metadata to pyproject.toml metdata | 15:33 |
| fungi | entirely possible | 15:33 |
| clarkb | give me a few minutes to write this change and load ssh keys and CI should confirm for us then we can decide what an appropriate course of action will be (I'm thinking accept it and move on may be appropriate) | 15:33 |
| clarkb | fungi: pbr commit 8e58c2fa58fd1aa6f9985dcb4e210508a73e1df7 says that the console scripts were replaced because the defaults are complex and slow | 15:46 |
| clarkb | and it points at entry poitns as the problem which seems to align with what I suspected | 15:46 |
| clarkb | I guess importantly if the behavior is different between the two paths we've probably already been relying on the pip supplied script content without knowing it and that has worked for us. Could still be slow I suppose but would be functional | 15:49 |
| clarkb | fungi: look like pip install -e gets you the PBR generated console script but pip install does not in the no pyproject.toml case. The updates I made to add pyproject.toml switch to pip supplied script for both cases (whcih makes sense now that pep660 and editable wheel installs can occur with pyproject.toml) | 15:58 |
| clarkb | https://zuul.opendev.org/t/openstack/build/30d08bf1a1b440b5bccfb09ea0790549 | 15:58 |
| clarkb | considering that is the existing behavior whether pbr likes it or not I think I'll go ahead and encode that into the updated test so that we can land it and at least start enforcing things so we detect behavior changes in the future | 15:59 |
| fungi | yeah, that was my suspicion. pip install (no -e) builds a wheel and then installs just what's in that, so loses any chance for setuptools/pbr to decide what the internals of the script look like | 16:18 |
| clarkb | the pip script is almost identical to the pbr one too | 16:19 |
| clarkb | a big change from what setuptools is/was generating with lots of entry point magic iirc | 16:19 |
| clarkb | ya thinking about this anyone using pip recently would've gotten this behavior anyway (and by recently I mean for at least a few years) | 16:21 |
| clarkb | so I think from our perspective its ok to accept that behavior, test that it doesn't change (so that we know if it does) and not worry about it too much | 16:22 |
| fungi | right, i think editable installs are already enough of a corner case that any regression in performance which makes them closer to regular installs is acceptable (maybe even preferable) | 16:32 |
| clarkb | fungi: the major question I've got is whether or not it is safe to put six>=1.16.0 in requirements.txt for pbr. I've limited the setuptools installation there to python3.12 and newer. I susppose I could do similar for six and then continue to manually install six where we need it elsewhere? | 17:16 |
| clarkb | the problem is pbr does actually rely on six and it doesn't seem to be consistently available at this point like it was in the past so I'm trying to be more explicit about it | 17:17 |
| clarkb | in theory it works with python2.7 and the python3 ersions we do test though so I think it is safe | 17:17 |
| clarkb | theory backed with our testing I mean | 17:17 |
| fungi | what is pbr still using six for? | 17:27 |
| fungi | as a bridge for its remaining 2.7 support? | 17:27 |
| clarkb | hrm looks like its mostly in testing with one location in the sphinx integration for a config parser import? | 17:28 |
| clarkb | we can probably just remove six | 17:28 |
| clarkb | I think that is a better idea. remove six, then redo packaging and testing | 17:29 |
| fungi | didn't we also drop the sphinx integration bits? or no i guess we just dropped the sphinx building wrapper? | 17:30 |
| clarkb | ya its just the command that got dropped iirc | 17:30 |
| fungi | right, because it had to be called as a setup.py subcommand and directly calling setup.py is deprecated | 17:31 |
| fungi | looks like the current patchset for 938332 is going to work everywhere besides noble | 18:26 |
| clarkb | yup I have a fix queued up locally that also rebases the entire stack on top of the six removal. I want zuul to post reuslts though so they are easier to find in the future so I'm waiting | 18:27 |
| fungi | looks like it has to do with missing setuptools in newer python venvs? | 18:30 |
| clarkb | yup I was trying to remove the use of setuptools from as many locations as possible to exercise teh inclusion of setuptools via requirements.txt and pyproject.toml. but in that case we're invoking setup.py directly so it doesn't work. I'm just going to add that case back in | 18:33 |
| clarkb | the entire test case could really be rewritten/reimagined with modern tools | 18:33 |
| clarkb | fungi: ok new patchsets pushed. I think this might be happy now. Then its a matter of determining if we are hapyp with it as reviewers | 18:36 |
| fungi | having skimmed all 4 up to this point, i think i am, but will give them a closer look once known passing | 18:37 |
| clarkb | the first two changes in the stack should be pretty safe. They are just improving what is already there without changing behaviors. The second two potentially change runtime behavior | 18:40 |
| clarkb | oh there is the fifth change to correct the testing of pbr changes in the integration tests too | 18:43 |
| clarkb | that one should also be safe | 18:43 |
| fungi | yeah, i already reviewed that one a while back | 18:43 |
| fungi | do these need rebasing onto it to ensure testing is complete? | 18:43 |
| clarkb | fungi: the last change in the stack depends on it | 18:44 |
| fungi | ah, good, i missed that it was using depends-on | 18:44 |
| clarkb | which I think is probably sufficient coverage if we look at things as a whole | 18:44 |
| fungi | agreed | 18:44 |
| clarkb | gerrit_file_diff and git_file_diff caches for gerrit are eachin the ~20GB size range for the backing file now | 19:06 |
| clarkb | still quite a bit smaller than the one extra large file we had previously. I'm beginning to wonder if we should detune our cache sizes so that daily cachine pruning has more room to trim which might keep the files smaller? | 19:07 |
| clarkb | when it isn't holiday hangover time we should also apply the update to compact the files on shutdown | 19:08 |
| clarkb | fungi: reading your comments how do you install pip into a venv without it being preinstalled? I'm guessing there is some magic with get-pip.py running against the venv python? | 19:34 |
| clarkb | and ya I think testing with build and wheel are good ideas but maybe as followups? | 19:35 |
| fungi | agreed, i don't think they need to be included in that change, just brainstorming | 19:36 |
| fungi | as for installing pip, these days you can run it without "installing" by downloading and executing its zipapp directly from python | 19:37 |
| clarkb | I'll clean things up a bit as far as comments and commit messages explaining things go after lunch today. I think adding more tools and updating mkvenv to install pip would be fine as followups but don't seem super necesasry in this first pass | 19:38 |
| clarkb | also checking the resulting pbr wheel is a good idea | 19:38 |
| clarkb | it does seem to work glancing at the integration testing job logs. We install a dev version of pbr from source that then functions to enable setup.py things to run against test-project | 19:38 |
| clarkb | but confirming directly is a good idea | 19:39 |
| fungi | as for a pip zipapp, you can find various versions (also virtualenv) under https://bootstrap.pypa.io/ | 19:39 |
| fungi | cpython can treat a pyz file like an installed module | 19:40 |
| fungi | worth thinking about in the future since it might be simpler than get-pip.py | 19:40 |
| fungi | https://python.readthedocs.io/en/stable/library/zipapp.html for the basic functionality. does work with 3.6 (though there's likely not an old enough pip.pyz for 3.6 support), but not with 2.7 | 19:42 |
| fungi | yeah, pip-22.3.1.pyz is the oldest available and that requires_python>=3.7 | 19:44 |
| fungi | all 5 changes lgtm, i left comments/questions on some but they look fine to merge | 19:51 |
| fungi | and barring any last-minute failures in the tempest-full jobs it seems like zuul will agree | 19:52 |
| fungi | clarkb: i've run build in both the tip of master and a checkout of 938332 and am starting to compare them for differences | 19:57 |
| fungi | first thing i notice is that the before sdist includes a pbr.egg-info/requires.txt while the after is missing it, but the after sdist has a requirements.txt that wasn't in the before | 19:58 |
| fungi | https://paste.opendev.org/show/bJ253Xx1CYFxIiVgU7xe/ | 20:00 |
| fungi | no, i misread. both the pbr.egg-info/requires.txt and requirements.txt are present, i suppose that's because of the setuptools addition | 20:01 |
| fungi | both are present in the after and not the before, i mean | 20:01 |
| fungi | yeah, pbr.egg-info/requires.txt just contains "setuptools" while requirements.txt contains "setuptools;python_version>='3.12'" so those can be attributed as expected outcomes of 938332 | 20:04 |
| fungi | the wheel file lists for before and after are identical, adjusting for where the version string gets embedded in the dist-info directory name | 20:10 |
| fungi | diffing the content of the metadata files between the wheels: https://paste.opendev.org/show/bxjvHTkMvglHyiPzodDZ/ | 20:12 |
| fungi | the only thing that's mildly concerning is the Generator field in the WHEEL file, which seems like we're somehow making setuptools think the pbr version is the version of setuptools | 20:13 |
| fungi | i wonder if we're shadowing or unintentionally monkeypatching one of its methods | 20:13 |
| fungi | but all things considered it's extremely minor | 20:13 |
| fungi | https://paste.opendev.org/show/b2y7S3WZfoNpV4TEEKr7/ is a similar diff of the sdist metadata files and the only oddity i see is the addition of AUTHORS as a license-file entry but i think that's setuptools' automated license file finder coming into play | 20:20 |
| fungi | overall i'm satisfied we're not introducing any breakage to the package contents or metadata with this change, though it might still be a good to figure out why the wheel's "generator" has pbr's version listed instead of setuptools' | 20:22 |
| fungi | i do have a separate concern looking at these, but it already exists in master: we're declaring "Metadata-Version: 2.1" but have fields that aren't valid until 2.4 (e.g. license-file) | 20:24 |
| fungi | https://packaging.python.org/en/latest/specifications/core-metadata/#license-file-multiple-use | 20:25 |
| fungi | however, i gather pypi/warehouse is currently rejecting metadata 2.4 uploads, so that might be for the best | 20:25 |
| fungi | anyway, all 5 changes are green in zuul and have my +2 | 20:28 |
| Clark[m] | Cool I do want to make some of your suggested edits once I've finished eating. Re the PBR version could it be due to having pbr essentially hand over to setup tools? So really it is setup tools generating for us? | 20:46 |
| fungi | well, it's claiming "Generator: setuptools (6.1.1.dev5)" in the new wheel | 20:52 |
| fungi | whereas the old wheel had "Generator: setuptools (75.6.0)" | 20:52 |
| fungi | but 6.1.1.dev5 is the version of pbr, not the version of setuptools | 20:53 |
| fungi | i don't think pbr is expressing that metadata field directly either way | 20:54 |
| fungi | but the wheel library is determining that setuptools is the metadata generator, yet for some reason finding the version string for pbr rather than setuptools | 20:55 |
| fungi | might be a bug in build/wheel, or it could be a sign that pbr is unintentionally shadowing setuptools' version string internally | 20:56 |
| clarkb | fungi: I just pushed some updates based on your feedback. Thanks | 21:22 |
| fungi | yep, i'm already checking the diffs. thanks! | 21:22 |
| clarkb | I'm not seeing anything obvious in setuptools that produces the Generator metadata. (it could still bein there and I'm missing it). I suspect that pip or build or whatever is running the pep517/pep660 process is producing that? | 21:27 |
| fungi | i think wheel sets it, but i'm not certain as i haven't started looking yet | 21:28 |
| clarkb | my hunch is that the pep517 hooks (or maybe less formal expectations?) get the generator via the hook so get setuptools since we're just a shim to setuptools but then when asking for the version do something like footool.version or whatever and get the pbr version? | 21:29 |
| clarkb | but ya we'd need to find what produces it first to understand that better. wheel does seem like a likely candidate | 21:30 |
| fungi | yeah, whatever it is, it's most likely being naive about deciding what to query for the version | 21:30 |
| fungi | oh, yes https://pypi.org/p/pyproject-hooks is also a possibility | 21:31 |
| clarkb | oh ok maybe setuptools is doing it. I dismissed the email related content but apparently they use that as a stdlib hack to get headers into a file | 21:37 |
| clarkb | self, wheelfile_base: str, generator: str = f"setuptools ({__version__})" | 21:38 |
| clarkb | I dont' see pbr setting __version__ though | 21:39 |
| fungi | i think __version__ may get automagically set under some circumstances | 21:44 |
| fungi | in theory there's supposed to be a distinction between the "dist package" version and the "import package" version reported by importlib, but pbr has traditionally equated the two for simplicity/consistency | 21:45 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!