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

Makefile wholearchive responsefile support #458

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

KageKirin
Copy link
Contributor

Hi,

this is the PR for the follow up changes to #456 I mentioned in the latter's PR.
This PR adds handling of wholearchive libs with responsefiles.
(Simply said, I had to link several libs as wholearchive for Android, and ran into this self-created bug).

The actual changes from this PR starts after b5a3826, the part before being the previous work to be as #456.

Cheers.

CC @rhoot for review as well, please.

@KageKirin
Copy link
Contributor Author

On second thought, I think it would be easier to just review and merge this PR instead of the 2.

- depend on libdeps
- iterate over lddeps to write responsefile

This conserves the order of dependency
@KageKirin KageKirin force-pushed the makefile-wholearchive-attfile branch from 16a45a9 to ad9cb41 Compare July 24, 2019 08:59
@KageKirin
Copy link
Contributor Author

I'm still testing, but I think I have a fix for the issue raised by @rhoot.
Please tell me if you think this version is acceptable to be merged.

@KageKirin KageKirin force-pushed the makefile-wholearchive-attfile branch from ad9cb41 to 23e33e1 Compare July 25, 2019 13:07
@KageKirin
Copy link
Contributor Author

So, I finished testing this (by building my work project for Android), and it works.

@rhoot Could you please review this again? Thanks.

@KageKirin
Copy link
Contributor Author

Hey @bkaradzic and @rhoot,
since this PR has been around for quite some time now, would you mind reviewing it, please?

Especially if there are things to fix or improve, I'd like to get feedback about them.
Cheers.

@KageKirin
Copy link
Contributor Author

Happy thanksgiving.
Maybe we can merge this now? Pretty please.

@bkaradzic
Copy link
Owner

Yeah waiting for @rhoot to review it...

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.

3 participants