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

extensions/kde-neon: use platform snap as plug name #3596

Merged
merged 2 commits into from
Nov 25, 2021

Conversation

ppd
Copy link
Contributor

@ppd ppd commented Nov 16, 2021

Using the same plug name "kde-frameworks-5-plug"
for all providers leads to problems when an
application updates its base or changes its default
provider because a new framework snap gets published
for the same base.

In that case, snapd will keep the old framework snap
connected and will happily connect the new framework
snap in addition to the old one.
This breaks applications because they won't find the correct
framework snap mounted at $SNAP/kf5.

Prevent this by using the same strategy that the gnome
extensions use: plug name = content snap name

See https://forum.snapcraft.io/t/snap-accumulates-content-interface-connections-to-multiple-platform-snaps/27523/8

  • Have you followed the guidelines for contributing?
  • Have you signed the CLA?
  • Have you successfully run ./runtests.sh static?
  • Have you successfully run ./runtests.sh tests/unit/project_loader/extensions?

I built a snap (anki-ppd) with this change and everything works as expected. Also CC @jriddell.
I'm not sure about the subsystem prefix; it seemed more complete than just "extensions".

Copy link
Collaborator

@sergiusens sergiusens 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, thanks.

snapcraft/internal/project_loader/_extensions/kde_neon.py Outdated Show resolved Hide resolved
@ppd ppd force-pushed the kde-neon-plug-name branch from 921e3b4 to 6829cb9 Compare November 24, 2021 08:09
Using the same plug name "kde-frameworks-5-plug"
for all providers leads to problems when an
application updates its base or changes its default
provider because a new framework snap gets published
for the same base.

In that case, snapd will keep the old framework snap
connected and will happily connect the new framework
snap in addition to the old one.
This breaks applications because they won't find the correct
framework snap mounted at $SNAP/kf5.

Prevent this by using the same strategy that the gnome
extensions use: plug name = content snap name

See https://forum.snapcraft.io/t/snap-accumulates-content-interface-connections-to-multiple-platform-snaps/27523/8
@ppd ppd force-pushed the kde-neon-plug-name branch from 6829cb9 to 5b5d7fb Compare November 24, 2021 08:35
@ppd
Copy link
Contributor Author

ppd commented Nov 24, 2021

I also removed the manual connection in the spread test. It is a) not required any more because auto-connect has been granted, and b) the plug name is different now, so we'd have to change it anyway.

@codecov-commenter
Copy link

Codecov Report

Merging #3596 (80ce3b9) into master (3148f1a) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3596   +/-   ##
=======================================
  Coverage   91.25%   91.25%           
=======================================
  Files         278      278           
  Lines       19375    19375           
=======================================
  Hits        17680    17680           
  Misses       1695     1695           
Impacted Files Coverage Δ
...ft/internal/project_loader/_extensions/kde_neon.py 100.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3ab5323...80ce3b9. Read the comment docs.

Copy link
Collaborator

@sergiusens sergiusens left a comment

Choose a reason for hiding this comment

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

thank you!

@sergiusens sergiusens merged commit ad0800a into canonical:master Nov 25, 2021
@ppd ppd deleted the kde-neon-plug-name branch November 25, 2021 13:22
ppd added a commit to ppd/qelectrotech-source-mirror that referenced this pull request Jan 13, 2022
Now that canonical/snapcraft#3596 has been
released in snapcraft 6.0.1, drop the prompt that tells users to
disconnect the old framework snap.

Also drop --enable-experimental-extensions from CI  because kde-neon
has been declared stable for core20.
ppd added a commit to qelectrotech/qelectrotech-source-mirror that referenced this pull request Jan 14, 2022
Now that canonical/snapcraft#3596 has been
released in snapcraft 6.0.1, drop the prompt that tells users to
disconnect the old framework snap.

Also drop --enable-experimental-extensions from CI  because kde-neon
has been declared stable for core20.
ppd added a commit to ppd/qelectrotech-source-mirror that referenced this pull request Jan 20, 2022
Now that canonical/snapcraft#3596 has been
released in snapcraft 6.0.1, drop the prompt that tells users to
disconnect the old framework snap.

Also drop --enable-experimental-extensions from CI  because kde-neon
has been declared stable for core20.
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