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 bazel/gradle compilation conflict #24

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

Conversation

carlonzo
Copy link
Contributor

@carlonzo carlonzo commented Jun 19, 2023

  • Renames BUILD file from bazel to BUILD.bazel to solve a conflict with the build folder created by gradle. probably is worth to rename all the bazel BUILD files?

  • Allows Ukey2ShellCppWrapper.java to use theukey2_shell binary from the default output folder of bazel

  • Upgrade protobuf to 0.9.3. From 0.9.1 changelog :

Fixed a regression from 0.8.x where the proto source set filters were not being applied, causing non-.proto files to be passed to protoc

This was causing the generation of proto fail given the proto folder contains a bazel BUILD file

@carlonzo
Copy link
Contributor Author

I am happy to split the PR in multiple pieces if is preferred

@carlonzo
Copy link
Contributor Author

also this repo requires some CI steps to validate the build. quite a few steps are broken after the introduction of bazel as seen from this and my previous PR.

would you accept a PR to add github actions to build and test the code?

@anayw2001
Copy link
Collaborator

also this repo requires some CI steps to validate the build. quite a few steps are broken after the introduction of bazel as seen from this and my previous PR.

would you accept a PR to add github actions to build and test the code?

i was actually thinking of doing this as well, so i absolutely would accept a pr for that!

@@ -50,6 +51,7 @@
public class Ukey2ShellCppWrapper {
// The path the the ukey2_shell binary.
private static final String BINARY_PATH = "build/src/main/cpp/src/securegcm/ukey2_shell";
Copy link
Collaborator

Choose a reason for hiding this comment

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

we don't build w cmake anymore, so this should probably be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you planning to support only bazel for the c++ library and gradle for java?

Copy link
Collaborator

Choose a reason for hiding this comment

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

yep, since this is built as part of https://github.com/google/nearby, so that's why a lot of this stuff wasn't updated correctly (my bad!)

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.

2 participants