clean behavior is confusing

[Gustavo Niemeyer]

When using snapcraft in practice I've been often stumbling upon the behavior of "cleaning".

Several small nits that come in mind right now:

1. Forcing a particular stage does not work

If one types "snapcraft build some-part", this should indeed build the part. Right now it skips if it was already done before, which is a fine behavior when a part name is not given, but when the developer requested a specific part, it should actually respect the request rather than assuming a cached result is okay.

2. "snapcraft clean" doesn't always work

For example:

    % snapcraft clean --step snap
    'snap' is not a valid step for part 'qt5-packages'

3. Removing content doesn't work

This is the most obvious way to "clean" when one really cannot tell snapcraft what to do. Just nuke the phase content (e.g. "rm -rf ./snap/"). Instead of taking the cue that the directory is simply not there, snapcraft barfs on internal state:

    % rm -rf snap
    % snapcraft snap
    Skipping pull qt5conf (already ran)
    (...)
    [Errno 2] No such file or directory:
    '/home/niemeyer/src/makemkv/snap/bin/qt5-launch'

To recover from this state I had to nuke the parts/qt5conf path, which is not intuitive.

We need to urgently improve the experience of people re-running stages, as this is foundation of the try-and-run cycle.

[Impact]

 * `snapcraft clean` is a convoluted cli to manage states for parts.

 * This introduces un and re prefixes for the lifecycle commands available for snapcraft

[Test Case]

 * Run snapcraft prime
 * Run snapcraft reprime
 * Verify the lifecycle step is rerun.
 * Repeat for pull, build and stage
 * Run unprime, unstage, unbuild and unpull
 * Run snapcraft reprime <part-name>
 * Verify the lifecycle step for only <part-name> is rerun.
 * Repeat for pull, build and stage
 * Run unprime, unstage, unbuild and unpull for <part-name>
 * Verify only <part-name> is cleaned.
 * Run snapcraft clean with and without <part-name> and -s <step> and verify it still works with a deprecation message shown to use the new syntax.

[Regression Potential]

 * This should had minimal effect on regressions as the clean command is staying for backwards compatibility.
 * The un and re commands will be UI modifications not touching the lower layers of snapcraft that manage states.
 * The "re" commands are totally new feature but with minimal impact (per plugin).

[Gustavo Niemeyer (niemeyer)]

I ended up nuking the whole parts directory because I couldn't get the system to move data correctly to the later stages. :-(

[Gustavo Niemeyer (niemeyer)]

Another one: this command is a very non-obvious way to clean the snap directory:

    snapcraft clean --step=strip

My vote is for removing the idea of "cleaning" altogether, and replacing it by "redoing".

In other words, if I want to remove ./snap, I should be able to just "rm -rf" it. If I want to update its content instead, I should be able to say something like "snapcraft strip --force", for example.

As a side problem related to this, the "strip" phase name is not great. I've reported that by itself on bug #1582515

[Gustavo Niemeyer (niemeyer)]

Sorry, I take back the part of removing cleaning. It's actually nice to be able to clean things up and see how it looks without that specific part on that specific phase. Sort of amazing, actually, that we're able to see the stage area undone, for example.

So, as another strawman proposal I suggest we use the terms "undo" and "redo" for navigating these concepts, with the following command lines:

Current behavior remains the same, ignoring phases if already done:

    snapcraft stage [<part>]

New language for what today is "snapcraft clean [<part>] --step=stage".

    snapcraft stage [<part>] --undo

New behavior, forces the staging phase alone to be undone and redone while previous phases are reused if cached:

    snapcraft stage [<part>] --redo

How does that feel?

[Mark Shuttleworth (sabdfl)]

How about "restage":

  snapcraft restage <part>

... which would both clean and redo.

Mark

[Gustavo Niemeyer (niemeyer)]

That sounds nice, but we'll need the full set in that case:

    repull
    rebuild
    restage
    restrip (*)

and then

    unpull
    unbuild
    unstage
    unstrip (*)

(*) The strip name here is awkward. There's a separate bug about this.

There's no need for unsnap and resnap, though.

Sounds good to me.

[Kyle Fazzari (kyrofa)]

Thanks for the report Gustavo! I'll try to address your concerns one at a time.

(1): To understand the motivation here, we've been trying to make something that behaves similarly to `make`. That is, ideally it understands when things need to be rebuilt, and doesn't build them if it thinks it doesn't need to do so. If one wants to build again, one typically cleans the given target first. We did this since we thought it would be a familiar workflow: run `make foo`, get foo. Run `make foo` again without changes, get "foo is up to date."

Of course, this is where we're _heading_, not where we are. Make works pretty well that way, since its state tracking is pretty trustworthy. In contrast, the current dirty state tracking in snapcraft is still a bit rudimentary. For instance, it still says a given part has already been built/staged/stripped even if one adds a new `stage-package` to the part's YAML. There are of course other things missing from this as well, and we're adding them over time.

Keeping that vision in mind, assuming the dirty state tracking gets to where we want it to be, do you still want the re<step> and un<step> commands?

(2): The per-step cleaning is currently also per-part, where `snap` isn't part of the lifecycle. Are you simply expecting `snapcraft clean --step snap` to remove the generated snap?

(3): Yeah, this is a bug. No need to clean something if it's already gone.

[Gustavo Niemeyer (niemeyer)]

Thanks Kyle.

(1) Yes, can we please have refoo and unfoo commands ASAP? What you describe is a very nice improvement that will be welcome when it lands, but it won't always work and it won't fix the issue with the cleaning command being unfriendly to use. Having a way to force snapcraft to undo+redo the given step will be appreciated nevertheless.

(2) I was actually expecting it to clean up the "snap" directory, which is probably related to the confusion about strip being a bad phase name. I suggest having "unsnap" as an alias to "unstrip" (which may become "unapply" or similar soon), just because it's so obvious to see that snap directory as the snap phase, and because there's no other better meaning for it. Removing the snap at the snapcraft.yaml directory with the expected name would be fine too, I think.

[Gustavo Niemeyer (niemeyer)]

Ping?

[Sergio Schvezov (sergiusens)]

Ok this bug is just about unfoo and refoo?

Just want to comment that refooing something that is a dependency of another thing will trigger a rebuild of that as it would affect it as well.

[Gustavo Niemeyer (niemeyer)]

Yes, just unfoo and refoo for all foo.

refooing dependents once foo refoos is footastic.

[Gustavo Niemeyer (niemeyer)]

More on the topic: snapcraft is warning about cleaning up in situations that it should just do it:

    $ snapcraft snap
    Loaded local plugin for dump
    Loaded local plugin for dump
    The 'pull' step of 'content' is out of date. Please clean that part's
    'pull' step in order to rebuild

Also, there's apparently a bug around this area. Right after the above command, if I actually do what it asks for, it then breaks:

    $ snapcraft clean --step=pull content
    Loaded local plugin for dump
    Loaded local plugin for dump
    Cleaning priming area for content
    Cleaning staging area for content
    Cleaning build for content
    Cleaning pulled source for content

    $ snapcraft snap
    Loaded local plugin for dump
    Loaded local plugin for dump
    Skipping pull mongodb (already ran)
    [Errno 17] File exists:
    '/home/niemeyer/src/snaps/mongodb/mongo22/parts/content/src'

[Kyle Fazzari (kyrofa)]

What is the plugin used for the "content" part? Sounds like its clean_pull might not be quite right.

[Gustavo Niemeyer (niemeyer)]

This is the local x-dump plugin, which does nothing on clean. This is the full plugin:

    class DumpPlugin(snapcraft.BasePlugin):

        @classmethod
        def schema(cls):
            schema = super().schema()
            return schema

        def build(self):
            super().build()
            self.run(['cp', '-a', '.', self.installdir])

[keshavbhatt (keshavnrj)]

i found this something similar to this https://bugs.launchpad.net/snapcraft/+bug/1593868

[Sergio Schvezov (sergiusens)]

Fix for the last comment from Gustavo: https://github.com/snapcore/snapcraft/pull/1416

[Gustavo Niemeyer (niemeyer)]

Sergio!

Any news here? The clean and redo experience with snapcraft sucks badly, for way too long. Can we please get this fixed? This is part of the PRIMARY experience that people have when they first start using the tool, and although people that persist eventually get used to the multiple warts, we should really prioritize this. As in, REALLY. :)

[Igor Ljubuncic (igorljubuncic-deactivatedaccount)]

Adding to this bug:

When re-running snapcraft build, the cleanup message is ambiguous:

"The requested action has already been taken. Consider
specifying parts, or clean the steps you want to run again."

The error message should specifically mention which action has been taken and which steps need to be cleaned.

[Anton Melser (melser-anton)]

Ok, so 2.5 years of total suckfullness and still no movement? Does this suggest that snaps are going to be abandoned? I wanted to submit a fix for ubuntu/microk8s but with this nightmare I might just use minikube instead.

[Gustavo Niemeyer (niemeyer)]

This situation has already been improved, and should be improved further. In fact, Snapcraft was almost entirely restructured since then, and there was a major amount of work on the snapd side as well.

With that said, we try to keep our conversations in this soace respectful, Anton. Venting out doesn't solve the problem, and it doesn't make productive engagement possible.

If you are actually interested on the problem, report what you found and what you expected instead, and include details please so people can understand what's going on.

[Jussi Lind (jussi-lind)]

I'm struggling with this clean-related issue:

https://stackoverflow.com/questions/62683170/snapcraft-doesnt-produce-any-snaps-after-cleaning-the-part

I have to always clean fully in order to get my snap. If I clean only my part, then it says it snaps, but the file will not be there. Maybe I'm doing something wrong, but this is still very confusing.