patch setuptools to allow for parallel usage

[ohanar]

Currently we have to force the spkgs that use setuptools to build serially because setuptools does not do any file locking with easy_install.pth. This is an inconvenience that has a fairly straightforward fix.

Depends on #11874
Depends on #12994

CC: @jdemeyer @kini @jasongrout

Component: build

Author: Volker Braun

Branch: b276471

Reviewer: R. Andrew Ohana

Issue created by migration from https://trac.sagemath.org/ticket/13201

[ohanar]

Author: R. Andrew Ohana

[jdemeyer]

comment:2

Why re-invent the wheel? Why not use http://docs.python.org/library/fcntl.html#fcntl.lockf?

Also, this should obviously be reported upstream.

[ohanar]

comment:3

Replying to @jdemeyer:

Why re-invent the wheel? Why not use http://docs.python.org/library/fcntl.html#fcntl.lockf?

Because I was being dumb and couldn't find that function :).

Also, this should obviously be reported upstream.

Yup, although upstream is basically dead. IMO we should switch to distribute (I'll see about making an SPKG).

[ohanar]

new patch applied to setuptools; for review purposes

[ohanar]

Attachment: easy_install_lock.patch.gz

Attachment: trac13201.patch.gz

apply to root repo

[ohanar]

Changed dependencies from #11874 to #11874, #12994

[jdemeyer]

comment:6

I unfortuntely don't understand setuptools well enough to review this. It seems there are some changes unrelated to the locking. What do they do?

Also, do we really need to do this every time Sage runs (could it be moved to sage-location?):

    # Hack around setuptools since --egg-path isn't fully respected 
    sed -i 's-.*sagenb.*-\.\./\.\./\.\./\.\./devel/sagenb-' \ 
        "$SAGE_LOCAL/lib/python/site-packages/easy-install.pth" 

You also need to handle the case that $SAGE_ROOT isn't writeable.

[ohanar]

comment:7

Replying to @jdemeyer:

I unfortuntely don't understand setuptools well enough to review this. It seems there are some changes unrelated to the locking. What do they do?

Internally setuptools identifies packages based on a normalized path (no absolute paths without symlinks, and a normalized case for case-insensitive filesystems). It reads these from easy-install.pth, and then creates a brand new easy-install.pth if it detects that it needs to. The issue is that it loads the list of packages when it starts, and then writes it back when it finishes, if any changes were made to the file in the meantime, setuptools doesn't detect them, and just overrides them with its new easy-install.pth. What I did was add a bit of code for reloading the file right before writing a new one -- making sure things that were deleted don't pop back up, and making sure things that were added don't disappear.

Also, do we really need to do this every time Sage runs (could it be moved to sage-location?):

    # Hack around setuptools since --egg-path isn't fully respected 
    sed -i 's-.*sagenb.*-\.\./\.\./\.\./\.\./devel/sagenb-' \ 
        "$SAGE_LOCAL/lib/python/site-packages/easy-install.pth" 

You also need to handle the case that $SAGE_ROOT isn't writeable.

Well no you don't have to run it every time sage starts, just every time a package using setuptools is installed, since setuptools always inscribes the absolute path into easy-install.pth (even if you specify a relative path for --egg-path).

[jdemeyer]

comment:8

Since sage-location specifically deals with rewriting paths, I would do it there.

[ohanar]

comment:9

I thought that was for rewriting paths when SAGE_ROOT changed. The issue is that setuptools rewrites paths, regardless of what happens to SAGE_ROOT, so we have to overwrite their rewrites. An alternative (now that we have the flask notebook has been merged), is to treat sagenb like any other upstream package, and just install it in site-packages (I don't think the sagenb developers would be opposed to this, we are already planning on doing this in the transition to git).

[jdemeyer]

comment:10

Replying to @ohanar:

I thought that was for rewriting paths when SAGE_ROOT changed.

Well, the reason you need to rewrite the setuptools paths is to allow relocation, right? (or am I missing something?)

[ohanar]

comment:11

Replying to @jdemeyer:

Well, the reason you need to rewrite the setuptools paths is to allow relocation, right? (or am I missing something?)

Well we also support clones for sagenb, which will brake every time setuptools is run (it readlinks everything). If we don't care about that, then we could.

I'm CCing Keshav and Jason on this ticket, they might have an opinion on how we handle this (since currently it only affects sagenb).

[jasongrout]

comment:12

Since sagenb is now distributed without a repository, it might make sense for it not to be installed in SAGE_ROOT/devel/. The reason for it to be in SAGE_ROOT/devel/sagenb is so that the barrier is way lower for people wanting to patch it and work with it. If we're making them clone the git repository anyway, the barrier isn't lower.

So what do you think, kini? Should we just install it like a normal spkg? If they want to develop, they need to git clone and do sage setup.py develop?

(personally, I'm mildly in favor of going back to installing the git repo in SAGE_ROOT/devel/sagenb...)

[kini]

comment:13

Absolutely. I 100% think that sagenb should be installed like any other python package, i.e. in the site packages directory.

[jdemeyer]

comment:14

In any case, this needs work because it assumes that $SAGE_ROOT is writable and it needs to be rebased.

[vbraun]

comment:23

New attempt... The patch didn't apply, so I wrote a different version. It is now IMHO clearer though perhaps at the cost of rewriting a pth file even if it is not dirty. But then it is hardly a performance-critical step.

[vbraun]

comment:24

We can't push the changes upstream since fcntl is unix only but setuptools has to work on (non-cygwin) windows

[vbraun]

comment:25

Critical as we currently have some races as I noticed in parallel testing.

[ohanar]

Changed author from Volker Braun, R. Andrew Ohana to Volker Braun

[ohanar]

comment:26

Looks fine, although I'll have to test it out. It would be good to get some sort of proper locking in upstream -- maybe doing something along the lines of http://code.activestate.com/recipes/65203/.

[ohanar]

Reviewer: R. Andrew Ohana

[ohanar]

comment:27

The patch version of setuptools will need a version bump to force a rebuild. Also, it would be good to have the patch documented in the SPKG.txt. Pending those two things, positive review.

[sagetrac-git]

Changed commit from 39481e1 to 25f77ee

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

25f77ee

added documentation, patchlevel bump

[vbraun]

comment:29

Done.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

b276471

resolved merge conflict

[sagetrac-git]

Changed commit from 25f77ee to b276471

[vbraun]

Changed branch from u/vbraun/parallel_setuptools to b276471

[jdemeyer]

Changed commit from b276471 to none

[jdemeyer]

comment:32

You just broke sagenb: #17268