Uploaded image for project: 'Subversion'
  1. Subversion
  2. SVN-2044

Fully customizable external diff invocations

    Details

    • Type: Improvement
    • Status: Open
    • Priority: Major
    • Resolution: Unresolved
    • Affects Version/s: all
    • Fix Version/s: unscheduled
    • Component/s: libsvn_subr
    • Labels:
      None

      Description

      Currently we have hardcoded arguments (mainly -L) in svn_io_run_diff and
      svn_io_run_diff3. This is bad, because arbitrary external diff programs won't
      understand these.
      
      People can work around it with wrapper scripts, but they should not have to.
      
      Instead, we need to be able to fully customize our diff invocations - including
      subsititutions for the labels and filenames.
      

        Issue Links

          Activity

          Hide
          breser Ben Reser added a comment -

          *** Issue 2074 has been marked as a duplicate of this issue. ***
          

          Show
          breser Ben Reser added a comment - *** Issue 2074 has been marked as a duplicate of this issue. ***
          Hide
          subversion-importer Subversion Importer added a comment -

          To me, it seems broken to ignore --extensions '' (empty value).
          
          It also feels wrong to support external diff programs, so long as they're GNU diff or option-compatible 
          with it, and make GNU diff behave just like the built-in differ anyway.
          

          Original comment by reppep

          Show
          subversion-importer Subversion Importer added a comment - To me, it seems broken to ignore --extensions '' (empty value). It also feels wrong to support external diff programs, so long as they're GNU diff or option-compatible with it, and make GNU diff behave just like the built-in differ anyway. Original comment by reppep
          Hide
          kfogel Karl Fogel added a comment -

          Changing this to enhancement, instead of defect, and putting into the '2.0'
          milestone, as I'm not sure we'd want to change the calling convention in the 1.x
          lines, due to compatibility concerns.
          

          Show
          kfogel Karl Fogel added a comment - Changing this to enhancement, instead of defect, and putting into the '2.0' milestone, as I'm not sure we'd want to change the calling convention in the 1.x lines, due to compatibility concerns.
          Hide
          maxb Max Bowsher added a comment -

          Taking back *out* of 2.0, since there is no reason we can't do this in 1.x, if
          we do it via a new set of APIs (deprecating svn_io_run_diff and
          svn_io_run_diff3), and enable it in response to a new config file option.
          

          Show
          maxb Max Bowsher added a comment - Taking back *out* of 2.0, since there is no reason we can't do this in 1.x, if we do it via a new set of APIs (deprecating svn_io_run_diff and svn_io_run_diff3), and enable it in response to a new config file option.
          Hide
          maxb Max Bowsher added a comment -

          *** Issue 1390 has been marked as a duplicate of this issue. ***
          

          Show
          maxb Max Bowsher added a comment - *** Issue 1390 has been marked as a duplicate of this issue. ***
          Hide
          cmpilato C. Michael Pilato added a comment -

          See this thread for some related thoughts on getting our external diff/diff3
          functionality up to par:  http://svn.haxx.se/dev/archive-2005-07/1334.shtml
          

          Show
          cmpilato C. Michael Pilato added a comment - See this thread for some related thoughts on getting our external diff/diff3 functionality up to par: http://svn.haxx.se/dev/archive-2005-07/1334.shtml
          Hide
          cmpilato C. Michael Pilato added a comment -

          *** Issue 2484 has been marked as a duplicate of this issue. ***
          

          Show
          cmpilato C. Michael Pilato added a comment - *** Issue 2484 has been marked as a duplicate of this issue. ***
          Hide
          dlr Daniel Rall added a comment -

          Supporting diffing and merging of "unusual file types" was suggested here (e.g.
          external diff/merge program chosen by target's MIME type):
          http://svn.haxx.se/dev/archive-2007-03/1199.shtml
          
          Improving usability via better argument passing was suggested here:
          http://svn.haxx.se/dev/archive-2007-03/1210.shtml
          
          
          

          Show
          dlr Daniel Rall added a comment - Supporting diffing and merging of "unusual file types" was suggested here (e.g. external diff/merge program chosen by target's MIME type): http://svn.haxx.se/dev/archive-2007-03/1199.shtml Improving usability via better argument passing was suggested here: http://svn.haxx.se/dev/archive-2007-03/1210.shtml
          Hide
          kfogel Karl Fogel added a comment -

          Issue #2447 has been filed for invocation of arbitrary diff/merge programs on
          arbitrary (e.g., non-text) file types. 
          

          Show
          kfogel Karl Fogel added a comment - Issue #2447 has been filed for invocation of arbitrary diff/merge programs on arbitrary (e.g., non-text) file types.
          Hide
          subversion-importer Subversion Importer added a comment -

          I would very much like a simpler way to do the equivalent of 
            cvs diff -up > something.patch.
          The equivalent for svn 
            svn diff --diff-cmd=/usr/bin/diff -up . > something.patch
          is so long and complicated that people can't be expected to use it, so we
          (GNOME) get patches that are difficult to review because they don't mention
          function name.
          

          Original comment by murrayc

          Show
          subversion-importer Subversion Importer added a comment - I would very much like a simpler way to do the equivalent of cvs diff -up > something.patch. The equivalent for svn svn diff --diff-cmd=/usr/bin/diff -up . > something.patch is so long and complicated that people can't be expected to use it, so we (GNOME) get patches that are difficult to review because they don't mention function name. Original comment by murrayc
          Hide
          kfogel Karl Fogel added a comment -

          Murray, this issue is the wrong place to bring that up -- it's not really the
          same topic.  Please post to users@subversion.tigris.org instead.  This issue is
          about ensuring that external diff invocation is fully customizable; your problem
          is about user interface, and has nothing to do with limits on extensibility.
          

          Show
          kfogel Karl Fogel added a comment - Murray, this issue is the wrong place to bring that up -- it's not really the same topic. Please post to users@subversion.tigris.org instead. This issue is about ensuring that external diff invocation is fully customizable; your problem is about user interface, and has nothing to do with limits on extensibility.
          Hide
          subversion-importer Subversion Importer added a comment -

          Since this bug is tracking enhancements to diff-cmd, I have request to 1)
          optionally omit printing the header output (Index: , ==== lines) before running
          the diff-cmd.  2) Provide a way to pass those header lines as argument(s) to the
          external diff-cmd.
          
          The problem is that internal and external diff don't behave the same--using
          external diff may cause svn to print out those aforementioned headers even if
          there is no external diff output.  See this example:
          
          $echo "1 a" > a
          $svn add a
          A         a
          $svn ci -m test
          Adding         a
          Transmitting file data .
          Committed revision 6.
          
          # add some whitespace:
          $echo "1  a" >! a
          $svn status
          M      a
          
          # both internal and external diff-cmd show the same output:
          $svn diff
          Index: a
          ===================================================================
          --- a   (revision 6)
          +++ a   (working copy)
          @@ -1 +1 @@
          -1 a
          +1  a
          $svn diff --diff-cmd=/usr/bin/diff
          Index: a
          ===================================================================
          --- a   (revision 6)
          +++ a   (working copy)
          @@ -1 +1 @@
          -1 a
          +1  a
          
          # now tell diff to ignore changes in whitespace.
          # internal diff shows no output, external diff shows the header:
          $svn diff -x -w
          #svn diff --diff-cmd=/usr/bin/diff -x -w
          Index: a
          ===================================================================
          $
          
          There is no easy way to fix this in the current sources.  libsvn_client/diff.c:
           diff_content_changed() conditionally prints the header for the internal-diff
          case, and unconditionally prints it for the external diff-cmd case.  The only
          way to fix that would be to buffer the external diff-cmd output into a file or
          memory, then check the exit status of the diff-cmd and use that to choose
          whether to print the header or not before printing the buffered output.
          
          As an alternative, I propose that an option be created to suppress the diff
          header lines completely--then at least you could use a wrapper script for
          diff-cmd that operated as desired.  Even better would be providing a way to pass
          these header lines to the external diff-cmd so it could do with them as it pleases.
          
          

          Original comment by cra

          Show
          subversion-importer Subversion Importer added a comment - Since this bug is tracking enhancements to diff-cmd, I have request to 1) optionally omit printing the header output (Index: , ==== lines) before running the diff-cmd. 2) Provide a way to pass those header lines as argument(s) to the external diff-cmd. The problem is that internal and external diff don't behave the same--using external diff may cause svn to print out those aforementioned headers even if there is no external diff output. See this example: $echo "1 a" > a $svn add a A a $svn ci -m test Adding a Transmitting file data . Committed revision 6. # add some whitespace: $echo "1 a" >! a $svn status M a # both internal and external diff-cmd show the same output: $svn diff Index: a =================================================================== --- a (revision 6) +++ a (working copy) @@ -1 +1 @@ -1 a +1 a $svn diff --diff-cmd=/usr/bin/diff Index: a =================================================================== --- a (revision 6) +++ a (working copy) @@ -1 +1 @@ -1 a +1 a # now tell diff to ignore changes in whitespace. # internal diff shows no output, external diff shows the header: $svn diff -x -w #svn diff --diff-cmd=/usr/bin/diff -x -w Index: a =================================================================== $ There is no easy way to fix this in the current sources. libsvn_client/diff.c: diff_content_changed() conditionally prints the header for the internal-diff case, and unconditionally prints it for the external diff-cmd case. The only way to fix that would be to buffer the external diff-cmd output into a file or memory, then check the exit status of the diff-cmd and use that to choose whether to print the header or not before printing the buffered output. As an alternative, I propose that an option be created to suppress the diff header lines completely--then at least you could use a wrapper script for diff-cmd that operated as desired. Even better would be providing a way to pass these header lines to the external diff-cmd so it could do with them as it pleases. Original comment by cra

            People

            • Assignee:
              Unassigned
              Reporter:
              maxb Max Bowsher
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:

                Development