Skip to content
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

linker: Update some outdated comments #99993

Merged
merged 1 commit into from
Aug 24, 2022
Merged

Conversation

petrochenkov
Copy link
Contributor

r? @bjorn3

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 31, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 31, 2022
// Historically it was needed because rustc linked rlibs as whole-archive in some cases.
// In that case linkers try to include all files located in an archive, so if metadata is stored
// in an archive then it needs to be of a form that the linker is able to process.
// Now it's not clear whether metadata still needs to be wrapped into an object file or not.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some comments in source code say that you cannot put a random non-object file into the first position in an archive.
However, I wasn't actually able to reproduce this - I tried an archive with a garbage first file on Linux and on windows-gnu targets, with both lld and bfd, and it worked successfully.
@bjorn3 Maybe you know where the relevant code in lld or binutils is? I did some search, but couldn't find it.

I'd like to make a YOLO PR that removes the object file wrapping from metadata, but still puts it on the first place in the archive, and run this PR through various targets available on CI.
Maybe in 1-2 weeks.

Copy link
Member

Choose a reason for hiding this comment

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

If I recall correctly on macOS non-object files must be at the end. I believe I recall hitting this requirement early on in the development of cg_clif.

@bjorn3
Copy link
Member

bjorn3 commented Aug 24, 2022

@bors r+ rollup=always

@bors
Copy link
Contributor

bors commented Aug 24, 2022

📌 Commit 04a7fef has been approved by bjorn3

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 24, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 24, 2022
…iaskrgr

Rollup of 15 pull requests

Successful merges:

 - rust-lang#99993 (linker: Update some outdated comments)
 - rust-lang#100220 (Properly forward `ByRefSized::fold` to the inner iterator)
 - rust-lang#100826 (sugg: take into count the debug formatting)
 - rust-lang#100855 (Extra documentation for new formatting feature)
 - rust-lang#100888 (Coherence negative impls implied bounds)
 - rust-lang#100901 (Make some methods private)
 - rust-lang#100906 (Suggest alternatives when trying to mutate a `HashMap`/`BTreeMap` via indexing)
 - rust-lang#100912 (Diagnose missing includes in run-make tests)
 - rust-lang#100919 (Use par_body_owners for liveness)
 - rust-lang#100922 (Rewrite error index generator to greatly reduce the size of the pages)
 - rust-lang#100926 (Update README.md)
 - rust-lang#100930 (Use `--userns=keep-id` when "docker" is really podman)
 - rust-lang#100938 (rustdoc: remove unused CSS rule)
 - rust-lang#100940 (Do not suggest adding a bound to a opaque type)
 - rust-lang#100945 (Add a missing test case for impl generic mismatch)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit bc05045 into rust-lang:master Aug 24, 2022
@rustbot rustbot added this to the 1.65.0 milestone Aug 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

6 participants