Teach mv to move submodules
v3 cover Teach mv to move submodules
Here is my third iteration of this series.
Changes to v2 are:
- I resolved the conflict with Duy's pathspec series by replacing the use of common_prefix() with relative_path().
- I separated the functions checking for modified unstaged .gitmodules and staging the changes to that file into another commit, as they are used by both mv and rm.
- mv and rm now die with the message "Please, stage your changes to .gitmodules or stash them to proceed" instead of changing and staging a .gitmodules file containing other unstaged modifications.
- Man pages for mv and rm are updated to tell the user what they do with the gitlink and the .gitmodules file in case of submodules.
- Minor changes according to the last review (typos and a bit more efficient coding).
This series applies cleanly on current pu (and I also ran t3600 and t7001 manually to make sure I don't hit the silent breakage my last series showed when I ran the whole test suite).
Jens Lehmann (5):
Teach mv to move submodules together with their work trees Teach mv to move submodules using a gitfile
submodule.c: add .gitmodules staging helper functions Teach mv to update the path entry in .gitmodules for moved submodules
rm: delete .gitmodules entry of submodules removed from the work tree
Documentation/git-mv.txt | 10 ++- Documentation/git-rm.txt | 8 ++- builtin/mv.c | 126 ++++++++++++++++++++++---------------- builtin/rm.c | 19 +++++- submodule.c | 147 +++++++++++++++++++++++++++++++++++++++++++++ submodule.h | 5 ++ t/t3600-rm.sh | 98 ++++++++++++++++++++++++++++-- t/t7001-mv.sh | 128 +++++++++++++++++++++++++++++++++++++++ t/t7400-submodule-basic.sh | 14 ++--- t/t7610-mergetool.sh | 6 +- 10 files changed, 484 insertions(+), 77 deletions(-)
Signature · 1 line
1.8.4.rc0.199.g7079aac
PATCH v3 1/5 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
@@ -118,55 +118,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
@@ -259,4 +259,38 @@ test_expect_success SYMLINKS 'check moved 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.4.rc0.199.g7079aac
PATCH v3 2/5 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.
Extend the man page to inform the user about that feature (and while at it change the description to not talk about a script anymore, as mv is a builtin for quite some time now).
--- Documentation/git-mv.txt | 8 +++++++- builtin/mv.c | 19 +++++++++++++++---- submodule.c | 31 +++++++++++++++++++++++++++++++ submodule.h | 1 + t/t7001-mv.sh | 19 +++++++++++++++++++ 5 files changed, 73 insertions(+), 5 deletions(-)
Documentation/git-mv.txt +7 -1
@@ -13,7 +13,7 @@ SYNOPSIS DESCRIPTION ----------- -This script is used to move or rename a file, directory or symlink. +Move or rename a file, directory or symlink. git mv [-v] [-f] [-n] [-k] <source> <destination> git mv [-v] [-f] [-n] [-k] <source> ... <destination directory>
@@ -44,6 +44,12 @@ OPTIONS --verbose:: Report the names of files as they are moved. +SUBMODULES +---------- +Moving a submodule using a gitfile (which means they were cloned +with a Git version 1.7.8 or newer) will update the gitfile and +core.worktree setting to make the submodule work in the new location. + GIT --- Part of the linkgit:git[1] suite
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>" ),
@@ -66,7 +67,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 ;
@@ -85,6 +86,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix) source =internal_copy_pathspec (prefix, argv, argc,0 ); modes =xcalloc (argc,sizeof (enum update_mode )); dest_path =internal_copy_pathspec (prefix, argv + argc,1 ,0 ); + submodule_gitfile =xcalloc (argc,sizeof (char *));if (dest_path[0 ][0 ] =='\0' )/* special case: "." was normalized to "" */
@@ -120,8 +122,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 ;
@@ -216,9 +224,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 +31 -0
@@ -1004,3 +1004,34 @@ 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 file_name =STRBUF_INIT ; +struct strbuf rel_path =STRBUF_INIT ; +const char *real_work_tree =xstrdup (real_path (work_tree)); +FILE *fp; + +/* Update gitfile */ +strbuf_addf (&file_name,"%s/.git" , work_tree); + fp =fopen (file_name.buf,"w" ); +if (!fp) +die (_ ("Could not create git link %s" ), file_name.buf); +fprintf (fp,"gitdir: %s\n" ,relative_path (git_dir, real_work_tree, + &rel_path)); +fclose (fp); + +/* Update core.worktree setting */ +strbuf_reset (&file_name); +strbuf_addf (&file_name,"%s/config" , git_dir); +if (git_config_set_in_file (file_name.buf,"core.worktree" , +relative_path (real_work_tree, git_dir, + &rel_path))) +die (_ ("Could not set core.worktree in %s" ), + file_name.buf); + +strbuf_release (&file_name); +strbuf_release (&rel_path); +free ((void *)real_work_tree); +}
submodule.h +1 -0
@@ -36,5 +36,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
@@ -293,4 +293,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.4.rc0.199.g7079aac
Re: [PATCH v3 2/5] Teach mv to move submodules using a gitfile
Quoted 20 lines · Jens Lehmann
submodule.c +20
-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 file_name =STRBUF_INIT ; +struct strbuf rel_path =STRBUF_INIT ; +const char *real_work_tree =xstrdup (real_path (work_tree)); +FILE *fp; + +/* Update gitfile */ +strbuf_addf (&file_name,"%s/.git" , work_tree); + fp =fopen (file_name.buf,"w" ); +if (!fp) +die (_ ("Could not create git link %s" ), file_name.buf); +fprintf (fp,"gitdir: %s\n" ,relative_path (git_dir, real_work_tree, + &rel_path)); +fclose (fp); + +/* Update core.worktree setting */ +strbuf_reset (&file_name); +strbuf_addf (&file_name,"%s/config" , git_dir);
submodule.c int merge_submodule(unsigned char result[20], const char *path, · line 1037
Quoted 10 lines
submodule.c +10
-0+if (git_config_set_in_file (file_name.buf,"core.worktree" , +relative_path (real_work_tree, git_dir, + &rel_path))) +die (_ ("Could not set core.worktree in %s" ), + file_name.buf); + +strbuf_release (&file_name); +strbuf_release (&rel_path); +free ((void *)real_work_tree); +}
Would it make sense to return an int here and do the dying in builtin/mv.c instead? Again thinking of "libgit" and the die_errno for non-submodule errors are in mv.c and not in rename().
If that's the case the same applies for stage_updated_gitmodules() and update_path_in_gitmodules().
update_path_in_gitmodules() also needs a strbuf_release() if you haven't already found it.
Signature · 5 lines
Med vänliga hälsningar Fredrik Gustafsson tel: 0733-608274 e-post: [email protected]
PATCH v3 3/5 submodule.c: add .gitmodules staging helper functions
In reply to Jens LehmannAdd the new is_staging_gitmodules_ok() and stage_updated_gitmodules() functions to submodule.c. The first makes it possible for call sites to see if the .gitmodules file did contain any unstaged modifications they would accidentally stage in addition to those they intend to stage themselves. The second function stages all modifications to the .gitmodules file, both will be used by subsequent patches for the mv and rm commands.
--- submodule.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ submodule.h | 2 ++ 2 files changed, 53 insertions(+)
submodule.c +51 -0
@@ -10,6 +10,7 @@#include "string-list.h" #include "sha1-array.h" #include "argv-array.h" +#include "blob.h" static struct string_list config_name_for_path;static struct string_list config_fetch_recurse_submodules_for_name;
@@ -30,6 +31,51 @@ static struct sha1_array ref_tips_after_fetch;*/ static int gitmodules_is_unmerged; +/* +* This flag is set if the .gitmodules file had unstaged modifications on +* startup. This must be checked before allowing modifications to the +* .gitmodules file with the intention to stage them later, because when +* continuing we would stage the modifications the user didn't stage herself +* too. That might change in a future version when we learn to stage the +* changes we do ourselves without staging any previous modifications. +*/ +static int gitmodules_is_modified; + + +int is_staging_gitmodules_ok () +{ +return !gitmodules_is_modified; +} + +void stage_updated_gitmodules (void ) +{ +struct strbuf buf =STRBUF_INIT ; +struct stat st; +int pos; +struct cache_entry *ce; +int namelen =strlen (".gitmodules" ); + + pos =cache_name_pos (".gitmodules" , namelen); +if (pos <0 ) { +warning (_ ("could not find .gitmodules in index" )); +return ; + } + ce = active_cache[pos]; + ce->ce_flags = namelen; +if (strbuf_read_file (&buf,".gitmodules" ,0 ) <0 ) +die (_ ("reading updated .gitmodules failed" )); +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); +if (remove_cache_entry_at (pos) <0 ) +die (_ ("unable to remove .gitmodules from index" )); +if (write_sha1_file (buf.buf, buf.len, blob_type, ce->sha1)) +die (_ ("adding updated .gitmodules failed" )); +if (add_cache_entry (ce,ADD_CACHE_OK_TO_ADD |ADD_CACHE_OK_TO_REPLACE )) +die (_ ("staging updated .gitmodules failed" )); +} +static int add_submodule_odb (const char *path) {struct strbuf objects_directory =STRBUF_INIT ;
@@ -116,6 +162,11 @@ void gitmodules_config(void) !memcmp(ce->name, ".gitmodules", 11)) gitmodules_is_unmerged =1 ; } + }else if (pos < active_nr) { +struct stat st; +if (lstat (".gitmodules" , &st) ==0 && +ce_match_stat (active_cache[pos], &st,0 ) &DATA_CHANGED ) + gitmodules_is_modified =1 ; }if (!gitmodules_is_unmerged)
submodule.h +2 -0
@@ -11,6 +11,8 @@ enum { RECURSE_SUBMODULES_ON = 2 }; +int is_staging_gitmodules_ok (); +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);
Signature · 1 line
1.8.4.rc0.199.g7079aac
Re: [PATCH v3 3/5] submodule.c: add .gitmodules staging helper functions
Quoted 1 line · Jens Lehmann
+int
is_staging_gitmodules_ok()
Will tweak this to:
int is_staging_gitmodules_ok(void)
and fix this as well:
submodule.h enum { · line 15
Quoted 2 lines
submodule.h +2
-0+int is_staging_gitmodules_ok (); +void stage_updated_gitmodules (void );
before queuing.
Thanks.
Re: [PATCH v3 3/5] submodule.c: add .gitmodules staging helper functions
Am 30.07.2013 23:37, schrieb Junio C Hamano:
Quoted 8 lines
Quoted 1 line · Jens Lehmann
+int
is_staging_gitmodules_ok()Will tweak this to:
int
is_staging_gitmodules_ok(void)
Quoted 7 lines
and fix this as well:
submodule.h enum { · line 15
Quoted 2 lines
submodule.h +2
-0+int is_staging_gitmodules_ok (); +void stage_updated_gitmodules (void );before queuing.
Thanks. As Frederik already noticed, this series is missing at least one strbuf_release(). Will fix both in v4.
PATCH v3 4/5 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.
Also extend the man page to inform the user about this new feature.
--- Documentation/git-mv.txt | 2 ++ builtin/mv.c | 10 ++++++- submodule.c | 33 +++++++++++++++++++++ submodule.h | 1 + t/t7001-mv.sh | 75 ++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 120 insertions(+), 1 deletion(-)
Documentation/git-mv.txt +2 -0
@@ -49,6 +49,8 @@ SUBMODULES Moving a submodule using a gitfile (which means they were cloned with a Git version 1.7.8 or newer) will update the gitfile and core.worktree setting to make the submodule work in the new location. +It also will attempt to update the submodule.<name>.path setting in +the linkgit:gitmodules[5] file and stage that file (unless -n is used). GIT ---
builtin/mv.c +9 -1
@@ -58,7 +58,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" )),
@@ -72,6 +72,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,
@@ -125,6 +126,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)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); +if (!is_staging_gitmodules_ok ()) +die (_ ("Please, stage your changes to .gitmodules or stash them to proceed" ));strbuf_addf (&submodule_dotgit,"%s/.git" , src); submodule_gitfile[i] =read_gitfile (submodule_dotgit.buf);if (submodule_gitfile[i])
@@ -229,6 +232,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 )
@@ -240,6 +245,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 +33 -0
@@ -47,6 +47,39 @@ int is_staging_gitmodules_ok()return !gitmodules_is_modified; } +/* +* Try to update the "path" entry in the "submodule.<name>" section of the +* .gitmodules file. Return 0 only if a .gitmodules file was found, a section +* with the correct path=<oldpath> setting was found and we could update it. +*/ +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 without .gitmodules */ +return -1 ; + +if (gitmodules_is_unmerged) +die (_ ("Cannot change unmerged .gitmodules, resolve merge conflicts first" )); + + path_option =unsorted_string_list_lookup (&config_name_for_path, oldpath); +if (!path_option) { +warning (_ ("Could not find section in .gitmodules where path=%s" ), oldpath); +return -1 ; + } +strbuf_addstr (&entry,"submodule." ); +strbuf_addstr (&entry, path_option->util); +strbuf_addstr (&entry,".path" ); +if (git_config_set_in_file (".gitmodules" , entry.buf, newpath) <0 ) { +/* Maybe the user already did that, don't error out here */ +warning (_ ("Could not update .gitmodules entry %s" ), entry.buf); +return -1 ; + } +strbuf_release (&entry); +return 0 ; +} +void stage_updated_gitmodules (void ) {struct strbuf buf =STRBUF_INIT ;
submodule.h +1 -0
@@ -12,6 +12,7 @@ enum { };int is_staging_gitmodules_ok (); +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);
t/t7001-mv.sh +75 -0
83 lines not on this page · raw message
Signature · 1 line
1.8.4.rc0.199.g7079aac
Re: [PATCH v4 4/5] Teach mv to update the path entry in .gitmodules for moved submodules
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. 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.
Also extend the man page to inform the user about this new feature.
---
This version fixes the missing strbuf_release() noticed by Frederik.
Documentation/git-mv.txt | 2 ++ builtin/mv.c | 10 ++++++- submodule.c | 34 ++++++++++++++++++++++ submodule.h | 1 + t/t7001-mv.sh | 75 ++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 121 insertions(+), 1 deletion(-)
Documentation/git-mv.txt +2 -0
@@ -49,6 +49,8 @@ SUBMODULES Moving a submodule using a gitfile (which means they were cloned with a Git version 1.7.8 or newer) will update the gitfile and core.worktree setting to make the submodule work in the new location. +It also will attempt to update the submodule.<name>.path setting in +the linkgit:gitmodules[5] file and stage that file (unless -n is used). GIT ---
builtin/mv.c +9 -1
@@ -58,7 +58,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" )),
@@ -72,6 +72,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,
@@ -125,6 +126,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)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); +if (!is_staging_gitmodules_ok ()) +die (_ ("Please, stage your changes to .gitmodules or stash them to proceed" ));strbuf_addf (&submodule_dotgit,"%s/.git" , src); submodule_gitfile[i] =read_gitfile (submodule_dotgit.buf);if (submodule_gitfile[i])
@@ -229,6 +232,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 )
@@ -240,6 +245,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 +34 -0
@@ -47,6 +47,40 @@ int is_staging_gitmodules_ok(void)return !gitmodules_is_modified; } +/* +* Try to update the "path" entry in the "submodule.<name>" section of the +* .gitmodules file. Return 0 only if a .gitmodules file was found, a section +* with the correct path=<oldpath> setting was found and we could update it. +*/ +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 without .gitmodules */ +return -1 ; + +if (gitmodules_is_unmerged) +die (_ ("Cannot change unmerged .gitmodules, resolve merge conflicts first" )); + + path_option =unsorted_string_list_lookup (&config_name_for_path, oldpath); +if (!path_option) { +warning (_ ("Could not find section in .gitmodules where path=%s" ), oldpath); +return -1 ; + } +strbuf_addstr (&entry,"submodule." ); +strbuf_addstr (&entry, path_option->util); +strbuf_addstr (&entry,".path" ); +if (git_config_set_in_file (".gitmodules" , entry.buf, newpath) <0 ) { +/* Maybe the user already did that, don't error out here */ +warning (_ ("Could not update .gitmodules entry %s" ), entry.buf); +strbuf_release (&entry); +return -1 ; + } +strbuf_release (&entry); +return 0 ; +} +void stage_updated_gitmodules (void ) {struct strbuf buf =STRBUF_INIT ;
submodule.h +1 -0
@@ -12,6 +12,7 @@ enum { };int is_staging_gitmodules_ok (void ); +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);
t/t7001-mv.sh +75 -0
83 lines not on this page · raw message
Signature · 1 line
1.8.4.rc0.199.g3f237fc
PATCH v3 5/5 rm: delete .gitmodules entry of submodules removed from the work tree
In reply to Jens LehmannCurrently using "git rm" on a submodule removes the submodule's work tree from that of the superproject and the gitlink from the index. But the submodule's section in .gitmodules is left untouched, which is a leftover of the now removed submodule and might irritate users (as opposed to the setting in .git/config, this must stay as a reminder that the user showed interest in this submodule so it will be repopulated later when an older commit is checked out).
Let "git rm" help the user by not only removing the submodule from the work tree but by also removing the "submodule.<submodule name>" section from the .gitmodules file and stage both. This doesn't happen when the "--cached" option is used, as it would modify the work tree. This also silently does nothing 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 removed the section by hand before issuing the "git rm" command (in which case the warning reminds him that rm would have done that for him). Only when .gitmodules is found and contains merge conflicts the rm command will fail and tell the user to resolve the conflict before trying again.
Also extend the man page to inform the user about this new feature. While at it promote the submodule sub-section to a chapter as it made not much sense under "REMOVING FILES THAT HAVE DISAPPEARED FROM THE FILESYSTEM".
In t7610 three uses of "git rm submod" had to be replaced with "git rm --cached submod" because that test expects .gitmodules and the work tree to stay untouched. Also in t7400 the tests for the remaining settings in the .gitmodules file had to be changed to assert that these settings are missing.
--- Documentation/git-rm.txt | 8 ++-- builtin/rm.c | 19 +++++++-- submodule.c | 32 +++++++++++++++ submodule.h | 1 + t/t3600-rm.sh | 98 +++++++++++++++++++++++++++++++++++++++++++--- t/t7400-submodule-basic.sh | 14 ++----- t/t7610-mergetool.sh | 6 +-- 7 files changed, 153 insertions(+), 25 deletions(-)
Documentation/git-rm.txt +5 -3
@@ -134,14 +134,16 @@ use the following command: git diff --name-only --diff-filter=D -z | xargs -0 git rm --cached ---------------- -Submodules -~~~~~~~~~~ +SUBMODULES +---------- Only submodules using a gitfile (which means they were cloned with a Git version 1.7.8 or newer) will be removed from the work tree, as their repository lives inside the .git directory of the superproject. If a submodule (or one of those nested inside it) still uses a .git directory, `git rm` will fail - no matter if forced -or not - to protect the submodule's history. +or not - to protect the submodule's history. If it exists the +submodule.<name> section in the linkgit:gitmodules[5] file will also +be removed and that file will be staged (unless --cached or -n are used). A submodule is considered up-to-date when the HEAD is the same as recorded in the index, no tracked files are modified and no untracked
builtin/rm.c +16 -3
@@ -283,6 +283,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)struct pathspec pathspec;char *seen; +gitmodules_config ();git_config (git_default_config,NULL ); argc = parse_options(argc, argv, prefix, builtin_rm_options,
@@ -324,7 +325,10 @@ int cmd_rm(int argc, const char **argv, const char *prefix)continue ;ALLOC_GROW (list.entry, list.nr +1 , list.alloc); list.entry[list.nr].name = ce->name; - list.entry[list.nr++].is_submodule =S_ISGITLINK (ce->ce_mode); + list.entry[list.nr].is_submodule =S_ISGITLINK (ce->ce_mode); +if (list.entry[list.nr++].is_submodule && + !is_staging_gitmodules_ok ()) +die (_ ("Please, stage your changes to .gitmodules or stash them to proceed" )); }if (pathspec.nr) {
@@ -396,13 +400,15 @@ int cmd_rm(int argc, const char **argv, const char *prefix)* in the middle) */ if (!index_only) { -int removed =0 ; +int removed =0 , gitmodules_modified =0 ;for (i =0 ; i < list.nr; i++) {const char *path = list.entry[i].name;if (list.entry[i].is_submodule) {if (is_empty_dir (path)) {if (!rmdir (path)) { removed =1 ; +if (!remove_path_from_gitmodules (path)) + gitmodules_modified =1 ;continue ; } }else {
@@ -410,9 +416,14 @@ int cmd_rm(int argc, const char **argv, const char *prefix)strbuf_addstr (&buf, path);if (!remove_dir_recursively (&buf,0 )) { removed =1 ; +if (!remove_path_from_gitmodules (path)) + gitmodules_modified =1 ;strbuf_release (&buf);continue ; - } + }else if (!file_exists (path)) +/* Submodule was removed by user */ +if (!remove_path_from_gitmodules (path)) + gitmodules_modified =1 ;strbuf_release (&buf);/* Fallthrough and let remove_path() fail. */ }
@@ -424,6 +435,8 @@ int cmd_rm(int argc, const char **argv, const char *prefix)if (!removed)die_errno ("git rm: '%s'" , path); } +if (gitmodules_modified) +stage_updated_gitmodules (); }if (active_cache_changed) {
submodule.c +32 -0
@@ -80,6 +80,38 @@ int update_path_in_gitmodules(const char *oldpath, const char *newpath)return 0 ; } +/* +* Try to remove the "submodule.<name>" section from .gitmodules where the given +* path is configured. Return 0 only if a .gitmodules file was found, a section +* with the correct path=<path> setting was found and we could remove it. +*/ +int remove_path_from_gitmodules (const char *path) +{ +struct strbuf sect =STRBUF_INIT ; +struct string_list_item *path_option; + +if (!file_exists (".gitmodules" ))/* Do nothing without .gitmodules */ +return -1 ; + +if (gitmodules_is_unmerged) +die (_ ("Cannot change unmerged .gitmodules, resolve merge conflicts first" )); + + path_option =unsorted_string_list_lookup (&config_name_for_path, path); +if (!path_option) { +warning (_ ("Could not find section in .gitmodules where path=%s" ), path); +return -1 ; + } +strbuf_addstr (§,"submodule." ); +strbuf_addstr (§, path_option->util); +if (git_config_rename_section_in_file (".gitmodules" , sect.buf,NULL ) <0 ) { +/* Maybe the user already did that, don't error out here */ +warning (_ ("Could not remove .gitmodules entry for %s" ), path); +return -1 ; + } +strbuf_release (§); +return 0 ; +} +void stage_updated_gitmodules (void ) {struct strbuf buf =STRBUF_INIT ;
submodule.h +1 -0
@@ -13,6 +13,7 @@ enum {int is_staging_gitmodules_ok ();int update_path_in_gitmodules (const char *oldpath,const char *newpath); +int remove_path_from_gitmodules (const char *path);void stage_updated_gitmodules (void );void set_diffopt_flags_from_submodule_config (struct diff_options *diffopt,const char *path);
t/t3600-rm.sh +92 -6
147 lines not on this page · raw message
t/t7400-submodule-basic.sh +4 -10
@@ -773,13 +773,11 @@ test_expect_success 'submodule add --name allows to replace a submodule with ano test_cmp expect .git ) && echo "repo" >expect && - git config -f .gitmodules submodule.repo.path >actual && - test_cmp expect actual && + test_must_fail git config -f .gitmodules submodule.repo.path && git config -f .gitmodules submodule.repo_new.path >actual && test_cmp expect actual&& echo "$submodurl/repo" >expect && - git config -f .gitmodules submodule.repo.url >actual && - test_cmp expect actual && + test_must_fail git config -f .gitmodules submodule.repo.url && echo "$submodurl/bare.git" >expect && git config -f .gitmodules submodule.repo_new.url >actual && test_cmp expect actual &&
@@ -799,12 +797,8 @@ test_expect_success 'submodule add with an existing name fails unless forced' ' git rm repo && test_must_fail git submodule add -q --name repo_new "$submodurl/repo.git" repo && test ! -d repo && - echo "repo" >expect && - git config -f .gitmodules submodule.repo_new.path >actual && - test_cmp expect actual&& - echo "$submodurl/bare.git" >expect && - git config -f .gitmodules submodule.repo_new.url >actual && - test_cmp expect actual && + test_must_fail git config -f .gitmodules submodule.repo_new.path && + test_must_fail git config -f .gitmodules submodule.repo_new.url && echo "$submodurl/bare.git" >expect && git config submodule.repo_new.url >actual && test_cmp expect actual &&
t/t7610-mergetool.sh +3 -3
@@ -253,7 +253,7 @@ test_expect_success 'deleted vs modified submodule' ' git checkout -b test6 branch1 && git submodule update -N && mv submod submod-movedaside && - git rm submod && + git rm --cached submod && git commit -m "Submodule deleted from branch" && git checkout -b test6.a test6 && test_must_fail git merge master &&
@@ -322,7 +322,7 @@ test_expect_success 'file vs modified submodule' ' git checkout -b test7 branch1 && git submodule update -N && mv submod submod-movedaside && - git rm submod && + git rm --cached submod && echo not a submodule >submod && git add submod && git commit -m "Submodule path becomes file" &&
@@ -453,7 +453,7 @@ test_expect_success 'submodule in subdirectory' ' test_expect_success 'directory vs modified submodule' ' git checkout -b test11 branch1 && mv submod submod-movedaside && - git rm submod && + git rm --cached submod && mkdir submod && echo not a submodule >submod/file16 && git add submod/file16 &&
Signature · 1 line
1.8.4.rc0.199.g7079aac
Re: [PATCH v3 5/5] rm: delete .gitmodules entry of submodules removed from the work tree
Quoted 21 lines · Jens Lehmann
submodule.c +21
-0+/* +* Try to remove the "submodule.<name>" section from .gitmodules where the given +* path is configured. Return 0 only if a .gitmodules file was found, a section +* with the correct path=<path> setting was found and we could remove it. +*/ +int remove_path_from_gitmodules (const char *path) +{ +struct strbuf sect =STRBUF_INIT ; +struct string_list_item *path_option; + +if (!file_exists (".gitmodules" ))/* Do nothing without .gitmodules */ +return -1 ; + +if (gitmodules_is_unmerged) +die (_ ("Cannot change unmerged .gitmodules, resolve merge conflicts first" )); + + path_option =unsorted_string_list_lookup (&config_name_for_path, path); +if (!path_option) { +warning (_ ("Could not find section in .gitmodules where path=%s" ), path); +return -1 ; + }
submodule.c int update_path_in_gitmodules(const char *oldpath, const char *newpath) · line 113
Quoted 10 lines
submodule.c +10
-0+strbuf_addstr (§,"submodule." ); +strbuf_addstr (§, path_option->util); +if (git_config_rename_section_in_file (".gitmodules" , sect.buf,NULL ) <0 ) { +/* Maybe the user already did that, don't error out here */ +warning (_ ("Could not remove .gitmodules entry for %s" ), path); +return -1 ; + } +strbuf_release (§); +return 0 ; +}
This question applies for this function and a few more functions in this patch that has the same characteristics.
If we're in a state when we need to return non-zero, we don't do any cleaning (that is strbuf_release()). Since this file is in the part called libgit AFAIK, shouldn't we always clean after us?
Would it make sense to have different return values for different errors?
I do like the comments above the function, more functions (at least non-static ones) should follow this good style IMHO.
Signature · 5 lines
Med vänliga hälsningar Fredrik Gustafsson tel: 0733-608274 e-post: [email protected]
Re: [PATCH v3 5/5] rm: delete .gitmodules entry of submodules removed from the work tree
Am 30.07.2013 22:15, schrieb Fredrik Gustafsson:
Quoted 36 lines · Jens Lehmann
Quoted 21 lines
submodule.c +21
-0+/* +* Try to remove the "submodule.<name>" section from .gitmodules where the given +* path is configured. Return 0 only if a .gitmodules file was found, a section +* with the correct path=<path> setting was found and we could remove it. +*/ +int remove_path_from_gitmodules (const char *path) +{ +struct strbuf sect =STRBUF_INIT ; +struct string_list_item *path_option; + +if (!file_exists (".gitmodules" ))/* Do nothing without .gitmodules */ +return -1 ; + +if (gitmodules_is_unmerged) +die (_ ("Cannot change unmerged .gitmodules, resolve merge conflicts first" )); + + path_option =unsorted_string_list_lookup (&config_name_for_path, path); +if (!path_option) { +warning (_ ("Could not find section in .gitmodules where path=%s" ), path); +return -1 ; + }submodule.c int update_path_in_gitmodules(const char *oldpath, const char *newpath) · line 113
Quoted 10 lines
submodule.c +10
-0+strbuf_addstr (§,"submodule." ); +strbuf_addstr (§, path_option->util); +if (git_config_rename_section_in_file (".gitmodules" , sect.buf,NULL ) <0 ) { +/* Maybe the user already did that, don't error out here */ +warning (_ ("Could not remove .gitmodules entry for %s" ), path); +return -1 ; + } +strbuf_release (§); +return 0 ; +}This question applies for this function and a few more functions in this patch that has the same characteristics.
Quoted 3 lines
If we're in a state when we need to return non-zero, we don't do any cleaning (that is
strbuf_release()). Since this file is in the part called libgit AFAIK, shouldn't we always clean after us?
Right you are, thanks for bringing that up. The last return needs a strbuf_release(), will fix that in v4.
Quoted 2 lines
Would it make sense to have different return values for different errors?
I don't think so. The caller only wants to know if the modification of the .gitmodules file happened or not, so he can decide if that needs to be staged.
Re: [PATCH v4 5/5] rm: delete .gitmodules entry of submodules removed from the work tree
In reply to Jens LehmannCurrently using "git rm" on a submodule removes the submodule's work tree from that of the superproject and the gitlink from the index. But the submodule's section in .gitmodules is left untouched, which is a leftover of the now removed submodule and might irritate users (as opposed to the setting in .git/config, this must stay as a reminder that the user showed interest in this submodule so it will be repopulated later when an older commit is checked out).
Let "git rm" help the user by not only removing the submodule from the work tree but by also removing the "submodule.<submodule name>" section from the .gitmodules file and stage both. This doesn't happen when the "--cached" option is used, as it would modify the work tree. This also silently does nothing 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 removed the section by hand before issuing the "git rm" command (in which case the warning reminds him that rm would have done that for him). Only when .gitmodules is found and contains merge conflicts the rm command will fail and tell the user to resolve the conflict before trying again.
Also extend the man page to inform the user about this new feature. While at it promote the submodule sub-section to a chapter as it made not much sense under "REMOVING FILES THAT HAVE DISAPPEARED FROM THE FILESYSTEM".
In t7610 three uses of "git rm submod" had to be replaced with "git rm --cached submod" because that test expects .gitmodules and the work tree to stay untouched. Also in t7400 the tests for the remaining settings in the .gitmodules file had to be changed to assert that these settings are missing.
---
This version fixes the missing strbuf_release() noticed by Frederik.
Documentation/git-rm.txt | 8 ++-- builtin/rm.c | 19 +++++++-- submodule.c | 33 ++++++++++++++++ submodule.h | 1 + t/t3600-rm.sh | 98 +++++++++++++++++++++++++++++++++++++++++++--- t/t7400-submodule-basic.sh | 14 ++----- t/t7610-mergetool.sh | 6 +-- 7 files changed, 154 insertions(+), 25 deletions(-)
Documentation/git-rm.txt +5 -3
@@ -134,14 +134,16 @@ use the following command: git diff --name-only --diff-filter=D -z | xargs -0 git rm --cached ---------------- -Submodules -~~~~~~~~~~ +SUBMODULES +---------- Only submodules using a gitfile (which means they were cloned with a Git version 1.7.8 or newer) will be removed from the work tree, as their repository lives inside the .git directory of the superproject. If a submodule (or one of those nested inside it) still uses a .git directory, `git rm` will fail - no matter if forced -or not - to protect the submodule's history. +or not - to protect the submodule's history. If it exists the +submodule.<name> section in the linkgit:gitmodules[5] file will also +be removed and that file will be staged (unless --cached or -n are used). A submodule is considered up-to-date when the HEAD is the same as recorded in the index, no tracked files are modified and no untracked
builtin/rm.c +16 -3
@@ -283,6 +283,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)struct pathspec pathspec;char *seen; +gitmodules_config ();git_config (git_default_config,NULL ); argc = parse_options(argc, argv, prefix, builtin_rm_options,
@@ -324,7 +325,10 @@ int cmd_rm(int argc, const char **argv, const char *prefix)continue ;ALLOC_GROW (list.entry, list.nr +1 , list.alloc); list.entry[list.nr].name = ce->name; - list.entry[list.nr++].is_submodule =S_ISGITLINK (ce->ce_mode); + list.entry[list.nr].is_submodule =S_ISGITLINK (ce->ce_mode); +if (list.entry[list.nr++].is_submodule && + !is_staging_gitmodules_ok ()) +die (_ ("Please, stage your changes to .gitmodules or stash them to proceed" )); }if (pathspec.nr) {
@@ -396,13 +400,15 @@ int cmd_rm(int argc, const char **argv, const char *prefix)* in the middle) */ if (!index_only) { -int removed =0 ; +int removed =0 , gitmodules_modified =0 ;for (i =0 ; i < list.nr; i++) {const char *path = list.entry[i].name;if (list.entry[i].is_submodule) {if (is_empty_dir (path)) {if (!rmdir (path)) { removed =1 ; +if (!remove_path_from_gitmodules (path)) + gitmodules_modified =1 ;continue ; } }else {
@@ -410,9 +416,14 @@ int cmd_rm(int argc, const char **argv, const char *prefix)strbuf_addstr (&buf, path);if (!remove_dir_recursively (&buf,0 )) { removed =1 ; +if (!remove_path_from_gitmodules (path)) + gitmodules_modified =1 ;strbuf_release (&buf);continue ; - } + }else if (!file_exists (path)) +/* Submodule was removed by user */ +if (!remove_path_from_gitmodules (path)) + gitmodules_modified =1 ;strbuf_release (&buf);/* Fallthrough and let remove_path() fail. */ }
@@ -424,6 +435,8 @@ int cmd_rm(int argc, const char **argv, const char *prefix)if (!removed)die_errno ("git rm: '%s'" , path); } +if (gitmodules_modified) +stage_updated_gitmodules (); }if (active_cache_changed) {
submodule.c +33 -0
@@ -81,6 +81,39 @@ int update_path_in_gitmodules(const char *oldpath, const char *newpath)return 0 ; } +/* +* Try to remove the "submodule.<name>" section from .gitmodules where the given +* path is configured. Return 0 only if a .gitmodules file was found, a section +* with the correct path=<path> setting was found and we could remove it. +*/ +int remove_path_from_gitmodules (const char *path) +{ +struct strbuf sect =STRBUF_INIT ; +struct string_list_item *path_option; + +if (!file_exists (".gitmodules" ))/* Do nothing without .gitmodules */ +return -1 ; + +if (gitmodules_is_unmerged) +die (_ ("Cannot change unmerged .gitmodules, resolve merge conflicts first" )); + + path_option =unsorted_string_list_lookup (&config_name_for_path, path); +if (!path_option) { +warning (_ ("Could not find section in .gitmodules where path=%s" ), path); +return -1 ; + } +strbuf_addstr (§,"submodule." ); +strbuf_addstr (§, path_option->util); +if (git_config_rename_section_in_file (".gitmodules" , sect.buf,NULL ) <0 ) { +/* Maybe the user already did that, don't error out here */ +warning (_ ("Could not remove .gitmodules entry for %s" ), path); +strbuf_release (§); +return -1 ; + } +strbuf_release (§); +return 0 ; +} +void stage_updated_gitmodules (void ) {struct strbuf buf =STRBUF_INIT ;
submodule.h +1 -0
@@ -13,6 +13,7 @@ enum {int is_staging_gitmodules_ok (void );int update_path_in_gitmodules (const char *oldpath,const char *newpath); +int remove_path_from_gitmodules (const char *path);void stage_updated_gitmodules (void );void set_diffopt_flags_from_submodule_config (struct diff_options *diffopt,const char *path);
t/t3600-rm.sh +92 -6
147 lines not on this page · raw message
t/t7400-submodule-basic.sh +4 -10
@@ -773,13 +773,11 @@ test_expect_success 'submodule add --name allows to replace a submodule with ano test_cmp expect .git ) && echo "repo" >expect && - git config -f .gitmodules submodule.repo.path >actual && - test_cmp expect actual && + test_must_fail git config -f .gitmodules submodule.repo.path && git config -f .gitmodules submodule.repo_new.path >actual && test_cmp expect actual&& echo "$submodurl/repo" >expect && - git config -f .gitmodules submodule.repo.url >actual && - test_cmp expect actual && + test_must_fail git config -f .gitmodules submodule.repo.url && echo "$submodurl/bare.git" >expect && git config -f .gitmodules submodule.repo_new.url >actual && test_cmp expect actual &&
@@ -799,12 +797,8 @@ test_expect_success 'submodule add with an existing name fails unless forced' ' git rm repo && test_must_fail git submodule add -q --name repo_new "$submodurl/repo.git" repo && test ! -d repo && - echo "repo" >expect && - git config -f .gitmodules submodule.repo_new.path >actual && - test_cmp expect actual&& - echo "$submodurl/bare.git" >expect && - git config -f .gitmodules submodule.repo_new.url >actual && - test_cmp expect actual && + test_must_fail git config -f .gitmodules submodule.repo_new.path && + test_must_fail git config -f .gitmodules submodule.repo_new.url && echo "$submodurl/bare.git" >expect && git config submodule.repo_new.url >actual && test_cmp expect actual &&
t/t7610-mergetool.sh +3 -3
@@ -253,7 +253,7 @@ test_expect_success 'deleted vs modified submodule' ' git checkout -b test6 branch1 && git submodule update -N && mv submod submod-movedaside && - git rm submod && + git rm --cached submod && git commit -m "Submodule deleted from branch" && git checkout -b test6.a test6 && test_must_fail git merge master &&
@@ -322,7 +322,7 @@ test_expect_success 'file vs modified submodule' ' git checkout -b test7 branch1 && git submodule update -N && mv submod submod-movedaside && - git rm submod && + git rm --cached submod && echo not a submodule >submod && git add submod && git commit -m "Submodule path becomes file" &&
@@ -453,7 +453,7 @@ test_expect_success 'submodule in subdirectory' ' test_expect_success 'directory vs modified submodule' ' git checkout -b test11 branch1 && mv submod submod-movedaside && - git rm submod && + git rm --cached submod && mkdir submod && echo not a submodule >submod/file16 && git add submod/file16 &&
Signature · 1 line
1.8.4.rc0.199.g3f237fc
Re: [PATCH v4 5/5] rm: delete .gitmodules entry of submodules removed from the work tree
Thanks, will replace the top two commits and queue. Looks like we are getting ready for 'next'?
Re: [PATCH v4 5/5] rm: delete .gitmodules entry of submodules removed from the work tree
Quoted 2 lines · Junio C Hamano
Thanks, will replace the top two commits and queue. Looks like we are getting ready for 'next'?
I'm a bit curious about if we should move towards a reentrent libgit (which would for example make multithreading easier) or not.
If so, I suggest that this patch only use die() in builtin/. However I know that there's a lot of die() all over libgit today, I'm curious about what direction we're heading.
Signature · 5 lines
Med vänliga hälsningar Fredrik Gustafsson tel: 0733-608274 e-post: [email protected]
Re: [PATCH v4 5/5] rm: delete .gitmodules entry of submodules removed from the work tree
Am 07.08.2013 20:28, schrieb Fredrik Gustafsson:
Quoted 6 lines · Junio C Hamano
Quoted 2 lines
Thanks, will replace the top two commits and queue. Looks like we are getting ready for 'next'?
I'm a bit curious about if we should move towards a reentrent libgit (which would for example make multithreading easier) or not.
I'm not aware of such an effort in core Git (I always thought that libgit2 is the project doing what you seem to aim for).
Quoted 3 lines
If so, I suggest that this patch only use
die()in builtin/. However I know that there's a lot ofdie()all over libgit today, I'm curious about what direction we're heading.
The die() calls are just one part. Global variables are another issue, we have memory which is implicitly freed on exit ... so unless we commit ourselves to fix all those issues I see no point in moving the die() calls into builtin/ in my series.
Re: [PATCH v4 5/5] rm: delete .gitmodules entry of submodules removed from the work tree
Quoted 11 lines · Jens Lehmann
Am 07.08.2013 20:28, schrieb Fredrik Gustafsson:
Quoted 6 lines · Junio C Hamano
Quoted 2 lines
Thanks, will replace the top two commits and queue. Looks like we are getting ready for 'next'?
I'm a bit curious about if we should move towards a reentrent libgit (which would for example make multithreading easier) or not.
I'm not aware of such an effort in core Git (I always thought that libgit2 is the project doing what you seem to aim for).
Quoted 8 lines
Quoted 3 lines
If so, I suggest that this patch only use
die()in builtin/. However I know that there's a lot ofdie()all over libgit today, I'm curious about what direction we're heading.The
die()calls are just one part. Global variables are another issue, we have memory which is implicitly freed on exit ... so unless we commit ourselves to fix all those issues I see no point in moving thedie()calls into builtin/ in my series.
Okay, thanks for your answer.
Signature · 5 lines
Med vänliga hälsningar Fredrik Gustafsson tel: 0733-608274 e-post: [email protected]
Re: [PATCH v4 5/5] rm: delete .gitmodules entry of submodules removed from the work tree
In reply to Junio C HamanoAm 06.08.2013 23:11, schrieb Junio C Hamano:
Quoted 2 lines
Thanks, will replace the top two commits and queue. Looks like we are getting ready for 'next'?
I hope so, I'm not aware of any open issues.