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

Rewrite signing using gpgme and gcrypt #1203

Merged
merged 2 commits into from
Jul 27, 2022
Merged

Rewrite signing using gpgme and gcrypt #1203

merged 2 commits into from
Jul 27, 2022

Conversation

TheAssassin
Copy link
Member

CI stuff is still missing (ideally we can just link both libraries statically and be done), will follow soon.

Hopefully this finally makes the signing process reliable. Working with those libs in plain C is annoying and time consuming.

@TheAssassin TheAssassin marked this pull request as draft July 20, 2022 14:28
@TheAssassin TheAssassin marked this pull request as ready for review July 27, 2022 15:15
After updating the AppImageBuild Docker images, we had to adjust some
details around the build of mksquashfs within libappimage 0.1.x.
@TheAssassin TheAssassin merged commit e15fd5f into master Jul 27, 2022
@TheAssassin TheAssassin deleted the gpgme-gcrypt branch July 27, 2022 15:30
@probonopd
Copy link
Member

probonopd commented Jul 28, 2022

Getting

Log: "appimagetool -v '/__w/linuxdeployqt/linuxdeployqt/linuxdeployqt.AppDir' -n -g"
appimagetool: error while loading shared libraries: libgpgme.so.11: cannot open shared object file: No such file or directory
Log: ret 32512

https://github.com/probonopd/linuxdeployqt/runs/7562119014?check_suite_focus=true#step:6:921

Is this caused by this change? If yes, please roll back or fix. Thanks!

Reference:

@TheAssassin
Copy link
Member Author

TheAssassin commented Jul 29, 2022

This is not the fault of this change. The built AppImage works fine, and the way linuxdeploy bundles appimagetool also works for many, many repositories just fine.

I guess you need to adjust how you bundle appimagetool. This, however, is not my job.

@TheAssassin
Copy link
Member Author

TheAssassin commented Jul 29, 2022

https://github.com/probonopd/linuxdeployqt/blob/adf1eb084055e68bd7f62a47273fed5291eda9f2/tests/tests-ci.sh#L24

This approach too naive. appimagetool has not been "dependency free" in years (libffi for instance has had to be bundled for quite a while). You need to properly deploy the contents of the appimagetool AppImage. Please feel free to take some inspiration from https://github.com/linuxdeploy/linuxdeploy-plugin-appimage/blob/master/ci/build-appimage.sh#L50-L58. Just copy the entire appimagetool AppDir into your AppImage and use the contained AppRun to call appimagetool.

@probonopd
Copy link
Member

probonopd commented Jul 29, 2022

Well, you know my philosophy. Something breaks something that was working before = it gets rolled back if it can't be fixed immediately. :)

@TheAssassin
Copy link
Member Author

TheAssassin commented Jul 29, 2022

Which is utterly wrong, counterproductive and off-putting to contributors and users. Especially in this case where the bundling within a third-party repository has been broken for years already. I hadn't even seen your comment here initially, I was sent here by a user who was afraid of exactly that behavior to intervene early.

@probonopd
Copy link
Member

So, how do we get probonopd/linuxdeployqt#541 unblocked?

@probonopd
Copy link
Member

appimagetool has not been "dependency free" in years (libffi for instance has had to be bundled for quite a while). You need to properly deploy the contents of the appimagetool AppImage.

Not saying that you are wrong here. Hence, the next version of the AppImage tooling shall all be statically linked entirely (both runtime and ingredients).

But what are we doing in the meantime?

@TheAssassin
Copy link
Member Author

TheAssassin commented Jul 29, 2022

See #1203 (comment). The bundling within your script has been too naive for years. Look at https://github.com/AppImage/AppImageKit/blame/master/ci/build-appdir.sh#L61-L71, the bundling of libffi has been necessary at least since Dec 2020, probably even earlier.

I've linked to how linuxdeploy's AppImage plugin bundles appimagetool. It extracts the entire AppDir as is and uses appimagetool from there. Your script just copies the main binary and hopes for the best. Recipe for disaster, that is.

Edit: if you open an issue over at linuxdeploy, ping me and I'll assist you with feedback.

@probonopd
Copy link
Member

We just need to make it work as good as it was before this change, not better (better of course would be, well, better, but that's unrelated to this breaking PR).

Since I don't have any time to look into this whole topic at the moment but need to be un-blocked, I'd like to ask you in the kindest way possible to please either fix this or roll back this PR. Thanks!

@TheAssassin
Copy link
Member Author

TheAssassin commented Jul 29, 2022

Do you realize that this blackmail is based on the fact that your bundling is broken? And that it is going to get worse with bundling desktop-file-utils and some other binaries (upcoming changes)? This is just nonsense. Why don't you ask your contributors to fix the issue? I'm sure they're willing to do so, just need some guidance. And I generously offered to assist you and your contributors in #1203 (comment).

I am not going to revert this PR. This PR is completely legitimate.

@probonopd
Copy link
Member

probonopd commented Jul 29, 2022

No, I didn't. but thanks to your explanation on irc I understand now that this is an issue with how linuxdeployqt bundes appimagetool, not with the appimagetool AppImage itself. So additional work in linuxdeployqt is required: probonopd/linuxdeployqt#542

Thanks for the clarification.

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.

2 participants