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

lumo: init #33691

Closed
wants to merge 2 commits into from
Closed

lumo: init #33691

wants to merge 2 commits into from

Conversation

jgertm
Copy link
Contributor

@jgertm jgertm commented Jan 10, 2018

Motivation for this change

lumo is a cool project that gives you an instant ClojureScript REPL.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-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/)
  • Fits CONTRIBUTING.md.


buildInputs = [ unzip ];

phases = [ "unpackPhase" "installPhase" ];
Copy link
Contributor

Choose a reason for hiding this comment

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

Please do not set phases explicitly, rather disable phases that are causing problems, if any.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

sha256 = "0p06994w48pbgy8xwc1sz3gg609ardsdhmjafdf7qk4gclyiqs5i";
};

buildInputs = [ unzip ];
Copy link
Contributor

Choose a reason for hiding this comment

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

unzip makes more sense as a nativeBuildInputs, as it is presumably used only during the build.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@joachifm
Copy link
Contributor

@GrahamcOfBorg build lumo

Copy link

@GrahamcOfBorg GrahamcOfBorg left a comment

Choose a reason for hiding this comment

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

Success for system: aarch64-linux

no Makefile, doing nothing
installing
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/34vm9hgqs6icsqjh2rfxs2z7fyvrkm0f-lumo-1.8.0-beta
shrinking /nix/store/34vm9hgqs6icsqjh2rfxs2z7fyvrkm0f-lumo-1.8.0-beta/bin/lumo
strip is /nix/store/c6qj0j45xizkrx58i65j75a5ysmqhgrs-binutils-2.28.1/bin/strip
stripping (with command strip and flags -S) in /nix/store/34vm9hgqs6icsqjh2rfxs2z7fyvrkm0f-lumo-1.8.0-beta/bin
patching script interpreter paths in /nix/store/34vm9hgqs6icsqjh2rfxs2z7fyvrkm0f-lumo-1.8.0-beta
checking for references to /build in /nix/store/34vm9hgqs6icsqjh2rfxs2z7fyvrkm0f-lumo-1.8.0-beta...
/nix/store/34vm9hgqs6icsqjh2rfxs2z7fyvrkm0f-lumo-1.8.0-beta

Copy link

@GrahamcOfBorg GrahamcOfBorg left a comment

Choose a reason for hiding this comment

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

Failure for system: x86_64-darwin

  inflating: lumo
patching sources
configuring
no configure script, doing nothing
building
no Makefile, doing nothing
installing
/nix/store/zsn4rc9p48hsq6a53357fmk314b0ahg4-stdenv-darwin/setup: line 1239: patchelf: command not found
builder for '/nix/store/nkjgijz1i5wsvs2gg22gg87ydfa67ndx-lumo-1.8.0-beta.drv' failed with exit code 127
error: build of '/nix/store/nkjgijz1i5wsvs2gg22gg87ydfa67ndx-lumo-1.8.0-beta.drv' failed

Copy link

@GrahamcOfBorg GrahamcOfBorg left a comment

Choose a reason for hiding this comment

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

Success for system: x86_64-linux

no Makefile, doing nothing
installing
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/5x7zw99vyf9fpp89s0nq6cdf7zbpyjkv-lumo-1.8.0-beta
shrinking /nix/store/5x7zw99vyf9fpp89s0nq6cdf7zbpyjkv-lumo-1.8.0-beta/bin/lumo
strip is /nix/store/wxn5gn8amxm1w0ikcx4gbs8a17wvss4j-binutils-2.28.1/bin/strip
stripping (with command strip and flags -S) in /nix/store/5x7zw99vyf9fpp89s0nq6cdf7zbpyjkv-lumo-1.8.0-beta/bin 
patching script interpreter paths in /nix/store/5x7zw99vyf9fpp89s0nq6cdf7zbpyjkv-lumo-1.8.0-beta
checking for references to /tmp/nix-build-lumo-1.8.0-beta.drv-0 in /nix/store/5x7zw99vyf9fpp89s0nq6cdf7zbpyjkv-lumo-1.8.0-beta...
/nix/store/5x7zw99vyf9fpp89s0nq6cdf7zbpyjkv-lumo-1.8.0-beta

@jgertm
Copy link
Contributor Author

jgertm commented Jan 12, 2018

Just discovered this:

