Skip to content

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

@ThierryBerger ThierryBerger 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

@ThierryBerger ThierryBerger 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. :)

@ThierryBerger
Copy link
Contributor

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.

@ThierryBerger ThierryBerger merged commit 381c577 into dimforge:master Aug 9, 2024
@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