Open bkabrda opened 10 years ago
I don't like the clutter of the new option.
install the wheel into /somepath/BUILDROOT/package-1.2.3-1/.
you're using --root=PATH
, right?
If so, you're builder is supplying PATH (and knows the sys.prefix it's using to build with), so it has enough information to understand the absolute script paths in RECORD, and adjust them during the install.
right?
marking for v1.5 to agree on a solution.
@bkabrda Could we handle this as a post-processing step on RECORD while still in the buildroot? That is, trawl through the installed RECORD file and strip the RPM_BUILD_ROOT prefix from the filenames? I guess the disadvantage is that it means we wouldn't be relying solely on pip as the build system at that point - we'd be relying on pip + the script that strips the prefixes, thus missing out on the decoupling benefit that Toshio and I were talking about at Flock.
Perhaps, rather than a --strip-file-prefix
option, an --installed-root
option would be better (and easier to explain)? Then, when writing RECORD, pip would take any path that starts with the root path and replace that portion with the nominated installation root instead.
So for an RPM build, we'd use something like --root $RPM_BUILD_ROOT --installed-root /
instead of --root $RPM_BUILD_ROOT --strip-file-prefix $RPM_BUILD_ROOT
That approach would then cover any situation where the build/unpack location differed from the final installation location, rather than being limited to the specific case of a building a system wide install somewhere else and then moving it into place later the way RPM does.
So to answer all the questions in one comment:
So this'll make things similar to --prefix and DESTDIR in a Makefile I suppose?
It occurs to me that pip install --root=$RPM_BUILD_ROOT --installed-root=/
would be a little weird, since we want pip to actually do the install to --root
as usual. Since we just want to alter the RECORD file contents, how does --recorded-root
sound? Then the command would look something like:
pip install --root=$RPM_BUILD_ROOT --recorded-root=/
I think that accurately reflects want we want to be able to do for RPM builds: tell pip to do the install in a particular location as normal, but also tell it to record the details in the metadata as if it was being done somewhere else.
to be clear, what ends up in RECORD for a --root=/buildroot
install , is something like this:
/buildroot/usr/local/bin/script
i.e. buildroot + sys.prefix + bin/script
and if we used --recorded-root=/recordroot, what I guess we want, is this?
/recordroot/bin/script
if so, that seems fine I guess, but during the RPM build, you know where it's going to be installed for sure?
question becomes whether this option should apply to installed-files.txt
for sdist installs as well (even it's not needed there atm)
--recorded-root
sounds fine to me.
@qwcode When we package an RPM, we know for sure where the files are going to be. RPM is in fact just an archive with some metadata. If during RPM build a file is in /buildroot/usr/bin
, then inside the RPM it will be in /usr/bin
and when installed, it will also end up in /usr/bin
. In other words, both the file/directory hierarchy of RPM buildroot and the file/directory hierarchy inside RPM are the same as what you get after installing the RPM.
One slight correction to @qwcode's comment. If the path in the build root is this:
/buildroot/usr/local/bin/script
Then with --recorded-root=/recordroot
set, I'd expect this:
/recordroot/usr/local/bin/script
Removing the "/usr/local/ part from the installation path within the build root is a separate question and already supported through --install-options - the proposed new option is solely about telling pip "yes, I'm telling you to install it here for now, but I'm going to move it somewhere else before I consider it fully installed".
So to get the RECORD file entry from Marcus's comment, we'd have this:
/buildroot/bin/script # Actual installation
/recordroot/bin/script # RECORD file
If you're curious why RPM works this way, it's to make it easy to build on one machine and deploy on another - on the build machine, you really don't want to be installing into the system directories, but you want to be generating metadata as if you were, since that's where everything will end up on the target system. It's a step beyond the way wheels work - we include all the "install time" generated files in the archive as well.
Explaining that actually made me realise something, though: the same s/^<buildroot>/^<recordroot>/
substitution would likely be needed for the shebang line in any generated scripts as well. sys.prefix
is going to have the buildroot in it, and we don't want that being written out in the generated files.
Removing the "/usr/local/ part from the installation path within the build root is a separate question and already supported through --install-options
I tried fiddling with various combinations of pip install --root
and pip install --install-option='OPTIONS'
for wheels, and didn't come up with a way right off that was working to end up with /buildroot/bin/script
. signing off for now though...
@ncoghlan I don't think that sys.prefix
gets alternated in any way during RPM build. There shouldn't be a problem for the scripts, IMO - at least my test builds seem to be fine.
I think this should be bumped until 1.6, It's not really related to getting PEP453 implemented and it's a new feature in the RC.
Yeah, with the rc already published, I agree this has missed the deadline for 1.5, so we'll need to work around this in the meantime. However, having --recorded-root in 1.6 would make needing such a workaround a temporary situation, rather than a permanent one.
Hey guys, just wanted to bump this. Unfortunately I have no time to provide a better patch for this, so I wanted to ask whether someone's planning to work on this for version 1.6. Thanks!
related issue: #1716
I'm surprised these aren't all relative paths from the location of RECORD? Also in this case the RECORD file is not that useful. It would be helping you to pip-uninstall an rpm-installed package.
Chiming back in on this since I want to get it in for pip 6.0.
Our --root
flag is basically equivalent to the fairly standard --prefix
flag to configure
. So I think the way that we should do this, is just add something similar to the other side of the standard, which is DESTDIR
, so we can add something like --dest-dir
or so. Probably we can even support the DESTDIR
environment variable itself that make uses. This would make the command something like pip install --dest-dir /buildroot/ ...
.
I think this is nicer than needing two flags, one to put it into a different directory, and another one to tell it that you really didn't mean to put it in that other directory.
Oh, to be clear, I plan on implementing that tomorrow (Sunday).
yes, make has DESTDIR, which is the functionality we want here, but I hesitate to take on that name, since people will chronically think its something that it's not, i.e. a way to just install to another location. --stage-dir
might be better, since that's how make describes the DESTDIR feature ("support for staged installs"), and it sounds much less like a usable install.
Also, in case it's not clear, I'd expect --root
to work with --stage-dir
, and I'd expect ROOT to be nested inside STAGEDIR and to be functional prefix for the install, when it's moved out of staging.
+1 from me for "stagedir" or something along those lines - that's precisely the concept of the RPM buildroot, in that we're not installing on the current machine, just using it as a staging system to create the installed layout for inclusion in the built RPM.
Yea I like the name --stage-dir
better than DESTDIR
I think. I only chose that because it's what make used. I totally agree that --root
should work with --stage-dir
.
This is probably not going to make it into 6.0, though it can probably make it into the one after that. I didn't have time to make the --stage-dir
patch and I just looked at #2117 and it only works for Wheels. I'm not really a fan of adding an option which should apply to both sdist and wheels but only actually applies to wheels.
Sadly that means that @bkabrda will need to carry a patch for awhile longer.. Sorry about that!
No problem @dstufft... and I actually like the --stage-dir idea better, too. So let's just go on with that!
Could the title on this issue be updated to reflect the --stage-dir idea?
Done.
@dstufft Should I add this to the 10.0 milestone?
I've labelled this issue as an "deferred till PR".
This label is essentially for indicating that further discussion related to this issue should be deferred until someone comes around to make a PR. This does not mean that the said PR would be accepted - it has not been determined whether this is a useful change to pip and that decision has been deferred until the PR is made.
Just a note: We (Fedora) still carry a patch for --strip-file-prefix
that started this issue 5 years ago. Yet we still mostly use setup.py
directly in RPM packages builds. With PEP 517 gaining more consumers, I believe we will utilize pip
more and revisiting this would gain more priority. That said, I still need to discuss how we'll adapt RPM packaging of PEP 517 enabled projects within Fedore before I'll have time to send any PRs here.
Hi, When I was trying to use pip installwith --root option, I didn't get absolute paths in a RECORD file as the poster of this issue. The paths in RECORD file are relative to the path given to --root option. Which solves posters and my problem having paths pointing outside of root directory. I want to ask if this will keep behaving like this? And why suddenly it works?
building: python3 -m pip wheel --no-deps --use-pep517 --no-build-isolation --progress-bar off --verbose .
installing: python3 -m pip install ./blurb-1.0.7-py3-none-any.whl --root --root /home/pkopkan/Documents/bordel/blurb/root/ --no-deps --progress-bar off --verbose
resultant RECORD:
../../../bin/blurb,sha256=SDKw7s_-fWAyEqWePm87cD8zGOHbkNSX8QkJ_MhrRpA,207 __pycache__/blurb.cpython-37.pyc,, blurb-1.0.7.dist-info/INSTALLER,sha256=zuuue4knoyJ-UwPPXg8fezS7VCrXJQrAP7zeNuwvFQg,4 blurb-1.0.7.dist-info/LICENSE.txt,sha256=0qEoBW33WzYSBxQigWtjP_Z01F0RZAjhAaAEd0mWFME,1588 blurb-1.0.7.dist-info/METADATA,sha256=a1mpHxh9HCnL5U2_ay1XZiXeg0ad6z0lgg7oczjVH3Y,9532 blurb-1.0.7.dist-info/RECORD,, blurb-1.0.7.dist-info/WHEEL,sha256=JXk7EE_UnY8Q4113Zu8f6SlrMizLH61VvvtIzqzkSKE,79 blurb-1.0.7.dist-info/entry_points.txt,sha256=Upsmaj7QHFDInnb-tOY8IlR5JafnUDmBBpOIQYzS9MM,36 blurb.py,sha256=GOHnNsU0aZs4Q3yQPWm0REoIg3dubTwdS-_WNmT5AYI,56100
built rpm
tested on blurb-1.0.7
From Fedora perspective, feel free to close this. As @PatrikKopkan says, the paths are relative.
Thanks @hroncok. I'll leave this open to track ensuring there's a test covering this case and optionally identifying the commit introducing the behavior change.
Great!
Hi, as noted in https://github.com/pypa/pip/issues/991#issuecomment-29504200, I'm currently trying to adapt pip to be acceptable as a tool used to install wheels during RPM build. Apart from the mentioned issue, I also have one more problem: During RPM build, we install the wheel into /somepath/BUILDROOT/package-1.2.3-1/. This turns out to be problematic with, since for entry_points/console_scripts, the whole path gets into wheel RECORD, e.g. /somepath/BUILDROOT/package-1.2.3-1/usr/bin/somebin. This is of course problem, since when the RPM gets installed, the file is in standard /usr/bin directory. Therefore I created a patch that adds "--strip-file-prefix" option to "pip install", that allows specifying arbitrary prefix path that will get strip from script paths. It's at my fork at https://github.com/bkabrda/pip/commit/aefacbb76661520415a1c35028f2984e70cfe0bf. Would this be acceptable for you? If not, is there a way to make it acceptable?
Thanks!
Edit: Later on in this thread we've settled on using --stage-dir to stage installations to a different directory.