Re: [PATCH v7 1/4] notes: document cat_sort_uniq rewriteMode

2015-08-15 Thread Johan Herland
On Sat, Aug 15, 2015 at 12:53 AM, Jacob Keller  wrote:
> On Fri, Aug 14, 2015 at 3:11 PM, Junio C Hamano  wrote:
>> Jacob Keller  writes:
>>
>>> diff --git a/Documentation/config.txt b/Documentation/config.txt
>>> index 75ec02e8e90a..de67ad1fdedf 100644
>>> --- a/Documentation/config.txt
>>> +++ b/Documentation/config.txt
>>> @@ -1947,8 +1947,8 @@ notes.rewriteMode::
>>>   When copying notes during a rewrite (see the
>>>   "notes.rewrite." option), determines what to do if
>>>   the target commit already has a note.  Must be one of
>>> - `overwrite`, `concatenate`, or `ignore`.  Defaults to
>>> - `concatenate`.
>>> + `overwrite`, `concatenate`, `cat_sort_uniq`, or `ignore`.
>>> + Defaults to `concatenate`.
>>>  +
>>>  This setting can be overridden with the `GIT_NOTES_REWRITE_MODE`
>>>  environment variable.
>>> diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
>>> index 851518d531b5..674682b34b83 100644
>>> --- a/Documentation/git-notes.txt
>>> +++ b/Documentation/git-notes.txt
>>> @@ -331,7 +331,8 @@ environment variable.
>>>  notes.rewriteMode::
>>>   When copying notes during a rewrite, what to do if the target
>>>   commit already has a note.  Must be one of `overwrite`,
>>> - `concatenate`, and `ignore`.  Defaults to `concatenate`.
>>> + `concatenate`, `cat_sort_uniq`, or `ignore`.  Defaults to
>>> + `concatenate`.
>>>  +
>>>  This setting can be overridden with the `GIT_NOTES_REWRITE_MODE`
>>>  environment variable.
>>
>> This obviously is not a problem introduced by this patch, but I
>> wonder why we have two similar but different set of modes for
>> rewrtie and merge.

We do. Rewrite builds directly on top of the combine_notes_* functions
that are part of the core/low-level notes code, added in 73f464b5
(2010-02-13, Refactor notes concatenation into a flexible interface for
combining notes). Notes merge (and its various strategies) were added
later, and also build on top of these combine_notes_* functions (see
merge_one_change() in notes-merge.c). In addition, notes-merge adds the
'manual' strategy, which depends on surrounding machinery that
notes-merge provides, but that the low-level combine_notes_* functions
cannot depend on.


>>  Isn't 'overwrite' like 'ours', 'ignore' like
>> 'theirs', and 'concat' like 'union', and if these are similar
>> enough, perhaps it would be helpful to the end user if we unified
>> the terms (or accepted both as synonyms for backward compatibility)?

The mapping (which is contained in merge_one_change() in notes-merge.c)
is:

  'manual'-> [custom handling in notes-merge]
  'ours'  -> combine_notes_ignore [or simply a no-op]
  'theirs'-> combine_notes_overwrite
  'union' -> combine_notes_concatenate
  'cat_sort_uniq' -> combine_notes_cat_sort_uniq

>>
>> Also I notice that you cannot manually reconcile while rewriting;
>> don't we want to have 'manual' there, too, I wonder?

No, as stated above, 'manual' requires some external mechanism for
creating a "worktree" where the notes conflicts can be checked out
and manipulated by the user. The guts of notes.c is not the correct
place to do that. I don't know if it's easy to rewrite "rewrite" to
do a (partial) notes merge instead of using the combine_notes_*
functions directly, but I imagine that would be the best way forward.

>>
>> [jc: Cc'ed Thomas who invented rewrite back when merge was not even
>> there, and Johan who added merge]
>>
>
> I was not sure. I believe that re-write doesn't do the same thing as
> merge? I think we could make all of them handle the "overwrite", which
> is basically a synonym of, I think "theirs" depending on the direction
> of the "merge".

Correct.

> I don't know if re-write actually supports manual mode at all!

It doesn't.

> Maybe we could make merge support the other names as synonyms, and
> then code re-write in terms of merging?

Adding the synonyms is fine, but I wouldn't publicize it heavily, as
the meaning of words like "overwrite" and "ignore" can become quite
confusing in the context of a merge.

Reimplementing re-write in terms of merge is probably a good idea,
though.

> I wasn't sure so I chose only to document the mode that was missing.

IMHO, that's good for now.


...Johan


-- 
Johan Herland, 
www.herland.net
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 1/4] notes: document cat_sort_uniq rewriteMode

2015-08-14 Thread Jacob Keller
On Fri, Aug 14, 2015 at 3:11 PM, Junio C Hamano  wrote:
> Jacob Keller  writes:
>
>> diff --git a/Documentation/config.txt b/Documentation/config.txt
>> index 75ec02e8e90a..de67ad1fdedf 100644
>> --- a/Documentation/config.txt
>> +++ b/Documentation/config.txt
>> @@ -1947,8 +1947,8 @@ notes.rewriteMode::
>>   When copying notes during a rewrite (see the
>>   "notes.rewrite." option), determines what to do if
>>   the target commit already has a note.  Must be one of
>> - `overwrite`, `concatenate`, or `ignore`.  Defaults to
>> - `concatenate`.
>> + `overwrite`, `concatenate`, `cat_sort_uniq`, or `ignore`.
>> + Defaults to `concatenate`.
>>  +
>>  This setting can be overridden with the `GIT_NOTES_REWRITE_MODE`
>>  environment variable.
>> diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
>> index 851518d531b5..674682b34b83 100644
>> --- a/Documentation/git-notes.txt
>> +++ b/Documentation/git-notes.txt
>> @@ -331,7 +331,8 @@ environment variable.
>>  notes.rewriteMode::
>>   When copying notes during a rewrite, what to do if the target
>>   commit already has a note.  Must be one of `overwrite`,
>> - `concatenate`, and `ignore`.  Defaults to `concatenate`.
>> + `concatenate`, `cat_sort_uniq`, or `ignore`.  Defaults to
>> + `concatenate`.
>>  +
>>  This setting can be overridden with the `GIT_NOTES_REWRITE_MODE`
>>  environment variable.
>
> This obviously is not a problem introduced by this patch, but I
> wonder why we have two similar but different set of modes for
> rewrtie and merge.  Isn't 'overwrite' like 'ours', 'ignore' like
> 'theirs', and 'concat' like 'union', and if these are similar
> enough, perhaps it would be helpful to the end user if we unified
> the terms (or accepted both as synonyms for backward compatibility)?
>
> Also I notice that you cannot manually reconcile while rewriting;
> don't we want to have 'manual' there, too, I wonder?
>
> [jc: Cc'ed Thomas who invented rewrite back when merge was not even
> there, and Johan who added merge]
>

I was not sure. I believe that re-write doesn't do the same thing as
merge? I think we could make all of them handle the "overwrite", which
is basically a synonym of, I think "theirs" depending on the direction
of the "merge".

I don't know if re-write actually supports manual mode at all!

Maybe we could make merge support the other names as synonyms, and
then code re-write in terms of merging?

I wasn't sure so I chose only to document the mode that was missing.

Regards,
Jake
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 1/4] notes: document cat_sort_uniq rewriteMode

2015-08-14 Thread Junio C Hamano
Jacob Keller  writes:

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 75ec02e8e90a..de67ad1fdedf 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1947,8 +1947,8 @@ notes.rewriteMode::
>   When copying notes during a rewrite (see the
>   "notes.rewrite." option), determines what to do if
>   the target commit already has a note.  Must be one of
> - `overwrite`, `concatenate`, or `ignore`.  Defaults to
> - `concatenate`.
> + `overwrite`, `concatenate`, `cat_sort_uniq`, or `ignore`.
> + Defaults to `concatenate`.
>  +
>  This setting can be overridden with the `GIT_NOTES_REWRITE_MODE`
>  environment variable.
> diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
> index 851518d531b5..674682b34b83 100644
> --- a/Documentation/git-notes.txt
> +++ b/Documentation/git-notes.txt
> @@ -331,7 +331,8 @@ environment variable.
>  notes.rewriteMode::
>   When copying notes during a rewrite, what to do if the target
>   commit already has a note.  Must be one of `overwrite`,
> - `concatenate`, and `ignore`.  Defaults to `concatenate`.
> + `concatenate`, `cat_sort_uniq`, or `ignore`.  Defaults to
> + `concatenate`.
>  +
>  This setting can be overridden with the `GIT_NOTES_REWRITE_MODE`
>  environment variable.

This obviously is not a problem introduced by this patch, but I
wonder why we have two similar but different set of modes for
rewrtie and merge.  Isn't 'overwrite' like 'ours', 'ignore' like
'theirs', and 'concat' like 'union', and if these are similar
enough, perhaps it would be helpful to the end user if we unified
the terms (or accepted both as synonyms for backward compatibility)?

Also I notice that you cannot manually reconcile while rewriting;
don't we want to have 'manual' there, too, I wonder?

[jc: Cc'ed Thomas who invented rewrite back when merge was not even
there, and Johan who added merge]

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v7 1/4] notes: document cat_sort_uniq rewriteMode

2015-08-14 Thread Jacob Keller
From: Jacob Keller 

Teach documentation about the cat_sort_uniq rewriteMode that got added
at the same time as the equivalent merge strategy.

Signed-off-by: Jacob Keller 
---
 Documentation/config.txt| 4 ++--
 Documentation/git-notes.txt | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 75ec02e8e90a..de67ad1fdedf 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1947,8 +1947,8 @@ notes.rewriteMode::
When copying notes during a rewrite (see the
"notes.rewrite." option), determines what to do if
the target commit already has a note.  Must be one of
-   `overwrite`, `concatenate`, or `ignore`.  Defaults to
-   `concatenate`.
+   `overwrite`, `concatenate`, `cat_sort_uniq`, or `ignore`.
+   Defaults to `concatenate`.
 +
 This setting can be overridden with the `GIT_NOTES_REWRITE_MODE`
 environment variable.
diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 851518d531b5..674682b34b83 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -331,7 +331,8 @@ environment variable.
 notes.rewriteMode::
When copying notes during a rewrite, what to do if the target
commit already has a note.  Must be one of `overwrite`,
-   `concatenate`, and `ignore`.  Defaults to `concatenate`.
+   `concatenate`, `cat_sort_uniq`, or `ignore`.  Defaults to
+   `concatenate`.
 +
 This setting can be overridden with the `GIT_NOTES_REWRITE_MODE`
 environment variable.
-- 
2.5.0.280.g4aaba03

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html