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

Python3 shebang for 3.1.0 #2261

Closed
Lestropie opened this issue Jan 19, 2021 · 6 comments
Closed

Python3 shebang for 3.1.0 #2261

Lestropie opened this issue Jan 19, 2021 · 6 comments

Comments

@Lestropie
Copy link
Member

Originally posted by @bjeurissen in #2047 (comment)

See #2047 and #2215.

D'Oh. Better to discover such while it's only on dev though.

  1. We can just revert to #!/usr/bin/env python3, despite the recommendations against use of env;
  2. This comment on the page Donald linked earlier suggests using some shell logic within the shebang itself to determine the appropriate interpreter;
  3. The recommendation to incorporate into the build system isn't ideal for us given we're now distributing pre-compiled code, i.e. we can't rely on configure or build. If however we had a script that set script shebangs, then configure could use logic of any arbitrary complexity to try to decide this, and those installing from pre-compiled packages could still use it if the default doesn't work on their system.
  4. Still not a fan of requiring users to explicitly specify the interpreter each time...
  5. Any other options still on the table?
@Lestropie Lestropie added this to the 3.1.0 updates milestone Jan 19, 2021
fionaEyoung added a commit to fionaEyoung/tractfinder that referenced this issue Aug 2, 2023
See MRtrix3/mrtrix3#2261 and MRtrix3/mrtrix3#2047. Am dropping python2 
support for tractfinder because I like fstrings :)
@daljit46
Copy link
Member

daljit46 commented May 8, 2024

Given the transition to CMake, I think providing a simple executable script in the installation directory that selects an appropriate shebang shouldn't be too much work. Previously, we had no delineation between build and install directories, so this might have been cumbersome, but that is no longer the case.

Personally, however, I would prefer just to use #!/usr/bin/env python3. As an end user, I don't want a script to hardcode the Python interpreter (I routinely use virtual environments to avoid polluting my system with pip packages).

@daljit46
Copy link
Member

Given that we merged #2958, can we close this?

@Lestropie
Copy link
Member Author

#2958 only deals with how a Python script already being executed will behave as it executes other Python scripts. It doesn't modify what will happen when an MRtrix3 Python script is first executed by the user from a shell.

With the CMake-generated Python executables produced in #2920, there is technically the option of hard-coding the Python interpreter of choice into the shebang of those short files; this is how FSL gets their code to be interpreted by their custom Conda environment. However I don't think we want to be doing the same thing ourselves. For one, it wouldn't work for shipped compiled tarballs.

Given your own prior comment, I had presumed that you would prefer those CMake-generated executables to change to #!/usr/bin/env python3. 3.0.x runs #!/usr/bin/env python, which has become problematic; on dev we shifted to #!/usr/bin/python3 rather than #!/usr/bin/env python3 based on this comment, but you had made an argument to the contrary at some point. So I'm quite content to change them all to #!/usr/bin/env python3, which should now only require a change in the CMake Python executable generation function.

@daljit46
Copy link
Member

Following #2958, I was under the (I guess wrong) impression that we wanted to hardcode the Python interpreter versions (including minor versions).

So I'm quite content to change them all to #!/usr/bin/env python3, which should now only require a change in the CMake Python executable generation function.

Yes, that still seems the most sensible option to me.

@Lestropie
Copy link
Member Author

I was under the (I guess wrong) impression that we wanted to hardcode the Python interpreter versions (including minor versions).

I did invest a bit more effort in what you're talking about, but not for the reason you're subsequently inferring.

The Python scripts (and the API more generally) can execute non-MRtrix3 commands, and those commands have shebangs outside of our control. Given we already had a mechanism for detecting when such a command is written for Python and utilising the same interpreter as that currently executing, I came to the realisation that if that command requests a specific minor version of Python3, and the currently executing interpreter does not match that, then the currently executing interpreter should not be used, as the specificity of that shebang may indicate a very specific dependency. So I'm trying to do something more clever in such circumstances, but only for speculative non-MRtrix3 shebangs, not reflecting an intent for MRtrix3 shebangs.

Though thinking about it more now, I'm concerned the solution I have is no good. But I'll post that in #2958.

@Lestropie
Copy link
Member Author

Closed by #2966.

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

No branches or pull requests

2 participants