-
Notifications
You must be signed in to change notification settings - Fork 1k
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
ci: Check various FreeBSD versions #4159
Conversation
r? @JohnTitor rustbot has assigned @JohnTitor. Use |
☔ The latest upstream changes (presumably #4157) made this pull request unmergeable. Please resolve the merge conflicts. |
5c96709
to
bb493a3
Compare
bb493a3
to
9300c04
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. There are a few other FreeBSD arches too, but you got the most important ones.
} | ||
|
||
freebsd_versions="\ | ||
11 \ | ||
12 \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will test 12 twice, once at line 69-70 and once at 82-83.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have any idea how best to avoid this? It wouldn't be that bad if there isn't anything straightforward.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default version of "12" is set in build.rs. So you can simply leave 12 off of the list. But then it would have to be manually updated in case the default ever changes. So if object files are retained between runs, then running cargo check
twice should be fast enough, I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's about the same conclusion I came to, it should be fast enough that it's better to not worry about forgetting to update the default.
I updated to only test on the Tier2 architectures. Thanks for taking a look at this!
Since we suport multiple versions and this is tier 2, we should make sure that we can build with a couple versions. This does not run tests. Additionally, introduce an environment variable for an easy way to override the version for testing. This includes an unrelated cleanup adjustment in `verify-build.sh`
For versions 10, 11, 12, 13, 14, 15, and architectures aarch64, i686, powerpc64, riscv64gc, and x86_64, I ran the following: RUST_LIBC_UNSTABLE_FREEBSD_VERSION=15 cargo fix \ -Zbuild-std=core \ --features extra_traits \ --allow-dirty \ --edition \ --broken-code \ --lib \ --target aarch64-unknown-freebsd
1e25591
to
9a942b3
Compare
Since we suport multiple versions and this is tier 2, we should make sure that we can build with a couple versions. This does not run tests. Additionally, introduce an environment variable for an easy way to override the version for testing. This includes an unrelated cleanup adjustment in `verify-build.sh` (backport <rust-lang#4159>) (cherry picked from commit 9c2f78e)
For versions 10, 11, 12, 13, 14, 15, and architectures aarch64, i686, powerpc64, riscv64gc, and x86_64, I ran the following: RUST_LIBC_UNSTABLE_FREEBSD_VERSION=15 cargo fix \ -Zbuild-std=core \ --features extra_traits \ --allow-dirty \ --edition \ --broken-code \ --lib \ --target aarch64-unknown-freebsd (backport <rust-lang#4159>) (cherry picked from commit 2e4ac8f) Not an exact cherry pick, I just reran the command.
Since we suport multiple versions and this is tier 2, we should make sure that we can build with a couple versions. This does not run tests.
This introduces an environment variable for an easy way to override the version for testing.