snap, hooks: make host-hunspell a system-files interface

[bboozzoo]

The host-hunspell plug was a mount-control interface, which means that the snap could request snapd to set up a mount from/to a specific location. The problem is that the mount is visible in the host mount namespace, i.e. the mounts created a system wide. The mount-control interface was really intended to be used in Ubuntu Core oriented scenarios, when for instance a gadget mounts some external storage at a system wide location.

The particular care of the Firefox snap only really needs access to the hunspell dictionaries on the host. In this case a system-files interface allowing access to /usr/share/hunspell inside the host filesystem (exposed to the snap mount ns at /var/lib/snapd/hostfs) will be sufficient.

This will fix issues like LP#2059195

[zyga]

This looks good to me

[alexmurray]

This will require an update to the firefox snap declaration in the store.

[bboozzoo]

Yes. Do you know if the store can handle the case where the plug name is unchanged? Maybe it'd be better if I changed the name to say host-usr-share-hunspell?

[bboozzoo]

I've renamed the plug to host-usr-share-hunspell to avoid potential name conflicts in snap declaration.

[alexmurray]

Thanks, yep I think this is better.

[bandali0]

Submitted store request for review/approval:

Request for auto-connection of firefox:host-usr-share-hunspell plug of system-files interface

[seb128]

Not a review but a side comment but we should start by targeting nightly and beta which are higher risk channels where we usually land changes to get testing before cherry-picking to stable

[bandali0]

Thanks for the PR. From a quick look and test, it looks good. I've pushed the change to the nightly and beta branches, and the corresponding builds are now pending manual review on the store. Once the approval is granted, I'll test a few other scenarios as well, before merging this into stable.

[ssbatema]

installs and runs on raspberry pi4b+ubuntu core22 (arm64), closing 52!

[bandali0]

Hi @bboozzoo, (all,)

As mentioned above, I'd cherry-picked your changes into nightly and beta earlier, and after closer examination/review and testing several scenarios since then, I found a few issues with the proposed changes:

• With the changes applied, attempting to remove the Firefox snap would fail midway through with the following error:

  error: cannot perform the following tasks:
  - Remove data for snap "firefox" (4111) (unlinkat /var/snap/firefox/common/host-hunspell/en_US.dic: read-only file system)
  

  Which appears to be due to the leftover mount point after the switch away from mount-control, and snapd not cleaning it up.
  As such, I added a commit to restore the mount-control declaration to snapcraft.yaml for the time being and also restored snap/hooks/disconnect-plug-host-hunspell to clean up after ourselves when disconnecting the plug. With this, running sudo snap remove firefox seems to be all good again and the removal completes successfully.

• Your patch set DICPATH to /var/lib/snapd/hostfs/usr/share/hunspell, but it was previously deliberately set to not point directly to the host dictionaries, mainly to avoid potential incompatibility issues between differing versions of hunspell in the snap and outside the snap on the host. So I added a commit restoring the previous DICPATH, and adapting snap/hooks/post-refresh accordingly along with comments explaining the setup and the reasons for it.

• Lastly, related to the first point, since your proposed changes had been live on nightly and beta for a few weeks, we can assume a nonzero number of people already their Firefox snap updated to those revisions, meaning their host-hunspell plug was disconnected without proper cleanup (since the initial proposed changes dropped snap/hooks/disconnect-plug-host-hunspell) so they would run into the aforementioned error if they were to attempt to remove the Firefox snap. For that reason, I added a third commit to attempt a cleanup as part of snap/hooks/post-refresh. I added the commit only to nightly and beta, and not this PR, since none of these changes are merged into stable yet, and so I believe for stable doing the cleanup only in snap/hooks/disconnect-plug-host-hunspell should be sufficient.

Thanks again for the patch, and thanks in advance for reviewing my proposed changes.

EDIT: also cc @lissyx

[bboozzoo]

LGTM, thanks for the update and extensive comments!

[seb128]

Thanks, if it's fine by the snapd reviewers I'm also +1 :)