Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow staging installs to an alternate directory #1351

Open
bkabrda opened this issue Nov 29, 2013 · 33 comments
Open

Allow staging installs to an alternate directory #1351

bkabrda opened this issue Nov 29, 2013 · 33 comments
Labels
type: maintenance Related to Development and Maintenance Processes

Comments

@bkabrda
Copy link

bkabrda commented Nov 29, 2013

Hi,
as noted in #991 (comment), 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.

@qwcode
Copy link
Contributor

qwcode commented Nov 30, 2013

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?

@qwcode
Copy link
Contributor

qwcode commented Nov 30, 2013

marking for v1.5 to agree on a solution.

@ncoghlan
Copy link
Member

ncoghlan commented Dec 2, 2013

@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.

@bkabrda
Copy link
Author

bkabrda commented Dec 3, 2013

So to answer all the questions in one comment:

  • Our builders could of course use custom RPM hooks to alter RECORD properly, but I don't like this solution. Almost all the build tools that I've seen so far had an option to strip the prefix (or more generally to alternate the built-in paths in some ways). It's precisely as @ncoghlan says - relying on more tools misses the decoupling benefit.
  • I understand that my patch seems to bring some unnecessary clutter and there might be better solutions - Nick's one is nice.
    (Just BTW is there any eta of 1.5 release?)

@dstufft
Copy link
Member

dstufft commented Dec 3, 2013

So this'll make things similar to --prefix and DESTDIR in a Makefile I suppose?

@ncoghlan
Copy link
Member

ncoghlan commented Dec 3, 2013

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.

@qwcode
Copy link
Contributor

qwcode commented Dec 4, 2013

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)

@bkabrda
Copy link
Author

bkabrda commented Dec 4, 2013

--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.

@ncoghlan
Copy link
Member

ncoghlan commented Dec 4, 2013

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

@ncoghlan
Copy link
Member

ncoghlan commented Dec 4, 2013

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.

@qwcode
Copy link
Contributor

qwcode commented Dec 4, 2013

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...

@bkabrda
Copy link
Author

bkabrda commented Dec 4, 2013

@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.

@dstufft
Copy link
Member

dstufft commented Dec 5, 2013

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.

@ncoghlan
Copy link
Member

ncoghlan commented Dec 5, 2013

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.

@bkabrda
Copy link
Author

bkabrda commented Mar 19, 2014

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!

@qwcode
Copy link
Contributor

qwcode commented Apr 13, 2014

related issue: #1716

@dholth
Copy link
Member

dholth commented Apr 14, 2014

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.

@dstufft
Copy link
Member

dstufft commented Dec 21, 2014

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.

@dstufft
Copy link
Member

dstufft commented Dec 21, 2014

Oh, to be clear, I plan on implementing that tomorrow (Sunday).

@qwcode
Copy link
Contributor

qwcode commented Dec 21, 2014

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.

@ncoghlan
Copy link
Member

+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.

@dstufft
Copy link
Member

dstufft commented Dec 21, 2014

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.

@dstufft
Copy link
Member

dstufft commented Dec 22, 2014

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!

@dstufft dstufft modified the milestones: 7.0, 6.0 Dec 22, 2014
@bkabrda
Copy link
Author

bkabrda commented Jan 5, 2015

No problem @dstufft... and I actually like the --stage-dir idea better, too. So let's just go on with that!

@ncoghlan
Copy link
Member

ncoghlan commented Jan 6, 2015

Could the title on this issue be updated to reflect the --stage-dir idea?

@dstufft dstufft changed the title Allow stripping given prefix from files in wheel RECORD Allow staging installs to an alternate directory Jan 6, 2015
@dstufft
Copy link
Member

dstufft commented Jan 6, 2015

Done.

@dstufft dstufft modified the milestones: 7.1, 7.0 May 21, 2015
@dstufft dstufft modified the milestones: 7.1, 7.2 Jun 30, 2015
@dstufft dstufft modified the milestones: 8.1, 7.2 Jan 18, 2016
@dstufft dstufft removed this from the 8.1 milestone Mar 3, 2016
@pradyunsg
Copy link
Member

@dstufft Should I add this to the 10.0 milestone?

@pradyunsg pradyunsg added resolution: deferred till PR Further discussion will happen when a PR is made type: enhancement Improvements to functionality labels Aug 17, 2017
@pradyunsg
Copy link
Member

pradyunsg commented Aug 19, 2017

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.

@hroncok
Copy link
Contributor

hroncok commented Feb 10, 2019

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.

@PatrikKopkan
Copy link

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

@hroncok
Copy link
Contributor

hroncok commented Sep 2, 2019

From Fedora perspective, feel free to close this. As @PatrikKopkan says, the paths are relative.

@chrahunt chrahunt added type: maintenance Related to Development and Maintenance Processes and removed resolution: deferred till PR Further discussion will happen when a PR is made type: enhancement Improvements to functionality labels Sep 3, 2019
@chrahunt
Copy link
Member

chrahunt commented Sep 3, 2019

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.

@hroncok
Copy link
Contributor

hroncok commented Sep 3, 2019

Great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: maintenance Related to Development and Maintenance Processes
Projects
None yet
Development

No branches or pull requests

9 participants