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

Allow for CMAKE_INSTALL_LIBDIR to be absolute #10090

Merged

Conversation

jonringer
Copy link
Contributor

@jonringer jonringer commented May 31, 2022

To address #10045 and #9809

Another attempt at #9822

Package repositories such as Nixpkgs will pass absolute paths to CMAKE variables.

  • Also clarify installation of cmake configuration
    files between the build and install directories.
[16:24:12] jon@jon-desktop /home/jon/projects/protobuf (fix-cmake-install-targets2)
$ tree /nix/store/khhxwdyhnc4fd28wsp4dfq0ixfy8ihjn-protobuf-git/lib/cmake
/nix/store/khhxwdyhnc4fd28wsp4dfq0ixfy8ihjn-protobuf-git/lib/cmake/
└── protobuf
    ├── protobuf-config.cmake
    ├── protobuf-config-version.cmake
    ├── protobuf-module.cmake
    ├── protobuf-options.cmake
    ├── protobuf-targets.cmake
    └── protobuf-targets-release.cmake

1 directory, 6 files

Package repositories such as NixOS will pass
an absolute paths to CMAKE.

- Also clarify installation of cmake configuration
files between the build and install directories.
@jonringer
Copy link
Contributor Author

cc @coryan

@jonringer jonringer changed the title Allow for CMAKE_INSTALL_CMAKEDIR to be absolute Allow for CMAKE_INSTALL_LIBDIR to be absolute May 31, 2022
@jonringer
Copy link
Contributor Author

Verified that the docker file correctly installs the files in both the build and install directory

FROM docker.io/library/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/jonringer/protobuf/archive/fix-cmake-install-targets2.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

Result:

$ docker build --tag protobuf -f test.Dockerfile tmp
$ docker run -t -i protobuf /bin/bash

root@af9bc88334da:/var/tmp/build/protobuf-testonly-z4OsGXYoPj# ls /usr/local/lib/cmake/protobuf/
protobuf-config-version.cmake  protobuf-module.cmake   protobuf-targets-release.cmake
protobuf-config.cmake          protobuf-options.cmake  protobuf-targets.cmake

root@af9bc88334da:/var/tmp/build/protobuf-testonly-z4OsGXYoPj# ls /var/tmp/build/protobuf-testonly-z4OsGXYoPj/.build/cmake/p
rotobuf/
protobuf-config-version.cmake  protobuf-config.cmake  protobuf-module.cmake  protobuf-options.cmake  protobuf-targets.cmake

@acozzette acozzette merged commit 046bde0 into protocolbuffers:main Jun 22, 2022
@acozzette
Copy link
Member

@jonringer Thanks and sorry for the delay in getting to this!

@jonringer jonringer deleted the fix-cmake-install-targets2 branch June 22, 2022 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants