Migrate emit-shared-files and emit-path-unhashed run-make tests to rmake
#127335
Conversation
rustbot
added
A-run-make
|
The run-make-support library was changed cc @jieyouxu This PR modifies cc @jieyouxu |
Oneirical
force-pushed
the
put-on-a-petestal
branch
from
28b636a
to
c049f28
Compare
|
I'll take a look later today, this looks like it warrants several try jobs. |
|
✌️ @Oneirical, you can now approve this pull request! If @jieyouxu told you to " |
Oneirical
force-pushed
the
put-on-a-petestal
branch
from
c049f28
to
4a15fe4
Compare
| // Specifying how rustc outputs a file can be done in different ways, such as | ||
| // the output flag or the KIND=NAME syntax. However, some of these methods used | ||
| // to result in different hashes on output files even though they yielded the | ||
| // exact same result otherwise. This was fixed in #86045, and this test checks | ||
| // that the hash is only modified when the output is made different, such as by | ||
| // adding a new output type (in this test, metadata). | ||
| // See https://github.com/rust-lang/rust/issues/86044 |
| // FIXME: this probably shouldn't have a suffix | ||
| assert!(Path::new("invocation-only/y-xxx.css").exists()); | ||
| // FIXME: this is technically incorrect (see `write_shared`) | ||
| assert!(!Path::new("invocation-only/main-xxx.js").exists()); |
There was a problem hiding this comment.
Discussion: I know this is pre-existing, but what is happening here lol
There was a problem hiding this comment.
cc @GuillaumeGomez sorry for the ping, but you reviewed the original PR #83478: do you happen to know what's the status for --emit=toolchain-shared-resources--emit=invocation-specific or what this test is trying to exercise here? In particular, I don't quite understand the FIXME even though I tried to read the original PR that introduced this test.
There was a problem hiding this comment.
Ok I see. So normally, static files (like main.js) are generated with the hash of the file itself. I suppose the --resource-suffix option flag overrides this. Not sure if it's a good or bad though... Better to leave it as is.
|
r=me if the battery of try jobs come back green. @bors try |
Migrate `emit-shared-files` and `emit-path-unhashed` `run-make` tests to rmake Part of rust-lang#121876 and the associated [Google Summer of Code project](https://blog.rust-lang.org/2024/05/01/gsoc-2024-selected-projects.html). try-job: x86_64-msvc try-job: aarch64-apple try-job: armhf-gnu try-job: test-various
|
☀️ Try build successful - checks-actions |
| #[track_caller] | ||
| pub fn run(&mut self) { | ||
| self.drop_bomb.defuse(); | ||
| fn run_common(&self) -> (&str, &str, String, String) { |
There was a problem hiding this comment.
Would be better to return a struct with named field instead of a tuple here.
There was a problem hiding this comment.
Hum although it's a private function, so not really a big issue either...
There was a problem hiding this comment.
Yeah I was also about to comment on that but I also realized that's private lol
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (289deb9): comparison URL. Overall result: ❌ regressions - no action needed@rustbot label: -perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)Results (secondary 0.7%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResults (secondary 0.5%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 700.652s -> 699.359s (-0.18%) |
Part of #121876 and the associated Google Summer of Code project.
try-job: x86_64-msvc
try-job: aarch64-apple
try-job: armhf-gnu
try-job: test-various