Re: [PATCH] Revert "Merge branch 'sb/submodule-core-worktree'" (was Re: Old submodules broken in 2.19rc1 and 2.19rc2)

2018-09-11 Thread Junio C Hamano
Johannes Sixt  writes:

>>> Like this (generated using "git revert -m1)?
>> 
>> OK.  Thanks for taking care of it.
>
> Please don't forget to remove the corresponding release notes entry.

Thanks for a reminder.  Very much appreciated.


Re: [PATCH] Revert "Merge branch 'sb/submodule-core-worktree'" (was Re: Old submodules broken in 2.19rc1 and 2.19rc2)

2018-09-10 Thread Stefan Beller
> >> Like this (generated using "git revert -m1)?
> >
> > OK.  Thanks for taking care of it.

Yes that looks good to me, thanks!

>
> Please don't forget to remove the corresponding release notes entry.

Makes sense, too.

Thanks,
Stefan


Re: [PATCH] Revert "Merge branch 'sb/submodule-core-worktree'" (was Re: Old submodules broken in 2.19rc1 and 2.19rc2)

2018-09-08 Thread Johannes Sixt
Am 08.09.2018 um 04:04 schrieb Junio C Hamano:
> Jonathan Nieder  writes:
> 
>> It is late in the release cycle, so revert the whole 3-patch series.
>> We can try again later for 2.20.
>>
>> Reported-by: Allan Sandfeld Jensen 
>> Helped-by: Stefan Beller 
>> Signed-off-by: Jonathan Nieder 
>> ---
>> Stefan Beller wrote:
>>> Jonathan Nieder wrote:
>>
 I think we
 should revert e98317508c0 in "master" (for 2.19) and keep making use
 of that 'second try' in "next" (for 2.20).
>>>
>>> Actually I'd rather revert the whole topic leading up to
>>> 7e25437d35a (Merge branch 'sb/submodule-core-worktree', 2018-07-18)
>>> as the last patch in there doesn't work well without e98317508c0 IIRC.
>>>
>>> And having only the first patch would bring an inconsistent state as
>>> then different commands behave differently w.r.t. setting core.worktree.
>>
>> Like this (generated using "git revert -m1)?
> 
> OK.  Thanks for taking care of it.

Please don't forget to remove the corresponding release notes entry.

diff --git a/Documentation/RelNotes/2.19.0.txt 
b/Documentation/RelNotes/2.19.0.txt
index bcbfbc2041..834454ffb9 100644
--- a/Documentation/RelNotes/2.19.0.txt
+++ b/Documentation/RelNotes/2.19.0.txt
@@ -296,12 +296,6 @@ Fixes since v2.18
to the submodule was changed in the range of commits in the
superproject, sometimes showing "(null)".  This has been corrected.
 
- * "git submodule" did not correctly adjust core.worktree setting that
-   indicates whether/where a submodule repository has its associated
-   working tree across various state transitions, which has been
-   corrected.
-   (merge 984cd77ddb sb/submodule-core-worktree later to maint).
-
  * Bugfix for "rebase -i" corner case regression.
(merge a9279c6785 pw/rebase-i-keep-reword-after-conflict later to maint).
 


Re: [PATCH] Revert "Merge branch 'sb/submodule-core-worktree'" (was Re: Old submodules broken in 2.19rc1 and 2.19rc2)

2018-09-07 Thread Junio C Hamano
Jonathan Nieder  writes:

> It is late in the release cycle, so revert the whole 3-patch series.
> We can try again later for 2.20.
>
> Reported-by: Allan Sandfeld Jensen 
> Helped-by: Stefan Beller 
> Signed-off-by: Jonathan Nieder 
> ---
> Stefan Beller wrote:
>> Jonathan Nieder wrote:
>
>>> I think we
>>> should revert e98317508c0 in "master" (for 2.19) and keep making use
>>> of that 'second try' in "next" (for 2.20).
>>
>> Actually I'd rather revert the whole topic leading up to
>> 7e25437d35a (Merge branch 'sb/submodule-core-worktree', 2018-07-18)
>> as the last patch in there doesn't work well without e98317508c0 IIRC.
>>
>> And having only the first patch would bring an inconsistent state as
>> then different commands behave differently w.r.t. setting core.worktree.
>
> Like this (generated using "git revert -m1)?

OK.  Thanks for taking care of it.

>
>  builtin/submodule--helper.c | 26 --
>  git-submodule.sh|  5 -
>  submodule.c | 14 --
>  submodule.h |  2 --
>  t/lib-submodule-update.sh   |  5 ++---
>  t/t7400-submodule-basic.sh  |  5 -
>  6 files changed, 2 insertions(+), 55 deletions(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index b56028ba9d..f6fb8991f3 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -1123,8 +1123,6 @@ static void deinit_submodule(const char *path, const 
> char *prefix,
>   if (!(flags & OPT_QUIET))
>   printf(format, displaypath);
>  
> - submodule_unset_core_worktree(sub);
> -
>   strbuf_release(&sb_rm);
>   }
>  
> @@ -2005,29 +2003,6 @@ static int check_name(int argc, const char **argv, 
> const char *prefix)
>   return 0;
>  }
>  
> -static int connect_gitdir_workingtree(int argc, const char **argv, const 
> char *prefix)
> -{
> - struct strbuf sb = STRBUF_INIT;
> - const char *name, *path;
> - char *sm_gitdir;
> -
> - if (argc != 3)
> - BUG("submodule--helper connect-gitdir-workingtree  
> ");
> -
> - name = argv[1];
> - path = argv[2];
> -
> - strbuf_addf(&sb, "%s/modules/%s", get_git_dir(), name);
> - sm_gitdir = absolute_pathdup(sb.buf);
> -
> - connect_work_tree_and_git_dir(path, sm_gitdir, 0);
> -
> - strbuf_release(&sb);
> - free(sm_gitdir);
> -
> - return 0;
> -}
> -
>  #define SUPPORT_SUPER_PREFIX (1<<0)
>  
>  struct cmd_struct {
> @@ -2041,7 +2016,6 @@ static struct cmd_struct commands[] = {
>   {"name", module_name, 0},
>   {"clone", module_clone, 0},
>   {"update-clone", update_clone, 0},
> - {"connect-gitdir-workingtree", connect_gitdir_workingtree, 0},
>   {"relative-path", resolve_relative_path, 0},
>   {"resolve-relative-url", resolve_relative_url, 0},
>   {"resolve-relative-url-test", resolve_relative_url_test, 0},
> diff --git a/git-submodule.sh b/git-submodule.sh
> index f7fd80345c..1cb2c0a31b 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -580,11 +580,6 @@ cmd_update()
>   die "$(eval_gettext "Unable to find current 
> \${remote_name}/\${branch} revision in submodule path '\$sm_path'")"
>   fi
>  
> - if ! $(git config -f "$(git rev-parse 
> --git-common-dir)/modules/$name/config" core.worktree) 2>/dev/null
> - then
> - git submodule--helper connect-gitdir-workingtree 
> "$name" "$sm_path"
> - fi
> -
>   if test "$subsha1" != "$sha1" || test -n "$force"
>   then
>   subforce=$force
> diff --git a/submodule.c b/submodule.c
> index 50cbf5f13e..a2b266fbfa 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1534,18 +1534,6 @@ int bad_to_remove_submodule(const char *path, unsigned 
> flags)
>   return ret;
>  }
>  
> -void submodule_unset_core_worktree(const struct submodule *sub)
> -{
> - char *config_path = xstrfmt("%s/modules/%s/config",
> - get_git_common_dir(), sub->name);
> -
> - if (git_config_set_in_file_gently(config_path, "core.worktree", NULL))
> - warning(_("Could not unset core.worktree setting in submodule 
> '%s'"),
> -   sub->path);
> -
> - free(config_path);
> -}
> -
>  static const char *get_super_prefix_or_empty(void)
>  {
>   const char *s = get_super_prefix();
> @@ -1711,8 +1699,6 @@ int submodule_move_head(const char *path,
>  
>   if (is_empty_dir(path))
>   rmdir_or_warn(path);
> -
> - submodule_unset_core_worktree(sub);
>   }
>   }
>  out:
> diff --git a/submodule.h b/submodule.h
> index 7d476cefa7..e452919aa4 100644
> --- a/submodule.h
> +++ b/submodule.h
> @@ -127,8 +127,6 @@ int submodule_move_head(const char *path,
>   const char *new_head,
>   unsigned flags);
>  
> -v

[PATCH] Revert "Merge branch 'sb/submodule-core-worktree'" (was Re: Old submodules broken in 2.19rc1 and 2.19rc2)

2018-09-07 Thread Jonathan Nieder
Subject: Revert "Merge branch 'sb/submodule-core-worktree'"

This reverts commit 7e25437d35a70791b345872af202eabfb3e1a8bc, reversing
changes made to 00624d608cc69bd62801c93e74d1ea7a7ddd6598.

v2.19.0-rc0~165^2~1 (submodule: ensure core.worktree is set after
update, 2018-06-18) assumes an "absorbed" submodule layout, where the
submodule's Git directory is in the superproject's .git/modules/
directory and .git in the submodule worktree is a .git file pointing
there.  In particular, it uses $GIT_DIR/modules/$name to find the
submodule to find out whether it already has core.worktree set, and it
uses connect_work_tree_and_git_dir if not, resulting in

fatal: could not open sub/.git for writing

The context behind that patch: v2.19.0-rc0~165^2~2 (submodule: unset
core.worktree if no working tree is present, 2018-06-12) unsets
core.worktree when running commands like "git checkout
--recurse-submodules" to switch to a branch without the submodule.  If
a user then uses "git checkout --no-recurse-submodules" to switch back
to a branch with the submodule and runs "git submodule update", this
patch is needed to ensure that commands using the submodule directly
are aware of the path to the worktree.

It is late in the release cycle, so revert the whole 3-patch series.
We can try again later for 2.20.

Reported-by: Allan Sandfeld Jensen 
Helped-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
---
Stefan Beller wrote:
> Jonathan Nieder wrote:

>> I think we
>> should revert e98317508c0 in "master" (for 2.19) and keep making use
>> of that 'second try' in "next" (for 2.20).
>
> Actually I'd rather revert the whole topic leading up to
> 7e25437d35a (Merge branch 'sb/submodule-core-worktree', 2018-07-18)
> as the last patch in there doesn't work well without e98317508c0 IIRC.
>
> And having only the first patch would bring an inconsistent state as
> then different commands behave differently w.r.t. setting core.worktree.

Like this (generated using "git revert -m1)?

 builtin/submodule--helper.c | 26 --
 git-submodule.sh|  5 -
 submodule.c | 14 --
 submodule.h |  2 --
 t/lib-submodule-update.sh   |  5 ++---
 t/t7400-submodule-basic.sh  |  5 -
 6 files changed, 2 insertions(+), 55 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index b56028ba9d..f6fb8991f3 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1123,8 +1123,6 @@ static void deinit_submodule(const char *path, const char 
*prefix,
if (!(flags & OPT_QUIET))
printf(format, displaypath);
 
-   submodule_unset_core_worktree(sub);
-
strbuf_release(&sb_rm);
}
 
@@ -2005,29 +2003,6 @@ static int check_name(int argc, const char **argv, const 
char *prefix)
return 0;
 }
 
-static int connect_gitdir_workingtree(int argc, const char **argv, const char 
*prefix)
-{
-   struct strbuf sb = STRBUF_INIT;
-   const char *name, *path;
-   char *sm_gitdir;
-
-   if (argc != 3)
-   BUG("submodule--helper connect-gitdir-workingtree  
");
-
-   name = argv[1];
-   path = argv[2];
-
-   strbuf_addf(&sb, "%s/modules/%s", get_git_dir(), name);
-   sm_gitdir = absolute_pathdup(sb.buf);
-
-   connect_work_tree_and_git_dir(path, sm_gitdir, 0);
-
-   strbuf_release(&sb);
-   free(sm_gitdir);
-
-   return 0;
-}
-
 #define SUPPORT_SUPER_PREFIX (1<<0)
 
 struct cmd_struct {
@@ -2041,7 +2016,6 @@ static struct cmd_struct commands[] = {
{"name", module_name, 0},
{"clone", module_clone, 0},
{"update-clone", update_clone, 0},
-   {"connect-gitdir-workingtree", connect_gitdir_workingtree, 0},
{"relative-path", resolve_relative_path, 0},
{"resolve-relative-url", resolve_relative_url, 0},
{"resolve-relative-url-test", resolve_relative_url_test, 0},
diff --git a/git-submodule.sh b/git-submodule.sh
index f7fd80345c..1cb2c0a31b 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -580,11 +580,6 @@ cmd_update()
die "$(eval_gettext "Unable to find current 
\${remote_name}/\${branch} revision in submodule path '\$sm_path'")"
fi
 
-   if ! $(git config -f "$(git rev-parse 
--git-common-dir)/modules/$name/config" core.worktree) 2>/dev/null
-   then
-   git submodule--helper connect-gitdir-workingtree 
"$name" "$sm_path"
-   fi
-
if test "$subsha1" != "$sha1" || test -n "$force"
then
subforce=$force
diff --git a/submodule.c b/submodule.c
index 50cbf5f13e..a2b266fbfa 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1534,18 +1534,6 @@ int bad_to_remove_submodule(const char *path, unsigned 
flags)
return ret;
 }
 
-void submodule_unset_core_worktree(const struct submodule *sub)
-{
-   char *config_pa