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

Add QT extension #4294

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open

Add QT extension #4294

wants to merge 19 commits into from

Conversation

jardon
Copy link

@jardon jardon commented Jul 26, 2023

  • Have you followed the guidelines for contributing?
  • Have you signed the CLA?
  • Have you successfully run make lint?
  • Have you successfully run pytest tests/unit?

This PR adds support for two QT extensions (6.5 and 5.15)

  • Add extension handling
  • Update unit tests
  • Fixes hardcoded gnome extension script
  • Update Extension class to pass in extension name

@jardon jardon force-pushed the qt-framework branch 2 times, most recently from 6843f00 to 3b07cab Compare September 6, 2023 16:18
@jardon jardon marked this pull request as ready for review September 6, 2023 16:18
@jardon jardon marked this pull request as draft September 11, 2023 13:51
@jardon jardon force-pushed the qt-framework branch 3 times, most recently from f9b8cdb to d52c124 Compare September 26, 2023 12:52
@jardon jardon marked this pull request as ready for review September 26, 2023 12:56
@codecov-commenter
Copy link

codecov-commenter commented Sep 26, 2023

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (d75fdaf) 89.19% compared to head (4d14e4a) 88.35%.
Report is 5 commits behind head on main.

Files Patch % Lines
snapcraft/extensions/qt_framework.py 95.83% 3 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4294      +/-   ##
==========================================
- Coverage   89.19%   88.35%   -0.85%     
==========================================
  Files         322      328       +6     
  Lines       21826    22035     +209     
==========================================
+ Hits        19468    19469       +1     
- Misses       2358     2566     +208     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@mr-cal mr-cal 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, nice work!

Can you create a spread test for 5.15 and 6.5 under tests/spread/extensions/? You can use the other spread tests in that directory as a reference.

schema/snapcraft.json Outdated Show resolved Hide resolved
snapcraft/extensions/qt_framework.py Outdated Show resolved Hide resolved
tests/unit/extensions/test_qt_framework.py Outdated Show resolved Hide resolved
@jardon jardon requested a review from mr-cal September 27, 2023 18:52
Copy link
Collaborator

@mr-cal mr-cal left a comment

Choose a reason for hiding this comment

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

Update for anyone watching:

This PR is waiting on @jardon getting auto-connect approvals for the two qt-framework snaps used by this extension

@jardon
Copy link
Author

jardon commented Oct 19, 2023

Store request submitted here

@sergiusens
Copy link
Collaborator

you store request has updates

@jardon
Copy link
Author

jardon commented Jan 4, 2024

The CI can be rerun when the snapcraft forum post is resolved.

@jardon
Copy link
Author

jardon commented Feb 12, 2024

@sergiusens @mr-cal do you know where snapctl is getting the qt-framework slot/plug from? I've renamed it in the snaps themselves after being requested in the forums to change it. I dont see it in any of the desktop launch scripts either.

error: error running snapctl: snap "qt6-6-hello" has no plug or slot named "qt-framework"
ERROR: not connected to the qt-framework content interface.
$ snap connections qt6-6-hello 
Interface                         Plug                                 Slot                                             Notes
content[icon-themes]              qt6-6-hello:icon-themes              gtk-common-themes:icon-themes                    -
content[qt-framework-6-6-core22]  qt6-6-hello:qt-framework-6-6-core22  qt-framework-6-6-core22:qt-framework-6-6-core22  -
content[sound-themes]             qt6-6-hello:sound-themes             gtk-common-themes:sound-themes                   -
desktop                           qt6-6-hello:desktop                  :desktop                                         -
desktop-legacy                    qt6-6-hello:desktop-legacy           :desktop-legacy                                  -
opengl                            qt6-6-hello:opengl                   :opengl                                          -
wayland                           qt6-6-hello:wayland                  :wayland                                         -
x11                               qt6-6-hello:x11                      :x11                                             -
jardon@lagaan:~/Projects/snapcraft$ snap connections qt-framework-6-6-core22 
Interface                         Plug                                 Slot                                             Notes
content[qt-framework-6-6-core22]  qt6-6-hello:qt-framework-6-6-core22  qt-framework-6-6-core22:qt-framework-6-6-core22  -

@jardon
Copy link
Author

jardon commented Feb 14, 2024

@mr-cal Can I get the CI kicked back off?

@jardon
Copy link
Author

jardon commented Feb 14, 2024

I updated the content snaps to correct the plug check issue

Copy link

codecov bot commented Feb 14, 2024

Codecov Report

Attention: Patch coverage is 97.26027% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 88.38%. Comparing base (d364154) to head (6ce8e13).

❗ Current head 6ce8e13 differs from pull request most recent head e1c49e5. Consider uploading reports for the commit e1c49e5 to get more accurate results

Files Patch % Lines
snapcraft/extensions/qt_framework.py 97.18% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4294      +/-   ##
==========================================
+ Coverage   88.35%   88.38%   +0.02%     
==========================================
  Files         328      329       +1     
  Lines       22015    22088      +73     
==========================================
+ Hits        19452    19523      +71     
- Misses       2563     2565       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@mr-cal mr-cal left a comment

Choose a reason for hiding this comment

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

FYI @cmatsuoka, jardon is no longer at Canonical, so I don't think any progress is being made on this. We need to decide what to do with his qt-framework content snaps and this PR.

@jardon
Copy link
Author

jardon commented Aug 22, 2024

that is more or less correct. this has been demoted in priority for me, but that doesnt mean that i cant work to get this pushed through.

there are a couple of reasons that i havent worked on this in quite a while. firstly, building and maintaining the sdk and content snaps is unfortunately too big of a burden. i cant build the images in github actions because of resource limitations as well as architecture. yes, theres launchpad, but a new coat of paint doesnt mean its basically abandonware (seriously stand up a gitlab instance and convert all the useful bits of launchpad into a plugin or something). secondly, the current snap architecture increases the amount of work beyond whats reasonable. having to have separate snaps for each combination of QT version and base, dealing with the annoyances of the content interface, as well as dealing with store requests makes this a lot to maintain.

if you can offer good solutions to my issues above, i'd be happy to help.


_SDK_SNAP = {"core22": "qt-framework-sdk"}

_CONTENT_SNAP = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Who owns these snaps?

Copy link
Author

Choose a reason for hiding this comment

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

that would be me.

@jardon
Copy link
Author

jardon commented Sep 9, 2024

For some more context here, I originally wanted to partner with the KDE snap people for creating snaps that would be the building blocks for both extensions. Unfortunately, it seems they got busy with the KDE Ubuntu spin and never responded which is another reason why this stalled.

@keshavbhatt
Copy link

Any update on this?

@jardon
Copy link
Author

jardon commented Oct 14, 2024

@keshavbhatt i have not heard back from canonical

@mr-cal
Copy link
Collaborator

mr-cal commented Oct 15, 2024

Hi @keshavbhatt, we haven't decided on how to proceed with the QT extension and content snaps.

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.

6 participants