Skip to content

Conversation

@compnerd
Copy link
Member

This is currently preventing the Windows nightlies from passing. Ensure that we build the brotli library for Android as well.

@compnerd
Copy link
Member Author

@swift-ci please smoke test

@compnerd
Copy link
Member Author

CC: @zhenchaoli @shahmishal

@zhenchaoli
Copy link
Contributor

Thank you. Sorry I missed this case.
I did kick off a Windows toolchain build on a similar change in #84865 CI status but unsure if it actually exercised the Android path. I am also running a local build to verify.

@compnerd
Copy link
Member Author

Well, the status seems to be positive - which is better than the previous state, so I think that we should continue with that and resolve any issues subsequently

@compnerd
Copy link
Member Author

@swift-ci please smoke test macOS platform

@compnerd
Copy link
Member Author

@swift-ci please smoke test

@compnerd
Copy link
Member Author

@swift-ci please smoke test

@zhenchaoli
Copy link
Contributor

Building Android SDK with build.cmd -Android seems unable to find Brotli. I see that you have one more updated commit. I will try to build with that change.
Somehow the {package}_DIR convention does not work for Android, but does for Windows build.

@compnerd
Copy link
Member Author

Building Android SDK with build.cmd -Android seems unable to find Brotli. I see that you have one more updated commit. I will try to build with that change. Somehow the {package}_DIR convention does not work for Android, but does for Windows build.

Yeah, I find that very odd. I cannot explain why that doesn't work for Android. However, I think that getting things working is more important currently.

@compnerd
Copy link
Member Author

@swift-ci please smoke test

@compnerd
Copy link
Member Author

@swift-ci please smoke test

@@ -2572,6 +2572,17 @@ function Build-CURL([Hashtable] $Platform) {
BUILD_SHARED_LIBS = "NO";
BUILD_TESTING = "NO";
CMAKE_POSITION_INDEPENDENT_CODE = "YES";
BROTLI_INCLUDE_DIR = "$SourceCache\brotli\c\include";
Copy link
Member

@etcwilde etcwilde Oct 15, 2025

Choose a reason for hiding this comment

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

Any idea why the find_package(Brotli) isn't finding these directories? It looks like brotli is getting installed under b\aarch64-unknown-linux-android28\{include\,lib\} (in the case of the Android build, of course)

Copy link
Member Author

Choose a reason for hiding this comment

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

brotli doesn't seem to have the config files for CMake to locate the package. pkg-config cannot be used as that is not a standard Windows tool. As a result, we need to pass along the paths ourselves.

I've taken this opportunity to simply not even install brotli. This is a micro-optimization as it avoids some additional disk I/O.

@compnerd
Copy link
Member Author

Please test with following PRs:
swiftlang/swift-installer-scripts#471

@swift-ci please build toolchain Windows platform

This is currently preventing the Windows nightlies from passing. Ensure
that we build the brotli library for Android as well.
@compnerd
Copy link
Member Author

Please test with following PRs:
swiftlang/swift-installer-scripts#471

@swift-ci please build toolchain Windows platform

@compnerd
Copy link
Member Author

@swift-ci please smoke test

When building Android, we would fail to find the brotli build.
Explicitly pass the library paths and include paths to workaround the
issue. This is not ideal, but will unblock the build. Avoid the
installation of the library as it is not required, which avoids updating
the mtime on a null-build.
@compnerd
Copy link
Member Author

Please test with following PRs:
swiftlang/swift-installer-scripts#471

@swift-ci please build toolchain Windows platform

@compnerd
Copy link
Member Author

@swift-ci please smoke test

@compnerd compnerd merged commit c3a1a4d into swiftlang:main Oct 16, 2025
4 checks passed
@compnerd compnerd deleted the brotli branch October 16, 2025 15:29
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