obi1kenobi / cargo-semver-checks-action Goto Github PK

Hi!

I'd like to move our custom action over to this but I am concerned with the automatic download of the latest release.

I'd like to pin the version of cargo semver-checks to make sure CI checks are reproducible and deterministic. Passing semver check is a merge requirement in our CI and updates to our tooling (Rust toolchain, clippy, etc) must go through a PR like any other change.

We are also pinning the versions of actions to a commit hash¹². My suggestion would be to hardcode the release to download with the action version. This way, users who just depend on this action with @v2 will always use the latest version and people who pin action version will have deterministic builds without needing to configure anything.

The only downside is that a new release of cargo semver-checks requires a new release of the action but IMO that is a small price to pay for the advantages we get from it.

I'd like to add some more info to the CONTRIBUTING.md file regarding how to login to the github npm registry, where the @action-rs packages are located. I've tried a few different solutions

• adding GITHUB_TOKEN to .npmrc (but not committing it)
• using npm-cli-login
• using npm login

None of which feels fantastic... What is your preferred local workflow?

Many crates in the Rust ecosystem are pre-v1.0, and do not have Semver compatibility guarantees. This Action is still useful for maintainers who want to know if a PR is breaking semver compliance, even if they want to merge the pull request anyway.

For comparison, check out Covcode's fail_ci_if_error input. This Action input allows the status code of the Action to be set to 0 even if the Action itself fails.

This action is our slowest PR validation check. I routinely see this action take ~2 minutes to build cargo-semver-checks from scratch on each PR validation run.

Please consider releasing binaries as part of your crate release process and downloading those boundaries in this GitHub action.

I refer you to https://github.com/aig787/cargo-udeps-action, which does this and is on-task in just a few seconds.

(Yes, I recognize the building-docs steps are going to be slow regardless.)

For me, its up to the reviewer to decide if a compatibility break is acceptable. I would love to be able to label the PR.

This can either come from direct support or documentation on how to integrate with other actions

In order to get cargo-semver-checks to run when publish = false is set, you need a combination of -p <> and --manifest-path to be set

Requested:
https://www.reddit.com/r/rust/comments/x10f5u/comment/imbrage/?context=3

Perhaps as simple as adding CARGO_UNSTABLE_SPARSE_REGISTRY=true to the action.
https://internals.rust-lang.org/t/call-for-testing-cargo-sparse-registry/16862

A lot of users don't know to look for Action inputs here. I would suggest including a table in the README to describe the inputs into the action explicitly.

We've been struggling with [cargo-semver-checks-action erroring out] and blocking PR validations and release builds.

I think we've found a temporary workaround, but wanted you to be aware of the issue.

Please read the log at https://github.com/contentauth/c2pa-rs/actions/runs/3734139307/jobs/6335827121.

In this log, version 0.16.0 is the prior release for comparison and you'll notice that the cargo rustdoc task for that release fails with:

error: unclosed HTML tag `u8`
   --> sdk/src/assertion.rs:175:58
175 | /// for Json assertions the data is a Json string and Vec<u8> for
                                                             ^^^^

… which is a valid error in Rust 1.66. However, at the time we released our crate 0.16.0, we were still on Rust 1.65 where the above doc comment went unnoticed.

