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

Use env to determine python3 location #7447

Merged
merged 4 commits into from
Feb 23, 2021
Merged

Conversation

juhannc
Copy link
Contributor

@juhannc juhannc commented Feb 15, 2021

In general, it is recommended to use #!/usr/bin/env python3 as the shebang instead of the hardcoded !#/usr/bin/python3.
See https://stackoverflow.com/a/5709632 for more information.
Related to JabRef-Browser-Extension #177, especially this comment.

  • Change in CHANGELOG.md described (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked documentation: Is the information available and up to date? If not created an issue at https://github.com/JabRef/user-documentation/issues or, even better, submitted a pull request to the documentation repository.

In general, it is recommended to use `#!/usr/bin/env python3` as the shebang instead of the hardcoded `!#/usr/bin/python3`.
See <https://stackoverflow.com/a/5709632> for more information.
Copy link
Member

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that's a nice improvement indeed. Could you please also change the mac version of the script https://github.com/JabRef/jabref/blob/master/buildres/mac/jabrefHost.py and add a short entry to the changelog.

@tobiasdiez tobiasdiez added the status: changes required Pull requests that are not yet complete label Feb 15, 2021
@juhannc
Copy link
Contributor Author

juhannc commented Feb 15, 2021

Thanks, that's a nice improvement indeed. Could you please also change the mac version of the script https://github.com/JabRef/jabref/blob/master/buildres/mac/jabrefHost.py and add a short entry to the changelog.

@tobiasdiez Done! Listed the change under Changed, hope thats fine :)

@LyzardKing
Copy link
Collaborator

I thought we had changed this already. Great change.
The mac version fails to run with env (it depends where python comes from, system or brew)

@juhannc
Copy link
Contributor Author

juhannc commented Feb 15, 2021

I see, I was only able to test it on my mac with Python installed via brew.
Any way to fix it for non-brew systems?

@LyzardKing
Copy link
Collaborator

I tried but there were too many error reports.
I'm not sure if there's a way to do it properly

@tobiasdiez
Copy link
Member

So on mac we still should use !#/usr/bin/python3 ?

@LyzardKing
Copy link
Collaborator

The issue was looked at here: JabRef/JabRef-Browser-Extension#177
If it works we should leave the mac one as is

@juhannc
Copy link
Contributor Author

juhannc commented Feb 23, 2021

I just confirmed on a brand new Mac.
Using /usr/bin/python3 works, but usr/bin/env python3 would require the XCode command line developer tools.
I'll revert I reverted the changes for macOS and then the PR would be is now ready from my side.

Using `/usr/bin/env python3` instead of `/usr/bin/python3` would require the XCode command line developer tools.
This is not feasible.
@tobiasdiez
Copy link
Member

Thanks, and sorry for the misleading suggestion from my side.

@tobiasdiez tobiasdiez merged commit 2594c28 into JabRef:master Feb 23, 2021
@Siedlerchr Siedlerchr mentioned this pull request Mar 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: changes required Pull requests that are not yet complete
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants