v21.0 does not install all the CMake configuration files

[coryan]

What version of protobuf and what language are you using?

Version: v21.0
Language: C++

What operating system (Linux, Windows, ...) and version?

Linux, but probably all operating systems are affected

What runtime / compiler are you using (e.g., python version or gcc version)

 gcc --version
gcc (Ubuntu 11.2.0-19ubuntu1) 11.2.0
Copyright (C) 2021 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

What did you do?

Compiled and installed protobuf using CMake. See this Dockerfile.txt

What did you expect to see

A working install.

What did you see instead?

Protobuf did not install the protobuf-config.cmake file, and thus find_package(protobuf CONFIG) is not functional.

Anything else we should know about your project / environment

[acozzette]

@jonringer I suspect this bug may be related to #9822. Do you have any thoughts on this?

[acozzette]

@jonringer I am going to revert #9822 for now to fix the regression in 3.21.0. I would be happy to merge some version of the original fix again if you can figure out how to avoid the problem in this issue, though.

[jonringer]

21.0 does install the related files

/nix/store/6h46s3n81xmsq2ag3rcddsaspw6dq66s-protobuf-3.21.0/lib/cmake
└── protobuf
    ├── protobuf-config.cmake
    ├── protobuf-config-version.cmake
    ├── protobuf-module.cmake
    ├── protobuf-options.cmake
    ├── protobuf-targets.cmake
    └── protobuf-targets-release.cmake

[jonringer]

potential problem is that it's installed under a protobuf directory, however, I just tried to emulate the existing install behavior. The only reason why it worked pre #9822 was because the build was installing the targets. So by reverting #9822, it's now more broken

[jonringer]

I'm on my honeymoon, I can come around to this in a few days; but I'm sure the fix will just be to remove the contents from the protobuf up to the cmake directory

[coryan]

AFAIK, on nixOS things like CMAKE_INSTALL_CMAKEDIR are absolute paths, so these lines:

protobuf/cmake/install.cmake

Lines 117 to 124 in 3b029b1

	configure_file(${protobuf_SOURCE_DIR}/cmake/protobuf-config.cmake.in
	${CMAKE_INSTALL_CMAKEDIR}/protobuf-config.cmake @ONLY)
	configure_file(${protobuf_SOURCE_DIR}/cmake/protobuf-config-version.cmake.in
	${CMAKE_INSTALL_CMAKEDIR}/protobuf-config-version.cmake @ONLY)
	configure_file(${protobuf_SOURCE_DIR}/cmake/protobuf-module.cmake.in
	${CMAKE_INSTALL_CMAKEDIR}/protobuf-module.cmake @ONLY)
	configure_file(${protobuf_SOURCE_DIR}/cmake/protobuf-options.cmake
	${CMAKE_INSTALL_CMAKEDIR}/protobuf-options.cmake @ONLY)

Directly create the file in the destination directory (/nix/store/6h46s3n81xmsq2ag3rcddsaspw6dq66s-protobuf-3.21.0/lib/cmake in your example). In other distributions and platforms CMAKE_INSTALL_CMAKEDIR is a relative directory. In those other platforms, the previous lines create the files in ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_INSTALL_CMAKEDIR}, and they are copied from that (temporary) location to the install destination by these lines:

protobuf/cmake/install.cmake

Lines 145 to 149 in 3b029b1

	install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_INSTALL_CMAKEDIR}/
	DESTINATION "${CMAKE_INSTALL_CMAKEDIR}"
	COMPONENT protobuf-export
	PATTERN protobuf-targets.cmake EXCLUDE
	)

Those latter lines are removed by #9822. In other words,#9822 fixed some things for nixOS, and broke other things in other distributions/platforms.

PS: all of this can wait until you return, enjoy your honeymoon.

[jonringer]

and they are copied from that (temporary) location to the install destination by these lines:

Yes, but the copied files will have targets paths which refer to the build directory, and this isn't desirable either.

[coryan]

and they are copied from that (temporary) location to the install destination by these lines:

Yes, but the copied files will have targets paths which refer to the build directory, and this isn't desirable either.

First, I am not arguing that the current code is correct, I agree it needs fixing to support nixOS. I am arguing that the code in #9822 was definitely wrong for other platforms. I tried to explain why it was incorrect, so the next iteration can avoid that pitfall.

Second, I may be reading your comment about references to the build directory wrong. It does not match the observed behavior in my testing. Naturally this may depend on how protobuf is configured and installed, and I just happen to use the right combination of options that "works" 🤷

# syntax=docker/dockerfile:1.3-labs
# test using
#   docker buildx build --progress plain -t test - <Dockerfile

FROM ubuntu:22.04

# Install development tools
ENV DEBIAN_FRONTEND=noninteractive
RUN apt-get update && \
    apt-get --no-install-recommends install -y apt-transport-https apt-utils \
        automake build-essential ccache cmake ca-certificates curl git \
        gcc g++ libc-ares-dev libc-ares2 libcurl4-openssl-dev libre2-dev \
        libssl-dev m4 make ninja-build pkg-config tar wget zlib1g-dev


# Compile and install Abseil
WORKDIR /var/tmp/build/abseil-cpp
RUN curl -sSL https://github.com/abseil/abseil-cpp/archive/20211102.0.tar.gz | \
    tar -xzf - --strip-components=1 && \
    cmake \
      -DCMAKE_BUILD_TYPE=Release \
      -DBUILD_TESTING=OFF \
      -DBUILD_SHARED_LIBS=yes \
      -DCMAKE_CXX_STANDARD=11 \
      -G Ninja -S . -B.build && \
    cmake --build .build && \
    cmake --build .build --target install && \
    ldconfig

# Compile and install Protobuf
WORKDIR /var/tmp/build/protobuf-testonly-z4OsGXYoPj
RUN curl -sSL https://github.com/protocolbuffers/protobuf/archive/v21.1.tar.gz | \
    tar -xzf - --strip-components=1 && \
    cmake \
        -DCMAKE_BUILD_TYPE=Release \
        -DBUILD_SHARED_LIBS=yes \
        -Dprotobuf_BUILD_TESTS=OFF \
        -Dprotobuf_ABSL_PROVIDER=package \
        -G Ninja -S . -B.build && \
    cmake --build .build && \
    cmake --build .build --target install  && \
    ldconfig

RUN if find /usr/local/lib/cmake/protobuf -type f xargs grep testonly-z4OsGXYoPj; then false; fi

Again, I understand the code in v21.1 does not work for nixOS. That certainly needs fixing. I think you would agree that the fixes should keep things working on other Linux distributions, and #9822 does not achieve that.