librelist archives

« back to archive

Patches and --git

Patches and --git

From:
John Peberdy
Date:
2012-05-22 @ 00:46
In the changeset that the --git flag was blacklisted Martin said the
intention was for commands to insert --git themselves by default.
Would anyone be opposed to having --git inserted unconditionally for
export and diff?

We can probably also blacklist the --text option.

Re: [javahg] Patches and --git

From:
Jan Sorensen
Date:
2012-05-22 @ 12:31
On Tue, May 22, 2012 at 2:46 AM, John Peberdy <johnpeb@gmail.com> wrote:
> In the changeset that the --git flag was blacklisted Martin said the
> intention was for commands to insert --git themselves by default.
> Would anyone be opposed to having --git inserted unconditionally for
> export and diff?

I don't understand Martin's argument. The diff and export commands
just return the raw output from the native hg commands, I don't see
the git flag is blacklisted. The client of the commands should decide
the format and parse it accordingly. Maybe I miss something?

Ideally the commands should return some kind of 'DiffResult' that
should have a convenient interface to access the diff info.

I would like to better understand the reason for the blacklisting of
the git flag before making an opinion.

> We can probably also blacklist the --text option.

Why? This is related to input, why should we restrict the caller to
use the text flag?

/Jan

Re: [javahg] Patches and --git

From:
John Peberdy
Date:
2012-05-22 @ 12:57
Hi Jan,

On Tue, May 22, 2012 at 8:31 AM, Jan Sorensen <sorensenjan@gmail.com> wrote:
> On Tue, May 22, 2012 at 2:46 AM, John Peberdy <johnpeb@gmail.com> wrote:
>> In the changeset that the --git flag was blacklisted Martin said the
>> intention was for commands to insert --git themselves by default.
>> Would anyone be opposed to having --git inserted unconditionally for
>> export and diff?
>
> I don't understand Martin's argument. The diff and export commands
> just return the raw output from the native hg commands, I don't see
> the git flag is blacklisted. The client of the commands should decide
> the format and parse it accordingly. Maybe I miss something?
>
> Ideally the commands should return some kind of 'DiffResult' that
> should have a convenient interface to access the diff info.
>
> I would like to better understand the reason for the blacklisting of
> the git flag before making an opinion.

I think the reason is not having --git there results in data loss -
patches containing binary files do not have diffs generated for them.
--text is similar in that it also generates diffs for binary files,
but in a way that's not as nice as the git format.

In MercurialEclipse --git is always added by default and I don't know
if anyone would miss it if they lost the ability to not use git
format.

>
>> We can probably also blacklist the --text option.
>
> Why? This is related to input, why should we restrict the caller to
> use the text flag?

I guess we don't want to blacklist it universally then. For diff its
related to whether or not diffs are generated for binary files and is
redundant if --git is supplied also.

>
> /Jan



-- 
John Peberdy

Re: [javahg] Patches and --git

From:
Martin Geisler
Date:
2012-05-22 @ 16:44
John Peberdy <johnpeb@gmail.com> writes:

> Hi Jan,
>
> On Tue, May 22, 2012 at 8:31 AM, Jan Sorensen <sorensenjan@gmail.com> wrote:
>> On Tue, May 22, 2012 at 2:46 AM, John Peberdy <johnpeb@gmail.com> wrote:
>>> In the changeset that the --git flag was blacklisted Martin said the
>>> intention was for commands to insert --git themselves by default.
>>> Would anyone be opposed to having --git inserted unconditionally for
>>> export and diff?
>>
>> I don't understand Martin's argument. The diff and export commands
>> just return the raw output from the native hg commands, I don't see
>> the git flag is blacklisted.

It was blacklisted in changeset 79c5ac90e1aa:

  https://bitbucket.org/aragost/javahg/changeset/79c5ac90e1aa

