Author: dsahlberg
Date: Thu Oct  5 07:36:08 2023
New Revision: 1912743

URL: http://svn.apache.org/viewvc?rev=1912743&view=rev
Log:
Fix issue #1778: Better handling if diff is not available.

r1824410 solves the basic issue, to use the internal diff functions
when available. However if diffoptions is not None, an external
diff command is still called. If diff (or diff.exe) is not found
in PATH, Python2 will raise an OSError and Python3 will raise a
FileNotFoundError (which inherits OSError).

r1912724 adds a docstring to FileDiff.get_pipe() documenting this
behaviour.

This revision add an improved error message. When dropping Python2
support, the code can catch FileNotFoundError and remove the check
for ENOENT.

* subversion/bindings/swig/python/svn/fs.py
  (FileDiff.get_pipe): Catch OSError/ENOENT and improve error msg

* subversion/bindings/swig/python/tests/fs.py
  (test_diff_repos_paths_external): Add note to change code when
   droping Python2 support. No functional change.


Modified:
    subversion/trunk/subversion/bindings/swig/python/svn/fs.py
    subversion/trunk/subversion/bindings/swig/python/tests/fs.py

Modified: subversion/trunk/subversion/bindings/swig/python/svn/fs.py
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/swig/python/svn/fs.py?rev=1912743&r1=1912742&r2=1912743&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/swig/python/svn/fs.py (original)
+++ subversion/trunk/subversion/bindings/swig/python/svn/fs.py Thu Oct  5 
07:36:08 2023
@@ -23,6 +23,7 @@
 #    under the License.
 ######################################################################
 
+import errno
 from libsvn.fs import *
 
 ######################################################################
@@ -182,8 +183,17 @@ class FileDiff:
             + [self.tempfile1, self.tempfile2]
 
       # open the pipe, and return the file object for reading from the child.
-      p = _subprocess.Popen(cmd, stdout=_subprocess.PIPE, bufsize=-1,
-                            close_fds=_sys.platform != "win32")
+      try:
+        p = _subprocess.Popen(cmd, stdout=_subprocess.PIPE, bufsize=-1,
+                              close_fds=_sys.platform != "win32")
+      # When removing Python 2 support: Change to FileNotFoundError and 
+      # remove check for ENOENT (FileNotFoundError "Corresponds to errno
+      # ENOENT" according to documentation)
+      except OSError as err:
+        if err.errno == errno.ENOENT:
+          err.strerror = "External diff command not found in PATH"
+        raise err
+
       return _PopenStdoutWrapper(p)
 
     else:

Modified: subversion/trunk/subversion/bindings/swig/python/tests/fs.py
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/swig/python/tests/fs.py?rev=1912743&r1=1912742&r2=1912743&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/swig/python/tests/fs.py (original)
+++ subversion/trunk/subversion/bindings/swig/python/tests/fs.py Thu Oct  5 
07:36:08 2023
@@ -308,6 +308,9 @@ class SubversionFSTestCase(unittest.Test
     try:
       diffout, differr = Popen(["diff"], stdin=PIPE, stderr=PIPE).communicate()
 
+    # When removing Python 2 support: Change to FileNotFoundError and 
+    # remove check for ENOENT (FileNotFoundError "Corresponds to errno
+    # ENOENT" according to documentation)
     except OSError as err:
       if err.errno == errno.ENOENT:
         self.skipTest("'diff' command not present")


Reply via email to