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

Update MacOS jabrefHost.py to find local installs #9487

Merged
merged 1 commit into from
Dec 21, 2022

Conversation

allydunham
Copy link
Contributor

@allydunham allydunham commented Dec 20, 2022

Fixes #9474

The previous MacOS hardcoded path fails if JabRef is installed to a users local applications directory rather than the system one (e.g. if the user doesn't have admin permissions). On my version of MacOS (12.6.1) and python (3.9.6) this change works correctly for local installs as well as global. The Mac app bundle is structured differently so an alternate path is still required.

If the difference in which behaviour is still a problem (maybe on older versions/different python versions?) the original hardcoded fallback could also be included as a final option. If that is still necessary the explanatory comment should be updated too, as should the docs to detail when the hardcoded fallback is needed and therefore when users may need to manually update it.

As mentioned on the issue thread the python script path in the browser extension JSON would still need to be updated for local installs, so I will submit a separate PR to the docs with notes on the process. If this script can't be updated I can expand the docs update to detail the hardcoded path change required to make the browser extension work for local app installs.

  • Change in CHANGELOG.md described in a way that is understandable for the average user (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 developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

Following issue [9474](JabRef#9474).

The previous MacOS hardcoded path fails if JabRef is installed to a users local applications directory rather than the system one (e.g. if the user doesn't have admin permissions). On my version of MacOS (12.6.1) this change works correctly for local installs as well. The Mac app bundle is structured differently so an alternate path is still required.

If the difference in `which` behaviour is still a problem (maybe on older versions?) the original hardcoded fallback could also be included as a final option. If that is still necessary the explanatory comment should be updated too.

As mentioned on the issue thread the python script path in the browser extension JSON would still need to be updated for local installs, so I will submit a separate PR to the docs with notes on the process. If this script can't be updated I can expand the docs update to detail the hardcoded path change required to make the browser extension work for local app installs.
@Siedlerchr
Copy link
Member

This change looks good to me!

@LyzardKing
Copy link
Collaborator

This seems good to me. MacOS has paths that are easily hardcoded

@k3KAW8Pnf7mkmdSMPHz27

This comment was marked as off-topic.

@Siedlerchr Siedlerchr marked this pull request as ready for review December 20, 2022 17:52
@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Dec 20, 2022
@Siedlerchr Siedlerchr merged commit 69d504a into JabRef:main Dec 21, 2022
@Siedlerchr
Copy link
Member

Thanks again, as we came to the conclusion that this is fine we merge it now

Siedlerchr added a commit to rafatd/jabref that referenced this pull request Dec 21, 2022
…of_groups_to_new_library

* upstream/main:
  Update MacOS jabrefHost.py to find local installs (JabRef#9487)
Siedlerchr added a commit to shafinkamal/jabref that referenced this pull request Dec 21, 2022
* upstream/main: (145 commits)
  Enable groups drag'n'drop to new library (JabRef#9460)
  Update MacOS jabrefHost.py to find local installs (JabRef#9487)
  Fix remember last open valid library with empty new one (JabRef#9489)
  Observable Preferences R (CitationKeyPatternPreferences) (JabRef#9486)
  Fixed ZBMathTest and extracted keyWordSeparator (JabRef#9485)
  New Crowdin updates (JabRef#9483)
  Add log for ignored excepton (JabRef#9302)
  Select Library to import into (JabRef#9402)
  Bump org.eclipse.jgit from 6.3.0.202209071007-r to 6.4.0.202211300538-r (JabRef#9476)
  Bump com.github.andygoossens.modernizer from 1.6.2 to 1.7.0 (JabRef#9478)
  Bump mockito-core from 4.9.0 to 4.10.0 (JabRef#9479)
  Bump checkstyle from 10.4 to 10.5.0 (JabRef#9477)
  Bump slf4j-api from 2.0.5 to 2.0.6 in /buildSrc (JabRef#9480)
  Bibtex month not deprecated (JabRef#9404)
  Show development information\n\n+semver: major
  Release v5.8
  Update external libraries add afterburner fx jabref add jakarta inject
  fix display name for artifact store
  Prepare CHANGELOG for release
  Also trigger on branch arm64mac-release
  ...

# Conflicts:
#	CHANGELOG.md
#	src/main/java/org/jabref/preferences/JabRefPreferences.java
Siedlerchr added a commit to Freeman6475/jabref that referenced this pull request Dec 22, 2022
* upstream/main: (120 commits)
  Enable groups drag'n'drop to new library (JabRef#9460)
  Update MacOS jabrefHost.py to find local installs (JabRef#9487)
  Fix remember last open valid library with empty new one (JabRef#9489)
  Observable Preferences R (CitationKeyPatternPreferences) (JabRef#9486)
  Fixed ZBMathTest and extracted keyWordSeparator (JabRef#9485)
  New Crowdin updates (JabRef#9483)
  Add log for ignored excepton (JabRef#9302)
  Select Library to import into (JabRef#9402)
  Bump org.eclipse.jgit from 6.3.0.202209071007-r to 6.4.0.202211300538-r (JabRef#9476)
  Bump com.github.andygoossens.modernizer from 1.6.2 to 1.7.0 (JabRef#9478)
  Bump mockito-core from 4.9.0 to 4.10.0 (JabRef#9479)
  Bump checkstyle from 10.4 to 10.5.0 (JabRef#9477)
  Bump slf4j-api from 2.0.5 to 2.0.6 in /buildSrc (JabRef#9480)
  Bibtex month not deprecated (JabRef#9404)
  Show development information\n\n+semver: major
  Release v5.8
  Update external libraries add afterburner fx jabref add jakarta inject
  fix display name for artifact store
  Prepare CHANGELOG for release
  Also trigger on branch arm64mac-release
  ...
Siedlerchr added a commit that referenced this pull request Dec 25, 2022
* upstream/main: (75 commits)
  Observable Preferences S (LastExportPath and Cleanups in JabRefPreferences and Globals) (#9493)
  Enable groups drag'n'drop to new library (#9460)
  Update MacOS jabrefHost.py to find local installs (#9487)
  Fix remember last open valid library with empty new one (#9489)
  Observable Preferences R (CitationKeyPatternPreferences) (#9486)
  Fixed ZBMathTest and extracted keyWordSeparator (#9485)
  New Crowdin updates (#9483)
  Add log for ignored excepton (#9302)
  Select Library to import into (#9402)
  Bump org.eclipse.jgit from 6.3.0.202209071007-r to 6.4.0.202211300538-r (#9476)
  Bump com.github.andygoossens.modernizer from 1.6.2 to 1.7.0 (#9478)
  Bump mockito-core from 4.9.0 to 4.10.0 (#9479)
  Bump checkstyle from 10.4 to 10.5.0 (#9477)
  Bump slf4j-api from 2.0.5 to 2.0.6 in /buildSrc (#9480)
  Bibtex month not deprecated (#9404)
  Show development information\n\n+semver: major
  Release v5.8
  Update external libraries add afterburner fx jabref add jakarta inject
  fix display name for artifact store
  Prepare CHANGELOG for release
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Browser extension doesn't work with local app install on MacOS
4 participants