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

ncurses: Create ABI-version'd libtinfo #21368

Merged
merged 2 commits into from Dec 25, 2016
Merged

Conversation

spacekitteh
Copy link
Contributor

@spacekitteh spacekitteh commented Dec 23, 2016

Motivation for this change

Making sure versioned library loads work correctly. Also, the PR works, just travis log length error.

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
    • Linux
  • 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.

@mention-bot
Copy link

@spacekitteh, thanks for your PR! By analyzing the history of the files in this pull request, we identified @wkennington, @edolstra and @vcunat to be potential reviewers.

@spacekitteh
Copy link
Contributor Author

Hmm, buildFHSUserEnv doesn't seem to want to copy the created symlinks to the chroot.

@spacekitteh
Copy link
Contributor Author

(works fine, just travis 4mb log limit)

@abbradar
Copy link
Member

This is ready now (and still needed), correct?

@spacekitteh
Copy link
Contributor Author

Yep!

@abbradar abbradar merged commit 921bdb7 into NixOS:staging Dec 25, 2016
@abbradar
Copy link
Member

Tested on tmux (as the first thing that came to mind); I haven't tested cross-platform builds but hope that you did ~_^.

@abbradar
Copy link
Member

c826ec6 breaks Vim. Can you look into this? Possibly we need to separate libtinfo (a compat derivation with just symlinks) and ncurses after all, but I'm not sure.

@spacekitteh
Copy link
Contributor Author

How does it break it?

@abbradar
Copy link
Member

http://hydra.nixos.org/build/45535843

Reproducible on my machine.

@spacekitteh
Copy link
Contributor Author

Alright, thanks. Will check out. (no pun intended)

@vcunat
Copy link
Member

vcunat commented Dec 28, 2016

The symlink points to bad target, I believe. It's libpanelw.so ATM. I fail to understand the intention of c826ec6 – EDIT: why attempt to link every library to libtinfo?

@spacekitteh
Copy link
Contributor Author

spacekitteh commented Dec 28, 2016 via email

@vcunat
Copy link
Member

vcunat commented Dec 28, 2016

I suspect the order might be non-deterministic, so you might've been lucky.

@vcunat
Copy link
Member

vcunat commented Dec 28, 2016

Hmm, no, bad speculation, it should be always "alphabetical".

@vcunat
Copy link
Member

vcunat commented Dec 28, 2016

@spacekitteh: so, what was wrong about "cross platform"? (i.e. what that commit was meant to fix?) Otherwise I'd just revert that one.

@vcunat
Copy link
Member

vcunat commented Dec 28, 2016

Staged: e4f23bc.

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

5 participants