Account for possibly patched compiler_builtins#27
Conversation
In order to help support changes to the standard library's Cargo workspace ([1], apply a patch for `crates-io.compiler-builtins` if the workspace `Cargo.toml` also has one. [1]: rust-lang/rust#141993
tgross35
force-pushed
the
patch-compiler-builtins
branch
from
4793aab to
59f03eb
Compare
|
I can adjust this if you prefer something other than regex (using a toml parser, something more hacky without the regex dep, |
| 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; |
There was a problem hiding this comment.
Would it make sense to just copy the entire [patch.crates-io] section if it exists?
There was a problem hiding this comment.
That would probably be reasonable - how would you like the logic to work with let patches = ...? We could replace the PATH portion of rustc-std-workspace-CRATE = { path = 'PATH' } with src_dir.join("rustc-std-workspace-CRATE") and then delete unneeded rustc-std-workspace-*.
There was a problem hiding this comment.
It might also be possible to copy the entire workspace Cargo.toml file but stripping [workspace] so you get the profile.release config https://github.com/rust-lang/rust/blob/f315e6145802e091ff9fceab6db627a4b4ec2b86/library/Cargo.toml#L16-L45 and anything else that winds up getting added. But this would mean Cargo warnings if e.g. gimli isn't in the crate graph because SysrootConfig::NoStd is used.
There was a problem hiding this comment.
Ah right, we need to overwrite some of the patches...
We could replace the PATH portion of rustc-std-workspace-CRATE = { path = 'PATH' } with src_dir.join("rustc-std-workspace-CRATE") and then delete unneeded rustc-std-workspace-*.
That sounds reasonable.
Probably best to have a toml parser around for getting this data out of the manifest.
It might also be possible to copy the entire workspace Cargo.toml file but stripping [workspace] so you get the profile.release config
I'd rather copy the profile part than copy everything by default.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
Regarding testing, the try build looks good, thanks! |
2 participants
In order to help support changes to the standard library's Cargo workspace (1, apply a patch for
crates-io.compiler-builtinsif the workspaceCargo.tomlalso has one.