axodotdev / cargo-dist

📦 shippable application packaging
https://axodotdev.github.io/cargo-dist/
Apache License 2.0
1.53k stars 73 forks source link

plan should warn if target that has no default github runner is specified w/o a configured custom runner #977

Open ashleygwilliams opened 6 months ago

ashleygwilliams commented 6 months ago

we currently support an infinite number of targets in the targets array, but github has a finite and specified number of default runners. if someone adds a target that does not have a default runner, AND does not specify a custom runner, we should have plan error. ideally, this means that cargo-dist will catch the error on the configuration update and not when someone is trying to release.

Gankra commented 6 months ago

So this is an interesting one in that there's a few different interacting checks.

Here we do a preliminary selection of what github runner to use for a given target triple, but in a very loose/permissive way:

https://github.com/axodotdev/cargo-dist/blob/e64776d69f7dda13f4e7dcb99d2714efc187740a/cargo-dist/src/backend/ci/github.rs#L292-L313

And here we check that result and warn if it returned None:

https://github.com/axodotdev/cargo-dist/blob/e64776d69f7dda13f4e7dcb99d2714efc187740a/cargo-dist/src/backend/ci/github.rs#L271-L273

So we do have a warning and it does fire after init/plan, but the "real" issue is that the preliminary selection is really generous, and arguably should be changed to be more strict, especially now that we have a proper config for the user to disambiguate (the looseness here was basically trying to let stuff work in a world where the user was doomed if we didn't pick something).

Also of note is this code that selects and expression for how to install cargo-dist for a given target triple, again very loose:

https://github.com/axodotdev/cargo-dist/blob/e64776d69f7dda13f4e7dcb99d2714efc187740a/cargo-dist/src/backend/ci/github.rs#L316-L331

This code is ~fine for now since we need to have builds for the platform for it to work.


So the "fix" here is: