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

[Android] Android get latest libs: curl,zlib,rappture #4747

Closed
wants to merge 1 commit into from

Conversation

talregev
Copy link
Contributor

@talregev talregev commented May 13, 2022

Android get latest libs: curl,zlib,rappture

@talregev talregev force-pushed the TalR/android_latest_curl branch from 950df6c to 11ce91b Compare May 13, 2022 21:55
@talregev talregev marked this pull request as draft May 13, 2022 21:59
@talregev talregev force-pushed the TalR/android_latest_curl branch from 11ce91b to 1269743 Compare May 13, 2022 22:22
@talregev talregev changed the title [Android] Android get latest curl [Android] Android get latest libs: curl,zlib May 13, 2022
@talregev talregev changed the title [Android] Android get latest libs: curl,zlib [Android] Android get latest libs: curl,zlib,rappture May 13, 2022
@talregev talregev marked this pull request as ready for review May 13, 2022 22:58
@AenBleidd
Copy link
Member

Does this mean that OpenSSL3 now builds without any issue on Android x86?

@talregev
Copy link
Contributor Author

talregev commented May 14, 2022

Does this mean that OpenSSL3 now builds without any issue on Android x86?

No. it still take the old openssl 1.1.1n.
But take the latest other libraries.
It just updated automated the latest vcpkg master head git hash inside the json file.

@talregev
Copy link
Contributor Author

Does this mean that OpenSSL3 now builds without any issue on Android x86?

We still need your fix for openssl 3.0.3 for Android.

@AenBleidd
Copy link
Member

If you want - you can implement this fix by yourself and don't wait for me.

@talregev
Copy link
Contributor Author

talregev commented May 14, 2022

If you want - you can implement this fix by yourself and don't wait for me.

I didn't understand your fix, so it better you do it. Also, you can review my PR until you have time for your fix.

@AenBleidd
Copy link
Member

AenBleidd commented May 15, 2022

There is an issue with this PR: every time you build binaries locally, vcpkg.json files are changed, and every time you need to remember to not commit these changes.
Is there any way to eliminate this?

@talregev
Copy link
Contributor Author

There is an issue with this PR: every time you build binaries locally, vcpkg.json files are changed, and every time you need to remember to not commit these changes. Is there any way to eliminate this?

Tell me the final thing you want to achieve.

@talregev
Copy link
Contributor Author

There is an issue with this PR: every time you build binaries locally, vcpkg.json files are changed, and every time you need to remember to not commit these changes. Is there any way to eliminate this?

avoid commits unchanged files?
I think we can put them in ignore

@AenBleidd
Copy link
Member

We can't put them in ignore because then you have to remember to force add them when you change them (e.g. by setting a new version).

I want to avoid errors as much as possible, and currently your solution will create errors later.

As far as I see, this later will not be needed when OpenSSL 3 issue for Android x86 is fixed.

So I'd not merge it at all for now.
The only case when we might need this is if we want to fix some particular version of any dependency on release branch but in that case vcpkg.json file change is expected.

So maybe it's better to always keep the last committed value of 'builtin-baseline' and update it from the script on demand only by passing some specific parameter to build script?

@talregev
Copy link
Contributor Author

We can't put them in ignore because then you have to remember to force add them when you change them (e.g. by setting a new version).

I want to avoid errors as much as possible, and currently your solution will create errors later.

As far as I see, this later will not be needed when OpenSSL 3 issue for Android x86 is fixed.

So I'd not merge it at all for now. The only case when we might need this is if we want to fix some particular version of any dependency on release branch but in that case vcpkg.json file change is expected.

So maybe it's better to always keep the last committed value of 'builtin-baseline' and update it from the script on demand only by passing some specific parameter to build script?

@AenBleidd When are you going to fix android openssl vcpkg? If it soon, there is no point to discuss.

@AenBleidd
Copy link
Member

Hopefully this week. I have a lot of stuff I need to do after vacation. Let's see how it goes.

@AenBleidd
Copy link
Member

Fix is provided and currently in review: microsoft/vcpkg#24801
@talregev, should we close this PR without merging or you have some other plans on it?

@talregev
Copy link
Contributor Author

Fix is provided and currently in review: microsoft/vcpkg#24801
@talregev, should we close this PR without merging or you have some other plans on it?

We should wait for your fix. After that we can discuss. Also if your fix is delay we can discuss again.

@talregev talregev closed this May 20, 2022
@talregev talregev deleted the TalR/android_latest_curl branch May 20, 2022 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants