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

Ajust and add target testcases #1073

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

zjp-CN
Copy link

@zjp-CN zjp-CN commented Jan 10, 2025

Context: #1071 (comment)

These conditinal compiled tests requring target can be tested if rustup target add riscv64gc-unknown-linux-gnu is set up. I've tested them locally, and they will pass with riscv64gc-unknown-linux-gnu target installed.

But I'm not sure if adding target field in ci.yml is enough. (Haven't added it yet)

It's weird riscv64gc-unknown-linux-gnu target can be recognized by cargo, but aarch64_unknown_none not, which is the problem I ran into. with_{env_var,flag}_aarch64_unknown_none testcases always fail on ["1.81", "1.82", "1.83", "stable", "beta"] toolchains and the nightly one before this weekend. So it'd be a bad idea to incorporate them for now and near future.

Feel free to close this PR if you do not accept it.

and make they normal testcases.

`rustup target add riscv64gc-unknown-linux-gnu` is required to make
them pass.

If riscv64gc-unknown-linux-gnu is not installed, there will be errors:

command=`CARGO_BUILD_TARGET="riscv64gc-unknown-linux-gnu" "/home/zjp/rust/cargo-semver-checks/target/debug/cargo-semver-checks" "semver-checks" "check-release" "--manifest
-path=test_crates/template/new" "--baseline-root=test_crates/template/old"`
code=1
stdout=""
stderr=```
    Building template v0.1.0 (current)
error: running cargo-doc on crate \'template\' failed with output:
-----
    Blocking waiting for file lock on build directory
 Documenting template v0.1.0 (/home/zjp/rust/cargo-semver-checks/test_crates/template/new)
error[E0463]: can\'t find crate for `std`
  |
  = note: the `riscv64gc-unknown-linux-gnu` target may not be installed
  = help: consider downloading the target with `rustup target add riscv64gc-unknown-linux-gnu`
  = help: consider building the standard library from source with `cargo build -Zbuild-std`

For more information about this error, try `rustc --explain E0463`.
error: could not document `template`
Both testcases require `rustup target add x86_64-unknown-linux-gnu`.

But currently, cargo has a bug for learning target info:

command=`CARGO_BUILD_TARGET="aarch64-unknown-none" "/home/zjp/rust/cargo-semver-checks/target/debug/cargo-semver-checks" "semver-checks" "check-release" "--manifest-path=t
est_crates/template/new" "--baseline-root=test_crates/template/old"`
code=1
stdout=""
stderr=```
    Building template v0.1.0 (current)
error: running cargo-doc on crate \'template\' failed with output:
-----
error: output of --print=file-names missing when learning about target-specific information from rustc
command was: `/home/zjp/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/rustc - --crate-name ___ --print=file-names --cap-lints=allow --target aarch64-unknown-none
 --crate-type bin --crate-type rlib --crate-type dylib --crate-type cdylib --crate-type staticlib --crate-type proc-macro --print=sysroot --print=split-debuginfo --print=c
rate-name --print=cfg`

--- stdout
___
lib___.rlib
lib___.a
/home/zjp/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu
off
___
debug_assertions
fmt_debug=\"full\"
overflow_checks
panic=\"abort\"
proc_macro
relocation_model=\"static\"
target_abi=\"\"
target_arch=\"aarch64\"
target_endian=\"little\"
target_env=\"\"
target_feature=\"neon\"
target_has_atomic
target_has_atomic=\"128\"
target_has_atomic=\"16\"
target_has_atomic=\"32\"
target_has_atomic=\"64\"
target_has_atomic=\"8\"
target_has_atomic=\"ptr\"
target_has_atomic_equal_alignment=\"128\"
target_has_atomic_equal_alignment=\"16\"
target_has_atomic_equal_alignment=\"32\"
target_has_atomic_equal_alignment=\"64\"
target_has_atomic_equal_alignment=\"8\"
target_has_atomic_equal_alignment=\"ptr\"
target_has_atomic_load_store
target_has_atomic_load_store=\"128\"
target_has_atomic_load_store=\"16\"
target_has_atomic_load_store=\"32\"
target_has_atomic_load_store=\"64\"
target_has_atomic_load_store=\"8\"
target_has_atomic_load_store=\"ptr\"
target_os=\"none\"
target_pointer_width=\"64\"
target_vendor=\"unknown\"
ub_checks

Hopefully, the fix from cargo will land in weekend.
Ref obi1kenobi#1068 (comment)
@obi1kenobi
Copy link
Owner

I feel these might be better as separate test jobs in ci.yml while keeping the conditional compilation attributes, than as tests that run on every cargo test including locally.

We already have a relatively complex setup for getting the repo bootstrapped to the point that tests pass. I don't think it's worth the maintenance burden of requiring contributors to do rustup target add riscv64gc-unknown-linux-gnu before e.g. contributing a new lint. This is especially the case if the targets have any potential of having compatibility issues on Windows or macOS.

@zjp-CN zjp-CN marked this pull request as draft January 11, 2025 03:56
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

Successfully merging this pull request may close these issues.

2 participants