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 github action to publish snap #6063

Merged
merged 2 commits into from
Mar 5, 2020
Merged

Use github action to publish snap #6063

merged 2 commits into from
Mar 5, 2020

Conversation

LyzardKing
Copy link
Collaborator

Use github actions to publish,
to try and reduce the upload time

@LyzardKing
Copy link
Collaborator Author

@Siedlerchr The upload is taking a lot longer than it should..
Maybe the "semi-official" github action can help..

@Siedlerchr
Copy link
Member

Please note that the snap craft upload is currently only for master branch, at least I saw a filter

@koppor
Copy link
Member

koppor commented Mar 4, 2020

I don't know which action @Siedlerchr is referring to. Maybe you can add a link?

@LyzardKing I would ❤️ if you added an ADR for the choice of the action. Just place it at https://github.com/JabRef/jabref/tree/master/docs/adr.

I found following action as alternative - can just be added to the list of options ;) --> https://github.com/marketplace/actions/snapcraft-action

@LyzardKing
Copy link
Collaborator Author

The GitHub actions are from https://forum.snapcraft.io/t/call-for-testing-github-action-for-snapcraft/14930
They are developed from someone in the snapcraft team.
At the moment we're already using his build action.
This pr adds the upload action

@koppor koppor force-pushed the master branch 2 times, most recently from b8ef7b7 to 21c6e5e Compare March 4, 2020 17:02
@JabRef JabRef deleted a comment from codecov bot Mar 4, 2020
@@ -117,11 +117,11 @@ jobs:
- name: Build snap (1) Setup snapcraft
Copy link
Member

Choose a reason for hiding this comment

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

I think it would make sense to run this in parallel as it's own job?

Copy link
Member

Choose a reason for hiding this comment

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

Currently does not work at all: https://github.com/JabRef/jabref/actions/runs/49715346

cannot read snap file "build/distribution/jabref*.snap"

Trying to debug.

Copy link
Member

Choose a reason for hiding this comment

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

It's also very difficult to do it in parallel in a separate workflow (which is not what you meant, right?). IMHO it depends on the steps above (including jpackage, ...). Thus, we would need to duplicate 100 SLOC of the build file.

