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.

Sign up for GitHub

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

Jump to bottom

Autoinstall extension in chrome #6442

Merged
merged 25 commits into from
Aug 5, 2020
Merged

Autoinstall extension in chrome #6442

merged 25 commits into from
Aug 5, 2020

Conversation

LyzardKing
Copy link
Collaborator

@LyzardKing LyzardKing commented May 7, 2020 •
edited by Siedlerchr
Loading

This fixes #6076
Draft PR to build macos releases with the extension json files.
This should add the postinst/prerm scripts for deb files to add the extension automatically
Add initial json files on macos

@LyzardKing LyzardKing marked this pull request as draft May 7, 2020 17:58
@LyzardKing
Copy link
Collaborator Author

@tobiasdiez
The linux (deb) install works.. but only (I think) if the extension isn't already been installed and removed..I need to verify this..
The rpm is setup, but needs testing to verify it works..
For the snap we would need another permission (that would probably not be granted, but we can ask..
The macos part is setup, but I need someone to test it.

@Siedlerchr Siedlerchr marked this pull request as ready for review May 21, 2020 17:52
@Siedlerchr
Copy link
Member

I have retriggered a build, because mac was not building For the mac os it's best to ask in the gitter channel. the version will be avaiable under
https://builds.jabref.org/pull/6442/merge/

tobiasdiez
tobiasdiez reviewed May 21, 2020
View reviewed changes
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.

The mac build failed again (no error message, it timed-out). I triggered another build, but if this also fails I guess there is a problem with the changes.

Other than that, looks good to me!

buildres/linux/postinst Outdated
# Trigger an auto-install of the browser addon for chrome/chromium browsers
install -D -m0644 /opt/jabref/lib/native-messaging-host/chromium/bifehkofibaamoeaopjglfkddgkijdlh.json /opt/google/chrome/extensions/bifehkofibaamoeaopjglfkddgkijdlh.json
install -D -m0644 /opt/jabref/lib/native-messaging-host/chromium/bifehkofibaamoeaopjglfkddgkijdlh.json /usr/share/google-chrome/extensions/bifehkofibaamoeaopjglfkddgkijdlh.json
DESKTOP_COMMANDS_INSTALL
Copy link
Member

Choose a reason for hiding this comment

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

Minor: indent is off here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ah.. thanks. I guess the IDE changed my settings.

@LyzardKing
Copy link
Collaborator Author

@tobiasdiez I tried retriggering the build..
on macos the build succeeds, but it then stops and does not build the installer..

@tobiasdiez
Copy link
Member

Yeah, not sure where the problem is. Your changes look fine to me. I tried to rename the postinstall file to see if that's the problem.

For reference: https://docs.oracle.com/en/java/javase/14/jpackage/override-jpackage-resources.html#GUID-405708DC-0243-49FC-84D9-B2A7F0A011A9
(could be that the postinstall is PKG specific and we also build a dmg)

@Siedlerchr
Copy link
Member

A while ago I enabled verbose output for the jlink/jpackage task on ci, so that we see what the task does.

@LyzardKing
Copy link
Collaborator Author

@tobiasdiez the postinstall is only used in the pkg.. but as far as I can tell the dmg should just ignore it

@tobiasdiez
Copy link
Member

Strange...it works with the postinstall file renamed :( Any idea what to do? Can we exclude the postinstall file for the dmg generation?

@Siedlerchr Siedlerchr added the status: changes required Pull requests that are not yet complete label May 25, 2020
@LyzardKing
Copy link
Collaborator Author

We could try and rename the file to the correct name before setting the .pkg
So the .app is without...
But I'm not sure, since the .app is not the partthat is failing here..

Siedlerchr added 2 commits May 26, 2020 13:30
@Siedlerchr
Merge remote-tracking branch 'upstream/master' into autoinstall_ext
da01d55 
* upstream/master: (76 commits)
  Fixes missing library properties context menu on library tab (#6508)
  Bump flexmark-ext-gfm-strikethrough from 0.61.30 to 0.61.32
  Bump archunit-junit5-api from 0.13.1 to 0.14.1
  Bump flexmark from 0.61.30 to 0.61.32
  Bump flexmark-ext-gfm-tasklist from 0.61.30 to 0.61.32
  Add javadoc and fix the preview update issue
  Refactor externalprefs (#6509)
  Extend the bib file for better screenshots
  Remove Grobid also from tests
  Fix help file tests
  Update ActionHelper.java
  Adjusted fix by using StateManager for clearing search bar
  fix checkstyle
  Return true in action helper if file is online link
  Reenable caching of gradle
  Refactor BibtexKeyPatternPreferences (#6489)
  Update CHANGELOG.md
  Add changelog entry and remove unnecessary code
  EasyBind revision part two
  Fix Drag and Drop on empty database
  ...
@Siedlerchr
try again with postinstall
6025b72
@Siedlerchr
Copy link
Member

I merged the master and renamed it back, lets see if it works now

@Siedlerchr
Copy link
Member

screw it, the damn apache repo is offline or has issues

@Siedlerchr
Copy link
Member

I triggered the build and it seems to have build now @LyzardKing

@LyzardKing
Copy link
Collaborator Author

@Siedlerchr Is it building with the proper filename?

@Siedlerchr
Copy link
Member

Yes, I renamed it back to "postinstall" without extension

@LyzardKing
Copy link
Collaborator Author

Great! I asked in the gitter channel if someone with a mac can test it out!
Hopefully it should work

@tobiasdiez
Copy link
Member

Ok perfect! Even if it's not working across all mac os systems, its already a huge improvement (since right now it's not working at all on mac). Maybe from the reports of people we can deduce when and when not its working.

@koppor feel free to merge this, it's good to go from my point of view. Further improvements by @LyzardKing can be included later as well, I would say.

@LyzardKing
Copy link
Collaborator Author

I'd wait a moment, since I just uploaded a change I need to test.
I'll tests it then revert the commit if it doesn't solve the issue.
I'll let you know after the builds are uploaded (and can be tested)

@koppor koppor added this to the v5.1 milestone Jul 27, 2020
@Siedlerchr
Copy link
Member

@LyzardKing If you need someone to test on a real mac, I can help now. I just a macbook at hand

@LyzardKing
Copy link
Collaborator Author

@Siedlerchr Thanks!
I'll have (briefly) a mac tomorrow, but if you can test it it would be wonderful!

@LyzardKing
Copy link
Collaborator Author

@Siedlerchr
it you want to test this you can find the build at https://builds.jabref.org/pull/6442/merge/

@Siedlerchr
Copy link
Member

I don't see any files or folders or stuff in LIbrary Application Support. I tested the pkg.
10.15.6 (19G73)

Have both firefox and chrome installed. (I'm new to mac, so just tell me where you look),
I also merged in master now, this reduces the package sizes a lot!

@koppor
Copy link
Member

koppor commented Aug 4, 2020

This fixes #6076 right?

@LyzardKing
Copy link
Collaborator Author

@koppor Yes.
It however works fine on linux (at least the deb/rpm)
On macos it woks in certain configurations: I got it to work if I installed chrome and then jabref without starting chrome.
I tried to read up on the postinstall function on macos and it seems to be a bit "opinionated".
Also the install command works differently on macos that linux, so I switched to using manually mkdir and cp, as suggested on an old mac forum...
but I cannot get it to work properly.

@Siedlerchr
Copy link
Member

@LyzardKing MacOSX is deprecating bash etc and some things may not be available as default anylonger : https://github.com/scriptingosx/pkgcheck So that might explain why it's not working always.
I'm happy to test any further versions

@LyzardKing
Copy link
Collaborator Author

@Siedlerchr Do you mind running the script directly? It needs root access (sudo) but it should work.
The script tries to run the sh environment, not bash

@LyzardKing
Copy link
Collaborator Author

This might be the issue...

So far, in macOS (including Catalina),sh invokes bash in sh-compatibility mode

@LyzardKing
Copy link
Collaborator Author

I tried forcing the proper path for /bin/sh.
@Siedlerchr try testing this version once it builds.
If it does I'd like to revert to using install to properly set the file permissions

@Siedlerchr
Copy link
Member

@LyzardKing The pkg file worked fine now! Both files copied to Chrome and Firefox folder.

@koppor
Copy link
Member

koppor commented Aug 4, 2020

Thus, this is good to go or do we need some more cleanup / testing?

@LyzardKing
Copy link
Collaborator Author

@koppor I'd like to test it with the install command, so we can explicitly set the file permissions.
@Siedlerchr Can you test it one more time after I upload the change?

@LyzardKing
Copy link
Collaborator Author

If this last version works we can merge. Windows is still missing, but we'll work on that later.
If it fails let's revert to cp again

@Siedlerchr
Copy link
Member

Siedlerchr commented Aug 5, 2020 •
edited
Loading

Just tested it, the pgk works fine and etension is registered in chrome

@LyzardKing
Copy link
Collaborator Author

Perfect! Thanks for the suggestion to set sh directly..
This can now be merged, then the windows version can be added later.

@Siedlerchr Siedlerchr merged commit 9f96958 into master Aug 5, 2020
@Siedlerchr Siedlerchr deleted the autoinstall_ext branch August 5, 2020 09:17
@Siedlerchr
Copy link
Member

Thanks again for your contribution! This is very cool now

Siedlerchr added a commit that referenced this pull request Aug 9, 2020
@Siedlerchr
Merge remote-tracking branch 'upstream/master' into annotationProcessor
e158be2 
* upstream/master: (47 commits)
  Fix copy pasting and delete via menu or key (#6740)
  Add instructions how to work with fetchers  (#6731)
  Autoinstall extension in chrome (#6442)
  Delete link after download (#6723)
  New translations JabRef_en.properties (Portuguese, Brazilian) (#6728)
  Bump pascalgn/automerge-action from v0.8.5 to v0.9.0 (#6736)
  Bump byte-buddy-parent from 1.10.13 to 1.10.14 (#6733)
  Bump mockito-core from 3.4.4 to 3.4.6 (#6734)
  Bump unirest-java from 3.8.06 to 3.9.00 (#6735)
  Bump org.beryx.jlink from 2.21.1 to 2.21.2 (#6732)
  Add testing interface, including a set of capabilities to tests for (#6687)
  Fix pasting on mac and linux (#6419)
  Add validation of "AUTHORS" file (#6722)
  Squashed 'src/main/resources/csl-styles/' changes from cacc4ee..827b986
  New Crowdin updates (#6721)
  Add missing AUTHORs
  Fix for issue 6639 (#6719)
  Fix more links
  Fix link
  New Crowdin updates (#6718)
  ...
koppor pushed a commit that referenced this pull request Mar 15, 2023
Squashed 'buildres/csl/csl-styles/' changes from e9fd2027de..ca943b70d7
e13e96d 
ca943b70d7 Fix malformed XML in raptor-journal.csl (#6455)
2fad8d1104 Update knee-surgery-sports-traumatology-arthroscopy.csl (#6441)
b4422b77b7 Update guide-des-citations-references-et-abreviations-juridiques.csl (#6450)
cfe85da320 Create raptor-journal.csl (#6451)
afbb963346 Create arkivoc.csl (#6134)
2f21ceb7b4 correct name disambiguation rules (#6442)
5b2191f38b Update bibliothek-forschung-und-praxis.csl (#6437)
aaea3097d1 Update sodertorns-hogskola-oxford.csl (#6439)
e4fbc605f8 Update universite-du-quebec-a-montreal.csl (#6438)
3653118f17 Small Fix to Bio-Protocol

git-subtree-dir: buildres/csl/csl-styles
git-subtree-split: ca943b70d73bd4c57c0b1266ee7c54907b8f9d4f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tobiasdiez tobiasdiez tobiasdiez left review comments

Assignees
No one assigned
Labels
status: changes required Pull requests that are not yet complete
Projects
None yet
Milestone
v5.1
Development

Successfully merging this pull request may close these issues.

Install chrome extension automatically
5 participants
@LyzardKing @Siedlerchr @tobiasdiez @abogatskiy @koppor