Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wookietreiber opened this issue Jul 12, 2023 · 4 comments
Open

Comments

@wookietreiber
Copy link
Contributor

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.

@wookietreiber
Copy link
Contributor Author

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]

@foresterre
Copy link
Owner

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!

@foresterre
Copy link
Owner

foresterre commented Jul 13, 2023

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

@wookietreiber
Copy link
Contributor Author

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>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants