auto-patchelf: don't use grep -q, as it causes Broken pipe #56958
Conversation
|
🤔 I see it doesn't cause mass rebuilds. Ok. |
| isExeResult="$(LANG=C readelf -h -l "$1" 2> /dev/null \ | ||
| | grep '^ *Type: *EXEC\>\|^ *INTERP\>')" | ||
| # not using grep -q, because it can cause Broken pipe | ||
| [ -n "$isExeResult" ] |
There was a problem hiding this comment.
What about using grep -q -m1 instead? This should stop after a match has been found and not result in a broken pipe error.
There was a problem hiding this comment.
How can we verify this works? I can't reproduce original issue. The closest thing I could do is generate broken pipe from pip list:
$ (pip list | grep Package) 2>&1 | head
You are using pip version 18.1, however version 19.0.3 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.
Package Version
$ (pip list | grep -q Package) 2>&1 | head
Traceback (most recent call last):
File "/nix/store/l95nkqp7bdimqnz9ixay1aahljzsz7vc-python-2.7.15/lib/python2.7/logging/__init__.py", line 892, in emit
self.flush()
File "/nix/store/l95nkqp7bdimqnz9ixay1aahljzsz7vc-python-2.7.15/lib/python2.7/logging/__init__.py", line 852, in flush
self.stream.flush()
IOError: [Errno 32] Broken pipe
Logged from file list.py, line 234
Traceback (most recent call last):
File "/nix/store/l95nkqp7bdimqnz9ixay1aahljzsz7vc-python-2.7.15/lib/python2.7/logging/__init__.py", line 892, in emit
self.flush()
$ (pip list | grep -q -m1 Package) 2>&1 | head
Traceback (most recent call last):
File "/nix/store/l95nkqp7bdimqnz9ixay1aahljzsz7vc-python-2.7.15/lib/python2.7/logging/__init__.py", line 892, in emit
self.flush()
File "/nix/store/l95nkqp7bdimqnz9ixay1aahljzsz7vc-python-2.7.15/lib/python2.7/logging/__init__.py", line 852, in flush
self.stream.flush()
IOError: [Errno 32] Broken pipe
Logged from file list.py, line 234
Traceback (most recent call last):
File "/nix/store/l95nkqp7bdimqnz9ixay1aahljzsz7vc-python-2.7.15/lib/python2.7/logging/__init__.py", line 892, in emit
self.flush()
| if isExecutable "$file"; then | ||
| # Skip if the executable is statically linked. | ||
| echo "$segmentHeaders" | grep -q "^ *INTERP\\>" || continue |
There was a problem hiding this comment.
This shouldn't ever result in a broken pipe error, at least in this context, so we don't need the workaround here.
There was a problem hiding this comment.
@danbst: Hm, apparently according to your description it does. Do you have an output of strace?
There was a problem hiding this comment.
@aszlig this is exactly the place where it failed, notice lines:
/nix/store/../setup-hook: line 213: echo: write error: Broken pipe
/nix/store/../setup-hook: line 210: echo: write error: Broken pipe
There was a problem hiding this comment.
sorry, missed your last comment. No, I can't reproduce this issue. It probably occurs only on high CPU load/slow CPUs.
| @@ -207,10 +209,11 @@ autoPatchelf() { | |||
| isELF "$file" || continue | |||
| segmentHeaders="$(LANG=C readelf -l "$file")" | |||
| # Skip if the ELF file doesn't have segment headers (eg. object files). | |||
| echo "$segmentHeaders" | grep -q '^Program Headers:' || continue | |||
| # not using grep -q, because it can cause Broken pipe | |||
| [ -n "$(echo "$segmentHeaders" | grep '^Program Headers:')" ] || continue | |||
|
@aszlig I'll merge this in a week if you don't object. I can't find shorter solution, which would work with It requires rebuilding Qt, so I'll leave this for |
Since when does this need to rebuild Qt? Looking at the changed paths reported by ofborg, I can't see anything related to that, am I blind? |
This rare sitation was caught when building zoom-us package: ``` automatically fixing dependencies for ELF files /nix/store/71d65fplq44y9yn2fvkpn2d3hrszracd-auto-patchelf-hook/nix-support/setup-hook: line 213: echo: write error: Broken pipe /nix/store/71d65fplq44y9yn2fvkpn2d3hrszracd-auto-patchelf-hook/nix-support/setup-hook: line 210: echo: write error: Broken pipe ``` The worst is that derivation continued and resulted into broken package: NixOS#55566 (comment) I hope, replacing `grep -q` with `grep` will remove this race condition.
danbst
force-pushed
the
auto-patchelf-hook-broken-pipe
branch
from
1176fd3
to
87dfe89
Compare
|
@aszlig you are correct, it was rebuilding Qt because of |
This rare sitation was caught when building zoom-us package:
The worst is that derivation continued and resulted into broken package:
#55566 (comment)
I hope, replacing
grep -qwithgrepwill remove this race condition.cc @dasJ @aszlig as I see there was previously some attempt to fix Broken pipe in other places (#40598)