snap: core20 (with ci) #271
Conversation
Can you point to any documentation that states this requirement? |
Sure, I've updated the description. Let me know if there are any other questions or suggestions. |
|
Trivial ci-only force-push to restore the prior CI skeleton with |
| @@ -8,11 +8,11 @@ description: | | |||
|
|
|||
| grade: stable | |||
| confinement: strict | |||
| base: core18 | |||
| base: core20 | |||
There was a problem hiding this comment.
Hm, probably better will be use latest core component. (core24)
This will be works on all older distribution include Ubuntu 18.04, but we can get access to new feature of snap.
You can live core20, but it will be deprecated soon
There was a problem hiding this comment.
This also requires a fixup of the try syntax, which was removed in later core versions.
I'll just do the smallest change here, to get 28.x and 29.x unblocked. For 30.x there may be a larger overhaul for qt6, so I think a larger bump can be done then.
There was a problem hiding this comment.
Okay, it's real more important for qt6, qt5 depends on older system libraries. For now, core20 will be more save and faster solution.
| @@ -22,27 +22,27 @@ apps: | |||
| # https://docs.snapcraft.io/environment-variables/7983 | |||
| HOME: $SNAP_USER_COMMON | |||
| qt: | |||
| command: desktop-launch bitcoin-qt | |||
| command: bin/desktop-launch bin/bitcoin-qt | |||
| plugs: [home, removable-media, network, network-bind, desktop, x11, unity7] | |||
There was a problem hiding this comment.
| plugs: [home, removable-media, network, network-bind, desktop, x11, unity7] | |
| plugs: [home, removable-media, network, network-bind, desktop, x11, wayland, unity7] |
Yes, it depends on first from a building application and deploy qt plugins, but x11 now is deprecated... Wayland, more relevant for linux desktops
There was a problem hiding this comment.
I agree, but I don't think this is related to changing core18 to core20. (The wayland plug should be available in both, so it should be possible to add it in a separate pull request)
However, I don't think it will work, see bitcoin/bitcoin#19950
In any case, I think a separate pull request with exact steps on how to test and confirm the change would be better.
There was a problem hiding this comment.
You are right
just ignore my notations
|
Is this rfm, or does it need more review and testing? |
|
|
||
| apps: | ||
| daemon: | ||
| command: bitcoind | ||
| command: bin/bitcoind |
There was a problem hiding this comment.
Why are these paths changing? The new binary location shouldn't make a difference for releases, and this would just end up breaking users.
There was a problem hiding this comment.
I don't have the build logs anymore, but IIRC this is just a warning that was turned into an error.
It should be possible to test by running the snap build on current main (before this pull request) and observe a warning about the binary paths.
When applying this diff, the warning will go away on main.
Similarly, with this pull, there is an error about the paths and it goes away with this patch.
Let me know if I should re-test this, or if I should submit this as a separate bugfix commit/pull.
There was a problem hiding this comment.
and this would just end up breaking users.
I don't think this changes or breaks any external commands. At most it could change the paths, when a user deploys a shell in the container, but probably the paths were requiring the bin prefix previously as well.
(Happy to re-test this as well, just let me know).
There was a problem hiding this comment.
I did not see the warning or error in the logs, but I do see that the install path remains the same.
There was a problem hiding this comment.
I did not see the warning or error in the logs
Interesting. For reference, I've recreated the error on top of current master (with this pull merged) with a diff in my riscv64 pod:
# uname --machine && git diff && snapcraft --use-lxd
riscv64
diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml
index 7b53752..824fcc9 100644
--- a/snap/snapcraft.yaml
+++ b/snap/snapcraft.yaml
@@ -12,7 +12,7 @@ base: core20
apps:
daemon:
- command: bin/bitcoind
+ command: bitcoind
plugs: [home, removable-media, network, network-bind]
environment:
# Override HOME so the datadir is located at
Launching a container.
Waiting for container to be ready
Waiting for network to be ready...
Skipping pull desktop-qt5 (already ran)
Skipping pull bitcoin-core (already ran)
Skipping build desktop-qt5 (already ran)
Skipping build bitcoin-core (already ran)
Skipping stage desktop-qt5 (already ran)
Skipping stage bitcoin-core (already ran)
Skipping prime desktop-qt5 (already ran)
Skipping prime bitcoin-core (already ran)
Failed to generate snap metadata: The specified command 'bitcoind' defined in the app 'daemon' does not exist.
Ensure that 'bitcoind' is installed with the correct path.
Run the same command again with --debug to shell into the environment if you wish to introspect this failure.|
Does this have any effects on the launchpad builder that we use to actually push to the snap store? |
I haven't tried it, so I can't promise anything, but I suspect that the existing docs are still applicable: https://github.com/bitcoin-core/packaging/blob/main/snap/local/build.md If anything, this only changes the base, but this should be handled already correctly. I suspect the selection will look something like: |
|
ACK ebc7d2e |
|
Finally !!! |
The
core18builder is insufficient, now that glibc in the release builds was bumped (bitcoin/bitcoin#29987).Fix it by bumping to
core20, becausecore18is also deprecated and only available in the snapcraft7.xtrack according to https://snapcraft.io/docs/base-snaps#core18.The change comes with required fixups in the
snapcraft.yaml.This change also comes with a required switch to lxd in the CI. This is, because "For core20, Multipass is the default provider on all platforms.", according to https://canonical-snapcraft.readthedocs-hosted.com/en/latest/howto/select-a-build-provider/#core20-override-methods. However, multipass by default uses a QEMU KVM driver, according to https://canonical.com/multipass/docs/driver#p-74200-default-drivers. So just use lxd in the CI, as KVM isn't available.