From: Johan Herland <hidden> Date: 2016-06-15 22:47:50
Hi,
Here is the 10th iteration of the git-notes series. Changes in this
iteration are as follows:
Changes to existing patches:
- Rebased onto 488bdf2... (Fix crasher on encountering SHA1-like non-note
in notes tree) (There are no conflicts resolved by this, but a test in
this series crashes without the crash fix in 488bdf2...).
- Yet another rewrite of the fast-import patch. After trying to implement
Shawn's suggestions, I found that hacking the mode bits of note entries
could not work, since they are stripped by a mktree()/load_tree()
round-trip. The new versions treats _all_ entries with note-compatible
paths (40 hex chars, not including directory separators) as note entries
(to be subjected to fanout adjustments) in branches where there are note
activity (one or more 'N' commands).
Branches without note activity are not touched, of course.
- Otherwise, Shawn's suggestions to the previous iteration have been
incorporated.
- Extended t9301 tests to verify that non-notes residing in a notes tree
are not clobbered by the fast-import patch.
- Fix t9301 test #12 to not "cheat" (by 'deleteall' followed by a few
additions). Instead, remove notes one-by-one to verify correct
fanout consolidation.
- Minor cleanups here and there
If Shawn is OK with the fast-import patch, I believe that at least
patches #1 - #3 (and possibly #4 - #5) are ready for 'next'.
Patches #6 - #11 drastically extend the notes API. Since there are
currently no users of that API, and it has not been discussed much
on the list (although these patches have already been present in a
few iterations), I would still consider them RFC quality.
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
Johan Herland (11):
fast-import: Proper notes tree manipulation
Rename t9301 to t9350, to make room for more fast-import tests
Add more testcases to test fast-import of notes
Minor style fixes to notes.c
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.c | 134 +++++-
notes.c | 345 +++++++++----
notes.h | 114 ++++-
pretty.c | 9 +-
t/t9300-fast-import.sh | 156 +++++-
t/t9301-fast-import-notes.sh | 623 ++++++++++++++++++++++
t/{t9301-fast-export.sh => t9350-fast-export.sh} | 0
7 files changed, 1259 insertions(+), 122 deletions(-)
create mode 100755 t/t9301-fast-import-notes.sh
rename t/{t9301-fast-export.sh => t9350-fast-export.sh} (100%)
From: Johan Herland <hidden> Date: 2016-06-15 22:47:50
This patch teaches 'git fast-import' to automatically organize note objects
in a fast-import stream into an appropriate fanout structure. The notes API
in notes.h is NOT used to accomplish this, because trying to keep the
fast-import and notes data structures in sync would yield a significantly
larger patch with higher complexity.
Note objects are added with the 'N' command, and accounted for with a
per-branch counter, which is used to trigger fanout restructuring when
needed. Note that when restructuring the branch tree, _any_ entry whose
path consists of 40 hex chars (not including directory separators) will
be recognized as a note object. It is therefore not advisable to
manipulate note entries with M/D/R/C commands.
Since note objects are stored in the same tree structure as other objects,
the unloading and reloading of a fast-import branches handle note objects
transparently.
This patch has been improved by the following contributions:
- Shawn O. Pearce: Several style- and logic-related improvements
Cc: Shawn O. Pearce <redacted>
Signed-off-by: Johan Herland <redacted>
---
As stated in the cover letter, I simply cannot store note information in
the tree_entry mode bits. So, I chose this somewhat more simple and crude
approach, which I still think solves the problems quite nicely.
Have fun! :)
...Johan
fast-import.c | 134 +++++++++++++++++++++++++++++++++++++++--
t/t9300-fast-import.sh | 156 ++++++++++++++++++++++++++++++++++++++++++++----
2 files changed, 272 insertions(+), 18 deletions(-)
@@ -1860,6 +1862,109 @@ static void load_branch(struct branch *b)}}+staticunsignedcharconvert_num_notes_to_fanout(uintmax_tnum_notes)+{+unsignedcharfanout=0;+while((num_notes>>=8))+fanout++;+returnfanout;+}++staticvoidconstruct_path_with_fanout(constchar*hex_sha1,+unsignedcharfanout,char*path)+{+unsignedinti=0,j=0;+if(fanout>=20)+die("Too large fanout (%u)",fanout);+while(fanout){+path[i++]=hex_sha1[j++];+path[i++]=hex_sha1[j++];+path[i++]='/';+fanout--;+}+memcpy(path+i,hex_sha1+j,40-j);+path[i+40-j]='\0';+}++staticuintmax_tdo_change_note_fanout(+structtree_entry*orig_root,structtree_entry*root,+char*hex_sha1,unsignedinthex_sha1_len,+char*fullpath,unsignedintfullpath_len,+unsignedcharfanout)+{+structtree_content*t=root->tree;+structtree_entry*e,leaf;+unsignedinti,tmp_hex_sha1_len,tmp_fullpath_len;+uintmax_tnum_notes=0;+unsignedcharsha1[20];+charrealpath[60];++for(i=0;t&&i<t->entry_count;i++){+e=t->entries[i];+tmp_hex_sha1_len=hex_sha1_len+e->name->str_len;+tmp_fullpath_len=fullpath_len;++/*+*We'reinterestedinEITHERexistingnoteentries(entries+*withexactly40hexcharsinpath,notincludingdirectory+*separators),ORdirectoryentriesthatmaycontainnote+*entries(with<40hexcharsinpath).+*Also,eachpathcomponentinanoteentrymustbeamultiple+*of2chars.+*/+if(!e->versions[1].mode||+tmp_hex_sha1_len>40||+e->name->str_len%2)+continue;++/* This _may_ be a note entry, or a subdir containing notes */+memcpy(hex_sha1+hex_sha1_len,e->name->str_dat,+e->name->str_len);+if(tmp_fullpath_len)+fullpath[tmp_fullpath_len++]='/';+memcpy(fullpath+tmp_fullpath_len,e->name->str_dat,+e->name->str_len);+tmp_fullpath_len+=e->name->str_len;+fullpath[tmp_fullpath_len]='\0';++if(tmp_hex_sha1_len==40&&!get_sha1_hex(hex_sha1,sha1)){+/* This is a note entry */+construct_path_with_fanout(hex_sha1,fanout,realpath);+if(!strcmp(fullpath,realpath)){+/* Note entry is in correct location */+num_notes++;+continue;+}++/* Rename fullpath to realpath */+if(!tree_content_remove(orig_root,fullpath,&leaf))+die("Failed to remove path %s",fullpath);+tree_content_set(orig_root,realpath,+leaf.versions[1].sha1,+leaf.versions[1].mode,+leaf.tree);+}elseif(S_ISDIR(e->versions[1].mode)){+/* This is a subdir that may contain note entries */+if(!e->tree)+load_tree(e);+num_notes+=do_change_note_fanout(orig_root,e,+hex_sha1,tmp_hex_sha1_len,+fullpath,tmp_fullpath_len,fanout);+}++/* The above may have reallocated the current tree_content */+t=root->tree;+}+returnnum_notes;+}++staticuintmax_tchange_note_fanout(structtree_entry*root,+unsignedcharfanout)+{+charhex_sha1[40],path[60];+returndo_change_note_fanout(root,root,hex_sha1,0,path,0,fanout);+}+staticvoidfile_change_m(structbranch*b){constchar*p=command_buf.buf+2;
@@ -2010,14 +2115,16 @@ static void file_change_cr(struct branch *b, int rename)leaf.tree);}-staticvoidnote_change_n(structbranch*b)+staticvoidnote_change_n(structbranch*b,unsignedcharold_fanout){constchar*p=command_buf.buf+2;staticstructstrbufuq=STRBUF_INIT;structobject_entry*oe=oe;structbranch*s;unsignedcharsha1[20],commit_sha1[20];+charpath[60];uint16_tinline_data=0;+unsignedcharnew_fanout;/* <dataref> or 'inline' */if(*p==':'){
@@ -2071,7 +2178,7 @@ static void note_change_n(struct branch *b)if(oe->type!=OBJ_BLOB)die("Not a blob (actually a %s): %s",typename(oe->type),command_buf.buf);-}else{+}elseif(!is_null_sha1(sha1)){enumobject_typetype=sha1_object_info(sha1,NULL);if(type<0)die("Blob not found: %s",command_buf.buf);
@@ -2213,6 +2330,7 @@ static void parse_new_commit(void)char*committer=NULL;structhash_list*merge_list=NULL;unsignedintmerge_count;+unsignedcharprev_fanout,new_fanout;/* Obtain the branch name from the rest of our command */sp=strchr(command_buf.buf,' ')+1;
@@ -2265,6 +2385,10 @@ static void parse_new_commit(void)break;}+new_fanout=convert_num_notes_to_fanout(b->num_notes);+if(new_fanout!=prev_fanout)+b->num_notes=change_note_fanout(&b->branch_tree,new_fanout);+/* build the tree and the commit */store_tree(&b->branch_tree);hashcpy(b->branch_tree.versions[0].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:50
This patch adds testcases verifying correct behaviour in several scenarios
regarding fast-import of notes:
- using a mixture of 'N' and 'M' commands
- updating existing notes
- concatenation of notes
- 'deleteall' also removes notes
- fanout schemes is added/removed when needed
- git-fast-import's branch unload/reload preserves notes
- non-notes are not clobbered in the presence of notes
Signed-off-by: Johan Herland <redacted>
---
t/t9301-fast-import-notes.sh | 623 ++++++++++++++++++++++++++++++++++++++++++
1 files changed, 623 insertions(+), 0 deletions(-)
create mode 100755 t/t9301-fast-import-notes.sh
@@ -0,0 +1,623 @@+#!/bin/sh+#+# Copyright (c) 2009 Johan Herland+#++test_description='test git fast-import of notes objects'+../test-lib.sh+++test_tick+cat>input<<INPUT_END+commitrefs/heads/master+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+data<<COMMIT+firstcommit+COMMIT++M644inlinefoo+data<<EOF+filefooinfirstcommit+EOF++M755inlinebar+data<<EOF+filebarinfirstcommit+EOF++M644inlinebaz/xyzzy+data<<EOF+filebaz/xyzzyinfirstcommit+EOF++commitrefs/heads/master+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+data<<COMMIT+secondcommit+COMMIT++M644inlinefoo+data<<EOF+filefooinsecondcommit+EOF++M755inlinebaz/xyzzy+data<<EOF+filebaz/xyzzyinsecondcommit+EOF++commitrefs/heads/master+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+data<<COMMIT+thirdcommit+COMMIT++M644inlinefoo+data<<EOF+filefoointhirdcommit+EOF++commitrefs/heads/master+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+data<<COMMIT+fourthcommit+COMMIT++M755inlinebar+data<<EOF+filebarinfourthcommit+EOF++INPUT_END++test_expect_success'set up master branch''++gitfast-import<input&&+gitwhatchangedmaster+'++commit4=$(gitrev-parserefs/heads/master)+commit3=$(gitrev-parse"$commit4^")+commit2=$(gitrev-parse"$commit4~2")+commit1=$(gitrev-parse"$commit4~3")++test_tick+cat>input<<INPUT_END+commitrefs/notes/test+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+data<<COMMIT+firstnotescommit+COMMIT++M644inline$commit1+data<<EOF+firstnoteforfirstcommit+EOF++M755inline$commit2+data<<EOF+firstnoteforsecondcommit+EOF++INPUT_END++cat>expect<<EXPECT_END+fourthcommit+thirdcommit+secondcommit+firstnoteforsecondcommit+firstcommit+firstnoteforfirstcommit+EXPECT_END++test_expect_success'add notes with simple M command''++gitfast-import<input&&+GIT_NOTES_REF=refs/notes/testgitlog|grep"^ ">actual&&+test_cmpexpectactual++'++test_tick+cat>input<<INPUT_END+commitrefs/notes/test+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+data<<COMMIT+secondnotescommit+COMMIT++fromrefs/notes/test^0+Ninline$commit3+data<<EOF+firstnoteforthirdcommit+EOF++Ninline$commit4+data<<EOF+firstnoteforfourthcommit+EOF++INPUT_END++cat>expect<<EXPECT_END+fourthcommit+firstnoteforfourthcommit+thirdcommit+firstnoteforthirdcommit+secondcommit+firstnoteforsecondcommit+firstcommit+firstnoteforfirstcommit+EXPECT_END++test_expect_success'add notes with simple N command''++gitfast-import<input&&+GIT_NOTES_REF=refs/notes/testgitlog|grep"^ ">actual&&+test_cmpexpectactual++'++test_tick+cat>input<<INPUT_END+commitrefs/notes/test+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+data<<COMMIT+thirdnotescommit+COMMIT++fromrefs/notes/test^0+Ninline$commit1+data<<EOF+secondnoteforfirstcommit+EOF++Ninline$commit2+data<<EOF+secondnoteforsecondcommit+EOF++Ninline$commit3+data<<EOF+secondnoteforthirdcommit+EOF++Ninline$commit4+data<<EOF+secondnoteforfourthcommit+EOF++INPUT_END++cat>expect<<EXPECT_END+fourthcommit+secondnoteforfourthcommit+thirdcommit+secondnoteforthirdcommit+secondcommit+secondnoteforsecondcommit+firstcommit+secondnoteforfirstcommit+EXPECT_END++test_expect_success'update existing notes with N command''++gitfast-import<input&&+GIT_NOTES_REF=refs/notes/testgitlog|grep"^ ">actual&&+test_cmpexpectactual++'++test_tick+cat>input<<INPUT_END+commitrefs/notes/test+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+data<<COMMIT+fourthnotescommit+COMMIT++fromrefs/notes/test^0+M644inline$(echo"$commit3"|sed"s|^..|&/|")+data<<EOF+prefixofnoteforthirdcommit+EOF++M644inline$(echo"$commit4"|sed"s|^..|&/|")+data<<EOF+prefixofnoteforfourthcommit+EOF++M644inline$(echo"$commit4"|sed"s|^\(..\)\(..\)|\1/\2/|")+data<<EOF+pre-prefixofnoteforfourthcommit+EOF++Ninline$commit1+data<<EOF+thirdnoteforfirstcommit+EOF++Ninline$commit2+data<<EOF+thirdnoteforsecondcommit+EOF++Ninline$commit3+data<<EOF+thirdnoteforthirdcommit+EOF++Ninline$commit4+data<<EOF+thirdnoteforfourthcommit+EOF+++INPUT_END++cat>expect<<EXPECT_END+fourthcommit+pre-prefixofnoteforfourthcommit+prefixofnoteforfourthcommit+thirdnoteforfourthcommit+thirdcommit+prefixofnoteforthirdcommit+thirdnoteforthirdcommit+secondcommit+thirdnoteforsecondcommit+firstcommit+thirdnoteforfirstcommit+EXPECT_END++test_expect_success'add concatentation notes with M command''++gitfast-import<input&&+GIT_NOTES_REF=refs/notes/testgitlog|grep"^ ">actual&&+test_cmpexpectactual++'++test_tick+cat>input<<INPUT_END+commitrefs/notes/test+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+data<<COMMIT+fifthnotescommit+COMMIT++fromrefs/notes/test^0+deleteall++INPUT_END++cat>expect<<EXPECT_END+fourthcommit+thirdcommit+secondcommit+firstcommit+EXPECT_END++test_expect_success'verify that deleteall also removes notes''++gitfast-import<input&&+GIT_NOTES_REF=refs/notes/testgitlog|grep"^ ">actual&&+test_cmpexpectactual++'++test_tick+cat>input<<INPUT_END+commitrefs/notes/test+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+data<<COMMIT+sixthnotescommit+COMMIT++fromrefs/notes/test^0+M644inline$commit1+data<<EOF+thirdnoteforfirstcommit+EOF++M644inline$commit3+data<<EOF+thirdnoteforthirdcommit+EOF++Ninline$commit1+data<<EOF+fourthnoteforfirstcommit+EOF++Ninline$commit3+data<<EOF+fourthnoteforthirdcommit+EOF++INPUT_END++cat>expect<<EXPECT_END+fourthcommit+thirdcommit+fourthnoteforthirdcommit+secondcommit+firstcommit+fourthnoteforfirstcommit+EXPECT_END++test_expect_success'verify that later N commands override earlier M commands''++gitfast-import<input&&+GIT_NOTES_REF=refs/notes/testgitlog|grep"^ ">actual&&+test_cmpexpectactual++'++# Write fast-import commands to create the given number of commits+fast_import_commits(){+my_ref=$1+my_num_commits=$2+my_append_to_file=$3+my_i=0+whiletest$my_i-lt$my_num_commits+do+my_i=$(($my_i+1))+test_tick+cat>>"$my_append_to_file"<<INPUT_END+commit$my_ref+mark:$my_i+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+data<<COMMIT+commit#$my_i+COMMIT++M644inlinefile+data<<EOF+filecontentsincommit#$my_i+EOF++INPUT_END+done+}++# Write fast-import commands to create the given number of notes annotating+# the commits created by fast_import_commits()+fast_import_notes(){+my_notes_ref=$1+my_num_commits=$2+my_append_to_file=$3+my_note_append=$4+test_tick+cat>>"$my_append_to_file"<<INPUT_END+commit$my_notes_ref+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+data<<COMMIT+committing$my_num_commitsnotes+COMMIT++INPUT_END++my_i=0+whiletest$my_i-lt$my_num_commits+do+my_i=$(($my_i+1))+cat>>"$my_append_to_file"<<INPUT_END+Ninline:$my_i+data<<EOF+noteforcommit#$my_i$my_note_append+EOF++INPUT_END+done+}+++rminputexpect+num_commits=400+# Create lots of commits+fast_import_commits"refs/heads/many_commits"$num_commitsinput+# Create one note per above commit+fast_import_notes"refs/notes/many_notes"$num_commitsinput+# Add a couple of non-notes as well+test_tick+cat>>input<<INPUT_END+commitrefs/notes/many_notes+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+data<<COMMIT+committingsomenon-notestothenotestree+COMMIT++M755inlinefoobar/non-note.txt+data<<EOF+Thisisnotanote,butratheraregularfileresidinginanotestree+EOF++M644inlinedeadbeef+data<<EOF+Non-notefile+EOF++M644inlinede/adbeef+data<<EOF+Anothernon-notefile+EOF++INPUT_END+# Finally create the expected output from all these notes and commits+i=$num_commits+whiletest$i-gt0+do+cat>>expect<<EXPECT_END+commit#$i+noteforcommit#$i+EXPECT_END+i=$(($i-1))+done++test_expect_success'add lots of commits and notes''++gitfast-import<input&&+GIT_NOTES_REF=refs/notes/many_notesgitlogrefs/heads/many_commits|+grep"^ ">actual&&+test_cmpexpectactual++'++test_expect_success'verify that lots of notes trigger a fanout scheme''++# None of the entries in the top-level notes tree should be a full SHA1+gitls-tree--name-onlyrefs/notes/many_notes|+whilereadpath+do+iftest$(exprlength"$path")-ge40+then+return1+fi+done++'++cat>>expect_non-note1<<EOF+Thisisnotanote,butratheraregularfileresidinginanotestree+EOF++cat>>expect_non-note2<<EOF+Non-notefile+EOF++cat>>expect_non-note3<<EOF+Anothernon-notefile+EOF++test_expect_success'verify that non-notes are untouched by a fanout change''++gitcat-file-prefs/notes/many_notes:foobar/non-note.txt>actual&&+test_cmpexpect_non-note1actual&&+gitcat-file-prefs/notes/many_notes:deadbeef>actual&&+test_cmpexpect_non-note2actual&&+gitcat-file-prefs/notes/many_notes:de/adbeef>actual&&+test_cmpexpect_non-note3actual++'+remaining_notes=10+test_tick+cat>>input<<INPUT_END+commitrefs/notes/many_notes+committer$GIT_COMMITTER_NAME<$GIT_COMMITTER_EMAIL>$GIT_COMMITTER_DATE+data<<COMMIT+removingallnotesbut$remaining_notes+COMMIT+fromrefs/notes/many_notes^0+INPUT_END++i=$remaining_notes+whiletest$i-lt$num_commits+do+i=$(($i+1))+cat>>input<<INPUT_END+N0000000000000000000000000000000000000000:$i+INPUT_END+done++i=$num_commits+rmexpect+whiletest$i-gt0+do+cat>>expect<<EXPECT_END+commit#$i+EXPECT_END+iftest$i-le$remaining_notes+then+cat>>expect<<EXPECT_END+noteforcommit#$i+EXPECT_END+fi+i=$(($i-1))+done++test_expect_success'remove lots of notes''++gitfast-import<input&&+GIT_NOTES_REF=refs/notes/many_notesgitlogrefs/heads/many_commits|+grep"^ ">actual&&+test_cmpexpectactual++'++test_expect_success'verify that removing notes trigger fanout consolidation''++# All entries in the top-level notes tree should be a full SHA1+gitls-tree--name-only-rrefs/notes/many_notes|+whilereadpath+do+# Explicitly ignore the non-note paths+test"$path"="foobar/non-note.txt"&&continue+test"$path"="deadbeef"&&continue+test"$path"="de/adbeef"&&continue++iftest$(exprlength"$path")-ne40+then+return1+fi+done++'++test_expect_success'verify that non-notes are untouched by a fanout change''++gitcat-file-prefs/notes/many_notes:foobar/non-note.txt>actual&&+test_cmpexpect_non-note1actual&&+gitcat-file-prefs/notes/many_notes:deadbeef>actual&&+test_cmpexpect_non-note2actual&&+gitcat-file-prefs/notes/many_notes:de/adbeef>actual&&+test_cmpexpect_non-note3actual++'+++rminputexpect+num_notes_refs=10+num_commits=16+some_commits=8+# Create commits+fast_import_commits"refs/heads/more_commits"$num_commitsinput+# Create one note per above commit per notes ref+i=0+whiletest$i-lt$num_notes_refs+do+i=$(($i+1))+fast_import_notes"refs/notes/more_notes_$i"$num_commitsinput+done+# Trigger branch reloading in git-fast-import by repeating the note creation+i=0+whiletest$i-lt$num_notes_refs+do+i=$(($i+1))+fast_import_notes"refs/notes/more_notes_$i"$some_commitsinput" (2)"+done+# Finally create the expected output from the notes in refs/notes/more_notes_1+i=$num_commits+whiletest$i-gt0+do+note_data="note for commit #$i"+iftest$i-le$some_commits+then+note_data="$note_data (2)"+fi+cat>>expect<<EXPECT_END+commit#$i+$note_data+EXPECT_END+i=$(($i-1))+done++test_expect_success"add notes to $num_commits commits in each of $num_notes_refs refs"'++gitfast-import--active-branches=5<input&&+GIT_NOTES_REF=refs/notes/more_notes_1gitlogrefs/heads/more_commits|+grep"^ ">actual&&+test_cmpexpectactual++'++test_done
diff --git a/t/t9301-fast-export.sh b/t/t9350-fast-export.shsimilarity index 100%rename from t/t9301-fast-export.shrename to t/t9350-fast-export.sh
--
1.6.5.3.433.g11067
From: Johan Herland <hidden> Date: 2016-06-15 22:47:50
This includes a first attempt at creating an optimal fanout scheme (which
is calculated on-the-fly, while traversing).
Signed-off-by: Johan Herland <redacted>
---
notes.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
notes.h | 17 ++++++++++
2 files changed, 118 insertions(+), 0 deletions(-)
@@ -28,6 +28,23 @@ 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);+/*+*Invokethespecifiedcallbackfunctionforeachnote+*+*Ifthecallbackreturnsnonzero,thenotewalkisaborted,andthereturn+*valuefromthecallbackisreturnedfromfor_each_note().+*+*IMPORTANT:ThecallbackfunctionisNOTallowedtochangethenotestree.+*Inotherwords,thefollowingfunctionscanNOTbeinvoked(onthecurrent+*notestree)fromwithinthecallback:+*-add_note()+*-free_notes()+*/+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:50
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(-)
From: Johan Herland <hidden> Date: 2016-06-15 22:47:50
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:50
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:50
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 fallback "default" notes
tree (declared in notes.c) is used.
Signed-off-by: Johan Herland <redacted>
---
notes.c | 67 ++++++++++++++++++++++++++++++++++++++-----------------------
notes.h | 53 +++++++++++++++++++++++++++++++++++-------------
pretty.c | 7 +++--
3 files changed, 84 insertions(+), 43 deletions(-)
@@ -10,26 +25,32 @@#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);/*-*Invokethespecifiedcallbackfunctionforeachnote+*Invokethespecifiedcallbackfunctionforeachnoteinthegivennotes_tree**Ifthecallbackreturnsnonzero,thenotewalkisaborted,andthereturn*valuefromthecallbackisreturnedfromfor_each_note().
@@ -43,10 +64,10 @@ const unsigned char *get_note(const unsigned char *object_sha1);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
@@ -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: Johan Herland <hidden> Date: 2016-06-15 22:47:50
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 | 135 ++++++++++++++++++++++++++++++++++++---------------------------
notes.h | 34 +++++++++++++++-
2 files changed, 109 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 */
@@ -434,7 +392,62 @@ 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: Shawn O. Pearce <hidden> Date: 2016-06-15 22:47:51
Johan Herland [off-list ref] wrote:
As stated in the cover letter, I simply cannot store note information in
the tree_entry mode bits. So, I chose this somewhat more simple and crude
approach, which I still think solves the problems quite nicely.
Oh, duh. Of course you can't. You lose the note mode when the
tree is flushed to disk, purged from memory, and reloaded later.
Whoops, sorry I missed that during the last round of review.
I think this function winds up processing all notes twice. Yuck.
tree_content_set() adds a new tree entry to the end of the current
tree. So when converting "1a9029b006484e8b9aca06ff261beb2324bb9916"
into "1a" (to go from fanout 0 to fanout 1) we'll place 1a at the
end of orig_root, and this function will walk 1a/ recursively,
examining 1a9029b006484e8b9aca06ff261beb2324bb9916 all over again.
If we're here, isn't it likely that *all* notes are in the wrong
path in the tree, and we need to move them all to a new location?
If that's true then should we instead just build an entirely new
tree and swap the root when we are done?
As we empty out a tree the object will be recycled into a pool of
trees which can be reused at a later point. It might actually make
sense to build the new tree under a different root. We won't scan
entries we've moved, and the memory difference should be fairly
small as tree_content_remove() will make a subtree available for
reuse as soon as its empty. So we're only dealing with a handful
of additional tree objects as we do the conversion.
--
Shawn.
I think this function winds up processing all notes twice. Yuck.
tree_content_set() adds a new tree entry to the end of the current
tree. So when converting "1a9029b006484e8b9aca06ff261beb2324bb9916"
into "1a" (to go from fanout 0 to fanout 1) we'll place 1a at the
end of orig_root, and this function will walk 1a/ recursively,
examining 1a9029b006484e8b9aca06ff261beb2324bb9916 all over again.
Yep, you're right. Still, we only do the tree_content_remove()/set() once
per note, so although performance is probably not abysmal, we are still
clearly suboptimal.
Also, keep in mind that change_note_fanout() is only called when the number
of notes crosses a power of 256. Thus for typical notes trees (which are
assumed to mostly accumulate notes over their lifetime),
change_note_fanout() will be called zero, one or two times (depending on the
final number of notes).
If we're here, isn't it likely that *all* notes are in the wrong
path in the tree, and we need to move them all to a new location?
If that's true then should we instead just build an entirely new
tree and swap the root when we are done?
Hmm. Not always. In your earlier scenario where we add 2,000,000 notes in a
single commit, the current code would need to rewrite 255 of them from
fanout 0 to fanout 2, and 65,535 of them from fanout 1 to fanout 2. But the
vast majority (1,934,465) would not require rewriting (having been added at
the correct fanout initially). However, if we build a new tree (by which I
assume you mean tree_content_remove() from the old tree and
tree_content_set() to the new tree for every single note (and non-note)), we
end up processing all 2,000,000 entries.
As we empty out a tree the object will be recycled into a pool of
trees which can be reused at a later point. It might actually make
sense to build the new tree under a different root. We won't scan
entries we've moved, and the memory difference should be fairly
small as tree_content_remove() will make a subtree available for
reuse as soon as its empty. So we're only dealing with a handful
of additional tree objects as we do the conversion.
I'm not sure I get the details here. How can we avoid doing the
_remove()/_set() from/to the old/new tree for every tree_entry? In other
words, how do we avoid removing and re-setting the 2,000,000 notes in the
above example?
Thanks for the review!
...Johan
--
Johan Herland, [off-list ref]
www.herland.net
Shouldn't convert_num_notes_to_fanout have a guard to prevent this
case from happening?
Well, it sort of already does (unless uintmax_t is more than 19 * 8 = 152
bits wide... ;)
Not sure what you're getting at:
- Should I add a "&& fanout < 19" condition to the while loop in
convert_num_notes_to_fanout()?
- Should I remove the "if (fanout >= 20) die(...)"? Of course,
construct_path_with_fanout() is only supposed to be called with values
returned from convert_num_notes_to_fanout(), so the condition only tests a
precondition that we believe to be true (FTR, it was converted from an
equivalent assert() in an earlier iteration), but I normally test for these
things anyway (when they are not blindingly obvious), just to make sure...
(and I believe a die(...) is kinder to the user than a segfault...)
...Johan
--
Johan Herland, [off-list ref]
www.herland.net
From: Shawn O. Pearce <hidden> Date: 2016-06-15 22:47:51
Johan Herland [off-list ref] wrote:
quoted
If we're here, isn't it likely that *all* notes are in the wrong
path in the tree, and we need to move them all to a new location?
If that's true then should we instead just build an entirely new
tree and swap the root when we are done?
Hmm. Not always. In your earlier scenario where we add 2,000,000 notes in a
single commit, the current code would need to rewrite 255 of them from
fanout 0 to fanout 2, and 65,535 of them from fanout 1 to fanout 2. But the
vast majority (1,934,465) would not require rewriting (having been added at
the correct fanout initially). However, if we build a new tree (by which I
assume you mean tree_content_remove() from the old tree and
tree_content_set() to the new tree for every single note (and non-note)), we
end up processing all 2,000,000 entries.
Well, by processing here you mean we wind up looking at them, only
to determine they are in the correct place already and skipping past.
I guess I see your point though. We're fairly bounded on how many
we might need to move, probably only 65,535, and the rest will be
at the right position so we're mostly just iterating through to
confirm they don't have to be moved.
I'm not sure I get the details here. How can we avoid doing the
_remove()/_set() from/to the old/new tree for every tree_entry? In other
words, how do we avoid removing and re-setting the 2,000,000 notes in the
above example?
You can't. But I realize now what you are saying... for the vast
majority of the notes we only need to validate they are in the
correct path.
--
Shawn.
From: Johan Herland <hidden> Date: 2016-06-15 22:47:51
On Tuesday 08 December 2009, Shawn O. Pearce wrote:
Johan Herland [off-list ref] wrote:
quoted
quoted
If we're here, isn't it likely that *all* notes are in the wrong
path in the tree, and we need to move them all to a new location?
If that's true then should we instead just build an entirely new
tree and swap the root when we are done?
Hmm. Not always. In your earlier scenario where we add 2,000,000 notes
in a single commit, the current code would need to rewrite 255 of them
from fanout 0 to fanout 2, and 65,535 of them from fanout 1 to fanout
2. But the vast majority (1,934,465) would not require rewriting
(having been added at the correct fanout initially). However, if we
build a new tree (by which I assume you mean tree_content_remove() from
the old tree and
tree_content_set() to the new tree for every single note (and
non-note)), we end up processing all 2,000,000 entries.
Well, by processing here you mean we wind up looking at them, only
to determine they are in the correct place already and skipping past.
No, (as far as I (mis?)understood your idea) by processing here I'm talking
about moving all 2,000,000 entries from the old tree to the new tree.
Here's my understanding of your idea:
- Create a new, empty tree
- For each entry in the old/existing tree:
- If not a note, move[*] verbatim to new tree
- If a correctly placed note, move[*] verbatim to new tree
- Else, move[*] note to the _correct_ place in the new tree
[*]: By "move" I assume you mean tree_content_remove() from the old tree,
followed by tree_content_set() into the new tree.
From this understanding, I cannot see how your idea improves on the
adding-2M-notes scenario.
I guess I see your point though. We're fairly bounded on how many
we might need to move, probably only 65,535, and the rest will be
at the right position so we're mostly just iterating through to
confirm they don't have to be moved.
Yep.
Have fun! :)
...Johan
--
Johan Herland, [off-list ref]
www.herland.net
From: Johan Herland <hidden> Date: 2016-06-15 22:47:53
On Tuesday 08 December 2009, Johan Herland wrote:
On Tuesday 08 December 2009, Shawn O. Pearce wrote:
quoted
Johan Herland [off-list ref] wrote:
quoted
quoted
If we're here, isn't it likely that *all* notes are in the wrong
path in the tree, and we need to move them all to a new location?
If that's true then should we instead just build an entirely new
tree and swap the root when we are done?
Hmm. Not always. In your earlier scenario where we add 2,000,000
notes in a single commit, the current code would need to rewrite 255
of them from fanout 0 to fanout 2, and 65,535 of them from fanout 1
to fanout 2. But the vast majority (1,934,465) would not require
rewriting (having been added at the correct fanout initially).
However, if we build a new tree (by which I assume you mean
tree_content_remove() from the old tree and
tree_content_set() to the new tree for every single note (and
non-note)), we end up processing all 2,000,000 entries.
Well, by processing here you mean we wind up looking at them, only
to determine they are in the correct place already and skipping past.
No, (as far as I (mis?)understood your idea) by processing here I'm
talking about moving all 2,000,000 entries from the old tree to the new
tree.
Here's my understanding of your idea:
- Create a new, empty tree
- For each entry in the old/existing tree:
- If not a note, move[*] verbatim to new tree
- If a correctly placed note, move[*] verbatim to new tree
- Else, move[*] note to the _correct_ place in the new tree
[*]: By "move" I assume you mean tree_content_remove() from the old tree,
followed by tree_content_set() into the new tree.
From this understanding, I cannot see how your idea improves on the
adding-2M-notes scenario.
quoted
I guess I see your point though. We're fairly bounded on how many
we might need to move, probably only 65,535, and the rest will be
at the right position so we're mostly just iterating through to
confirm they don't have to be moved.
Yep.
Do you have more comments/suggestions on this patch? Or is it ok to include
in fast-import as-is?
Have fun! :)
...Johan
--
Johan Herland, [off-list ref]
www.herland.net
From: Johan Herland <hidden> Date: 2016-06-15 22:47:53
On Thursday 10 December 2009, Shawn O. Pearce wrote:
Johan Herland [off-list ref] wrote:
quoted
Do you have more comments/suggestions on this patch? Or is it ok to
include in fast-import as-is?
Oops, sorry.
No, no additional comments. I am happy with this patch.
Acked-by: Shawn O. Pearce <redacted>
Thanks.
Junio: With the above Ack, I believe patches #1 - #4 (and possibly #5)
from this series are ready for 'next'.
You may want to hold off on the remainder of the series until I get
around to writing some functionality that actually _uses_ the new API.
Have fun! :)
...Johan
--
Johan Herland, [off-list ref]
www.herland.net