Skip to content

progressive linking #564

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

Merged
merged 2 commits into from
Nov 18, 2020
Merged

progressive linking #564

merged 2 commits into from
Nov 18, 2020

Conversation

maxded
Copy link
Contributor

@maxded maxded commented Nov 17, 2020

Should fix both #547 and #496

  • When building from msvc, linking is split into chunks based on the estimated command line length and the number of objects that need linking. If the estimated command line length doesn't exceed 1024 * 6 it will process all files in one go like before.
  • On non msvc hosts, nothing has changed as the objects are not split into chunks.

All tests succeeded locally and as an additional test I've compiled the huge physx crate from msvc to both msvc and Android.

Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

Thanks for this!

src/lib.rs Outdated

for chunk in objs.chunks(chunk_size) {
Copy link
Member

Choose a reason for hiding this comment

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

Instead of calculating chunk_size could we perhaps conservatively just say the chunk size is 100? That seems like it would probably cover almost all use cases I think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing!

@alexcrichton alexcrichton merged commit e4ff872 into rust-lang:master Nov 18, 2020
@alexcrichton
Copy link
Member

👍

@maxded maxded mentioned this pull request Nov 18, 2020
alexcrichton added a commit that referenced this pull request Nov 20, 2020
The `r` option means "add the object while replacing it if it already
exists", while the `q` option says "always add to the end". We actually
want the latter of these due to how this crate works because we're
always building archives from scratch, and we always want all our
objects to go in the archive instead of trying to replace previous
versions.

This should fix a recent issue with #564 where previously if everything
was in one giant command line `ar` would append two files but now
afterwards with multiple invocations the second invocation may overwrite
files added in the first invocation.

Closes #568
alexcrichton added a commit that referenced this pull request Nov 20, 2020
The `r` option means "add the object while replacing it if it already
exists", while the `q` option says "always add to the end". We actually
want the latter of these due to how this crate works because we're
always building archives from scratch, and we always want all our
objects to go in the archive instead of trying to replace previous
versions.

This should fix a recent issue with #564 where previously if everything
was in one giant command line `ar` would append two files but now
afterwards with multiple invocations the second invocation may overwrite
files added in the first invocation.

Closes #568
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants