Skip to content

llvmPackages.libcxx: use hasSharedLibraries instead of isStatic#362848

Merged
alyssais merged 1 commit intoNixOS:masterfrom
kuruczgy:pr/llvmPackages-libcxx-use-hasSharedLibraries-instead-of-isStatic
Dec 22, 2024
Merged

llvmPackages.libcxx: use hasSharedLibraries instead of isStatic#362848
alyssais merged 1 commit intoNixOS:masterfrom
kuruczgy:pr/llvmPackages-libcxx-use-hasSharedLibraries-instead-of-isStatic

Conversation

@kuruczgy
Copy link
Contributor

@kuruczgy kuruczgy commented Dec 7, 2024

Split from #352629, as this should be a relatively uncontroversial bugfix that also makes sense on its own.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
    • riscv32-none-elf
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.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
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added the 6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related label Dec 7, 2024
@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Dec 7, 2024
@alyssais
Copy link
Member

alyssais commented Dec 7, 2024

Huh — what's the difference between isStatic and hasSharedLibraries? When would one be true but not the other?

@kuruczgy
Copy link
Contributor Author

kuruczgy commented Dec 7, 2024

@alyssais see this comment:

# The difference between `isStatic` and `hasSharedLibraries` is mainly the
# addition of the `staticMarker` (see make-derivation.nix). Some
# platforms, like embedded machines without a libc (e.g. arm-none-eabi)
# don't support dynamic linking, but don't get the `staticMarker`.
# `pkgsStatic` sets `isStatic=true`, so `pkgsStatic.hostPlatform` always
# has the `staticMarker`.
isStatic = final.isWasi || final.isRedox;

@alyssais
Copy link
Member

Ah, I see! So really we should basically always be checking hasSharedLibraries instead of isStatic?

@kuruczgy
Copy link
Contributor Author

Ah, I see! So really we should basically always be checking hasSharedLibraries instead of isStatic?

Not sure in what other cases it needs to be checked, but yes, I believe hasSharedLibraries should often be more correct.

@alyssais
Copy link
Member

Lots of packages check it to e.g. tell CMake not to try to build shared libraries.

@alyssais
Copy link
Member

I think a rebase would fix OfBorg.

@kuruczgy kuruczgy force-pushed the pr/llvmPackages-libcxx-use-hasSharedLibraries-instead-of-isStatic branch from 5a52d92 to cefa795 Compare December 18, 2024 22:50
@github-actions github-actions bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Dec 18, 2024
@alyssais
Copy link
Member

This can go to master — it doesn't really rebuild anything.

@kuruczgy kuruczgy force-pushed the pr/llvmPackages-libcxx-use-hasSharedLibraries-instead-of-isStatic branch from cefa795 to d2b4629 Compare December 19, 2024 09:30
@kuruczgy kuruczgy changed the base branch from staging to master December 19, 2024 09:30
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation This PR adds or changes documentation 8.has: changelog This PR adds or changes release notes 8.has: module (update) This PR changes an existing module in `nixos/` and removed 6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related labels Dec 19, 2024
@github-actions github-actions bot added 6.topic: policy discussion Discuss policies to work in and around Nixpkgs 6.topic: golang Go is a high-level general purpose programming language that is statically typed and compiled. 6.topic: vscode A free and versatile code editor that supports almost every major programming language. 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` 6.topic: dotnet Language: .NET 6.topic: continuous integration Affects continuous integration (CI) in Nixpkgs, including Ofborg and GitHub Actions 6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. and removed 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation This PR adds or changes documentation 8.has: changelog This PR adds or changes release notes 8.has: module (update) This PR changes an existing module in `nixos/` 6.topic: policy discussion Discuss policies to work in and around Nixpkgs 6.topic: golang Go is a high-level general purpose programming language that is statically typed and compiled. 6.topic: vscode A free and versatile code editor that supports almost every major programming language. 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` 6.topic: dotnet Language: .NET 6.topic: continuous integration Affects continuous integration (CI) in Nixpkgs, including Ofborg and GitHub Actions 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Dec 19, 2024
Copy link
Member

@alyssais alyssais left a comment

Choose a reason for hiding this comment

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

Would welcome PRs to update more isStatic checks to hasSharedLibraries. :)

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one person. label Dec 19, 2024
@alyssais alyssais merged commit 304d149 into NixOS:master Dec 22, 2024
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants