Re: [PATCH v4 4/4] notes: teach git-notes about notes.<ref>.merge option

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH v4 4/4] notes: teach git-notes about notes.<ref>.merge option

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:06:10

Jacob Keller [off-list ref] writes:
I spoke to soon. We have an "init_notes_check" function which shows
that it does refuse to merge outside of refs/notes/* It prevents all
notes operations outside of refs/notes
OK.  Then it is OK to limit notes.<ref>.mergestrategy so that <ref>
refers to what comes after refs/notes/, because we will not allow
merging to happen outside the hierarchy.

If you are planning to break that promise, however, <ref> must be
always spelled fully (i.e. with refs/notes/ prefix for those inside
the hierarchy) to avoid ambiguity.  Otherwise it will be hard to
interpret a configuration that does something like this (note that
these could come from multiple places, e.g. $HOME/.gitconfig and
$GIT_DIR/config):

    [notes "commits"]
        mergestrategy = concatenate
    [notes "notes/commits"]
        mergestrategy = cat_sort_uniq
    [notes "refs/notes/commits"]
        mergestrategy = overwrite

The three entries in the above example obviously are all meant to
refer to the same refs/notes/commits notes tree, and the usual "last
one wins" rule should apply.  But with the recent git_config_get_*()
interface, you cannot tell which one among them was given the last,
overriding the previous entries.

Re: [PATCH v4 4/4] notes: teach git-notes about notes.<ref>.merge option

From: Jacob Keller <hidden>
Date: 2016-06-15 23:06:10

On Wed, Aug 12, 2015 at 3:41 PM, Junio C Hamano [off-list ref] wrote:
Jacob Keller [off-list ref] writes:
quoted
I spoke to soon. We have an "init_notes_check" function which shows
that it does refuse to merge outside of refs/notes/* It prevents all
notes operations outside of refs/notes
OK.  Then it is OK to limit notes.<ref>.mergestrategy so that <ref>
refers to what comes after refs/notes/, because we will not allow
merging to happen outside the hierarchy.

If you are planning to break that promise, however, <ref> must be
always spelled fully (i.e. with refs/notes/ prefix for those inside
the hierarchy) to avoid ambiguity.  Otherwise it will be hard to
interpret a configuration that does something like this (note that
these could come from multiple places, e.g. $HOME/.gitconfig and
$GIT_DIR/config):
Agreed. Today, we do not allow any notes operations at all that
function outside of refs/notes/*

I suggest we enforce that all configs for merge strategy must be the
unqualified notation, and not allow the variance of refs/notes/* and
such that DWIM does on the command line.
    [notes "commits"]
        mergestrategy = concatenate
    [notes "notes/commits"]
        mergestrategy = cat_sort_uniq
    [notes "refs/notes/commits"]
        mergestrategy = overwrite

The three entries in the above example obviously are all meant to
refer to the same refs/notes/commits notes tree, and the usual "last
one wins" rule should apply.  But with the recent git_config_get_*()
interface, you cannot tell which one among them was given the last,
overriding the previous entries.
Ya, I'd like to avoid this if possible.

Regards,
Jake
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help