fix(cli): don't validate flags when requesting --help

[helderco]

Fixes #7398

The --help flag is now global (interspersed). So the help will no longer be shown next to the command where it’s defined if there’s more to the right. It’s position in the chain no longer matters, it will be the same as adding it to the end. This was needed to look ahead and skip some validations while that flag wasn’t parsed yet, and while it was possible to preserve the previous behavior, making it global seems more consistent with CLI conventions, besides being simpler.

Before

❯ dagger call cli --help file
Develop the Dagger CLI

USAGE
  dagger call cli <function>

After

❯ dagger call cli --help file
Build the CLI binary

USAGE
  dagger call cli file [arguments] <function>

Refactor

Did a bit of refactoring to make the logic easier to follow and encapsulate some responsibilities better in functions.

There’s also some groundwork in preparation for:

• Allow calling core functions from the CLI #6947

Silence errors

Error showing twice after:

• cli: Cloud traces support via dagger login #7125

Error: required flag(s) "source" not set
Error: required flag(s) "source" not set
exit status 1

Changed telemetry span

The "initialize" telemetry span was being applied to processing constructor flags and building out the next commands, which showed the module loading as failed, and context upload step adding noise to a missing required flag error, for example. So moved only the module initialization to it’s own function to limit the "initialize" span scope.

Before

❯ dagger call cli
✘ initialize 12.1s
! required flag(s) "source" not set
  ✔ Module.initialize: Module! 1.4s
  ✔ ModuleSource.resolveFromCaller: ModuleSource! 0.6s
    ✔ upload /Users/.../dagger from xxx (client id: oq8yf7p509lm3o9qqhden90vx) (exclude: **/.git) (include: ci/**/*, **/go.mod, **/go.sum, **/go.work, **/go.work.sum, **/vendor/, **/*.go, dagger.json) 0.6s

Error: required flag(s) "source" not set

After

❯ dagger call version string

✔ initialize 6.7s
✘ prepare 0.0s
! required flag(s) "source" not set

Error: required flag(s) "source" not set
Run 'dagger call --help' for usage.
exit status 1

Removed aliases

When they were added, the API returned the original name used by the SDK. Somewhere along the way, to avoid the need for SDKs to handle naming conventions, the engine started to store the original name under a private field, and converted name to the normalized GraphQL convention for fields. So, for a while now, the aliases haven’t been the name that users see in code, which was why we added them in the first place.

[nipuna-perera]

Hey @helderco I have a request. It's not necessary to be done as part of this PR but I wanted to bring it to the team's attention. I am trying to run some dagger functions in parallel within my CI system (Jenkins). Since the function calls are the first thing dagger related that happens in the pipeline, I have a separate stage to run dagger call --help to "initialize" the engine (basically pull the image and get it ready).

I see this same pattern in the dagger's own CI too. There is a "Wait for engine to be ready" step in the actions.

Now the issue is, my modules are in a daggerverse folder and I when I do dagger call --help I have to pass in a module like dagger call -m ./mymodule --help. Which isn't that ergonomic. Is there a better way to get the engine to "initialize" without calling --help? If not, can you make the dagger call --help not return a nonzero when it's outside of a module folder?

[helderco]

Hey @nipuna-perera, you'll be able to do that with:

• cli: allow calling core functions directly #7310

Then I'd suggest you call the version core function:

❯ dagger -m- call version
v0.11.5

That has much less output but still informative and it can be run from anywhere. Doesn't need to load any module.

[nipuna-perera]

Hey @nipuna-perera, you'll be able to do that with:

• cli: allow calling core functions directly #7310

Then I'd suggest you call the version core function:

❯ dagger -m- call version
v0.11.5

That has much less output but still informative and it can be run from anywhere. Doesn't need to load any module.

That is perfect! Thank you! Looking forward to these DX cleanup/improve PRs getting merged :)

[jedevc]

The "initialize" telemetry span was being applied to processing constructor flags and building out the next commands, which showed the module loading as failed, and context upload step adding noise to a missing required flag error, for example. So moved only the module initialization to it’s own function to limit the "initialize" span scope.

Something in this change seems to effectively revert #7347

I think this is because all cli args were moved to be part of that span - including user args. I'm not sure what the answer is here, ideally we should keep the spans for loading these attached to the intitialize context, but I agree that we shouldn't unfold this on an error.

Potentially, we should only error the entire span if there's a loading error, but on a required flag error, the span shouldn't error - initialization has succeeded, and we can propagate the error back up, but not error the span.

[helderco]

The "initialize" telemetry span was being applied to processing constructor flags and building out the next commands, which showed the module loading as failed, and context upload step adding noise to a missing required flag error, for example. So moved only the module initialization to it’s own function to limit the "initialize" span scope.

Something in this change seems to effectively revert #7347

Thanks for catching that, I'll see what I can do.

[helderco]

If I put in another span it's cleaner.

Before

❯ dagger call version string

✘ initialize 6.9s
! required flag(s) "source" not set
  ✔ ModuleSource.resolveFromCaller: ModuleSource! 0.7s
    ✔ upload /Users/helder/Projects/dagger from Sirius (client id: mr82245cdk074u11yv694vty4) (exclude: **/.git) (include: **/*.go, dagger.json, ci/**/*, **/go.mod, **/go.sum, **/go.work, **/go.work.sum, **/vendor/) 0.7s

Error: required flag(s) "source" not set

After

❯ dagger call version string

✔ initialize 6.7s
✘ prepare 0.0s
! required flag(s) "source" not set

Error: required flag(s) "source" not set
Run 'dagger call --help' for usage.

Don't love the repetition that I wasn't getting before:

❯ dagger call version string

✔ initialize 4.8s

Error: required flag(s) "source" not set
Run 'dagger call --help' for usage.
exit status 1

But we already have an issue on that:

• CLI: don't show stderr twice on error #7007

[helderco]

One thing to note is that most (probably, haven't counted) cli args will show at the root anyway, rather than this span. Even for a Directory arg, if it's from git, the flag returns a lazy value. Some flags return a lazy value, others don't. So at this point, I'm not sure of the value of putting them in their own span. Whatever you see above the main pipeline is just the inputs to that pipeline. That's consistent with the general feel in the Cloud UI.

[jedevc]

As discussed (note to self):

• We'll use with prepare that's marked as encapsulate
  • Eventually, we could have a top-level span for each flag
• We might unlazy git arguments to get them to show at the right top-level span, instead of later on

[helderco]

@wingyplus, any idea why the Elixir test is failing?

EDIT: fluke