Support testing for different targets?

[joshlf]

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!

[obi1kenobi]

Thanks for opening this, it's a good idea and we should support and document this.

I think this is mostly a docs issue + a minor bug that can be worked around. I'd love a PR that adds an example for this if you have the time.

cargo-semver-checks just uses the underlying Rust installation it finds. You can set the action's rust-toolchain input to manual, which will let you supply the proper Rust toolchain instead of having it install one for you. So, for example, if you install a riscv64gc-unknown-linux-gnu toolchain and run cargo-semver-checks, that will generate rustdoc JSON that describes riscv64gc-unknown-linux-gnu and that's what will get semver-checked.

#55 is a minor bug you might run into here that I just realized. You can work around it by setting an explicit cache key that depends on the target name in the meantime.

I unfortunately don't think I'll have time to jump into this anytime soon, but I'd be happy to merge PRs and mentor as needed. I'd of course be happy to reorder my priorities if your employer could be convinced to make a substantial (at minimum one of the "large company" tiers) contribution to support the cargo-semver-checks effort: https://github.com/sponsors/obi1kenobi

[joshlf]

Thanks for the guidance! I probably also won't have time soon, but I'll keep this on the back burner and maybe another zerocopy contributor will be interested in tackling it.

[obi1kenobi]

Sounds good, thank you!

…

On Fri, Sep 8, 2023, 6:27 PM Joshua Liebow-Feeser ***@***.***> wrote: Thanks for the guidance! I probably also won't have time soon, but I'll keep this on the back burner and maybe another zerocopy contributor will be interested in tackling it. — Reply to this email directly, view it on GitHub <#54 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAR5MSXOIHYSU75VOKLIVK3XZOLT3ANCNFSM6AAAAAA4P2DKS4> . You are receiving this because you commented.Message ID: ***@***.***>

[Sh0g0-1758]

@obi1kenobi , I have been assigned this issue in Google/zerocopy. It referenced this issue and Hence I have raised a PR to resolve this. Can you please review it and suggest changes in it if required?

[memark]

I recently started looking into google/zerocopy#357 and I think there is a small mixup of concepts here:

@joshlf talks about using this action with a different target, while @obi1kenobi mentions using a different toolchain.

The toolchain can be configured as described, either via the action property, or by setting it up manually beforehand and using manual. But what that does is download binaries (cargo, rustc etc) for that specific platform. Most likely these binaries won't be runnable on the host system in question. The difference can also be visualized as cargo +abc build vs cargo build --target xyz.

Looking at the documentation here it correctly talks only about toolchain, so I think the confusing is local to this issue only.

What we're after here is something like a target: xyz property on the action, which would then be passed right along as cargo semver-checks --target xyz when you execute the underlying binary.

Is that something you would consider adding, or accept a PR for?

[obi1kenobi]

Could you describe what you see cargo semver-checks --target xyz as doing? Perhaps even provide a full example with actual names that zerocopy issue might use, just to make it extra obvious?

I'm having trouble understanding how what you describe matches the target terminology used the linked issue, and also how it is different that the existing --package flag in the CLI and action: https://github.com/obi1kenobi/cargo-semver-checks-action?tab=readme-ov-file#input-options

[memark]

In my view it would be a 1:1 mapping between action and CLI.

The action has an input called package, mapping to the CLI argument --package, which decides which package in a cargo workspace to run the analysis on. cargo semver-checks -h describes it as

-p, --package Package to process (see cargo help pkgid)

The CLI also has another argument called --target, which decides which platform (OS, CPU, etc) to compile for when doing the analysis. cargo semver-checks -h describes it as

 --target <BUILD_TARGET>        Which target to build the crate for, to check platform-specific APIs, e.g. `x86_64-unknown-linux-gnu`

But the ability to specify that is missing from the action. x86_64-unknown-linux-gnu is indeed a good example of a target we would specify, other examples are x86_64-pc-windows-msvc and aarch64-unknown-linux-gnu.

Perhaps a bit lazy of me, but given that the action is (mostly) a wrapper of the CLI, I would argue that all the reasons for having argument --target in the CLI would apply to having input target in the action as well.

Edit: The CLI help texts above are from cargo-semver-checks 0.32.0.

[obi1kenobi]

Nice, thanks! I'd be happy to merge a PR that exposes and documents a target argument in the action, and implements it by passing it as-is to the CLI invocation.

It would be great if you could make sure to document any prerequisites for using the new target argument in the action's docs. For example, if there's a requirement to use a compatible toolchain or anything else.

[memark]

This issue can now be closed.

[obi1kenobi]

Resolved by #82, released as tag v2.5 and repointed v2 tag to it.

[memark]

Great!

Would you add a github release describing the new version as well, please?

[obi1kenobi]

Done, thanks for the reminder!