failure in GitLab runner: error: unexpected argument 'sh' found #742

Open wookietreiber opened 1 year ago

original snippet from .gitlab-ci.yml:

msrv:
  stage: test
  image: foresterre/cargo-msrv:latest
  before_script:
    - rustc --version
    - cargo --version
    - cargo msrv --version
  script:
    - cargo msrv verify

error is: (full job log here)

error: unexpected argument 'sh' found

It's due to the way GitLab runner works with the entrypoint, which requires a shell, docs here. The current image (as is) is not compatible with the GitLab runner workflow, because the entrypoint doesn't point to a shell but to the cargo-msrv binary. This was unexpected and I wondered for a bit where this sh came from. Luckily, the entrypoint can be overriden with configuration, this now passes (job log here):

msrv:
  stage: test
  image:
    name: foresterre/cargo-msrv:latest
    entrypoint: [""]
  before_script:
    - rustc --version
    - cargo --version
    - cargo msrv --version
  script:
    - cargo msrv verify

I don't want to request to change the entrypoint you currently have in your Dockerfile, because that would likely screw with ppls current usage of the image/container. Would you be open to adding a CI section to the cargo-msrv book so others have an easier reference in avoiding this? I'd be open to provide a PR.

Note: I explicitly avoided doing cargo install cargo-msrv becausing building that every time the pipelines runs just takes too long, wastes energy, and would quickly consume the compute usage quota that the gitlab.com instance has.

What I wonder though is why there is no output in the runner. In an interactive shell I get:

$ cargo msrv verify
Fetching index
Verifying the Minimum Supported Rust Version (MSRV) for toolchain x86_64-unknown-linux-gnu
Using check command cargo check
   Finished Satisfied MSRV check: 1.56.1 █████████ 00:00:00

In the GitLab runner logs there is just nothing. I'm assuming that's because CI jobs are non-interactive shells, no TERM env var is set, or some other reason, dunno for what you're checking.

It would certainly be nice if there was a more CI-friendly output mode for cargo msrv verify that'd be just a one-liner for success, as in:

$ cargo msrv verify
success: MSRV 1.56.1 verified

... and the output of the verify commands in case of failure, so you can immediately see what the problems are:

$ cargo msrv verify [-- <COMMANDS>...]
error: MSRV 1.56.1 not verified
[output of <COMMANDS>... here]

Would you be open to adding a CI section to the cargo-msrv book so others have an easier reference in avoiding this? I'd be open to provide a PR.

Certainly!

What I wonder though is why there is no output in the runner.

It would certainly be nice if there was a more CI-friendly output mode for cargo msrv verify that'd be just a one-liner for success

Would --output-format json (or --output-format minimal) suffice, or do you think having a slightly less minimal ci output mode is a plus?

Alternatively, I could modify the output of the minimal output format which is also meant for CI use and tooling: e.g. by adding reason for failure, instead of true or false for cargo msrv verify.

This is what --output-format json looks like in the upcoming release:

$ cargo msrv verify --output-format json
{"type":"meta","instance":"cargo-msrv","version":"0.16.0-beta.14","sha_short":null,"target_triple":"x86_64-pc-windows-msvc","cargo_features":"","rustc":"1.70.0"}
{"type":"subcommand_init","subcommand_id":"verify"}
{"type":"fetch_index","source":"rust_changelog","scope":{"id":0,"marker":"start"}}
{"type":"fetch_index","source":"rust_changelog","scope":{"id":0,"marker":"end"}}
{"type":"check_toolchain","toolchain":{"version":"1.36.0","target":"x86_64-pc-windows-msvc"},"scope":{"id":1,"marker":"start"}}
{"type":"setup_toolchain","toolchain":{"version":"1.36.0","target":"x86_64-pc-windows-msvc"},"scope":{"id":2,"marker":"start"}}
{"type":"setup_toolchain","toolchain":{"version":"1.36.0","target":"x86_64-pc-windows-msvc"},"scope":{"id":2,"marker":"end"}}
{"type":"check_method","toolchain":{"version":"1.36.0","target":"x86_64-pc-windows-msvc"},"method":{"type":"rustup_run","args":["1.36.0-x86_64-pc-windows-msvc","cargo","check"],"path":"C:\\run\\yare"}}
{"type":"check_result","toolchain":{"version":"1.36.0","target":"x86_64-pc-windows-msvc"},"is_compatible":false,"error":"error: failed to parse lock file at: C:\\run\\yare\\Cargo.lock\n\nCaused by:\n  invalid serialized PackageId for key `package.dependencies`\n"}
{"type":"check_toolchain","toolchain":{"version":"1.36.0","target":"x86_64-pc-windows-msvc"},"scope":{"id":1,"marker":"end"}}
{"type":"subcommand_result","subcommand_id":"verify","result":{"toolchain":{"version":"1.36.0","target":"x86_64-pc-windows-msvc"},"is_compatible":false,"error":"error: failed to parse lock file at: C:\\run\\yare\\Cargo.lock\n\nCaused by:\n  invalid serialized PackageId for key `package.dependencies`\n"}}
{"type":"terminate_with_failure","reason":{"description":"Crate source was found to be incompatible with Rust version '1.36' specified as MSRV in the Cargo manifest located at 'C:\\run\\yare\\Cargo.toml'"}}

and --output-format minimal:

$ cargo msrv verify --output-format minimal
false

Would --output-format json (or --output-format minimal) suffice, or do you think having a slightly less minimal ci output mode is a plus?

I think it would be beneficial for CI output to show the actual failures to avoid having to run cargo +msrv <check|test|...> manually to figure out what the problems were. Then it'd be easier / less hassle to either bump MSRV because the new functions/features are required/wanted or tell contributors to avoid these functions/features shown in that output to keep MSRV where it is.

case everything ok

minimalistic output:

success: msrv 1.56.1 verified

case failure

include output of the check command:

error: msrv 1.56.1 not verified
<output of cargo +msrv check>

foresterre / cargo-msrv

failure in GitLab runner: error: unexpected argument 'sh' found #742

case everything ok

case failure