-
Notifications
You must be signed in to change notification settings - Fork 10
Account for possibly patched compiler_builtins
#27
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,3 +13,4 @@ tempfile = "3" | |
| rustc_version = "0.4" | ||
| anyhow = "1.0" | ||
| walkdir = "2.4" | ||
| regex = "1.11.1" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ use std::path::{Path, PathBuf}; | |
| use std::process::Command; | ||
|
|
||
| use anyhow::{bail, Context, Result}; | ||
| use regex::Regex; | ||
| use tempfile::TempDir; | ||
| use walkdir::WalkDir; | ||
|
|
||
|
|
@@ -348,6 +349,18 @@ path = {src_dir_test:?} | |
| ), | ||
| }; | ||
|
|
||
| let builtins_patch = if let Some(path) = builtins_patch_location(src_dir) { | ||
| format!( | ||
| r#" | ||
| [patch.crates-io.compiler_builtins] | ||
| path = {src_dir_workspace_builtins:?} | ||
| "#, | ||
| src_dir_workspace_builtins = src_dir.join(path), | ||
| ) | ||
| } else { | ||
| String::new() | ||
| }; | ||
|
|
||
| // If we include a patch for rustc-std-workspace-std for no_std sysroot builds, we get a | ||
| // warning from Cargo that the patch is unused. If this patching ever breaks that lint will | ||
| // probably be very helpful, so it would be best to not disable it. | ||
|
|
@@ -359,6 +372,7 @@ path = {src_dir_test:?} | |
| r#" | ||
| [patch.crates-io.rustc-std-workspace-core] | ||
| path = {src_dir_workspace_core:?} | ||
| {builtins_patch} | ||
| "#, | ||
| src_dir_workspace_core = src_dir.join("rustc-std-workspace-core"), | ||
| ), | ||
|
|
@@ -370,6 +384,7 @@ path = {src_dir_workspace_core:?} | |
| path = {src_dir_workspace_alloc:?} | ||
| [patch.crates-io.rustc-std-workspace-std] | ||
| path = {src_dir_workspace_std:?} | ||
| {builtins_patch} | ||
| "#, | ||
| src_dir_workspace_core = src_dir.join("rustc-std-workspace-core"), | ||
| src_dir_workspace_alloc = src_dir.join("rustc-std-workspace-alloc"), | ||
|
|
@@ -559,3 +574,50 @@ panic = 'unwind' | |
| Ok(SysrootStatus::SysrootBuilt) | ||
| } | ||
| } | ||
|
|
||
| /// If the workspace Cargo.toml needs a `compiler_builtins` patch, return the path | ||
| fn builtins_patch_location(src_dir: &Path) -> Option<String> { | ||
| // Assume no patch is needed if the workspace Cargo.toml doesn't exist | ||
| let f = fs::read_to_string(src_dir.join("Cargo.toml")).ok()?; | ||
|
|
||
| // Crudely extract only the patches and see if one exists for `compiler_builtins` | ||
| let patches = f.split_once("[patch.crates-io]")?.1; | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it make sense to just copy the entire
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That would probably be reasonable - how would you like the logic to work with
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might also be possible to copy the entire workspace
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah right, we need to overwrite some of the patches...
That sounds reasonable. Probably best to have a toml parser around for getting this data out of the manifest.
I'd rather copy the Overall this expands the scope of the PR quite a bit, so I am also fine with landing the approach you have here -- it's only needed temporarily anyway. The nicer version (that copies the profile etc) can be done later. Let me know which option you want to pursue for now.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you are okay with what is here, I have a mild preference to go with this for now since this covers the current usecase. I'm happy to implement a smarter version with a parser / printer afterward, but that won't be for another couple of days and it would be nice to be able to get the big update list rolling.
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just copying the patch section also has the problem of copying too match for no-std builds, where we currently have special logic to avoid a cargo warning... So yeah let's just land this as-is for now. |
||
| let re = | ||
| Regex::new(r#"(?m)^\s*compiler_builtins\s*=\s*\{.*path\s*=\s*"(?P<path>.*)".*\}"#).unwrap(); | ||
| let path = re.captures(patches)?.name("path").unwrap().as_str(); | ||
| Some(path.to_string()) | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
|
|
||
| #[test] | ||
| fn check_patch_location() { | ||
| let has_patch = r#" | ||
| [workspace] | ||
| # ... | ||
| [patch.crates-io] | ||
| other = { path = "foo" } | ||
| compiler_builtins = {path = "dir1/dir2"} | ||
| "#; | ||
| let no_patch = r#" | ||
| [workspace] | ||
| # ... | ||
| # Dep rather than a patch | ||
| compiler_builtins = { path = "dir1/dir2" } | ||
| "#; | ||
|
|
||
| let dir = tempfile::tempdir().unwrap(); | ||
| let f = dir.path().join("Cargo.toml"); | ||
| fs::write(&f, has_patch).unwrap(); | ||
|
|
||
| assert_eq!( | ||
| builtins_patch_location(dir.path()), | ||
| Some("dir1/dir2".to_string()) | ||
| ); | ||
|
|
||
| fs::write(&f, no_patch).unwrap(); | ||
| assert_eq!(builtins_patch_location(dir.path()), None); | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.