[Git][reproducible-builds/diffoscope][master] 4 commits: Improve help text for the --exclude-directory-metadata argument.

Chris Lamb gitlab at salsa.debian.org
Mon Jan 18 12:21:59 UTC 2021



Chris Lamb pushed to branch master at Reproducible Builds / diffoscope


Commits:
959d4233 by Chris Lamb at 2021-01-18T11:44:45+00:00
Improve help text for the --exclude-directory-metadata argument.

- - - - -
f7d8be3e by Dimitrios Apostolou at 2021-01-18T11:45:35+00:00
Avoid calling external stat command

We compare (some fields at least) using python's os.lstat().
Only if we find differences, we call external stat command
so that we can generate a diff using its output.

- - - - -
9e3abf9d by Dimitrios Apostolou at 2021-01-18T11:45:35+00:00
Introduce options --no-acl and --no-xattr

This offers a significant speedup to directory comparisons
by skipping invocation of utilities getfacl and lsattr.

- - - - -
8317da34 by Chris Lamb at 2021-01-18T12:01:37+00:00
Collapse --acl and --xattr into --extended-filesystem-attributes to cover all of these extended attributes, defaulting the new option to false (ie. to not check these very expensive external calls).

- - - - -


3 changed files:

- diffoscope/comparators/directory.py
- diffoscope/config.py
- diffoscope/main.py


Changes:

=====================================
diffoscope/comparators/directory.py
=====================================
@@ -89,6 +89,20 @@ else:
             return line.encode("utf-8")
 
 
+# compare only what matters
+def stat_results_same(stat1, stat2):
+    return all(
+        getattr(stat1, i) == getattr(stat2, i)
+        for i in [
+            "st_mode",
+            "st_uid",
+            "st_gid",
+            "st_size",
+            "st_mtime",
+        ]
+    )
+
+
 @tool_required("lsattr")
 def lsattr(path):
     """
@@ -154,41 +168,56 @@ def compare_meta(path1, path2):
         return []
 
     logger.debug("compare_meta(%r, %r)", path1, path2)
-    differences = []
 
     # Don't run any commands if any of the paths do not exist
-    if not os.path.exists(path1) or not os.path.exists(path2):
-        return differences
-
+    # or have other issues.
     try:
-        differences.append(
-            Difference.from_operation(Stat, path1, path2, short=True)
+        stat1 = os.lstat(path1)
+        stat2 = os.lstat(path2)
+    except Exception as e:
+        logger.warning(
+            f'Unable to stat file "{path1}" or "{path2}" ({str(e)})'
         )
-    except RequiredToolNotFound:
-        logger.error("Unable to find 'stat'! Is PATH wrong?")
+        return []
+
+    differences = []
+    if stat_results_same(stat1, stat2):
+        logger.debug("Stat structs are identical, moving on!")
+    else:
+        try:
+            differences.append(
+                Difference.from_operation(Stat, path1, path2, short=True)
+            )
+        except RequiredToolNotFound:
+            logger.error("Unable to find 'stat'! Is PATH wrong?")
+
     if os.path.islink(path1) or os.path.islink(path2):
         return [d for d in differences if d is not None]
-    try:
-        differences.append(
-            Difference.from_operation(Getfacl, path1, path2, short=True)
-        )
-    except RequiredToolNotFound:
-        logger.info(
-            "Unable to find 'getfacl', some directory metadata differences might not be noticed."
-        )
-    try:
-        lsattr1 = lsattr(path1)
-        lsattr2 = lsattr(path2)
-        differences.append(
-            Difference.from_text(
-                lsattr1, lsattr2, path1, path2, source="lsattr"
+
+    if Config().extended_filesystem_attributes:
+        try:
+            differences.append(
+                Difference.from_operation(Getfacl, path1, path2, short=True)
             )
-        )
-    except RequiredToolNotFound:
-        logger.info(
-            "Unable to find 'lsattr', some directory metadata differences might not be noticed."
-        )
-    differences.append(xattr(path1, path2))
+        except RequiredToolNotFound:
+            logger.info(
+                "Unable to find 'getfacl', some directory metadata differences might not be noticed."
+            )
+
+        try:
+            lsattr1 = lsattr(path1)
+            lsattr2 = lsattr(path2)
+            differences.append(
+                Difference.from_text(
+                    lsattr1, lsattr2, path1, path2, source="lsattr"
+                )
+            )
+        except RequiredToolNotFound:
+            logger.info(
+                "Unable to find 'lsattr', some directory metadata differences might not be noticed."
+            )
+        differences.append(xattr(path1, path2))
+
     return [d for d in differences if d is not None]
 
 


=====================================
diffoscope/config.py
=====================================
@@ -61,6 +61,8 @@ class Config:
         self.excludes = ()
         self.exclude_commands = ()
         self.exclude_directory_metadata = "no"
+        self.acl = True
+        self.xattr = True
         self.compute_visual_diffs = False
         self.max_container_depth = 50
         self.use_dbgsym = "auto"


=====================================
diffoscope/main.py
=====================================
@@ -304,12 +304,21 @@ def create_parser():
         "Metadata of archive members remain un-excluded "
         'except if "recursive" choice is set. '
         "Use this option to ignore permissions, timestamps, "
-        "xattrs etc. Default: False if comparing two "
-        'directories, else True. Note that "file" metadata '
-        "actually a property of its containing directory, "
+        "xattrs etc. Default: 'no' if comparing two "
+        "directories, else 'yes'. Note that \"file\" metadata is "
+        "actually a property of its containing directory "
         "and is not relevant when distributing the file across "
         "systems.",
     )
+    group3.add_argument(
+        "--extended-filesystem-attributes",
+        "--no-extended-filesystem-attributes",
+        action=BooleanAction,
+        default=False,
+        help="Check potentially-expensive filesystem extended "
+        "attributes such as POSIX ACLs, lsattr(1) attributes etc. "
+        "(default: False)",
+    )
     group3.add_argument(
         "--diff-mask",
         metavar="REGEX_PATTERN",
@@ -645,6 +654,9 @@ def configure(parsed_args):
     Config().exclude_directory_metadata = (
         parsed_args.exclude_directory_metadata
     )
+    Config().extended_filesystem_attributes = (
+        parsed_args.extended_filesystem_attributes
+    )
     Config().diff_masks = parsed_args.diff_masks
 
     Config().compute_visual_diffs = PresenterManager().compute_visual_diffs()



View it on GitLab: https://salsa.debian.org/reproducible-builds/diffoscope/-/compare/201d2505fe06ae815e789f6bab7d5a73dd6f0389...8317da340b75863d2aa6555094dc135c1581646d

-- 
View it on GitLab: https://salsa.debian.org/reproducible-builds/diffoscope/-/compare/201d2505fe06ae815e789f6bab7d5a73dd6f0389...8317da340b75863d2aa6555094dc135c1581646d
You're receiving this email because of your account on salsa.debian.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.reproducible-builds.org/pipermail/rb-commits/attachments/20210118/8f33cb12/attachment.htm>


More information about the rb-commits mailing list