-
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 std-core-cycle
, obey-crate-type-flag
, mixing-libs
and issue-18943
run-make
tests to rmake.rs
#126484
Conversation
This PR modifies cc @jieyouxu The run-make-support library was changed cc @jieyouxu These commits modify the If this was unintentional then you should revert the changes before this PR is merged. |
This comment has been minimized.
This comment has been minimized.
std-core-cycle
, obey-crate-type-flag
and issue-18943
run-make
tests to rmake.rs
std-core-cycle
, obey-crate-type-flag
, mixing-libs
and issue-18943
run-make
tests to rmake.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.
Thanks for the PR. I have some concerns regarding the APIs and their implementation introduced in this PR that warrants further discussion, but the tests themselves look very reasonable.
I've also took the liberty to edit the PR description to include try jobs for apple (darwin), windows and linux for the std-core-cycle. This PR should be run on those try jobs before approval. @bors rollup=iffy (the std-core-cycle looks platform-difference sensitive) |
I have removed glob once more and removed all instances of @rustbot review |
@bors delegate+ (for running try job, don't r+ just yet) |
✌️ @Oneirical, you can now approve this pull request! If @jieyouxu told you to " |
This comment has been minimized.
This comment has been minimized.
☔ The latest upstream changes (presumably #127044) made this pull request unmergeable. Please resolve the merge conflicts. |
b2a7e20
to
3fe832d
Compare
@rustbot review |
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.
Looks good, r=Kobzol and I after addressing the suggestions.
|
||
fn main() { | ||
rustc().input("foo.rs").crate_type("lib").run(); | ||
fs_wrapper::remove_file(rust_lib_name("foo")); |
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: this should be checking for existence of the lib file, not checking-by-proxy via trying to remove it I feel like?
fs_wrapper::remove_file(dynamic_lib_name("test")); | ||
fs_wrapper::remove_file(rust_lib_name("test")); |
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: ditto here about checking for existence, not checking-by-proxy via trying to remove file.
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.
In this particular case, removing the rust lib is necessary for the final check to work, but to make it explicit, I made it so it checks existence then removes it. Even though the removal checks for existence, it should be clearer for the test reader.
Forgot to queue this one up. |
☀️ Test successful - checks-actions |
Finished benchmarking commit (032be6f): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results (primary 7.1%)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.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 699.147s -> 700.459s (0.19%) |
Part of #121876 and the associated Google Summer of Code project.
try-job: x86_64-apple-1
try-job: x86_64-msvc
try-job: aarch64-gnu