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

ci: Use a regular cargo to build no-std targets. #231

Merged
merged 1 commit into from
Aug 9, 2024

Conversation

waywardmonkeys
Copy link
Contributor

xargo is no longer needed and has been in maintenance mode for over 7 years.

This can run on stable rather than requiring nightly

The build doesn't need rustfmt, so don't require that it be installed.

@waywardmonkeys
Copy link
Contributor Author

This will conflict with #227, but I'm happy to fix up any merge conflicts after whichever lands first.

`xargo` is no longer needed and has been in maintenance mode for
over 7 years.

This can run on `stable` rather than requiring `nightly`

The build doesn't need `rustfmt`, so don't require that it be
installed.
Copy link
Contributor

@Vrixyz Vrixyz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm approving, but I have a minor concern over dropping nightly compilation: that means we don't have any ci testing nightly.

It's most definitely correct for this PR, but should we add a periodic CI step on master to test against nightly ? @sebcrozet

@Vrixyz Vrixyz requested a review from sebcrozet July 23, 2024 15:08
@sebcrozet
Copy link
Member

sebcrozet commented Jul 26, 2024

Thank you for this PR @waywardmonkeys !

Back in the days, xargo was the only way to ensure that compilation actually fails if any crate (including indirect dependencies) attempts to link against the stdlib. Is that what we gain from cargo now as well? i.e. if we add some crate that needs the stdlib to parry’s Cargo.toml, will it fail to compile when targetting thumbv7em-none-eabihf?

It's most definitely correct for this PR, but should we add a periodic CI step on master to test against nightly ?

@Vrixyz We should keep a nightly test (on PRs) that checkes the simd-nightly feature. (But, yes, this should be in a separate PR.)

@waywardmonkeys
Copy link
Contributor Author

Back in the days, xargo was the only way to ensure that compilation actually fails if any crate (including indirect dependencies) attempts to link against the stdlib. Is that what we gain from cargo now as well? i.e. if we add some crate that needs the stdlib to parry’s Cargo.toml, will it fail to compile when targetting thumbv7em-none-eabihf?

Yes.

Using cargo with a target that has no stdlib is (now) the typical way to verify that no_std is indeed working correctly.

This is what we do in kurbo for example as well:

https://github.com/linebender/kurbo/blob/16cb9af39eda5010ec30eb086822f741edfdf9f5/.github/workflows/ci.yml#L50

@waywardmonkeys
Copy link
Contributor Author

As an aside, this would be a great time to bring up the work started in PR #170 and see about finding a victim to do more of that. :)

@Vrixyz
Copy link
Contributor

Vrixyz commented Aug 9, 2024

Is this related to the failure I'm seeing in #249 ? 👀

Copy link
Member

@sebcrozet sebcrozet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for checking @waywardmonkeys! Looks good.

@Vrixyz Vrixyz merged commit 381c577 into dimforge:master Aug 9, 2024
6 checks passed
@waywardmonkeys waywardmonkeys deleted the remove-xargo-usage branch August 9, 2024 14:27
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.

3 participants