Skip to content

Conversation

@clementperon
Copy link
Contributor

Allow users to specify a custom protoc executable by setting the OR_TOOLS_PROTOC_EXECUTABLE variable, which takes precedence over the default cross-compilation and system protoc detection logic.

@clementperon
Copy link
Contributor Author

Building OR-Tools via Yocto, I need to specify where to find the protoc host binary

@lperron lperron requested a review from Mizux June 10, 2025 17:19
@Mizux Mizux self-assigned this Jun 11, 2025
@Mizux Mizux added Feature Request Missing Feature/Wrapper Build: CMake CMake based build issue labels Jun 11, 2025
@Mizux
Copy link
Collaborator

Mizux commented Jun 11, 2025

two things:

  1. Could you submit your PR against the main branch instead ?

  2. I'm wondering whether it would be better to use if(DEFINED $ENV{PROTOC_EXECUTABLE}) aka using env variable instead.

note:

git grep -in "if(DEFINED ENV" **/*.cmake
cmake/FindCPLEX.cmake:54:  if(DEFINED ENV{UNIX_CPLEX_DIR})
cmake/FindCPLEX.cmake:56:  elseif(DEFINED ENV{CPLEX_ROOT})
cmake/FindGLPK.cmake:69:if(DEFINED ENV{GLPK_ROOT})
cmake/FindJulia.cmake:46:if(DEFINED ENV{JULIA_BINDIR})
cmake/FindJulia.cmake:52:if(DEFINED ENV{JULIA_BINDIR})
cmake/dotnet.cmake:343:if(DEFINED ENV{DOTNET_SNK})
cmake/utils.cmake:39:  if(DEFINED ENV{OR_TOOLS_MAJOR} AND DEFINED ENV{OR_TOOLS_MINOR})
cmake/utils.cmake:55:  if(DEFINED ENV{OR_TOOLS_PATCH})

@clementperon
Copy link
Contributor Author

clementperon commented Jun 11, 2025

two things:

  1. Could you submit your PR against the main branch instead ?

Yes will do

  1. I'm wondering whether it would be better to use if(DEFINED $ENV{PROTOC_EXECUTABLE}) aka using env variable instead.

note:

git grep -in "if(DEFINED ENV" **/*.cmake
cmake/FindCPLEX.cmake:54:  if(DEFINED ENV{UNIX_CPLEX_DIR})
cmake/FindCPLEX.cmake:56:  elseif(DEFINED ENV{CPLEX_ROOT})
cmake/FindGLPK.cmake:69:if(DEFINED ENV{GLPK_ROOT})
cmake/FindJulia.cmake:46:if(DEFINED ENV{JULIA_BINDIR})
cmake/FindJulia.cmake:52:if(DEFINED ENV{JULIA_BINDIR})
cmake/dotnet.cmake:343:if(DEFINED ENV{DOTNET_SNK})
cmake/utils.cmake:39:  if(DEFINED ENV{OR_TOOLS_MAJOR} AND DEFINED ENV{OR_TOOLS_MINOR})
cmake/utils.cmake:55:  if(DEFINED ENV{OR_TOOLS_PATCH})

The ENV you're pointing is for FindPackage CMake Module it could be a good hint to find some places where to look for a specific package.
Here I don't want to give a hint but really want to enforce which HOST PROTOC binary to use.
I don't think using an env variable is more appropriate.

@clementperon clementperon changed the base branch from stable to master June 11, 2025 11:24
…XECUTABLE

Allow users to specify a custom protoc executable by setting the
OR_TOOLS_PROTOC_EXECUTABLE variable, which takes precedence over
the default cross-compilation and system protoc detection logic.

Signed-off-by: Clément Péron <peron.clem@gmail.com>
@clementperon clementperon changed the base branch from master to main June 11, 2025 11:26
Copy link
Collaborator

@Mizux Mizux left a comment

Choose a reason for hiding this comment

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

LGTM

@Mizux Mizux merged commit f8f4651 into google:main Jun 11, 2025
109 of 145 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Build: CMake CMake based build issue Feature Request Missing Feature/Wrapper

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants