#2523 PR `merged`: Completely overhauled ldd test in 990_verify_rootfs.sh¶

Labels: enhancement, cleanup, fixed / solved / done, minor bug

jsmeix opened issue at 2020-11-23 13:48:¶

Type: Bug Fix / Enhancement / Cleanup
Impact: Low / High
Low for normal use cases with 'tar' as backup tool.
Probably high for users with third-party backup tools
that require special LD_LIBRARY_PATH settings.
Reference to related issue (URL):
https://github.com/rear/rear/issues/2508#issuecomment-726711801
How was this pull request tested?
"rear mkrescue" still works well for me with backup tool 'tar'
Brief description of the changes in this pull request:

The whole ldd testing code in 990_verify_rootfs.sh was completely overhauled:

Now a special LD_LIBRARY_PATH is only set directly before the "chroot ... ldd" test
and afterwards it is restored to what it was before.

Now each binary is first tested without any LD_LIBRARY_PATH set (which is the normal case)
and when that failed it is tested with the LD_LIBRARY_PATH setting while "rear mkrescue" is run
(if there is such a LD_LIBRARY_PATH setting) and when that also failed it is tested with a
BACKUP method specific LD_LIBRARY_PATH setting (if there is such a LD_LIBRARY_PATH setting).

Only when all tests fail the binary is considered to have actually missing shared objects dependencies.
This way we do not need to know which binaries require what specific LD_LIBRARY_PATH setting.

We assume when the ldd test succeeds with one of those LD_LIBRARY_PATH settings that setting
will be somehow 'magically' also happen inside the recovery system to make things work therein
because special backup tools should have their methods to set LD_LIBRARY_PATH as they need it.

jsmeix commented at 2020-11-24 09:42:¶

Before this pull request the ldd test code was basically

# Set special LD_LIBRARY_PATH
export LD_LIBRARY_PATH=$special_stuff
# Test all binaries with special LD_LIBRARY_PATH set
for binary in $( find_binaries ) ; do
    chroot $ROOTFS_DIR /bin/bash --login -c "... ldd $binary"
done
# Restore previous LD_LIBRARY_PATH
... export LD_LIBRARY_PATH=$saved_LD_LIBRARY_PATH ...

which is bad because after export LD_LIBRARY_PATH=$special_stuff
all programs run with that special LD_LIBRARY_PATH set, cf.
https://github.com/rear/rear/issues/2508#issuecomment-726711801

With the current pull request the ldd test code is basically

for binary in $( find_binaries ) ; do
    # Set special LD_LIBRARY_PATH
    export LD_LIBRARY_PATH=$special_stuff
    # Test the binary with special LD_LIBRARY_PATH set
    chroot $ROOTFS_DIR /bin/bash --login -c "... ldd $binary"
    # Restore previous LD_LIBRARY_PATH
    ... export LD_LIBRARY_PATH=$saved_LD_LIBRARY_PATH ...
done

which is better because only chroot ... ldd is run with special LD_LIBRARY_PATH set.

But this still looks wrong because the actual program that needs to run
with special LD_LIBRARY_PATH set is only ldd inside the chroot environment.
There is no reason to set the special LD_LIBRARY_PATH in the bash that runs rear.
So I will further change the code to be like this:

for binary in $( find_binaries ) ; do
    # Test the binary with special LD_LIBRARY_PATH set
    chroot $ROOTFS_DIR /bin/bash --login -c "export LD_LIBRARY_PATH=$special_stuff ... ldd $binary"
done

Now the special LD_LIBRARY_PATH is set only in the bash that runs ldd and
that bash exits after ldd finished so the special LD_LIBRARY_PATH setting is gone with it.
So there is no need to remember and restore some previously set LD_LIBRARY_PATH
because nothing was changed in the bash that runs rear.

jsmeix commented at 2020-11-24 11:26:¶

@OliverO2
only FYI:
Via this pull request https://github.com/rear/rear/pull/2523
I completely overhauled the ldd test in 990_verify_rootfs.sh

See
https://github.com/rear/rear/pull/2523#issuecomment-732779380
for a summary how it was before and what I changed.

If you are interested and have time for it I would appreciate it
if you could test if the current one in this pull request works for you.

To get a git clone of the current state of this pull request you could do:

# git clone --single-branch --branch jsmeix-overhauled-ldd-test https://github.com/rear/rear.git

# mv rear rear.jsmeix-overhauled-ldd-test

# cd rear.jsmeix-overhauled-ldd-test

# vi etc/rear/local.conf

# usr/sbin/rear -D mkrescue

OliverO2 commented at 2020-11-24 11:53:¶

@jsmeix
I'll have a look and I'll do a test run. Will probably take 2-3 days, but if we're lucky, I can squeeze it in faster...

OliverO2 commented at 2020-11-25 14:05:¶

@jsmeix
I ran your branch with minimal local configuration on Ubuntu 20.04.1 LTS Desktop. Everything worked as expected. Relevant debug output was:

Testing that the recovery system in /tmp/rear.mUiqWGXc9CVbk2Y/rootfs contains a usable system
Testing each binary with 'ldd' and look for 'not found' libraries within the recovery system
Testing that each program in the PROGS array can be found as executable command within the recovery system
Testing that each program in the REQUIRED_PROGS array can be found as executable command within the recovery system

(Tried to boot the recovery system on VirtualBox VM, but that froze after completing its startup to the root login prompt. However, this is an issue most probably completely unrelated to ReaR.)

I have also looked at the code and did not notice anything suspicious ;-). So LGTM...

jsmeix commented at 2020-11-25 14:31:¶

@OliverO2
thank you very much for testing it and for looking at the code!

@rear/contributors
if there are no objections I would like to merge it tomorrow afternoon.

[Export of Github issue for rear/rear.]

#2523 PR merged: Completely overhauled ldd test in 990_verify_rootfs.sh¶