Skip to content

Verible: compile on Linux and Darwin.#214797

Closed
hzeller wants to merge 1 commit intoNixOS:masterfrom
hzeller:20230205-verible-extend-platforms-all
Closed

Verible: compile on Linux and Darwin.#214797
hzeller wants to merge 1 commit intoNixOS:masterfrom
hzeller:20230205-verible-extend-platforms-all

Conversation

@hzeller
Copy link
Copy Markdown
Contributor

@hzeller hzeller commented Feb 5, 2023

Widening the scope of compiled platforms to include Darwin (was: Linux only).

Description of changes

Widen the systems to compile on by adding necessary derivation hashes.

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:
    • All unit tests pass (which includes invoking the built binaries).
  • 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/)
    • Yes for *-linux platforms.
  • 23.05 Release Notes (or backporting 22.11 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.

@ofborg ofborg bot added 6.topic: darwin Running or building packages on Darwin 8.has: package (new) This PR adds a new package labels Feb 5, 2023
@ofborg ofborg bot requested a review from newAM February 5, 2023 19:01
@ofborg ofborg bot added 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Feb 5, 2023
@hzeller
Copy link
Copy Markdown
Contributor Author

hzeller commented Feb 5, 2023

@ofborg build verible

@hzeller
Copy link
Copy Markdown
Contributor Author

hzeller commented Feb 5, 2023

Attempting to compile on Darwin fails on Arm and x86.

The compiler fails to find the #include <fstream>, which is a standard c++ header.
I suspect maybe the bazel compiler set-up might not be working well on Darwin ?

Adding @aherrmann @ylecornec who are bazel on nix maintainers.

@hzeller
Copy link
Copy Markdown
Contributor Author

hzeller commented Feb 5, 2023

This looks like #150655

@hzeller
Copy link
Copy Markdown
Contributor Author

hzeller commented Feb 5, 2023

CC: @NixOS/bazel

Comment on lines 52 to 55
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note that the hashes are per architecture, not per platform, is that correct? If so can you specify them by arch? That way if it becomes per platform later on it's more obviously a regression.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I am not sure, but you're right, it does look like only the architecture is influencing the hash as the derivation worked up to that point (the compile failed afterwards).

The objective of this change is foremost to get Darwin to compile (which it does not quite yet, which seems to be a bazel issue), then I might simplify that to extract the arch first and make the hashes depend on the arch. One step at a time: Initially I only want to change the minimum to introduce Darwin.

What is the best way to get the architecture ? I am thinking of something like

  arch = lib.elemAt (lib.splitString "-" stdenv.hostPlatform.system) 0;

Copy link
Copy Markdown
Contributor

@uri-canva uri-canva Feb 5, 2023

Choose a reason for hiding this comment

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

Something like:

if stdenv.isx86_64 then ...
else if stdenv.isAarch64 then ...
else throw "Unsupported architecture";

@uri-canva
Copy link
Copy Markdown
Contributor

Did you manage to get the info you needed from #150655 and the PRs that reference it? I think you're right to think that's the issue, let me know if you need pointers on how to work around it.

Widening the scope of compiled platforms to include Darwin
(was: Linux only).

Signed-off-by: Henner Zeller <h.zeller@acm.org>
@hzeller hzeller force-pushed the 20230205-verible-extend-platforms-all branch from 36e1224 to b50da2c Compare February 5, 2023 23:30
@hzeller
Copy link
Copy Markdown
Contributor Author

hzeller commented Feb 5, 2023

I am now trying the workaround from #145149

@hzeller
Copy link
Copy Markdown
Contributor Author

hzeller commented Feb 5, 2023

@ofborg build verible

@hzeller
Copy link
Copy Markdown
Contributor Author

hzeller commented Feb 6, 2023

The workaround to tell the compiler that we compile c++ here seems to get things a bit further but ultimately still fail.

C++ files compile (headers are found this time), but then the resulting binaries fail to execute with the following error:

dyld[72484]: missing symbol called

Probably someone knowing how the compiler-set-up on Darwin is can help here. From what I gather, this is dynamically linked binaries not working.

@uri-canva
Copy link
Copy Markdown
Contributor

I could repro it locally, notice the difference between the linting binary built outside of nixpkgs and inside nixpkgs:

Outside:

> otool -L bazel-bin/verilog/tools/lint/verible-verilog-lint
bazel-bin/verilog/tools/lint/verible-verilog-lint:
	/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 1300.23.0)
	/System/Library/Frameworks/Foundation.framework/Versions/C/Foundation (compatibility version 300.0.0, current version 1858.112.0)
	/usr/lib/libobjc.A.dylib (compatibility version 1.0.0, current version 228.0.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1311.100.3)
	/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 1858.112.0)

Inside:

bazel-out/host/bin/verilog/tools/lint/verible-verilog-lint:
	/nix/store/q4ipnw0nq98dff6xli2wv0w7wag49a8q-libcxx-11.1.0/lib/libc++.1.0.dylib (compatibility version 1.0.0, current version 1.0.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1292.60.1)

It looks like it's missing a couple of libraries that the default toolchain links to automatically, but the nixpkgs one doesn't.

@hzeller
Copy link
Copy Markdown
Contributor Author

hzeller commented Feb 12, 2023

Closing for now, looks like Darwin builds with bazel don't work yet, and I don't have any Darwin machine to test locally.

@thuvasooriya
Copy link
Copy Markdown

i don't have much experience in fixing nix package builds but i have a mac, is there any way i can help to fix this? @hzeller

@hzeller
Copy link
Copy Markdown
Contributor Author

hzeller commented Jul 10, 2024

I don't know - I think a big problem is that clang and bazel on nixos don't work together well, but since I don't have a Mac, I don't fully understand what is broken.

Suspicion is, that #216047 and #150655 might have something to do with it.

I guess a good start for you would be to nix-build -A verible and see what happens.

@simonchatts simonchatts mentioned this pull request Mar 14, 2025
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: darwin Running or building packages on Darwin 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants