Teach mv to move submodules
cover Teach mv to move submodules
This is the last topic I intend to finish before preparing the recursive submodule update. The other prerequisites for that next step are Heiko's "allow more sources for config values" and "fetch moved submodules on-demand" topics, which are necessary to populate submodules that were not present in the currently checked out commit and to have all commits of moved or currently not present submodules fetched for later checkout respectively. But these are other threads ...
Let's enable users to move submodules (just as easy as they can already remove them with current Git with "git rm") by using a plain "git mv". While using "git submodule update" on a moved submodule cloned by a coworker will still leave the submodule's work tree lying around at the old path, that will be fixed by the recursive submodule update (and is not different from what Git currently does with removed submodules). Replacing a directory containing files tracked by Git or vice versa is still not possible, another issue to be fixed by the recursive submodule update. I tried to CC all parties which showed interest in this topic, hopefully I didn't forget anyone.
Jens Lehmann (3):
Teach mv to move submodules together with their work trees Teach mv to move submodules using a gitfile
Teach mv to update the path entry in .gitmodules for moved submodules
builtin/mv.c | 126 ++++++++++++++++++++++++++++++++++------------------------ submodule.c | 126 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ submodule.h | 3 ++ t/t7001-mv.sh | 94 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 297 insertions(+), 52 deletions(-)
Signature · 1 line
1.8.2.377.g1bdb7d0
PATCH 1/3 Teach mv to move submodules together with their work trees
Currently the attempt to use "git mv" on a submodule errors out with:
fatal: source directory is empty, source=<src>, destination=<dest>
The reason is that mv searches for the submodule with a trailing slash in the index, which it doesn't find (because it is stored without a trailing slash). As it doesn't find any index entries inside the submodule it claims the directory would be empty even though it isn't.
Fix that by searching for the name without a trailing slash and continue if it is a submodule. Then rename() will move the submodule work tree just like it moves a file.
--- builtin/mv.c | 99 +++++++++++++++++++++++++++++++---------------------------- t/t7001-mv.sh | 34 ++++++++++++++++++++ 2 files changed, 86 insertions(+), 47 deletions(-)
builtin/mv.c +52 -47
@@ -117,55 +117,60 @@ int cmd_mv(int argc, const char **argv, const char *prefix) && lstat(dst, &st) == 0) bad =_ ("cannot move directory over file" ); else if (src_is_dir) { -const char *src_w_slash =add_slash (src); -int len_w_slash = length +1 ; -int first, last; - - modes[i] =WORKING_DIRECTORY ; - - first =cache_name_pos (src_w_slash, len_w_slash); -if (first >=0 ) -die (_ ("Huh? %.*s is in index?" ), - len_w_slash, src_w_slash); - - first =-1 - first; -for (last = first; last < active_nr; last++) { -const char *path = active_cache[last]->name; -if (strncmp (path, src_w_slash, len_w_slash)) -break ; - } -free ((char *)src_w_slash); - -if (last - first <1 ) - bad =_ ("source directory is empty" ); -else { -int j, dst_len; - -if (last - first >0 ) { - source =xrealloc (source, - (argc + last - first) - *sizeof (char *)); - destination =xrealloc (destination, - (argc + last - first) - *sizeof (char *)); - modes =xrealloc (modes, - (argc + last - first) - *sizeof (enum update_mode )); +int first =cache_name_pos (src, length); +if (first >=0 ) { +if (!S_ISGITLINK (active_cache[first]->ce_mode)) +die (_ ("Huh? Directory %s is in index and no submodule?" ), src); + }else { +const char *src_w_slash =add_slash (src); +int last, len_w_slash = length +1 ; + + modes[i] =WORKING_DIRECTORY ; + + first =cache_name_pos (src_w_slash, len_w_slash); +if (first >=0 ) +die (_ ("Huh? %.*s is in index?" ), + len_w_slash, src_w_slash); + + first =-1 - first; +for (last = first; last < active_nr; last++) { +const char *path = active_cache[last]->name; +if (strncmp (path, src_w_slash, len_w_slash)) +break ; } - - dst =add_slash (dst); - dst_len =strlen (dst); - -for (j =0 ; j < last - first; j++) { -const char *path = - active_cache[first + j]->name; - source[argc + j] = path; - destination[argc + j] = -prefix_path (dst, dst_len, - path + length +1 ); - modes[argc + j] =INDEX ; +free ((char *)src_w_slash); + +if (last - first <1 ) + bad =_ ("source directory is empty" ); +else { +int j, dst_len; + +if (last - first >0 ) { + source =xrealloc (source, + (argc + last - first) + *sizeof (char *)); + destination =xrealloc (destination, + (argc + last - first) + *sizeof (char *)); + modes =xrealloc (modes, + (argc + last - first) + *sizeof (enum update_mode )); + } + + dst =add_slash (dst); + dst_len =strlen (dst); + +for (j =0 ; j < last - first; j++) { +const char *path = + active_cache[first + j]->name; + source[argc + j] = path; + destination[argc + j] = +prefix_path (dst, dst_len, + path + length +1 ); + modes[argc + j] =INDEX ; + } + argc += last - first; } - argc += last - first; } } else if (cache_name_pos(src, length) < 0) bad =_ ("not under version control" );
t/t7001-mv.sh +34 -0
@@ -255,4 +255,38 @@ test_expect_success SYMLINKS 'git mv should overwrite file with a symlink' ' rm -f moved symlink +test_expect_success 'setup submodule' ' + git commit -m initial && + git reset --hard && + git submodule add ./. sub && + echo content >file && + git add file && + git commit -m "added sub and file" +' + +test_expect_success 'git mv cannot move a submodule in a file' ' + test_must_fail git mv sub file +' + +test_expect_success 'git mv moves a submodule with a .git directory and no .gitmodules' ' + entry="$(git ls-files --stage sub | cut -f 1)" && + git rm .gitmodules && + ( + cd sub && + rm -f .git && + cp -a ../.git/modules/sub .git && + GIT_WORK_TREE=. git config --unset core.worktree + ) && + mkdir mod && + git mv sub mod/sub && + ! test -e sub && + [ "$entry" = "$(git ls-files --stage mod/sub | cut -f 1)" ] && + ( + cd mod/sub && + git status + ) && + git update-index --refresh && + git diff-files --quiet +' + test_done
Signature · 1 line
1.8.2.377.g1bdb7d0
Re: [PATCH/RFC 1/3] Teach mv to move submodules together with their work trees
Quoted 6 lines · Jens Lehmann
Currently the attempt to use "git mv" on a submodule errors out with:
fatal: source directory is empty, source=<src>, destination=<dest>
The reason is that mv searches for the submodule with a trailing slash in the index, which it doesn't find (because it is stored without a trailing slash). As it doesn't find any index entries inside the submodule it claims the directory would be empty even though it isn't.
Why does it search for a submodule with a trailing slash in the index?
You make it sound like it's doing something unnatural; in reality, it
does this because it executes lstat() on the filesystem path specified, and the stat mode matches S_ISDIR (because it _is_ a directory on the filesystem). It is stored in the index as an entry (without a trailing slash) with the mode 160000, gitlink.
What happens if I attempt to git mv oldpath/ newpath/ (with the literal slashes, probably because I'm using a stupid shell completion)?
Quoted 2 lines
Fix that by searching for the name without a trailing slash and continue if it is a submodule.
So, the correct solution is not to "search for a name without a trailing slash", but rather to handle the gitlink entry in the index appropriately.
Quoted 2 lines
Then
rename()will move the submodule work tree just like it moves a file.
What is this rename() function you're talking about? I don't see it anywhere.
Quoted 36 lines
builtin/mv.c +0
-28@@ -117,55 +117,60 @@ int cmd_mv(int argc, const char **argv, const char *prefix) && lstat(dst, &st) == 0) bad =_ ("cannot move directory over file" ); else if (src_is_dir) { -const char *src_w_slash =add_slash (src); -int len_w_slash = length +1 ; -int first, last; - - modes[i] =WORKING_DIRECTORY ; - - first =cache_name_pos (src_w_slash, len_w_slash); -if (first >=0 ) -die (_ ("Huh? %.*s is in index?" ), - len_w_slash, src_w_slash); - - first =-1 - first; -for (last = first; last < active_nr; last++) { -const char *path = active_cache[last]->name; -if (strncmp (path, src_w_slash, len_w_slash)) -break ; - } -free ((char *)src_w_slash); - -if (last - first <1 ) - bad =_ ("source directory is empty" ); -else { -int j, dst_len; - -if (last - first >0 ) { - source =xrealloc (source, - (argc + last - first) - *sizeof (char *));
builtin/mv.c int cmd_mv(int argc, const char **argv, const char *prefix) · line 123
Quoted 10 lines
builtin/mv.c +4
-6- destination =xrealloc (destination, - (argc + last - first) - *sizeof (char *)); - modes =xrealloc (modes, - (argc + last - first) - *sizeof (enum update_mode )); +int first =cache_name_pos (src, length); +if (first >=0 ) { +if (!S_ISGITLINK (active_cache[first]->ce_mode)) +die (_ ("Huh? Directory %s is in index and no submodule?" ), src);
I didn't understand this. Why does it have to be a gitlink if it is stored at index position >= 0?
I'm assuming the else-case has nothing to do with the actual moving but rather something specific to directories (enumerating entries in it?), which is why it doesn't get executed when we find a gitlink.
Quoted 7 lines
builtin/mv.c +7
-0+ }else { +const char *src_w_slash =add_slash (src); +int last, len_w_slash = length +1 ; + + modes[i] =WORKING_DIRECTORY ; + + first =cache_name_pos (src_w_slash, len_w_slash);
builtin/mv.c int cmd_mv(int argc, const char **argv, const char *prefix) · line 140
Quoted 10 lines
builtin/mv.c +9
-0+if (first >=0 ) +die (_ ("Huh? %.*s is in index?" ), + len_w_slash, src_w_slash); + + first =-1 - first; +for (last = first; last < active_nr; last++) { +const char *path = active_cache[last]->name; +if (strncmp (path, src_w_slash, len_w_slash)) +break ; }
Mostly unchanged, but I didn't understand the line first = -1 - first in the original. What is it doing? So, I'm guessing first is the cache position of the directory itself, and last stores the index of the "last" entry in the cache? What does that even mean?
Quoted 6 lines
builtin/mv.c +0
-6- - dst =add_slash (dst); - dst_len =strlen (dst); - -for (j =0 ; j < last - first; j++) { - const char *path =
builtin/mv.c int cmd_mv(int argc, const char **argv, const char *prefix) · line 144
Quoted 10 lines
builtin/mv.c +4
-6- active_cache[first + j]->name; - source[argc + j] = path; - destination[argc + j] = -prefix_path (dst, dst_len, - path + length +1 ); - modes[argc + j] =INDEX ; +free ((char *)src_w_slash); + +if (last - first <1 ) + bad =_ ("source directory is empty" );
This is exactly what was tripping us up earlier. Can you explain what last - first < 1 means?
Quoted 19 lines
builtin/mv.c +19
-0+else { +int j, dst_len; + +if (last - first >0 ) { + source =xrealloc (source, + (argc + last - first) + *sizeof (char *)); + destination =xrealloc (destination, + (argc + last - first) + *sizeof (char *)); + modes =xrealloc (modes, + (argc + last - first) + *sizeof (enum update_mode )); + } + + dst =add_slash (dst); + dst_len =strlen (dst); + +for (j =0 ; j < last - first; j++) {
builtin/mv.c int cmd_mv(int argc, const char **argv, const char *prefix) · line 173
Quoted 10 lines
builtin/mv.c +9
-0+const char *path = + active_cache[first + j]->name; + source[argc + j] = path; + destination[argc + j] = +prefix_path (dst, dst_len, + path + length +1 ); + modes[argc + j] =INDEX ; + } + argc += last - first; }
Mostly unchanged, but hard to review because I can't easily see what changed and what didn't.
builtin/mv.c int cmd_mv(int argc, const char **argv, const char *prefix) · line 168
Quoted 1 line
builtin/mv.c +0
-1- argc += last - first;
Why did you remove this line?
Re: [PATCH/RFC 1/3] Teach mv to move submodules together with their work trees
Quoted 10 lines · Ramkumar Ramachandra
Why does it search for a submodule with a trailing slash in the index?
You make it sound like it's doing something unnatural; in reality, it
does this because it executeslstat()on the filesystem path specified, and the stat mode matchesS_ISDIR(because it _is_ a directory on the filesystem). It is stored in the index as an entry (without a trailing slash) with the mode 160000, gitlink.What happens if I attempt to git mv oldpath/ newpath/ (with the literal slashes, probably because I'm using a stupid shell completion)?
I think it should work.
mkdir a && >a/f && git add a/f && git mv a/ b/
Quoted 6 lines
Quoted 2 lines
Fix that by searching for the name without a trailing slash and continue if it is a submodule.
So, the correct solution is not to "search for a name without a trailing slash", but rather to handle the gitlink entry in the index appropriately.
For moving an entire directory's contents, because the index is flat, you would look for "name/", because you know all of the paths contained in it will share that prefix.
But when dealing with a submodule, you need to see if the path that found to be a directory in the working tree is a gitlink in the index. And the way to do so is to look for it without trailing slash, because there is nothing hanging under it in the index.
So the right implementation of "handle appropriately" is to "search without slash". They are saying the same thing, and the latter is a more specific way to point out in what way the existing code that wanted to delegate moving to "submodule mv" and not having to worry about gitlinks was unprepared for it, and what change is needed to make it "handle appropriately".
So I think there is no need to rephrase here, but your comment makes me wonder if the patch covers the case where you have a submodule at "a/x" and the user does "git mv a/ b/". The src_is_dir thing will notice "a/" is a directory, finds all the paths inside a/ including "a/x" (and we won't see any paths inside the submodule's working tree) to "b/", and update the cache entries and the working tree. Does the adjusting of the path for that moved submodule, which is a theme for [PATCH 3/3], cover that case, too?
Another thing to wonder is if we correctly reject "git mv a/x/f here" in the same example where a/x is a directory. The path is beyond the lower boundary of our working tree and should not be touched.
Re: [PATCH/RFC 1/3] Teach mv to move submodules together with their work trees
In reply to Ramkumar RamachandraQuoted 4 lines · Ramkumar Ramachandra
Quoted 2 lines
Then
rename()will move the submodule work tree just like it moves a file.What is this
rename()function you're talking about? I don't see it anywhere.
"man 2 rename"; it is called from a generic part of builtin/mv.c to rename one path to another and can move both files and directories.
Quoted 6 lines
builtin/mv.c int cmd_mv(int argc, const char **argv, const char *prefix) · line 123
Quoted 3 lines
builtin/mv.c +3
-0+if (first >=0 ) { +if (!S_ISGITLINK (active_cache[first]->ce_mode)) +die (_ ("Huh? Directory %s is in index and no submodule?" ), src);I didn't understand this. Why does it have to be a gitlink if it is stored at index position >= 0?
The path is not in the middle of the conflict, but the index records something that is not a gitlink. E.g. you start from A (regular file) in the index, "rm A && mkdir A" and make it the top of a working tree of a separate project. "git mv A elsewhere" will say "src_is_dir" but the index still thinks A should be a regular file blob.
Quoted 3 lines
I'm assuming the else-case has nothing to do with the actual moving but rather something specific to directories (enumerating entries in it?), which is why it doesn't get executed when we find a gitlink.
It wants to move all the paths in the directory to a new destination, e.g. "git mv srcdir dstdir", and update
Quoted 14 lines
Quoted 14 lines
builtin/mv.c +14
-0+ }else { +const char *src_w_slash =add_slash (src); +int last, len_w_slash = length +1 ; + + modes[i] =WORKING_DIRECTORY ; + + first =cache_name_pos (src_w_slash, len_w_slash); +if (first >=0 ) +die (_ ("Huh? %.*s is in index?" ), + len_w_slash, src_w_slash); + + first =-1 - first; +for (last = first; last < active_nr; last++) { +const char *path = active_cache[last]->name;
Quoted 8 lines
builtin/mv.c int cmd_mv(int argc, const char **argv, const char *prefix) · line 140
Quoted 3 lines
builtin/mv.c +2
-0+if (strncmp (path, src_w_slash, len_w_slash)) +break ; }Mostly unchanged, but I didn't understand the line first = -1 - first in the original. What is it doing? So, I'm guessing first is the cache position of the directory itself, and last stores the index of the "last" entry in the cache? What does that even mean?
cache_name_pos() either returns the position for a path at stage #0 that exists in the index, or the position the given path _would_ be inserted if you were to add it to the index for a path that does not exist in the index at stage #0 (I think this part of the code does not consider that the given path is unmerged, either by being sloppy or detecting that case much earlier---I didn't check), and when doing the latter, it encodes the position by negating it and offsetting it by 1 (otherwise you cannot tell "it would come at the very beginning" and "it is at the very beginning", because negated zero is still zero). The "-1 - first" is an idiom used everywhere by callers of cache_name_pos() to recover the latter from the returned value.
If you start at a position "src/" would have been inserted, and iterate over the index while the entry's path prefix-matches with "src/", you will find where the entries in the "src/" directory ends.
Quoted 5 lines
builtin/mv.c int cmd_mv(int argc, const char **argv, const char *prefix) · line 144
Quoted 2 lines
builtin/mv.c +2
-0+if (last - first <1 ) + bad =_ ("source directory is empty" );This is exactly what was tripping us up earlier. Can you explain what last - first < 1 means?
I think the above covers it.
Asking questions to learn the basic part of Git internals on this list, e.g. "I found this existing code, and I do not understand what it is doing. Can somebody shed a light on it?", is perfectly fine, but can you do so outside the review discussion?
It clutters the review discussions when such a request for education is labeled as if it were a review or mixed with a message with genuine review comments.
PATCH 2/3 Teach mv to move submodules using a gitfile
In reply to Jens LehmannWhen moving a submodule which uses a gitfile to point to the git directory stored in .git/modules/<name> of the superproject two changes must be made to make the submodule work: the .git file and the core.worktree setting must be adjusted to point from work tree to git directory and back.
Achieve that by remembering which submodule uses a gitfile by storing the result of read_gitfile() of each submodule. If that is not NULL the new function connect_work_tree_and_git_dir() is called after renaming the submodule's work tree which updates the two settings to the new values.
--- builtin/mv.c | 19 ++++++++++++++---- submodule.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ submodule.h | 1 + t/t7001-mv.sh | 19 ++++++++++++++++++ 4 files changed, 99 insertions(+), 4 deletions(-)
builtin/mv.c +15 -4
@@ -9,6 +9,7 @@#include "cache-tree.h" #include "string-list.h" #include "parse-options.h" +#include "submodule.h" static const char *const builtin_mv_usage[] = {N_ ("git mv [options] <source>... <destination>" ),
@@ -65,7 +66,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)OPT_BOOLEAN ('k' ,NULL , &ignore_errors,N_ ("skip move/rename errors" )), OPT_END(), }; -const char **source, **destination, **dest_path; +const char **source, **destination, **dest_path, **submodule_gitfile;enum update_mode {BOTH =0 ,WORKING_DIRECTORY ,INDEX } *modes;struct stat st;struct string_list src_for_dst =STRING_LIST_INIT_NODUP ;
@@ -84,6 +85,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix) source =copy_pathspec (prefix, argv, argc,0 ); modes =xcalloc (argc,sizeof (enum update_mode )); dest_path =copy_pathspec (prefix, argv + argc,1 ,0 ); + submodule_gitfile =xcalloc (argc,sizeof (char *));if (dest_path[0 ][0 ] =='\0' )/* special case: "." was normalized to "" */
@@ -119,8 +121,14 @@ int cmd_mv(int argc, const char **argv, const char *prefix)else if (src_is_dir) {int first =cache_name_pos (src, length);if (first >=0 ) { +struct strbuf submodule_dotgit =STRBUF_INIT ;if (!S_ISGITLINK (active_cache[first]->ce_mode))die (_ ("Huh? Directory %s is in index and no submodule?" ), src); +strbuf_addf (&submodule_dotgit,"%s/.git" , src); + submodule_gitfile[i] =read_gitfile (submodule_dotgit.buf); +if (submodule_gitfile[i]) + submodule_gitfile[i] =xstrdup (submodule_gitfile[i]); +strbuf_release (&submodule_dotgit); }else {const char *src_w_slash =add_slash (src);int last, len_w_slash = length +1 ;
@@ -215,9 +223,12 @@ int cmd_mv(int argc, const char **argv, const char *prefix)int pos;if (show_only || verbose)printf (_ ("Renaming %s to %s\n" ), src, dst); -if (!show_only && mode !=INDEX && -rename (src, dst) <0 && !ignore_errors) -die_errno (_ ("renaming '%s' failed" ), src); +if (!show_only && mode !=INDEX ) { +if (rename (src, dst) <0 && !ignore_errors) +die_errno (_ ("renaming '%s' failed" ), src); +if (submodule_gitfile[i]) +connect_work_tree_and_git_dir (dst, submodule_gitfile[i]); + }if (mode ==WORKING_DIRECTORY )continue ;
submodule.c +64 -0
67 lines not on this page · raw message
submodule.h +1 -0
@@ -35,5 +35,6 @@ int merge_submodule(unsigned char result[20], const char *path, const unsigned cint find_unpushed_submodules (unsigned char new_sha1[20 ],const char *remotes_name,struct string_list *needs_pushing);int push_unpushed_submodules (unsigned char new_sha1[20 ],const char *remotes_name); +void connect_work_tree_and_git_dir (const char *work_tree,const char *git_dir);#endif
t/t7001-mv.sh +19 -0
@@ -289,4 +289,23 @@ test_expect_success 'git mv moves a submodule with a .git directory and no .gitm git diff-files --quiet ' +test_expect_success 'git mv moves a submodule with gitfile' ' + rm -rf mod/sub && + git reset --hard && + git submodule update && + entry="$(git ls-files --stage sub | cut -f 1)" && + ( + cd mod && + git mv ../sub/ . + ) && + ! test -e sub && + [ "$entry" = "$(git ls-files --stage mod/sub | cut -f 1)" ] && + ( + cd mod/sub && + git status + ) && + git update-index --refresh && + git diff-files --quiet +' + test_done
Signature · 1 line
1.8.2.377.g1bdb7d0
Re: [PATCH/RFC 2/3] Teach mv to move submodules using a gitfile
submodule.c int merge_submodule(unsigned char result[20], const char *path, · line 1041
Quoted 9 lines · Jens Lehmann
submodule.c +3
-0@@ -1001,3 +1001,67 @@ int merge_submodule(unsigned char result[20], const char *path, +if (!fp) +die (_ ("Could not create git link %s" ), gitfile_name.buf); +fprintf (fp, gitfile_content.buf);
Perhaps.
fprintf(fp, "%s", gitfile_content.buf);
Re: [PATCH/RFC 2/3] Teach mv to move submodules using a gitfile
Am 10.04.2013 01:08, schrieb Junio C Hamano:
Quoted 14 lines
submodule.c int merge_submodule(unsigned char result[20], const char *path, · line 1041
Quoted 9 lines · Jens Lehmann
submodule.c +3
-0@@ -1001,3 +1001,67 @@ int merge_submodule(unsigned char result[20], const char *path, +if (!fp) +die (_ ("Could not create git link %s" ), gitfile_name.buf); +fprintf (fp, gitfile_content.buf);Perhaps.
Quoted 1 line
fprintf(fp, "%s", gitfile_content.buf);
Sure. Will fix in v2.
PATCH v2 2/3 Teach mv to move submodules using a gitfile
When moving a submodule which uses a gitfile to point to the git directory stored in .git/modules/<name> of the superproject two changes must be made to make the submodule work: the .git file and the core.worktree setting must be adjusted to point from work tree to git directory and back.
Achieve that by remembering which submodule uses a gitfile by storing the result of read_gitfile() of each submodule. If that is not NULL the new function connect_work_tree_and_git_dir() is called after renaming the submodule's work tree which updates the two settings to the new values.
---
Am 10.04.2013 18:59, schrieb Jens Lehmann:
Quoted 15 lines
Am 10.04.2013 01:08, schrieb Junio C Hamano:
Quoted 14 lines
submodule.c int merge_submodule(unsigned char result[20], const char *path,
Quoted 9 lines · Jens Lehmann
submodule.c +3
-0@@ -1001,3 +1001,67 @@ int merge_submodule(unsigned char result[20], const char *path, +if (!fp) +die (_ ("Could not create git link %s" ), gitfile_name.buf); +fprintf (fp, gitfile_content.buf);Perhaps.
Quoted 3 lines
Quoted 1 line
fprintf(fp, "%s", gitfile_content.buf);Sure. Will fix in v2.
Here we go.
builtin/mv.c | 19 ++++++++++++++---- submodule.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ submodule.h | 1 + t/t7001-mv.sh | 19 ++++++++++++++++++ 4 files changed, 99 insertions(+), 4 deletions(-)
builtin/mv.c +15 -4
@@ -9,6 +9,7 @@#include "cache-tree.h" #include "string-list.h" #include "parse-options.h" +#include "submodule.h" static const char *const builtin_mv_usage[] = {N_ ("git mv [options] <source>... <destination>" ),
@@ -65,7 +66,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)OPT_BOOLEAN ('k' ,NULL , &ignore_errors,N_ ("skip move/rename errors" )), OPT_END(), }; -const char **source, **destination, **dest_path; +const char **source, **destination, **dest_path, **submodule_gitfile;enum update_mode {BOTH =0 ,WORKING_DIRECTORY ,INDEX } *modes;struct stat st;struct string_list src_for_dst =STRING_LIST_INIT_NODUP ;
@@ -84,6 +85,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix) source =copy_pathspec (prefix, argv, argc,0 ); modes =xcalloc (argc,sizeof (enum update_mode )); dest_path =copy_pathspec (prefix, argv + argc,1 ,0 ); + submodule_gitfile =xcalloc (argc,sizeof (char *));if (dest_path[0 ][0 ] =='\0' )/* special case: "." was normalized to "" */
@@ -119,8 +121,14 @@ int cmd_mv(int argc, const char **argv, const char *prefix)else if (src_is_dir) {int first =cache_name_pos (src, length);if (first >=0 ) { +struct strbuf submodule_dotgit =STRBUF_INIT ;if (!S_ISGITLINK (active_cache[first]->ce_mode))die (_ ("Huh? Directory %s is in index and no submodule?" ), src); +strbuf_addf (&submodule_dotgit,"%s/.git" , src); + submodule_gitfile[i] =read_gitfile (submodule_dotgit.buf); +if (submodule_gitfile[i]) + submodule_gitfile[i] =xstrdup (submodule_gitfile[i]); +strbuf_release (&submodule_dotgit); }else {const char *src_w_slash =add_slash (src);int last, len_w_slash = length +1 ;
@@ -215,9 +223,12 @@ int cmd_mv(int argc, const char **argv, const char *prefix)int pos;if (show_only || verbose)printf (_ ("Renaming %s to %s\n" ), src, dst); -if (!show_only && mode !=INDEX && -rename (src, dst) <0 && !ignore_errors) -die_errno (_ ("renaming '%s' failed" ), src); +if (!show_only && mode !=INDEX ) { +if (rename (src, dst) <0 && !ignore_errors) +die_errno (_ ("renaming '%s' failed" ), src); +if (submodule_gitfile[i]) +connect_work_tree_and_git_dir (dst, submodule_gitfile[i]); + }if (mode ==WORKING_DIRECTORY )continue ;
submodule.c +64 -0
67 lines not on this page · raw message
submodule.h +1 -0
@@ -35,5 +35,6 @@ int merge_submodule(unsigned char result[20], const char *path, const unsigned cint find_unpushed_submodules (unsigned char new_sha1[20 ],const char *remotes_name,struct string_list *needs_pushing);int push_unpushed_submodules (unsigned char new_sha1[20 ],const char *remotes_name); +void connect_work_tree_and_git_dir (const char *work_tree,const char *git_dir);#endif
t/t7001-mv.sh +19 -0
@@ -289,4 +289,23 @@ test_expect_success 'git mv moves a submodule with a .git directory and no .gitm git diff-files --quiet ' +test_expect_success 'git mv moves a submodule with gitfile' ' + rm -rf mod/sub && + git reset --hard && + git submodule update && + entry="$(git ls-files --stage sub | cut -f 1)" && + ( + cd mod && + git mv ../sub/ . + ) && + ! test -e sub && + [ "$entry" = "$(git ls-files --stage mod/sub | cut -f 1)" ] && + ( + cd mod/sub && + git status + ) && + git update-index --refresh && + git diff-files --quiet +' + test_done
Signature · 1 line
1.8.2.1.345.gb37ac0e
Re: [PATCH v2 2/3] Teach mv to move submodules using a gitfile
Quoted 4 lines · Jens Lehmann
When moving a submodule which uses a gitfile to point to the git directory stored in .git/modules/<name> of the superproject two changes must be made to make the submodule work: the .git file and the core.worktree setting must be adjusted to point from work tree to git directory and back.
Isn't it untrue that the git directory is stored in .git/modules/<name>: it is stored in .git/modules/path/to/module. I thought the whole point of this complex scheme was to avoid name conflicts with submodules with the same name in other directories. Then why aren't you moving the object store as well? What happens if I try to create a submodule in the old path of this moved submodule in the future? How will you fail, and how will the user recover from this?
A nit on the wording. Perhaps: "the .git file in the worktree must be adjusted to point to $GITDIR, and core.worktree must be set to point to the worktree."
Quoted 4 lines
Achieve that by remembering which submodule uses a gitfile by storing the result of
read_gitfile()of each submodule. If that is not NULL the new functionconnect_work_tree_and_git_dir()is called after renaming the submodule's work tree which updates the two settings to the new values.
Oh God. Can't you figure out at mv-time if the submodule uses a gitfile? Why are you storing it? Where are you storing it exactly?
Quoted 22 lines
builtin/mv.c +2
-1@@ -9,6 +9,7 @@#include "cache-tree.h" #include "string-list.h" #include "parse-options.h" +#include "submodule.h" static const char *const builtin_mv_usage[] = {N_ ("git mv [options] <source>... <destination>" ),@@ -65,7 +66,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)OPT_BOOLEAN ('k' ,NULL , &ignore_errors,N_ ("skip move/rename errors" )), OPT_END(), }; -const char **source, **destination, **dest_path; +const char **source, **destination, **dest_path, **submodule_gitfile;enum update_mode {BOTH =0 ,WORKING_DIRECTORY ,INDEX } *modes;struct stat st;struct string_list src_for_dst =STRING_LIST_INIT_NODUP ;
builtin/mv.c int cmd_mv(int argc, const char **argv, const char *prefix) · line 88
Quoted 4 lines
builtin/mv.c +1
-0@@ -84,6 +85,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix) source =copy_pathspec (prefix, argv, argc,0 ); modes =xcalloc (argc,sizeof (enum update_mode )); dest_path =copy_pathspec (prefix, argv + argc,1 ,0 ); + submodule_gitfile =xcalloc (argc,sizeof (char *));
This is cmd_mv, and you're allocating argc number of char * pointers to submodule_gitfile. Why? Are you guaranteed that there are argc number of submodule_gitfiles?
builtin/mv.c int cmd_mv(int argc, const char **argv, const char *prefix) · line 128
Quoted 11 lines
builtin/mv.c +3
-0if (dest_path[0 ][0 ] =='\0' )/* special case: "." was normalized to "" */ @@ -119,8 +121,14 @@ int cmd_mv(int argc, const char **argv, const char *prefix)else if (src_is_dir) {int first =cache_name_pos (src, length);if (first >=0 ) { +struct strbuf submodule_dotgit =STRBUF_INIT ;if (!S_ISGITLINK (active_cache[first]->ce_mode))die (_ ("Huh? Directory %s is in index and no submodule?" ), src); +strbuf_addf (&submodule_dotgit,"%s/.git" , src); + submodule_gitfile[i] =read_gitfile (submodule_dotgit.buf);
What?! read_gitfile() returns the path to the git directory, if it is found. How are you assigning the path to a $GITDIR to a variable named submodule_gitfile?
builtin/mv.c int cmd_mv(int argc, const char **argv, const char *prefix) · line 130
Quoted 2 lines
builtin/mv.c +2
-0+if (submodule_gitfile[i]) + submodule_gitfile[i] =xstrdup (submodule_gitfile[i]);
Doesn't read as smoothly, but saves you an extra char *. Okay.
Quoted 7 lines
builtin/mv.c +1
-0+strbuf_release (&submodule_dotgit); }else {const char *src_w_slash =add_slash (src);int last, len_w_slash = length +1 ;@@ -215,9 +223,12 @@ int cmd_mv(int argc, const char **argv, const char *prefix)int pos;if (show_only || verbose)
builtin/mv.c int cmd_mv(int argc, const char **argv, const char *prefix) · line 231
Quoted 10 lines
builtin/mv.c +6
-3printf (_ ("Renaming %s to %s\n" ), src, dst); -if (!show_only && mode !=INDEX && -rename (src, dst) <0 && !ignore_errors) -die_errno (_ ("renaming '%s' failed" ), src); +if (!show_only && mode !=INDEX ) { +if (rename (src, dst) <0 && !ignore_errors) +die_errno (_ ("renaming '%s' failed" ), src); +if (submodule_gitfile[i]) +connect_work_tree_and_git_dir (dst, submodule_gitfile[i]); + }
Okay, scratch my previous comment about allocating argc char * pointers for submodule_gitfile(). Since your logic is in a loop that loops argc times, you really have no choice.
Quoted 30 lines
submodule.c +22
-0@@ -1001,3 +1001,67 @@ int merge_submodule(unsigned char result[20], const char *path,free (merges.objects);return 0 ; } + +/* Update gitfile and core.worktree setting to connect work tree and git dir */ +void connect_work_tree_and_git_dir (const char *work_tree,const char *git_dir) +{ +struct strbuf core_worktree_setting =STRBUF_INIT ; +struct strbuf configfile_name =STRBUF_INIT ; +struct strbuf gitfile_content =STRBUF_INIT ; +struct strbuf gitfile_name =STRBUF_INIT ; +const char *real_work_tree =real_path (work_tree); +const char *pathspec[] = { real_work_tree, git_dir,NULL }; +const char *max_prefix =common_prefix (pathspec); +FILE *fp; + +if (max_prefix) {/* skip common prefix */ +size_t max_prefix_len =strlen (max_prefix); + real_work_tree += max_prefix_len; + git_dir += max_prefix_len; + } + +/* +* Update gitfile +*/
submodule.c int merge_submodule(unsigned char result[20], const char *path, · line 1035
Quoted 10 lines
submodule.c +10
-0+strbuf_addstr (&gitfile_content,"gitdir: " ); +if (real_work_tree[0 ]) { +const char *s = real_work_tree; +do { +strbuf_addstr (&gitfile_content,"../" ); + s++; + }while ((s =strchr (s,'/' ))); + } +strbuf_addstr (&gitfile_content, git_dir); +strbuf_addch (&gitfile_content,'\n' );
Yuck. Just yuck. Why don't you just fopen the gitfile and write one line to it directly (ie. the absolute path of the worktree)? Why do you need a strbuf for this at all? Ah, absolute paths would break if you moved the entire superproject to a different location. This is reason #347 I don't like keeping GITDIRs in the superproject's .git/modules/. In any case, why don't you use relative_path() in path.c instead of reinventing the wheel?
submodule.c int merge_submodule(unsigned char result[20], const char *path, · line 1037
Quoted 1 line
submodule.c +1
-0+strbuf_addf (&gitfile_name,"%s/.git" , work_tree);
Why work_tree, not real_work_tree?
strbuf is an overkill for this: why not mkpath()?
submodule.c int merge_submodule(unsigned char result[20], const char *path, · line 1041
Quoted 4 lines
submodule.c +4
-0+ fp =fopen (gitfile_name.buf,"w" ); +if (!fp) +die (_ ("Could not create git link %s" ), gitfile_name.buf); +fprintf (fp,"%s" , gitfile_content.buf);
Not checking the return value of fprintf() for possible failure?
Quoted 5 lines
submodule.c +5
-0+fclose (fp); + +strbuf_release (&gitfile_content); +strbuf_release (&gitfile_name); +
submodule.c int merge_submodule(unsigned char result[20], const char *path, · line 1056
Quoted 10 lines
submodule.c +10
-0+/* +* Update core.worktree setting +*/ +if (git_dir[0 ]) { +const char *s = git_dir; +do { +strbuf_addstr (&core_worktree_setting,"../" ); + s++; + }while ((s =strchr (s,'/' ))); + }
Can't use relative_path()?
submodule.c int merge_submodule(unsigned char result[20], const char *path, · line 1059
Quoted 3 lines
submodule.c +3
-0+strbuf_addstr (&core_worktree_setting, real_work_tree); + +strbuf_addf (&configfile_name,"%s/config" , git_dir);
mkpath() to avoid the strbuf. If you have to use strbufs, why aren't you re-using a "pathbuf" strbuf?
Re: [PATCH v2 2/3] Teach mv to move submodules using a gitfile
Quoted 11 lines · Ramkumar Ramachandra
Quoted 4 lines · Jens Lehmann
When moving a submodule which uses a gitfile to point to the git directory stored in .git/modules/<name> of the superproject two changes must be made to make the submodule work: the .git file and the core.worktree setting must be adjusted to point from work tree to git directory and back.
Isn't it untrue that the git directory is stored in .git/modules/<name>: it is stored in .git/modules/path/to/module.
I thought the whole point of this complex scheme was to avoid name conflicts with submodules with the same name in other directories.
I think Jens is right on this one. There are three things the code needs to know about a submodule: its name, path and URL.
A canonical use case to think about is a project that builds an appliance:
* Its zero-th version only has the sources to the userspace for the appliance.
* The first version adds the Linux kernel as a submodule bound at kernel/, taking it from git://k.org/linux-2.6.git/.
* The second version adds a choice to build the appliance with the BSD kernel, and the project reorganizes the source tree to have Linux kernel at path linux/ and adds the bsd kernel at path bsd/.
* By the time the third version is released, the URL to the Linux has migrated to git://k.org/linux.git/; it is still logically the same (i.e. continuation of) the old 2.6.git repository [*1*].
We would want to make it possible to "git checkout" smoothly between these four versions. Switching from v1 to v0 would have to remove the submodule working tree at kernel/ but the user may want to switch back to v1 without having to re-download the kernel submodule, so the kernel/.git repository needs to be stashed away somewhere. Somewhere in $GIT_DIR of the superproject, but where? Switching from v1 to v2 would need to move kernel/ to linux/ and move kernel/.git to linux/.git. The design choice made long time ago (if you recall the collection of old threads I gave you some time ago, this is what was called "Steven's three-level thing") was to give a stable "logical" name for the Linux kernel component, so that no matter where in the working tree the version that happens to be at the tip of the current branch has it, we know where in the superproject's .git/modules it is found.
So at the second version when we move the submodule that used to be at kernel/ to linux/, we move the working tree of it, adjust the "path" of the submodule, but keep the name. And that name gives an identity to the submodule, and that is what is used as a key inside $GIT_DIR of the superproject to decide where the repository (together with its object store) of the submodule is stashed away.
[Footnote]
*1* I mentioned the URL thing only for completeness; it does not come into play in the "checkout" scenario, but when you start thinking about remote interactions, you need to be aware of how that value and the one copied to the configuration upon "submodule init" need to be managed. Which is a separate topic but is an integral part of the canonical example.
Ideally, a user who has followed along the life of this project should:
* first encounter git://k.org/linux-2.6.git/ in v1; "git submodule init" would copy it to her .git/config in the superproject.
* later notice that .gitmodules has git://k.org/linux.git/ location that she hasn't seen for the submodule, and is given a chance to have the URL entry updated in her .git/config.
This is becuase even when she checks out an older branch that has 2.6 in .gitmodules, "git submodule update" _should_ go to the new URL, not to the defunct 2.6 URL. We do the "copy initially", but do not do the latter "offer a chance to update when seeing a new one" (at least, not yet).
PATCH 3/3 Teach mv to update the path entry in .gitmodules for moved submodules
In reply to Jens LehmannCurrently using "git mv" on a submodule moves the submodule's work tree in that of the superproject. But the submodule's path setting in .gitmodules is left untouched, which is now inconsistent with the work tree and makes git commands that rely on the proper path -> name mapping (like status and diff) behave strangely.
Let "git mv" help here by not only moving the submodule's work tree but also updating the "submodule.<submodule name>.path" setting from the .gitmodules file and stage both. This doesn't happen when no .gitmodules file is found and only issues a warning when it doesn't have a section for this submodule. This is because the user might just use plain gitlinks without the .gitmodules file or has already updated the path setting by hand before issuing the "git mv" command (in which case the warning reminds him that mv would have done that for him). Only when .gitmodules is found and contains merge conflicts the mv command will fail and tell the user to resolve the conflict before trying again.
--- builtin/mv.c | 8 +++++++- submodule.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ submodule.h | 2 ++ t/t7001-mv.sh | 41 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 112 insertions(+), 1 deletion(-)
builtin/mv.c +7 -1
@@ -57,7 +57,7 @@ static struct lock_file lock_file;int cmd_mv (int argc,const char **argv,const char *prefix) { -int i, newfd; +int i, newfd, gitmodules_modified =0 ;int verbose =0 , show_only =0 , force =0 , ignore_errors =0 ;struct option builtin_mv_options[] = {OPT__VERBOSE (&verbose,N_ ("be verbose" )),
@@ -71,6 +71,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)struct stat st;struct string_list src_for_dst =STRING_LIST_INIT_NODUP ; +gitmodules_config ();git_config (git_default_config,NULL ); argc = parse_options(argc, argv, prefix, builtin_mv_options,
@@ -228,6 +229,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)die_errno (_ ("renaming '%s' failed" ), src);if (submodule_gitfile[i])connect_work_tree_and_git_dir (dst, submodule_gitfile[i]); +if (!update_path_in_gitmodules (src, dst)) + gitmodules_modified =1 ; }if (mode ==WORKING_DIRECTORY )
@@ -239,6 +242,9 @@ int cmd_mv(int argc, const char **argv, const char *prefix)rename_cache_entry_at (pos, dst); } +if (gitmodules_modified) +stage_updated_gitmodules (); +if (active_cache_changed) {if (write_cache (newfd, active_cache, active_nr) ||commit_locked_index (&lock_file))
submodule.c +62 -0
74 lines not on this page · raw message
submodule.h +2 -0
@@ -11,6 +11,8 @@ enum { RECURSE_SUBMODULES_ON = 2 }; +int update_path_in_gitmodules (const char *oldpath,const char *newpath); +void stage_updated_gitmodules (void );void set_diffopt_flags_from_submodule_config (struct diff_options *diffopt,const char *path);int submodule_config (const char *var,const char *value,void *cb);
t/t7001-mv.sh +41 -0
47 lines not on this page · raw message
Signature · 1 line
1.8.2.377.g1bdb7d0
Re: [PATCH/RFC 3/3] Teach mv to update the path entry in .gitmodules for moved submodules
Quoted 11 lines · Jens Lehmann
Currently using "git mv" on a submodule moves the submodule's work tree in that of the superproject. But the submodule's path setting in .gitmodules is left untouched, which is now inconsistent with the work tree and makes git commands that rely on the proper path -> name mapping (like status and diff) behave strangely.
Let "git mv" help here by not only moving the submodule's work tree but also updating the "submodule.<submodule name>.path" setting from the .gitmodules file and stage both. This doesn't happen when no .gitmodules file is found and only issues a warning when it doesn't have a section for this submodule.
Should it happen when the user has other changes to .gitmodules that hasn't been updated to the index?
As this is done in the same final "per-path" loop as adjusting the gitfile links, the worry I expressed in an earlier message about "git mv a/ b/" when "a/x" is a submodule turns out to be unfounded, which is good ;-)
Re: [PATCH/RFC 3/3] Teach mv to update the path entry in .gitmodules for moved submodules
In reply to Jens LehmannQuoted 2 lines · Jens Lehmann
Currently using "git mv" on a submodule moves the submodule's work tree in that of the superproject.
It's not "Currently": it's the result of your last two patches. The wording is very unclear. Perhaps: As a result of the last two patches in this series, a 'git mv' moves the submodule directory to a new path in the superproject's worktree, and updates the cache entry in the index appropriately.
Quoted 4 lines
But the submodule's path setting in .gitmodules
is left untouched, which is now inconsistent with the work tree and makes git commands that rely on the proper path -> name mapping (like status and diff) behave strangely.
I'm frankly a little surprised that your previous two patches didn't break any tests. That's probably because we don't have enough tests to exercise git-core with submodules. I'm not saying that it's a bad thing though: submodules are still immature, and tight tests would get in the way of new patches.
Quoted 8 lines
Let "git mv" help here by not only moving the submodule's work tree but also updating the "submodule.<submodule name>.path" setting from the .gitmodules file and stage both.
This doesn't happen when no .gitmodules
file is found and only issues a warning when it doesn't have a section for this submodule. This is because the user might just use plain gitlinks without the .gitmodules file
How will git-core work without a .gitmodules? Shouldn't we create a fresh .gitmodules here?
Quoted 3 lines
or has already updated the path setting by
hand before issuing the "git mv" command (in which case the warning reminds him that mv would have done that for him).
Shouldn't these two cases issue different warnings?
Besides, why is this explanation missing in your rm patch's commit message?
builtin/mv.c int cmd_mv(int argc, const char **argv, const char *prefix) · line 247
Quoted 11 lines
builtin/mv.c +3
-0@@ -239,6 +242,9 @@ int cmd_mv(int argc, const char **argv, const char *prefix)rename_cache_entry_at (pos, dst); } +if (gitmodules_modified) +stage_updated_gitmodules (); +
I'm unhappy with this, but we I'll have to live with my dissatisfaction until we get rid of ".gitmodules" (if that ever happens). mv has a clear task: it should perform a filesystem mv, manipulate the index, and write out the changed index. Adding an unrelated file to the index has nothing to do with its primary task.
submodule.c
Quoted 4 lines
submodule.c +0
-0
All my comments in the rm review apply here too, so I won't repeat myself.
submodule.c line 13
Quoted 5 lines
submodule.c +1
-0@@ -10,6 +10,7 @@#include "string-list.h" #include "sha1-array.h" #include "argv-array.h" +#include "blob.h"
Interesting. Let's see why you need blob.h.
Quoted 7 lines
submodule.c +1
-0static struct string_list config_name_for_path;static struct string_list config_fetch_recurse_submodules_for_name;@@ -30,6 +31,67 @@ static struct sha1_array ref_tips_after_fetch;*/ static int gitmodules_is_unmerged; +/*
submodule.c static struct sha1_array ref_tips_after_fetch; · line 44
Quoted 10 lines
submodule.c +10
-0+* Try to update the "path" entry in the "submodule.<name>" section of the +* .gitmodules file. +*/ +int update_path_in_gitmodules (const char *oldpath,const char *newpath) +{ +struct strbuf entry =STRBUF_INIT ; +struct string_list_item *path_option; + +if (!file_exists (".gitmodules" ))/* Do nothing whithout .gitmodules */ +return -1 ;
s/whithout/without. Why are you not returning an error()? Don't you want to tell the user that no ".gitmodules" was found?
submodule.c static struct sha1_array ref_tips_after_fetch; · line 66
Quoted 1 line
submodule.c +1
-0+void stage_updated_gitmodules (void )
void return? Don't you want the caller to know whether we were successful or not?
Why does the name contain "updated"? Why does the function care if I updated the .gitmodules or not? It's just staging .gitmodules, as it is on the filesystem.
submodule.c static struct sha1_array ref_tips_after_fetch; · line 74
Quoted 8 lines
submodule.c +8
-0+{ +struct strbuf buf =STRBUF_INIT ; +struct stat st; +int pos; +struct cache_entry *ce; +int namelen =strlen (".gitmodules" ); + + pos =cache_name_pos (".gitmodules" ,strlen (".gitmodules" ));
And you forgot about namelen just like that.
submodule.c static struct sha1_array ref_tips_after_fetch; · line 78
Quoted 4 lines
submodule.c +4
-0+if (pos <0 ) { +warning (_ ("could not find .gitmodules in index" )); +return ; + }
What could this mean? I might have an untracked .gitmodules file in my worktree, or I might not have a .gitmodules file at all. Since you're already checking the latter case in the previous function, can't you persist the result, and return a more sensible error?
submodule.c static struct sha1_array ref_tips_after_fetch; · line 82
Quoted 4 lines
submodule.c +4
-0+ ce = active_cache[pos]; + ce->ce_flags = namelen; +if (strbuf_read_file (&buf,".gitmodules" ,0 ) <0 ) +die (_ ("reading updated .gitmodules failed" ));
You're reading it because? What does "_updated_ .gitmodules" mean here?
submodule.c static struct sha1_array ref_tips_after_fetch; · line 86
Quoted 4 lines
submodule.c +4
-0+if (lstat (".gitmodules" , &st) <0 ) +die_errno (_ ("unable to stat updated .gitmodules" )); +fill_stat_cache_info (ce, &st); + ce->ce_mode =ce_mode_from_stat (ce, st.st_mode);
I don't understand why you're taking the pains to fill out a cache_entry.
submodule.c static struct sha1_array ref_tips_after_fetch; · line 88
Quoted 2 lines
submodule.c +2
-0+if (remove_file_from_cache (".gitmodules" ) <0 ) +die (_ ("unable to remove .gitmodules from index" ));
You already have pos, so why not just remove_cache_entry_at()?
submodule.c static struct sha1_array ref_tips_after_fetch; · line 90
Quoted 2 lines
submodule.c +2
-0+if (write_sha1_file (buf.buf, buf.len, blob_type, ce->sha1)) +die (_ ("adding updated .gitmodules failed" ));
Ah, you need blob.h for blob_type.
Wait, why are you just reading and writing .gitmodules? What changed?
And why are you manually writing a blob to the object store? Doesn't
git-core already do this if you just add it to the index? See the S_IFLNK case in index_path().
What happens if there's a race? Shouldn't you be locking .gitmodules before updating it, like we do with the index and just about everything else?
submodule.c static struct sha1_array ref_tips_after_fetch; · line 92
Quoted 2 lines
submodule.c +2
-0+if (add_cache_entry (ce,ADD_CACHE_OK_TO_ADD |ADD_CACHE_OK_TO_REPLACE )) +die (_ ("staging updated .gitmodules failed" ));
This is all you need: it calls index_path() and writes your blob object to the database.