(Just adding a link to make sure we're talking about the same thing and
so others don't have to dig it up themselves.)

>> The client of the commands should decide the format and parse it
>> accordingly. Maybe I miss something?
>>
>> Ideally the commands should return some kind of 'DiffResult' that
>> should have a convenient interface to access the diff info.
>>
>> I would like to better understand the reason for the blacklisting of
>> the git flag before making an opinion.
>
> I think the reason is not having --git there results in data loss -
> patches containing binary files do not have diffs generated for them.
> --text is similar in that it also generates diffs for binary files,
> but in a way that's not as nice as the git format.

The non-git format diff you get with --text is not guaranteed to apply
again. It's kind-of for humans only. I think --text is mostly used when
you want to diff a UTF-16 file which contain lots of NUL bytes, despite
being a text file. With --text you can at least get something that your
UTF-16 terminal can attempt to interpret (though the diff output will be
a mix between ASCII and UTF-16).

I just tried this here on my Linux machine:

  $ hg init
  $ echo abc | recode UTF-16 > a
  $ hexdump a
  0000000 fffe 6100 6200 6300 0a00
  000000a
  $ hg add a
  $ hg commit -m first
  $ echo abcxyz | recode ASCII..UTF-16 > a
  $ hg diff --config diff.git=no --text > a.patch
  $ hg revert --all
  reverting a
  $ hg import --no-commit a.patch
  applying a.patch
  $ ls -l
  total 12
  -rw-r--r-- 1 mg mg   2 May 22 18:38 a
  -rw-r--r-- 1 mg mg  16 May 22 18:37 a.orig
  -rw-r--r-- 1 mg mg 145 May 22 18:37 a.patch

So the a file was 16 bytes before (BOM + 7 characters) and is now only 2
bytes. A hexdump shows that just the BOM remains:

  $ hexdump a
  0000000 fffe
  0000002

So I think the --text flag is close to useless for an API.

> In MercurialEclipse --git is always added by default and I don't know
> if anyone would miss it if they lost the ability to not use git
> format.

Yeah, I think that is what I thought too... :) The --git flag is not on
by default in Mercurial because Matt insists that 'hg diff' should be
compatible with normal patch(1) -- we don't have that constraint and so
we can prevent data loss by turning it on by default.

>>> We can probably also blacklist the --text option.
>>
>> Why? This is related to input, why should we restrict the caller to
>> use the text flag?
>
> I guess we don't want to blacklist it universally then. For diff its
> related to whether or not diffs are generated for binary files and is
> redundant if --git is supplied also.

-- 
Martin Geisler

aragost Trifork
Commercial Mercurial support
http://aragost.com/mercurial/

Re: [javahg] Patches and --git

From:
John Peberdy
Date:
2012-05-23 @ 01:15
On Tue, May 22, 2012 at 12:44 PM, Martin Geisler <mg@aragost.com> wrote:
> John Peberdy <johnpeb@gmail.com> writes:
>
>> Hi Jan,
>>
>> On Tue, May 22, 2012 at 8:31 AM, Jan Sorensen <sorensenjan@gmail.com> wrote:
>>> On Tue, May 22, 2012 at 2:46 AM, John Peberdy <johnpeb@gmail.com> wrote:
>>>> In the changeset that the --git flag was blacklisted Martin said the
>>>> intention was for commands to insert --git themselves by default.
>>>> Would anyone be opposed to having --git inserted unconditionally for
>>>> export and diff?
>>>
>>> I don't understand Martin's argument. The diff and export commands
>>> just return the raw output from the native hg commands, I don't see
>>> the git flag is blacklisted.
>
> It was blacklisted in changeset 79c5ac90e1aa:
>
>  https://bitbucket.org/aragost/javahg/changeset/79c5ac90e1aa
>
> (Just adding a link to make sure we're talking about the same thing and
> so others don't have to dig it up themselves.)
>
>>> The client of the commands should decide the format and parse it
>>> accordingly. Maybe I miss something?
>>>
>>> Ideally the commands should return some kind of 'DiffResult' that
>>> should have a convenient interface to access the diff info.
>>>
>>> I would like to better understand the reason for the blacklisting of
>>> the git flag before making an opinion.
>>
>> I think the reason is not having --git there results in data loss -
>> patches containing binary files do not have diffs generated for them.
>> --text is similar in that it also generates diffs for binary files,
>> but in a way that's not as nice as the git format.
>
> The non-git format diff you get with --text is not guaranteed to apply
> again. It's kind-of for humans only. I think --text is mostly used when
> you want to diff a UTF-16 file which contain lots of NUL bytes, despite
> being a text file. With --text you can at least get something that your
> UTF-16 terminal can attempt to interpret (though the diff output will be
> a mix between ASCII and UTF-16).
>
> I just tried this here on my Linux machine:
>
>  $ hg init
>  $ echo abc | recode UTF-16 > a
>  $ hexdump a
>  0000000 fffe 6100 6200 6300 0a00
>  000000a
>  $ hg add a
>  $ hg commit -m first
>  $ echo abcxyz | recode ASCII..UTF-16 > a
>  $ hg diff --config diff.git=no --text > a.patch
>  $ hg revert --all
>  reverting a
>  $ hg import --no-commit a.patch
>  applying a.patch
>  $ ls -l
>  total 12
>  -rw-r--r-- 1 mg mg   2 May 22 18:38 a
>  -rw-r--r-- 1 mg mg  16 May 22 18:37 a.orig
>  -rw-r--r-- 1 mg mg 145 May 22 18:37 a.patch
>
> So the a file was 16 bytes before (BOM + 7 characters) and is now only 2
> bytes. A hexdump shows that just the BOM remains:
>
>  $ hexdump a
>  0000000 fffe
>  0000002
>
> So I think the --text flag is close to useless for an API.
>
>> In MercurialEclipse --git is always added by default and I don't know
>> if anyone would miss it if they lost the ability to not use git
>> format.
>
> Yeah, I think that is what I thought too... :) The --git flag is not on
> by default in Mercurial because Matt insists that 'hg diff' should be
> compatible with normal patch(1) -- we don't have that constraint and so
> we can prevent data loss by turning it on by default.

Thanks for the guidance. I pushed
https://bitbucket.org/aragost/javahg/changeset/ea9b15c40dac

>>>> We can probably also blacklist the --text option.
>>>
>>> Why? This is related to input, why should we restrict the caller to
>>> use the text flag?
>>
>> I guess we don't want to blacklist it universally then. For diff its
>> related to whether or not diffs are generated for binary files and is
>> redundant if --git is supplied also.
>
> --
> Martin Geisler
>
> aragost Trifork
> Commercial Mercurial support
> http://aragost.com/mercurial/



-- 
John Peberdy

Re: [javahg] Patches and --git

From:
Martin Geisler
Date:
2012-05-23 @ 09:26
John Peberdy <johnpeb@gmail.com> writes:

> On Tue, May 22, 2012 at 12:44 PM, Martin Geisler <mg@aragost.com> wrote:
>> John Peberdy <johnpeb@gmail.com> writes:
>>
>>> In MercurialEclipse --git is always added by default and I don't
>>> know if anyone would miss it if they lost the ability to not use git
>>> format.
>>
>> Yeah, I think that is what I thought too... :) The --git flag is not
>> on by default in Mercurial because Matt insists that 'hg diff' should
>> be compatible with normal patch(1) -- we don't have that constraint
>> and so we can prevent data loss by turning it on by default.
>
> Thanks for the guidance. I pushed
> https://bitbucket.org/aragost/javahg/changeset/ea9b15c40dac

Nice! I've added a small followup where I set the --git flag in the
constructor instead of the execute methods -- that way the flag wont
accumulate for each execution.

-- 
Martin Geisler

aragost Trifork
Commercial Mercurial support
http://aragost.com/mercurial/