-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Migrate libs-through-symlinks
and translation
run-make tests to rmake
#129011
base: master
Are you sure you want to change the base?
Conversation
This PR modifies cc @jieyouxu |
This comment has been minimized.
This comment has been minimized.
☔ The latest upstream changes (presumably #129092) made this pull request unmergeable. Please resolve the merge conflicts. |
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.
Suggestion: can we please group the test cases alongside their comments, like the original Makefile? E.g.
fn test_builtin_fallback_bundle() {
// TODO
}
Dumping them together is a bit hard to follow.
|
||
fn main() { | ||
// Check that the test works normally, using the built-in fallback bundle. | ||
rustc().input("test.rs").run_fail().assert_stderr_contains("struct literal body without path"); |
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.
Suggestion: the RUSTC_TRANSLATION_NO_DEBUG_ASSERT=1
env var might be load-bearing, see
rust/compiler/rustc_errors/src/translation.rs
Lines 109 to 118 in 69e36d6
// Always yeet out for errors on debug (unless | |
// `RUSTC_TRANSLATION_NO_DEBUG_ASSERT` is set in the environment - this allows | |
// local runs of the test suites, of builds with debug assertions, to test the | |
// behaviour in a normal build). | |
Some(Err(primary)) | |
if cfg!(debug_assertions) | |
&& env::var("RUSTC_TRANSLATION_NO_DEBUG_ASSERT").is_err() => | |
{ | |
do yeet primary | |
} |
We probably want to have a Rustc
factory fn that presets that env var:
fn rustc_no_translation_assert() -> Rustc {
rustc().env("RUSTC_TRANSLATION_NO_DEBUG_ASSERT", "1")
}
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.
@rustbot author
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.
rustc()
returns a temporary &mut Rustc
which cannot be returned at first glance (unless there is some lifetime magic to pull it off).
I added the environment variable to the rustc calls in the test manually, though in my local tests it seemed to have no effect. Then again, this might be due to me using stage 0 compilation.
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.
Oh right, my bad. Hm.
// translation of rustc's error messages. | ||
rustc() | ||
.arg("-Ztranslate-lang=tlh") | ||
// .input("test.rs") |
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.
Remark: commented out
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.
Original test:
sysroot-missing:
$(RUSTC) $< -Ztranslate-lang=tlh 2>&1 | $(CGREP) "missing locale directory"
Note how the ususal test.rs
next to the section header sysroot-missing:
is absent. I have no idea what the $<
is slotting in.
ea3ebb5
to
af4bdbf
Compare
The job Click to see the possible cause of the failure (guessed by this bot)
|
rm -f $(FAKEROOT)/share | ||
mkdir -p $(FAKEROOT)/share/locale/zh-CN/ | ||
ln -s $(CURDIR)/working.ftl $(FAKEROOT)/share/locale/zh-CN/basic-translation.ftl | ||
$(RUSTC) $< --sysroot $(FAKEROOT) -Ztranslate-lang=zh-CN 2>&1 | $(CGREP) "this is a test message" |
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.
Remark (to myself): $< ?
☔ The latest upstream changes (presumably #129443) made this pull request unmergeable. Please resolve the merge conflicts. |
@rustbot blocked (on fixing symlink cleanup in compiletest and bootstrap) |
The symlink trouble I observed previously from compiletest and bootstrap likely came from somehow under non-admin mode artifacts can be produced under admin ownership, which doesn't block this PR now that run_make_support symlink handling is rectified. |
Part of #121876 and the associated Google Summer of Code project.
Everything here works locally... except line 57 (the test works if it is commented out):
This will therefore initially fail CI.
try-job: aarch64-gnu
try-job: aarch64-apple
try-job: x86_64-msvc
try-job: i686-mingw
try-job: test-various
try-job: armhf-gnu