-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Make rustc's own lib directory configurable and change the default to ru... #11118
Conversation
This seems like a fairly roundabout way to fix the issue. I much prefer @brson's suggestion which is basically what you were doing in #11045. The host DLLs should all be in Did you have a particular use case for configuring the name of this directory? |
In the last comment from brson he adviced to rename the rustc dir to IMHO this should also be merged. rustlib seems like a better name for the I made it configurable because there might be some restrictions in distro |
I like this solution, and never really liked the |
@@ -15,7 +15,7 @@ $(SNAPSHOT_RUSTC_POST_CLEANUP): \ | |||
# Note: the variable "SNAPSHOT_FILE" is generally not set, and so | |||
# we generally only pass one argument to this script. | |||
ifdef CFG_ENABLE_LOCAL_RUST | |||
$(Q)$(S)src/etc/local_stage0.sh $(CFG_BUILD) $(CFG_LOCAL_RUST_ROOT) | |||
$(Q)$(S)src/etc/local_stage0.sh $(CFG_BUILD) $(CFG_LOCAL_RUST_ROOT) $(CFG_RUSTLIBDIR) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that this is quite right when dealing with the snapshot. If the snapshot has one directory configured and the local installation has another, there will be a mismatch. We can guarantee that all snapshots are built with rustlib
as the subdirectory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if I understand this correctly. Doesn't CFG_ENABLE_LOCAL_RUST mean that the build process shouldn't use a snapshot?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah good point, I didn't look closely enough!
Any news about merging this? Tests fail because of snapshots using |
If there's trouble with the snapshots, you can always add |
The problem is that the snapshot expects the old directory structure. Therefore The solution is to rebuild the snapshots (with my trick described here: #3319 (comment) ). I can't do that. |
This is going to need some temporary logic for stage0 because we don't do special snapshots. The idea of the snapshots are to be a history of the repository, and every snapshot is based off of some commit found in the history of the master branch. If we made new custom snapshots they would not be based on a commit in master, or the snapshots would only work to merge one patch (blocking all others). The stage0 logic may need some patches in the makefiles for now, but it'll all get removed after the next snapshot. |
The makefiles can have a bunch of lines like |
:'( working on it ... |
Okay new patch copies the files in both directories for stage0. Should I create a PR/patch which removes these lines which can be merged after the next snapshot? |
Okay I was using too many |
I'm very sorry for the failed test, I didn't notice it. This new commit should work (added |
...stlib. Fixes #3319