Skip to content
This repository was archived by the owner on Oct 31, 2025. It is now read-only.

Comments

Use DefaultMetadataLoader for loading dylib metadata#698

Merged
khyperia merged 1 commit intoEmbarkStudios:mainfrom
bjorn3:patch-1
Aug 3, 2021
Merged

Use DefaultMetadataLoader for loading dylib metadata#698
khyperia merged 1 commit intoEmbarkStudios:mainfrom
bjorn3:patch-1

Conversation

@bjorn3
Copy link
Contributor

@bjorn3 bjorn3 commented Jul 29, 2021

If you switch archives from tar to ar archives, it would be possible to use DefaultMetadataLoader for everything.

@bjorn3 bjorn3 requested review from eddyb and khyperia as code owners July 29, 2021 14:46
@eddyb eddyb enabled auto-merge (rebase) July 29, 2021 15:53
@eddyb
Copy link
Contributor

eddyb commented Jul 29, 2021

Looks like the Android CI is just broken nowadays, but we should definitely merge this PR whenever CI is unbroken.

@bjorn3
Copy link
Contributor Author

bjorn3 commented Jul 29, 2021

bevyengine/bevy#2533 was the "fix" for bevy. Basically uninstall the broken android build tools version 31 first, so cargo-apk can install version 30 I think.

@khyperia khyperia mentioned this pull request Aug 2, 2021
Copy link
Contributor

@khyperia khyperia left a comment

Choose a reason for hiding this comment

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

If you switch archives from tar to ar archives, it would be possible to use DefaultMetadataLoader for everything.

What do you mean by "everything"? I guess like, yeah, sure, sounds great, tbh I didn't realize there was a difference between tar and ar archives, haha.

Anyway, #701 should have fixed the CI, could you rebase?

auto-merge was automatically disabled August 3, 2021 09:44

Head branch was pushed to by a user without write access

@bjorn3
Copy link
Contributor Author

bjorn3 commented Aug 3, 2021

What do you mean by "everything"?

For rlibs too and not just for dylibs.

I guess like, yeah, sure, sounds great, tbh I didn't realize there was a difference between tar and ar archives, haha.

Ar archives are much more limited. For example they don't support directories, symlinks, etc. On the other hand both the gnu and bsd flavor support a symbol table that maps from symbol name to the object file that contains said symbol. This can improve linker speed and is required by most linkers.

Anyway, #701 should have fixed the CI, could you rebase?

Done

@khyperia khyperia enabled auto-merge (squash) August 3, 2021 09:59
@khyperia khyperia merged commit cccb973 into EmbarkStudios:main Aug 3, 2021
@bjorn3 bjorn3 deleted the patch-1 branch August 3, 2021 10:25
This was referenced Aug 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants