Refactor UpCommand

[lann]

The most complex refactoring is easier to read in just the second commit: 3073adb

[itowlson]

The refactoring looks great - love getting that knot of application origins untangled and getting a unified path for figuring out the trigger args.

The request changes is for an integration test change that seems unrelated to all this, but appears to be removing a desired check that we've had regressions on in the past. But I may be misunderstanding the change!

[itowlson]

This appears to be changing the test - the test is meant to assert that help works even when the app has not been build (i.e. source failed to resolve at all). I.e. that spin new > spin up --help works correctly. This seems (if I'm reading it right) to be using a file that exists but isn't valid Wasm, which isn't what this test wants to prove What am I missing?

[lann]

Ah, I didn't realize this was testing that intentionally; indeed this is a regression for that situation. The basic problem is that deep in spin_trigger::locked::build_locked_app we call canonicalize on all our paths, which requires those paths to exist. Maybe I can resolve that problem...

[itowlson]

Yes, I think that was something Brian hit when trying to unify around the locked app, and is (at least part of) why the local-vs-remote code paths came to diverge.

[lann]

OK it looks like that canonicalize just wasn't (ever?) necessary; Application paths appear to come pre-absolutized.

[itowlson]

Certainly the local loader tries to absolutise paths as early as possible.

One of these days I want AbsolutePath and RelativePath types so we can document this behaviour at the type level and actually enforce that it remains true...

[fibonacci1729]

I was just testing this with a malformed spin.toml that is an http trigger, not sure if this has been resolved just yet but i see the following notably missing the trigger options:

❯  spin up -f examples/http-rust -h
spin-up
Start the Spin application

USAGE:
    spin up [OPTIONS]

OPTIONS:
        --direct-mounts         For local apps with directory mounts and no excluded files, mount
                                them directly instead of using a temporary directory
    -e, --env <ENV>             Pass an environment variable (key=value) to all components of the
                                application
    -f, --from <APPLICATION>    The application to run. This may be a manifest (spin.toml) file, a
                                directory containing a spin.toml file, or a remote registry
                                reference. If omitted, it defaults to "spin.toml"
    -h, --help
    -k, --insecure              Ignore server certificate errors from bindle server or registry
        --temp <TMP>            Temporary directory for the static assets of the components

[itowlson]

Ugh, good catch. I bet an actually malformed spin.toml is a whole new failure mode. Guessing it gets past the "no app so show default trigger options" stage, but then can't deserialise the file to work out the trigger and so gives up rather than calling to help-only trigger. It's an existing problem though, not an issue with this PR; I'd do it as a separate issue.