Skip to content

ncurses: add --enable-symlinks in configure flag#1633

Closed
wavewave wants to merge 2 commits intoNixOS:masterfrom
wavewave:ncurses
Closed

ncurses: add --enable-symlinks in configure flag#1633
wavewave wants to merge 2 commits intoNixOS:masterfrom
wavewave:ncurses

Conversation

@wavewave
Copy link
Contributor

If nix store is on AFS file system, ncurses build fails since terminfo generation tries to hardlink beyond file system boundary. By adding --enable-symlinks in configure flag, the build is successful.

Reference discussion:
http://lists.bestpractical.com/pipermail/shipwright/2010-June/000029.html

@peti
Copy link
Member

peti commented Jan 30, 2014

@edolstra, everyone agrees that the stdenv-updates branch experiment is something we don't want to repeat ... but how do we deal with this kind of change? A topic branch for this single patch is probably overkill, right? Yet, committing it to master without some prior testing is probably not a good idea either.

@wavewave
Copy link
Contributor Author

@peti : Sorry that if I didn't follow some rules in nixpkgs development. Github is rather too easy to make a pull request (just one button away from my forked repo) in web interface, so I was a little hasty in sharing what I found. I had better make an issue for this or email nixos dev mailing list before making a pull request. Would you mind suggesting me some guideline? If some general guide line of nixpkgs development exists as some blog article or something like that, it would be helpful.

@vcunat
Copy link
Member

vcunat commented Jan 30, 2014

@wavewave: I don't know about anything better you could have done. It's only the fact that we haven't solved yet how to test changes that rebuild lots of things.

Besides, I strongly believe stdenv should not depend on such things as ncurses. If I read the build-time graph correctly, the dependency is only through texinfo. For generating docs we don't need ncurses at all, and that's the main use in nixpkgs AFAIK. Maybe we should split into ncurses and ncursesInteractive, like it's for bash (for example).

@peti
Copy link
Member

peti commented Jan 30, 2014

@wavewave, yes, Vladimír is right. The problem is that we as a community haven't figured out how to deal best with changes that modify stdenv (and thus re-build virtually every single package). Submitting a pull request for this change was the right thing to do, though.

@wavewave
Copy link
Contributor Author

@vcunat @peti : Thanks for your messages. I understand maintenance difficulties which involves this massive number of commits, so I was wondering if I miss some obvious guideline. If not yet, it is good to make some note visibly on developing stdenv-updates.

As for this hard-link problem, I have encountered the same kind of problem in different packages (e.g. e2fsprogs). In my case, I need to locate whole nix/store on AFS (that's only allowed for my cluster setup), but fixing all cross-device hard-links may incur too invasive changes to usual users. If we have some way to make an option for it in the long run, that will be great

@edolstra
Copy link
Member

edolstra commented Feb 1, 2014

I've added a jobset for this: http://hydra.nixos.org/jobset/nixpkgs/pr-1633

In any case, this patch seems a good idea because Nix in general doesn't preserve hard links (e.g. when installing via the binary cache). So symlinks are better. However, I imagine that there might be quite a few packages that fail to build on AFS, if it really doesn't allow cross-directory hard links.

@wavewave
Copy link
Contributor Author

I also added commit a90105d for e2fsprogs to my ncurses branch. e2fsprogs also had the same problem with afs due to hard links.

@vcunat
Copy link
Member

vcunat commented Feb 27, 2014

There's a strange error when building nix on Hydra http://hydra.nixos.org/build/8672834/nixlog/6/tail-reload, but I'm unable to reproduce it in a newer branch https://github.com/vcunat/nixpkgs/commits/5b0ddd2.

I think this PR is mergeable.

vcunat added a commit to vcunat/nixpkgs that referenced this pull request Mar 8, 2014
@shlevy
Copy link
Member

shlevy commented Mar 15, 2014

@vcunat This is part of your p/stdenv branch right?

@vcunat
Copy link
Member

vcunat commented Mar 15, 2014

@shlevy: yes, it's in there.

vcunat added a commit to vcunat/nixpkgs that referenced this pull request Apr 2, 2014
@shlevy
Copy link
Member

shlevy commented Apr 5, 2014

@vcunat this should be fixed now, right?

@vcunat
Copy link
Member

vcunat commented Apr 5, 2014

Yes, in master now.

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.

5 participants