Build failure: neovide (both nixpkgs-23.11 and nixpkgs-unstable) on x86_64-darwin

pitkling commented 8 months ago

Steps To Reproduce

Steps to reproduce the behavior:

build neovide from nixpkgs-23.11 or nixpkgs-unstable

Build log

See the hydra log files for the log file (from nixpkgs-23.11). The error on nixpkgs-unstable seems to be the same. I get the same errors locally (let me know if I should attach the local logs).

Additional context

Note that the build of neovide from nixpkgs-23.11 is also failing for aarch64-darrwin, but with a different error. Moreover, it is fixed in nixpkgs-unstable via #279185 by @n8henrie.

Notify maintainers

@ck3d @MultisampledNight

Metadata

[user@system:~]$ nix-shell -p nix-info --run "nix-info -m"
 - system: `"x86_64-darwin"`
 - host os: `Darwin 22.6.0, macOS 10.16`
 - multi-user?: `yes`
 - sandbox: `no`
 - version: `nix-env (Nix) 2.13.6`
 - channels(peter): `""`
 - channels(root): `""`
 - nixpkgs: `/nix/store/3s69yxbbl116zwga3i6cy7prplywq0bn-source`

Add a :+1: reaction to issues you find important.

MultisampledNight commented 8 months ago

~~Neovide 0.12.2 depends on skia 0.68.0:~~

~~https://github.com/neovide/neovide/blob/4a40294fe8a33a804b52f4843089cbcdfffe4b93/Cargo.toml#L59~~

~~While the derivation pulls in skia 0.67.3:~~

~~https://github.com/NixOS/nixpkgs/blob/2a56c2ca6814a31f8211dad09302398a06acbcdf/pkgs/applications/editors/neovim/neovide/default.nix#L46~~

~~I'll update the derivation sometime today.~~

EDIT: See 3 messages below, that's not it.

MultisampledNight commented 8 months ago

