Skip to content

Use env to determine python3 location#7447

Merged
tobiasdiez merged 4 commits into
JabRef:masterfrom
juhannc:patch-1
Feb 23, 2021
Merged

Use env to determine python3 location#7447
tobiasdiez merged 4 commits into
JabRef:masterfrom
juhannc:patch-1

Conversation

@juhannc

@juhannc juhannc commented Feb 15, 2021

Copy link
Copy Markdown
Contributor

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.

@tobiasdiez tobiasdiez left a comment

Copy link
Copy Markdown
Member

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

juhannc commented Feb 15, 2021

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
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

juhannc commented Feb 15, 2021

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
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
Copy Markdown
Member

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

@LyzardKing

Copy link
Copy Markdown
Collaborator

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

@juhannc

juhannc commented Feb 23, 2021

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
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