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

CMakeLists.txt should be in top-level directory #9596

Closed
Arfrever opened this issue Mar 8, 2022 · 5 comments · Fixed by #9603
Closed

CMakeLists.txt should be in top-level directory #9596

Arfrever opened this issue Mar 8, 2022 · 5 comments · Fixed by #9603

Comments

@Arfrever
Copy link
Contributor

Arfrever commented Mar 8, 2022

The standard practice is to put main CMakeLists.txt in top-level directory, not in subdirectory called cmake or similar.
Some examples of projects:
abseil-cpp
gflags
glog

I would suggest to move Protobuf's cmake/CMakeLists.txt to top-level directory and adjust appropriate paths.
If there is agreement with this suggestion, then I can try to work on it.

@acozzette
Copy link
Member

Is there a way to do this without breaking scripts that rely on the existing location? Maybe we could leave a symlink at the old location if that works.

@bsergean
Copy link

bsergean commented Mar 8, 2022

Old versions of Windows don't support symlinks, but newer might. It would be great if the Cmakefile was usable on linux/mac as well.

@Arfrever
Copy link
Contributor Author

Arfrever commented Mar 8, 2022

cmake/CMakeLists.txt can become a small regular file with few lines:

cmake_minimum_required(VERSION 3.5)

message(WARNING "Calling of cmake with Protocol Buffers' \"cmake\" subdirectory as source directory is deprecated. Top-level directory should be used instead.")

project(protobuf C CXX)

set(protobuf_DEPRECATED_CMAKE_SUBDIRECTORY_USAGE TRUE)

include(../CMakeLists.txt)

@bsergean
Copy link

bsergean commented Mar 8, 2022 via email

@acozzette
Copy link
Member

Yeah, that looks like a good way to do it!

Arfrever added a commit to Arfrever/protobuf that referenced this issue Mar 8, 2022
cmake/CMakeLists.txt is kept for compatibility and it includes CMakeLists.txt.

Fixes: protocolbuffers#9596
Arfrever added a commit to Arfrever/protobuf that referenced this issue Mar 8, 2022
Arfrever added a commit to Arfrever/protobuf that referenced this issue Mar 8, 2022
Arfrever added a commit to Arfrever/protobuf that referenced this issue Mar 8, 2022
…variable (protocolbuffers#9596)

protobuf_source_dir variable is no longer needed and has the same value
as protobuf_SOURCE_DIR variable, which is instance of CMake-standard
<PROJECT-NAME>_SOURCE_DIR variable:
https://cmake.org/cmake/help/latest/variable/PROJECT-NAME_SOURCE_DIR.html
Arfrever added a commit to Arfrever/protobuf that referenced this issue Mar 9, 2022
Arfrever added a commit to Arfrever/protobuf that referenced this issue Mar 9, 2022
Arfrever added a commit to Arfrever/protobuf that referenced this issue Mar 9, 2022
…variable (protocolbuffers#9596)

protobuf_source_dir variable is no longer needed and has the same value
as protobuf_SOURCE_DIR variable, which is instance of CMake-standard
<PROJECT-NAME>_SOURCE_DIR variable:
https://cmake.org/cmake/help/latest/variable/PROJECT-NAME_SOURCE_DIR.html
acozzette pushed a commit that referenced this issue Mar 9, 2022
…variable (#9596)

protobuf_source_dir variable is no longer needed and has the same value
as protobuf_SOURCE_DIR variable, which is instance of CMake-standard
<PROJECT-NAME>_SOURCE_DIR variable:
https://cmake.org/cmake/help/latest/variable/PROJECT-NAME_SOURCE_DIR.html
ejona86 added a commit to ejona86/grpc-java that referenced this issue Jun 24, 2022
This should fix the build, but I don't see any warning about it being
deprecated in the logs. Looks like the compat for the old way isn't
working. protocolbuffers/protobuf#9596
ejona86 added a commit to ejona86/grpc-java that referenced this issue Jun 28, 2022
This should fix the build, but I don't see any warning about it being
deprecated in the logs. Looks like the compat for the old way isn't
working. protocolbuffers/protobuf#9596
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 a pull request may close this issue.

3 participants