(This is my fault, haven't checked for a possible skia update when the r-ryantm bot updated the derivation)

MultisampledNight commented 8 months ago

Hmm, nix-build -A neovide from nixpkgs master (a3047b383c75) and nix-shell -p neovide locally succeed just fine on NixOS 23.11 for me.

MultisampledNight commented 8 months ago

The skia-safe is one-off due to skia-safe 0.68.0 depending on skia 0.67.3: https://github.com/rust-skia/rust-skia/blob/0.68.0/skia-bindings/Cargo.toml#L33, so that should build just fine. Ignore my first message above, then.

MultisampledNight commented 8 months ago

@pitkling Could you possibly share your local logs, if they are different from the ones in Hydra? In Hydra, only jobs on darwin seem to fail, but I don't have a darwin platform.

n8henrie commented 8 months ago

Thanks for opening.

I was getting an error about std::min on cross-compilation, but didn't know if it was a cross issue or not. Will try to reproduce.

n8henrie commented 8 months ago

https://gist.github.com/n8henrie/c7a8a3f0bb333a944cae49f9ecf45a40

pitkling commented 8 months ago

@pitkling Could you possibly share your local logs, if they are different from the ones in Hydra? In Hydra, only jobs on darwin seem to fail, but I don't have a darwin platform.

@MultisampledNight Sure! See here for the log file of building neovide 0.11.2 from nixpkgs-23.11 (via nix develop nixpkgs#neovide and genericBuild). As far as I can see, at least the first error message (no matching function for call to 'min') is the same as in the log file from @n8henrie for neovide 0.12.2 and as in the hydra logs.

pitkling commented 8 months ago

In case you guys have an idea what the culprit may be but require an actual x86_64-darwin machine to debug this further, just let me know how I can help. At the moment I just don't know which direction to look into.

kirillrdy commented 8 months ago

@pitkling thanks for the offer

not an advertisement, i sometimes use https://github.com/MatthewCroughan/NixThePlanet, which is essentially https://github.com/kholia/OSX-KVM but one command

MultisampledNight commented 8 months ago

@pitkling It'd be great if you could try if you could try building https://github.com/neovide/neovide from source without nix fetching skia (e.g. using my rust shell.nix)! In that scenario, rust-skia should fetch a skia binary instead, it'd be interesting to know if that one works. (I guess it should, since no incorrect C++ stdlib is linked, but who knows.)

MultisampledNight commented 8 months ago

@kirillrdy Thanks for the suggestion, but as the macOS EULA denies installation on any "non-Apple-branded computer" (see https://www.apple.com/legal/sla/docs/macOSSonoma.pdf point 2.J), I don't think I can use that (and I'm not particularly keen on finding out whether or not that holds up in court).

pitkling commented 8 months ago

@pitkling It'd be great if you could try if you could try building https://github.com/neovide/neovide from source without nix fetching skia (e.g. using my rust shell.nix)! In that scenario, rust-skia should fetch a skia binary instead, it'd be interesting to know if that one works. (I guess it should, since no incorrect C++ stdlib is linked, but who knows.)

@MultisampledNight Alright, I tried to compile Neovide 0.12.2 from source using a variant of your shell.nix and the instructions from the Neovide homepage. I first tested it on my M1 Mac (aarch64-darwin), where it builds fine. However, on my x86_64-darwin Mac, there is a (again skia-related) linker error. See here for the log.

There are some linker warnings about artifacts being built for a newer macOS version (10.13) than being linked (10.12). Not sure how severe these are (my system is 13.6.4). The actual errors are about several undefined symbols architecture x86_64, like for example operator new(unsigned long). Might this be related to the error in the nix build?

pitkling commented 7 months ago

The problem can be reproduced on x86_64-darwin using the nix 23.11 version of clang and the following bug.cpp file (basically one of the lines from the skia source causing the build to fail):

#include <algorithm>
int bar;
int foo = std::min(SIZE_MAX / sizeof(bar), (size_t)INT_MAX);

Compiling it with clang++ -c bug.cpp on x86_64-darwin using the nix 23.11 version of clang causes a similar error as in the build logs of neovide:

bug.cpp:3:11: error: no matching function for call to 'min'
int foo = std::min(SIZE_MAX / sizeof(bar), (size_t)INT_MAX);
          ^~~~~~~~
/nix/store/zmdykn37lbbcg08hl57id820fdpaniqf-libcxx-16.0.6-dev/include/c++/v1/__algorithm/min.h:40:1: note: candidate template ignored: deduced conflicting types for parameter '_Tp' ('unsigned long long' vs. 'size_t' (aka 'unsigned long'))
min(const _Tp& __a, const _Tp& __b)
^
/nix/store/zmdykn37lbbcg08hl57id820fdpaniqf-libcxx-16.0.6-dev/include/c++/v1/__algorithm/min.h:51:1: note: candidate template ignored: could not match 'initializer_list<_Tp>' against 'unsigned long long'
min(initializer_list<_Tp> __t, _Compare __comp)
^
/nix/store/zmdykn37lbbcg08hl57id820fdpaniqf-libcxx-16.0.6-dev/include/c++/v1/__algorithm/min.h:60:1: note: candidate function template not viable: requires single argument '__t', but 2 arguments were provided
min(initializer_list<_Tp> __t)
^
/nix/store/zmdykn37lbbcg08hl57id820fdpaniqf-libcxx-16.0.6-dev/include/c++/v1/__algorithm/min.h:31:1: note: candidate function template not viable: requires 3 arguments, but 2 were provided
min(const _Tp& __a, const _Tp& __b, _Compare __comp)
^
1 error generated.

The compilation goes through fine when done on the same machine using Apple's clang or when done on aarch64-darwin using the nix 23.11 version of clang.

The problem seems to be that the nix version of clang++ on x86_64-darwin defines SIZE_MAX as unsigned long long, while sizeof(…) still corresponds to unsigned long. Both the nix version of clang++ on aarch64-darwin and Apple's clang++ have SIZE_MAX (and sizeof(…)) defined as unsigned long. I'm not a C++ expert, but I somehow expected SIZE_MAX is simple the maximal value of the size_t type returned by sizeof(…), so shouldn't they have the same type?

pitkling commented 7 months ago

So it seems indeed that some old OSX headers mistakenly define SIZE_MAX as 'unsigned long long'. This is incompatible with the skia version used by neovide in nix 23.11 and later, in particular this line (and maybe others).

As far as I understand, the issue manifests only on x86_64-darwin since it seemingly still defaults to Apple's 10.12 SDK, while aarch64-darwin uses the 11.0 SDK. I tried to work around this using some input from #239384. In particular:

instead of setting stdenv = clangStdenv; in …/neovide/default.nix I tried to use overrideSDK together with a patch to make-rust-platform.nix (as described in this comment by @reckenrode).
use darwin.apple_sdk_11_0 instead of darwin.apple_sdk in …/neovide/default.nix

While it seemingly gets the builder further, it still fails with similar issues. Probably some other dependency still draws in the old SDK? Or maybe I'm doing something wrong…

reckenrode commented 7 months ago

neovide requires https://github.com/NixOS/nixpkgs/pull/287609 plus a PR I haven’t opened yet to add rustPlatform.overrideSDK (or something similar).

The gist of it is rustPlatform uses the top-level rust package to specify CC_HOST and a bunch of other compiler-related environment variables. Just overriding the SDK in the stdenv is not enough because those variables still point to the wrong clang.

The other aspect is the current overrideSDK has some limitations that prevent it from working correctly for certain applications (e.g., lapce fails to build because overrideSDK doesn’t override the frameworks propagated by wrapGAppsHook). https://github.com/NixOS/nixpkgs/pull/287609 fixes the limitations, but I’m currently working through severe evaluation performance regressions.

NixOS / nixpkgs