Make snap more self-contained

[dilyn-corner]

First commit is self-explanatory -- I don't think fdisk, gdisk, or mtools are ever used during the build and only matter for running ubuntu-image.

Second commit resolves a bug that exists in a particular release of mtools in Jammy. I only saw this bug in the case where my Classic gadget snap was installing a firmware file to a bare partition (writing directly to sectors on the image). I don't really know why this doesn't show up when creating Core images.

Third commit adds a required package (python3-apt) which matters when creating a Classic image which adds a custom-ppa to the image (for instance, when building a Classic image which uses a kernel provided by some private Launchpad project).
python3-apt doesn't play nicely with particular Python interpreters. For instance, it isn't sufficient to stage python3-apt on 23.04. Instead, you must stage all of the python3 available in 22.04. Normally you would just add python3 as a stage-package, but because of how Snapcraft handles certain dependencies which may be available in the base snap, we have to explicitly install all the packages python3 would normally get us. (Snapcraft itself has this same problem).

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.35%. Comparing base (3adcd79) to head (07716b7).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #163   +/-   ##
=======================================
  Coverage   87.35%   87.35%           
=======================================
  Files          15       15           
  Lines        3921     3921           
=======================================
  Hits         3425     3425           
  Misses        436      436           
  Partials       60       60           

Flag	Coverage Δ
unittests	87.35% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

[sil2100]

2 of the 3 proposed changes are good. I am a -1 on the mtools work-around, see inline comment for information.

[sil2100]

I must say I'm do not want to take this change. If there is a regression in mtools, I would prefer for the regression to be addressed as an SRU and updated properly in the archive instead. ubuntu-image is a tool which is used for building official Ubuntu image, so it should not use self-compiled tooling from outside of the archive.

[dilyn-corner]

That's fair. I'll try to investigate what patch specifically resolves the issue - there's a post in the mailing list but I can't find a patch or any hosted VCS; I guess I have to dig into the release tarballs.
Once I've got it I'll try to initiative the SRU for it, however that works. Might be slow, I'm at the RISC-V Summit this week.

[dilyn-corner]

mtools is actually more frustrating than I thought it would be, but conveniently the change between 4.0.33 and 4.0.34 (the release cut to fix this regression) is just:

dilyn@Ares:~/Downloads -> busybox diff mtools-4.0.33/mcopy.c mtools-4.0.34/mcopy.c
--- mtools-4.0.33/mcopy.c
+++ mtools-4.0.34/mcopy.c
@@ -107,7 +107,6 @@
 	Stream_t *File=mp->File;
 	Stream_t *Target, *Source;
 	struct MT_STAT stbuf;
-	ssize_t ret;
 	char errmsg[80];
 
 	File->Class->get_data(File, &mtime, 0, 0, 0);
@@ -164,7 +163,7 @@
 	if ((Target = SimpleFileOpen(0, 0, unixFile,
 				    O_WRONLY | O_CREAT | O_TRUNC,
 				    errmsg, 0, 0, 0))) {
-		ret = 0;
+		mt_off_t ret = 0;
 		if(needfilter && arg->textmode){
 			Source = open_filter(COPY(File),arg->convertCharset);
 			if (!Source)

Will test...

[dilyn-corner]

This is actually a funny joke -- it's more complicated, because the Jammy package is actually 4.0.32. Harder to identify what small change is required.

[dilyn-corner]

Update on this outstanding PR (sorry!), I've filed a bug against the mtools package and have started (I think...) the SRU process.

[dilyn-corner]

I'm simply too tired to figure out mtools. Why mcopy is broken is escaping me -- it works on 4.0.31 and breaks for every release after that until 4.0.41. I don't have the energy to work with a codebase that doesn't use version control :) Dropping the vendor patch.

[upils]

I am generally +1 on these changes. Now that I think of it, I even think the python3-docutils pkg in build-packages is also probably not used anymore (a leftover of a previous version of ubuntu-image, written in python). So it should be fine to remove it.

And kpartx is now only used in our test suite, so I think it should not be needed anymore.

Side note: the added stage-packages explicitly reference python3.10. I guess it will have to be updated when we migrate to core24 (planned once it is available to unblock another PR) to use python 3.12.

[dilyn-corner]

Yeah the python version will have to be watched when the base changes. Ideally the *craft team will have a solution for staging python in snaps before core24 releases but I haven't heard any news on that front since the end of last year (this issue also hits snapcraft itself so they have a vested interest in figuring out the cleanest way to use python in a snap).

I've removed python3-docutils and kpartx from the packages lists!

[upils]

LGTM

[upils]

@dilyn-corner would you mind rebasing you branch on main? We merged several branches recently and I would like to make sure your changes pass in CI (some tests are expected to fail but I can tell which ones).

[dilyn-corner]

Rebased, spread has run; indeed the failures seem unrelated!

[sil2100]

Ok, seems sensible.