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

After installation the shebang of bin/git-{cola,dag} is replaced to non portable /usr/bin/python #850

Closed
Lin-Buo-Ren opened this Issue Jun 29, 2018 · 3 comments

Comments

Projects
None yet
2 participants
@Lin-Buo-Ren
Contributor

Lin-Buo-Ren commented Jun 29, 2018

This breaks portability where the python interpreter isn't installed in /usr/bin, including but not limited to the snap runtime environment which is located at /snap/snap_name/current/usr/bin/python

It should be kept the same with the original one, which uses /usr/bin/env to locate the proper intepreter location

@davvid

This comment has been minimized.

Show comment
Hide comment
@davvid

davvid Jun 30, 2018

Member

We used to hack around distutils, which is where the replacement happens.

Take a look at these two commits:

6bf3db4 which reverted
544032b

We could restore that behavior, but make it opt-in. e.g. add a Makefile flag, make USE_ENV_PYTHON=1 ... which exports an environment variable for use by setup.py.

Would that be sufficient for you use case?

Another question -- why is setup.py being run with a different interpreter than the one that is used at runtime? A solution might be to make sure the python you want is in your $PATH, or to pass the PYTHON path to the makefile, e.g. make PYTHON=/path/to/python ...

It'll then burn in that python instead. That said, it makes it non-portable. It would be really nice if distutils let us do that without monkey-patching their regex.

Member

davvid commented Jun 30, 2018

We used to hack around distutils, which is where the replacement happens.

Take a look at these two commits:

6bf3db4 which reverted
544032b

We could restore that behavior, but make it opt-in. e.g. add a Makefile flag, make USE_ENV_PYTHON=1 ... which exports an environment variable for use by setup.py.

Would that be sufficient for you use case?

Another question -- why is setup.py being run with a different interpreter than the one that is used at runtime? A solution might be to make sure the python you want is in your $PATH, or to pass the PYTHON path to the makefile, e.g. make PYTHON=/path/to/python ...

It'll then burn in that python instead. That said, it makes it non-portable. It would be really nice if distutils let us do that without monkey-patching their regex.

@Lin-Buo-Ren

This comment has been minimized.

Show comment
Hide comment
@Lin-Buo-Ren

Lin-Buo-Ren Aug 1, 2018

Contributor

Would that be sufficient for you use case?

Though I've already workarounded it at my end, I believe the env python shebang should be in the default build product instead of being opted-in. Distributions that ship multiple interpreter versions( python2, python3, python36) and custom environment that prefers a interpreter not in the executable search path should be treated as special case.

why is setup.py being run with a different interpreter than the one that is used at runtime?

Snapcraft, the tool used to build snap packages runs the executables under $SNAPCRAFT_STAGE/*bin and the build host's $PATH s during each part's build phase but in the runtime all the executables are expected to be in the snap package with the PATH="$SNAP/usr/sbin:$SNAP/usr/bin:$SNAP/sbin:$SNAP/bin:$PATH" ($SNAP expands to /snap/_snap_name_/current ) with a very limited exception that is exposed from the core snap (a minimized version of Ubuntu 16.04).

Contributor

Lin-Buo-Ren commented Aug 1, 2018

Would that be sufficient for you use case?

Though I've already workarounded it at my end, I believe the env python shebang should be in the default build product instead of being opted-in. Distributions that ship multiple interpreter versions( python2, python3, python36) and custom environment that prefers a interpreter not in the executable search path should be treated as special case.

why is setup.py being run with a different interpreter than the one that is used at runtime?

Snapcraft, the tool used to build snap packages runs the executables under $SNAPCRAFT_STAGE/*bin and the build host's $PATH s during each part's build phase but in the runtime all the executables are expected to be in the snap package with the PATH="$SNAP/usr/sbin:$SNAP/usr/bin:$SNAP/sbin:$SNAP/bin:$PATH" ($SNAP expands to /snap/_snap_name_/current ) with a very limited exception that is exposed from the core snap (a minimized version of Ubuntu 16.04).

@davvid davvid closed this in 502a9ad Aug 2, 2018

@davvid

This comment has been minimized.

Show comment
Hide comment
@davvid

davvid Aug 2, 2018

Member

Thanks for the details. To avoid regressing on #66 we should keep the default behavior as-is.

For snap, we've added a Makefile and setup.py knob to inhibit this behavior. Please use make USE_ENV_PYTHON=1 prefix=... install and it'll retain the original shebang lines.

Member

davvid commented Aug 2, 2018

Thanks for the details. To avoid regressing on #66 we should keep the default behavior as-is.

For snap, we've added a Makefile and setup.py knob to inhibit this behavior. Please use make USE_ENV_PYTHON=1 prefix=... install and it'll retain the original shebang lines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment