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

cmake: check if NIX_CC exists before using it #192943

Merged
merged 3 commits into from
Nov 3, 2022

Conversation

KiruyaMomochi
Copy link
Contributor

Description of changes

Due to the patch added by us, cmake package does not work when NIX_CC environment variable does not exists.
When $ENV{NIX_CC} is empty, the condition IS_DIRECTORY AND EXISTS /nix-support/orig-libc AND EXISTS /nix-support/orig-libc-dev is invalid.

# List common include file locations not under the common prefixes.
+if(IS_DIRECTORY $ENV{NIX_CC}
+ AND EXISTS $ENV{NIX_CC}/nix-support/orig-libc
+ AND EXISTS $ENV{NIX_CC}/nix-support/orig-libc-dev)
+ file(STRINGS "$ENV{NIX_CC}/nix-support/orig-libc" _nix_cmake_libc)
+ file(STRINGS "$ENV{NIX_CC}/nix-support/orig-libc-dev" _nix_cmake_libc_dev)
+else()
+ set(_nix_cmake_libc @libc_lib@)
+ set(_nix_cmake_libc_dev @libc_dev@)
+endif()
+

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@wentasah
Copy link
Contributor

It seems this could fix the problem I've just encountered. When I call cmake manually, it fails with the following error:

CMake Error at /nix/store/366p8r6ja02pv27xrgpafha353rzvglv-cmake-cursesUI-qt5UI-3.24.1/share/cmake-3.24/Modules/Platform/UnixPaths.cmake:51 (if):
  if given arguments:

    "IS_DIRECTORY" "AND" "EXISTS" "/nix-support/orig-libc" "AND" "EXISTS" "/nix-support/orig-libc-dev"

  Unknown arguments specified
Call Stack (most recent call first):
  /nix/store/366p8r6ja02pv27xrgpafha353rzvglv-cmake-cursesUI-qt5UI-3.24.1/share/cmake-3.24/Modules/Platform/Linux.cmake:85 (include)
  /nix/store/366p8r6ja02pv27xrgpafha353rzvglv-cmake-cursesUI-qt5UI-3.24.1/share/cmake-3.24/Modules/CMakeSystemSpecificInformation.cmake:37 (include)

Things work when I set NIX_CC manually.

I'm not testing this PR due to high number of rebuilds :-(.

@wentasah
Copy link
Contributor

I managed to test this PR. I built just cmake and verified that manual cmake invocation does not fail with my projects.

SharzyL added a commit to chipsalliance/t1 that referenced this pull request Oct 22, 2022
sequencer pushed a commit to chipsalliance/t1 that referenced this pull request Oct 25, 2022
sequencer pushed a commit to chipsalliance/t1 that referenced this pull request Oct 26, 2022
sequencer pushed a commit to chipsalliance/t1 that referenced this pull request Oct 27, 2022
sequencer pushed a commit to chipsalliance/t1 that referenced this pull request Oct 28, 2022
SharzyL added a commit to chipsalliance/t1 that referenced this pull request Nov 1, 2022
@lovesegfault lovesegfault requested review from trofi and stephank and removed request for trofi November 1, 2022 18:02
Copy link
Contributor

@trofi trofi left a comment

Choose a reason for hiding this comment

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

The fix looks good. Thank you!

sequencer pushed a commit to chipsalliance/t1 that referenced this pull request Nov 2, 2022
@LucioFranco
Copy link

Thanks for the patch and the reviews! I can confirm this fixes my issue on my M1 Mac! Thanks!

Would it make sense to backport this to 22.05?

@stephank
Copy link
Contributor

stephank commented Nov 2, 2022

The run-time libc detection was added after 22.05 in #181431. Here's the simpler patch currently in 22.05, which doesn't have the new checks: https://github.com/NixOS/nixpkgs/blob/nixos-22.05/pkgs/development/tools/build-managers/cmake/search-path.patch

I don't think #181431 should be backported, so this shouldn't be either.

Though, should we be targeting staging for this, considering the Darwin rebuilds?

@lovesegfault
Copy link
Member

Yup, this needs to target staging

@KiruyaMomochi KiruyaMomochi changed the base branch from master to staging November 2, 2022 16:46
qinjun-li pushed a commit to chipsalliance/t1 that referenced this pull request Nov 3, 2022
@lovesegfault lovesegfault merged commit 1c0ceea into NixOS:staging Nov 3, 2022
sequencer pushed a commit to chipsalliance/t1 that referenced this pull request Nov 3, 2022
workaround for cmake bug in nix
details: NixOS/nixpkgs#192943
@Janik-Haag Janik-Haag added the 12. first-time contribution This PR is the author's first one; please be gentle! label Jun 12, 2023
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.

7 participants