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

llvmPackages_11: 11.0.0rc5 -> 11.0.0, enable on Darwin #99984

Merged
merged 2 commits into from
Oct 16, 2020

Conversation

ggreif
Copy link
Contributor

@ggreif ggreif commented Oct 7, 2020

Motivation for this change
Things done
  • Added conditionals
  • python3 now mandatory for building libcxx everywhere
  • Bumped to v11.0.0 final
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Oct 7, 2020
@ggreif
Copy link
Contributor Author

ggreif commented Oct 7, 2020

@GrahamcOfBorg build llvm_11 lld_11 clang_11

@ofborg ofborg bot requested review from dtzWill and 7c6f434c October 7, 2020 22:00
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-linux: 0 This PR does not cause any packages to rebuild labels Oct 7, 2020
@ggreif ggreif changed the title llvm_11: don't pass a build-id to linker on Darwin llvmPackages_11: enable on Darwin Oct 8, 2020
@ofborg ofborg bot added 10.rebuild-linux: 11-100 and removed 10.rebuild-linux: 0 This PR does not cause any packages to rebuild labels Oct 8, 2020
@ggreif
Copy link
Contributor Author

ggreif commented Oct 8, 2020

@DieGoldeneEnte this builds now for Darwin, but stopped working for Linux. I have no clue why.

@ggreif ggreif changed the title llvmPackages_11: enable on Darwin llvmPackages_11: 11.0.0rc5 -> 11.0.0rc6, enable on Darwin Oct 8, 2020
@ggreif ggreif mentioned this pull request Oct 9, 2020
10 tasks
@DieGoldeneEnte
Copy link
Contributor

libc++ failed,because it coudn't find python, it build normally once python3 is added as native build input.
Could this be, because the llvm-source is added as input?

I need to take another look, why it suddenly needs python...

@ggreif ggreif changed the title llvmPackages_11: 11.0.0rc5 -> 11.0.0rc6, enable on Darwin llvmPackages_11: 11.0.0rc5 -> 11.0.0 enable on Darwin Oct 12, 2020
@ggreif ggreif changed the title llvmPackages_11: 11.0.0rc5 -> 11.0.0 enable on Darwin llvmPackages_11: 11.0.0rc5 -> 11.0.0, enable on Darwin Oct 12, 2020
@ggreif ggreif marked this pull request as ready for review October 12, 2020 12:42
@ggreif
Copy link
Contributor Author

ggreif commented Oct 12, 2020

Could this be, because the llvm-source is added as input?

Then it would fail on Darwin too, right?

@DieGoldeneEnte
Copy link
Contributor

Then it would fail on Darwin too, right?

That's my problem. I couldn't find a relevant difference between the working package in nixpkgs and this.
If I read it correctly, python is required when building libc++ standalone (I don't think we are setting LIBCXX_STANDALONE_BUILD, but python should only be needed if it is set) and using cmake > 3.12. Both should be given for the current nixpkgs derivation.

@ggreif
Copy link
Contributor Author

ggreif commented Oct 12, 2020

@DieGoldeneEnte on Darwin I get cmake-3.18.2 with nix-build -A cmake. Same on Linux?

@ggreif
Copy link
Contributor Author

ggreif commented Oct 12, 2020

@GrahamcOfBorg build lldb_11

@ggreif
Copy link
Contributor Author

ggreif commented Oct 12, 2020

@GrahamcOfBorg build llvmPackages_11

@ggreif
Copy link
Contributor Author

ggreif commented Oct 12, 2020

@DieGoldeneEnte Maybe the cmake machinery now runs some tests that require python3. I included Linux now, let's see what OfBorg says.

@ggreif ggreif marked this pull request as draft October 13, 2020 20:51
@DieGoldeneEnte
Copy link
Contributor

DieGoldeneEnte commented Oct 14, 2020

I just tried rebuilding the current master version of libcxx, but it is actually broken, because of the missing monorepo-layout :(
Is this the same problem as on darwin (before the changes here)?

Regarding LIBCXX_STANDALONE_BUILD, this is set by cmake (in libcxx/cmake/Modules/HandleOutOfTreeLLVM.cmake). So I don't think it should be set.
Reading the CMakeLists.txt file, it seems python is mandatory when building standalone (and cmake >3.12). I don't understand, why darwin doesn't need it, since the cmake code is (shortened):

if (LIBCXX_STANDALONE_BUILD)
  if(CMAKE_VERSION VERSION_LESS 3.12)
    [...]
  else()
    find_package(Python3 COMPONENTS Interpreter)
    if(NOT Python3_Interpreter_FOUND)
      find_package(Python2 COMPONENTS Interpreter REQUIRED)
      [...]
    endif()
  endif()
endif()

Are you building libc++ in a sandbox? Maybe cmake finds a python version from the system?

error:

libc++abi now requires being built in a monorepo layout with libcxx available

@ggreif
Copy link
Contributor Author

ggreif commented Oct 14, 2020

@DieGoldeneEnte I am debugging this right now. I'll trigger you after having done the fix.

@ggreif
Copy link
Contributor Author

ggreif commented Oct 14, 2020 via email

@DieGoldeneEnte
Copy link
Contributor

shouldn't the test (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR) be true, because we run cmake in the libc++ directory. This would be false if the CMakeLists.txt file would be added via add_subdirectory/add_llvm_external_project (that's at least my understanding given my limited cmake knowledge).

@ggreif
Copy link
Contributor Author

ggreif commented Oct 14, 2020

@DieGoldeneEnte yes that test should be true. It is true. So this is a standalone build in a monorepo-layout source.
Darwin looks for a Python3 and doesn't find it. But on my machine it finds some Python2 in a framework, thus it can proceed. It would break in a sandbox.

I am making python3 mandatory now.

@ggreif ggreif marked this pull request as ready for review October 14, 2020 18:22
@ggreif
Copy link
Contributor Author

ggreif commented Oct 14, 2020

@DieGoldeneEnte things look good on my side!

@DieGoldeneEnte
Copy link
Contributor

@ggreif Why is the llvm source needed for libc++*? Is it to simulate the monorepo?
Builds without problems on my machine :)

