Missing bits to allow snaps to be generated using mkosi #281

keszybz · 2018-11-04T16:41:06Z

The patch for mksquashfs args is fairly ugly, I know. Suggestions for alternative approaches are very much welcome.

Instructions:

$ git clone https://gitlab.com/keszybz/fedora29
$ cd fedora29
$ sudo SNAP_VERSION=$(date +%Y.%m.%d) SNAP_ARCH=amd64 mkosi -i -f --default mkosi.default.fedora -r 29 -o fedora29.$(date +%Y.%m.%d).amd64.snap
$ sudo SNAP_VERSION=$(date +%Y.%m.%d) SNAP_ARCH=i386 mkosi -i -f --default mkosi.default.fedora -r 29 --arch=i686 -o fedora29.$(date +%Y.%m.%d).i686.snap

zyga

Thank you for writing snap support for mkosi. I added one comment about the name of the post-installation script but other than that +1.

zyga · 2018-11-04T16:58:04Z

mkosi

@@ -180,15 +180,17 @@ GPT_FOOTER_SIZE = 1024*1024

 GPTRootTypePair = collections.namedtuple('GPTRootTypePair', 'root verity')

-def gpt_root_native():
-    """The tag for the native GPT root partition
+def gpt_root_native(arch: str) -> Tuple[uuid.UUID, uuid.UUID]:


Nice type annotations :-)

zyga · 2018-11-04T17:00:06Z

mkosi

@@ -2560,6 +2583,7 @@ def parse_args() -> CommandLineArguments:
    group.add_argument("--build-dir", help='Path to use as persistent build directory', metavar='PATH')
    group.add_argument("--build-package", action=CommaDelimitedListAction, dest='build_packages', default=[], help='Additional packages needed for build script', metavar='PACKAGE')
    group.add_argument("--postinst-script", help='Postinstall script to run inside image', metavar='PATH')
+    group.add_argument("--postinst2-script", help='Postinstall script to run outside image', metavar='PATH')


I don't know if there is any precedent for naming thing like that but perhaps --postinst-outer-script would be more direct, the 2 is a bit magic (granted the help message is clear and explicit).

poettering

Hmm, can't say I like the "postinst2" moniker. I think it's well understood that "postinst" stuff runs in context of the package manager or close to it (since the term is used there already), maybe we should pick for this stuff a completely new name, possibly "mkosi.finalize" or "mkosi.shrinkwrap" or "mkosi.finish" or so?

poettering · 2018-11-05T19:13:40Z

mkosi

    with complete_step('Creating squashfs file system'):
        f = tempfile.NamedTemporaryFile(dir=os.path.dirname(args.output), prefix=".mkosi-squashfs")
-        run(["mksquashfs", os.path.join(workspace, "root"), f.name, "-noappend", *comp_args],
+        run(["mksquashfs", os.path.join(workspace, "root"), f.name, *comp_args],


hmm, I am kinda afraid that this might end up in an inflation of similar options sooner or later for all the other tools, and hence I'd prefer if we'd find a generic solution for this so that we can cover other tools with a similar approach.

here's one idea: maybe, let's change run() to redirect the executed command line to ./mkosi.tool-<command> if that file exists and is executable. i.e. if you want to replace or tweak the "mksquashfs" command you'd drop a shell script in called `./mkosi.tool-mksquashfs", and it would just exec the real mksquashfs but add in some args. Such a scheme could eventually be useful for allow other, less established distros to be hooked in, i.e. if "./mkosi.tool-pkg-install" or so exists we'd use that tool instead of apt-get/dnf/...

poettering · 2018-11-05T19:14:53Z

mkosi

@@ -2529,6 +2539,7 @@ def parse_args() -> CommandLineArguments:
    group.add_argument('-r', "--release", help='Distribution release to install')
    group.add_argument('-m', "--mirror", help='Distribution mirror to use')
    group.add_argument("--repositories", action=CommaDelimitedListAction, dest='repositories', help='Repositories to use', metavar='REPOS')
+    group.add_argument('--arch', help='Override the architecture of installation')


let's call this "architecture" please, this is exotic enough to not benefit from abbreviation

poettering · 2018-11-05T19:16:08Z

mkosi

@@ -3745,6 +3759,10 @@ def check_root():
    if os.getuid() != 0:
        die("Must be invoked as root.")

+def check_native(args: CommandLineArguments) -> None:
+    if args.arch is not None and args.arch != platform.machine() and args.build_script:
+        die('Cannot (currently) override the architecture and run build commands')


We could eventually make this useful for #138 (comment)

Right now, when I call dnf for archs others than amd64 and i386, installation fails. But they might work if binfmt is set up properly. I need to figure this out. If that is true, then #138 would just work.

@keszybz This is documented here as a way that works (at least for me): https://wiki.mageia.org/en/Using_DNF#Setting_up_a_container_for_a_non-native_architecture

poettering · 2018-11-05T19:17:29Z

README.md

+
+* `mkosi.postinst2` may be an executable script. If it exists it is
+  invoked as last step of preparing an image, from the host system.
+  It is called as `mkosi.postinst2 final $ROOT`, where `$ROOT` is the


why is this not called for the "build" operation?

poettering · 2018-11-06T10:12:31Z

btw, i think another option would be to define "snap" as output type, and it would result in the right mksquashfs options to be picked. I think snap is well established enough to deserve its own first-level output mode

keszybz · 2018-11-06T12:58:03Z

Updated as suggested:

mkosi.postinst2 is renamed to mkosi.finalize
it is also called with "build"
mkosi.mksquashfs-tool will be autodetected, but can also be overridden using configuration, similarly to other options
--arch is renamed to --architecture

keszybz · 2018-11-06T13:11:40Z

another option would be to define "snap" as output type

I considered that, but I don't think we're quite there yet. This only work "base" snaps, for now, because any kind of layering is not implemented. Also, the output is not really verified, etc. I'd prefer to "start small", without declaring this any kind of official snap output, and maybe add "snap" as a format later, if it turns out that people actually want to use mkosi to produce snaps.

poettering

hmm, regarding the discovery of mkosi.mksquashfs-tool: my idea was to do this automatically, for all tools we invoke, as part of the run() function. i.e. not specific to mksquashfs but automatically for all tools we use...

poettering · 2018-11-06T14:05:27Z

mkosi

+    if args.finalize_script is None:
+        return
+
+    with complete_step('Running external postinstall script'):


should be updated: "external postinstall" → "finalize". let's not confuse users with different terminology for the same stuff

poettering · 2018-11-06T14:08:29Z

mkosi

+    args_find_path(args, 'build_sources',   ".")
+    args_find_path(args, 'build_dir',       "mkosi.builddir/")
+    args_find_path(args, 'postinst_script', "mkosi.postinst");
+    args_find_path(args, 'output_dir',      "mkosi.output/");


not that it matters, but is there are a reason for using '' quotes on the second parameter, but "" quotes on the third?

It's written in the commit message. For visual distinction between variable names and paths.

poettering · 2018-11-06T14:09:14Z

mkosi

@@ -3005,6 +3005,12 @@ def find_skeleton(args: CommandLineArguments) -> None:
    if os.path.isfile("mkosi.skeleton.tar"):
        args.skeleton_trees.append("mkosi.skeleton.tar")

+def args_find_path(args: CommandLineArguments, name: str, path: str) -> None:
+    if getattr(args, name) is not None:
+        return None


why does this return None here? it shoud not return anything at all, no?

Left-over from previous version of the code. I'll remove it.

keszybz · 2018-11-06T15:54:24Z

We call mount, btrfs, dnf, yum, chattr, sfdisk, sync, losetup, mkswap, mkfs.fat/ext4/btrfs/xfs, cryptsetup, umount, systemctl, systemd-nspawn, swupd-extract, debootstrap, pacman, pacman-key, pacstrap, gpg-connect-agent, fuser, zypper, git, tar, mksquashfs, sfdisk, dd, veritysetup, sbsign, xz/pxz, qemu-img, qemu*, gpg, bmaptool. I guess we could allow wrappers for all those tools, but I don't see the use case. We call mksquashfs just once, and in a fairly simple way, so it's easy for the user to add a few options. For many tools we either make a few different calls with completely different options, or even if we call them just once, we construct an elaborate command lines. Overriding those cases is going to be pretty hard, because the override script can either just pass through all the options, in which case it case just append some options that don't conflict, or it must "understand" all options and filter them or such. So I'd prefer to add support for overrides for specific tools as the use cases come up.

Edit: I guess that this could be pretty useful for the qemu commands. But I'd still prefer to do it as a separate PR.

This is actually more readable because the variable name definitions and paths are defined at one place and not spread over all the itty bitty functions. I used '' for the variable names and "" for paths. I think that makes the two types of arguments a bit more distinguishable.

For example, snaps have to be generated with "-noappend -no-xattrs -no-fragments -comp xz". The last argument is configurable through Compress=xz, but the other ones weren't so far. Let's just override the mksquashfs executable. To be nice and flexible, 'mkosi.mksquashfs-tool' will be autodetected. It may be also specified explicitly: [Output] Mksquashfs=/some/special/mksquashfs If specfied explicitly, an argument list may be included: [Output] Mksquashfs=mksquashfs -noappend -no-xattrs -no-fragments When args are present, they replace "-noappend" that we add by default. The arguments for compression (-comp xx) is always added if configured by Compress=. This makes the two settings orthogonal.

v2: - s/arch/architecture/g - add an optional line in the status output

This is similar to 'mkosi.postinst', but is called in the host. This makes it much easier to copy-in files. It is also much quicker when creating images for a foreign architecture. v2: - s/postinst2/finalize/g - call the script first with "build" too - pass the root directory as $BUILDROOT (like rpmbuild) and not as a positional argument. (This is easier to consume for scripts and also easier to extend with additional variables in the future.) - update to use args_find_path()

keszybz · 2018-11-06T15:57:27Z

Updated with the two simple fixes. I left out mkosi.*-tool support out for now.

zyga reviewed Nov 4, 2018

View reviewed changes

poettering requested changes Nov 5, 2018

View reviewed changes

keszybz force-pushed the snap-support branch from 4eaa348 to a021a7b Compare November 6, 2018 12:46

poettering requested changes Nov 6, 2018

View reviewed changes

keszybz added 4 commits November 6, 2018 16:54

Allow forcing the installation architecture

8058bd3

v2: - s/arch/architecture/g - add an optional line in the status output

keszybz force-pushed the snap-support branch from a021a7b to b8c1a39 Compare November 6, 2018 15:57

poettering approved these changes Nov 6, 2018

View reviewed changes

poettering merged commit a140514 into systemd:master Nov 6, 2018

keszybz deleted the snap-support branch November 8, 2018 12:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Missing bits to allow snaps to be generated using mkosi #281

Missing bits to allow snaps to be generated using mkosi #281

keszybz commented Nov 4, 2018

zyga left a comment

zyga Nov 4, 2018

zyga Nov 4, 2018

poettering left a comment

poettering Nov 5, 2018

poettering Nov 5, 2018

poettering Nov 5, 2018

keszybz Nov 6, 2018

Conan-Kudo Nov 19, 2018

poettering Nov 5, 2018

poettering commented Nov 6, 2018

keszybz commented Nov 6, 2018

keszybz commented Nov 6, 2018

poettering left a comment

poettering Nov 6, 2018

poettering Nov 6, 2018

keszybz Nov 6, 2018

poettering Nov 6, 2018

keszybz Nov 6, 2018

keszybz commented Nov 6, 2018 •

edited

Loading

keszybz commented Nov 6, 2018

Missing bits to allow snaps to be generated using mkosi #281

Missing bits to allow snaps to be generated using mkosi #281

Conversation

keszybz commented Nov 4, 2018

zyga left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

poettering left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

poettering commented Nov 6, 2018

keszybz commented Nov 6, 2018

keszybz commented Nov 6, 2018

poettering left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keszybz commented Nov 6, 2018 • edited Loading

keszybz commented Nov 6, 2018

keszybz commented Nov 6, 2018 •

edited

Loading