Skip to content

Migrate libs-through-symlink to rmake.rs#134829

Merged
bors merged 1 commit intorust-lang:masterfrom
jieyouxu:migrate-libs-through-symlinks
Dec 28, 2024
Merged

Migrate libs-through-symlink to rmake.rs#134829
bors merged 1 commit intorust-lang:masterfrom
jieyouxu:migrate-libs-through-symlinks

Conversation

@jieyouxu
Copy link
Member

@jieyouxu jieyouxu commented Dec 27, 2024 •
edited
Loading

Part of #121876.

This PR migrates tests/run-make/libs-through-symlink/ to use rmake.rs.

I.e. it is now

$test_output/           # rustc foo.rs -o actual_lib_dir/libfoo.rlib
    actual_lib_dir/     
        libfoo.rlib
    symlink_lib_dir/    # CWD set; rustc -L . bar.rs
        libfoo.rlib --> $test_output/actual_lib_dir/libfoo.rlib

Partially supersedes #129011.
This PR is co-authored with @Oneirical.

r? compiler

@rustbot rustbot added A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 27, 2024
@jieyouxu jieyouxu force-pushed the migrate-libs-through-symlinks branch from 416193f to 2b33ce9 Compare December 27, 2024 18:32
This was referenced Dec 27, 2024
Closed
Closed
@jieyouxu
Copy link
Member Author

jieyouxu commented Dec 27, 2024

@bors rollup=iffy (symlinks)

lqd
lqd reviewed Dec 27, 2024
View reviewed changes
@jieyouxu jieyouxu force-pushed the migrate-libs-through-symlinks branch from 2b33ce9 to 3e8ec15 Compare December 27, 2024 18:52
lqd
lqd reviewed Dec 27, 2024
View reviewed changes
lqd
lqd approved these changes Dec 27, 2024
View reviewed changes
Copy link
Member

@lqd lqd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sweet, thanks!

398989730-2c284c24-7ac7-47e4-9c4c-740ab31dd9bb r=me with the above comment, as well as "After this was fixed in #13903, this test checks that compilation succeeds when using such a symlink."

@jieyouxu jieyouxu force-pushed the migrate-libs-through-symlinks branch 2 times, most recently from 94ab101 to 1280dcd Compare December 27, 2024 19:40
@jieyouxu @Oneirical
tests: migrate libs-through-symlink to rmake.rs
b77ab2d 
- Document test intent, backlink to rust-lang#13890 and fix PR rust-lang#13903.
- Fix the test logic: the `Makefile` version seems to not actually be
  exercising the "library search traverses symlink" logic, because the
  actual symlinked-to-library is present under the directory tree when
  `bar.rs` is compiled, because the `$(RUSTC)` invocation has an
  implicit `-L $(TMPDIR)`. The symlink itself was actually broken, i.e.
  it should've been `ln -nsf $(TMPDIR)/outdir/$(NAME) $(TMPDIR)` but it
  used `ln -nsf outdir/$(NAME) $(TMPDIR)`.

Co-authored-by: Oneirical <manchot@videotron.ca>
@jieyouxu jieyouxu force-pushed the migrate-libs-through-symlinks branch from 1280dcd to b77ab2d Compare December 28, 2024 03:53
@jieyouxu
Copy link
Member Author

jieyouxu commented Dec 28, 2024

Fixed the comments.
@bors r=@lqd rollup

@bors
Copy link
Collaborator

bors commented Dec 28, 2024

📌 Commit b77ab2d has been approved by lqd

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 28, 2024
@Zalathar Zalathar mentioned this pull request Dec 28, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 28, 2024
@bors
Auto merge of rust-lang#134844 - Zalathar:rollup-1225wh9, r=Zalathar
8b3f7ac 
Rollup of 5 pull requests

Successful merges:

 - rust-lang#134737 (Implement `default_overrides_default_fields` lint)
 - rust-lang#134760 (Migrate `branch-protection-check-IBT` to rmake.rs)
 - rust-lang#134829 (Migrate `libs-through-symlink` to rmake.rs)
 - rust-lang#134832 (Update `compiler-builtins` to 0.1.140)
 - rust-lang#134840 (compiletest: Only pass the post-colon value to `parse_normalize_rule`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3048c4a into rust-lang:master Dec 28, 2024
6 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Dec 28, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 28, 2024
@rust-timer
Unrolled build for rust-lang#134829
23c47aa 
Rollup merge of rust-lang#134829 - jieyouxu:migrate-libs-through-symlinks, r=lqd

Migrate `libs-through-symlink` to rmake.rs

Part of rust-lang#121876.

This PR migrates `tests/run-make/libs-through-symlink/` to use rmake.rs.

- Regression test for rust-lang#13890.
- Original fix PR is rust-lang#13903.
- Document test intent, backlink to rust-lang#13890 and fix PR rust-lang#13903.
- Fix the test logic: the `Makefile` version seems to not actually be exercising the "library search traverses symlink" logic, because the actual symlinked-to-library is present under the `$(TMPDIR)` directory tree when `bar.rs` is compiled, because the `$(RUSTC)` invocation has an implicit `-L $(TMPDIR)`. The symlink itself was actually broken, i.e. it should've been `ln -nsf $(TMPDIR)/outdir/$(NAME) $(TMPDIR)` but it used `ln -nsf outdir/$(NAME) $(TMPDIR)`. The rmake.rs version now explicitly separates the two directory trees and sets the CWD of the `bar.rs` rustc invocation so that the actual library is *not* present under its CWD tree.

I.e. it is now

```
$test_output/           # rustc foo.rs -o actual_lib_dir/libfoo.rlib
    actual_lib_dir/
        libfoo.rlib
    symlink_lib_dir/    # CWD set; rustc -L . bar.rs
        libfoo.rlib --> $test_output/actual_lib_dir/libfoo.rlib
```

Partially supersedes rust-lang#129011.
This PR is co-authored with `@Oneirical.`

r? compiler
@jieyouxu jieyouxu deleted the migrate-libs-through-symlinks branch December 28, 2024 11:17
poliorcetics pushed a commit to poliorcetics/rust that referenced this pull request Dec 28, 2024
@Zalathar @poliorcetics
Rollup merge of rust-lang#134829 - jieyouxu:migrate-libs-through-syml…
dec9404 
…inks, r=lqd

Migrate `libs-through-symlink` to rmake.rs

Part of rust-lang#121876.

This PR migrates `tests/run-make/libs-through-symlink/` to use rmake.rs.

- Regression test for rust-lang#13890.
- Original fix PR is rust-lang#13903.
- Document test intent, backlink to rust-lang#13890 and fix PR rust-lang#13903.
- Fix the test logic: the `Makefile` version seems to not actually be exercising the "library search traverses symlink" logic, because the actual symlinked-to-library is present under the `$(TMPDIR)` directory tree when `bar.rs` is compiled, because the `$(RUSTC)` invocation has an implicit `-L $(TMPDIR)`. The symlink itself was actually broken, i.e. it should've been `ln -nsf $(TMPDIR)/outdir/$(NAME) $(TMPDIR)` but it used `ln -nsf outdir/$(NAME) $(TMPDIR)`. The rmake.rs version now explicitly separates the two directory trees and sets the CWD of the `bar.rs` rustc invocation so that the actual library is *not* present under its CWD tree.

I.e. it is now

```
$test_output/           # rustc foo.rs -o actual_lib_dir/libfoo.rlib
    actual_lib_dir/
        libfoo.rlib
    symlink_lib_dir/    # CWD set; rustc -L . bar.rs
        libfoo.rlib --> $test_output/actual_lib_dir/libfoo.rlib
```

Partially supersedes rust-lang#129011.
This PR is co-authored with `@Oneirical.`

r? compiler
poliorcetics pushed a commit to poliorcetics/rust that referenced this pull request Dec 28, 2024
@bors @poliorcetics
Auto merge of rust-lang#134844 - Zalathar:rollup-1225wh9, r=Zalathar
042b5dd 
Rollup of 5 pull requests

Successful merges:

 - rust-lang#134737 (Implement `default_overrides_default_fields` lint)
 - rust-lang#134760 (Migrate `branch-protection-check-IBT` to rmake.rs)
 - rust-lang#134829 (Migrate `libs-through-symlink` to rmake.rs)
 - rust-lang#134832 (Update `compiler-builtins` to 0.1.140)
 - rust-lang#134840 (compiletest: Only pass the post-colon value to `parse_normalize_rule`)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lqd lqd lqd approved these changes

Assignees

@SparrowLii SparrowLii

Labels

A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.85.0

Development

Successfully merging this pull request may close these issues.

5 participants

@jieyouxu @bors @lqd @rustbot @SparrowLii