Use a realpath for the temporary build directory. #3079

zvezdan · 2015-09-06T06:23:10Z

Some systems have /tmp symlinked which confuses custom builds, such as
numpy. This ensures that real path is passed and that such builds
resolve their paths correctly during build and install.

Added test for the change and also for the previous related fix:
#707

zvezdan · 2015-09-06T06:45:57Z

Let me provide more information and rationale for this change and tests.
Apparently, this bug resurfaced multiple times in pip history:
#707

The last fix in version 6.* still didn't fix the problem.
I recently debugged an issue that showed up only on build machines while it never showed on my Linux desktop. The symptom was that scipy couldn't find *.ini files from the successfully built numpy package because they were missing from the expected location.
After examining the artifacts and debugging the build I found out that the numpy gets built either in a virtual environment or in a wheel as follows:

...
/path/to/venv_or_wheel/numpy/core/somefile.py
...
/path/to/venv_or_wheel/numpy/core/somedir
...
path/to/venv_or_wheel/export/tmp/pip-sb4Nm5-build/numpy/core/lib/npy-pkg-config/npymath.ini
... another file like this
... one library from lib missing

These files should have been in

/path/to/venv_or_wheel/numpy/core/lib/...

While debugging the build I also observed that the paths for these *.ini files were misconfigured early on during the requirements setup. This later resulted in messages during the installation that copied them into .../export/tmp/pip-sb4Nm5-build/numpy/core/lib/... instead of .../numpy/core/lib/.... The library libnpymath.a was simply copied directly back into the temporary directory /export/tmp/pip-... instead of .../numpy/core/lib/libnpymath.a

The build machines had tmp -> /export/tmp. :-)

This fix in pip allows numpy to build without any issues and correctly package the core lib.
After that the scipy builds without any issues because it's able to find the *.ini files and the library.

zvezdan · 2015-09-06T06:50:46Z

I forgot to add that it would be really great if this small patch could be released as part of pip-7.1.3 bug-fix release instead of waiting for 8.*. It seems to be critical for people who depend on numpy and scipy and have build systems with symlinks. Thanks!

dsully · 2015-09-09T23:59:24Z

+1 for this fix.

zvezdan · 2015-09-23T07:30:00Z

Can someone please merge this upstream?
Do you want me to rebase and resolve the conflict in CHANGES.txt first?
If yes, what approach does pip usually use: (a) a clean rebase with a forced push to this branch with PR, or (b) merge of upstream with recorded merge commit.
Looking at the history of pip commits it seems that (a) is preferred and that would be my preference too.
Please, let me know and lets get this change in to help people who build numpy on system with symlinked tmp directory.

Ivoz · 2016-02-21T02:08:02Z

pip/req/req_install.py

@@ -312,7 +312,13 @@ def build_location(self, build_dir):
            # package is not available yet so we create a temp directory
            # Once run_egg_info will have run, we'll be able
            # to fix it via _correct_build_location
-            self._temp_build_dir = tempfile.mkdtemp('-build', 'pip-')
+            #
+            # NOTE: Some systems have /tmp as a symlink which confuses


Since the above comment is already a note, it makes little sense to demarkate this paragraph specifically as one.

Do you want me to:

remove the comment completely?

remove the marker "NOTE: " only?

merge the comment with the one above on line 314?

Let me know, I can do whatever is preferred here.
It's important to draw attention to the reason for realpath, though, to avoid future regressions. This bug resurfaced multiple times through pip history and we want to avoid that. :-)

Ivoz · 2016-02-21T02:09:58Z

I'm wondering why numpy shouldn't be fixing its custom build system getting confused by such a mundane feature as symlinks, but instead pip should be making allowances for it?

zvezdan · 2016-02-21T04:37:32Z

@Ivoz
I completely agree with you that numpy needs to address their custom building approach and make it more robust. I am looking at this only as someone who needs to ensure stable builds of large Python apps that consume numpy as a dependency.

That said, I still believe this needs to be fixed in pip for the following reasons:

