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

Fix building with non bazel commits of boringssl #215

Merged
merged 1 commit into from
Feb 2, 2024
Merged

Conversation

rushilmehra
Copy link
Collaborator

We need to add /build/crypto and /build/ssl to the library search path to handle the case where we pass BORING_BSSL_SOURCE_PATH when building without enabling any fips features. Otherwise, non bazel commits will not work because /build/ itself will not contain any crypto libraries to link with

@nox
Copy link
Collaborator

nox commented Jan 24, 2024

We need to add /build/crypto and /build/ssl to the library search path to handle the case where we pass BORING_BSSL_SOURCE_PATH when building without enabling any fips features.

There are other issues when you do that, patches won't apply for example. The fix needs to be a bit more complex than that, e.g. require ASSUME_PATCHED if features that require patches are enabled.

I'd also rather tweak the build to check if it is a bazel build or not, and adjust the paths accordingly.

@rushilmehra
Copy link
Collaborator Author

We need to add /build/crypto and /build/ssl to the library search path to handle the case where we pass BORING_BSSL_SOURCE_PATH when building without enabling any fips features.

There are other issues when you do that, patches won't apply for example. The fix needs to be a bit more complex than that, e.g. require ASSUME_PATCHED if features that require patches are enabled.

I'd also rather tweak the build to check if it is a bazel build or not, and adjust the paths accordingly.

I updated the PR to require BORING_BSSL_ASSUME_PATCHED if you pass BORING_BSSL_SOURCE_PATH and depend on a feature that would apply a patch. As for updating paths based on the boringssl passed being bazel or not, I'm going to figure that out in a follow up PR because it's going to be a bit annoying. All the git patches assume a src/ directory is available, but for non bazel builds that isn't the case. I have a feeling I'll have to redo the patches to remove the src/ and just set src/ as the working directory for bazel versions of boringssl

@rushilmehra rushilmehra force-pushed the fix-build branch 2 times, most recently from 2a474eb to 97c3faf Compare January 27, 2024 00:06
@rushilmehra
Copy link
Collaborator Author

rushilmehra commented Jan 27, 2024

Okay now we pass -p2 to git apply when we identify a non bazel build. I tested this by changing the boringssl submodule to both a bazel and non-bazel commit, and enabling the rpk feature.

As an extra goodie, I made #217 to make builds faster

We need to add `/build/crypto` and `/build/ssl` to the library search
path to handle the case where we pass `BORING_BSSL_SOURCE_PATH` when
building without enabling any fips features. Otherwise, non bazel
commits will not work because `/build/` itself will not contain any
crypto libraries to link with
@ghedo ghedo merged commit 5aed467 into master Feb 2, 2024
23 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