> lumo                                                                                                                                                                                                    
lumo: lumo: no version information available (required by lumo)                                                                                                                                                                                               
lumo: lumo: no version information available (required by lumo)
lumo: lumo: no version information available (required by lumo)
lumo: lumo: no version information available (required by lumo)
lumo: lumo: no version information available (required by lumo)
lumo: lumo: no version information available (required by lumo)
lumo: lumo: no version information available (required by lumo)
lumo: lumo: no version information available (required by lumo)
lumo: lumo: no version information available (required by lumo)
lumo: lumo: no version information available (required by lumo)
lumo: lumo: no version information available (required by lumo)
lumo: lumo: no version information available (required by lumo)
lumo: lumo: no version information available (required by lumo)
lumo: lumo: no version information available (required by lumo)
lumo: lumo: no version information available (required by lumo)
lumo: lumo: no version information available (required by lumo)
lumo: lumo: no version information available (required by lumo)
lumo: lumo: no version information available (required by lumo)
lumo: lumo: no version information available (required by lumo)
lumo: lumo: no version information available (required by lumo)
lumo: lumo: no version information available (required by lumo)
lumo: lumo: no version information available (required by lumo)
lumo: lumo: no version information available (required by lumo)
lumo: symbol lookup error: lumo: undefined symbol: 

after removing pkgs.nodejs-8_x from my environment.systemPackages.

Let's hold off on merging this. Any input is thoroughly appreciated though.

@andreivolt
Copy link
Contributor

@jgertm I'm getting the same error and I do have pkgs.nodejs-8_x in my system packages

@andreivolt andreivolt mentioned this pull request Apr 8, 2018
8 tasks
@hlolli
Copy link
Member

hlolli commented Apr 15, 2018

@jgertm and @andreivolt like I mentioned in andreivolt's PR, that lumo 1.8.0 was built and released with node 9.2.0, the 1.8.0-beta was probably in 8_x. But I don't think it should matter.

After trying to build lumo with boot, I can't see a way to prevent those http calls that boot wants to make before starting the build, not to mention the yarn calls and nexe's call to V8 snapshots. So it would probably mean a long list of sha256/url tuplest to download all the classpath dependencies for boot and lumo's dependencies. To me the patchelf sounds good, in that if it works it would be the cleanest way to solve this.

Let me see if it makes sense to use npm2nix and use the entrypoint file lumo.js, it could solve the problems for some distros, as well as seeing if you're useing patchelf correctly aiming at the right libc/libc ++ for the build-id in the nix store. If success, I'll make a PR to your branch.

added: it makes no sense to use the npm2nix as we wouldn't have control which zip file it decides to download :) but at least what won't work is to allow this on all platforms, we could support x86_64-linux and x86_64-darwin, but then we'd have to target the corresponding binaries. Nexe has arm and 32 bit comilations options, but lumo isn't being released for them, so we probably shouldn't support them on nixpkgs.

@hlolli
Copy link
Member

hlolli commented Apr 15, 2018

Thinking out loud: I explicily replaced the path to libstdc++.so.6 (which ldd did not found) and changed the intepreted like was in the PR. It's becoming clear that we are dealing with stripped binary, there's to some, some part is ELF and other part is dynamic, meaning the binary looses its runtime dependencies declerations, so we just have to provide them, guessing we need to provide the same libs as nodejs needs. But that's not my problem at hand tough it will be. The segfault I'm currently receiving idicates that part of the binary was compiled useing really old glibc

readelf -sW /nix/store/b891mhri338df0dbpnfxs09jhcq5ir1p-lumo-1.8.0/bin/lumo | grep strchr
   146: 0000000000000000     0 FUNC    GLOBAL DEFAULT  UND strchr@GLIBC_2.2.5 (4)
 23803: 0000000001468c20   268 FUNC    GLOBAL DEFAULT   13 u_strchr_60
 28829: 00000000014692b0    94 FUNC    GLOBAL DEFAULT   13 u_strchr32_60
 21409: 0000000001468c20   268 FUNC    GLOBAL DEFAULT   13 u_strchr_60
 30744: 0000000000000000     0 FUNC    GLOBAL DEFAULT  UND strchr@@GLIBC_2.2.5
 39405: 00000000014692b0    94 FUNC    GLOBAL DEFAULT   13 u_strchr32_60

This undefined strchr function seems to be problematic, similar problem has been reported
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=43556
where difference in gcc versions can matter for segfaults.

investigateing further...

@hlolli hlolli mentioned this pull request Jul 25, 2018
9 tasks
@lukateras
Copy link
Member

Closing in favor of #44076.

@lukateras lukateras closed this Oct 13, 2018
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