* [PATCH] mv: allow moving nested submodules
2016-04-15 18:24 'git mv' doesn't move submodule if it's in a subdirectory Stefan Beller
@ 2016-04-15 19:11 ` Stefan Beller
0 siblings, 0 replies; 6+ messages in thread
From: Stefan Beller @ 2016-04-15 19:11 UTC (permalink / raw)
To: gitster; +Cc: git, Jens.Lehmann, gmane, Stefan Beller
When directories are moved using `git mv` all files in the directory
have been just moved, but no further action was taken on them. This
was done by assigning the mode = WORKING_DIRECTORY to the files
inside a moved directory.
submodules however need to update their link to the git directory as
well as updates to the .gitmodules file. By removing the condition of
`mode != INDEX` (the remaining modes are BOTH and WORKING_DIRECTORY) for
the required submodule actions, we perform these for submodules in a
moved directory.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
Albin,
I think this would fix your problem.
Developed on origin/master (it may apply on older versions, though I did not test)
Thanks,
Stefan
builtin/mv.c | 7 +++++--
t/t7001-mv.sh | 16 ++++++++++++++++
2 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/builtin/mv.c b/builtin/mv.c
index aeae855..65fff11 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -252,9 +252,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) {
- if (rename(src, dst) < 0 && !ignore_errors)
+ if (!show_only) {
+ if (mode != INDEX &&
+ rename(src, dst) < 0 &&
+ !ignore_errors)
die_errno(_("renaming '%s' failed"), src);
+
if (submodule_gitfile[i]) {
if (submodule_gitfile[i] != SUBMODULE_WITH_GITDIR)
connect_work_tree_and_git_dir(dst, submodule_gitfile[i]);
diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index 4008fae..4a2570e 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -292,6 +292,9 @@ test_expect_success 'setup submodule' '
echo content >file &&
git add file &&
git commit -m "added sub and file" &&
+ mkdir -p deep/directory/hierachy &&
+ git submodule add ./. deep/directory/hierachy/sub &&
+ git commit -m "added another submodule" &&
git branch submodule
'
@@ -475,4 +478,17 @@ test_expect_success 'mv -k does not accidentally destroy submodules' '
git checkout .
'
+test_expect_success 'moving a submodule in nested directories' '
+ (
+ cd deep &&
+ git mv directory ../ &&
+ # git status would fail if the update of linking git dir to
+ # work dir of the submodule failed.
+ git status &&
+ git config -f ../.gitmodules submodule.deep/directory/hierachy/sub.path >../actual &&
+ echo "directory/hierachy/sub" >../expect
+ ) &&
+ test_cmp actual expect
+'
+
test_done
--
2.8.1.211.g630c034
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] mv: allow moving nested submodules
@ 2016-04-18 16:54 Stefan Beller
2016-04-18 20:54 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Beller @ 2016-04-18 16:54 UTC (permalink / raw)
To: git, gitster; +Cc: Jens.Lehmann, gmane, Stefan Beller
When directories are moved using `git mv` all files in the directory
have been just moved, but no further action was taken on them. This
was done by assigning the mode = WORKING_DIRECTORY to the files
inside a moved directory.
submodules however need to update their link to the git directory as
well as updates to the .gitmodules file. By removing the condition of
`mode != INDEX` (the remaining modes are BOTH and WORKING_DIRECTORY) for
the required submodule actions, we perform these for submodules in a
moved directory.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
builtin/mv.c | 7 +++++--
t/t7001-mv.sh | 16 ++++++++++++++++
2 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/builtin/mv.c b/builtin/mv.c
index aeae855..65fff11 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -252,9 +252,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) {
- if (rename(src, dst) < 0 && !ignore_errors)
+ if (!show_only) {
+ if (mode != INDEX &&
+ rename(src, dst) < 0 &&
+ !ignore_errors)
die_errno(_("renaming '%s' failed"), src);
+
if (submodule_gitfile[i]) {
if (submodule_gitfile[i] != SUBMODULE_WITH_GITDIR)
connect_work_tree_and_git_dir(dst, submodule_gitfile[i]);
diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index 4008fae..4a2570e 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -292,6 +292,9 @@ test_expect_success 'setup submodule' '
echo content >file &&
git add file &&
git commit -m "added sub and file" &&
+ mkdir -p deep/directory/hierachy &&
+ git submodule add ./. deep/directory/hierachy/sub &&
+ git commit -m "added another submodule" &&
git branch submodule
'
@@ -475,4 +478,17 @@ test_expect_success 'mv -k does not accidentally destroy submodules' '
git checkout .
'
+test_expect_success 'moving a submodule in nested directories' '
+ (
+ cd deep &&
+ git mv directory ../ &&
+ # git status would fail if the update of linking git dir to
+ # work dir of the submodule failed.
+ git status &&
+ git config -f ../.gitmodules submodule.deep/directory/hierachy/sub.path >../actual &&
+ echo "directory/hierachy/sub" >../expect
+ ) &&
+ test_cmp actual expect
+'
+
test_done
--
2.8.0.26.gba39a1b.dirty
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] mv: allow moving nested submodules
2016-04-18 16:54 Stefan Beller
@ 2016-04-18 20:54 ` Junio C Hamano
2016-04-18 21:13 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2016-04-18 20:54 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, Jens.Lehmann, gmane
Stefan Beller <sbeller@google.com> writes:
> if (show_only || verbose)
> printf(_("Renaming %s to %s\n"), src, dst);
> - if (!show_only && mode != INDEX) {
> - if (rename(src, dst) < 0 && !ignore_errors)
> + if (!show_only) {
> + if (mode != INDEX &&
> + rename(src, dst) < 0 &&
> + !ignore_errors)
> die_errno(_("renaming '%s' failed"), src);
> +
If ignore-errors is set and rename fails, this would fall through
and try to touch this codepath...
> if (submodule_gitfile[i]) {
> if (submodule_gitfile[i] != SUBMODULE_WITH_GITDIR)
> connect_work_tree_and_git_dir(dst, submodule_gitfile[i]);
... but I am not sure if this thing is prepared to cope with such a
case? src should have been moved to dst but if rename() failed we
wouldn't see what we expect at dst, or would we?
> diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
> index 4008fae..4a2570e 100755
> --- a/t/t7001-mv.sh
> +++ b/t/t7001-mv.sh
> @@ -292,6 +292,9 @@ test_expect_success 'setup submodule' '
> echo content >file &&
> git add file &&
> git commit -m "added sub and file" &&
> + mkdir -p deep/directory/hierachy &&
> + git submodule add ./. deep/directory/hierachy/sub &&
> + git commit -m "added another submodule" &&
> git branch submodule
> '
>
> @@ -475,4 +478,17 @@ test_expect_success 'mv -k does not accidentally destroy submodules' '
> git checkout .
> '
>
> +test_expect_success 'moving a submodule in nested directories' '
> + (
> + cd deep &&
> + git mv directory ../ &&
> + # git status would fail if the update of linking git dir to
> + # work dir of the submodule failed.
> + git status &&
> + git config -f ../.gitmodules submodule.deep/directory/hierachy/sub.path >../actual &&
> + echo "directory/hierachy/sub" >../expect
> + ) &&
> + test_cmp actual expect
> +'
> +
> test_done
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mv: allow moving nested submodules
2016-04-18 20:54 ` Junio C Hamano
@ 2016-04-18 21:13 ` Junio C Hamano
2016-04-18 21:26 ` Stefan Beller
0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2016-04-18 21:13 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, Jens.Lehmann, gmane
Junio C Hamano <gitster@pobox.com> writes:
> If ignore-errors is set and rename fails, this would fall through
> and try to touch this codepath...
>
>> if (submodule_gitfile[i]) {
>> if (submodule_gitfile[i] != SUBMODULE_WITH_GITDIR)
>> connect_work_tree_and_git_dir(dst, submodule_gitfile[i]);
>
> ... but I am not sure if this thing is prepared to cope with such a
> case? src should have been moved to dst but if rename() failed we
> wouldn't see what we expect at dst, or would we?
In other words, I was wondering if this part should read more like
this:
builtin/mv.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/builtin/mv.c b/builtin/mv.c
index aeae855..37ed0fc 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -252,9 +252,14 @@ 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) {
- if (rename(src, dst) < 0 && !ignore_errors)
+ if (show_only)
+ ;
+ else {
+ if (mode != INDEX && rename(src, dst) < 0) {
+ if (ignore_errors)
+ continue;
die_errno(_("renaming '%s' failed"), src);
+ }
if (submodule_gitfile[i]) {
if (submodule_gitfile[i] != SUBMODULE_WITH_GITDIR)
connect_work_tree_and_git_dir(dst, submodule_gitfile[i]);
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] mv: allow moving nested submodules
2016-04-18 21:13 ` Junio C Hamano
@ 2016-04-18 21:26 ` Stefan Beller
0 siblings, 0 replies; 6+ messages in thread
From: Stefan Beller @ 2016-04-18 21:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org, Jens Lehmann, Albin Otterhäll
On Mon, Apr 18, 2016 at 2:13 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> If ignore-errors is set and rename fails, this would fall through
>> and try to touch this codepath...
>>
>>> if (submodule_gitfile[i]) {
>>> if (submodule_gitfile[i] != SUBMODULE_WITH_GITDIR)
>>> connect_work_tree_and_git_dir(dst, submodule_gitfile[i]);
>>
>> ... but I am not sure if this thing is prepared to cope with such a
>> case? src should have been moved to dst but if rename() failed we
>> wouldn't see what we expect at dst, or would we?
>
> In other words, I was wondering if this part should read more like
> this:
>
> builtin/mv.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/mv.c b/builtin/mv.c
> index aeae855..37ed0fc 100644
> --- a/builtin/mv.c
> +++ b/builtin/mv.c
> @@ -252,9 +252,14 @@ 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) {
> - if (rename(src, dst) < 0 && !ignore_errors)
> + if (show_only)
> + ;
> + else {
> + if (mode != INDEX && rename(src, dst) < 0) {
I agree until here.
> + if (ignore_errors)
> + continue;
> die_errno(_("renaming '%s' failed"), src);
This I thought would be better as:
if (!ignore_errors)
die_errno(...);
and not continue, but continuing is the right thing I would expect.
Speaking of which, connect_work_tree_and_git_dir as well as
update_path_in_gitmodules need to learn about the ignore_errors
flag, too. You would expect them to not die at the faintest problem,
but rather honor the promise of "Skip move or rename actions which
would lead to an error condition."
Thanks for a starting pointer for a new patch!
Stefan
> + }
> if (submodule_gitfile[i]) {
> if (submodule_gitfile[i] != SUBMODULE_WITH_GITDIR)
> connect_work_tree_and_git_dir(dst, submodule_gitfile[i]);
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] mv: allow moving nested submodules
@ 2016-04-19 18:32 Stefan Beller
0 siblings, 0 replies; 6+ messages in thread
From: Stefan Beller @ 2016-04-19 18:32 UTC (permalink / raw)
To: gitster; +Cc: git, Jens.Lehmann, gmane, Stefan Beller
When directories are moved using `git mv` all files in the directory
have been just moved, but no further action was taken on them. This
was done by assigning the mode = WORKING_DIRECTORY to the files
inside a moved directory.
submodules however need to update their link to the git directory as
well as updates to the .gitmodules file. By removing the condition of
`mode != INDEX` (the remaining modes are BOTH and WORKING_DIRECTORY) for
the required submodule actions, we perform these for submodules in a
moved directory.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
> Can you do only the 2/2 on top of maint (or maint-2.6) for now then?
This is developed on maint-2.6.
builtin/mv.c | 22 +++++++++++++---------
t/t7001-mv.sh | 16 ++++++++++++++++
2 files changed, 29 insertions(+), 9 deletions(-)
diff --git a/builtin/mv.c b/builtin/mv.c
index d1d4316..77f3ec5 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -251,15 +251,19 @@ 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) {
- if (rename(src, dst) < 0 && !ignore_errors)
- die_errno(_("renaming '%s' failed"), src);
- if (submodule_gitfile[i]) {
- if (submodule_gitfile[i] != SUBMODULE_WITH_GITDIR)
- connect_work_tree_and_git_dir(dst, submodule_gitfile[i]);
- if (!update_path_in_gitmodules(src, dst))
- gitmodules_modified = 1;
- }
+ if (show_only)
+ continue;
+ if (mode != INDEX &&
+ rename(src, dst) < 0) {
+ if (ignore_errors)
+ continue;
+ die_errno(_("renaming '%s' failed"), src);
+ }
+ if (submodule_gitfile[i]) {
+ if (submodule_gitfile[i] != SUBMODULE_WITH_GITDIR)
+ connect_work_tree_and_git_dir(dst, submodule_gitfile[i]);
+ if (!update_path_in_gitmodules(src, dst))
+ gitmodules_modified = 1;
}
if (mode == WORKING_DIRECTORY)
diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index 7b56081..fcfc953 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -292,6 +292,9 @@ test_expect_success 'setup submodule' '
echo content >file &&
git add file &&
git commit -m "added sub and file" &&
+ mkdir -p deep/directory/hierachy &&
+ git submodule add ./. deep/directory/hierachy/sub &&
+ git commit -m "added another submodule" &&
git branch submodule
'
@@ -475,4 +478,17 @@ test_expect_success 'mv -k does not accidentally destroy submodules' '
git checkout .
'
+test_expect_success 'moving a submodule in nested directories' '
+ (
+ cd deep &&
+ git mv directory ../ &&
+ # git status would fail if the update of linking git dir to
+ # work dir of the submodule failed.
+ git status &&
+ git config -f ../.gitmodules submodule.deep/directory/hierachy/sub.path >../actual &&
+ echo "directory/hierachy/sub" >../expect
+ ) &&
+ test_cmp actual expect
+'
+
test_done
--
2.6.6.dirty
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-04-19 18:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-19 18:32 [PATCH] mv: allow moving nested submodules Stefan Beller
-- strict thread matches above, loose matches on Subject: below --
2016-04-18 16:54 Stefan Beller
2016-04-18 20:54 ` Junio C Hamano
2016-04-18 21:13 ` Junio C Hamano
2016-04-18 21:26 ` Stefan Beller
2016-04-15 18:24 'git mv' doesn't move submodule if it's in a subdirectory Stefan Beller
2016-04-15 19:11 ` [PATCH] mv: allow moving nested submodules Stefan Beller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).