Skip to content

Don't link pthread on Android#86

Merged
martinthomson merged 4 commits intomartinthomson:mainfrom
gruberb:android-dyn-libs-pthread
Nov 18, 2025
Merged

Don't link pthread on Android#86
martinthomson merged 4 commits intomartinthomson:mainfrom
gruberb:android-dyn-libs-pthread

Conversation

@gruberb
Copy link
Contributor

@gruberb gruberb commented Oct 21, 2025

No description provided.

@martinthomson
Copy link
Owner

Hi @gruberb, do you know of a way to test this effectively? I haven't wired up CI for android here, so I'm a little reluctant to take a change like this without some confirmation that it's safe. Have you tested this with Fenix?

@gruberb
Copy link
Contributor Author

gruberb commented Oct 22, 2025

Hey @martinthomson. Yeah, we are currently trying to integrate ohttp into application-services and the CI for Fenix crashed because of the missing lpthread library. With the recent commit, the CI runs fine for Fenix.

I put it in Draft because of that. I am not sure currently how to make sure everything works as before. Maybe we keep it in this branch/commit and run it through a full cycle, or we find another way to test this.

@gruberb gruberb force-pushed the android-dyn-libs-pthread branch from d4b3878 to fc74f7c Compare October 22, 2025 13:07
@bendk
Copy link

bendk commented Nov 4, 2025

Getting this to work in the app-services taskcluster CI took some investigation, but I think I was able to figure it out. Removing the -lpthreads worked, but then we ran into a second issue when trying to run bindgen to generate the Rust files for the NSS header files. That failed when trying to cross-compile for Darwin. I needed to add some flags so that bindgen could see the OSX header files. After that it everything seems to be working okay.

mozilla/application-services#7040

@martinthomson
Copy link
Owner

Thanks for confirming. Let me know that you are ready to proceed by marking this ready for review (it looks like it is already).

Copy link
Owner

@martinthomson martinthomson left a comment

Choose a reason for hiding this comment

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

LGTM

@gruberb gruberb marked this pull request as ready for review November 5, 2025 18:17
@gruberb gruberb force-pushed the android-dyn-libs-pthread branch from a94efa4 to 160e546 Compare November 7, 2025 13:50
@gruberb
Copy link
Contributor Author

gruberb commented Nov 12, 2025

Can you run the workflow once more @martinthomson? Should be all good to go now! Thanks!

@martinthomson martinthomson merged commit 95dd82f into martinthomson:main Nov 18, 2025
4 checks passed
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