Merge lp:~ace17/bzr/fixDiff into lp:bzr

Proposed by Sebastien Alaiwan on 2014-06-11
Status: Needs review
Proposed branch: lp:~ace17/bzr/fixDiff
Merge into: lp:bzr
Diff against target: 31 lines (+10/-9)
1 file modified
bzrlib/diff.py (+10/-9)
To merge this branch: bzr merge lp:~ace17/bzr/fixDiff
Reviewer Review Type Date Requested Status
Richard Wilbur 2014-06-11 Needs Information on 2014-06-12
Review via email: mp+222864@code.launchpad.net

Description of the Change

Fixes a bug causing --diff-options being passed to the external GNU diff program when a custom diff tool has been specified.

ace@ANTEC bzr-test % bzr status -V
added:
  File1
modified:
  File2
ace@ANTEC bzr-test % bzr diff --using gvimdiff --diff-options "-f"
=== added file 'File1'
diff: conflicting output style options
diff: Try `diff --help' for more information.

bzr: ERROR: external diff failed with exit code 2; command: ['diff', '--label', u'File1\t1970-01-01 00:00:00 +0000', '/tmp/bzr-diff-old-x6Y5up', '--label', u'File1\t2014-06-11 17:22:29 +0000', '/tmp/bzr-diff-new-MNV8Ut', '--binary', '-u', '-f']

To post a comment you must log in.
Richard Wilbur (richard-wilbur) wrote :

I'd be happy to review this merge proposal but when I look at the web page it says:
    Updating diff...

    An updated diff will be available in a few minutes. Reload to see the changes.

I tried reloading the page ≥5 minutes later with identical results.

Next I checked the web page for the branch as I can generally look at the diff's for each revision and understand what changes are entailed. Surprisingly it said the branch is empty with no revisions!

I'm not sure what happened to cause this situation. My normal recipe for creating a branch is as follows:
bzr branch lp:bzr
cd bzr
[edit the file(s) with your favourite editor]
bzr commit -m 'Fix external diff....'
bzr push lp:~ace17/bzr/fixDiff

Then visit here:
https://code.launchpad.net/~ace17/bzr/fixDiff/+register-merge

Fill out the form and submit.

review: Needs Information
Sebastien Alaiwan (ace17) wrote :

Hi, thanks for your reply.
Here's how I did:

bzr branch --stacked lp:bzr
cd bzr
[edit the file(s) with your favourite editor]
bzr commit -m 'Fix external diff....'
bzr push lp:~ace17/bzr/fixDiff

I received a message from launchpad:
"Launchpad encountered an internal error during the following operation: generating the diff for a merge proposal. It was logged with id OOPS-8239c35b3d57eabeca064b77c3513d04. Sorry for the inconvenience."

I use "--stacked" because the whole bzr repository is very long to download. This might have cause the issue. I'm retrying to recreate the whole thing ('bzr reconfigure --unstacked' seems to have no effect)

Sebastien Alaiwan (ace17) wrote :

OK, i've created a new branch (which seems to contain revisions this time!).
Richard, once you've read these two messages I'll delete this broken merge proposal. Let's continue the discussion here: https://code.launchpad.net/~ace17/bzr/fixDiff4/+merge/222900

Richard Wilbur (richard-wilbur) wrote :

Sebastien, thanks for creating one that works. I'm sorry that the
"stacked" branch didn't work. I have never tried a "stacked" branch
so I hadn't previously encountered that problem. To be continued on
the new branch.

lp:~ace17/bzr/fixDiff updated on 2014-09-24
6599. By Sebastien Alaiwan on 2014-09-24

Merge

Unmerged revisions

6599. By Sebastien Alaiwan on 2014-09-24

Merge

6598. By Sebastien Alaiwan on 2014-06-11

Avoid calling 'external_diff' with options passed to --diff-options if --using is specified. Fixes the following from failing 'bzr add somefile.txt ; bzr diff --using gvimdiff --diff-options '-f -n -d'

Updating diff...

An updated diff will be available in a few minutes. Reload to see the changes.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bzrlib/diff.py'
2--- bzrlib/diff.py 2012-07-24 16:56:07 +0000
3+++ bzrlib/diff.py 2014-06-12 05:05:27 +0000
4@@ -930,17 +930,18 @@
5 """
6 if using is not None:
7 extra_factories = [DiffFromTool.make_from_diff_tree(using, external_diff_options)]
8+ diff_file = internal_diff
9 else:
10 extra_factories = []
11- if external_diff_options:
12- opts = external_diff_options.split()
13- def diff_file(olab, olines, nlab, nlines, to_file, path_encoding=None, context_lines=None):
14- """:param path_encoding: not used but required
15- to match the signature of internal_diff.
16- """
17- external_diff(olab, olines, nlab, nlines, to_file, opts)
18- else:
19- diff_file = internal_diff
20+ if external_diff_options:
21+ opts = external_diff_options.split()
22+ def diff_file(olab, olines, nlab, nlines, to_file, path_encoding=None, context_lines=None):
23+ """:param path_encoding: not used but required
24+ to match the signature of internal_diff.
25+ """
26+ external_diff(olab, olines, nlab, nlines, to_file, opts)
27+ else:
28+ diff_file = internal_diff
29 diff_text = DiffText(old_tree, new_tree, to_file, path_encoding,
30 old_label, new_label, diff_file, context_lines=context_lines)
31 return klass(old_tree, new_tree, to_file, path_encoding, diff_text,