The problem is that (short of disabling the cargo-semver-checks-action, which is how we're now working around it), the previous build check can't pass and so the step fails.

Not sure how to work around this, but thought you should be aware of the issue.

Currently the github action fails with this because I use version tags without the v. My tags are like 0.5.1.

Comparison version: 0.5.1
+ export COMPARISON_TAG=v0.5.1
+ COMPARISON_TAG=v0.5.1
+ git fetch --depth=1 origin +refs/tags/v0.5.1:refs/tags/v0.5.1
fatal: couldn't find remote ref refs/tags/v0.5.1
Error: Process completed with exit code 128.

Hi,

As far as I can tell, the action currently assumes there's a Cargo.toml at the root of the repository (either a crate or a workspace).

Could it be possible to specify the directory containing the Cargo.toml? (the default would be . i.e. the root of the repository)

Thanks!

...or at least that's what the error message I get when I try to use the action with a "feature" option suggests ;)

Warning: Unexpected input(s) 'feature', valid inputs are ['package', 'exclude', 'manifest-path', 'feature-group', 'features', 'verbose', 'release-type', 'rust-toolchain', 'shared-key', 'prefix-key', 'github-token']

Steps to reproduce the bug with the above code

I was upgrading a crate from 0.1.0-rc2 to 0.1.0. No actual code changes were present, just exiting the release candidate phase.

Actual Behaviour

I used

      - name: Check semver
        uses: obi1kenobi/cargo-semver-checks-action@v1
        with:
          crate-name: qr-rs-lib

as part of my CD pipeline and got this error:

Crate qr-rs-lib current version: 0.1.0
Versions on crates.io:
0.1.0-rc2
0.1.0-rc1

There is no suitable comparison version.
The current version 0.1.0 is smaller than any version published on crates.io
+ export COMPARISON_TAG=v
+ COMPARISON_TAG=v
+ git fetch --depth=1 origin +refs/tags/v:refs/tags/v
fatal: couldn't find remote ref refs/tags/v
Error: Process completed with exit code 128.

My workflow can be found here and the relevant part is this.

Expected Behaviour

This should not throw an error. The interesting thing is how the CLI does not consider this to be wrong (which is the expected behavior).

In fact, that is how I fixed my pipeline, by using the CLI instead of the action

      - name: Check semver
        working-directory: lib
        run: |
          cargo install cargo-semver-checks --locked
          cargo semver-checks check-release

Generated System Information

Software version

cargo-semver-checks 0.17.1 (af71be0)

Operating system

Windows 6.2.9200

Command-line

C:\Users\anton\.cargo\bin\cargo-semver-checks.exe semver-checks --bugreport

cargo version

> cargo -V
cargo 1.67.1 (8ecd4f20a 2023-01-10)

Compile time information

• Profile: release
• Target triple: x86_64-pc-windows-msvc
• Family: windows
• OS: windows
• Architecture: x86_64
• Pointer width: 64
• Endian: little
• CPU features: fxsr,sse,sse2
• Host: x86_64-pc-windows-msvc

Build Configuration

My config.toml can be found here however, it only contains way too many lints and was present both in the failing and passing scenarios.

Additional Context

No response

Just noticed this in one of my CI jobs:

Node.js 16 actions are deprecated. Please update the following actions to use Node.js 20: obi1kenobi/cargo-semver-checks-action@v2. For more information see: https://github.blog/changelog/2023-09-22-github-actions-transitioning-from-node-16-to-node-20/.

Hi, I'm using this action script in a project with rust-toolchain set to 1.63.0, and getting Error: rustdoc format v15 is not supported. What should I do?

Compiling trustfall_rustdoc v0.3.1
584
   Compiling git2 v0.15.0
585
   Compiling crates-index v0.18.9
586
   Compiling cargo-semver-checks v0.12.0
[587](https://github.com/MystenLabs/mysten-infra/actions/runs/3160462909/jobs/5144946004#step:4:591)
    Finished release [optimized] target(s) in 5m 37s
[588](https://github.com/MystenLabs/mysten-infra/actions/runs/3160462909/jobs/5144946004#step:4:592)
  Installing /home/runner/.cargo/bin/cargo-semver-checks
[589](https://github.com/MystenLabs/mysten-infra/actions/runs/3160462909/jobs/5144946004#step:4:593)
   Installed package `cargo-semver-checks v0.12.0` (executable `cargo-semver-checks`)
[590](https://github.com/MystenLabs/mysten-infra/actions/runs/3160462909/jobs/5144946004#step:4:594)
+ cargo semver-checks check-release --current /tmp/current.json --baseline /tmp/baseline.json
[591](https://github.com/MystenLabs/mysten-infra/actions/runs/3160462909/jobs/5144946004#step:4:595)
Error: rustdoc format v15 for file /tmp/baseline.json is not supported
[592](https://github.com/MystenLabs/mysten-infra/actions/runs/3160462909/jobs/5144946004#step:4:596)
Error: Process completed with exit code 1.

The project is open source:
https://github.com/MystenLabs/mysten-infra

Many thanks

Hey! 🐻

I would like to use the output of cargo-semver-checks-action in other steps/jobs as follows:

  semver:
    name: Semver
    runs-on: ubuntu-22.04
    steps:
      - name: Checkout
        uses: actions/checkout@v4

      - name: Check semver
        id: check_semver
        uses: obi1kenobi/cargo-semver-checks-action@v2
        with:
          package: git-cliff-core

      - name: Comment on PR
        if: always() && (steps.check_semver.outputs.error_message != null) && (github.event_name == 'pull_request')
        uses: marocchino/sticky-pull-request-comment@v2
        with:
          header: pr-semver-check-error
          message: |
            Thank you for opening this pull request! ⛰️

            There seems to be semver incompatibility issues reported by [cargo-semver-checks](https://github.com/obi1kenobi/cargo-semver-checks).

            Details:

            > ${{ steps.check_semver.outputs.error_message }}

      - name: Update comment on PR
        uses: marocchino/sticky-pull-request-comment@v2
        if: (steps.check_semver.outputs.error_message == null) && (github.event_name == 'pull_request')
        with:
          header: pr-semver-check-error
          delete: true

But it does not set any outputs (i.e. outputs.error_message) so I cannot achieve this.

Inspiration: https://github.com/amannn/action-semantic-pull-request?tab=readme-ov-file#outputs

Can we get this feature in? What do you think?

The cargo-semver-checks cache key starts with a dash, like -semver-<whatever> and the gh actions-cache delete command was treating it as a flag, which it did not recognize. Hopefully by passing the key after -- will force it to treat it positionally.
cc @obi1kenobi - you might be interested to know that the leading dash in the cargo-semver-checks cache key can be a footgun for automated tooling.

Originally posted by @davidhewitt in PyO3/pyo3#3970

We should remove the leading dash from our cache keys. I don't recall adding it there on purpose.

- name: Check semver
  uses: obi1kenobi/cargo-semver-checks-action@v2
- name: Publish to crates.io
  run: # your `cargo publish` code here

GitHub action errors when I use obi1kenobi/cargo-semver-checks-action@v2 also the link on top of the repo re directing to the marketplace is broken giving a 404.

When a check fails, annotating the source will provide a better way of reporting the results to the authors and reviewers.

This can be done with special output, e.g. https://github.com/crate-ci/typos/blob/master/docker/format_gh.sh

Looking specifically at:

• CARGO_INCREMENTAL=0
• CARGO_TERM_COLOR=always
• CARGO_REGISTRIES_CRATES_IO_PROTOCOL=sparse

https://github.com/dtolnay/rust-toolchain/blob/master/action.yml#L86-L106

To provide increased insight into which checks were run even when successful, I would like to enable verbose output. It does not seem like the action provides a way to do that presently.

It's possible I'm misreading the documentation, but it appears as though there's no way to instruct the action to build for a particular target. In CI, we use a job matrix to build and test on different targets, and I'd like to be able to use cargo-semver-checks-action as a step in the matrix so that we're checking for semver compatibility for each target (we have some target-specific features like trait impls for SIMD types). It'd be great if this were supported at some point!

(node:1636) [DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead.
(Use `node --trace-deprecation ...` to show where the warning was created)

Seen in https://github.com/tokio-rs/tokio/actions/runs/4788275544/jobs/8514678819

Treating this as a bug.

What happened

I used this action in a recent workflow and it generated a warning:

Warning: The set-output command is deprecated and will be disabled soon. Please upgrade to using Environment Files. For more information see: https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/

What you expected to happen

No such warning

How to reproduce it

Run any GitHub Actions workflow containing this action.

Environment

At the moment, there isn't a CI running to verify the action is correct

We should let cargo-semver-checks find the appropriate version to compare against instead of finding it in a bash script, and we should use the --workspace flag.

From: obi1kenobi/cargo-semver-checks#116 (comment)

Open questions:

• How do we handle the custom config options: crate-name, crate-target, version-tag-prefix? Are they even necessary?
• Will we be able to make the switch without breaking changes, or will this need a v2 of the action?

In v2 of the action, the precompiled binaries are correctly chosen only on x86_64 runners, and moreover, the action is not tested on ARM-based platforms in any way.

Strongly related to obi1kenobi/cargo-semver-checks#390.

See https://github.com/libp2p/rust-libp2p/actions/runs/6087635818/job/16516621765?pr=4248 for example.

This PR introduces a new crate which is not published yet. We would only publish it once the PR is merged. I think the action should ignore those until then.

Right now, our cache key includes rustc --version but does not include the target name. This can be a caching problem if someone wants to test multiple targets on the same commit and with the same Rust version, which is entirely reasonable (e.g. #54), if a bit rare.

This is one feature i seem to be missing while looking through the code, where it isn't possible to say "look at the base branch, and compare against that".

This'd be useful for knowing if the current PR includes version-incompatible changes, and that then those breakages can be judged on their own merits, and merged or not.

Many repositories using cargo-semver-checks in multi-crate workspaces run it separately on each crate in a single job (matrix strategy or separate steps), for example:

• https://github.com/libp2p/rust-libp2p/blob/master/.github/actions/cargo-semver-checks/action.yml
• https://github.com/ngrok/ngrok-rs/blob/main/.github/workflows/ci.yml#L78-L92
• https://github.com/risinglightdb/sqllogictest-rs/blob/main/.github/workflows/ci.yml#L60-L76

If we run the action in these repos specifying only the input package, the runs for different crates will share the same cache key. This leads to poor results especially if they are executed concurrently - then only the cache of the recently checked crate can be restored. The current solution is to use input prefix-key with the same value as package, but I think it'd be better if we changed the default behavior of the action. Now it's not so easy to notice that something is going wrong and I think many users can make such a mistake...

I propose to include values of package and manifest-path in the cache key. The question is - should they be made a part of the default value for cache-key input (currently the name of the job) or should they be included in the unmodifiable part of the key (like the versions of rustc and cargo-semver-checks)?

Zerocopy's CI jobs started failing yesterday, e.g. here. In total, we've seen this issue on PRs google/zerocopy#749 and google/zerocopy#750. The relevant error seems to be:

/home/runner/.cargo/bin/cargo semver-checks check-release --package zerocopy --all-features
     Parsing zerocopy v0.8.0-alpha.1 (current)
Error: rustdoc format v28 for file /home/runner/work/zerocopy/zerocopy/semver-checks/target/semver-checks/target/doc/zerocopy.json is not supported
Error: The process '/home/runner/.cargo/bin/cargo' failed with exit code 1

The PR most recently merged into main is google/zerocopy#748, and its CI tests passed, so I don't believe this is caused by a change in zerocopy's codebase.

Just leaving a note to remind us that the action's caching behavior should version based on the Rust toolchain version, and not the rustdoc format version: new Rust versions can ship bugfixes in rustdoc JSON that don't require a format change.

The cache should also be versioned by the cargo-semver-checks version, too.

Related: obi1kenobi/cargo-semver-checks#415 (comment)