seb-m / pyinotify

Monitoring filesystems events with inotify on Linux.
http://github.com/seb-m/pyinotify/wiki
MIT License
2.29k stars 379 forks source link

`python setup.py egg_info` should run on other platforms #175

Open cscutcher opened 5 years ago

cscutcher commented 5 years ago

This is a bit gnarly, and to be honest I'm not 100% on what's the right thing to do here.

The block in setup.py:

if not platform.startswith('linux') and not platform.startswith('freebsd'):
    sys.stderr.write("inotify is not available on %s\n" % platform)
sys.exit(1)

Will cause any invocation of setup.py to fail on platforms other linux and freebsd. I get this is intentional to prevent users installing the package on platforms it's not suited for. Unfortunately this breaks another use case. Pipenv and I believe other tools will run python setup.py egg_info to get metadata about the package.

In our Pipfile we have;

# Doit requirements
doit = "*"
pyinotify = {version = "*", sys_platform = "== 'linux'"}
macfsevents = {version = "*", sys_platform = "== 'darwin'"}

This will prevent pyinotify being installed on any platform that isn't linux when pipenv sync is executed by using PEP508 to specify the platform dependency . However if pipenv lock is run on a platform that is unsupported, such as OSX, Pipenv (or perhaps indirectly Pip) runs python setup.py egg_info to discover information about the package. Presumably, while it doesn't need to install or run the package, it needs to know enough to be able to update the lock file and so tries to discover the metadata. Unfortunately though this will result in;

pipenv.patched.notpip._internal.exceptions.InstallationError: Command "python setup.py egg_info" failed with error code 1 in /var/folders/_2/mv_rzk9d4lx4cf6v0d187xwc0000gn/T/tmppyzfuvb_build/pyinotify/

I suppose one could make the argument that the fault is with pipenv, but seems reasonable to me that pipenv runs egg_info to discover metadata about a package, and it'd certainly be more difficult to change that end of things.

Potential Solutions

An easy solution would be to just remove the if statement from the setup.py, however that would mean users on unsupported platforms would presumably get a less useful error message. Perhaps you could log a warning instead of sys.exit(1)? This seems to me to be the best compromise but it's obviously your privilege to make that decision.

However I can't see a quick, clean and easy way to ensure that the egg_info can be run but install is prevented.

Unfortunately, while PEP508 specifiers seem to work find for dependencies, I can't see how to use them to define which platforms the package itself can be installed on in a way that ensures tools like pip will obey them. I experimented with the classifiers and platforms args and pip seems to ignore them both.

A more complex solution would be to only raise an exception if install is being attempted on an unsupported platform. I'm not sure exactly the best way to do this, but the distutils docs describe extending commands so perhaps that's one option, or perhaps intercepting the arguments and skipping the block if the command is "egg_info"


I'm happy to contribute via a PR or further debugging if wanted, but obviously I'll wait for your feedback before doing so.

Cheers