Commit ee76f92f authored by Mike Hommey's avatar Mike Hommey Committed by Junio C Hamano

notes: allow treeish expressions as notes ref

init_notes() is the main point of entry to the notes API. It ensures
that the input can be used as ref, because it needs a ref to update to
store notes tree after modifying it.

There however are many use cases where notes tree is only read, e.g.
"git log --notes=...".  Any notes-shaped treeish could be used for such
purpose, but it is not allowed due to existing restriction.

Allow treeish expressions to be used in the case the notes tree is going
to be used without write "permissions".  Add a flag to distinguish
whether the notes tree is intended to be used read-only, or will be
updated.

With this change, operations that use notes read-only can be fed any
notes-shaped tree-ish can be used, e.g. git log --notes=notes@{1}.
Signed-off-by: default avatarMike Hommey <mh@glandium.org>
Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
parent 75488425
...@@ -43,7 +43,7 @@ people using 80-column terminals. ...@@ -43,7 +43,7 @@ people using 80-column terminals.
commit may be copied to the output. commit may be copied to the output.
ifndef::git-rev-list[] ifndef::git-rev-list[]
--notes[=<ref>]:: --notes[=<treeish>]::
Show the notes (see linkgit:git-notes[1]) that annotate the Show the notes (see linkgit:git-notes[1]) that annotate the
commit, when showing the commit log message. This is the default commit, when showing the commit log message. This is the default
for `git log`, `git show` and `git whatchanged` commands when for `git log`, `git show` and `git whatchanged` commands when
...@@ -54,8 +54,8 @@ By default, the notes shown are from the notes refs listed in the ...@@ -54,8 +54,8 @@ By default, the notes shown are from the notes refs listed in the
'core.notesRef' and 'notes.displayRef' variables (or corresponding 'core.notesRef' and 'notes.displayRef' variables (or corresponding
environment overrides). See linkgit:git-config[1] for more details. environment overrides). See linkgit:git-config[1] for more details.
+ +
With an optional '<ref>' argument, show this notes ref instead of the With an optional '<treeish>' argument, use the treeish to find the notes
default notes ref(s). The ref specifies the full refname when it begins to display. The treeish can specify the full refname when it begins
with `refs/notes/`; when it begins with `notes/`, `refs/` and otherwise with `refs/notes/`; when it begins with `notes/`, `refs/` and otherwise
`refs/notes/` is prefixed to form a full name of the ref. `refs/notes/` is prefixed to form a full name of the ref.
+ +
...@@ -71,7 +71,7 @@ being displayed. Examples: "--notes=foo" will show only notes from ...@@ -71,7 +71,7 @@ being displayed. Examples: "--notes=foo" will show only notes from
"--notes --notes=foo --no-notes --notes=bar" will only show notes "--notes --notes=foo --no-notes --notes=bar" will only show notes
from "refs/notes/bar". from "refs/notes/bar".
--show-notes[=<ref>]:: --show-notes[=<treeish>]::
--[no-]standard-notes:: --[no-]standard-notes::
These options are deprecated. Use the above --notes/--no-notes These options are deprecated. Use the above --notes/--no-notes
options instead. options instead.
......
...@@ -286,7 +286,7 @@ static int notes_copy_from_stdin(int force, const char *rewrite_cmd) ...@@ -286,7 +286,7 @@ static int notes_copy_from_stdin(int force, const char *rewrite_cmd)
if (!c) if (!c)
return 0; return 0;
} else { } else {
init_notes(NULL, NULL, NULL, 0); init_notes(NULL, NULL, NULL, NOTES_INIT_WRITABLE);
t = &default_notes_tree; t = &default_notes_tree;
} }
...@@ -329,15 +329,18 @@ static int notes_copy_from_stdin(int force, const char *rewrite_cmd) ...@@ -329,15 +329,18 @@ static int notes_copy_from_stdin(int force, const char *rewrite_cmd)
return ret; return ret;
} }
static struct notes_tree *init_notes_check(const char *subcommand) static struct notes_tree *init_notes_check(const char *subcommand,
int flags)
{ {
struct notes_tree *t; struct notes_tree *t;
init_notes(NULL, NULL, NULL, 0); const char *ref;
init_notes(NULL, NULL, NULL, flags);
t = &default_notes_tree; t = &default_notes_tree;
if (!starts_with(t->ref, "refs/notes/")) ref = (flags & NOTES_INIT_WRITABLE) ? t->update_ref : t->ref;
if (!starts_with(ref, "refs/notes/"))
die("Refusing to %s notes in %s (outside of refs/notes/)", die("Refusing to %s notes in %s (outside of refs/notes/)",
subcommand, t->ref); subcommand, ref);
return t; return t;
} }
...@@ -360,7 +363,7 @@ static int list(int argc, const char **argv, const char *prefix) ...@@ -360,7 +363,7 @@ static int list(int argc, const char **argv, const char *prefix)
usage_with_options(git_notes_list_usage, options); usage_with_options(git_notes_list_usage, options);
} }
t = init_notes_check("list"); t = init_notes_check("list", 0);
if (argc) { if (argc) {
if (get_sha1(argv[0], object)) if (get_sha1(argv[0], object))
die(_("Failed to resolve '%s' as a valid ref."), argv[0]); die(_("Failed to resolve '%s' as a valid ref."), argv[0]);
...@@ -420,7 +423,7 @@ static int add(int argc, const char **argv, const char *prefix) ...@@ -420,7 +423,7 @@ static int add(int argc, const char **argv, const char *prefix)
if (get_sha1(object_ref, object)) if (get_sha1(object_ref, object))
die(_("Failed to resolve '%s' as a valid ref."), object_ref); die(_("Failed to resolve '%s' as a valid ref."), object_ref);
t = init_notes_check("add"); t = init_notes_check("add", NOTES_INIT_WRITABLE);
note = get_note(t, object); note = get_note(t, object);
if (note) { if (note) {
...@@ -511,7 +514,7 @@ static int copy(int argc, const char **argv, const char *prefix) ...@@ -511,7 +514,7 @@ static int copy(int argc, const char **argv, const char *prefix)
if (get_sha1(object_ref, object)) if (get_sha1(object_ref, object))
die(_("Failed to resolve '%s' as a valid ref."), object_ref); die(_("Failed to resolve '%s' as a valid ref."), object_ref);
t = init_notes_check("copy"); t = init_notes_check("copy", NOTES_INIT_WRITABLE);
note = get_note(t, object); note = get_note(t, object);
if (note) { if (note) {
...@@ -589,7 +592,7 @@ static int append_edit(int argc, const char **argv, const char *prefix) ...@@ -589,7 +592,7 @@ static int append_edit(int argc, const char **argv, const char *prefix)
if (get_sha1(object_ref, object)) if (get_sha1(object_ref, object))
die(_("Failed to resolve '%s' as a valid ref."), object_ref); die(_("Failed to resolve '%s' as a valid ref."), object_ref);
t = init_notes_check(argv[0]); t = init_notes_check(argv[0], NOTES_INIT_WRITABLE);
note = get_note(t, object); note = get_note(t, object);
prepare_note_data(object, &d, edit ? note : NULL); prepare_note_data(object, &d, edit ? note : NULL);
...@@ -652,7 +655,7 @@ static int show(int argc, const char **argv, const char *prefix) ...@@ -652,7 +655,7 @@ static int show(int argc, const char **argv, const char *prefix)
if (get_sha1(object_ref, object)) if (get_sha1(object_ref, object))
die(_("Failed to resolve '%s' as a valid ref."), object_ref); die(_("Failed to resolve '%s' as a valid ref."), object_ref);
t = init_notes_check("show"); t = init_notes_check("show", 0);
note = get_note(t, object); note = get_note(t, object);
if (!note) if (!note)
...@@ -809,7 +812,7 @@ static int merge(int argc, const char **argv, const char *prefix) ...@@ -809,7 +812,7 @@ static int merge(int argc, const char **argv, const char *prefix)
expand_notes_ref(&remote_ref); expand_notes_ref(&remote_ref);
o.remote_ref = remote_ref.buf; o.remote_ref = remote_ref.buf;
t = init_notes_check("merge"); t = init_notes_check("merge", NOTES_INIT_WRITABLE);
if (strategy) { if (strategy) {
if (parse_notes_merge_strategy(strategy, &o.strategy)) { if (parse_notes_merge_strategy(strategy, &o.strategy)) {
...@@ -901,7 +904,7 @@ static int remove_cmd(int argc, const char **argv, const char *prefix) ...@@ -901,7 +904,7 @@ static int remove_cmd(int argc, const char **argv, const char *prefix)
argc = parse_options(argc, argv, prefix, options, argc = parse_options(argc, argv, prefix, options,
git_notes_remove_usage, 0); git_notes_remove_usage, 0);
t = init_notes_check("remove"); t = init_notes_check("remove", NOTES_INIT_WRITABLE);
if (!argc && !from_stdin) { if (!argc && !from_stdin) {
retval = remove_one_note(t, "HEAD", flag); retval = remove_one_note(t, "HEAD", flag);
...@@ -943,7 +946,7 @@ static int prune(int argc, const char **argv, const char *prefix) ...@@ -943,7 +946,7 @@ static int prune(int argc, const char **argv, const char *prefix)
usage_with_options(git_notes_prune_usage, options); usage_with_options(git_notes_prune_usage, options);
} }
t = init_notes_check("prune"); t = init_notes_check("prune", NOTES_INIT_WRITABLE);
prune_notes(t, (verbose ? NOTES_PRUNE_VERBOSE : 0) | prune_notes(t, (verbose ? NOTES_PRUNE_VERBOSE : 0) |
(show_only ? NOTES_PRUNE_VERBOSE|NOTES_PRUNE_DRYRUN : 0) ); (show_only ? NOTES_PRUNE_VERBOSE|NOTES_PRUNE_DRYRUN : 0) );
......
...@@ -32,14 +32,14 @@ void notes_cache_init(struct notes_cache *c, const char *name, ...@@ -32,14 +32,14 @@ void notes_cache_init(struct notes_cache *c, const char *name,
const char *validity) const char *validity)
{ {
struct strbuf ref = STRBUF_INIT; struct strbuf ref = STRBUF_INIT;
int flags = 0; int flags = NOTES_INIT_WRITABLE;
memset(c, 0, sizeof(*c)); memset(c, 0, sizeof(*c));
c->validity = xstrdup(validity); c->validity = xstrdup(validity);
strbuf_addf(&ref, "refs/notes/%s", name); strbuf_addf(&ref, "refs/notes/%s", name);
if (!notes_cache_match_validity(ref.buf, validity)) if (!notes_cache_match_validity(ref.buf, validity))
flags = NOTES_INIT_EMPTY; flags |= NOTES_INIT_EMPTY;
init_notes(&c->tree, ref.buf, combine_notes_overwrite, flags); init_notes(&c->tree, ref.buf, combine_notes_overwrite, flags);
strbuf_release(&ref); strbuf_release(&ref);
} }
...@@ -49,7 +49,8 @@ int notes_cache_write(struct notes_cache *c) ...@@ -49,7 +49,8 @@ int notes_cache_write(struct notes_cache *c)
unsigned char tree_sha1[20]; unsigned char tree_sha1[20];
unsigned char commit_sha1[20]; unsigned char commit_sha1[20];
if (!c || !c->tree.initialized || !c->tree.ref || !*c->tree.ref) if (!c || !c->tree.initialized || !c->tree.update_ref ||
!*c->tree.update_ref)
return -1; return -1;
if (!c->tree.dirty) if (!c->tree.dirty)
return 0; return 0;
...@@ -59,8 +60,8 @@ int notes_cache_write(struct notes_cache *c) ...@@ -59,8 +60,8 @@ int notes_cache_write(struct notes_cache *c)
if (commit_tree(c->validity, strlen(c->validity), tree_sha1, NULL, if (commit_tree(c->validity, strlen(c->validity), tree_sha1, NULL,
commit_sha1, NULL, NULL) < 0) commit_sha1, NULL, NULL) < 0)
return -1; return -1;
if (update_ref("update notes cache", c->tree.ref, commit_sha1, NULL, if (update_ref("update notes cache", c->tree.update_ref, commit_sha1,
0, UPDATE_REFS_QUIET_ON_ERR) < 0) NULL, 0, UPDATE_REFS_QUIET_ON_ERR) < 0)
return -1; return -1;
return 0; return 0;
......
...@@ -37,7 +37,7 @@ void commit_notes(struct notes_tree *t, const char *msg) ...@@ -37,7 +37,7 @@ void commit_notes(struct notes_tree *t, const char *msg)
if (!t) if (!t)
t = &default_notes_tree; t = &default_notes_tree;
if (!t->initialized || !t->ref || !*t->ref) if (!t->initialized || !t->update_ref || !*t->update_ref)
die(_("Cannot commit uninitialized/unreferenced notes tree")); die(_("Cannot commit uninitialized/unreferenced notes tree"));
if (!t->dirty) if (!t->dirty)
return; /* don't have to commit an unchanged tree */ return; /* don't have to commit an unchanged tree */
...@@ -48,7 +48,7 @@ void commit_notes(struct notes_tree *t, const char *msg) ...@@ -48,7 +48,7 @@ void commit_notes(struct notes_tree *t, const char *msg)
create_notes_commit(t, NULL, buf.buf, buf.len, commit_sha1); create_notes_commit(t, NULL, buf.buf, buf.len, commit_sha1);
strbuf_insert(&buf, 0, "notes: ", 7); /* commit message starts at index 7 */ strbuf_insert(&buf, 0, "notes: ", 7); /* commit message starts at index 7 */
update_ref(buf.buf, t->ref, commit_sha1, NULL, 0, update_ref(buf.buf, t->update_ref, commit_sha1, NULL, 0,
UPDATE_REFS_DIE_ON_ERR); UPDATE_REFS_DIE_ON_ERR);
strbuf_release(&buf); strbuf_release(&buf);
...@@ -148,7 +148,7 @@ struct notes_rewrite_cfg *init_copy_notes_for_rewrite(const char *cmd) ...@@ -148,7 +148,7 @@ struct notes_rewrite_cfg *init_copy_notes_for_rewrite(const char *cmd)
free(c); free(c);
return NULL; return NULL;
} }
c->trees = load_notes_trees(c->refs); c->trees = load_notes_trees(c->refs, NOTES_INIT_WRITABLE);
string_list_clear(c->refs, 0); string_list_clear(c->refs, 0);
free(c->refs); free(c->refs);
return c; return c;
......
...@@ -1011,13 +1011,16 @@ void init_notes(struct notes_tree *t, const char *notes_ref, ...@@ -1011,13 +1011,16 @@ void init_notes(struct notes_tree *t, const char *notes_ref,
t->first_non_note = NULL; t->first_non_note = NULL;
t->prev_non_note = NULL; t->prev_non_note = NULL;
t->ref = xstrdup_or_null(notes_ref); t->ref = xstrdup_or_null(notes_ref);
t->update_ref = (flags & NOTES_INIT_WRITABLE) ? t->ref : NULL;
t->combine_notes = combine_notes; t->combine_notes = combine_notes;
t->initialized = 1; t->initialized = 1;
t->dirty = 0; t->dirty = 0;
if (flags & NOTES_INIT_EMPTY || !notes_ref || if (flags & NOTES_INIT_EMPTY || !notes_ref ||
read_ref(notes_ref, object_sha1)) get_sha1_treeish(notes_ref, object_sha1))
return; return;
if (flags & NOTES_INIT_WRITABLE && read_ref(notes_ref, object_sha1))
die("Cannot use notes ref %s", notes_ref);
if (get_tree_entry(object_sha1, "", sha1, &mode)) if (get_tree_entry(object_sha1, "", sha1, &mode))
die("Failed to read notes tree referenced by %s (%s)", die("Failed to read notes tree referenced by %s (%s)",
notes_ref, sha1_to_hex(object_sha1)); notes_ref, sha1_to_hex(object_sha1));
...@@ -1027,7 +1030,7 @@ void init_notes(struct notes_tree *t, const char *notes_ref, ...@@ -1027,7 +1030,7 @@ void init_notes(struct notes_tree *t, const char *notes_ref,
load_subtree(t, &root_tree, t->root, 0); load_subtree(t, &root_tree, t->root, 0);
} }
struct notes_tree **load_notes_trees(struct string_list *refs) struct notes_tree **load_notes_trees(struct string_list *refs, int flags)
{ {
struct string_list_item *item; struct string_list_item *item;
int counter = 0; int counter = 0;
...@@ -1035,7 +1038,7 @@ struct notes_tree **load_notes_trees(struct string_list *refs) ...@@ -1035,7 +1038,7 @@ struct notes_tree **load_notes_trees(struct string_list *refs)
trees = xmalloc((refs->nr+1) * sizeof(struct notes_tree *)); trees = xmalloc((refs->nr+1) * sizeof(struct notes_tree *));
for_each_string_list_item(item, refs) { for_each_string_list_item(item, refs) {
struct notes_tree *t = xcalloc(1, sizeof(struct notes_tree)); struct notes_tree *t = xcalloc(1, sizeof(struct notes_tree));
init_notes(t, item->string, combine_notes_ignore, 0); init_notes(t, item->string, combine_notes_ignore, flags);
trees[counter++] = t; trees[counter++] = t;
} }
trees[counter] = NULL; trees[counter] = NULL;
...@@ -1071,7 +1074,7 @@ void init_display_notes(struct display_notes_opt *opt) ...@@ -1071,7 +1074,7 @@ void init_display_notes(struct display_notes_opt *opt)
item->string); item->string);
} }
display_notes_trees = load_notes_trees(&display_notes_refs); display_notes_trees = load_notes_trees(&display_notes_refs, 0);
string_list_clear(&display_notes_refs, 0); string_list_clear(&display_notes_refs, 0);
} }
......
...@@ -44,6 +44,7 @@ extern struct notes_tree { ...@@ -44,6 +44,7 @@ extern struct notes_tree {
struct int_node *root; struct int_node *root;
struct non_note *first_non_note, *prev_non_note; struct non_note *first_non_note, *prev_non_note;
char *ref; char *ref;
char *update_ref;
combine_notes_fn combine_notes; combine_notes_fn combine_notes;
int initialized; int initialized;
int dirty; int dirty;
...@@ -71,6 +72,13 @@ const char *default_notes_ref(void); ...@@ -71,6 +72,13 @@ const char *default_notes_ref(void);
*/ */
#define NOTES_INIT_EMPTY 1 #define NOTES_INIT_EMPTY 1
/*
* By default, the notes tree is only readable, and the notes ref can be
* any treeish. The notes tree can however be made writable with this flag,
* in which case only strict ref names can be used.
*/
#define NOTES_INIT_WRITABLE 2
/* /*
* Initialize the given notes_tree with the notes tree structure at the given * Initialize the given notes_tree with the notes tree structure at the given
* ref. If given ref is NULL, the value of the $GIT_NOTES_REF environment * ref. If given ref is NULL, the value of the $GIT_NOTES_REF environment
...@@ -276,7 +284,7 @@ void format_display_notes(const unsigned char *object_sha1, ...@@ -276,7 +284,7 @@ void format_display_notes(const unsigned char *object_sha1,
* Load the notes tree from each ref listed in 'refs'. The output is * Load the notes tree from each ref listed in 'refs'. The output is
* an array of notes_tree*, terminated by a NULL. * an array of notes_tree*, terminated by a NULL.
*/ */
struct notes_tree **load_notes_trees(struct string_list *refs); struct notes_tree **load_notes_trees(struct string_list *refs, int flags);
/* /*
* Add all refs that match 'glob' to the 'list'. * Add all refs that match 'glob' to the 'list'.
......
...@@ -83,6 +83,16 @@ test_expect_success 'edit existing notes' ' ...@@ -83,6 +83,16 @@ test_expect_success 'edit existing notes' '
test_must_fail git notes show HEAD^ test_must_fail git notes show HEAD^
' '
test_expect_success 'show notes from treeish' '
test "b3" = "$(git notes --ref commits^{tree} show)" &&
test "b4" = "$(git notes --ref commits@{1} show)"
'
test_expect_success 'cannot edit notes from non-ref' '
test_must_fail git notes --ref commits^{tree} edit &&
test_must_fail git notes --ref commits@{1} edit
'
test_expect_success 'cannot "git notes add -m" where notes already exists' ' test_expect_success 'cannot "git notes add -m" where notes already exists' '
test_must_fail git notes add -m "b2" && test_must_fail git notes add -m "b2" &&
test_path_is_missing .git/NOTES_EDITMSG && test_path_is_missing .git/NOTES_EDITMSG &&
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment