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

upgpkg: cargo-msrv #2104

Merged
merged 1 commit into from
Jan 10, 2023
Merged

upgpkg: cargo-msrv #2104

merged 1 commit into from
Jan 10, 2023

Conversation

light4
Copy link
Contributor

@light4 light4 commented Jan 10, 2023

original PKGBUILD set rust toolchain, fix patch

original PKGBUILD set rust toolchain, fix patch
@felixonmars felixonmars merged commit 298014e into felixonmars:master Jan 10, 2023
@light4 light4 deleted the cargo-msrv branch January 10, 2023 08:51
@SpriteOvO
Copy link
Contributor

@light4 Unfortunately, this package still does not pass the tests.

failures:
    minimum_from_edition::msrv_min_with_edition_in_cargo_toml
    minimum_from_edition::msrv_no_minimum_with_flag
    msrv_unsupported
    msrv_using_bisect_method::case_0
    msrv_using_bisect_method::case_1
    msrv_using_bisect_method::case_2
    msrv_using_bisect_method::case_3
    msrv_using_linear_method::case_0
    msrv_using_linear_method::case_1
    msrv_using_linear_method::case_2
    msrv_using_linear_method::case_3
    msrv_with_custom_command::case_0
    msrv_with_custom_command::case_1
    msrv_with_custom_command::case_2
    msrv_with_custom_command::case_3
    msrv_with_old_lockfile
    msrv_with_release_source::case_0
    msrv_with_release_source::case_1

test result: FAILED. 6 passed; 18 failed; 0 ignored; 0 measured; 0 filtered out; finished in 139.88s

The main challenge here is that Rust does not officially provide binaries with riscv64gc triples, and the tests of this package requires installing old versions of Rust by rustup.

@light4
Copy link
Contributor Author

light4 commented Jan 10, 2023

Maybe disable some tests on riscv? I'll make a PR to upstream.

@SpriteOvO
Copy link
Contributor

Maybe disable some tests on riscv? I'll make a PR to upstream.

@light4 I prefer to leave this package broken, as:

  • The problem is on rustup.

  • It is a bin package, it does not block other packages as a dependency, so disabling tests does not provide benefits.

  • Even if the tests are disabled, the package will not work at all for users.

@light4
Copy link
Contributor Author

light4 commented Jan 10, 2023

This package actually works fine at riscv64. The dist riscv64gc became available from 1.47.0
Disable some little tests won't affect users.

@SpriteOvO
Copy link
Contributor

This package actually works fine at riscv64. The dist riscv64gc became available from 1.47.0 Disable some little tests won't affect users.

@light4 Oh, you are right, so this is actually a bug from package upstream. It would be great if you would open a PR fix it there :)

@r-value
Copy link
Collaborator

r-value commented Jan 10, 2023

I suggest fixing the package by adding a global MSRV for certain platforms or file an issue like "Panicked on new platforms if MSRV is too old" instead of just disabling the test at upstream. If you fixed it, you can take the patch here before it's merged and released to accelerate the fixing process.

@light4
Copy link
Contributor Author

light4 commented Jan 10, 2023

Filed an issue foresterre/cargo-msrv#585.

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.

4 participants