Hello,
Some symlinks in some RPMs resolve to binaries outside of the package. In
those cases, avoid considering them.
* src/abg-tools-utils.cc (maybe_get_symlink_target_file_path): Do
not require that the file path points to a symlink. A file path
can point to a file that is not a symlink and yet the whole path
can be in parent directory that is a symlink. In this case,
realpath will correctly resolve to the correct target file.
* tools/abipkgdiff.cc (maybe_update_package_content): A path to a
binary that is not inside the RPM (because a symlink resolved to a
file outside of the RPM) should not be added to the set of
binaries to be analyzed.
* tests/data/test-diff-pkg/symlink-dir-test1-report1.txt: Add new
test file.
* tests/data/test-diff-pkg-ctf/symlink-dir-test1-report1.txt:
Likewise.
* tests/data/Makefile.am: Add new test file to source
distribution.
* tests/test-diff-pkg.cc (in_out_specs): for the
data/test-diff-pkg/symlink-dir-test1/dir{1,2}/symlinks test, the
root dir of the package is
data/test-diff-pkg/symlink-dir-test1/dir{1,2}. Use that to test
that the symlinks are properly handled. Also, use the
data/test-diff-pkg/symlink-dir-test1/dir{1,2}/symlinks as a root
of an alternative package for which the symlinks resolve outside
the package, under
data/test-diff-pkg/symlink-dir-test1/dir{1,2}/targets. In this
later case, the symlinked files should be ignored in the
comparison. Likewise for
data/test-diff-pkg-ctf/symlink-dir-test1/dir1/symlinks.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Applied to master.
---
src/abg-tools-utils.cc | 3 --
tests/data/Makefile.am | 2 ++
.../symlink-dir-test1-report1.txt | 0
.../symlink-dir-test1-report1.txt | 0
tests/test-diff-pkg.cc | 32 ++++++++++++++++---
tools/abipkgdiff.cc | 25 ++++++++++++---
6 files changed, 50 insertions(+), 12 deletions(-)
create mode 100644 tests/data/test-diff-pkg-ctf/symlink-dir-test1-report1.txt
create mode 100644 tests/data/test-diff-pkg/symlink-dir-test1-report1.txt
@@ -723,9 +723,6 @@ maybe_get_symlink_target_file_path(const string& file_path,
if (!get_stat(file_path, &st))
return false;
- if (!S_ISLNK(st.st_mode))
- return false;
-
char *link_target_path = realpath(file_path.c_str(), NULL);
if (!link_target_path)
return false;
@@ -2055,6 +2055,7 @@ test-diff-pkg/dirpkg-3-report-1.txt \
test-diff-pkg/dirpkg-3-report-2.txt \
test-diff-pkg/dirpkg-3.suppr \
test-diff-pkg/symlink-dir-test1-report0.txt \
+test-diff-pkg/symlink-dir-test1-report1.txt \
test-diff-pkg/symlink-dir-test1/dir1/symlinks/foo.o \
test-diff-pkg/symlink-dir-test1/dir1/symlinks/libfoo.so \
test-diff-pkg/symlink-dir-test1/dir1/targets/foo.c \
@@ -2300,6 +2301,7 @@ test-diff-pkg-ctf/cracklib-2.9.6-15-ol8u0.x86_64.rpm \
test-diff-pkg-ctf/isl-debuginfo-0.16.1-6.x86_64.rpm \
test-diff-pkg-ctf/tarpkg-1-dir1.tar.gz \
test-diff-pkg-ctf/symlink-dir-test1-report0.txt \
+test-diff-pkg-ctf/symlink-dir-test1-report1.txt \
\
test-fedabipkgdiff/dbus-glib-0.104-3.fc23.x86_64.rpm \
test-fedabipkgdiff/dbus-glib-debuginfo-0.104-3.fc23.x86_64.rpm \
new file mode 100644
new file mode 100644
@@ -149,6 +149,18 @@ static InOutSpec in_out_specs[] =
"data/test-diff-pkg/dirpkg-3-report-2.txt",
"output/test-diff-pkg/dirpkg-3-report-2.txt"
},
+ {
+ "data/test-diff-pkg/symlink-dir-test1/dir1",
+ "data/test-diff-pkg/symlink-dir-test1/dir2",
+ "--no-default-suppression ",
+ "",
+ "",
+ "",
+ "",
+ "",
+ "data/test-diff-pkg/symlink-dir-test1-report0.txt",
+ "output/test-diff-pkg/symlink-dir-test1-report0.txt"
+ },
{
"data/test-diff-pkg/symlink-dir-test1/dir1/symlinks",
"data/test-diff-pkg/symlink-dir-test1/dir2/symlinks",
@@ -158,8 +170,8 @@ static InOutSpec in_out_specs[] =
"",
"",
"",
- "data/test-diff-pkg/symlink-dir-test1-report0.txt ",
- "output/test-diff-pkg/symlink-dir-test1-report0.txt "
+ "data/test-diff-pkg/symlink-dir-test1-report1.txt",
+ "output/test-diff-pkg/symlink-dir-test1-report1.txt"
},
#if WITH_TAR
{
@@ -864,6 +876,18 @@ static InOutSpec in_out_specs[] =
"data/test-diff-pkg-ctf/dirpkg-3-report-2.txt",
"output/test-diff-pkg-ctf/dirpkg-3-report-2.txt"
},
+ {
+ "data/test-diff-pkg-ctf/symlink-dir-test1/dir1",
+ "data/test-diff-pkg-ctf/symlink-dir-test1/dir2",
+ "--ctf --no-default-suppression ",
+ "",
+ "",
+ "",
+ "",
+ "",
+ "data/test-diff-pkg-ctf/symlink-dir-test1-report0.txt",
+ "output/test-diff-pkg-ctf/symlink-dir-test1-report0.txt"
+ },
{
"data/test-diff-pkg-ctf/symlink-dir-test1/dir1/symlinks",
"data/test-diff-pkg-ctf/symlink-dir-test1/dir2/symlinks",
@@ -873,8 +897,8 @@ static InOutSpec in_out_specs[] =
"",
"",
"",
- "data/test-diff-pkg-ctf/symlink-dir-test1-report0.txt ",
- "output/test-diff-pkg-ctf/symlink-dir-test1-report0.txt "
+ "data/test-diff-pkg-ctf/symlink-dir-test1-report1.txt",
+ "output/test-diff-pkg-ctf/symlink-dir-test1-report1.txt"
},
#if WITH_TAR
{
@@ -2323,14 +2323,20 @@ typedef shared_ptr<self_compare_task> self_compare_task_sptr;
/// function only looks for a file name which name is the same as the
/// value of this parameter.
///
+/// @param parent_dir_name the name of the directory that the file
+/// name denoted by @p entry should belong to. If it doesn't (because
+/// it's a symlink that resolves to a file outside of that directory)
+/// then the vector of paths of is not updated.
+///
/// @param paths out parameter. This is the set of meaningful paths
/// of the current directory tree being analyzed. These paths are
/// those that are going to be involved in ABI comparison.
static void
-maybe_update_package_content(const FTSENT *entry,
- options &opts,
- const string& file_name_to_look_for,
- unordered_set<string>& paths)
+maybe_update_package_content(const FTSENT* entry,
+ options& opts,
+ const string& file_name_to_look_for,
+ const string& parent_dir_name,
+ unordered_set<string>& paths)
{
if (entry == NULL
|| (entry->fts_info != FTS_F && entry->fts_info != FTS_SL)
@@ -2340,6 +2346,15 @@ maybe_update_package_content(const FTSENT *entry,
string path = entry->fts_path;
maybe_get_symlink_target_file_path(path, path);
+ string parent_dir = parent_dir_name;
+ maybe_get_symlink_target_file_path(parent_dir, parent_dir);
+
+ if (!parent_dir_name.empty())
+ {
+ string s;
+ if (!string_suffix(path, parent_dir, s))
+ return;
+ }
if (!file_name_to_look_for.empty())
{
@@ -2391,7 +2406,7 @@ get_interesting_files_under_dir(const string dir,
FTSENT *entry;
unordered_set<string> files;
while ((entry = fts_read(file_hierarchy)))
- maybe_update_package_content(entry, opts, file_name_to_look_for, files);
+ maybe_update_package_content(entry, opts, file_name_to_look_for, dir, files);
for (unordered_set<string>::const_iterator i = files.begin();
i != files.end();