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

vscode: fix rpath for native modules #52419

Merged
merged 2 commits into from Feb 10, 2019
Merged

Conversation

eadwu
Copy link
Member

@eadwu eadwu commented Dec 17, 2018

Motivation for this change

Affects at least VSCode Insiders 1544768411 and 1545113902.
Current VSCode stable 1.30.1 doesn't need this.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@eadwu
Copy link
Member Author

eadwu commented Dec 22, 2018

Oh, looks like the Insiders build works now, though the stable build is now broken, ldd seems to be showing everything got patched and code --verbose shows nothing. Gonna look at it again when I have more time.

Actually, I think I just botched both.

@eadwu
Copy link
Member Author

eadwu commented Dec 22, 2018

Seems like the executable works? Though the renderer and extHost don't start up at all, which basically screws everything.

@eadwu
Copy link
Member Author

eadwu commented Dec 22, 2018

Alright it seems like it might be due to an RPATH issue since it seems to use the libgcc_s.so.1 from glibc over gcc. The interpreter looks to be fine. Any idea on how to put gcc on a higher priority than glibc for autoPatchelfHook? Tried adding gcc as a build input and manually patching it afterwards with patchelf --set-rpath "${lib.makeLibraryPath[gcc]}:$(patchelf --print-rpath $out/lib/vscode/${executableName})".

From old to autoPatchelfHook, libgcc_s.so.1 => /nix/store/x6inizi5ahlyhqxxwv1rvn05a25icarq-gcc-7.3.0-lib/lib/libgcc_s.so.1 (0x00007f70c248b000) to libgcc_s.so.1 => /nix/store/xdsjx0gba4id3yyqxv66bxnm2sqixkjj-glibc-2.27/lib/libgcc_s.so.1 (0x00007f5b88498000).

Used gcc-unwrapped but still the same problem, though if I keep the patchelf used originally for the executable everything works fine.

@eadwu eadwu changed the title vscode: fix rpath for native modules [WIP] vscode: fix rpath for native modules Dec 23, 2018
@Mic92
Copy link
Member

Mic92 commented Dec 23, 2018

@aszlig might know

@eadwu
Copy link
Member Author

eadwu commented Dec 23, 2018

Looks like it's a problem with RPATH since patching in systemd and fontconfig into the RPATH allows it to work. autoPatchelfHook doesn't do it automatically since it doesn't even seem to be required though it does exist when I run ldd. libsystemd.so.0 only seems to be needed somewhere after the patching.

original ldd output: https://pastebin.com/raw/Px8PAbsT

after build: https://pastebin.com/raw/ugiykiWU

@aszlig
Copy link
Member

aszlig commented Dec 23, 2018

The interpreter looks to be fine. Any idea on how to put gcc on a higher priority than glibc for autoPatchelfHook?

There is no direct way to assign priorities, but you can give a dependency precedence over others by using addAutoPatchelfSearchPath "$gcclibs" before postFixup.

@eadwu eadwu changed the title [WIP] vscode: fix rpath for native modules vscode: fix rpath for native modules Dec 23, 2018
@eadwu eadwu mentioned this pull request Feb 6, 2019
10 tasks
@worldofpeace
Copy link
Contributor

#55362 needs this.

@Mic92 Any other reservations about this?

I removed the hack for libsecret, introduced in[0] and
I didn't encounter any problems at runtime.

[0]: NixOS#29127
@worldofpeace
Copy link
Contributor

@eadwu I can merge this if you can verify 3513063 doesn't break anything for you.

@eadwu
Copy link
Member Author

eadwu commented Feb 10, 2019

Works fine for me, also a crude test of find -type f -executable -exec ldd "{}" \; shows all dependencies are found.

@worldofpeace worldofpeace merged commit d9b1486 into NixOS:master Feb 10, 2019
@eadwu eadwu deleted the vscode/fix-rpath branch November 17, 2020 23:34
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.

None yet

6 participants