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

Misc fixups that may be of interest (don't merge as-is) #28

Merged
merged 6 commits into from
Mar 4, 2018

Conversation

dtzWill
Copy link
Contributor

@dtzWill dtzWill commented Feb 26, 2018

Builds on top of #26

  • Fixes Executing an AppImage bundled with nix2appimage.sh failing with Segmentation fault. #24 (using mostly the suggested approach)
  • Fixes to work with latest nixpkgs which does not have musl-gcc, instead just use musl-native stdenv
  • minor cleanup to use dynamicLinker string from bintools, but might not want this since requires however-recent nixpkgs. Shrug.
  • Updated appimagetool to 10 and reverted, didn't explore what broke 😇
  • Add a really quick nix file for patchelf'ing appimage's generated so they execute.

With these, was able to do:

$ NIX_PATH=nixpkgs=channel:nixos-unstable ./nix2appimage.sh vlc
$ NIX_PATH=nixpkgs=channel:nixos-unstable nix build -f test-appimage.nix --arg appimagefile ./VLC*AppImage
$ ./result
<VLC runs>

Don't have immediate access to a non-NixOS machine with a display server (and new-enough kernel/permissions to support appimage-ness), so haven't tested elsewhere.

LMK if any of this looks useful, or just cherry-pick as you see fit, just thought I'd share in case it helps.

@dtzWill
Copy link
Contributor Author

dtzWill commented Feb 26, 2018

  • Update to appimagetool 10, seems to work?

@matthewbauer
Copy link
Member

Does this still have the issue with release.nix in #26 (comment) ?

@dtzWill
Copy link
Contributor Author

dtzWill commented Feb 26, 2018

Not anymore, although TBH I'm not sure this is best-- not sure I follow what the expected workflow is here? So might be missing a dep on the source directory or something.

@matthewbauer
Copy link
Member

matthewbauer commented Mar 3, 2018

These actually look mostly good to me. Ideally we could move the AppImage derivations into Nixpkgs so others could use it. Besides that though it's all helpful.

If you can rebase and squash, I can definitely merge this

Makefile Outdated
install: nix-bundle.sh nix-run.sh appdir.nix appimagetool.nix appimage.nix AppRun.c appimage-top.nix default.nix appdir.sh nix2appimage.sh
.PHONY: install

install:: nix-bundle.sh nix-run.sh appdir.nix appimagetool.nix appimage.nix AppRun.c appimage-top.nix default.nix appdir.sh nix2appimage.sh
Copy link
Member

Choose a reason for hiding this comment

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

I'm confused what's going on here- but I think I've resolved this issue in dcbfa1a.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Different approaches, I wasn't sure of how this was intended to be used so tried to be more careful about modifying things :).

I'll rebase+squash as requested, and drop this.

@dtzWill
Copy link
Contributor Author

dtzWill commented Mar 3, 2018

Note that hardcoded config strings "x86_64-unknown-linux-musl" means this won't work on non-x86_64 machines, not sure if nix-bundle (or even AppImage) are expected to work on other platforms anyway but just thought I'd draw your attention to it.

Cleaned history a bit, but didn't fully squash if that's what you meant. Suit yourself if you'd like to do so as you merge (github has option for that I think?).

@matthewbauer matthewbauer merged commit 393f415 into nix-community:master Mar 4, 2018
@matthewbauer
Copy link
Member

Looks good! I just meant to get rid of the "revert" commit everything else is fine.

I think it's okay to include the hardcoded localSystem.config but I think @Ericson2314 has a way to generate these in lib/systems/parse.nix. In the future, we might want to support multi-arch bundles (this should be possible in arx).

@probonopd
Copy link

What does the unknown-linux-musl part mean? Will the AppImage only run on systems that have musl installed, and if so, why? Couldn't musl be bundled inside the AppImage?

@matthewbauer
Copy link
Member

Will the AppImage only run on systems that have musl installed, and if so, why?

Musl should statically compile everything for us. You should only need musl at build time (which Nixpkgs will pull in for you).

@matthewbauer
Copy link
Member

matthewbauer commented Mar 4, 2018

Also @dtzWill I am having issues getting Musl to bootstrap correctly:

$ NIX_PATH=nixpkgs=channel:nixos-unstable ./nix2appimage.sh vlc
...
building '/nix/store/nxp9a6bs669n0mxx336hc7j1wlmphl1a-bootstrap-tools.drv'...
Unpacking the bootstrap tools...
Patching the bootstrap tools...
/nix/store/bcklmclsf0if47a2n4na73f3rrqv4s7l-unpack-bootstrap-tools.sh: line 19: /nix/store/v0vw2xad4vxgp7kdfzczsy0xk15amliq-bootstrap-tools/lib/ld-*so.?: not found
builder for '/nix/store/nxp9a6bs669n0mxx336hc7j1wlmphl1a-bootstrap-tools.drv' failed with exit code 127

I think it's related to a bad wildcard pattern (/nix/store/v0vw2xad4vxgp7kdfzczsy0xk15amliq-bootstrap-tools/lib/ld-musl-x86_64.so.1 exists). I can file a bug report if necessary.

@dtzWill
Copy link
Contributor Author

dtzWill commented Mar 4, 2018

That's strange. Unfortunately I'm AFK for the day, file an issue if this is urgent or if you don't hear back from me within ~12 hours. Pattern looks right to me but... well we'll see.

