cmake.setupHook: define shareDocName #93166

jtojnar · 2020-07-15T07:35:49Z

Motivation for this change

The docdir flag needs to include PROJECT_NAME according to GNU guidelines. We are passing -DCMAKE_INSTALL_DOCDIR=${!outputDoc}/share/doc/${shareDocName} but $shareDocName was unset.

The multiple-outputs.sh setup hook actually only defines shareDocName as a local variable so it was not available for cmake setup hook. Making it global would be of limited usability, since it primarily tries to extract the project name from configure script. Additionally, it would not be set because the setup hook defines setOutputFlags=, preventing the function defining shareDocName from running. And lastly, the function would not run for single-output derivations.

Previously, we tried not disabling setOutputFlags and passing the directory flags only for multi-output derivations that do not disable setOutputFlags but that meant having two different branches of code, making it harder to check correctness. The multi-output one did in fact not work due to aforementioned undefined shareDocName. It also broke derivations that set setOutputFlags= like qtModule function does (probably because some Qt modules have configure scripts incompatible with configureFlags defined by multiple-outputs.sh setup hook). For that reason, it was reverted, putting us back to start.

Let’s try to extract the project name from CMake in the cmake setup hook.

CMake has a -L flag for dumping variables but PROJECT_NAME did not seem to be among them when I tested, so I had to resort to parsing the CMakeLists.txt file.

The extraction function is limited, it does not deal with

project name on different line from the project( command opening
- that will just not get matched so we will fall back to using the derivation name
variable interpolation
- we will just fall back to using derivation name when the extracted project_name contains a dollar character
multiple project commands
- The command sets PROJECT_NAME variable anew with each call, so the last project call before include(GNUInstallDirs) command will be used when the included module would cache the CMAKE_INSTALL_DOCDIR variable. We will just take the first discovered project command for simplicity. Hopefully, there are not many projects that use multiple project calls before including GNUInstallDirs.

In either case, we will have some subdirectory so the conflicts will be minimized.

jtojnar · 2020-07-15T07:38:11Z

Tested the parser using:

urls=(
    https://github.com/AndreasRoither/die-together/blob/5932e7562bcc94ce91ab848dba9ea0f3ce4153d2/DieTogether/CMakeLists.txt
    https://github.com/nhorro/libcsvevent/blob/0a016579e56eb09dc0c7454888d17adb00d056c9/CMakeLists.txt
    https://github.com/stream-labs/lib-streamlabs-datalane/blob/69edf8a083f74dd4a467d49e3b67a099399fb23b/CMakeLists.txt
    https://github.com/ojdkbuild/deps_curl/blob/6eb890ae3e86cc2ab91ea26dabbf5d2bb8d16ed7/resources/curl_7290_cmake/CMakeLists.txt
    https://github.com/JJscott/Gain2015/blob/ac48563ab755c85fb835397d5542ea9c46d89be6/work/CMakeLists.txt
    https://github.com/SolitaryThinker/C-YCSB/blob/814013c055081b7250a828c1a1e21b860fc65abc/CMakeLists.txt
    https://github.com/pyrech/telemetre/blob/9c03ff5e774a7ab42d4b04fbefc1303da0592750/projet_cmake/Plotter/CMakeLists.txt
    https://github.com/a1batross/xray-16-travis-playground/blob/84724ac518c53e1cb1138b332ce94a4daebc6c77/src/Layers/xrRenderPC_GL/CMakeLists.txt
    https://github.com/skypjack/eventpp/blob/42b905fa122a3ebe6ba5daae3d43be9fe195b194/CMakeLists.txt
    https://github.com/zakharov-binp/Scattering-study/blob/c46e4255e2d766287f9b4ee8b75aa7196d1464a3/Moliere_code/tbsw/source/TBTools/CMakeLists.txt
)
for url in "${urls[@]}"; do
    curl -s -L "${url//\/blob\///raw/}" | grep --only-matching --perl-regexp --ignore-case '\bproject\s*\(\s*"?\K([^[:space:]")]+)' | head -n1
done

The expected output

UE4
csvevent
lib-datalane
lookaside_curl
CGRA_PROJECT_${CGRA_PROJECT}
C-YCSB
xrRender_GL
event++
TBTools

This is a very common path that often collides with other packages.

alyssais · 2020-07-15T17:00:01Z

I just tried this on a CMake project and it worked to get the project name: cmake --system-information | grep '^CMAKE_PROJECT_NAME:STATIC=' | cut -d = -f 2- Maybe we can avoid the parser?

alyssais · 2020-07-15T17:02:02Z

The reverts should probably be after the fix. :)

The docdir flag needs to include `PROJECT_NAME` according to [GNU guidelines]. We are passing `-DCMAKE_INSTALL_DOCDIR=${!outputDoc}/share/doc/${shareDocName}` but `$shareDocName` was unset. The `multiple-outputs.sh` setup hook actually only defines `shareDocName` as a local variable so it was not available for cmake setup hook. Making it global would be of limited usability, since it primarily tries to extract the project name from configure script. Additionally, it would not be set because the setup hook defines `setOutputFlags=`, preventing the function defining `shareDocName` from running. And lastly, the function would not run for single-output derivations. Previously, we tried [not disabling `setOutputFlags`] and passing the directory flags only for multi-output derivations that do not disable `setOutputFlags` but that meant having two different branches of code, making it harder to check correctness. The multi-output one did in fact not work due to aforementioned undefined `shareDocName`. It also broke derivations that set `setOutputFlags=` like [`qtModule` function does] (probably because some Qt modules have configure scripts incompatible with `configureFlags` defined by `multiple-outputs.sh` setup hook). For that reason, it was [reverted], putting us back to start. Let’s try to extract the project name from CMake in the cmake setup hook. CMake has a `-L` flag for dumping variables but `PROJECT_NAME` did not seem to be among them when I tested, so I had to resort to parsing the `CMakeLists.txt` file. The extraction function is limited, it does not deal with * project name on different line from the `project(` command opening - that will just not get matched so we will fall back to using the derivation name * variable interpolation - we will just fall back to using derivation name when the extracted `project_name` contains a dollar character * multiple [`project`] commands - The command sets `PROJECT_NAME` variable anew with each call, so the last `project` call before `include(GNUInstallDirs)` command will be used when the included module would [cache the `CMAKE_INSTALL_DOCDIR` variable]. We will just take the first discovered `project` command for simplicity. Hopefully, there are not many projects that use multiple `project` calls before including `GNUInstallDirs`. In either case, we will have some subdirectory so the conflicts will be minimized. [GNU guidelines]: https://www.gnu.org/prep/standards/html_node/Directory-Variables.html#index-docdir [not disabling `setOutputFlags`]: NixOS@be1b225 [`qtModule` function does]: NixOS#12740 [reverted]: NixOS#92298 [`PROJECT_NAME`]: https://cmake.org/cmake/help/v3.18/variable/PROJECT_NAME.html [`project`]: https://cmake.org/cmake/help/v3.18/command/project.html [cache the `CMAKE_INSTALL_DOCDIR` variable]: https://github.com/Kitware/CMake/blob/92e30d576d66ac05254bba0f0ff7d655947beb0f/Modules/GNUInstallDirs.cmake#L298-L299

This reverts commit 25bc56f.

This reverts commit 5d7cbc6.

jtojnar · 2020-07-16T02:38:14Z

I just tried this on a CMake project and it worked to get the project name: cmake --system-information | grep '^CMAKE_PROJECT_NAME:STATIC=' | cut -d = -f 2- Maybe we can avoid the parser?

I tried --system-information too and it does not seem to work for me with transmission package.

Neither in `preConfigure` or `postConfigure`

diff --git a/pkgs/applications/networking/p2p/transmission/default.nix b/pkgs/applications/networking/p2p/transmission/default.nix
index d59cdff34fd..e77449997fc 100644
--- a/pkgs/applications/networking/p2p/transmission/default.nix
+++ b/pkgs/applications/networking/p2p/transmission/default.nix
@@ -70,6 +70,12 @@ in stdenv.mkDerivation {
   ++ lib.optionals stdenv.isLinux [ inotify-tools ]
   ;
 
+  preConfigure = ''
+    echo "5555555555555555555555555555555555555555555"
+    cmake --system-information | grep '^CMAKE_PROJECT_NAME:STATIC=' | cut -d = -f 2-
+    die
+  '';
+
   NIX_LDFLAGS = lib.optionalString stdenv.isDarwin "-framework CoreFoundation";
 
   # Reduce the risk of collisions

diff --git a/pkgs/applications/networking/p2p/transmission/default.nix b/pkgs/applications/networking/p2p/transmission/default.nix
index d59cdff34fd..e3c0f502ef0 100644
--- a/pkgs/applications/networking/p2p/transmission/default.nix
+++ b/pkgs/applications/networking/p2p/transmission/default.nix
@@ -70,6 +70,14 @@ in stdenv.mkDerivation {
   ++ lib.optionals stdenv.isLinux [ inotify-tools ]
   ;
 
+  postConfigure = ''
+    pushd ..
+    echo "5555555555555555555555555555555555555555555"
+    cmake --system-information
+    popd
+    die
+  '';
+
   NIX_LDFLAGS = lib.optionalString stdenv.isDarwin "-framework CoreFoundation";
 
   # Reduce the risk of collisions

The reverts should probably be after the fix. :)

Thanks for spotting that. Fixed.

jtojnar · 2020-07-16T02:41:11Z

Oh, --system-information seems to work in the build directory after CMake is configured. We could do that but we would need to configure CMake twice.

danieldk · 2020-08-07T15:55:27Z

I saw the error grep: Invalid range end in a CMake-based derivation. It seems to stem from this PR:

$ grep -q '[^a-zA-Z0-9_-+]'
grep: Invalid range end

acowley · 2020-08-07T16:32:22Z

And trying that grep on its own gives me the same error.

jtojnar · 2020-08-07T16:46:53Z

You are right, I must have missed that during testing. - always needs to be last in the square brackets.

jtojnar requested review from FRidh and jonringer as code owners July 15, 2020 07:35

jtojnar changed the base branch from master to staging July 15, 2020 07:36

jtojnar requested review from adisbladis, vcunat and alyssais and removed request for FRidh and jonringer July 15, 2020 07:36

jtojnar referenced this pull request Jul 15, 2020

transmission: Remove $out/share/doc

25bc56f

This is a very common path that often collides with other packages.

jtojnar referenced this pull request Jul 15, 2020

darktable: Remove $out/share/doc

5d7cbc6

This is a very common path that often collides with other packages.

jtojnar mentioned this pull request Jul 15, 2020

Revert "cmake: only set output paths with multiple outputs" #92298

Merged

10 tasks

jtojnar force-pushed the cmake-docdir branch 2 times, most recently from 36ab21e to 4266691 Compare July 15, 2020 07:45

ofborg bot added the 10.rebuild-darwin-stdenv label Jul 15, 2020

ofborg bot requested review from cillianderoiste, flosse, mrVanDalo, astsmtl and wizeman July 15, 2020 08:08

ofborg bot added 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ labels Jul 15, 2020

jtojnar added 3 commits July 16, 2020 04:38

Revert "transmission: Remove $out/share/doc"

2596c96

This reverts commit 25bc56f.

Revert "darktable: Remove $out/share/doc"

812d124

This reverts commit 5d7cbc6.

jtojnar force-pushed the cmake-docdir branch from 4266691 to 812d124 Compare July 16, 2020 02:38

jtojnar merged commit 7dff6d6 into NixOS:staging Jul 18, 2020

jtojnar deleted the cmake-docdir branch July 18, 2020 04:51

This was referenced Jul 19, 2020

Qemu documentation is installed into share/doc/index.html, creating conflicts #90486

Closed

CMAKE_INSTALL_DOCDIR set incorrectly without multiple outputs #82304

Closed

darktable: move documentation to it's own directory #72160

Closed

danieldk mentioned this pull request Aug 7, 2020

rocclr: fix build (no OpenCL library is required) #94871

Merged

10 tasks

alyssais mentioned this pull request Aug 8, 2020

cmake: fix grep in setupHook #94943

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmake.setupHook: define shareDocName #93166

cmake.setupHook: define shareDocName #93166

jtojnar commented Jul 15, 2020 •

edited

jtojnar commented Jul 15, 2020 •

edited

alyssais commented Jul 15, 2020 via email •

edited

alyssais commented Jul 15, 2020

jtojnar commented Jul 16, 2020 •

edited

jtojnar commented Jul 16, 2020

danieldk commented Aug 7, 2020 •

edited

acowley commented Aug 7, 2020

jtojnar commented Aug 7, 2020

cmake.setupHook: define shareDocName #93166

cmake.setupHook: define shareDocName #93166

Conversation

jtojnar commented Jul 15, 2020 • edited

Motivation for this change

jtojnar commented Jul 15, 2020 • edited

alyssais commented Jul 15, 2020 via email • edited

alyssais commented Jul 15, 2020

jtojnar commented Jul 16, 2020 • edited

jtojnar commented Jul 16, 2020

danieldk commented Aug 7, 2020 • edited

acowley commented Aug 7, 2020

jtojnar commented Aug 7, 2020

jtojnar commented Jul 15, 2020 •

edited

jtojnar commented Jul 15, 2020 •

edited

alyssais commented Jul 15, 2020 via email •

edited

jtojnar commented Jul 16, 2020 •

edited

danieldk commented Aug 7, 2020 •

edited