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

Turn std_detect into a no_std crate #1005

Merged
merged 5 commits into from
Feb 14, 2021
Merged

Conversation

Amanieu
Copy link
Member

@Amanieu Amanieu commented Feb 14, 2021

The goal of this PR is to turn std_detect into a proper no_std crate so that we can avoid dealing with the mess of including it as a submodule of std.

@rust-highfive
Copy link

@Amanieu: no appropriate reviewer found, use r? to override

@Amanieu Amanieu force-pushed the detect_cleanup branch 8 times, most recently from 935acb8 to fbf6e04 Compare February 14, 2021 06:23
Copy link
Contributor

@lu-zero lu-zero left a comment

Choose a reason for hiding this comment

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

Seems good to me.

@Amanieu
Copy link
Member Author

Amanieu commented Feb 14, 2021

It's not finished yet, I still need to remove dependencies on std::* and use libc instead.

@lu-zero
Copy link
Contributor

lu-zero commented Feb 14, 2021

Poke me and I'll look into it again when ready :)

@Amanieu Amanieu marked this pull request as ready for review February 14, 2021 12:07
@Amanieu Amanieu force-pushed the detect_cleanup branch 5 times, most recently from 33ebdd1 to eb0d413 Compare February 14, 2021 12:58
@Amanieu
Copy link
Member Author

Amanieu commented Feb 14, 2021

@lu-zero Should be ready for review now.

@@ -121,31 +121,11 @@ impl Cache {
}
}

cfg_if::cfg_if! {
Copy link
Contributor

Choose a reason for hiding this comment

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

why do you want to remove it?

Copy link
Member Author

@Amanieu Amanieu Feb 14, 2021

Choose a reason for hiding this comment

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

  • It's never used.
  • It's never tested.
  • In fact the feature flag is even missing from Cargo.toml.

So apparently I'm bad at grep and completely missed the test cases in the CI config in .github. I'll undo the removal.

Copy link
Member Author

Choose a reason for hiding this comment

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

I changed std_detect_env_override to use libc instead of std.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you a lot!

Copy link
Member

Choose a reason for hiding this comment

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

Does this work on Windows? I did guess not.

Copy link
Contributor

@lu-zero lu-zero left a comment

Choose a reason for hiding this comment

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

Looks fine to me, thank you :)

@Amanieu Amanieu merged commit 2d6375a into rust-lang:master Feb 14, 2021
@Amanieu Amanieu deleted the detect_cleanup branch February 14, 2021 22:14
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