Deprecate intermixed spin up and trigger args in favor of spin up [args] -- [trigger args]

[cardoso]

As means to unblock #1198 + taking inspiration from #1155 and #1207

Since the trigger args in spin up aren't known ahead of time, and actually meant to be forwarded to a spin trigger [...] command, allowing them to be intermixed with spin up args is not conventional. It appears the expected pattern is "cmd [args] -- [next args]"

Compare with cargo run which is analogous:

NAME
       cargo-run — Run the current package

SYNOPSIS
       cargo run [options] [-- args]

DESCRIPTION
       Run a binary or example of the local package.

       All the arguments following the two dashes (--) are passed 
to the
       binary to run. If you’re passing arguments to both Cargo an
d the
       binary, the ones after -- go to the binary, the ones before
 go to
       Cargo.

This avoids any kind of ambiquity with future spin up args and makes parsing easier possible in clap 4.

[vdice]

Thanks @cardoso. This seems like a sensible pattern to consider. Curious to get input from @itowlson and @calebschoepp.

As you've mentioned, if we continue to allow intermixed up opts and trigger opts, it appears we're tied to clap 3.x, at least pending the upstream issue mentioned in #1198 (comment).

This will apply to the anticipated spin watch feature as well, plus spin build until/unless we decide to remove spin build --up (#1089).

[cardoso]

I would disregard that clap issue. It's super low priority and the motivation wouldn't be allowing this. Seems like a good time to think about removing the rough edges in these commands. The cool thing is Clap 4's derive should remove a lot of the CLI code that's just in the way of the good stuff

[calebschoepp]

I'm a big fan of this change. This was my initial assumption of how trigger args worked until I dug into the code. My only real concern is backwards compatibility. Perhaps we could try to get this change in before 1.0? Not sure that we have enough time to do that though. @melissaklein24 might have a better sense if that is realistic time wise.

[itowlson]

From the user point of view, they shouldn't need to care that the trigger is a separate process or has separate arguments. (And especially that half our common up are actually defined on the trigger.) The user just wants to run their app, and they want it to listen on port 4545 instead of the default.

To me, this is not like cargo run where there is a clear delineation of cargo needing a set of controls for how to build the app, and the app needing a separate set of controls for doing app-specific stuff. This is all Spin, it's all running the user's app. By contrast, I could buy into separating the up args in spin build --up, because of the clear distinction between "controlling the build" and "controlling app execution." Whereas in spin up the handoff to the trigger is deliberately kept as an internal implementation detail.

[itowlson]

That said, the current convention is inherited behaviour from when triggers ran in-process. We tried to keep it the same when we moved triggers out of process; but the abstraction does leak, and, as @cardoso notes, it blocks other UI changes such as flagless app source. So there is certainly grounds for reviewing it. I'd be reluctant to make such a significant breaking change when we don't have much time to prepare users for it before stabilisation though...

[cardoso]

@itowlson that could work if spin up itself had no args and the triggers have all the args, with some effort, but I feel I'm starting to lack context to provide a fully educated opinion beyond personal preference and what seems to play well with other CLI tools.

Particularly, I'm not a big fan of running a command without knowing that it delegates to some other process which could invalidate whatever property I set or expected of the first one. I'm sure I could explain it better, but it would be a long rant around nodejs, npm, etc...