brp-15: revert stripping if strip outputs to stderr #7
Conversation
GNU strip doesn't return any error code if it doesn't recognise the archive. This results in us still committing a stripping that may have gone incorrectly, which is unsafe. Instead, check that the stripping didn't output any errors and only copy over the stripped archive if there were no errors. Fixes: boo#964546 Signed-off-by: Aleksa Sarai <asarai@suse.com>
|
Is this behavior of strip intentional? Sounds more like a bug than anything else |
|
I don't know if it's intentional, but even if it was not intentional (and they fixed it) we'd still need to implement reverting because |
|
This is also related to golang/go#17890. |
| chmod u+w "$f" || : | ||
| strip -p --discard-locals -R .comment -R .note "$f" || : |
There was a problem hiding this comment.
maybe there is actually an error code and it is just ignored with the || :
> echo foo > foo
> strip foo
strip:foo: File format not recognized
> echo $?
1
(the above worked on both current Tumbleweed and old SLE-11-SP3)
There was a problem hiding this comment.
I've checked and there isn't an error code.
% cp /usr/lib64/go/pkg/linux_amd64/runtime.a .
% strip runtime.a
strip:runtime.a(__.PKGDEF): Unable to recognise the format of file: File format not recognized
strip:runtime.a(_go_.o): Unable to recognise the format of file: File format not recognized
strip:runtime.a(asm.o): Unable to recognise the format of file: File format not recognized
strip:runtime.a(asm_amd64.o): Unable to recognise the format of file: File format not recognized
strip:runtime.a(duff_amd64.o): Unable to recognise the format of file: File format not recognized
strip:runtime.a(memclr_amd64.o): Unable to recognise the format of file: File format not recognized
strip:runtime.a(memmove_amd64.o): Unable to recognise the format of file: File format not recognized
strip:runtime.a(rt0_linux_amd64): Unable to recognise the format of file: File format not recognized
strip:runtime.a(sys_linux_amd64): Unable to recognise the format of file: File format not recognized
% echo $?
0
The problem is that strip does recognise the .a archive (so it won't error out because of that) but it will incorrectly modify the archive because it doesn't understand the format.
TBH this looks like a mix of a strip bug (not returning a non-zero error code) but there's also a bug in this repo because we're ignoring errors. Also, note that we have to implement reverting anyway.
There was a problem hiding this comment.
This is strange, I could have sworn that strip can recognize .a files from Go code...
There was a problem hiding this comment.
It can "recognise" them, but it incorrectly strips them (this bug is a bit old so I can't remember the precise details, but the general point is that it breaks __.PKGDEF by slightly changing the contents, which Go really doesn't like). I submitted a patch to Go which would ignore the way that strip messes with .a files, but they rejected it on the basis of "we don't support you using strip on Go binaries, the go compiler has a flag that does stripping for you".
For the past year we've solved this problem by simply disabling stripping for all Go packages, but that's quite fickle (you can't really add a macro to do it -- it needs to be added as a #nodebuginfo comment in the spec file, even if you set the magic BRP_... macro variable).
|
please rebase if still needed and reopen. |
GNU strip doesn't return any error code if it doesn't recognise the
archive. This results in us still committing a stripping that may have
gone incorrectly, which is unsafe.
Instead, check that the stripping didn't output any errors and only copy
over the stripped archive if there were no errors.
Fixes: boo#964546
Signed-off-by: Aleksa Sarai asarai@suse.com