From: Johan Herland <hidden> Date: 2016-06-15 22:47:29
Hi,
(Feel free to put this on hold until v1.6.5 is released. In any case,
I'm going to Berlin for the weekend, and don't expect to read much
email...)
Here is the 7th iteration of the git-notes series. Changes in this
iteration are as follows:
- Rebased onto current 'next'
- Patch 1: Include minor leak fix
- Patch 10: Rename free_commit_notes() to free_notes() (Notes are
no longer bound to commits only, see patch 15 for details)
- Patch 12: Remove tests that are invalidated by concatenation code
in patch 13.
Overall, I consider the 12 first patches fairly stable at this point.
There's also a slew of new patches, that has more of an RFC status:
- Patches 13-14: Concatenation of multiple notes annotating the same
commit/object. This was originally suggested by mugwump many months
ago, and the suggestion was re-iterated by Dscho. This change has a
minor perfomance impact (see [1]), but I still think it's worth it.
- Patch 15: Allow notes to be attached to any object (not just commits).
Rename get_commit_notes() to format_note() to reflect this change.
- Patch 16-19: Expand notes API in preparation for querying and
manipulating notes from elsewhere in Git (see patch 22 for examples).
- Patch 20: Add a new notes_tree struct, and use it as the first
parameter to all functions in the notes API. This allows API users to
maintain their own (multiple, concurrent) notes trees (see patch 22
for an example). We still have a default notes tree in notes.c as a
fallback (when NULL is passed as to an API function).
- Patch 21: The default behaviour when there are multiple notes for a
given object is to concatenate them. However, some callers (see patch
22) want to tweak this behaviour. This patch defines a new function
type: combine_notes_fn, for combining two notes that reference the
same object. The notes API is then expanded to allow the caller to
specify a suitable combine_notes_fn. For convenience, three simple
combine_notes functions are available in the notes API:
- combine_notes_concatenate(): Concatenates the contents of the two
notes. (This is the default behaviour)
- combine_notes_overwrite(): Overwrite the existing note with the
new note.
- combine_notes_ignore(): Keep the existing note, and ignore the new
note.
- Patch 22: This teaches fast-import to use the new notes API when
adding note objects to a commit. Since adding a note to a notes tree
might cause restructuring of that notes tree, the note objects must
be handled differently from regular blobs.
There are some testcases for the new behaviour in this patch, but not
enough. These will be added later.
This patch is still very much in RFC mode...
Although this iteration brings the jh/notes topic towards feature-
completion, there are still some things left to do before I consider
the git notes feature fully complete:
- Builtin-ify git-notes shell script to take advantage of notes API
- Garbage-collect notes whose referenced objects are unreachable
- Handle note objects that are not blobs, but trees (e.g.
refs/notes/<topic>:<commit>/<subtopic>)
- Add a simple notation for referring to an object's note (e.g.
"<object>^{note}")
- Probably more that I haven't thought of yet...
However, It might be a good idea to consider merging the early/stable
parts of jh/notes, instead of waiting for everything to complete.
Have fun! :)
...Johan
[1] Performance impact of the concatenation rewrite.
In order to concatenate notes correctly, the tree traversal code must be
changed to more proactively unpack subtree entries (so that we can safely
determine whether there are multiple notes for a given key).
As before, the test case is as follows:
Linux kernel repo with 157101 commits, 1 note per commit, organized into
various fanout schemes. Hardware is Intel Core 2 Quad with 4GB RAM.
Algorithm / Notes tree git log -n10 (x100) git log --all
next / no-notes 4.78s 63.90s
before / no-notes 4.77s 63.61s
before / no-fanout 56.59s 65.19s
16tree / no-notes 4.73s 63.80s
16tree / no-fanout 30.21s 65.11s
16tree / 2_38 5.53s 65.24s
16tree / 2_2_36 5.15s 65.12s
concat / no-notes 4.80s 64.21s
concat / no-fanout 30.66s 65.35s
concat / 2_38 5.64s 65.87s
concat / 2_2_36 5.23s 66.44s
Conclusion: There is a small, but measurable impact (about .1s or so in
the 100 x 'git log -n10' case), but I think this is small enough to be
acceptable.
Johan Herland (17):
Teach "-m <msg>" and "-F <file>" to "git notes edit"
fast-import: Add support for importing commit notes
t3302-notes-index-expensive: Speed up create_repo()
Add flags to get_commit_notes() to control the format of the note string
Teach notes code to free its internal data structures on request
Teach the notes lookup code to parse notes trees with various fanout schemes
Add selftests verifying that we can parse notes trees with various fanouts
Refactor notes code to concatenate multiple notes annotating the same object
Add selftests verifying concatenation of multiple notes for the same commit
Notes API: get_commit_notes() -> format_note() + remove the commit restriction
Notes API: init_notes(): Initialize the notes tree from the given notes ref
Notes API: add_note(): Add note objects to the internal notes tree structure
Notes API: get_note(): Return the note annotating the given object
Notes API: for_each_note(): Traverse the entire notes tree with a callback
Notes API: Allow multiple concurrent notes trees with new struct notes_tree
Refactor notes concatenation into a flexible interface for combining notes
fast-import: Proper notes tree manipulation using the notes API
Johannes Schindelin (5):
Introduce commit notes
Add a script to edit/inspect notes
Speed up git notes lookup
Add an expensive test for git-notes
Add '%N'-format for pretty-printing commit notes
.gitignore | 1 +
Documentation/config.txt | 13 +
Documentation/git-fast-import.txt | 45 +++-
Documentation/git-notes.txt | 60 ++++
Documentation/pretty-formats.txt | 1 +
Makefile | 3 +
cache.h | 4 +
command-list.txt | 1 +
commit.c | 1 +
config.c | 5 +
environment.c | 1 +
fast-import.c | 176 +++++++++++-
git-notes.sh | 121 ++++++++
notes.c | 579 +++++++++++++++++++++++++++++++++++++
notes.h | 113 +++++++
pretty.c | 10 +
t/t3301-notes.sh | 150 ++++++++++
t/t3302-notes-index-expensive.sh | 118 ++++++++
t/t3303-notes-subtrees.sh | 188 ++++++++++++
t/t9300-fast-import.sh | 296 +++++++++++++++++++
20 files changed, 1875 insertions(+), 11 deletions(-)
create mode 100644 Documentation/git-notes.txt
create mode 100755 git-notes.sh
create mode 100644 notes.c
create mode 100644 notes.h
create mode 100755 t/t3301-notes.sh
create mode 100755 t/t3302-notes-index-expensive.sh
create mode 100755 t/t3303-notes-subtrees.sh
From: Johan Herland <hidden> Date: 2016-06-15 22:47:29
From: Johannes Schindelin <redacted>
git-notes have the potential of being pretty expensive, so test with
a lot of commits. A lot. So to make things cheaper, you have to
opt-in explicitely, by setting the environment variable
GIT_NOTES_TIMING_TESTS.
This patch has been improved by the following contributions:
- Junio C Hamano: tests: fix "export var=val"
Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Johan Herland <redacted>
Signed-off-by: Junio C Hamano <redacted>
---
t/t3302-notes-index-expensive.sh | 98 ++++++++++++++++++++++++++++++++++++++
1 files changed, 98 insertions(+), 0 deletions(-)
create mode 100755 t/t3302-notes-index-expensive.sh
From: Johan Herland <hidden> Date: 2016-06-15 22:47:29
From: Johannes Schindelin <redacted>
Commit notes are blobs which are shown together with the commit
message. These blobs are taken from the notes ref, which you can
configure by the config variable core.notesRef, which in turn can
be overridden by the environment variable GIT_NOTES_REF.
The notes ref is a branch which contains "files" whose names are
the names of the corresponding commits (i.e. the SHA-1).
The rationale for putting this information into a ref is this: we
want to be able to fetch and possibly union-merge the notes,
maybe even look at the date when a note was introduced, and we
want to store them efficiently together with the other objects.
This patch has been improved by the following contributions:
- Thomas Rast: fix core.notesRef documentation
- Tor Arne Vestbø: fix printing of multi-line notes
- Alex Riesen: Using char array instead of char pointer costs less BSS
- Johan Herland: Plug leak when msg is good, but msglen or type causes return
Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Thomas Rast <redacted>
Signed-off-by: Tor Arne Vestbø <redacted>
Signed-off-by: Johan Herland <redacted>
Signed-off-by: Junio C Hamano <redacted>
get_commit_notes(): Plug memory leak when 'if' triggers, but not because of read_sha1_file() failure
---
Documentation/config.txt | 13 ++++++++
Makefile | 2 +
cache.h | 4 ++
commit.c | 1 +
config.c | 5 +++
environment.c | 1 +
notes.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++
notes.h | 7 ++++
pretty.c | 5 +++
9 files changed, 108 insertions(+), 0 deletions(-)
create mode 100644 notes.c
create mode 100644 notes.h
@@ -458,6 +458,19 @@ On some file system/operating system combinations, this is unreliable. Set this config setting to 'rename' there; However, This will remove the check that makes sure that existing object files will not get overwritten.+core.notesRef::+ When showing commit messages, also show notes which are stored in+ the given ref. This ref is expected to contain files named+ after the full SHA-1 of the commit they annotate.+++If such a file exists in the given ref, the referenced blob is read, and+appended to the commit message, separated by a "Notes:" line. If the+given ref itself does not exist, it is not an error, but means that no+notes should be printed.+++This setting defaults to "refs/notes/commits", and can be overridden by+the `GIT_NOTES_REF` environment variable.+ add.ignore-errors:: Tells 'git-add' to continue adding files when some files cannot be added due to indexing errors. Equivalent to the '--ignore-errors'
@@ -0,0 +1,70 @@+#include"cache.h"+#include"commit.h"+#include"notes.h"+#include"refs.h"+#include"utf8.h"+#include"strbuf.h"++staticintinitialized;++voidget_commit_notes(conststructcommit*commit,structstrbuf*sb,+constchar*output_encoding)+{+staticconstcharutf8[]="utf-8";+structstrbufname=STRBUF_INIT;+unsignedcharsha1[20];+char*msg,*msg_p;+unsignedlonglinelen,msglen;+enumobject_typetype;++if(!initialized){+constchar*env=getenv(GIT_NOTES_REF_ENVIRONMENT);+if(env)+notes_ref_name=getenv(GIT_NOTES_REF_ENVIRONMENT);+elseif(!notes_ref_name)+notes_ref_name=GIT_NOTES_DEFAULT_REF;+if(notes_ref_name&&read_ref(notes_ref_name,sha1))+notes_ref_name=NULL;+initialized=1;+}++if(!notes_ref_name)+return;++strbuf_addf(&name,"%s:%s",notes_ref_name,+sha1_to_hex(commit->object.sha1));+if(get_sha1(name.buf,sha1))+return;++if(!(msg=read_sha1_file(sha1,&type,&msglen))||!msglen||+type!=OBJ_BLOB){+free(msg);+return;+}++if(output_encoding&&*output_encoding&&+strcmp(utf8,output_encoding)){+char*reencoded=reencode_string(msg,output_encoding,utf8);+if(reencoded){+free(msg);+msg=reencoded;+msglen=strlen(msg);+}+}++/* we will end the annotation by a newline anyway */+if(msglen&&msg[msglen-1]=='\n')+msglen--;++strbuf_addstr(sb,"\nNotes:\n");++for(msg_p=msg;msg_p<msg+msglen;msg_p+=linelen+1){+linelen=strchrnul(msg_p,'\n')-msg_p;++strbuf_addstr(sb," ");+strbuf_add(sb,msg_p,linelen);+strbuf_addch(sb,'\n');+}++free(msg);+}
From: Johan Herland <hidden> Date: 2016-06-15 22:47:29
Introduce a 'notemodify' subcommand of the 'commit' command. This subcommand
is similar to 'filemodify', except that no mode is supplied (all notes have
mode 0644), and the path is set to the hex SHA1 of the given "comittish".
This enables fast import of note objects along with their associated commits,
since the notes can now be named using the mark references of their
corresponding commits.
The patch also includes a test case of the added functionality.
Signed-off-by: Johan Herland <redacted>
Acked-by: Shawn O. Pearce <redacted>
---
Documentation/git-fast-import.txt | 45 +++++++++--
fast-import.c | 88 +++++++++++++++++++-
t/t9300-fast-import.sh | 166 +++++++++++++++++++++++++++++++++++++
3 files changed, 289 insertions(+), 10 deletions(-)
@@ -348,14 +348,13 @@ commit message use a 0 length data. Commit messages are free-form and are not interpreted by Git. Currently they must be encoded in UTF-8, as fast-import does not permit other encodings to be specified.-Zero or more `filemodify`, `filedelete`, `filecopy`, `filerename`-and `filedeleteall` commands+Zero or more `filemodify`, `filedelete`, `filecopy`, `filerename`,+`filedeleteall` and `notemodify` commands may be included to update the contents of the branch prior to creating the commit. These commands may be supplied in any order. However it is recommended that a `filedeleteall` command precede-all `filemodify`, `filecopy` and `filerename` commands in the same-commit, as `filedeleteall`-wipes the branch clean (see below).+all `filemodify`, `filecopy`, `filerename` and `notemodify` commands in+the same commit, as `filedeleteall` wipes the branch clean (see below). The `LF` after the command is optional (it used to be required).
@@ -604,6 +603,40 @@ more memory per active branch (less than 1 MiB for even most large projects); so frontends that can easily obtain only the affected paths for a commit are encouraged to do so.+`notemodify`+^^^^^^^^^^^^+Included in a `commit` command to add a new note (annotating a given+commit) or change the content of an existing note. This command has+two different means of specifying the content of the note.++External data format::+ The data content for the note was already supplied by a prior+ `blob` command. The frontend just needs to connect it to the+ commit that is to be annotated.+++....+ 'N' SP <dataref> SP <committish> LF+....+++Here `<dataref>` can be either a mark reference (`:<idnum>`)+set by a prior `blob` command, or a full 40-byte SHA-1 of an+existing Git blob object.++Inline data format::+ The data content for the note has not been supplied yet.+ The frontend wants to supply it as part of this modify+ command.+++....+ 'N' SP 'inline' SP <committish> LF+ data+....+++See below for a detailed description of the `data` command.++In both formats `<committish>` is any of the commit specification+expressions also accepted by `from` (see above).+ `mark` ~~~~~~ Arranges for fast-import to save a reference to the current object, allowing
@@ -22,8 +22,8 @@ Format of STDIN stream:('author'spnamesp'<'email'>'spwhenlf)?'committer'spnamesp'<'email'>'spwhenlfcommit_msg-('from'sp(ref_str|hexsha1|sha1exp_str|idnum)lf)?-('merge'sp(ref_str|hexsha1|sha1exp_str|idnum)lf)*+('from'spcommittishlf)?+('merge'spcommittishlf)*file_change*lf?;commit_msg::=data;
@@ -41,15 +41,18 @@ Format of STDIN stream:file_obm::='M'spmodesp(hexsha1|idnum)sppath_strlf;file_inm::='M'spmodesp'inline'sppath_strlfdata;+note_obm::='N'sp(hexsha1|idnum)spcommittishlf;+note_inm::='N'sp'inline'spcommittishlf+data;new_tag::='tag'sptag_strlf-'from'sp(ref_str|hexsha1|sha1exp_str|idnum)lf+'from'spcommittishlf('tagger'spnamesp'<'email'>'spwhenlf)?tag_msg;tag_msg::=data;reset_branch::='reset'spref_strlf-('from'sp(ref_str|hexsha1|sha1exp_str|idnum)lf)?+('from'spcommittishlf)?lf?;checkpoint::='checkpoint'lf
@@ -88,6 +91,7 @@ Format of STDIN stream:# stream formatting is: \, " and LF. Otherwise these values# are UTF8.#+committish::=(ref_str|hexsha1|sha1exp_str|idnum);ref_str::=ref;sha1exp_str::=sha1exp;tag_str::=tag;
@@ -2056,6 +2060,80 @@ static void file_change_cr(struct branch *b, int rename)leaf.tree);}+staticvoidnote_change_n(structbranch*b)+{+constchar*p=command_buf.buf+2;+staticstructstrbufuq=STRBUF_INIT;+structobject_entry*oe=oe;+structbranch*s;+unsignedcharsha1[20],commit_sha1[20];+uint16_tinline_data=0;++/* <dataref> or 'inline' */+if(*p==':'){+char*x;+oe=find_mark(strtoumax(p+1,&x,10));+hashcpy(sha1,oe->sha1);+p=x;+}elseif(!prefixcmp(p,"inline")){+inline_data=1;+p+=6;+}else{+if(get_sha1_hex(p,sha1))+die("Invalid SHA1: %s",command_buf.buf);+oe=find_object(sha1);+p+=40;+}+if(*p++!=' ')+die("Missing space after SHA1: %s",command_buf.buf);++/* <committish> */+s=lookup_branch(p);+if(s){+hashcpy(commit_sha1,s->sha1);+}elseif(*p==':'){+uintmax_tcommit_mark=strtoumax(p+1,NULL,10);+structobject_entry*commit_oe=find_mark(commit_mark);+if(commit_oe->type!=OBJ_COMMIT)+die("Mark :%"PRIuMAX" not a commit",commit_mark);+hashcpy(commit_sha1,commit_oe->sha1);+}elseif(!get_sha1(p,commit_sha1)){+unsignedlongsize;+char*buf=read_object_with_reference(commit_sha1,+commit_type,&size,commit_sha1);+if(!buf||size<46)+die("Not a valid commit: %s",p);+free(buf);+}else+die("Invalid ref name or SHA1 expression: %s",p);++if(inline_data){+staticstructstrbufbuf=STRBUF_INIT;++if(p!=uq.buf){+strbuf_addstr(&uq,p);+p=uq.buf;+}+read_next_command();+parse_data(&buf);+store_object(OBJ_BLOB,&buf,&last_blob,sha1,0);+}elseif(oe){+if(oe->type!=OBJ_BLOB)+die("Not a blob (actually a %s): %s",+typename(oe->type),command_buf.buf);+}else{+enumobject_typetype=sha1_object_info(sha1,NULL);+if(type<0)+die("Blob not found: %s",command_buf.buf);+if(type!=OBJ_BLOB)+die("Not a blob (actually a %s): %s",+typename(type),command_buf.buf);+}++tree_content_set(&b->branch_tree,sha1_to_hex(commit_sha1),sha1,+S_IFREG|0644,NULL);+}+staticvoidfile_change_deleteall(structbranch*b){release_tree_content_recursive(b->branch_tree.tree);
@@ -1089,6 +1089,172 @@ test_expect_success 'P: fail on blob mark in gitlink' 'test_must_failgitfast-import<input'###+### series Q (notes)+###++note1_data="Note for the first commit"+note2_data="Note for the second commit"+note3_data="Note for the third commit"++test_tick+cat>input<<INPUT_END+blob+mark:2+data<<EOF+$file2_data+EOF++commitrefs/heads/notes-test+mark:3+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+data<<COMMIT+first(:3)+COMMIT++M644:2file2++blob+mark:4+data$file4_len+$file4_data+commitrefs/heads/notes-test+mark:5+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+data<<COMMIT+second(:5)+COMMIT++M644:4file4++commitrefs/heads/notes-test+mark:6+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+data<<COMMIT+third(:6)+COMMIT++M644inlinefile5+data<<EOF+$file5_data+EOF++M755inlinefile6+data<<EOF+$file6_data+EOF++blob+mark:7+data<<EOF+$note1_data+EOF++blob+mark:8+data<<EOF+$note2_data+EOF++commitrefs/notes/foobar+mark:9+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+data<<COMMIT+notes(:9)+COMMIT++N:7:3+N:8:5+Ninline:6+data<<EOF+$note3_data+EOF++INPUT_END+test_expect_success\+'Q: commit notes'\+'gitfast-import<input&&+gitwhatchangednotes-test'+test_expect_success\+'Q: verify pack'\+'for p in .git/objects/pack/*.pack;do git verify-pack $p||exit;done'++commit1=$(gitrev-parsenotes-test~2)+commit2=$(gitrev-parsenotes-test^)+commit3=$(gitrev-parsenotes-test)++cat>expect<<EOF+author$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE++first(:3)+EOF+test_expect_success\+'Q: verify first commit'\+'gitcat-filecommitnotes-test~2|sed1d>actual&&+test_cmpexpectactual'++cat>expect<<EOF+parent$commit1+author$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE++second(:5)+EOF+test_expect_success\+'Q: verify second commit'\+'gitcat-filecommitnotes-test^|sed1d>actual&&+test_cmpexpectactual'++cat>expect<<EOF+parent$commit2+author$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE++third(:6)+EOF+test_expect_success\+'Q: verify third commit'\+'gitcat-filecommitnotes-test|sed1d>actual&&+test_cmpexpectactual'++cat>expect<<EOF+author$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE++notes(:9)+EOF+test_expect_success\+'Q: verify notes commit'\+'gitcat-filecommitrefs/notes/foobar|sed1d>actual&&+test_cmpexpectactual'++cat>expect.unsorted<<EOF+100644blob$commit1+100644blob$commit2+100644blob$commit3+EOF+catexpect.unsorted|sort>expect+test_expect_success\+'Q: verify notes tree'\+'gitcat-file-prefs/notes/foobar^{tree}|sed"s/ [0-9a-f]* / /">actual&&+test_cmpexpectactual'++echo"$note1_data">expect+test_expect_success\+'Q: verify note for first commit'\+'git cat-file blob refs/notes/foobar:$commit1 >actual && test_cmp expect actual'++echo"$note2_data">expect+test_expect_success\+'Q: verify note for second commit'\+'git cat-file blob refs/notes/foobar:$commit2 >actual && test_cmp expect actual'++echo"$note3_data">expect+test_expect_success\+'Q: verify note for third commit'\+'git cat-file blob refs/notes/foobar:$commit3 >actual && test_cmp expect actual'++###### series R (feature and option)###
From: Johan Herland <hidden> Date: 2016-06-15 22:47:29
There's no need to be rude to memory-concious callers...
This patch has been improved by the following contributions:
- Junio C Hamano: avoid old-style declaration
Signed-off-by: Junio C Hamano <redacted>
Signed-off-by: Johan Herland <redacted>
---
notes.c | 7 +++++++
notes.h | 3 +++
2 files changed, 10 insertions(+), 0 deletions(-)
@@ -123,6 +123,7 @@ The placeholders are: - '%s': subject - '%f': sanitized subject line, suitable for a filename - '%b': body+- '%N': commit notes - '%Cred': switch color to red - '%Cgreen': switch color to green - '%Cblue': switch color to blue
@@ -702,6 +702,10 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder,case'd':format_decoration(sb,commit);return1;+case'N':+get_commit_notes(commit,sb,git_log_output_encoding?+git_log_output_encoding:git_commit_encoding,0);+return1;}/* For the rest we have to parse the commit header. */
From: Johan Herland <hidden> Date: 2016-06-15 22:47:29
From: Johannes Schindelin <redacted>
To avoid looking up each and every commit in the notes ref's tree
object, which is very expensive, speed things up by slurping the tree
object's contents into a hash_map.
The idea for the hashmap singleton is from David Reiss, initial
benchmarking by Jeff King.
Note: the implementation allows for arbitrary entries in the notes
tree object, ignoring those that do not reference a valid object. This
allows you to annotate arbitrary branches, or objects.
This patch has been improved by the following contributions:
- Junio C Hamano: fixed an obvious error in initialize_hash_map()
Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Johan Herland <redacted>
Signed-off-by: Junio C Hamano <redacted>
---
notes.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
1 files changed, 102 insertions(+), 10 deletions(-)
@@ -4,15 +4,112 @@#include"refs.h"#include"utf8.h"#include"strbuf.h"+#include"tree-walk.h"++structentry{+unsignedcharcommit_sha1[20];+unsignedcharnotes_sha1[20];+};++structhash_map{+structentry*entries;+off_tcount,size;+};staticintinitialized;+staticstructhash_maphash_map;++staticinthash_index(structhash_map*map,constunsignedchar*sha1)+{+inti=((*(unsignedint*)sha1)%map->size);++for(;;){+unsignedchar*current=map->entries[i].commit_sha1;++if(!hashcmp(sha1,current))+returni;++if(is_null_sha1(current))+return-1-i;++if(++i==map->size)+i=0;+}+}++staticvoidadd_entry(constunsignedchar*commit_sha1,+constunsignedchar*notes_sha1)+{+intindex;++if(hash_map.count+1>hash_map.size>>1){+inti,old_size=hash_map.size;+structentry*old=hash_map.entries;++hash_map.size=old_size?old_size<<1:64;+hash_map.entries=(structentry*)+xcalloc(sizeof(structentry),hash_map.size);++for(i=0;i<old_size;i++)+if(!is_null_sha1(old[i].commit_sha1)){+index=-1-hash_index(&hash_map,+old[i].commit_sha1);+memcpy(hash_map.entries+index,old+i,+sizeof(structentry));+}+free(old);+}++index=hash_index(&hash_map,commit_sha1);+if(index<0){+index=-1-index;+hash_map.count++;+}++hashcpy(hash_map.entries[index].commit_sha1,commit_sha1);+hashcpy(hash_map.entries[index].notes_sha1,notes_sha1);+}++staticvoidinitialize_hash_map(constchar*notes_ref_name)+{+unsignedcharsha1[20],commit_sha1[20];+unsignedmode;+structtree_descdesc;+structname_entryentry;+void*buf;++if(!notes_ref_name||read_ref(notes_ref_name,commit_sha1)||+get_tree_entry(commit_sha1,"",sha1,&mode))+return;++buf=fill_tree_descriptor(&desc,sha1);+if(!buf)+die("Could not read %s for notes-index",sha1_to_hex(sha1));++while(tree_entry(&desc,&entry))+if(!get_sha1(entry.path,commit_sha1))+add_entry(commit_sha1,entry.sha1);+free(buf);+}++staticunsignedchar*lookup_notes(constunsignedchar*commit_sha1)+{+intindex;++if(!hash_map.size)+returnNULL;++index=hash_index(&hash_map,commit_sha1);+if(index<0)+returnNULL;+returnhash_map.entries[index].notes_sha1;+}voidget_commit_notes(conststructcommit*commit,structstrbuf*sb,constchar*output_encoding){staticconstcharutf8[]="utf-8";-structstrbufname=STRBUF_INIT;-unsignedcharsha1[20];+unsignedchar*sha1;char*msg,*msg_p;unsignedlonglinelen,msglen;enumobject_typetype;
From: Johan Herland <hidden> Date: 2016-06-15 22:47:29
Created by a simple refactoring of initialize_notes().
Also add a new 'flags' parameter, which is a bitwise combination of notes
initialization flags. For now, there is only one flag - NOTES_INIT_EMPTY -
which indicates that the notes tree should not auto-load the contents of
the given (or default) notes ref, but rather should leave the notes tree
initialized to an empty state. This will become useful in the future when
manipulating the notes tree through the notes API.
Signed-off-by: Johan Herland <redacted>
---
notes.c | 27 ++++++++++++++++-----------
notes.h | 20 ++++++++++++++++++++
2 files changed, 36 insertions(+), 11 deletions(-)
From: Johan Herland <hidden> Date: 2016-06-15 22:47:29
Also verify that multiple references to the _same_ note blob are _not_
concatenated.
Signed-off-by: Johan Herland <redacted>
---
t/t3303-notes-subtrees.sh | 84 +++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 84 insertions(+), 0 deletions(-)
@@ -101,4 +101,88 @@ test_expect_success 'verify notes in 4/36-fanout' 'verify_notes' test_expect_success'test notes in 2/2/36-fanout''test_sha1_based "s|^\(..\)\(..\)|\1/\2/|"' test_expect_success'verify notes in 2/2/36-fanout''verify_notes'+test_same_notes(){+(+start_note_commit&&+nr=$number_of_commits&&+gitrev-listrefs/heads/master|+whilereadsha1;do+first_note_path=$(echo"$sha1"|sed"$1")+second_note_path=$(echo"$sha1"|sed"$2")+cat<<INPUT_END&&+M100644inline$second_note_path+data<<EOF+noteforcommit#$nr+EOF++M100644inline$first_note_path+data<<EOF+noteforcommit#$nr+EOF++INPUT_END++nr=$(($nr-1))+done+)|+gitfast-import--quiet+}++test_expect_success'test same notes in 4/36-fanout and 2/38-fanout''test_same_notes "s|^..|&/|" "s|^....|&/|"'+test_expect_success'verify same notes in 4/36-fanout and 2/38-fanout''verify_notes'++test_expect_success'test same notes in 2/38-fanout and 2/2/36-fanout''test_same_notes "s|^\(..\)\(..\)|\1/\2/|" "s|^..|&/|"'+test_expect_success'verify same notes in 2/38-fanout and 2/2/36-fanout''verify_notes'++test_expect_success'test same notes in 4/36-fanout and 2/2/36-fanout''test_same_notes "s|^\(..\)\(..\)|\1/\2/|" "s|^....|&/|"'+test_expect_success'verify same notes in 4/36-fanout and 2/2/36-fanout''verify_notes'++test_concatenated_notes(){+(+start_note_commit&&+nr=$number_of_commits&&+gitrev-listrefs/heads/master|+whilereadsha1;do+first_note_path=$(echo"$sha1"|sed"$1")+second_note_path=$(echo"$sha1"|sed"$2")+cat<<INPUT_END&&+M100644inline$second_note_path+data<<EOF+secondnoteforcommit#$nr+EOF++M100644inline$first_note_path+data<<EOF+firstnoteforcommit#$nr+EOF++INPUT_END++nr=$(($nr-1))+done+)|+gitfast-import--quiet+}++verify_concatenated_notes(){+gitlog|grep"^ ">output&&+i=$number_of_commits&&+while[$i-gt0];do+echo" commit #$i"&&+echo" first note for commit #$i"&&+echo" second note for commit #$i"&&+i=$(($i-1));+done>expect&&+test_cmpexpectoutput+}++test_expect_success'test notes in 4/36-fanout concatenated with 2/38-fanout''test_concatenated_notes "s|^..|&/|" "s|^....|&/|"'+test_expect_success'verify notes in 4/36-fanout concatenated with 2/38-fanout''verify_concatenated_notes'++test_expect_success'test notes in 2/38-fanout concatenated with 2/2/36-fanout''test_concatenated_notes "s|^\(..\)\(..\)|\1/\2/|" "s|^..|&/|"'+test_expect_success'verify notes in 2/38-fanout concatenated with 2/2/36-fanout''verify_concatenated_notes'++test_expect_success'test notes in 4/36-fanout concatenated with 2/2/36-fanout''test_concatenated_notes "s|^\(..\)\(..\)|\1/\2/|" "s|^....|&/|"'+test_expect_success'verify notes in 4/36-fanout concatenated with 2/2/36-fanout''verify_concatenated_notes'+ test_done
From: Johan Herland <hidden> Date: 2016-06-15 22:47:29
Hi,
Here is the 7th iteration of the git-notes series. Changes in this
iteration are as follows:
- Rebased onto current 'next'
- Patch 1: Include minor leak fix
- Patch 10: Rename free_commit_notes() to free_notes() (Notes are
no longer bound to only commits, see patch 15 for details)
- Patch 12: Remove tests that are invalidated by concatenation code
in patch 13.
There's also a slew of new patches:
- Patches 13-14: Concatenation of multiple notes annotating the same
commit/object. This was originally suggested by mugwump many months
ago, and the suggestion was re-iterated by Dscho. This change has a
minor perfomance impact (see [1]), but I still think it's worth it.
- Patch 15: Allow notes to be attached to any object (not just commits).
Rename get_commit_notes() to format_note() to reflect this change.
- Patch 16-19: Expand notes API in preparation for querying and manipulating notes from other parts of Git.
TODO:
- Builtin-ify git-notes shell script to take advantage of notes API
- Garbage collect notes whose referenced object is unreachable (gc_notes())
- Handle note objects that are not blobs, but trees
Have fun! :)
...Johan
[1] Performance impact of the concatenation rewrite.
In order to concatenate notes correctly, the tree traversal code must be
changed to more proactively unpack subtree entries (so that we can safely
determine whether there are multiple notes for a given key).
As before, the test case is as follows:
Linux kernel repo with 157101 commits, 1 note per commit, organized into
various fanout schemes. Hardware is Intel Core 2 Quad with 4GB RAM.
Algorithm / Notes tree git log -n10 (x100) git log --all
next / no-notes 4.78s 63.90s
before / no-notes 4.77s 63.61s
before / no-fanout 56.59s 65.19s
16tree / no-notes 4.73s 63.80s
16tree / no-fanout 30.21s 65.11s
16tree / 2_38 5.53s 65.24s
16tree / 2_2_36 5.15s 65.12s
concat / no-notes 4.80s 64.21s
concat / no-fanout 30.66s 65.35s
concat / 2_38 5.64s 65.87s
concat / 2_2_36 5.23s 66.44s
Conclusion: There is a measurable impact (about .1s or so in the 100 x
'git log -n10' case), but I think this is low enough to be acceptable.
Johan Herland (17):
Teach "-m <msg>" and "-F <file>" to "git notes edit"
fast-import: Add support for importing commit notes
t3302-notes-index-expensive: Speed up create_repo()
Add flags to get_commit_notes() to control the format of the note string
Teach notes code to free its internal data structures on request.
Teach the notes lookup code to parse notes trees with various fanout schemes
Add selftests verifying that we can parse notes trees with various fanouts
Refactor notes code to concatenate multiple notes annotating the same object
Add selftests verifying that multiple notes for the same commits are concatenated correctly
Notes API: get_commit_notes() -> format_note() + remove the commit restriction
Notes API: init_notes(): Initialize the notes tree from the given notes ref
Notes API: add_note(): Add note objects to the internal notes tree structure
Notes API: get_note(): Return the note annotating the given object
Notes API: for_each_note(): Traverse the entire notes tree with a callback
Notes API: Allow multiple concurrent notes trees with new struct notes_tree
Refactor notes concatenation into a flexible interface for combining notes
fast-import: Proper notes tree manipulation using the notes API
Johannes Schindelin (5):
Introduce commit notes
Add a script to edit/inspect notes
Speed up git notes lookup
Add an expensive test for git-notes
Add '%N'-format for pretty-printing commit notes
.gitignore | 1 +
Documentation/config.txt | 13 +
Documentation/git-fast-import.txt | 45 +++-
Documentation/git-notes.txt | 60 ++++
Documentation/pretty-formats.txt | 1 +
Makefile | 3 +
cache.h | 4 +
command-list.txt | 1 +
commit.c | 1 +
config.c | 5 +
environment.c | 1 +
fast-import.c | 176 +++++++++++-
git-notes.sh | 121 ++++++++
notes.c | 579 +++++++++++++++++++++++++++++++++++++
notes.h | 121 ++++++++
pretty.c | 10 +
t/t3301-notes.sh | 150 ++++++++++
t/t3302-notes-index-expensive.sh | 118 ++++++++
t/t3303-notes-subtrees.sh | 188 ++++++++++++
t/t9300-fast-import.sh | 296 +++++++++++++++++++
20 files changed, 1883 insertions(+), 11 deletions(-)
create mode 100644 Documentation/git-notes.txt
create mode 100755 git-notes.sh
create mode 100644 notes.c
create mode 100644 notes.h
create mode 100755 t/t3301-notes.sh
create mode 100755 t/t3302-notes-index-expensive.sh
create mode 100755 t/t3303-notes-subtrees.sh
From: Johan Herland <hidden> Date: 2016-06-15 22:47:29
When adding a note to an object that already has an existing note, the
current solution is to concatenate the contents of the two notes. However,
the caller may instead wish to _overwrite_ the existing note with the new
note, or maybe even _ignore_ the new note, and keep the existing one. There
might also be other ways of combining notes that are only known to the
caller.
Therefore, instead of unconditionally concatenating notes, we let the caller
specify how to combine notes, by passing in a pointer to a function for
combining notes. The caller may choose to implement its own function for
notes combining, but normally one of the following three conveniently
supplied notes combination functions will be sufficient:
- combine_notes_concatenate() combines the two notes by appending the
contents of the new note to the contents of the existing note.
- combine_notes_overwrite() replaces the existing note with the new note.
- combine_notes_ignore() keeps the existing note, and ignores the new note.
A combine_notes function can be passed to init_notes() to choose a default
combine_notes function for that notes tree. If NULL is given, the notes tree
falls back to combine_notes_concatenate() as the ultimate default.
A combine_notes function can also be passed directly to add_note(), to
control the notes combining behaviour for a note addition in particular.
If NULL is passed, the combine_notes function registered for the given
notes tree is used.
Signed-off-by: Johan Herland <redacted>
---
notes.c | 132 +++++++++++++++++++++++++++++++++++---------------------------
notes.h | 34 +++++++++++++++-
2 files changed, 106 insertions(+), 60 deletions(-)
@@ -127,55 +127,12 @@ static struct leaf_node *note_tree_find(struct int_node *tree, unsigned char n,returnNULL;}-/* Create a new blob object by concatenating the two given blob objects */-staticintconcatenate_notes(unsignedchar*cur_sha1,-constunsignedchar*new_sha1)-{-char*cur_msg,*new_msg,*buf;-unsignedlongcur_len,new_len,buf_len;-enumobject_typecur_type,new_type;-intret;--/* read in both note blob objects */-new_msg=read_sha1_file(new_sha1,&new_type,&new_len);-if(!new_msg||!new_len||new_type!=OBJ_BLOB){-free(new_msg);-return0;-}-cur_msg=read_sha1_file(cur_sha1,&cur_type,&cur_len);-if(!cur_msg||!cur_len||cur_type!=OBJ_BLOB){-free(cur_msg);-free(new_msg);-hashcpy(cur_sha1,new_sha1);-return0;-}--/* we will separate the notes by a newline anyway */-if(cur_msg[cur_len-1]=='\n')-cur_len--;--/* concatenate cur_msg and new_msg into buf */-buf_len=cur_len+1+new_len;-buf=(char*)xmalloc(buf_len);-memcpy(buf,cur_msg,cur_len);-buf[cur_len]='\n';-memcpy(buf+cur_len+1,new_msg,new_len);--free(cur_msg);-free(new_msg);--/* create a new blob object from buf */-ret=write_sha1_file(buf,buf_len,"blob",cur_sha1);-free(buf);-returnret;-}-/**Toinsertaleaf_node:*Searchtothetreelocationappropriateforthegivenleaf_node'skey:*-Iflocationisunused(NULL),storethetweakedpointerdirectlythere*-Iflocationholdsanoteentrythatmatchesthenote-to-be-inserted,then-*concatenatethetwonotes.+*combinethetwonotes(bycallingthegivencombine_notesfunction).*-Iflocationholdsanoteentrythatmatchesthesubtree-to-be-inserted,*thenunpackthesubtree-to-be-insertedintothelocation.*-Iflocationholdsamatchingsubtreeentry,unpackthesubtreeatthat
@@ -184,7 +141,8 @@ static int concatenate_notes(unsigned char *cur_sha1,*node-to-be-inserted,andstorethenewint_nodeintothelocation.*/staticvoidnote_tree_insert(structint_node*tree,unsignedcharn,-structleaf_node*entry,unsignedchartype)+structleaf_node*entry,unsignedchartype,+combine_notes_fncombine_notes){structint_node*new_node;structleaf_node*l;
@@ -205,12 +163,11 @@ static void note_tree_insert(struct int_node *tree, unsigned char n,if(!hashcmp(l->val_sha1,entry->val_sha1))return;-if(concatenate_notes(l->val_sha1,-entry->val_sha1))-die("failed to concatenate note %s "-"into note %s for object %s",-sha1_to_hex(entry->val_sha1),+if(combine_notes(l->val_sha1,entry->val_sha1))+die("failed to combine notes %s and %s"+" for object %s",sha1_to_hex(l->val_sha1),+sha1_to_hex(entry->val_sha1),sha1_to_hex(l->key_sha1));free(entry);return;
@@ -243,9 +200,9 @@ static void note_tree_insert(struct int_node *tree, unsigned char n,assert(GET_PTR_TYPE(*p)==PTR_TYPE_NOTE||GET_PTR_TYPE(*p)==PTR_TYPE_SUBTREE);new_node=(structint_node*)xcalloc(sizeof(structint_node),1);-note_tree_insert(new_node,n+1,l,GET_PTR_TYPE(*p));+note_tree_insert(new_node,n+1,l,GET_PTR_TYPE(*p),combine_notes);*p=SET_PTR_TYPE(new_node,PTR_TYPE_INTERNAL);-note_tree_insert(new_node,n+1,entry,type);+note_tree_insert(new_node,n+1,entry,type,combine_notes);}/* Free the entire notes data contained in the given tree */
@@ -432,7 +390,59 @@ redo:return0;}-voidinit_notes(structnotes_tree*t,constchar*notes_ref,intflags)+intcombine_notes_concatenate(unsignedchar*cur_sha1,constunsignedchar*new_sha1)+{+char*cur_msg,*new_msg,*buf;+unsignedlongcur_len,new_len,buf_len;+enumobject_typecur_type,new_type;+intret;++/* read in both note blob objects */+new_msg=read_sha1_file(new_sha1,&new_type,&new_len);+if(!new_msg||!new_len||new_type!=OBJ_BLOB){+free(new_msg);+return0;+}+cur_msg=read_sha1_file(cur_sha1,&cur_type,&cur_len);+if(!cur_msg||!cur_len||cur_type!=OBJ_BLOB){+free(cur_msg);+free(new_msg);+hashcpy(cur_sha1,new_sha1);+return0;+}++/* we will separate the notes by a newline anyway */+if(cur_msg[cur_len-1]=='\n')+cur_len--;++/* concatenate cur_msg and new_msg into buf */+buf_len=cur_len+1+new_len;+buf=(char*)xmalloc(buf_len);+memcpy(buf,cur_msg,cur_len);+buf[cur_len]='\n';+memcpy(buf+cur_len+1,new_msg,new_len);+free(cur_msg);+free(new_msg);++/* create a new blob object from buf */+ret=write_sha1_file(buf,buf_len,"blob",cur_sha1);+free(buf);+returnret;+}++intcombine_notes_overwrite(unsignedchar*cur_sha1,constunsignedchar*new_sha1)+{+hashcpy(cur_sha1,new_sha1);+return0;+}++intcombine_notes_ignore(unsignedchar*cur_sha1,constunsignedchar*new_sha1)+{+return0;+}++voidinit_notes(structnotes_tree*t,constchar*notes_ref,+combine_notes_fncombine_notes,intflags){unsignedcharsha1[20],object_sha1[20];unsignedmode;
@@ -36,14 +61,19 @@ struct notes_tree {**Ifyoupasst==NULL,thedefaultinternalnotes_treewillbeinitialized.*+*Thecombine_notesfunctionthatispassedbecomesthedefaultcombine_notes+*functionforthegivennotes_tree.IfNULLispassed,thedefault+*combine_notesfunctioniscombine_notes_concatenate().+**Precondition:Thenotes_treestructureiszeroed(thiscanbeachievedwith*memset(t,0,sizeof(structnotes_tree)))*/-voidinit_notes(structnotes_tree*t,constchar*notes_ref,intflags);+voidinit_notes(structnotes_tree*t,constchar*notes_ref,+combine_notes_fncombine_notes,intflags);/* Add the given note object to the given notes_tree structure */voidadd_note(structnotes_tree*t,constunsignedchar*object_sha1,-constunsignedchar*note_sha1);+constunsignedchar*note_sha1,combine_notes_fncombine_notes);/* Get the note object SHA1 containing the note data for the given object */constunsignedchar*get_note(structnotes_tree*t,
From: Johan Herland <hidden> Date: 2016-06-15 22:47:29
From: Johannes Schindelin <redacted>
The script 'git notes' allows you to edit and show commit notes, by
calling either
git notes show <commit>
or
git notes edit <commit>
This patch has been improved by the following contributions:
- Tor Arne Vestbø: fix printing of multi-line notes
- Michael J Gruber: test and handle empty notes gracefully
- Thomas Rast:
- only clean up message file when editing
- use GIT_EDITOR and core.editor over VISUAL/EDITOR
- t3301: fix confusing quoting in test for valid notes ref
- t3301: use test_must_fail instead of !
- refuse to edit notes outside refs/notes/
- Junio C Hamano: tests: fix "export var=val"
- Christian Couder: documentation: fix 'linkgit' macro in "git-notes.txt"
- Johan Herland: minor cleanup and bugfixing in git-notes.sh (v2)
Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Tor Arne Vestbø <redacted>
Signed-off-by: Michael J Gruber <redacted>
Signed-off-by: Thomas Rast <redacted>
Signed-off-by: Christian Couder <redacted>
Signed-off-by: Johan Herland <redacted>
Signed-off-by: Junio C Hamano <redacted>
---
.gitignore | 1 +
Documentation/git-notes.txt | 46 +++++++++++++++++
Makefile | 1 +
command-list.txt | 1 +
git-notes.sh | 73 +++++++++++++++++++++++++++
t/t3301-notes.sh | 114 +++++++++++++++++++++++++++++++++++++++++++
6 files changed, 236 insertions(+), 0 deletions(-)
create mode 100644 Documentation/git-notes.txt
create mode 100755 git-notes.sh
create mode 100755 t/t3301-notes.sh
@@ -0,0 +1,46 @@+git-notes(1)+============++NAME+----+git-notes - Add/inspect commit notes++SYNOPSIS+--------+[verse]+'git-notes' (edit | show) [commit]++DESCRIPTION+-----------+This command allows you to add notes to commit messages, without+changing the commit. To discern these notes from the message stored+in the commit object, the notes are indented like the message, after+an unindented line saying "Notes:".++To disable commit notes, you have to set the config variable+core.notesRef to the empty string. Alternatively, you can set it+to a different ref, something like "refs/notes/bugzilla". This setting+can be overridden by the environment variable "GIT_NOTES_REF".+++SUBCOMMANDS+-----------++edit::+ Edit the notes for a given commit (defaults to HEAD).++show::+ Show the notes for a given commit (defaults to HEAD).+++Author+------+Written by Johannes Schindelin <johannes.schindelin@gmx.de>++Documentation+-------------+Documentation by Johannes Schindelin++GIT+---+Part of the linkgit:git[7] suite
@@ -0,0 +1,73 @@+#!/bin/sh++USAGE="(edit | show) [commit]"+.git-sh-setup++test-n"$3"&&usage++test-z"$1"&&usage+ACTION="$1";shift++test-z"$GIT_NOTES_REF"&&GIT_NOTES_REF="$(gitconfigcore.notesref)"+test-z"$GIT_NOTES_REF"&&GIT_NOTES_REF="refs/notes/commits"++COMMIT=$(gitrev-parse--verify--defaultHEAD"$@")||+die"Invalid commit: $@"++case"$ACTION"in+edit)+if["${GIT_NOTES_REF#refs/notes/}"="$GIT_NOTES_REF"];then+die"Refusing to edit notes in $GIT_NOTES_REF (outside of refs/notes/)"+fi++MSG_FILE="$GIT_DIR/new-notes-$COMMIT"+GIT_INDEX_FILE="$MSG_FILE.idx"+exportGIT_INDEX_FILE++trap'+test-f"$MSG_FILE"&&rm"$MSG_FILE"+test-f"$GIT_INDEX_FILE"&&rm"$GIT_INDEX_FILE"+'0++GIT_NOTES_REF=gitlog-1$COMMIT|sed"s/^/#/">"$MSG_FILE"++CURRENT_HEAD=$(gitshow-ref"$GIT_NOTES_REF"|cut-f1-d' ')+if[-z"$CURRENT_HEAD"];then+PARENT=+else+PARENT="-p $CURRENT_HEAD"+gitread-tree"$GIT_NOTES_REF"||die"Could not read index"+gitcat-fileblob:$COMMIT>>"$MSG_FILE"2>/dev/null+fi++core_editor="$(gitconfigcore.editor)"+${GIT_EDITOR:-${core_editor:-${VISUAL:-${EDITOR:-vi}}}}"$MSG_FILE"++grep-v^#<"$MSG_FILE"|gitstripspace>"$MSG_FILE".processed+mv"$MSG_FILE".processed"$MSG_FILE"+if[-s"$MSG_FILE"];then+BLOB=$(githash-object-w"$MSG_FILE")||+die"Could not write into object database"+gitupdate-index--add--cacheinfo0644$BLOB$COMMIT||+die"Could not write index"+else+test-z"$CURRENT_HEAD"&&+die"Will not initialise with empty tree"+gitupdate-index--force-remove$COMMIT||+die"Could not update index"+fi++TREE=$(gitwrite-tree)||die"Could not write tree"+NEW_HEAD=$(echoAnnotate$COMMIT|gitcommit-tree$TREE$PARENT)||+die"Could not annotate"+gitupdate-ref-m"Annotate $COMMIT"\+"$GIT_NOTES_REF"$NEW_HEAD$CURRENT_HEAD+;;+show)+gitrev-parse-q--verify"$GIT_NOTES_REF":$COMMIT>/dev/null||+die"No note for commit $COMMIT."+gitshow"$GIT_NOTES_REF":$COMMIT+;;+*)+usage+esac
From: Johan Herland <hidden> Date: 2016-06-15 22:47:29
Creating repos with 10/100/1000/10000 commits and notes takes a lot of time.
However, using git-fast-import to do the job is a lot more efficient than
using plumbing commands to do the same.
This patch decreases the overall run-time of this test on my machine from
~3 to ~1 minutes.
Signed-off-by: Johan Herland <redacted>
Acked-by: Johannes Schindelin <redacted>
---
t/t3302-notes-index-expensive.sh | 74 ++++++++++++++++++++++++--------------
1 files changed, 47 insertions(+), 27 deletions(-)
From: Johan Herland <hidden> Date: 2016-06-15 22:47:29
The new struct notes_tree encapsulates access to a specific notes tree.
It is provided to allow callers to interface with several different notes
trees simultaneously.
A struct notes_tree * parameter is added to every function in the notes API.
In all cases, NULL can be passed, in which case, a falback "default" notes
tree (declared in notes.c) is used.
Signed-off-by: Johan Herland <redacted>
---
notes.c | 67 ++++++++++++++++++++++++++++++++++++++-----------------------
notes.h | 57 +++++++++++++++++++++++++++++++++++++--------------
pretty.c | 4 +-
3 files changed, 85 insertions(+), 43 deletions(-)
@@ -10,35 +25,43 @@#define NOTES_INIT_EMPTY 1/*-*Initializeinternalnotestreestructurewiththenotestreeatthegiven+*Initializethegivennotes_treewiththenotestreestructureatthegiven*ref.IfgivenrefisNULL,thevalueofthe$GIT_NOTES_REFenvironment*variableisused,andifthatismissing,thedefaultnotesrefisused*("refs/notes/commits").*-*Ifyouneedtore-intializetheinternalnotestreestructure(e.g.loading-*fromadifferentnotesref),pleasefirstde-initializethecurrentnotes-*treebycallingfree_notes().+*Ifyouneedtore-intializeanotes_treestructure(e.g.whenswitchingfrom+*onenotesreftoanother),youmustfirstde-initializethenotes_tree+*structurebycallingfree_notes(structnotes_tree*).+*+*Ifyoupasst==NULL,thedefaultinternalnotes_treewillbeinitialized.+*+*Precondition:Thenotes_treestructureiszeroed(thiscanbeachievedwith+*memset(t,0,sizeof(structnotes_tree)))*/-voidinit_notes(constchar*notes_ref,intflags);+voidinit_notes(structnotes_tree*t,constchar*notes_ref,intflags);-/* Add the given note object to the internal notes tree structure */-voidadd_note(constunsignedchar*object_sha1,+/* Add the given note object to the given notes_tree structure */+voidadd_note(structnotes_tree*t,constunsignedchar*object_sha1,constunsignedchar*note_sha1);/* Get the note object SHA1 containing the note data for the given object */-constunsignedchar*get_note(constunsignedchar*object_sha1);+constunsignedchar*get_note(structnotes_tree*t,+constunsignedchar*object_sha1);/*-*Callsthespecifiedfunctionforeachnoteuntilitreturnsnonzero,-*andreturnsthevalue+*Callsthespecifiedfunctionforeachnoteinthegivennotes_tree+*+*Ifthecallbackreturnsnonzero,thenotewalkisaborted,andthereturn+*valuefromthecallbackisreturnedfromfor_each_note().*/typedefinteach_note_fn(constunsignedchar*object_sha1,constunsignedchar*note_sha1,constchar*note_tree_path,void*cb_data);-intfor_each_note(each_note_fnfn,void*cb_data);+intfor_each_note(structnotes_tree*t,each_note_fnfn,void*cb_data);-/* Free (and de-initialize) the internal notes tree structure */-voidfree_notes(void);+/* Free (and de-initialize) the give notes_tree structure */+voidfree_notes(structnotes_tree*t);/* Flags controlling how notes are formatted */#define NOTES_SHOW_HEADER 1
From: Johan Herland <hidden> Date: 2016-06-15 22:47:29
This patch adds the following flags to get_commit_notes() for adjusting the
format of the produced note string:
- NOTES_SHOW_HEADER: Print "Notes:" line before the notes contents
- NOTES_INDENT: Indent notes contents by 4 spaces
Suggested-by: Johannes Schindelin <redacted>
Signed-off-by: Johan Herland <redacted>
---
notes.c | 8 +++++---
notes.h | 5 ++++-
pretty.c | 3 ++-
3 files changed, 11 insertions(+), 5 deletions(-)
From: Johan Herland <hidden> Date: 2016-06-15 22:47:29
This patch teaches 'git fast-import' to use the notes API to organize
the manipulation of note objects through a fast-import stream. Note
objects are added to the notes tree through the 'N' command, and when
we're about to store the tree object for the current commit, we walk
through the notes tree and insert all the notes into the stored tree.
Signed-off-by: Johan Herland <redacted>
---
fast-import.c | 98 ++++++++++++++++++++++++++++--
t/t9300-fast-import.sh | 156 ++++++++++++++++++++++++++++++++++++++++++++----
2 files changed, 235 insertions(+), 19 deletions(-)
@@ -2254,6 +2264,68 @@ static struct hash_list *parse_merge(unsigned int *count)returnlist;}+staticstructnotes_tree*new_notes_tree(structbranch*b)+{+structnotes_tree_list*ret=(structnotes_tree_list*)+xcalloc(sizeof(structnotes_tree_list),1);+init_notes(&ret->tree,b->name,combine_notes_overwrite,NOTES_INIT_EMPTY);+ret->next=notes_trees;+notes_trees=ret;+b->has_notes=1;+return&ret->tree;+}++staticstructnotes_tree*get_notes_tree(structbranch*b)+{+structnotes_tree_list*cur=notes_trees;+if(!b->has_notes)+returnNULL;+while(cur&&strcmp(cur->tree.ref,b->name))+cur=cur->next;+assert(cur);+return&cur->tree;+}++staticvoiddelete_notes_tree(structbranch*b,structnotes_tree**tree)+{+structnotes_tree_list*cur=notes_trees,*prev=NULL;+while(cur&&strcmp(cur->tree.ref,b->name)){+prev=cur;+cur=cur->next;+}+assert(cur&&&cur->tree==*tree);+if(prev)+prev->next=cur->next;+else+notes_trees=cur->next;+free_notes(&cur->tree);+free(cur);+*tree=NULL;+b->has_notes=0;+}++staticintwrite_notes_set_helper(+constunsignedchar*object_sha1,+constunsignedchar*note_sha1,+constchar*note_tree_path,+void*cb_data)+{+structtree_entry*t=(structtree_entry*)cb_data;+tree_content_set(t,note_tree_path,note_sha1,S_IFREG|0644,NULL);+return0;+}++staticintwrite_notes_remove_helper(+constunsignedchar*object_sha1,+constunsignedchar*note_sha1,+constchar*note_tree_path,+void*cb_data)+{+structtree_entry*t=(structtree_entry*)cb_data;+tree_content_remove(t,note_tree_path,NULL);+return0;+}+staticvoidparse_new_commit(void){staticstructstrbufmsg=STRBUF_INIT;
@@ -2263,6 +2335,7 @@ static void parse_new_commit(void)char*committer=NULL;structhash_list*merge_list=NULL;unsignedintmerge_count;+structnotes_tree*notes;/* Obtain the branch name from the rest of our command */sp=strchr(command_buf.buf,' ')+1;
@@ -2316,6 +2400,8 @@ static void parse_new_commit(void)}/* build the tree and the commit */+if(notes)+for_each_note(notes,write_notes_set_helper,&b->branch_tree);store_tree(&b->branch_tree);hashcpy(b->branch_tree.versions[0].sha1,b->branch_tree.versions[1].sha1);
@@ -1092,9 +1092,12 @@ test_expect_success 'P: fail on blob mark in gitlink' '### series Q (notes)###-note1_data="Note for the first commit"-note2_data="Note for the second commit"-note3_data="Note for the third commit"+note1_data="The first note for the first commit"+note2_data="The first note for the second commit"+note3_data="The first note for the third commit"+note1b_data="The second note for the first commit"+note1c_data="The third note for the first commit"+note2b_data="The second note for the second commit" test_tick cat>input<<INPUT_END
From: Johan Herland <hidden> Date: 2016-06-15 22:47:29
Currently, having multiple notes referring to the same commit from various
locations in the notes tree is strongly discouraged, since only one of those
notes will be parsed and shown.
This patch teaches the notes code to _concatenate_ multiple notes that
annotate the same commit. Notes are concatenated by creating a new blob
object containing the concatenation of the notes in question, and
replacing them with the concatenated note in the internal notes tree
structure.
Getting the concatenation right requires being more proactive in unpacking
subtree entries in the internal notes tree structure, so that we don't return
a note prematurely (i.e. before having found all other notes that annotate
the same object). As such, this patch may incur a small performance penalty.
Suggested-by: Sam Vilain <redacted>
Re-suggested-by: Johannes Schindelin [off-list ref]
Signed-off-by: Johan Herland <redacted>
---
notes.c | 243 +++++++++++++++++++++++++++++++++++++++++---------------------
1 files changed, 161 insertions(+), 82 deletions(-)
@@ -59,115 +59,196 @@ static void load_subtree(struct leaf_node *subtree, struct int_node *node,unsignedintn);/*-*Tofindaleaf_node:+*Searchthetreeuntiltheappropriatelocationforthegivenkeyisfound:*1.Startattherootnode,withn=0-*2.Usethenthnibbleofthekeyasanindexintoa:-*-Ifa[n]isanint_node,recurseintothatnodeandincrementn-*-Ifaleaf_nodewithmatchingkey,returnleaf_node(assertnoteentry)+*2.Ifa[0]atthecurrentlevelisamatchingsubtreeentry,unpackthat+*subtreeentryandremoveit;restartsearchatthecurrentlevel.+*3.Usethenthnibbleofthekeyasanindexintoa:+*-Ifa[n]isanint_node,recursefrom#2intothatnodeandincrementn*-Ifamatchingsubtreeentry,unpackthatsubtreeentry(andremoveit);*restartsearchatthecurrentlevel.-*-Otherwise,weendupataNULLpointer,oranon-matchingleaf_node.-*Backtrackoutoftherecursion,onelevelatatimeandchecka[0]:-*-Ifa[0]atthecurrentlevelisamatchingsubtreeentry,unpackthat-*subtreeentry(andremoveit);restartsearchatthecurrentlevel.+*-Otherwise,wehavefoundoneofthefollowing:+*-asubtreeentrywhichdoesnotmatchthekey+*-anoteentrywhichmayormaynotmatchthekey+*-anunusedleafnode(NULL)+*Inanycase,set*treeand*n,andreturnpointertothetreelocation.*/-staticstructleaf_node*note_tree_find(structint_node*tree,unsignedcharn,-constunsignedchar*key_sha1)+staticvoid**note_tree_search(structint_node**tree,+unsignedchar*n,constunsignedchar*key_sha1){structleaf_node*l;-unsignedchari=GET_NIBBLE(n,key_sha1);-void*p=tree->a[i];+unsignedchari;+void*p=(*tree)->a[0];+if(GET_PTR_TYPE(p)==PTR_TYPE_SUBTREE){+l=(structleaf_node*)CLR_PTR_TYPE(p);+if(!SUBTREE_SHA1_PREFIXCMP(key_sha1,l->key_sha1)){+/* unpack tree and resume search */+(*tree)->a[0]=NULL;+load_subtree(l,*tree,*n);+free(l);+returnnote_tree_search(tree,n,key_sha1);+}+}++i=GET_NIBBLE(*n,key_sha1);+p=(*tree)->a[i];switch(GET_PTR_TYPE(p)){casePTR_TYPE_INTERNAL:-l=note_tree_find(CLR_PTR_TYPE(p),n+1,key_sha1);-if(l)-returnl;-break;-casePTR_TYPE_NOTE:-l=(structleaf_node*)CLR_PTR_TYPE(p);-if(!hashcmp(key_sha1,l->key_sha1))-returnl;/* return note object matching given key */-break;+*tree=CLR_PTR_TYPE(p);+(*n)++;+returnnote_tree_search(tree,n,key_sha1);casePTR_TYPE_SUBTREE:l=(structleaf_node*)CLR_PTR_TYPE(p);if(!SUBTREE_SHA1_PREFIXCMP(key_sha1,l->key_sha1)){/* unpack tree and resume search */-tree->a[i]=NULL;-load_subtree(l,tree,n);+(*tree)->a[i]=NULL;+load_subtree(l,*tree,*n);free(l);-returnnote_tree_find(tree,n,key_sha1);+returnnote_tree_search(tree,n,key_sha1);}-break;-casePTR_TYPE_NULL:+/* fall through */default:-assert(!p);-break;+return&((*tree)->a[i]);}+}-/*-*Didnotfindkeyatthis(oranylower)level.-*Checkifthere'samatchingsubtreeentryintree->a[0].-*Ifso,unpacktreeandresumesearch.-*/-p=tree->a[0];-if(GET_PTR_TYPE(p)!=PTR_TYPE_SUBTREE)-returnNULL;-l=(structleaf_node*)CLR_PTR_TYPE(p);-if(!SUBTREE_SHA1_PREFIXCMP(key_sha1,l->key_sha1)){-/* unpack tree and resume search */-tree->a[0]=NULL;-load_subtree(l,tree,n);-free(l);-returnnote_tree_find(tree,n,key_sha1);+/*+*Tofindaleaf_node:+*Searchtothetreelocationappropriateforthegivenkey:+*Ifanoteentrywithmatchingkey,returnthenoteentry,elsereturnNULL.+*/+staticstructleaf_node*note_tree_find(structint_node*tree,unsignedcharn,+constunsignedchar*key_sha1)+{+void**p=note_tree_search(&tree,&n,key_sha1);+if(GET_PTR_TYPE(*p)==PTR_TYPE_NOTE){+structleaf_node*l=(structleaf_node*)CLR_PTR_TYPE(*p);+if(!hashcmp(key_sha1,l->key_sha1))+returnl;}returnNULL;}+/* Create a new blob object by concatenating the two given blob objects */+staticintconcatenate_notes(unsignedchar*cur_sha1,+constunsignedchar*new_sha1)+{+char*cur_msg,*new_msg,*buf;+unsignedlongcur_len,new_len,buf_len;+enumobject_typecur_type,new_type;+intret;++/* read in both note blob objects */+new_msg=read_sha1_file(new_sha1,&new_type,&new_len);+if(!new_msg||!new_len||new_type!=OBJ_BLOB){+free(new_msg);+return0;+}+cur_msg=read_sha1_file(cur_sha1,&cur_type,&cur_len);+if(!cur_msg||!cur_len||cur_type!=OBJ_BLOB){+free(cur_msg);+free(new_msg);+hashcpy(cur_sha1,new_sha1);+return0;+}++/* we will separate the notes by a newline anyway */+if(cur_msg[cur_len-1]=='\n')+cur_len--;++/* concatenate cur_msg and new_msg into buf */+buf_len=cur_len+1+new_len;+buf=(char*)xmalloc(buf_len);+memcpy(buf,cur_msg,cur_len);+buf[cur_len]='\n';+memcpy(buf+cur_len+1,new_msg,new_len);++free(cur_msg);+free(new_msg);++/* create a new blob object from buf */+ret=write_sha1_file(buf,buf_len,"blob",cur_sha1);+free(buf);+returnret;+}+/**Toinsertaleaf_node:-*1.Startattherootnode,withn=0-*2.Usethenthnibbleofthekeyasanindexintoa:-*-Ifa[n]isNULL,storethetweakedpointerdirectlyintoa[n]-*-Ifa[n]isanint_node,recurseintothatnodeandincrementn-*-Ifa[n]isaleaf_node:-*1.Checkifthey'reequal,andhandlethat(abort?overwrite?)-*2.Createanewint_node,andstorebothleaf_nodesthere-*3.Storethenewint_nodeintoa[n].+*Searchtothetreelocationappropriateforthegivenleaf_node'skey:+*-Iflocationisunused(NULL),storethetweakedpointerdirectlythere+*-Iflocationholdsanoteentrythatmatchesthenote-to-be-inserted,then+*concatenatethetwonotes.+*-Iflocationholdsanoteentrythatmatchesthesubtree-to-be-inserted,+*thenunpackthesubtree-to-be-insertedintothelocation.+*-Iflocationholdsamatchingsubtreeentry,unpackthesubtreeatthat+*location,andrestarttheinsertoperationfromthatlevel.+*-Else,createanewint_node,holdingboththenode-at-locationandthe+*node-to-be-inserted,andstorethenewint_nodeintothelocation.*/-staticintnote_tree_insert(structint_node*tree,unsignedcharn,-conststructleaf_node*entry,unsignedchartype)+staticvoidnote_tree_insert(structint_node*tree,unsignedcharn,+structleaf_node*entry,unsignedchartype){structint_node*new_node;-conststructleaf_node*l;-intret;-unsignedchari=GET_NIBBLE(n,entry->key_sha1);-void*p=tree->a[i];-assert(GET_PTR_TYPE(entry)==PTR_TYPE_NULL);-switch(GET_PTR_TYPE(p)){+structleaf_node*l;+void**p=note_tree_search(&tree,&n,entry->key_sha1);++assert(GET_PTR_TYPE(entry)==0);/* no type bits set */+l=(structleaf_node*)CLR_PTR_TYPE(*p);+switch(GET_PTR_TYPE(*p)){casePTR_TYPE_NULL:-assert(!p);-tree->a[i]=SET_PTR_TYPE(entry,type);-return0;-casePTR_TYPE_INTERNAL:-returnnote_tree_insert(CLR_PTR_TYPE(p),n+1,entry,type);-default:-assert(GET_PTR_TYPE(p)==PTR_TYPE_NOTE||-GET_PTR_TYPE(p)==PTR_TYPE_SUBTREE);-l=(conststructleaf_node*)CLR_PTR_TYPE(p);-if(!hashcmp(entry->key_sha1,l->key_sha1))-return-1;/* abort insert on matching key */-new_node=(structint_node*)-xcalloc(sizeof(structint_node),1);-ret=note_tree_insert(new_node,n+1,-CLR_PTR_TYPE(p),GET_PTR_TYPE(p));-if(ret){-free(new_node);-return-1;+assert(!*p);+*p=SET_PTR_TYPE(entry,type);+return;+casePTR_TYPE_NOTE:+switch(type){+casePTR_TYPE_NOTE:+if(!hashcmp(l->key_sha1,entry->key_sha1)){+/* skip concatenation if l == entry */+if(!hashcmp(l->val_sha1,entry->val_sha1))+return;++if(concatenate_notes(l->val_sha1,+entry->val_sha1))+die("failed to concatenate note %s "+"into note %s for commit %s",+sha1_to_hex(entry->val_sha1),+sha1_to_hex(l->val_sha1),+sha1_to_hex(l->key_sha1));+free(entry);+return;+}+break;+casePTR_TYPE_SUBTREE:+if(!SUBTREE_SHA1_PREFIXCMP(l->key_sha1,+entry->key_sha1)){+/* unpack 'entry' */+load_subtree(entry,tree,n);+free(entry);+return;+}+break;+}+break;+casePTR_TYPE_SUBTREE:+if(!SUBTREE_SHA1_PREFIXCMP(entry->key_sha1,l->key_sha1)){+/* unpack 'l' and restart insert */+*p=NULL;+load_subtree(l,tree,n);+free(l);+note_tree_insert(tree,n,entry,type);+return;}-tree->a[i]=SET_PTR_TYPE(new_node,PTR_TYPE_INTERNAL);-returnnote_tree_insert(new_node,n+1,entry,type);+break;}++/* non-matching leaf_node */+assert(GET_PTR_TYPE(*p)==PTR_TYPE_NOTE||+GET_PTR_TYPE(*p)==PTR_TYPE_SUBTREE);+new_node=(structint_node*)xcalloc(sizeof(structint_node),1);+note_tree_insert(new_node,n+1,l,GET_PTR_TYPE(*p));+*p=SET_PTR_TYPE(new_node,PTR_TYPE_INTERNAL);+note_tree_insert(new_node,n+1,entry,type);}/* Free the entire notes data contained in the given tree */
From: Johan Herland <hidden> Date: 2016-06-15 22:47:29
The "-m" and "-F" options are already the established method
(in both git-commit and git-tag) to specify a commit/tag message
without invoking the editor. This patch teaches "git notes edit"
to respect the same options for specifying a notes message without
invoking the editor.
Multiple "-m" and/or "-F" options are concatenated as separate
paragraphs.
The patch also updates the "git notes" documentation and adds
selftests for the new functionality. Unfortunately, the added
selftests include a couple of lines with trailing whitespace
(without these the test will fail). This may cause git to warn
about "whitespace errors".
This patch has been improved by the following contributions:
- Thomas Rast: fix trailing whitespace in t3301
Signed-off-by: Johan Herland <redacted>
---
Documentation/git-notes.txt | 16 ++++++++++-
git-notes.sh | 64 +++++++++++++++++++++++++++++++++++++-----
t/t3301-notes.sh | 36 ++++++++++++++++++++++++
3 files changed, 107 insertions(+), 9 deletions(-)
@@ -33,6 +33,20 @@ show:: Show the notes for a given commit (defaults to HEAD).+OPTIONS+-------+-m <msg>::+ Use the given note message (instead of prompting).+ If multiple `-m` (or `-F`) options are given, their+ values are concatenated as separate paragraphs.++-F <file>::+ Take the note message from the given file. Use '-' to+ read the note message from the standard input.+ If multiple `-F` (or `-m`) options are given, their+ values are concatenated as separate paragraphs.++ Author ------ Written by Johannes Schindelin <johannes.schindelin@gmx.de>
@@ -0,0 +1,104 @@+#!/bin/sh++test_description='Test commit notes organized in subtrees'++../test-lib.sh++number_of_commits=100++start_note_commit(){+test_tick&&+cat<<INPUT_END+commitrefs/notes/commits+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+data<<COMMIT+notes+COMMIT++fromrefs/notes/commits^0+deleteall+INPUT_END++}++verify_notes(){+gitlog|grep"^ ">output&&+i=$number_of_commits&&+while[$i-gt0];do+echo" commit #$i"&&+echo" note for commit #$i"&&+i=$(($i-1));+done>expect&&+test_cmpexpectoutput+}++test_expect_success"setup: create $number_of_commits commits"'++(+nr=0&&+while[$nr-lt$number_of_commits];do+nr=$(($nr+1))&&+test_tick&&+cat<<INPUT_END+commitrefs/heads/master+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+data<<COMMIT+commit#$nr+COMMIT++M644inlinefile+data<<EOF+fileincommit#$nr+EOF++INPUT_END++done&&+test_tick&&+cat<<INPUT_END+commitrefs/notes/commits+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+data<<COMMIT+nonotes+COMMIT++deleteall++INPUT_END++)|+gitfast-import--quiet&&+gitconfigcore.notesRefrefs/notes/commits+'++test_sha1_based(){+(+start_note_commit&&+nr=$number_of_commits&&+gitrev-listrefs/heads/master|+whilereadsha1;do+note_path=$(echo"$sha1"|sed"$1")+cat<<INPUT_END&&+M100644inline$note_path+data<<EOF+noteforcommit#$nr+EOF++INPUT_END++nr=$(($nr-1))+done+)|+gitfast-import--quiet+}++test_expect_success'test notes in 2/38-fanout''test_sha1_based "s|^..|&/|"'+test_expect_success'verify notes in 2/38-fanout''verify_notes'++test_expect_success'test notes in 4/36-fanout''test_sha1_based "s|^....|&/|"'+test_expect_success'verify notes in 4/36-fanout''verify_notes'++test_expect_success'test notes in 2/2/36-fanout''test_sha1_based "s|^\(..\)\(..\)|\1/\2/|"'+test_expect_success'verify notes in 2/2/36-fanout''verify_notes'++test_done
From: Johan Herland <hidden> Date: 2016-06-15 22:47:29
The semantics used when parsing notes trees (with regards to fanout subtrees)
follow Dscho's proposal fairly closely:
- No concatenation/merging of notes is performed. If there are several notes
objects referencing a given commit, only one of those objects are used.
- If a notes object for a given commit is present in the "root" notes tree,
no subtrees are consulted; the object in the root tree is used directly.
- If there are more than one subtree that prefix-matches the given commit,
only the subtree with the longest matching prefix is consulted. This
means that if the given commit is e.g. "deadbeef", and the notes tree have
subtrees "de" and "dead", then the following paths in the notes tree are
searched: "deadbeef", "dead/beef". Note that "de/adbeef" is NOT searched.
- Fanout directories (subtrees) must references a whole number of bytes
from the SHA1 sum they subdivide. E.g. subtrees "dead" and "de" are
acceptable; "d" and "dea" are not.
- Multiple levels of fanout are allowed. All the above rules apply
recursively. E.g. "de/adbeef" is preferred over "de/adbe/ef", etc.
This patch changes the in-memory datastructure for holding parsed notes:
Instead of holding all note (and subtree) entries in a hash table, a
simple 16-tree structure is used instead. The tree structure consists of
16-arrays as internal nodes, and note/subtree entries as leaf nodes. The
tree is traversed by indexing subsequent nibbles of the search key until
a leaf node is encountered. If a subtree entry is encountered while
searching for a note, the subtree is unpacked into the 16-tree structure,
and the search continues into that subtree.
The new algorithm performs significantly better in the cases where only
a fraction of the notes need to be looked up (this is assumed to be the
common case for notes lookup). The new code even performs marginally
better in the worst case (where _all_ the notes are looked up).
In addition to this, comes the massive performance win associated with
organizing the notes tree according to some fanout scheme. Even a simple
2/38 fanout scheme is dramatically quicker to traverse (going from tens of
seconds to sub-second runtimes).
As for memory usage, the new code is marginally better than the old code in
the worst case, but in the case of looking up only some notes from a notes
tree with proper fanout, the new code uses only a small fraction of the
memory needed to hold the entire notes tree.
However, there is one casualty of this patch. The old notes lookup code was
able to parse notes that were associated with non-SHA1s (e.g. refs). The new
code requires the referenced object to be named by a SHA1 sum. Still, this
is not considered a major setback, since the notes infrastructure was not
originally intended to annotate objects outside the Git object database.
Cc: Johannes Schindelin <redacted>
Signed-off-by: Johan Herland <redacted>
---
notes.c | 317 +++++++++++++++++++++++++++++++++++++++++++++++++--------------
1 files changed, 248 insertions(+), 69 deletions(-)
@@ -6,109 +6,288 @@#include"strbuf.h"#include"tree-walk.h"-structentry{-unsignedcharcommit_sha1[20];-unsignedcharnotes_sha1[20];+/*+*Useanon-balancingsimple16-treestructurewithstructint_nodeas+*internalnodes,andstructleaf_nodeasleafnodes.Eachint_nodehasa+*16-arrayofpointerstoitschildren.+*Thebottom2bitsofeachpointerisusedtoidentifythepointertype+*-ptr&3==0-NULLpointer,assert(ptr==NULL)+*-ptr&3==1-pointertonextinternalnode-casttostructint_node*+*-ptr&3==2-pointertonoteentry-casttostructleaf_node*+*-ptr&3==3-pointertosubtreeentry-casttostructleaf_node*+*+*Therootnodeisastaticallyallocatedstructint_node.+*/+structint_node{+void*a[16];};-structhash_map{-structentry*entries;-off_tcount,size;+/*+*Leafnodescomeintwovariants,noteentriesandsubtreeentries,+*distinguishedbytheLSboftheleafnodepointer(seeabove).+*Asanoteentry,thekeyistheSHA1ofthereferencedcommit,andthe+*valueistheSHA1ofthenoteobject.+*Asasubtreeentry,thekeyistheprefixSHA1(w/trailingNULs)ofthe+*referencedcommit,usingthelastbyteofthekeytostorethelengthof+*theprefix.ThevalueistheSHA1ofthetreeobjectcontainingthenotes+*subtree.+*/+structleaf_node{+unsignedcharkey_sha1[20];+unsignedcharval_sha1[20];};-staticintinitialized;-staticstructhash_maphash_map;+#define PTR_TYPE_NULL 0+#define PTR_TYPE_INTERNAL 1+#define PTR_TYPE_NOTE 2+#define PTR_TYPE_SUBTREE 3-staticinthash_index(structhash_map*map,constunsignedchar*sha1)-{-inti=((*(unsignedint*)sha1)%map->size);+#define GET_PTR_TYPE(ptr) ((uintptr_t) (ptr) & 3)+#define CLR_PTR_TYPE(ptr) ((void *) ((uintptr_t) (ptr) & ~3))+#define SET_PTR_TYPE(ptr, type) ((void *) ((uintptr_t) (ptr) | (type)))-for(;;){-unsignedchar*current=map->entries[i].commit_sha1;+#define GET_NIBBLE(n, sha1) (((sha1[n >> 1]) >> ((~n & 0x01) << 2)) & 0x0f)-if(!hashcmp(sha1,current))-returni;+#define SUBTREE_SHA1_PREFIXCMP(key_sha1, subtree_sha1) \+(memcmp(key_sha1,subtree_sha1,subtree_sha1[19]))-if(is_null_sha1(current))-return-1-i;+staticstructint_noderoot_node;-if(++i==map->size)-i=0;+staticintinitialized;++staticvoidload_subtree(structleaf_node*subtree,structint_node*node,+unsignedintn);++/*+*Tofindaleaf_node:+*1.Startattherootnode,withn=0+*2.Usethenthnibbleofthekeyasanindexintoa:+*-Ifa[n]isanint_node,recurseintothatnodeandincrementn+*-Ifaleaf_nodewithmatchingkey,returnleaf_node(assertnoteentry)+*-Ifamatchingsubtreeentry,unpackthatsubtreeentry(andremoveit);+*restartsearchatthecurrentlevel.+*-Otherwise,weendupataNULLpointer,oranon-matchingleaf_node.+*Backtrackoutoftherecursion,onelevelatatimeandchecka[0]:+*-Ifa[0]atthecurrentlevelisamatchingsubtreeentry,unpackthat+*subtreeentry(andremoveit);restartsearchatthecurrentlevel.+*/+staticstructleaf_node*note_tree_find(structint_node*tree,unsignedcharn,+constunsignedchar*key_sha1)+{+structleaf_node*l;+unsignedchari=GET_NIBBLE(n,key_sha1);+void*p=tree->a[i];++switch(GET_PTR_TYPE(p)){+casePTR_TYPE_INTERNAL:+l=note_tree_find(CLR_PTR_TYPE(p),n+1,key_sha1);+if(l)+returnl;+break;+casePTR_TYPE_NOTE:+l=(structleaf_node*)CLR_PTR_TYPE(p);+if(!hashcmp(key_sha1,l->key_sha1))+returnl;/* return note object matching given key */+break;+casePTR_TYPE_SUBTREE:+l=(structleaf_node*)CLR_PTR_TYPE(p);+if(!SUBTREE_SHA1_PREFIXCMP(key_sha1,l->key_sha1)){+/* unpack tree and resume search */+tree->a[i]=NULL;+load_subtree(l,tree,n);+free(l);+returnnote_tree_find(tree,n,key_sha1);+}+break;+casePTR_TYPE_NULL:+default:+assert(!p);+break;}++/*+*Didnotfindkeyatthis(oranylower)level.+*Checkifthere'samatchingsubtreeentryintree->a[0].+*Ifso,unpacktreeandresumesearch.+*/+p=tree->a[0];+if(GET_PTR_TYPE(p)!=PTR_TYPE_SUBTREE)+returnNULL;+l=(structleaf_node*)CLR_PTR_TYPE(p);+if(!SUBTREE_SHA1_PREFIXCMP(key_sha1,l->key_sha1)){+/* unpack tree and resume search */+tree->a[0]=NULL;+load_subtree(l,tree,n);+free(l);+returnnote_tree_find(tree,n,key_sha1);+}+returnNULL;}-staticvoidadd_entry(constunsignedchar*commit_sha1,-constunsignedchar*notes_sha1)+/*+*Toinsertaleaf_node:+*1.Startattherootnode,withn=0+*2.Usethenthnibbleofthekeyasanindexintoa:+*-Ifa[n]isNULL,storethetweakedpointerdirectlyintoa[n]+*-Ifa[n]isanint_node,recurseintothatnodeandincrementn+*-Ifa[n]isaleaf_node:+*1.Checkifthey'reequal,andhandlethat(abort?overwrite?)+*2.Createanewint_node,andstorebothleaf_nodesthere+*3.Storethenewint_nodeintoa[n].+*/+staticintnote_tree_insert(structint_node*tree,unsignedcharn,+conststructleaf_node*entry,unsignedchartype){-intindex;--if(hash_map.count+1>hash_map.size>>1){-inti,old_size=hash_map.size;-structentry*old=hash_map.entries;--hash_map.size=old_size?old_size<<1:64;-hash_map.entries=(structentry*)-xcalloc(sizeof(structentry),hash_map.size);--for(i=0;i<old_size;i++)-if(!is_null_sha1(old[i].commit_sha1)){-index=-1-hash_index(&hash_map,-old[i].commit_sha1);-memcpy(hash_map.entries+index,old+i,-sizeof(structentry));-}-free(old);+structint_node*new_node;+conststructleaf_node*l;+intret;+unsignedchari=GET_NIBBLE(n,entry->key_sha1);+void*p=tree->a[i];+assert(GET_PTR_TYPE(entry)==PTR_TYPE_NULL);+switch(GET_PTR_TYPE(p)){+casePTR_TYPE_NULL:+assert(!p);+tree->a[i]=SET_PTR_TYPE(entry,type);+return0;+casePTR_TYPE_INTERNAL:+returnnote_tree_insert(CLR_PTR_TYPE(p),n+1,entry,type);+default:+assert(GET_PTR_TYPE(p)==PTR_TYPE_NOTE||+GET_PTR_TYPE(p)==PTR_TYPE_SUBTREE);+l=(conststructleaf_node*)CLR_PTR_TYPE(p);+if(!hashcmp(entry->key_sha1,l->key_sha1))+return-1;/* abort insert on matching key */+new_node=(structint_node*)+xcalloc(sizeof(structint_node),1);+ret=note_tree_insert(new_node,n+1,+CLR_PTR_TYPE(p),GET_PTR_TYPE(p));+if(ret){+free(new_node);+return-1;+}+tree->a[i]=SET_PTR_TYPE(new_node,PTR_TYPE_INTERNAL);+returnnote_tree_insert(new_node,n+1,entry,type);}+}-index=hash_index(&hash_map,commit_sha1);-if(index<0){-index=-1-index;-hash_map.count++;+/* Free the entire notes data contained in the given tree */+staticvoidnote_tree_free(structint_node*tree)+{+unsignedinti;+for(i=0;i<16;i++){+void*p=tree->a[i];+switch(GET_PTR_TYPE(p)){+casePTR_TYPE_INTERNAL:+note_tree_free(CLR_PTR_TYPE(p));+/* fall through */+casePTR_TYPE_NOTE:+casePTR_TYPE_SUBTREE:+free(CLR_PTR_TYPE(p));+}}+}-hashcpy(hash_map.entries[index].commit_sha1,commit_sha1);-hashcpy(hash_map.entries[index].notes_sha1,notes_sha1);+/*+*ConvertapartialSHA1hexstringtothecorrespondingpartialSHA1value.+*-hex-PartialSHA1segmentinASCIIhexformat+*-hex_len-Lengthofabovesegment.Mustbemultipleof2between0and40+*-sha1-PartialSHA1valueiswrittenhere+*-sha1_len-Max#bytestostoreinsha1,Mustbe>=hex_len/2,and<20+*Returns-1onerror(invalidargumentsorinvalidSHA1(notinhexformat).+*Otherwise,returnsnumberofbyteswrittentosha1(i.e.hex_len/2).+*Padssha1withNULsuptosha1_len(notincludedinreturnedlength).+*/+staticintget_sha1_hex_segment(constchar*hex,unsignedinthex_len,+unsignedchar*sha1,unsignedintsha1_len)+{+unsignedinti,len=hex_len>>1;+if(hex_len%2!=0||len>sha1_len)+return-1;+for(i=0;i<len;i++){+unsignedintval=(hexval(hex[0])<<4)|hexval(hex[1]);+if(val&~0xff)+return-1;+*sha1++=val;+hex+=2;+}+for(;i<sha1_len;i++)+*sha1++=0;+returnlen;}-staticvoidinitialize_hash_map(constchar*notes_ref_name)+staticvoidload_subtree(structleaf_node*subtree,structint_node*node,+unsignedintn){-unsignedcharsha1[20],commit_sha1[20];-unsignedmode;+unsignedcharcommit_sha1[20];+unsignedintprefix_len;+intstatus;+void*buf;structtree_descdesc;structname_entryentry;-void*buf;++buf=fill_tree_descriptor(&desc,subtree->val_sha1);+if(!buf)+die("Could not read %s for notes-index",+sha1_to_hex(subtree->val_sha1));++prefix_len=subtree->key_sha1[19];+assert(prefix_len*2>=n);+memcpy(commit_sha1,subtree->key_sha1,prefix_len);+while(tree_entry(&desc,&entry)){+intlen=get_sha1_hex_segment(entry.path,strlen(entry.path),+commit_sha1+prefix_len,20-prefix_len);+if(len<0)+continue;/* entry.path is not a SHA1 sum. Skip */+len+=prefix_len;++/*+*IfcommitSHA1iscomplete(len==20),assumenoteobject+*IfcommitSHA1isincomplete(len<20),assumenotesubtree+*/+if(len<=20){+unsignedchartype=PTR_TYPE_NOTE;+structleaf_node*l=(structleaf_node*)+xcalloc(sizeof(structleaf_node),1);+hashcpy(l->key_sha1,commit_sha1);+hashcpy(l->val_sha1,entry.sha1);+if(len<20){+l->key_sha1[19]=(unsignedchar)len;+type=PTR_TYPE_SUBTREE;+}+status=note_tree_insert(node,n,l,type);+assert(!status);+}+}+free(buf);+}++staticvoidinitialize_notes(constchar*notes_ref_name)+{+unsignedcharsha1[20],commit_sha1[20];+unsignedmode;+structleaf_noderoot_tree;if(!notes_ref_name||read_ref(notes_ref_name,commit_sha1)||get_tree_entry(commit_sha1,"",sha1,&mode))return;-buf=fill_tree_descriptor(&desc,sha1);-if(!buf)-die("Could not read %s for notes-index",sha1_to_hex(sha1));--while(tree_entry(&desc,&entry))-if(!get_sha1(entry.path,commit_sha1))-add_entry(commit_sha1,entry.sha1);-free(buf);+hashclr(root_tree.key_sha1);+hashcpy(root_tree.val_sha1,sha1);+load_subtree(&root_tree,&root_node,0);}staticunsignedchar*lookup_notes(constunsignedchar*commit_sha1){-intindex;--if(!hash_map.size)-returnNULL;--index=hash_index(&hash_map,commit_sha1);-if(index<0)-returnNULL;-returnhash_map.entries[index].notes_sha1;+structleaf_node*found=note_tree_find(&root_node,0,commit_sha1);+if(found)+returnfound->val_sha1;+returnNULL;}voidfree_notes(void){-free(hash_map.entries);-memset(&hash_map,0,sizeof(structhash_map));+note_tree_free(&root_node);+memset(&root_node,0,sizeof(structint_node));initialized=0;}
From: Johan Herland <hidden> Date: 2016-06-15 22:47:29
This includes a first attempt at creating an optimal fanout scheme (which
is created on-the-fly, while traversing).
Signed-off-by: Johan Herland <redacted>
---
notes.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
notes.h | 9 +++++
2 files changed, 110 insertions(+), 0 deletions(-)
@@ -28,6 +28,15 @@ void add_note(const unsigned char *object_sha1,/* Get the note object SHA1 containing the note data for the given object */constunsignedchar*get_note(constunsignedchar*object_sha1);+/*+*Callsthespecifiedfunctionforeachnoteuntilitreturnsnonzero,+*andreturnsthevalue+*/+typedefinteach_note_fn(constunsignedchar*object_sha1,+constunsignedchar*note_sha1,constchar*note_tree_path,+void*cb_data);+intfor_each_note(each_note_fnfn,void*cb_data);+/* Free (and de-initialize) the internal notes tree structure */voidfree_notes(void);
From: Johan Herland <hidden> Date: 2016-06-15 22:47:29
Created by a simple cleanup and rename of lookup_notes().
Signed-off-by: Johan Herland <redacted>
---
notes.c | 15 ++++++++-------
notes.h | 3 +++
2 files changed, 11 insertions(+), 7 deletions(-)
@@ -25,6 +25,9 @@ void init_notes(const char *notes_ref, int flags);voidadd_note(constunsignedchar*object_sha1,constunsignedchar*note_sha1);+/* Get the note object SHA1 containing the note data for the given object */+constunsignedchar*get_note(constunsignedchar*object_sha1);+/* Free (and de-initialize) the internal notes tree structure */voidfree_notes(void);
From: Johan Herland <hidden> Date: 2016-06-15 22:47:29
There is really no reason why only commit objects can be annotated. By
changing the struct commit parameter to get_commit_notes() into a sha1 we
gain the ability to annotate any object type. To reflect this in the function
naming as well, we rename get_commit_notes() to format_note().
This patch also fixes comments and variable names throughout notes.c as a
consequence of the removal of the unnecessary 'commit' restriction.
Signed-off-by: Johan Herland <redacted>
---
notes.c | 33 ++++++++++++++++-----------------
notes.h | 11 ++++++++++-
pretty.c | 8 ++++----
3 files changed, 30 insertions(+), 22 deletions(-)
@@ -21,6 +21,10 @@*/voidinit_notes(constchar*notes_ref,intflags);+/* Add the given note object to the internal notes tree structure */+voidadd_note(constunsignedchar*object_sha1,+constunsignedchar*note_sha1);+/* Free (and de-initialize) the internal notes tree structure */voidfree_notes(void);
From: Shawn O. Pearce <hidden> Date: 2016-06-15 22:47:29
Johan Herland [off-list ref] wrote:
This patch teaches 'git fast-import' to use the notes API to organize
the manipulation of note objects through a fast-import stream. Note
objects are added to the notes tree through the 'N' command, and when
we're about to store the tree object for the current commit, we walk
through the notes tree and insert all the notes into the stored tree.
Some high level comments about this patch:
- You don't destroy the struct notes_tree during unload_one_branch()
which means notes trees stay in memory even if the branch table
is overflowing. I think you should discard the notes tree when
a branch unloads, and recreate it when the branch loads.
- Destroying and adding back all notes is OK with ~20k notes, but
doing that with ~150k-~800k notes is going to slow down a lot,
losing the "fast" part.
--
Shawn.
From: Johan Herland <hidden> Date: 2016-06-15 22:47:44
On Friday 09 October 2009, Shawn O. Pearce wrote:
Johan Herland [off-list ref] wrote:
quoted
This patch teaches 'git fast-import' to use the notes API to organize
the manipulation of note objects through a fast-import stream. Note
objects are added to the notes tree through the 'N' command, and when
we're about to store the tree object for the current commit, we walk
through the notes tree and insert all the notes into the stored tree.
Some high level comments about this patch:
- You don't destroy the struct notes_tree during unload_one_branch()
which means notes trees stay in memory even if the branch table
is overflowing. I think you should discard the notes tree when
a branch unloads, and recreate it when the branch loads.
- Destroying and adding back all notes is OK with ~20k notes, but
doing that with ~150k-~800k notes is going to slow down a lot,
losing the "fast" part.
Thanks for the comments. I've tried to address them in the 8th iteration of
this series (Patch 8/10 to be more precise), just submitted to the mailing
list.
...Johan
--
Johan Herland, [off-list ref]
www.herland.net
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:50:28
Dear fast importers,
Another week, another fast-import protocol extension.
Most DVCSes do not allow one to non-disruptively change the log
message for a commit. But sometimes people want to attach information to a
commit after the fact:
- whether it was tested and worked correctly
- who liked or disliked the commit (Acked-by, Reviewed-by)
- corresponding revision number after export to another version
control system
- bug number
- corresponding compiled binary
The N command allows such notes to be attached to commits, like so:
1. first the commit is imported as usual (let's say it's ":1").
2. commit annotations are added separately, like so:
commit refs/notes/commits
committer A. U. Thor [off-list ref] Mon, 31 Jan 2011 12:15:59 -0600
data <<END
Notes after review.
END
N inline :1
data <<END
Acked-by: me
END
Details:
- there can be multiple categories of notes: "refs/notes/commits"
contains ordinary addenda to the commit message, but one might also
see refs/notes/bugzilla, refs/notes/svn-commit, and so on.
- each commit gets at most one blob of notes in each category. Later
notemodify (N) commands overwrite the effect from earlier ones.
- the syntax of a notemodify command is as follows:
'N' sp <dataref> sp <committish> lf
The <dataref> represents a blob with the annotations to be used
("inline" is allowed, too, just like with filemodify). The
<committish> can be any expression allowed in a 'from' command
(branch name, mark reference :<idnum>, other commit name) and
represents the commit that is to be annotated.
- this has been supported in git since v1.6.6. There is no
"feature" for it --- I don't think the feature declaration
facility existed yet.
Do other DVCSes support something like this? Should it get a
feature name?
Jonathan
Heya,
On Mon, Jan 31, 2011 at 19:33, Jonathan Nieder [off-list ref] wrote:
Most DVCSes do not allow one to non-disruptively change the log
message for a commit. But sometimes people want to attach information to a
commit after the fact:
- whether it was tested and worked correctly
- who liked or disliked the commit (Acked-by, Reviewed-by)
- corresponding revision number after export to another version
control system
- bug number
- corresponding compiled binary
I talked with Augie Fackler (from hg) about this on IM and he says:
We don't support anything like that at present (no demand, when we check
nobody really seems to use git notes for anything)
so it doesn't seem relevant in fast-export
So at least HG doesn't (currently) have any functionality that they
could use to implement the importing of such a stream.
--
Cheers,
Sverre Rabbelier
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:50:29
Sverre Rabbelier wrote:
I talked with Augie Fackler (from hg) about this on IM and he says:
quoted
We don't support anything like that at present (no demand, when we check
nobody really seems to use git notes for anything)
so it doesn't seem relevant in fast-export
So at least HG doesn't (currently) have any functionality that they
could use to implement the importing of such a stream.
Thanks, good to know. I suppose this definitely needs a feature name,
then (I'll send a patch to make it "feature notes").
Jonathan
[Aside: I suspect part of the reason "git notes" adoption is not so
great is the lack of git notes fetch/git notes push. Sample size
of 1: I use notes heavily as a consumer, to dig up mailing list
threads[1], but have put off sharing my own annotations until I can
figure out how to make it convenient for others to use.]
[1] http://thread.gmane.org/gmane.comp.version-control.git/109074/focus=109542
Heya,
On Mon, Jan 31, 2011 at 20:01, Jonathan Nieder [off-list ref] wrote:
Thanks, good to know. I suppose this definitely needs a feature name,
then (I'll send a patch to make it "feature notes").
SGTM.
[Aside: I suspect part of the reason "git notes" adoption is not so
great is the lack of git notes fetch/git notes push. Sample size
of 1: I use notes heavily as a consumer, to dig up mailing list
threads[1], but have put off sharing my own annotations until I can
figure out how to make it convenient for others to use.]
That's another thing Augie mentioned that he (and I guess the hg
community at large) dislikes, the fact that they're not propagated.
--
Cheers,
Sverre Rabbelier
From: Sam Vilain <hidden> Date: 2016-06-15 22:50:29
On 01/02/11 10:19, Sverre Rabbelier wrote:
That's another thing Augie mentioned that he (and I guess the hg
community at large) dislikes, the fact that they're not propagated.
This is not a "fact".
If you add configuration in your git config to fetch and push the refs,
then they are propagated.
Just because you disagree with the current interface or defaults doesn't
mean the design is wrong. I hear the same arguments against submodules,
which is a shame because the message isn't getting through to people
that the porcelain can and should be extended to make people's lives
easier. It's just that instead of second-guessing what shape they
should take, the design and plumbing are written so that people can
write scripts to make it work.
It's a slower path, but you end up with a better tool at the end of it.
Sam