@ggreif
Copy link
Contributor Author

ggreif commented Oct 14, 2020

@ggreif Why is the llvm source needed for libc++*? Is it to simulate the monorepo?

That's what I understand, too.

Builds without problems on my machine :)

THX! 🥇

@DieGoldeneEnte
Copy link
Contributor

LGTM then!

I think we will need to rewrite the llvm package since building the projects standalone is less and less supported.

@DieGoldeneEnte
Copy link
Contributor

DieGoldeneEnte commented Oct 15, 2020

Problem is with compilert-rt:

CMake Error at cmake/Modules/AddCompilerRT.cmake:318 (add_library):
  No SOURCES given to target: clang_rt.builtins-i686
Call Stack (most recent call first):
  lib/builtins/CMakeLists.txt:636 (add_compiler_rt_runtime)

fixed it (at least for i686) in my PR targeting this PR-branch.

@ggreif
Copy link
Contributor Author

ggreif commented Oct 15, 2020

@volth @DieGoldeneEnte but this is not a regression vs. 11.0.0rcs, right? Is it a regression vs. 10.0.1?

Anyway, better open a PR against those separately.

@DieGoldeneEnte
Copy link
Contributor

@ggreif It was introduced with 11.0.0-rc1, 10.0.1 does not have this change.

@ggreif
Copy link
Contributor Author

ggreif commented Oct 15, 2020 via email

primeos pushed a commit that referenced this pull request Oct 16, 2020
compiler-rt (and as a result clang) can't be build for i686 (as noticed here: #99984).
The patch adds the required variables and should result in the same behavior as in the nixpkgs-llvm10. It essentially forces to use i386 buildins when using i486, i586 or i686, which are not supported.

Fixes #100392
Copy link
Member

@primeos primeos left a comment

Choose a reason for hiding this comment

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

The diff LGTM :) Thanks @ggreif and @DieGoldeneEnte!

@ggreif is this ready to be merged now?
Edit: I just re-read any comments and everything seems to be resolved :) I'll go ahead and merge this then. Huge thanks again!

@primeos primeos merged commit 86a4a50 into NixOS:master Oct 16, 2020
@ggreif ggreif deleted the darwin-llvm11 branch October 16, 2020 13:36
primeos pushed a commit to primeos/nixpkgs that referenced this pull request Nov 4, 2020
compiler-rt (and as a result clang) can't be build for i686 (as noticed here: NixOS#99984).
The patch adds the required variables and should result in the same behavior as in the nixpkgs-llvm10. It essentially forces to use i386 buildins when using i486, i586 or i686, which are not supported.

Fixes NixOS#100392

(cherry picked from commit 6948875)
rvem pushed a commit to serokell/nixpkgs that referenced this pull request Nov 8, 2022
compiler-rt (and as a result clang) can't be build for i686 (as noticed here: NixOS#99984).
The patch adds the required variables and should result in the same behavior as in the nixpkgs-llvm10. It essentially forces to use i386 buildins when using i486, i586 or i686, which are not supported.

Fixes NixOS#100392

(cherry picked from commit 6948875)
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