We could work with different jobs in the same build file. Then, we would need to introduce uploading the build directory (more?) to GitHub as data passing between jobs can only be done that way. This is not comfortable (we have to be sure that we don't forget something)

Copy link
Member

Choose a reason for hiding this comment

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

Cannot read is because the * is not expanded. Fixed it in cd70746

@LyzardKing
Copy link
Collaborator Author

LyzardKing commented Mar 4, 2020

Thanks both for looking at this..
I didn't have much time to do this while on a train with no stable internet connection.
EDIT I'll look at adding the markdown file with the selected solution tomorrow morning.

@Siedlerchr
Copy link
Member

Both master and the master-release fail at some point in the snapcraft upload
##[error]The operation was canceled.
@LyzardKing Btw, we still also have the snapcraft.yml , so I could try to trigger a rebuild there at the official snapcraft ci if that is possible with our infrastructure

@koppor koppor merged commit 29ed8fe into master Mar 5, 2020
@koppor koppor deleted the snapcraft-publish-action branch March 5, 2020 05:09
@LyzardKing
Copy link
Collaborator Author

@koppor I changed the snap to use https://builds.jabref.org/master/JabRef-5.0-portable_linux.tar.gz instead of the build artifact (you can see the comment in the snapcraft.yaml) to test it..
If we keep the source as https://builds.jabref.org/master/JabRef-5.0-portable_linux.tar.gz it might be easier to add it to the snapcraft CI and use it there (it should rebuild every time a new tar.gz is uploaded.
If we want to use the github action instead is it possible to make the snap action run after the artifact build action?

@LyzardKing
Copy link
Collaborator Author

@Siedlerchr I think it might be easier if we try and run the snapcraft CI..
The snapcraft as it is now (with the full url as source) will work with that.
If it works then we can remove the snap part in the github actions, it seems to complicated to get working properly..

@Siedlerchr
Copy link
Member

Snap Status
I triggered a new build

@Siedlerchr
Copy link
Member

@LyzardKing Snapcraft runs but shows this weird message regarding license:

https://build.snapcraft.io/user/JabRef/jabref/858770

Could not find '/snap/core/current/usr/bin/snap', validation of the license string will only take place once pushed to the store.
Snapping 'jabref' ...

Snapped jabref_5.0.30532_amd64.snap
Revoking proxy token...
RUN: /usr/share/launchpad-buildd/bin/in-target scan-for-processes --backend=lxd --series=bionic --arch=amd64 SNAPBUILD-858770
Scanning for processes to kill in build SNAPBUILD-858770

@LyzardKing
Copy link
Collaborator Author

@Siedlerchr The review is manual and will need roughly two weeks.
https://forum.snapcraft.io/t/system-files-under-mozilla-native-messaging-hosts/13627/41

@Siedlerchr
Copy link
Member

Ah okay thanks for checking that!

@koppor
Copy link
Member

koppor commented Mar 9, 2020

@LyzardKing The "manual review" is for each published version or a general "OK" for this and all upcoming next versions?

@LyzardKing
Copy link
Collaborator Author

As I understand it the approval should be done once for be permissions we asked.
So unless we change the permissions it shouldn't be done any more

koppor pushed a commit that referenced this pull request Jul 15, 2022
39fede5 Update associacao-brasileira-de-normas-tecnicas.csl (#6138)
fde7695 Include chapter title (#6140)
1e3d8b4 Update n.d. abbreivation for DGP style (#6136)
ebb728b suffix '.' after first group; changed e-mail (#6135)
eed4f07 Update and rename sciences-po-ecole-doctorale-note-french.csl to scie… (#6127)
f194647 Delete TU Dresden Medizin as requested by library (#6131)
d8423d8 Create entomological-review.csl (#6120)
064a394 Create australasian-journal-of-philosophy.csl (#6063)
a998ded Add composer.json (#5668)
37083c9 Update copernicus-publications.csl (#6062)
694c97b Create chaucer review (#6061)
625a424 Create haffner-style-manual.csl (#6054)
8b7224b make annals-of-allergy-asthma-and-immunology independent (#6041)
710748c Create university-of-pretoria-harvard-theology-religion.csl (#6106)
d16dffd Create health-physics.csl (#6040)
ca9e184 Update style-manual-australian-government.csl (#6119)
e412277 Create chemical-engineering-technology.csl (#6039)
bebdb48 Create bibliothek-forschung-und-praxis.csl (#6038)
29e49cd Update nature.csl (#6117)
891897d fix short title for SBL (#6118)

git-subtree-dir: buildres/csl/csl-styles
git-subtree-split: 39fede5
koppor pushed a commit that referenced this pull request Aug 1, 2022
c750b6e APA: Put conditional event-title logic in a macro (#6161)
a87414f Remove month from association-for-compuational-linguistics.csl (#6158)
6153db0 Remove issue numbers from BJOC style (#6155)
e231ea3 Bug fix for `event` regression (#6154)
0dab651 Add event-title to other APA styles (#6153)
698cf1c APA: `event-title` and conditional `event` (#6152)
58d3f8f Update vancouver-author-date.csl (#6148)
f1638a9 add substitute to Vancouver author date (#6147)
39fede5 Update associacao-brasileira-de-normas-tecnicas.csl (#6138)
fde7695 Include chapter title (#6140)
1e3d8b4 Update n.d. abbreivation for DGP style (#6136)
ebb728b suffix '.' after first group; changed e-mail (#6135)
eed4f07 Update and rename sciences-po-ecole-doctorale-note-french.csl to scie… (#6127)
f194647 Delete TU Dresden Medizin as requested by library (#6131)
d8423d8 Create entomological-review.csl (#6120)
064a394 Create australasian-journal-of-philosophy.csl (#6063)
a998ded Add composer.json (#5668)
37083c9 Update copernicus-publications.csl (#6062)
694c97b Create chaucer review (#6061)
625a424 Create haffner-style-manual.csl (#6054)
8b7224b make annals-of-allergy-asthma-and-immunology independent (#6041)
710748c Create university-of-pretoria-harvard-theology-religion.csl (#6106)
d16dffd Create health-physics.csl (#6040)
ca9e184 Update style-manual-australian-government.csl (#6119)
e412277 Create chemical-engineering-technology.csl (#6039)
bebdb48 Create bibliothek-forschung-und-praxis.csl (#6038)
29e49cd Update nature.csl (#6117)
891897d fix short title for SBL (#6118)

git-subtree-dir: buildres/csl/csl-styles
git-subtree-split: c750b6e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants