bzr send should offer the --diff-options option

Bug #449923 reported by David Strauss
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Bazaar
Invalid
Wishlist
Unassigned

Bug Description

This is necessary for consistency with bzr diff.

Revision history for this message
Aaron Bentley (abentley) wrote :

bzr merge validates that the diff is a true representation of the requested change, by generating its own diff and ensuring it matches. Changing the format of the diff in bzr send would make the diff fail validation.

Using --diff-options also causes an external program to be invoked, which may have output variations across different platforms, so that even if the same diff options were applied when merging, the chance of a spurious validation failure is higher. Providing metadata to reproduce the diff options would likely require a new merge directive format.

Changed in bzr:
status: New → Invalid
Revision history for this message
Andrew Bennetts (spiv) wrote :

A couple of thoughts:

- Some --diff-options, like -p, would produce semantically equivalent diffs that bzr would parse ok. So if bzr generated the diff with --diff-options and then checked that it still validates like a regular diff would, then that could be useful.

- Not all uses of 'bzr send' are intended to preserve perfect fidelity. e.g. a bzr send of a cherry-pick to an upstream that doesn't use bzr, you may as well just send a merge directive with no bundle, and as the target isn't bzr validation isn't a concern.

That said, I'm not sure that it would be worth the effort to make this work well and predictably. I just wanted to point out that I do see some merit in the idea. We'd need a stronger motivation than "to be consistent with 'bzr diff'" though...

David, did you have a specific use-case in mind? It would be good to know why you wanted this so that at the least we can consider how best to accomodate if/when we next revise the merge directive format. (It might be more useful to post that to the mailing list rather than this bug though.)

Andrew Bennetts (spiv)
Changed in bzr:
importance: Undecided → Wishlist
status: Invalid → Confirmed
Revision history for this message
Aaron Bentley (abentley) wrote : Re: [Bug 449923] Re: bzr send should offer the --diff-options option

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Andrew Bennetts wrote:
> A couple of thoughts:
>
> - Some --diff-options, like -p, would produce semantically equivalent
> diffs that bzr would parse ok. So if bzr generated the diff with
> --diff-options and then checked that it still validates like a regular
> diff would, then that could be useful.

The validation is text-based. If you don't generate the same text as
bzr itself would generate, validation will fail. It doesn't matter how
it parses. If we did use the parser to parse a -p-style diff, it would
fail, because our parser only parses vanilla unified diffs. If we
changed the parser to accept -p-style-diffs, it should parse the
information about function context and roundtrip it. If we changed the
parser to parse function context, then it should not consider two
patches equal that have different information about function context.

Finally, such merge directives would be incompatible with common
versions of bzr.

Simply adding support for --diff-options without revising the merge
directive format does not make sense.

> - Not all uses of 'bzr send' are intended to preserve perfect fidelity.
> e.g. a bzr send of a cherry-pick to an upstream that doesn't use bzr,
> you may as well just send a merge directive with no bundle

This doesn't really follow. When a bundle is omitted, then bzr uses the
specified branches instead. You still get perfect fidelity.

I think you mean a situation where the merge directive is applied as a
patch. But if want to provide a cherry-pick to an upstream that doesn't
use bzr, you should send a patch.

>, and as the
> target isn't bzr validation isn't a concern.

In any context where bzr data might be used rather than the patch, we
need to ensure that the patch is an honest representation of the merge
directive.

In any context where bzr data will not be used, we should not send the
bzr data-- we should just send a patch.

There are no cases where bzr send must be used but perfect fidelity need
not be maintained.

 status invalid
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkrX/BsACgkQ0F+nu1YWqI1QxwCePIvYsCxDskNAUvm+cKKA1BAb
zKcAn2XEe4pOp2aPEzjc1oEC4+CZFX16
=Y5r+
-----END PGP SIGNATURE-----

Changed in bzr:
status: Confirmed → Invalid
Revision history for this message
Aaron Bentley (abentley) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Aaron Bentley wrote:
> Andrew Bennetts wrote:
>> A couple of thoughts:
>
>> - Some --diff-options, like -p, would produce semantically equivalent
>> diffs that bzr would parse ok. So if bzr generated the diff with
>> --diff-options and then checked that it still validates like a regular
>> diff would, then that could be useful.
>
> The validation is text-based. If you don't generate the same text as
> bzr itself would generate, validation will fail. It doesn't matter how
> it parses. If we did use the parser to parse a -p-style diff, it would
> fail, because our parser only parses vanilla unified diffs.

It's also worth mentioning that while the diffs would describe the same
result, they would not describe the delta in the same way, because
/usr/bin/diff doesn't use the patience algorithm for matching.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkrYuM0ACgkQ0F+nu1YWqI3DqACeKlxqIiicOjOoO+aWMPYowU0o
FtwAn0SmEfPOjqd6qhFhJwG4cj3MVgAm
=yCn6
-----END PGP SIGNATURE-----

Revision history for this message
Robert Collins (lifeless) wrote :

Aaron, I'm confused why you're saying that this bug is invalid:
technically challenging for us, sure, but is it really 'not a bug', or
even 'something we wouldn't accept patches for' ?

On the 'the diffs won't match' aspect, I can imagine us offering the
user an interdiff when the diff isn't identical, and letting them
confirm that its ok, which would work tolerably without a bundle format
change.

And as you note, a bundle format change would permit the use of
different options when generating the bundle.

We can offer a some of the --diff-options stuff ourselves to eliminate
per-platform differences in diff.

-Rob

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.