cargo-dist
cargo-dist copied to clipboard
plan should warn if target that has no default github runner is specified w/o a configured custom runner
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.
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:
- change github_runner_for_target to an explicit allow-list of full target triples, using constants defined in axoproject https://docs.rs/axoproject/latest/axoproject/platforms/index.html
- improve the warning to mention the custom runner setting
- maybe make the warning a hard error?