Skip to content

Conversation

Enselic
Copy link
Member

@Enselic Enselic commented Aug 12, 2023

Since filename_for_metadata() and
OutputFilenames::path(OutputType::Metadata) had different logic for the name of the metadata file, the .d file contained a file name different from the actual name used. Share the logic to fix the out-of-sync name.

Without this fix, the .d file contained

dash-separated_something-extra.rmeta: dash-separated.rs

instead of

libdash_separated_something-extra.rmeta: dash-separated.rs

which is the name of the file that is actually written by the compiler.

Worth noting: It took me several iterations to get all tests to pass, so I am relatively confident that this PR does not break anything.

Closes #68839

@rustbot
Copy link
Collaborator

rustbot commented Aug 12, 2023

r? @compiler-errors

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 12, 2023
@compiler-errors
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 30, 2023

📌 Commit 282e67408b5db63bea6f36e062d35fc491c30ea8 has been approved by compiler-errors

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 Aug 30, 2023
@bors
Copy link
Collaborator

bors commented Aug 30, 2023

⌛ Testing commit 282e67408b5db63bea6f36e062d35fc491c30ea8 with merge d0172f44523c555494946d64fe220fcfa519310d...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented Aug 30, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 30, 2023
@Enselic
Copy link
Member Author

Enselic commented Aug 31, 2023

I'll look into the failure on Windows.

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 31, 2023
@Enselic Enselic force-pushed the metadata-dep-info branch from 282e674 to 1e6d41f Compare August 31, 2023 04:35
@Enselic
Copy link
Member Author

Enselic commented Aug 31, 2023

Looks like I just have to adjust how TMPDIR is stripped in the test to handle when /c/ is replaced with C:\ under the hood.

The diff from what you already approved is minimal: click here to see diff

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 31, 2023
@compiler-errors
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 2, 2023

📌 Commit 1e6d41f72fcc55452b2b6b92e39cbee3b90cf917 has been approved by compiler-errors

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Sep 2, 2023

🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened.

@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 Sep 2, 2023
@Mark-Simulacrum
Copy link
Member

@bors treeclosed-

@bors
Copy link
Collaborator

bors commented Sep 2, 2023

⌛ Testing commit 1e6d41f72fcc55452b2b6b92e39cbee3b90cf917 with merge 24b78bb32be9e844f4afcecca7eec88bd880cac9...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented Sep 2, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 2, 2023
Since `filename_for_metadata()` and
`OutputFilenames::path(OutputType::Metadata)` had different logic for
the name of the metadata file, the `.d` file contained a file name
different from the actual name used. Share the logic to fix the
out-of-sync name.

Closes 68839.
@Enselic
Copy link
Member Author

Enselic commented Sep 3, 2023

Apparently GNU sed and Mac/BSD sed have sed -i incompatibilities, so I had to change the test code again. Diff from what you already approved is small and low-risk.

(symbolic:)
@rustbot ready

@Enselic
Copy link
Member Author

Enselic commented Sep 11, 2023

@compiler-errors Friendly ping as I'm starting to suspect you might have missed this was ready for re-review with a small diff compared to what you already approved? (See comment above)

@compiler-errors
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 16, 2023

📌 Commit 04d81ba has been approved by compiler-errors

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 Sep 16, 2023
@bors
Copy link
Collaborator

bors commented Sep 17, 2023

⌛ Testing commit 04d81ba with merge 8ed1d4a...

@bors
Copy link
Collaborator

bors commented Sep 17, 2023

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing 8ed1d4a to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 17, 2023
@bors bors merged commit 8ed1d4a into rust-lang:master Sep 17, 2023
@rustbot rustbot added this to the 1.74.0 milestone Sep 17, 2023
@Enselic Enselic deleted the metadata-dep-info branch September 17, 2023 13:38
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (8ed1d4a): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

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.

mean range count
Regressions ❌
(primary)
2.2% [2.2%, 2.2%] 1
Regressions ❌
(secondary)
3.0% [2.1%, 3.7%] 6
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 2.2% [2.2%, 2.2%] 1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 634.734s -> 636.351s (0.25%)
Artifact size: 318.52 MiB -> 318.44 MiB (-0.03%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dep-info rmeta path is wrong
7 participants