As described in the comments above this bug had been fixed before and then regressed again (see Pip 1.2.x fails to completely install NumPy (OSX global install) #707). Since this is a regression in pip code, I believe we should fix it.
The consumers of numpy (even if they fix numpy build) can still widely use older versions of packages for whatever reason and be affected by this problem. Since problem is so easily fixable in pip, again, it's a good place to apply the fix.
Finally, I do believe that returning realpath() in this spot is the right thing to do.

I stumbled upon this issue when I was approached for help by a developer whose app used to work perfectly fine with the older build that used older version of pip and then suddenly exhibited strange breakage in scipy/numpy dependency. To make things more difficult to debug, the build was working perfectly fine on our developer workstations and failing only on automated build machines. It took a while to track down what really happened and the issue #707 provided the clue.

I hope you'll agree with me and accept the patch.

Thanks a lot for reviewing!

I'll rebase from develop and address your code comment now.

zvezdan · 2016-03-05T21:56:21Z

Can we merge this fix for regression of #707 now?
It's rebased from develop and passes all the tests.

srholmes · 2016-03-07T08:41:10Z

+1 for this

dstufft · 2016-05-18T11:25:55Z

Accidentally closed this, reopening. Sorry!

BrownTruck · 2016-05-19T11:58:11Z

Hello!

As part of an effort to ease the contribution process and adopt a more standard workflow pip has switched to doing development on the master branch. However, this Pull Request was made against the develop branch so it will need to be resubmitted against master. Unfortunately, this pull request does not cleanly merge against the current master branch.

If you do nothing, this Pull Request will be automatically closed by @BrownTruck since it cannot be merged.

If this pull request is still valid, please rebase it against master (or merge master into it) and resubmit it against the master branch, closing and referencing the original Pull Request.

If you choose to rebase/merge and resubmit this Pull Request, here is an example message that you can copy and paste:

Some systems have /tmp symlinked which confuses custom builds, such as
numpy. This ensures that real path is passed and that such builds
resolve their paths correctly during build and install.

Added test for the change and also for the previous related fix:
#707

---

*This was migrated from pypa/pip#3079 to reparent it to the ``master`` branch. Please see original pull request for any previous discussion.*

Some systems have /tmp symlinked which confuses custom builds, such as numpy. This ensures that real path is passed and that such builds resolve their paths correctly during build and install. Added test for the change and also for the previous related fix: pypa#707 --- *This was migrated from pypa#3079 to reparent it to the ``master`` branch. Please see original pull request for any previous discussion.*

zvezdan · 2016-05-19T23:28:47Z

Re-parented from master and submitted a new pull request: #3701
Closing this one as required.

Some systems have /tmp symlinked which confuses custom builds, such as numpy. This ensures that real path is passed and that such builds resolve their paths correctly during build and install. Added test for the change and also for the previous related fix: #707 --- *This was migrated from #3079 to reparent it to the ``master`` branch. Please see original pull request for any previous discussion.*

We are using a custom version that fixes the build of numpy until the upstream project releases our pull request: pypa/pip#3079 RB=563664 G=python-foundation-reviewers R=sholsapp A=sholsapp

zvezdan force-pushed the numpy-build-fix branch from 731545e to 9e97761 Compare February 20, 2016 13:15

zvezdan mentioned this pull request Feb 20, 2016

docs conf.py intersphinx mapping causes build failure in Travis CI #3499

Closed

Ivoz reviewed Feb 21, 2016
View reviewed changes

zvezdan force-pushed the numpy-build-fix branch from 9e97761 to 42b64e4 Compare February 21, 2016 04:43

zvezdan force-pushed the numpy-build-fix branch from 42b64e4 to f1fcf87 Compare March 5, 2016 21:24

zvezdan force-pushed the numpy-build-fix branch from f1fcf87 to 227e8c7 Compare March 30, 2016 07:28

dstufft closed this May 18, 2016

dstufft reopened this May 18, 2016

BrownTruck added the asked to reparent label May 19, 2016

zvezdan force-pushed the numpy-build-fix branch from 227e8c7 to e59301a Compare May 19, 2016 23:21

zvezdan force-pushed the numpy-build-fix branch from e59301a to 5958656 Compare May 19, 2016 23:23

zvezdan mentioned this pull request May 19, 2016

Use a realpath for the temporary build directory. #3701

Merged

zvezdan closed this May 19, 2016

dstufft added migrated to master and removed asked to reparent labels May 20, 2016

lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 3, 2019

lock bot locked as resolved and limited conversation to collaborators Jun 3, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use a realpath for the temporary build directory. #3079

Use a realpath for the temporary build directory. #3079

zvezdan commented Sep 6, 2015

zvezdan commented Sep 6, 2015

zvezdan commented Sep 6, 2015

dsully commented Sep 9, 2015

zvezdan commented Sep 23, 2015

Ivoz Feb 21, 2016

zvezdan Feb 21, 2016

Ivoz commented Feb 21, 2016

zvezdan commented Feb 21, 2016

zvezdan commented Mar 5, 2016

srholmes commented Mar 7, 2016

dstufft commented May 18, 2016

BrownTruck commented May 19, 2016

zvezdan commented May 19, 2016

Use a realpath for the temporary build directory. #3079

Use a realpath for the temporary build directory. #3079

Conversation

zvezdan commented Sep 6, 2015

zvezdan commented Sep 6, 2015

zvezdan commented Sep 6, 2015

dsully commented Sep 9, 2015

zvezdan commented Sep 23, 2015

Ivoz Feb 21, 2016

Choose a reason for hiding this comment

zvezdan Feb 21, 2016

Choose a reason for hiding this comment

Ivoz commented Feb 21, 2016

zvezdan commented Feb 21, 2016

zvezdan commented Mar 5, 2016

srholmes commented Mar 7, 2016

dstufft commented May 18, 2016

BrownTruck commented May 19, 2016

zvezdan commented May 19, 2016