transpile: save platform-specific .rs files, too
#1291
Conversation
…er platform-specific snapshots
Since `-o /dev/null` doesn't work since intermediates are created in the same directly, we compile to the normal `.rlib` output path and then delete it. This just makes that explicit with `-o` so it's more obviously correct, and we can name the `.rlib` anything we want as well.
We already save platform-specific `.snap` files for snapshot tests,
but the `.rs` files still clash, and we aren't checking them in for all platforms.
This moves the `.rs` files to `.{platform}.rs`.
And since the crate name can't have a `.` in it,
we need to explicitly set the crate name to the file stem now.
There was a problem hiding this comment.
Is there any major benefit to having these .rs files checked in? They seem redundant with the
.snapfiles. I would prefer fewer files and less redundancy if possible.
I originally checked them in because they were easier for local review. *.rs files are easier to work with (syntax highlighting, etc.), but now that I think about it, that doesn't necessarily mean they need to be checked in, just generated. The one significant advantage I've still seen is that diff tools like delta and GitHub's review are better at syntax highlighting *.rs diffs that are recognized as Rust vs. *.snap diffs that are not. So it does help with review.
That said, if we want to stop checking in the generated *.rs files, I'd prefer to do that in a separate PR. There are a few other small changes here, and even if we don't check them in locally, it's still helpful to have them correctly named.
|
A separate PR is fine. Could it precede this one to simplify the diffstats for everything that's in flight? |
We already save platform-specific
.snapfiles for snapshot tests, but the.rsfiles still clash, and we aren't checking them in for all platforms. This moves the.rsfiles to.{platform}.rs. And since the crate name can't have a.in it, we need to explicitly set the crate name to the file stem now.This also cleans up some other little stuff.
dummy.cisn't needed anymore since we now have 2 other platform-specific snapshots. Also, set the.rlibpath with-oexplicitly instead of guessing what the output file will be named.