@dtzWill
Copy link
Contributor Author

dtzWill commented Mar 5, 2018

Unfortunately my initial attempts are reproducing, namely using a redirected store, fail early in the the first build.... but for both musl and glibc :(. And not during unpacking/patching of bootstrap tools, that succeeds, but the first build using them.

What version of Nix are you using?

I'll try in a docker container or something fresh in a bit.... sorry not sure why that'd be happening :(.

@matthewbauer
Copy link
Member

What version of Nix are you using?

It's Nix 2.0 but in a weird environment. No root access so running through a Proot.

Shouldn't Muslpkgs be in the binary cache, though? Tried the same thing on my VM and got memory issues bootstrapping on gcc.

@dtzWill
Copy link
Contributor Author

dtzWill commented Mar 5, 2018

Not yet in official binary caches, although they are for cross-compiling to musl :). There's some cleanup to be done to make this possible with release.nix I think, although I haven't pursued since the RFC hasn't actually been accepted yet[1].

I am somewhat surprised by this behavior (similarly that gcc bootstrap doesn't work?) since musl packages are available in my binary cache which you can use with something like:

$ nix run -store $HOME/allvm-store --option binary-caches "https://cache.nixos.org https://cache.allvm.org" --option trusted-binary-caches "gravity.cs.illinois.edu-1:yymmNS/WMf0iTj2NnD0nrVV8cBOXM9ivAkEdO1Lro3U= cache.nixos.org-1:6NCHdD59X431o0gWypbMrAURkbJ16ZPMQFGspcDShjY="

I'm investigating this currently.

Strangely bootstrap seems to work better on aarch64 ^_^.

[1] NixOS/rfcs#23

@dtzWill
Copy link
Contributor Author

dtzWill commented Mar 6, 2018

@matthewbauer won't be available in anyone's cache but can you check if using https://github.com/dtzWill/nixpkgs/tree/musl-native-bootstrap fixes the problem you're seeing when unpacking the tools?

Something like:

$  NIX_PATH=nixpkgs=https://github.com/dtzWill/nixpkgs/archive/musl-native-bootstrap.tar.gz nix-build '<nixpkgs>' --arg localSystem '{config="x86_64-unknown-linux-musl";}' -A bash

(or same NIX_PATH I give above but with the nix2appimage.sh command you were trying earlier)

Using this fixed a similar--different but hopefully not too much so-- problem that caused problems bootstrapping musl from within a Docker container, so I'm hoping it helps in your proot case too.

@matthewbauer
Copy link
Member

Still no luck- getting the same issue.

I'm starting to think it's a local system issue. This is a university machine so the Nix store has to be put on an NFS drive and it sometimes breaks with big files. There could also be some issue with it being an older kernel:

Linux e1005-1690G3GB2 4.4.0-89-generic #112-Ubuntu SMP Mon Jul 31 19:38:41 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux

I'll start up a VirtualBox machine to see if I can get things to work.

@dtzWill
Copy link
Contributor Author

dtzWill commented Mar 6, 2018

As soon as the RFC is approved I'll get working on having Hydra at least cache the stdenv, sorry about this :).

If you're on NixOS, the instructions under "Quickstart with Binary Cache" on the musl PR will configure your system to use the ALLVM binary cache as well, which will save you a bunch of building.

NixOS/nixpkgs#34645

I use it all the time, all builds on sandboxed nodes running NixOS... but make your own trust decisions 👍.

@matthewbauer
Copy link
Member

Ok looks good! Finally got it to work.

It's too bad we can't get it to run natively on NixOS but I suppose those are already available to us.

Closure size of VLC is surprisingly bad:

12M     cb3slv3szhp46xkrczqw7mscy5mnk64l-coreutils-8.29
13M     4csy6xvbrqxkp3mk6ngxp199xkr476lj-glib-2.54.3
17M     97644adnv5jgr4sfc615nkpi82m29vzx-ffmpeg-2.8.13
23M     2kcrj1ksd2a14bm5sky182fv2xwfhfap-glibc-2.26-131
29M     jbvydjnafr5fbd6m0j0im4jf8idw6v7b-gtk+-2.24.32
50M     7b2z0vfbs9539ga4pxx5gmli47rz5y3n-python-2.7.14
50M     s029gmjpgvm776wfb56naqny9qja9339-perl-5.24.3
57M     hny7nanlkh4nbf19j7dsca7xcq8f7p55-vlc-2.2.8
57M     q3ml927fd333h0abna9h8gq8z2icnpgk-samba-4.7.4
124M    isg6mkqw13n6hn55zs98v18adj6q86c0-qt-4.8.7
192M    7m31df5kk2s0s6b4j0whky06g03ayjns-systemd-237

I wonder if we can get shrink systemd or samba with multiple outputs?

@dtzWill
Copy link
Contributor Author

dtzWill commented Mar 7, 2018

systemd seems pretty unhelpful for vlc offhand, but doubly so when bundled up?

(note that "...systemd-237-lib" would be for libsystemd which has possibly utility, but it seems rather unlikely VLC will require systemd binaries ;))

Samba seems a bit rough too, would expect to only need lib outputs there too.

Good news is closure size impacts everyone so this is something that can be taken to nixpkgs "upstream" and be useful to everyone to improve :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants