Skip to content

Glibc with shell#4998

Closed
shlevy wants to merge 4 commits intoNixOS:masterfrom
shlevy:glibc-with-shell
Closed

Glibc with shell#4998
shlevy wants to merge 4 commits intoNixOS:masterfrom
shlevy:glibc-with-shell

Conversation

@shlevy
Copy link
Member

@shlevy shlevy commented Nov 16, 2014

On linux, combine the final glibc with the final bash, and use the sh from bash for glibc's system(3), popen(3), etc.

Fixes #1424

@shlevy
Copy link
Member Author

shlevy commented Nov 16, 2014

cc @edolstra

@Fuuzetsu Fuuzetsu added the 0.kind: enhancement Add something new or improve an existing system. label Nov 16, 2014
Copy link
Contributor

Choose a reason for hiding this comment

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

This worries me a little, doesn't this match .*/bin/sh.*? I'd feel safer with a \w kind of delimiter but I think sed doesn't do that :-/

Copy link
Member Author

Choose a reason for hiding this comment

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

I checked with grep and all matches are actually /bin/sh, but if you have something better I'm happy to switch to it.

Copy link
Member

Choose a reason for hiding this comment

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

s@\</bin/sh\>@$out/bin/sh@g ?

@7c6f434c
Copy link
Member

Is it a good idea to also put libgcc_s into this glibc while you are at it?It is a 100K library with a tendency to pull in a runtime GCC dependency.

@edolstra
Copy link
Member

@7c6f434c This is already the case, see 6522156.

@7c6f434c
Copy link
Member

Ah, right, missed this staging commit

And as we rebuild glibc late in the bootstrap, we get the correct gcc anyway.

Thanks.

@shlevy
Copy link
Member Author

shlevy commented Nov 17, 2014

@edolstra thoughts on this approach?

@edolstra
Copy link
Member

Hm, I'm not really enthusiastic about this approach. Combining Glibc and bash into one package seems kind of ugly to me...

Also, what does "final Glibc" mean? We build Glibc only once.

@wmertens
Copy link
Contributor

@edolstra The reasoning is in #1424. There is a circular dependency between bash and glibc for the system() call. Normally this is snipped by /bin/sh but on other platforms this is impure.
So the glibc+bash package is one solution, another option would be to move the global state elsewhere, like /nix/store/global/bin/sh or /nix/var/nix/bin/sh...

@shlevy
Copy link
Member Author

shlevy commented Nov 18, 2014

@edolstra Do you have a better solution for #1424 ? Also, the final term was just copied from the preexisting comment 😄

@vcunat
Copy link
Member

vcunat commented Nov 20, 2014

@edolstra: the dependency here is naturally cyclic. Allowing cycles in outputs is probably not worth the complications, so either we use different versions, or we use some out-of-store/impure overrides/symlinks (like /bin/sh), or we collapse the cycle into a single output.

Usually the impure bash doesn't cause any problems, but I like it less than adding a single bin/bash file to glibc.

@vcunat
Copy link
Member

vcunat commented Nov 25, 2014

I guess it's better to leave this after branching off the release.

@copumpkin
Copy link
Member

Any progress on this now that the release is branched?

@shlevy
Copy link
Member Author

shlevy commented Jan 19, 2015

This works, but @edolstra didn't seem to want it.

@copumpkin
Copy link
Member

@edolstra if you really don't want this (please say so, if so) I may have another approach that is a little less entangled, but someone still needs to do the work, and we should probably talk it over with you ahead of time to avoid throwaway work.

@shlevy
Copy link
Member Author

shlevy commented Jan 28, 2015

Closing this as it doesn't seem wanted.

For what it's worth, I don't currently expect to do work with wide impact like this in the future. The way these conversations always end up with no response is seriously demotivating and a huge waste of my time.

@shlevy shlevy closed this Jan 28, 2015
@vcunat
Copy link
Member

vcunat commented Jan 28, 2015

I wonder how to avoid such "lost work" in general, as this wasn't the first time. My long-term view is that working on high-impact changes around nixos.org truly is demotivating, which I perceive as a serious problem for future of the projects.

For similar (potentially) controversial changes, should we create an issue first to agree on a solution? I see this was discussed in a few issues a lot, although without a clear consensus what to do about that cyclic dependency... we tend to get stuck on such things.

@wmertens
Copy link
Contributor

@shlevy sorry to hear 😢

@vcunat yeah I try to open issues first but indeed they seldom come to resolutions. This is not unique to NixOS though, it's because of the come-all model of github I suppose. People without a strong opinion abstain from commenting, but so do people with opinions but no time to look at a complex issue. That leads to lingering issues.

In this particular case though I'm a bit disappointed by @edolstra not commenting any more after saying he wasn't enthousiastic. What does that mean, that:

  • if someone else decides to merge it and follow up on trouble, it's fine?
  • he doesn't want it at all?

Since he's the big boss it doesn't feel right to merge big-impact things without his explicit approval, but OTOH this is git and everything can be reverted.

It just seems rather confrontational to merge in the absence of communication and I'd like to have a policy about that.

@edolstra did comment on this PR indirectly in #1424 (comment)

It comes down to

package glibc+sh patch glibc during build
+ doesn't patch upstream source - patch source
+ build-time behavior identical - build-time behavior different, with theoretical issues
- adds bash dependency to entire system - dependency on /bin/sh is hidden
- adds extra build stage + no extra stage
+ runtime doesn't use /bin/sh - runtime behavior unchanged

Reopening for further discussion, hopefully more definite this time.

@edolstra
Copy link
Member

@wmertens There is a third possibility, namely "do nothing", which is a perfectly valid choice given that the /bin/sh issue has never bothered me all that much.

A fourth possibility that I experimented with is to have Glibc refer to a statically linked bash. This doesn't require any special tricks, but has the downside that our bootstrap tools will have to be updated (since they don't contain static libraries).

@7c6f434c
Copy link
Member

@wmertens There is a third possibility, namely "do nothing", which is a perfectly valid choice given that the /bin/sh issue has never bothered me all that much.

Well, having spurious chroot build failures at Hydra on glibc updates
seems to be a real problem...

A fourth possibility that I experimented with is to have Glibc refer to a statically linked bash. This doesn't require any special tricks, but has the downside that our bootstrap tools will have to be updated (since they don't contain static libraries).

@copumpkin
Copy link
Member

@edolstra there's what to do about the particular issue this PR is addressing, where "do nothing" might be a reasonable third option.

But when deciding what to do about communicating that decision, I don't think "do nothing" is a viable option if you want people to not quit in despair as @shlevy has now done. If you come out and tell him, "yo, shlevy, I appreciate all the work you put into this, but it really doesn't seem that bad or worth the additional complexity. Would you be really disappointed if we closed this?" I see that as strictly better than what happened here. Sure, it'll be disappointing in this instance that it gets closed, but at least he gets the feedback and doesn't have to feel like he's sending PRs into a vacuum.

Given that the exact same thing has happened in several other PRs, this seems like a problem. In all of those like this one, the "do nothing" code response might have been fine, but the "do nothing" people response was enough to accumulate and convince him that contributing to this project was a waste of his time.

@shlevy
Copy link
Member Author

shlevy commented Jan 28, 2015

To add to/support what @copumpkin was saying: My issue is not with my work not being accepted (though enough "no"s to my work would probably lead me to pull back as well of course). My issue is that several times I've done work that fits either a) too involved/complex/touches someone else's domain for me to be comfortable merging without their say-so or b) someone (usually @edolstra , sometimes @rbvermaa ) has indicated that they would like to review or are concerned about a particular change, and then the people I need to hear back from to move forward never get back to me. Going for months with periodic pings and no response is bad enough by itself, but on top of that while I'm waiting the code base changes around me and I have to actively work to keep the code up-to-date, tested, and mergable.

@vcunat
Copy link
Member

vcunat commented Feb 25, 2016

This issue is going on a year and a quarter, are these changes still applicable or desirable?

There wasn't a clear consensus, so the state remained as it was. I don't expect the opinions to be significantly different today.

@shlevy
Copy link
Member Author

shlevy commented Aug 11, 2016

Stalled again, closing

@shlevy shlevy closed this Aug 11, 2016
@abbradar
Copy link
Member

I would like to see this in Nixpkgs very much now for yet another reason: it would remove need for (at least make a way to) removing /bin/sh from build-sandbox-paths. At work we use CentOS on production servers but Nix is becoming more common to handle packaging. Unfortunately, you can't use build-use-sandbox now on non-NixOS without manually specifying path to Bash and all its dependencies in the Nix store, like this:

build-use-sandbox = true
build-sandbox-paths =  /bin/sh=/nix/store/nyj6xd7s1n1w8c0xdwk5ddhi7bjcyi9x-bash-4.3-p46/bin/bash /nix/store/6fix3zqpnahyml8zp2sxi2rwan55rgb8-glibc-2.24 /nix/store/nyj6xd7s1n1w8c0xdwk5ddhi7bjcyi9x-bash-4.3-p46

Specifying this by hand is unacceptable -- this would break after any more or less big Nixpgks update. If I understand correctly, we have two sources of this impurity:

1. Various build scripts that call `/bin/sh` -- those should be patched, like we already do for `/usr/bin/env`;
2. This exact issue.

Am I correct in my understanding? If yes, what do you think we can do with this? @edolstra also named another way to solve this: by using static-linked Bash. I'm not sure which is better, but doing nothing seems to create a big hindrance on the way of Nix adoption on non-NixOS systems (again, that is only if my understanding about /bin/sh usage is correct).

@abbradar
Copy link
Member

BTW (unrelated to this issue), a workaround to my issue is to use system Bash:

build-use-sandbox = true
build-sandbox-paths = /bin/sh /bin/bash /lib64/libtinfo.so.5 /lib64/libdl.so.2 /lib64/libc.so.6 /lib64/ld-linux-x86-64.so.2

However, I don't like this leakage of system libraries at all!

@7c6f434c
Copy link
Member

@abbradar You should just go in the opposite direction and use the nice = feature, try:

build-sandbox-paths = /bin/sh=/nix/store/a6gkm5vdylpjnsp9wzcb0c7kz11a3pak-bash-4.3-p46/bin/sh /nix/store/54k2pd57hqs7bz6dam7aw05slry9zzhh-glibc-2.24 /nix/store/a6gkm5vdylpjnsp9wzcb0c7kz11a3pak-bash-4.3-p46

Obviously, the bash and glibc paths should be substituted; obviously, you should autogenerate that text on update.

@abbradar
Copy link
Member

abbradar commented Oct 12, 2016

@7c6f434c Yes, I've seen that it can be done this way but IMO this is worse because now any user who runs nix-collect-garbage can potentially break builds.

It can be workarounded by a pinned bash but system administrator needs to update it manually and add it and all its dependencies to build-sandbox-paths. Of course this can be automated but this seems clunky anyway and I believe that getting rid of an impurity altogether is better.

EDIT: reworded because it looked like a mess _

@7c6f434c
Copy link
Member

@7c6f434c Yes, I've seen that this can be done this way but in my opinion this is worse because now any user who runs nix-collect-garbage can potentially break builds;

That's what autogeneration should also solve…

bash needs to be pinned and system administrator needs to update it manually and add it and all its dependencies to build-sandbox-paths.

All its dependencies are: glibc, nothing else.

Of course this can be automated but this seems clunky anyway and I believe that getting rid of an impurity altogether is better.

Well, getting rid of an impurity is complicated. It would be nice in
theory, but it requires more agreement on details than can be reached
cheaply. But yes, #1424 is almost impossible to avoid otherwise.

if your nix.conf is generated by an expression, and it is put into
place by nix-build -o, GC won't touch the current version. This can
even be put into a cron job.

@abbradar
Copy link
Member

Indeed, autogeneration would help as you have said (I wasn't arguing about that). It's just that it creates additional ad-hoc scripts and bullet points that administrator should follow. This also prevents user from just installing Nix via RPM or another package manager and using multi-user sandboxed setup without additional work that requires understanding of how Nix internals work. I'm working on improving that recently.

To give you more context: ideally the only thing that administrator herself needs to do from time to time is to update system Nix. This can be done by e.g. installing a package with the system package manager (well, it's not that easy now but I'm working on improving the situation and we are fairly close). Any other maintenance, including updates of repositories and local packages, can be done by users -- Nix model provides that for free.

However, now we introduce another step: the administrator needs to write a script for updating and pinning Bash in GC and either run it by herself from time to time or adding it to cron (which, I argue, is a thing one should never do on a server -- autoupdates are dangerous! However that's unrelated to this discussion). All that is just for this one impurity!

Of course rushing this issue to be fixed somehow is not what I (or anyone of us) want; my point was that "doing nothing", like @edolstra suggested, has more negative consequences than was already shown here, so we may want to return to the discussion table.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

0.kind: enhancement Add something new or improve an existing system.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

libc: purity problems with /bin/sh used to implement system(3)

8 participants