This series depends on a merge of en/merge-ort-3 and
en/merge-ort-recursive. It does not depend on the (not-yet-picked-up)
ort-conflict-handling series[1].
This series mostly implements directory rename detection for
merge-ort; I'll cover the "mostly" bit below. If one merges this
series with en/merge-tests and the ort-conflict-handling series[1],
then this series drops the number of failing tests in the testsuite
under GIT_TEST_MERGE_ALGORITHM=ort from 60 down to 8.
There's a lot of code here, but almost all of the logic is just copied
over from similarly named functions in merge-recursive.c, as
repeatedly noted in the commit messages. There are several minor
differences spread throughout that make it not be a direct copy:
* using strmap API instead of direct hashmap calls
* ort keeps track of all files and directories and their shas in
opt->priv->paths; no need to re-walk tree objects
* keeping the necessary invariants for opt->priv->paths
* we can pre-compute which directories are removed (stored in
dirs_removed), avoiding the need for some post-processing
* since ort already has struct rename_info, add the extra data
there and allocate/free it with the rest of the rename_info
* no non_unique_new_dir field, leading to the failure of test 2b;
this will be addressed in a different way with upcoming
performance work.
These differences make a direct comparison difficult, but there's not
really any new or novel logic; the logic for how directory rename
detection is performed is identical to what is found in
merge-recursive; it's just packed slightly differently.
...with one exception -- the final patch in the series modifies the
logic and makes it different than merge-recursive in order to fix a
known bug (testcase 12f of t6423).
There are still four failing tests in t6423 (directory rename tests)
after this series:
* one test (2b) where merge-ort erroneously prints a "directory
rename split" conflict message, despite the fact that there is no
new file and thus no need for a directory rename to be detected.
This comes from the lack of a non_unique_new_dir field that I
didn't bother copying, since performance work will address it in a
completely different way.
* two tests (12b1 and 12c1) where merge-ort produces the same result
at merge-recursive (these tests are marked as test_expect_failure
for merge-recursive). Some performance work will fix these two
tests.
* one test (12f) where merge-ort produces a better result than
merge-recursive.c (this test is marked as test_expect_failure for
merge-recursive), but where merge-ort does not yet manage to pass
the final four lines of the test related to performance checking.
[1] https://lore.kernel.org/git/pull.815.v2.git.1609468488.gitgitgadget@gmail.com/
Elijah Newren (17):
merge-ort: add new data structures for directory rename detection
merge-ort: initialize and free new directory rename data structures
merge-ort: collect which directories are removed in dirs_removed
merge-ort: add outline for computing directory renames
merge-ort: add outline of get_provisional_directory_renames()
merge-ort: copy get_renamed_dir_portion() from merge-recursive.c
merge-ort: implement compute_rename_counts()
merge-ort: implement handle_directory_level_conflicts()
merge-ort: modify collect_renames() for directory rename handling
merge-ort: implement compute_collisions()
merge-ort: implement apply_dir_rename() and check_dir_renamed()
merge-ort: implement check_for_directory_rename()
merge-ort: implement handle_path_level_conflicts()
merge-ort: add a new toplevel_dir field
merge-ort: implement apply_directory_rename_modifications()
merge-ort: process_renames() now needs more defensiveness
merge-ort: fix a directory rename detection bug
merge-ort.c | 831 ++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 812 insertions(+), 19 deletions(-)
--
2.29.1.106.g3ff750dc32.dirty
This is modelled on the version of handle_directory_level_conflicts()
from merge-recursive.c, but is massively simplified due to the following
factors:
* strmap API provides simplifications over using direct hashamp
* we have a dirs_removed field in struct rename_info that we have an
easy way to populate from collect_merge_info(); this was already
used in compute_rename_counts() and thus we do not need to check
for condition #2.
* The removal of condition #2 by handling it earlier in the code also
obviates the need to check for condition #3 -- if both sides renamed
a directory, meaning that the directory no longer exists on either
side, then neither side could have added any new files to that
directory, and thus there are no files whose locations we need to
move due to such a directory rename.
In fact, the same logic that makes condition #3 irrelevant means
condition #1 is also irrelevant so we could drop this function.
However, it is cheap to check if both sides rename the same directory,
and doing so can save future computation. So, simply remove any
directories that both sides renamed from the list of directory renames.
Signed-off-by: Elijah Newren <redacted>
---
merge-ort.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
This is nearly a wholesale copy of compute_collisions() from
merge-recursive.c, and the logic remains the same, but it has been
tweaked slightly due to:
* using strmap.h API (instead of direct hashmaps)
* allocation/freeing of data structures were done separately in
merge_start() and clear_or_reinit_internal_opts() in an earlier
patch in this series
* there is no non_unique_new_dir data field in merge-ort; that will
be handled a different way
It does depend on two new functions, apply_dir_rename() and
check_dir_renamed() which were introduced with simple
die-not-yet-implemented shells and will be implemented in subsequent
patches.
Signed-off-by: Elijah Newren <redacted>
---
merge-ort.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 67 insertions(+), 1 deletion(-)
This is copied from merge-recursive.c, with minor tweaks due to using strmap
API and the fact that it can use opt->priv->paths to get all pathnames that
exist instead of taking a tree object.
This depends on a new function, handle_path_level_conflicts(), which
just has a placeholder die-not-yet-implemented implementation for now; a
subsequent patch will implement it.
Signed-off-by: Elijah Newren <redacted>
---
merge-ort.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 66 insertions(+), 1 deletion(-)
Port some directory rename handling changes from merge-recursive.c's
detect_and_process_renames() to the same-named function of merge-ort.c.
This does not yet add any use or handling of directory renames, just the
outline for where we start to compute them. Thus, a future patch will
add port additional changes to merge-ort's detect_and_process_renames().
Signed-off-by: Elijah Newren <redacted>
---
merge-ort.c | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
@@ -1139,6 +1139,18 @@ static int handle_content_merge(struct merge_options *opt,/*** Function Grouping: functions related to directory rename detection ***/+staticvoidget_provisional_directory_renames(structmerge_options*opt,+unsignedside,+int*clean)+{+die("Not yet implemented!");+}++staticvoidhandle_directory_level_conflicts(structmerge_options*opt)+{+die("Not yet implemented!");+}+/*** Function Grouping: functions related to regular rename detection ***/staticintprocess_renames(structmerge_options*opt,
@@ -1504,13 +1516,24 @@ static int detect_and_process_renames(struct merge_options *opt,{structdiff_queue_structcombined;structrename_info*renames=&opt->priv->renames;-ints,clean=1;+intneed_dir_renames,s,clean=1;memset(&combined,0,sizeof(combined));detect_regular_renames(opt,merge_base,side1,MERGE_SIDE1);detect_regular_renames(opt,merge_base,side2,MERGE_SIDE2);+need_dir_renames=+!opt->priv->call_depth&&+(opt->detect_directory_renames==MERGE_DIRECTORY_RENAMES_TRUE||+opt->detect_directory_renames==MERGE_DIRECTORY_RENAMES_CONFLICT);++if(need_dir_renames){+for(s=MERGE_SIDE1;s<=MERGE_SIDE2;s++)+get_provisional_directory_renames(opt,s,&clean);+handle_directory_level_conflicts(opt);+}+ALLOC_GROW(combined.queue,renames->pairs[1].nr+renames->pairs[2].nr,combined.alloc);
@@ -1139,6 +1139,110 @@ static int handle_content_merge(struct merge_options *opt,/*** Function Grouping: functions related to directory rename detection ***/+MAYBE_UNUSED+staticvoidget_renamed_dir_portion(constchar*old_path,constchar*new_path,+char**old_dir,char**new_dir)+{+char*end_of_old,*end_of_new;++/* Default return values: NULL, meaning no rename */+*old_dir=NULL;+*new_dir=NULL;++/*+*For+*"a/b/c/d/e/foo.c"->"a/b/some/thing/else/e/foo.c"+*the"e/foo.c"partisthesame,wejustwanttoknowthat+*"a/b/c/d"wasrenamedto"a/b/some/thing/else"+*so,forthisexample,thisfunctionreturns"a/b/c/d"in+**old_dirand"a/b/some/thing/else"in*new_dir.+*/++/*+*Ifthebasenameofthefilechanged,wedon'tcare.Wewant+*toknowwhichportionofthedirectory,ifany,changed.+*/+end_of_old=strrchr(old_path,'/');+end_of_new=strrchr(new_path,'/');++/*+*Ifend_of_oldisNULL,old_pathwasn'tinadirectory,sothere+*couldnotbeadirectoryrename(ourruleelsewherethata+*directorywhichstillexistsisnotconsideredtohavebeen+*renamedmeanstherootdirectorycanneverberenamed--because+*therootdirectoryalwaysexists).+*/+if(end_of_old==NULL)+return;/* Note: *old_dir and *new_dir are still NULL */++/*+*Ifnew_pathcontainsnodirectory(end_of_newisNULL),thenwe+*havearenameofold_path'sdirectorytotherootdirectory.+*/+if(end_of_new==NULL){+*old_dir=xstrndup(old_path,end_of_old-old_path);+*new_dir=xstrdup("");+return;+}++/* Find the first non-matching character traversing backwards */+while(*--end_of_new==*--end_of_old&&+end_of_old!=old_path&&+end_of_new!=new_path)+;/* Do nothing; all in the while loop */++/*+*Ifbothgotbacktothebeginningoftheirstrings,thenthe+*directorydidn'tchangeatall,onlythebasenamedid.+*/+if(end_of_old==old_path&&end_of_new==new_path&&+*end_of_old==*end_of_new)+return;/* Note: *old_dir and *new_dir are still NULL */++/*+*Ifend_of_newgotbacktothebeginningofitsstring,and+*end_of_oldgotbacktothebeginningofsomesubdirectory,then+*wehavearename/mergeofasubdirectoryintotheroot,which+*needsslightlyspecialhandling.+*+*Note:Thereisnoneedtoconsidertheoppositecase,witha+*rename/mergeoftherootdirectoryintosomesubdirectory+*becauseasnotedabovetherootdirectoryalwaysexistssoit+*cannotbeconsideredtoberenamed.+*/+if(end_of_new==new_path&&+end_of_old!=old_path&&end_of_old[-1]=='/'){+*old_dir=xstrndup(old_path,--end_of_old-old_path);+*new_dir=xstrdup("");+return;+}++/*+*We'vefoundthefirstnon-matchingcharacterinthedirectory+*paths.Thatmeansthecurrentcharacterswewerelookingat+*werepartofthefirstnon-matchingsubdirnamegoingbackfrom+*theendofthestrings.Getthewholenamebyadvancingboth+*end_of_oldandend_of_newtotheNEXT'/'character.Thatwill+*representtheentiredirectoryrename.+*+*Thereasonfortheincrementiscaseslike+*a/b/star/foo/whatever.c->a/b/tar/foo/random.c+*Afterdroppingthebasenameandgoingbacktothefirst+*non-matchingcharacter,we'renowcomparing:+*a/b/sanda/b/+*andwewanttobecomparing:+*a/b/star/anda/b/tar/+*butwithoutthepre-increment,theoneontherightwouldstay+*a/b/.+*/+end_of_old=strchr(++end_of_old,'/');+end_of_new=strchr(++end_of_new,'/');++/* Copy the old and new directories into *old_dir and *new_dir. */+*old_dir=xstrndup(old_path,end_of_old-old_path);+*new_dir=xstrndup(new_path,end_of_new-new_path);+}+staticvoidcompute_rename_counts(structdiff_queue_struct*pairs,structstrmap*dir_rename_count,structstrset*dirs_removed)
Due to the string-equality-iff-pointer-equality requirements placed on
merged_info.directory_name, apply_directory_rename_modifications() will
need to have access to the exact toplevel directory name string pointer
and can't just use a new empty string. Store it in a field that we can
use.
Signed-off-by: Elijah Newren <redacted>
---
merge-ort.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
This function is based on merge-recursive.c's get_directory_renames(),
except that the first half has been split out into a not-yet-implemented
compute_rename_counts(). The primary difference here is our lack of the
non_unique_new_dir boolean in our strmap. The lack of that field will
at first cause us to fail testcase 2b of t6423; however, future
optimizations will obviate the need for that ugly field so we have just
left it out.
Signed-off-by: Elijah Newren <redacted>
---
merge-ort.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 59 insertions(+), 1 deletion(-)
@@ -1139,11 +1139,69 @@ static int handle_content_merge(struct merge_options *opt,/*** Function Grouping: functions related to directory rename detection ***/+staticvoidcompute_rename_counts(structdiff_queue_struct*pairs,+structstrmap*dir_rename_count,+structstrset*dirs_removed)+{+die("Not yet implemented!");+}+staticvoidget_provisional_directory_renames(structmerge_options*opt,unsignedside,int*clean){-die("Not yet implemented!");+structhashmap_iteriter;+structstrmap_entry*entry;+structrename_info*renames=&opt->priv->renames;++compute_rename_counts(&renames->pairs[side],+&renames->dir_rename_count[side],+&renames->dirs_removed[side]);+/*+*Collapse+*dir_rename_count:old_directory->{new_directory->count}+*downto+*dir_renames:old_directory->best_new_directory+*wherebest_new_directoryistheonewiththeuniquehighestcount.+*/+strmap_for_each_entry(&renames->dir_rename_count[side],&iter,entry){+constchar*source_dir=entry->key;+structstrintmap*counts=entry->value;+structhashmap_itercount_iter;+structstrmap_entry*count_entry;+intmax=0;+intbad_max=0;+constchar*best=NULL;++strintmap_for_each_entry(counts,&count_iter,count_entry){+constchar*target_dir=count_entry->key;+intptr_tcount=(intptr_t)count_entry->value;++if(count==max)+bad_max=max;+elseif(count>max){+max=count;+best=target_dir;+}+}++if(max==0)+continue;++if(bad_max==max){+path_msg(opt,source_dir,0,+_("CONFLICT (directory rename split): "+"Unclear where to rename %s to; it was "+"renamed to multiple other directories, with "+"no destination getting a majority of the "+"files."),+source_dir);+*clean&=0;+}else{+strmap_put(&renames->dir_renames[side],+source_dir,(void*)best);+}+}}staticvoidhandle_directory_level_conflicts(structmerge_options*opt)
collect_renames() is similar to merge-recursive.c's get_renames(), but
lacks the directory rename handling found in the latter. Port that code
structure over to merge-ort. This introduces three new
die-not-yet-implemented functions that will be defined in future
commits.
Signed-off-by: Elijah Newren <redacted>
---
merge-ort.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 74 insertions(+), 4 deletions(-)
@@ -1139,6 +1139,11 @@ static int handle_content_merge(struct merge_options *opt,/*** Function Grouping: functions related to directory rename detection ***/+structcollision_info{+structstring_listsource_files;+unsignedreported_already:1;+};+staticvoidget_renamed_dir_portion(constchar*old_path,constchar*new_path,char**old_dir,char**new_dir){
@@ -1378,6 +1383,31 @@ static void handle_directory_level_conflicts(struct merge_options *opt)string_list_clear(&duplicated,0);}+staticvoidcompute_collisions(structstrmap*collisions,+structstrmap*dir_renames,+structdiff_queue_struct*pairs)+{+die("Not yet implemented.");+}++staticchar*check_for_directory_rename(structmerge_options*opt,+constchar*path,+unsignedside_index,+structstrmap*dir_renames,+structstrmap*dir_rename_exclusions,+structstrmap*collisions,+int*clean_merge)+{+die("Not yet implemented.");+}++staticvoidapply_directory_rename_modifications(structmerge_options*opt,+structdiff_filepair*pair,+char*new_path)+{+die("Not yet implemented.");+}+/*** Function Grouping: functions related to regular rename detection ***/staticintprocess_renames(structmerge_options*opt,
@@ -1703,22 +1733,44 @@ static void detect_regular_renames(struct merge_options *opt,*/staticintcollect_renames(structmerge_options*opt,structdiff_queue_struct*result,-unsignedside_index)+unsignedside_index,+structstrmap*dir_renames_for_side,+structstrmap*rename_exclusions){inti,clean=1;+structstrmapcollisions;structdiff_queue_struct*side_pairs;+structhashmap_iteriter;+structstrmap_entry*entry;structrename_info*renames=&opt->priv->renames;side_pairs=&renames->pairs[side_index];+compute_collisions(&collisions,dir_renames_for_side,side_pairs);for(i=0;i<side_pairs->nr;++i){structdiff_filepair*p=side_pairs->queue[i];+char*new_path;/* non-NULL only with directory renames */-if(p->status!='R'){+if(p->status!='A'&&p->status!='R'){diff_free_filepair(p);continue;}+new_path=check_for_directory_rename(opt,p->two->path,+side_index,+dir_renames_for_side,+rename_exclusions,+&collisions,+&clean);++if(p->status!='R'&&!new_path){+diff_free_filepair(p);+continue;+}++if(new_path)+apply_directory_rename_modifications(opt,p,new_path);+/**p->scorecomesbackfromdiffcore_rename_extended()with*thesimilarityoftherenamedfile.Thesimilarityis
@@ -1733,6 +1785,20 @@ static int collect_renames(struct merge_options *opt,result->queue[result->nr++]=p;}+/* Free each value in the collisions map */+strmap_for_each_entry(&collisions,&iter,entry){+structcollision_info*info=entry->value;+string_list_clear(&info->source_files,0);+}+/*+*Incompute_collisions(),wesetcollisions.strdup_stringsto0+*sothatwewouldn'thavetomakeanothercopyofthenew_path+*allocatedbyapply_dir_rename().Butnowthatwe'veusedthem+*andhavenootherreferencestothesestrings,itistimeto+*deallocatethem.+*/+free_strmap_strings(&collisions);+strmap_clear(&collisions,1);returnclean;}
@@ -1764,8 +1830,12 @@ static int detect_and_process_renames(struct merge_options *opt,ALLOC_GROW(combined.queue,renames->pairs[1].nr+renames->pairs[2].nr,combined.alloc);-clean&=collect_renames(opt,&combined,MERGE_SIDE1);-clean&=collect_renames(opt,&combined,MERGE_SIDE2);+clean&=collect_renames(opt,&combined,MERGE_SIDE1,+&renames->dir_renames[2],+&renames->dir_renames[1]);+clean&=collect_renames(opt,&combined,MERGE_SIDE2,+&renames->dir_renames[1],+&renames->dir_renames[2]);QSORT(combined.queue,combined.nr,compare_pairs);clean&=process_renames(opt,&combined);
This function is based on the first half of get_directory_renames() from
merge-recursive.c
Signed-off-by: Elijah Newren <redacted>
---
merge-ort.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 51 insertions(+), 2 deletions(-)
@@ -1139,7 +1139,6 @@ static int handle_content_merge(struct merge_options *opt,/*** Function Grouping: functions related to directory rename detection ***/-MAYBE_UNUSEDstaticvoidget_renamed_dir_portion(constchar*old_path,constchar*new_path,char**old_dir,char**new_dir){
@@ -1243,11 +1242,61 @@ static void get_renamed_dir_portion(const char *old_path, const char *new_path,*new_dir=xstrndup(new_path,end_of_new-new_path);}+staticvoidincrement_count(structstrmap*dir_rename_count,+char*old_dir,+char*new_dir)+{+structstrintmap*counts;+structstrmap_entry*e;++/* Get the {new_dirs -> counts} mapping using old_dir */+e=strmap_get_entry(dir_rename_count,old_dir);+if(e){+counts=e->value;+}else{+counts=xmalloc(sizeof(*counts));+strintmap_init_with_options(counts,0,NULL,1);+strmap_put(dir_rename_count,old_dir,counts);+}++/* Increment the count for new_dir */+strintmap_incr(counts,new_dir,1);+}+staticvoidcompute_rename_counts(structdiff_queue_struct*pairs,structstrmap*dir_rename_count,structstrset*dirs_removed){-die("Not yet implemented!");+inti;++for(i=0;i<pairs->nr;++i){+char*old_dir,*new_dir;+structdiff_filepair*pair=pairs->queue[i];++if(pair->status!='R')+continue;++/* Get the old and new directory names */+get_renamed_dir_portion(pair->one->path,pair->two->path,+&old_dir,&new_dir);+if(!old_dir)+/* Directory didn't change at all; ignore this one. */+continue;++/*+*Makedir_rename_countcontainamapofamap:+*old_directory->{new_directory->count}+*Inotherwords,foreverypairlookatthedirectoriesfor+*theoldfilenameandthenewfilenameandcounthowmany+*timesthatpairingoccurs.+*/+if(strset_contains(dirs_removed,old_dir))+increment_count(dir_rename_count,old_dir,new_dir);++/* Free resources we don't need anymore */+free(old_dir);+free(new_dir);+}}staticvoidget_provisional_directory_renames(structmerge_options*opt,
This function roughly follows the same outline as the function of the
same name from merge-recursive.c, but the code diverges in multiple
ways due to some special considerations:
* merge-ort's version needs to update opt->priv->paths with any new
paths (and opt->priv->paths points to struct conflict_infos which
track quite a bit of metadata for each path); merge-recursive's
version would directly update the index
* merge-ort requires that opt->priv->paths has any leading directories
of any relevant files also be included in the set of paths. And
due to pointer equality requirements on merged_info.directory_name,
we have to be careful how we compute and insert these.
* due to the above requirements on opt->priv->paths, merge-ort's
version starts with a long comment to explain all the special
considerations that need to be handled
* merge-ort can use the full data stored in opt->priv->paths to avoid
making expensive get_tree_entry() calls to regather the necessary
data.
* due to messages being deferred automatically in merge-ort, this is
the best place to handle conflict messages whereas in
merge-recursive.c they are deferred manually so that processing of
entries does all the printing
Signed-off-by: Elijah Newren <redacted>
---
merge-ort.c | 168 +++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 167 insertions(+), 1 deletion(-)
@@ -1642,7 +1642,173 @@ static void apply_directory_rename_modifications(struct merge_options *opt,structdiff_filepair*pair,char*new_path){-die("Not yet implemented.");+/*+*Thebasicideaistogettheconflict_infofromopt->priv->paths+*atoldpath,andinsertitintonew_path;basicallyjustthis:+*ci=strmap_get(&opt->priv->paths,old_path);+*strmap_remove(&opt->priv->paths,old_path,0);+*strmap_put(&opt->priv->paths,new_path,ci);+*However,therearesomefactorscomplicatingthis:+*-opt->priv->pathsmayalreadyhaveanentryatnew_path+*-Eachcitracksitscontainingdirectory,soweneedto+*updatethat+*-Ifanothercihasthesamecontainingdirectory,then+*thetwochar*'sMUSTpointtothesamelocation.Seethe+*commentinstructmerged_info.strcmpequalityisnot+*enough;weneedpointerequality.+*-opt->priv->pathsmustholdtheparentdirectoriesofany+*entriesthatareadded.So,ifthisdirectoryrename+*causesentirelynewdirectories,wemustrecursivelyadd+*parentdirectories.+*-Foreachparentdirectoryaddedtoopt->priv->paths,we+*alsoneedtogetitsparentdirectorystoredinits+*conflict_info->merged.directory_namewithallthesame+*requirementsaboutpointerequality.+*/+structstring_listdirs_to_insert=STRING_LIST_INIT_NODUP;+structconflict_info*ci,*new_ci;+structstrmap_entry*entry;+constchar*branch_with_new_path,*branch_with_dir_rename;+constchar*old_path=pair->two->path;+constchar*parent_name;+constchar*cur_path;+inti,len;++entry=strmap_get_entry(&opt->priv->paths,old_path);+old_path=entry->key;+ci=entry->value;+VERIFY_CI(ci);++/* Find parent directories missing from opt->priv->paths */+cur_path=new_path;+while(1){+/* Find the parent directory of cur_path */+char*last_slash=strrchr(cur_path,'/');+if(last_slash){+parent_name=xstrndup(cur_path,last_slash-cur_path);+}else{+parent_name=opt->priv->toplevel_dir;+break;+}++/* Look it up in opt->priv->paths */+entry=strmap_get_entry(&opt->priv->paths,parent_name);+if(entry){+free((char*)parent_name);+parent_name=entry->key;/* reuse known pointer */+break;+}++/* Record this is one of the directories we need to insert */+string_list_append(&dirs_to_insert,parent_name);+cur_path=parent_name;+}++/* Traverse dirs_to_insert and insert them into opt->priv->paths */+for(i=dirs_to_insert.nr-1;i>=0;--i){+structconflict_info*dir_ci;+char*cur_dir=dirs_to_insert.items[i].string;++dir_ci=xcalloc(1,sizeof(*dir_ci));++dir_ci->merged.directory_name=parent_name;+len=strlen(parent_name);+/* len+1 because of trailing '/' character */+dir_ci->merged.basename_offset=(len>0?len+1:len);+dir_ci->dirmask=ci->filemask;+strmap_put(&opt->priv->paths,cur_dir,dir_ci);++parent_name=cur_dir;+}++/*+*Weareremovingold_pathfromopt->priv->paths.old_pathalsowill+*eventuallyneedtobefreed,butitmaystillbeusedbye.g.+*ci->pathnames.So,storeitinanotherstring-listfornow.+*/+string_list_append(&opt->priv->paths_to_free,old_path);++assert(ci->filemask==2||ci->filemask==4);+assert(ci->dirmask==0);+strmap_remove(&opt->priv->paths,old_path,0);++branch_with_new_path=(ci->filemask==2)?opt->branch1:opt->branch2;+branch_with_dir_rename=(ci->filemask==2)?opt->branch2:opt->branch1;++/* Now, finally update ci and stick it into opt->priv->paths */+ci->merged.directory_name=parent_name;+len=strlen(parent_name);+ci->merged.basename_offset=(len>0?len+1:len);+new_ci=strmap_get(&opt->priv->paths,new_path);+if(!new_ci){+/* Place ci back into opt->priv->paths, but at new_path */+strmap_put(&opt->priv->paths,new_path,ci);+}else{+intindex;++/* A few sanity checks */+VERIFY_CI(new_ci);+assert(ci->filemask==2||ci->filemask==4);+assert((new_ci->filemask&ci->filemask)==0);+assert(!new_ci->merged.clean);++/* Copy stuff from ci into new_ci */+new_ci->filemask|=ci->filemask;+if(new_ci->dirmask)+new_ci->df_conflict=1;+index=(ci->filemask>>1);+new_ci->pathnames[index]=ci->pathnames[index];+new_ci->stages[index].mode=ci->stages[index].mode;+oidcpy(&new_ci->stages[index].oid,&ci->stages[index].oid);++free(ci);+ci=new_ci;+}++if(opt->detect_directory_renames==MERGE_DIRECTORY_RENAMES_TRUE){+/* Notify user of updated path */+if(pair->status=='A')+path_msg(opt,new_path,1,+_("Path updated: %s added in %s inside a "+"directory that was renamed in %s; moving "+"it to %s."),+old_path,branch_with_new_path,+branch_with_dir_rename,new_path);+else+path_msg(opt,new_path,1,+_("Path updated: %s renamed to %s in %s, "+"inside a directory that was renamed in %s; "+"moving it to %s."),+pair->one->path,old_path,branch_with_new_path,+branch_with_dir_rename,new_path);+}else{+/*+*opt->detect_directory_renameshasthevalue+*MERGE_DIRECTORY_RENAMES_CONFLICT,somarktheseasconflicts.+*/+ci->path_conflict=1;+if(pair->status=='A')+path_msg(opt,new_path,0,+_("CONFLICT (file location): %s added in %s "+"inside a directory that was renamed in %s, "+"suggesting it should perhaps be moved to "+"%s."),+old_path,branch_with_new_path,+branch_with_dir_rename,new_path);+else+path_msg(opt,new_path,0,+_("CONFLICT (file location): %s renamed to %s "+"in %s, inside a directory that was renamed "+"in %s, suggesting it should perhaps be "+"moved to %s."),+pair->one->path,old_path,branch_with_new_path,+branch_with_dir_rename,new_path);+}++/*+*Finally,recordthenewlocation.+*/+pair->two->path=new_path;}/*** Function Grouping: functions related to regular rename detection ***/
As noted in commit 902c521a35 ("t6423: more involved directory rename
test", 2020-10-15), when we have a case where
* dir/subdir/ has several files
* almost all files in dir/subdir/ are renamed to folder/subdir/
* one of the files in dir/subdir/ is renamed to folder/subdir/newsubdir/
* the other side of history (that doesn't do the renames) adds a
new file to dir/subdir/
Then for the majority of the file renames, the directory rename of
dir/subdir/ -> folder/subdir/
is actually not represented that way but as
dir/ -> folder/
We also had one rename that was represented as
dir/subdir/ -> folder/subdir/newsubdir/
Now, since there's a new file in dir/subdir/, where does it go? Well,
there's only one rule for dir/subdir/, so the code previously noted that
this rule had the "majority" of the one "relevant" rename and thus
erroneously used it to place the file in folder/subdir/newsubdir/. We
really want the heavy weight associated with dir/ -> folder/ to also be
treated as dir/subdir/ -> folder/subdir/, so that we correctly place the
file in folder/subdir/.
Add a bunch of logic to make sure that we use all relevant renamings in
directory rename detection.
Note that testcase 12f of t6423 still fails after this, but it gets
further than merge-recursive does. There are some performance related
bits in that testcase (the region_enter messages) that do not yet
succeed, but the rest of the testcase works after this patch.
Subsequent patch series will fix up the performance side.
Signed-off-by: Elijah Newren <redacted>
---
merge-ort.c | 198 +++++++++++++++++++++-------------------------------
1 file changed, 81 insertions(+), 117 deletions(-)
@@ -1182,109 +1182,6 @@ static char *apply_dir_rename(struct strmap_entry *rename_info,returnstrbuf_detach(&new_path,NULL);}-staticvoidget_renamed_dir_portion(constchar*old_path,constchar*new_path,-char**old_dir,char**new_dir)-{-char*end_of_old,*end_of_new;--/* Default return values: NULL, meaning no rename */-*old_dir=NULL;-*new_dir=NULL;--/*-*For-*"a/b/c/d/e/foo.c"->"a/b/some/thing/else/e/foo.c"-*the"e/foo.c"partisthesame,wejustwanttoknowthat-*"a/b/c/d"wasrenamedto"a/b/some/thing/else"-*so,forthisexample,thisfunctionreturns"a/b/c/d"in-**old_dirand"a/b/some/thing/else"in*new_dir.-*/--/*-*Ifthebasenameofthefilechanged,wedon'tcare.Wewant-*toknowwhichportionofthedirectory,ifany,changed.-*/-end_of_old=strrchr(old_path,'/');-end_of_new=strrchr(new_path,'/');--/*-*Ifend_of_oldisNULL,old_pathwasn'tinadirectory,sothere-*couldnotbeadirectoryrename(ourruleelsewherethata-*directorywhichstillexistsisnotconsideredtohavebeen-*renamedmeanstherootdirectorycanneverberenamed--because-*therootdirectoryalwaysexists).-*/-if(end_of_old==NULL)-return;/* Note: *old_dir and *new_dir are still NULL */--/*-*Ifnew_pathcontainsnodirectory(end_of_newisNULL),thenwe-*havearenameofold_path'sdirectorytotherootdirectory.-*/-if(end_of_new==NULL){-*old_dir=xstrndup(old_path,end_of_old-old_path);-*new_dir=xstrdup("");-return;-}--/* Find the first non-matching character traversing backwards */-while(*--end_of_new==*--end_of_old&&-end_of_old!=old_path&&-end_of_new!=new_path)-;/* Do nothing; all in the while loop */--/*-*Ifbothgotbacktothebeginningoftheirstrings,thenthe-*directorydidn'tchangeatall,onlythebasenamedid.-*/-if(end_of_old==old_path&&end_of_new==new_path&&-*end_of_old==*end_of_new)-return;/* Note: *old_dir and *new_dir are still NULL */--/*-*Ifend_of_newgotbacktothebeginningofitsstring,and-*end_of_oldgotbacktothebeginningofsomesubdirectory,then-*wehavearename/mergeofasubdirectoryintotheroot,which-*needsslightlyspecialhandling.-*-*Note:Thereisnoneedtoconsidertheoppositecase,witha-*rename/mergeoftherootdirectoryintosomesubdirectory-*becauseasnotedabovetherootdirectoryalwaysexistssoit-*cannotbeconsideredtoberenamed.-*/-if(end_of_new==new_path&&-end_of_old!=old_path&&end_of_old[-1]=='/'){-*old_dir=xstrndup(old_path,--end_of_old-old_path);-*new_dir=xstrdup("");-return;-}--/*-*We'vefoundthefirstnon-matchingcharacterinthedirectory-*paths.Thatmeansthecurrentcharacterswewerelookingat-*werepartofthefirstnon-matchingsubdirnamegoingbackfrom-*theendofthestrings.Getthewholenamebyadvancingboth-*end_of_oldandend_of_newtotheNEXT'/'character.Thatwill-*representtheentiredirectoryrename.-*-*Thereasonfortheincrementiscaseslike-*a/b/star/foo/whatever.c->a/b/tar/foo/random.c-*Afterdroppingthebasenameandgoingbacktothefirst-*non-matchingcharacter,we'renowcomparing:-*a/b/sanda/b/-*andwewanttobecomparing:-*a/b/star/anda/b/tar/-*butwithoutthepre-increment,theoneontherightwouldstay-*a/b/.-*/-end_of_old=strchr(++end_of_old,'/');-end_of_new=strchr(++end_of_new,'/');--/* Copy the old and new directories into *old_dir and *new_dir. */-*old_dir=xstrndup(old_path,end_of_old-old_path);-*new_dir=xstrndup(new_path,end_of_new-new_path);-}-staticintpath_in_way(structstrmap*paths,constchar*path,unsignedside_mask){structmerged_info*mi=strmap_get(paths,path);
@@ -1391,6 +1296,76 @@ static void increment_count(struct strmap *dir_rename_count,strintmap_incr(counts,new_dir,1);}+staticvoidupdate_dir_rename_counts(structstrmap*dir_rename_count,+structstrset*dirs_removed,+constchar*oldname,+constchar*newname)+{+char*old_dir=xstrdup(oldname);+char*new_dir=xstrdup(newname);+charnew_dir_first_char=new_dir[0];+intfirst_time_in_loop=1;++while(1){+dirname_munge(old_dir);+dirname_munge(new_dir);++/*+*Whenrenaming+*"a/b/c/d/e/foo.c"->"a/b/some/thing/else/e/foo.c"+*thenthissuggeststhatboth+*a/b/c/d/e/=>a/b/some/thing/else/e/+*a/b/c/d/=>a/b/some/thing/else/+*sowewanttoincrementcountersforboth.WedoNOT,+*however,alsowanttosuggestthattherewasthefollowing+*rename:+*a/b/c/=>a/b/some/thing/+*soweneedtoquitatthatpoint.+*+*Notethewhenfirst_time_in_loop,weonlystripoffthe+*basename,andwedon'tcareifthat'sdifferent.+*/+if(!first_time_in_loop){+char*old_sub_dir=strchr(old_dir,'\0')+1;+char*new_sub_dir=strchr(new_dir,'\0')+1;+if(!*new_dir){+/*+*Specialcasewhenrenamingtorootdirectory,+*i.e.whennew_dir=="".Inthiscase,wehad+*somethinglike+*a/b/subdir=>subdir+*andsodirname_munge()setsthingsupsothat+*old_dir="a/b\0subdir\0"+*new_dir="\0ubdir\0"+*Wedidn'thavea'/'tooverwritea'\0'onto+*innew_dir,sowehavetocomparedifferently.+*/+if(new_dir_first_char!=old_sub_dir[0]||+strcmp(old_sub_dir+1,new_sub_dir))+break;+}else{+if(strcmp(old_sub_dir,new_sub_dir))+break;+}+}++if(strset_contains(dirs_removed,old_dir))+increment_count(dir_rename_count,old_dir,new_dir);+else+break;++/* If we hit toplevel directory ("") for old or new dir, quit */+if(!*old_dir||!*new_dir)+break;++first_time_in_loop=0;+}++/* Free resources we don't need anymore */+free(old_dir);+free(new_dir);+}+staticvoidcompute_rename_counts(structdiff_queue_struct*pairs,structstrmap*dir_rename_count,structstrset*dirs_removed)
@@ -1398,19 +1373,11 @@ static void compute_rename_counts(struct diff_queue_struct *pairs,inti;for(i=0;i<pairs->nr;++i){-char*old_dir,*new_dir;structdiff_filepair*pair=pairs->queue[i];if(pair->status!='R')continue;-/* Get the old and new directory names */-get_renamed_dir_portion(pair->one->path,pair->two->path,-&old_dir,&new_dir);-if(!old_dir)-/* Directory didn't change at all; ignore this one. */-continue;-/**Makedir_rename_countcontainamapofamap:*old_directory->{new_directory->count}
@@ -1418,12 +1385,9 @@ static void compute_rename_counts(struct diff_queue_struct *pairs,*theoldfilenameandthenewfilenameandcounthowmany*timesthatpairingoccurs.*/-if(strset_contains(dirs_removed,old_dir))-increment_count(dir_rename_count,old_dir,new_dir);--/* Free resources we don't need anymore */-free(old_dir);-free(new_dir);+update_dir_rename_counts(dir_rename_count,dirs_removed,+pair->one->path,+pair->two->path);}}
Both of these are copied from merge-recursive.c, with just minor tweaks
due to using strmap API and not having a non_unique_new_dir field.
Signed-off-by: Elijah Newren <redacted>
---
merge-ort.c | 37 +++++++++++++++++++++++++++++++++++--
1 file changed, 35 insertions(+), 2 deletions(-)
This is copied from merge-recursive.c, with minor tweaks due to:
* using strmap API
* merge-ort not using the non_unique_new_dir field, since it'll
obviate its need entirely later with performance improvements
* adding a new path_in_way() function that uses opt->priv->paths
instead of doing an expensive tree_has_path() lookup to see if
a tree has a given path.
Signed-off-by: Elijah Newren <redacted>
---
merge-ort.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 71 insertions(+), 1 deletion(-)
@@ -1294,7 +1304,67 @@ static char *handle_path_level_conflicts(struct merge_options *opt,structstrmap_entry*rename_info,structstrmap*collisions){-die("Not yet implemented");+char*new_path=NULL;+structcollision_info*c_info;+intclean=1;+structstrbufcollision_paths=STRBUF_INIT;++/*+*entryhasthemappingofolddirectorynametonewdirectoryname+*thatwewanttoapplytopath.+*/+new_path=apply_dir_rename(rename_info,path);+if(!new_path)+BUG("Failed to apply directory rename!");++/*+*Thecallerneedstohaveensuredthatithaspre-populated+*collisionswithallpathsthatmaptonew_path.Doaquickcheck+*toensurethat'sthecase.+*/+c_info=strmap_get(collisions,new_path);+if(c_info==NULL)+BUG("c_info is NULL");++/*+*Checkforone-sidedadd/add/.../addconflicts,i.e.+*whereimplicitrenamesfromtheothersidedoing+*directoryrename(s)canaffectthissideofhistory+*toputmultiplepathsintothesamelocation.Warn+*andbailondirectoryrenamesforsuchpaths.+*/+if(c_info->reported_already){+clean=0;+}elseif(path_in_way(&opt->priv->paths,new_path,1<<side_index)){+c_info->reported_already=1;+strbuf_add_separated_string_list(&collision_paths,", ",+&c_info->source_files);+path_msg(opt,new_path,0,+_("CONFLICT (implicit dir rename): Existing file/dir "+"at %s in the way of implicit directory rename(s) "+"putting the following path(s) there: %s."),+new_path,collision_paths.buf);+clean=0;+}elseif(c_info->source_files.nr>1){+c_info->reported_already=1;+strbuf_add_separated_string_list(&collision_paths,", ",+&c_info->source_files);+path_msg(opt,new_path,0,+_("CONFLICT (implicit dir rename): Cannot map more "+"than one path to %s; implicit directory renames "+"tried to put these paths there: %s"),+new_path,collision_paths.buf);+clean=0;+}++/* Free memory we no longer need */+strbuf_release(&collision_paths);+if(!clean&&new_path){+free(new_path);+returnNULL;+}++returnnew_path;}staticvoidincrement_count(structstrmap*dir_rename_count,
Since directory rename detection adds new paths to opt->priv->paths and
removes old ones, process_renames() needs to now check whether
pair->one->path actually exists in opt->priv->paths instead of just
assuming it does.
Signed-off-by: Elijah Newren <redacted>
---
merge-ort.c | 26 +++++++++++++++++++++-----
1 file changed, 21 insertions(+), 5 deletions(-)