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

Qwebengine fix #5

Merged
merged 11 commits into from
Apr 7, 2022
Merged

Qwebengine fix #5

merged 11 commits into from
Apr 7, 2022

Conversation

zocker-160
Copy link
Contributor

As discussed on Reddit, this is a complete rewrite from scratch of all manifests.
It uses the latest version of PyQt5 and fixes the QWebengine problem.

This is a draft for now so I can do some testing with the version build by the flathub-bot.

in order to keep the git diffs humanly readable...
@hfiguiere
Copy link
Contributor

changing the format of the manifest is the best way to obfuscate the changes. Is this really necessary?

@hfiguiere
Copy link
Contributor

hfiguiere commented Apr 2, 2022

also "python3-modules.json" was likely autogenerated by the flatpak-builder-tools, making the use of the tool impossible down the way, making the maintainer life harder.

ch.theologeek.Manuskript.yml Outdated Show resolved Hide resolved
@zocker-160
Copy link
Contributor Author

changing the format of the manifest is the best way to obfuscate the changes. Is this really necessary?

This is a complete rewrite, nothing to be obfuscated here. YML is the easier format to work with, that is why I use it.

regenerating the autogenerated file is not a problem

@zocker-160
Copy link
Contributor Author

the autogenerated module is back to json now

Co-authored-by: Hubert Figuière <hub@figuiere.net>
ch.theologeek.Manuskript.yml Outdated Show resolved Hide resolved
Co-authored-by: Hubert Figuière <hub@figuiere.net>
@zocker-160
Copy link
Contributor Author

I guess I need to convert this to an actual PR in order to get the flathubbot building?

@hfiguiere
Copy link
Contributor

I guess I need to convert this to an actual PR in order to get the flathubbot building?

no, it should have been detected

bot, build

@flathubbot
Copy link

Queued test build for ch.theologeek.Manuskript.

@flathubbot
Copy link

Started test build 84573

@flathubbot
Copy link

Build 84573 failed

@TheJackiMonster
Copy link
Collaborator

Okay, so I tested the build and it seems to work as intended. The PDF preview finally works which is awesome. The installation gets cleaned up a bit now. The only downside would be that the build for aarch64 fails.

The log states that it mostly fails because of the missing type QOpenGLTimerQuery. So I would think enabling QtOpenGL for PyQt5 might fix that.

Signed-off-by: TheJackiMonster <thejackimonster@gmail.com>
@flathubbot
Copy link

Started test build 84771

@flathubbot
Copy link

Build 84771 failed

@zocker-160
Copy link
Contributor Author

zocker-160 commented Apr 4, 2022

The only downside would be that the build for aarch64 fails.

Worst case would be to disable the pdf preview for aarch64.

I wonder why the build failed this time though, it looks like some network problem, since it did not even start to build.

@flathubbot
Copy link

Queued test build for ch.theologeek.Manuskript.

@flathubbot
Copy link

Started test build 84782

@zocker-160
Copy link
Contributor Author

looks like the same error is happening again :(

@hfiguiere
Copy link
Contributor

You need to disable desktopGL on aarch64. Because the SDK is built without it on aarch64.

It worked with the previous manifest though.

@flathubbot
Copy link

Build 84782 failed

attempt to fix build issues on aarch64
@flathubbot
Copy link

Queued test build for ch.theologeek.Manuskript.

1 similar comment
@flathubbot
Copy link

Queued test build for ch.theologeek.Manuskript.

@flathubbot
Copy link

Started test build 84809

@flathubbot
Copy link

Build 84809 failed

@zocker-160
Copy link
Contributor Author

I don't quite understand what the error is, but it is happening during the build of QtGui, so I am not sure if this is even related to QtOpenGL

@flathubbot
Copy link

Started test build 85251

@flathubbot
Copy link

Build 85251 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/83095/ch.theologeek.Manuskript.flatpakref

@zocker-160
Copy link
Contributor Author

zocker-160 commented Apr 7, 2022

@TheJackiMonster alright so building all PyQt modules (like the old manifest did) seems to solve the build issues on aarch64.

It does increase the size of the image since it includes modules that are not needed.

@flathubbot
Copy link

Started test build 85262

@flathubbot
Copy link

Queued test build for ch.theologeek.Manuskript.

1 similar comment
@flathubbot
Copy link

Queued test build for ch.theologeek.Manuskript.

@flathubbot
Copy link

Queued test build for ch.theologeek.Manuskript.

1 similar comment
@flathubbot
Copy link

Queued test build for ch.theologeek.Manuskript.

@flathubbot
Copy link

Build 85262 failed

@flathubbot
Copy link

Started test build 85268

@flathubbot
Copy link

Build 85268 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/83112/ch.theologeek.Manuskript.flatpakref

@zocker-160
Copy link
Contributor Author

nice it seems to work! Please review @TheJackiMonster :)

Signed-off-by: TheJackiMonster <thejackimonster@gmail.com>
@flathubbot
Copy link

Started test build 85274

Copy link
Collaborator

@TheJackiMonster TheJackiMonster left a comment

Choose a reason for hiding this comment

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

Looks good to me. I will probably add the import into the startup script from the repository or maybe even into the script using the QtWebWidgets. Then I should replace the manuskript.py with the usual startup script from repository again.

But the import in the added manuskript.py seems to be necessary for the PDF preview to work. For now this PR seems to be a good solution. I will make the required adjustments with the next release but these changes should improve the current flatpak already for version 0.13.1 a lot. ^^

Thank you very much.

@flathubbot
Copy link

Build 85274 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/83118/ch.theologeek.Manuskript.flatpakref

@TheJackiMonster TheJackiMonster marked this pull request as ready for review April 7, 2022 16:58
@TheJackiMonster TheJackiMonster merged commit cde1d8d into flathub:master Apr 7, 2022
@zocker-160 zocker-160 deleted the qwebengine-fix branch April 7, 2022 17:08
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.

None yet

4 participants