New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Simplify and cleanup the binaries and libraries copying code. #1521
Merged
jsmeix
merged 4 commits into
rear:master
from
jsmeix:also_find_libraries_required_by_libraries_in_recovery_system_issue_1518
Oct 4, 2017
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
3973399
Simplify and cleanup the binaries and libraries copying code
jsmeix efc53aa
More explanatory comments and using 'readlink -m' because no componen…
jsmeix dc44fb4
Replaced actually local variable 'BINARIES' with 'all_binbaries'
jsmeix 6f364bb
Merge branch 'master' into also_find_libraries_required_by_libraries_…
jsmeix File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,53 +1,72 @@ | ||
# 400_copy_as_is.sh | ||
# | ||
# copy files and directories that should be copied over as-is to the rescue | ||
# systems. Checks also for library dependencies of executables and adds | ||
# them to the LIBS list, if they are not included in the copied files. | ||
# Copy files and directories that should be copied as-is into the recovery system. | ||
# Check also for library dependencies of executables in all the copied files and | ||
# add them to the LIBS list if they are not yet included in the copied files. | ||
|
||
LogPrint "Copying files and directories" | ||
Log "Files being copied: ${COPY_AS_IS[@]}" | ||
Log "Files being excluded: ${COPY_AS_IS_EXCLUDE[@]}" | ||
|
||
for f in "${COPY_AS_IS_EXCLUDE[@]}" ; do echo "$f" ; done >$TMP_DIR/copy-as-is-exclude | ||
tar -v -X $TMP_DIR/copy-as-is-exclude \ | ||
-P -C / -c "${COPY_AS_IS[@]}" 2>$TMP_DIR/copy-as-is-filelist | \ | ||
tar $v -C $ROOTFS_DIR/ -x >/dev/null | ||
StopIfError "Could not copy files and directories" | ||
Log "Finished copying COPY_AS_IS" | ||
|
||
# fix ReaR directory if running from checkout | ||
if [[ "$REAR_DIR_PREFIX" ]] ; then | ||
for dir in /usr/share/rear /var/lib/rear ; do | ||
ln $v -sf $REAR_DIR_PREFIX$dir $ROOTFS_DIR$dir>/dev/null | ||
local copy_as_is_filelist_file="$TMP_DIR/copy-as-is-filelist" | ||
local copy_as_is_exclude_file="$TMP_DIR/copy-as-is-exclude" | ||
|
||
# Build the list of files and directories that are excluded from being copied: | ||
local excluded_file="" | ||
for excluded_file in "${COPY_AS_IS_EXCLUDE[@]}" ; do | ||
echo "$excluded_file" | ||
done >$copy_as_is_exclude_file | ||
|
||
# Copy files and directories as-is into the recovery system except the excluded ones and | ||
# remember what files and directories were actually copied in a copy_as_is_filelist_file | ||
# which is the reason that the first 'tar' must be run in verbose mode: | ||
if ! tar -v -X $copy_as_is_exclude_file -P -C / -c "${COPY_AS_IS[@]}" 2>$copy_as_is_filelist_file | tar $v -C $ROOTFS_DIR/ -x 1>/dev/null ; then | ||
Error "Failed to copy files and directories in COPY_AS_IS minus COPY_AS_IS_EXCLUDE" | ||
fi | ||
Log "Finished copying files and directories in COPY_AS_IS minus COPY_AS_IS_EXCLUDE" | ||
|
||
# Build an array of the actual regular files that are executable in all the copied files: | ||
local copy_as_is_executables=() | ||
local copy_as_is_file="" | ||
while read -r copy_as_is_file ; do | ||
# Skip non-regular files (like directories and device files): | ||
test -f "$copy_as_is_file" || continue | ||
# Skip symbolic links (only care about symbolic link targets): | ||
test -L "$copy_as_is_file" && continue | ||
# Remember actual regular files that are executable: | ||
test -x "$copy_as_is_file" && copy_as_is_executables=( "${copy_as_is_executables[@]}" "$copy_as_is_file" ) | ||
done <$copy_as_is_filelist_file | ||
Log "copy_as_is_executables = ${copy_as_is_executables[@]}" | ||
|
||
# Check for library dependencies of executables in all the copied files and | ||
# add them to the LIBS list if they are not yet included in the copied files: | ||
Log "Adding required libraries of executables in all the copied files to LIBS" | ||
local required_library="" | ||
for required_library in $( RequiredSharedOjects "${copy_as_is_executables[@]}" ) ; do | ||
# Skip when the required library was already actually copied by 'tar' above: | ||
grep -q "$required_library" $copy_as_is_filelist_file && continue | ||
# Skip when the required library is already in LIBS: | ||
IsInArray "$required_library" "${LIBS[@]}" && continue | ||
Log "Adding required library '$required_library' to LIBS" | ||
LIBS=( "${LIBS[@]}" "$required_library" ) | ||
done | ||
Log "LIBS = ${LIBS[@]}" | ||
|
||
# Fix ReaR directories when running from checkout: | ||
if test "$REAR_DIR_PREFIX" ; then | ||
Log "Fixing ReaR directories when running from checkout" | ||
local rear_dir="" | ||
for rear_dir in /usr/share/rear /var/lib/rear ; do | ||
ln $v -sf $REAR_DIR_PREFIX$rear_dir $ROOTFS_DIR$rear_dir 1>/dev/null | ||
done | ||
fi | ||
|
||
### Copy configuration directory | ||
Log "Copying ReaR configuration directory" | ||
# Copy ReaR configuration directory: | ||
mkdir $v -p $ROOTFS_DIR/etc/rear | ||
# This will do same job as lines below. | ||
# On top of that, it does not throw log warning like: | ||
# "cp: missing destination file operand after" | ||
# if hidden file (.<filename>) is missing in $CONFIG_DIR | ||
cp $v -r $CONFIG_DIR/. $ROOTFS_DIR/etc/rear/ >&2 | ||
|
||
COPY_AS_IS_EXELIST=() | ||
while read -r ; do | ||
if [[ ! -d "$REPLY" && -x "$REPLY" ]]; then | ||
COPY_AS_IS_EXELIST=( "${COPY_AS_IS_EXELIST[@]}" "$REPLY" ) | ||
fi | ||
done <$TMP_DIR/copy-as-is-filelist | ||
Log "COPY_AS_IS_EXELIST = ${COPY_AS_IS_EXELIST[@]}" | ||
|
||
Log "Adding required libraries to LIBS with checking COPY_AS_IS_EXELIST" | ||
# add required libraries to LIBS, skip libraries that are part of the copied files. | ||
while read -r ; do | ||
lib="$REPLY" | ||
if ! IsInArray "$lib" "${COPY_AS_IS_EXELIST[@]}"; then | ||
# if $lib is NOT part of the copy-as-is fileset, then add it to the global libs | ||
LIBS=( ${LIBS[@]} $lib ) | ||
else | ||
Log "Not adding $lib to LIBS because it is already in COPY_AS_IS_EXELIST" | ||
fi | ||
done < <( SharedObjectFiles "${COPY_AS_IS_EXELIST[@]}" | sed -e 's#^#/#' ) | ||
Log "LIBS = ${LIBS[@]}" | ||
cp $v -r $CONFIG_DIR/. $ROOTFS_DIR/etc/rear/ 1>&2 | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @jsmeix , do you know why to skip symbolic links? The previous versions did not have this (the test was
if [[ ! -d "$REPLY" && -x "$REPLY" ]]; then
, which includes symlinks).I believe skipping symlinks is wrong: if the symlink is dangling in the recovery system, the target will be eventually copied by
build/default/490_fix_broken_links.sh
, but without libraries, and verification of the recovery system will fail and abortrear mkrescue
.(If the symlink is not dangling, there will be duplicates in the library list if we don't skip it, but I suppose that's not a big problem?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing this line does not seem to have any ill effect in a quick test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pcahyna
could you please make a new GitHub issue for it
where you explain how things went wrong for you
so I could better understand the whole thing
and we could better discuss it
(I do no longer trust comments in pull requests).
I don't remember why I skip symlinks there.
The initial comment of this code part
also only tells that symlinks are not considered
but it does not tell why.
I assume your failing case is a rather special case
because I committed that change on Sep 29 2017 as
3973399
and I merged its pull request on Oct 4 2017
so since 6 years it had worked without issues
(at least without reported issues as far as I know)
which is why I like to fully understand the root cause
behind your current issue why it now fails for you
before I "just change" any symlink behaviour in ReaR
because in practice symlinks are a hell of endless pain
(not only in ReaR but everywhere).
For the (not so) fun of it:
In the end symlinks are just one implementation of
RFC 1925 item 6a
"It is always possible to add another level of indirection"
which is (in my experience) a root cause behind 90% of issues.
In my experience another root cause behind 90% of issues
is RFC 1925 item 5 "aglutenate [sic] multiple separate
problems into a single complex interdependent solution"
so about 81% of issues have RFC 1925 items 5 and 6a
as root causes :-(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jsmeix , #3064 . It also explains why this problem does not arise very often.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pcahyna
thank you!
I will have a look tomorrow...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After quick reading #3064 (comment)
I can now imagine even a possible reason why COPY_AS_IS
does not care about symlinks like PROGS does:
Because COPY_AS_IS means "copy as is but nothing more"?
This is only an offhanded guess.