Proposal: architectures keyword for snap CI systems

My point is that it ignores the conversation without any rationale for why. Even if we don’t end up with the syntax proposed there, the conversation was extensive and fruitful, and it’d be wise not to ignore it.

What you read diverges from what’s written. That very literally says “architecture amd64 runs on all”.

Let’s please meet to discuss these ideas in a place with more bandwidth.

@niemeyer and I had a nice chat about this a few minutes ago, and agreed to tweak the syntax proposed here to turn it into more exact/readable list of objects instead of a single object with multiple keys. I’ve updated the proposal to reflect (remember you can hit the pencil icon to see the old proposal, as well as the diff).

Thanks for writing the proposal down, Kyle.

Looking at the repetition in some of these cases got me thinking if we should further specify that an entry such as:

architectures:
    - amd64

is exactly equivalent to:

architectures:
    - build-on: amd64
      run-on: amd64

With that, we blend the two syntaxes and turn one into a special case of the other. This would also support mixing, so that we can have three simple entries, and one special for ignoring errors for example.

That avoids much of the repetition in the samples above.

@niemeyer well, that’s the syntax we support today, but your suggestion changes its meaning. The current meaning of that syntax given in the NEW syntax is this:

# old:
architectures: [amd64]

# new equivalent meaning
architectures:
  - build-on: all
    run-on: [amd64]

Granted, it doesn’t change it much, but it’s enough that it worries me. I don’t want to break people already using this keyword.

It does change its meaning, but this is a larger problem that we cannot avoid, in the sense that either we make it awkward because the two different syntaxes mean something completely different, or we adopt the different syntax in the two cases.

Before making a decision, we might ask the store team to have a quick look and see how many of our snaps are multi-arch today. This will give us an idea of the actual impact.

The store doesn’t have access to the publisher’s intent using the current syntax (snapcraft.yaml), just the resulted revisions. Considering that multi-arch means: the same snap revision (blob) targeting more than one architecture. We have about 50 snaps (snap_ids) with at least one revision in this scenario.

In that list there are snaps that were not updated for more than one year, some that were never released to stable and some that were already modified to build architecture-specific revisions. The list can be refined as needed, but at this level (50) manual review might be more effective.

Also note that, due to the lack of a snapcraft.yaml, this list doesn’t include any snaps where the developer uses a single architecture in the architectures field (e.g. architectures: [amd64]).

It may be worth pointing out that changing the meaning of existing syntax will have impact in other ways, too: existing documentation, any blog posts out there referencing this behavior, etc. Our ISVs tell us that breakage slows down their flow tremendously, I naturally like to avoid that wherever possible.

I am conscious that we will break, we should try not to, but the current behavior is already broken and having it behave differently already requires us to update documentation. This is also a rather obscure feature.

With all that said I do agree with @niemeyer. We could compromise by using a new keyword and give up the nice name of architectures as part of a feature we have had a long desire to kill (for which in the case of using a new keyword we will only deprecate with warnings), but given how obscure this is, is it really worth it?

Alright, I am also conscious of the significant conversation that has already happened around this feature. I’m okay with being outvoted! I’ll get started on the change.

So are we good to move forward with the implementation? @niemeyer I believe you made some final comments on IRC which I have missed.

I’m still trying to avoid the syntax issue mentioned above without introducing the ambiguity.

How about this minor tweak in the proposal we previously discussed that was detailed by @kyrofa above:

We change the default value when run-on: is missing so that it matches build-on:. In other words, this:

architectures:
    - build-on: [i386, amd64]

becomes equivalent to:

architectures:
    - build-on: [i386, amd64]
      run-on: [i386, amd64]

which is also equivalent to:

architectures: [i386, amd64]

This may sound pointless since that latter form is already supported, but the point of doing that is actually having this:

architectures:
    - build-on: i386
    - build-on: amd64

being equivalent to:

architectures:
    - build-on: i386
      run-on: i386
    - build-on: amd64
      run-on: amd64

That default for run-on is more useful than having all as the default, since given the rule that an architecture can only appear once in only one of the run-on statements, by definition we can only have all when we have a single architecture entry for the snap.

How does that sound?

1 Like

This does flow in a natural way in the sense that it feels intuitive to think that the snap built could run where we say it can build.

To be on the same page wrt run-on: all, does this mean that,

architectures:
    - build-on: amd64

or

architectures:
    - build-on: [amd64, i386]

have an implicit run-on: amd64 (or run-on: [amd64, i386]) and that if it does need to run everywhere, run-on: all should be specified explicitly?

Right, the implicit is always to just copy it over, so it’s very intuitive. In that example, this would be equivalent to:

architectures:
    - build-on: [amd64, i386]
      run-on: [amd64, i386]
1 Like

Wouldn’t this cause the builder to spin an amd64 machine AND and i386 machine, and BOTH of those machines produce a package that claims it runs on both amd64 and i386? That doesn’t sound desirable, because it will produce two supposedly identical packages but built differently…

I think the default should be to run-on only the architecture of the builder when nothing is specified.

So it would look like:

architectures:
- build-on: [amd64, i386]

# converts into equivalent of:
architectures:
- build-on: amd64
  run-on: amd64
- build-on: i386
  run-on: i386

These snippets are both valid, but they convey different intentions, so we cannot map one into the other. The first says build “on either amd64 or i386 something that can install on both amd64 and i386”, while the second says “build on amd64 something that can run on amd64” and “build on i386 something that can run on i386”.

A good quality of that syntax is that I pretty much read what was written down.

So you’re saying that build-on: [amd64, i386] will only build one package using whichever builder is first to respond of one of those architectures? i.e. it will not build on both architectures for each commit…

That is correct, this is a variation of Example 5 in the original proposal.

@kyrofa mind updating the proposal with the latest agreements?

@lucyllewy A simple way to understand it is this: each entry under architectures will produce a single snap.

ok, that makes it clearer, thanks :slight_smile: We’ll need to be sure to document that explicitly when the feature lands so that others don’t think the way I did :smiley:

1 Like

Done! I’ll also get started on more formal documentation, although it’ll look pretty much like this.