* [PATCH] submodule--helper: set alternateLocation for cloned submodules
From: vi0oss @ 2016-12-08 0:39 UTC (permalink / raw)
To: git; +Cc: stefanbeller, Vitaly "_Vi" Shukela
From: "Vitaly \"_Vi\" Shukela" <vi0oss@gmail.com>
In 31224cbdc7 (clone: recursive and reference option triggers
submodule alternates, 2016-08-17) a mechanism was added to
have submodules referenced. It did not address _nested_
submodules, however.
This patch makes all not just the root repository, but also
all submodules (recursively) have submodule.alternateLocation
and submodule.alternateErrorStrategy configured, making Git
search for possible alternates for nested submodules as well.
As submodule's alternate target does not end in .git/objects
(rather .git/modules/qqqqqq/objects), this alternate target
path restriction for in add_possible_reference_from_superproject
relates from "*.git/objects" to just */objects".
New tests have been added to t7408-submodule-reference.
Signed-off-by: Vitaly _Vi Shukela <vi0oss@gmail.com>
---
Notes:
Another round of fixups:
* Corrected code style
* Refactored and fixed the test
Previously test contained errorneous
test_must_fail, which was masked by
missing &&.
Mailmap patch omitted this time.
builtin/submodule--helper.c | 19 ++++++++++--
t/t7408-submodule-reference.sh | 66 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 83 insertions(+), 2 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 4beeda5..92fd676 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -498,9 +498,9 @@ static int add_possible_reference_from_superproject(
/*
* If the alternate object store is another repository, try the
- * standard layout with .git/modules/<name>/objects
+ * standard layout with .git/(modules/<name>)+/objects
*/
- if (ends_with(alt->path, ".git/objects")) {
+ if (ends_with(alt->path, "/objects")) {
char *sm_alternate;
struct strbuf sb = STRBUF_INIT;
struct strbuf err = STRBUF_INIT;
@@ -583,6 +583,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
struct strbuf rel_path = STRBUF_INIT;
struct strbuf sb = STRBUF_INIT;
struct string_list reference = STRING_LIST_INIT_NODUP;
+ char *sm_alternate = NULL, *error_strategy = NULL;
struct option module_clone_options[] = {
OPT_STRING(0, "prefix", &prefix,
@@ -672,6 +673,20 @@ static int module_clone(int argc, const char **argv, const char *prefix)
die(_("could not get submodule directory for '%s'"), path);
git_config_set_in_file(p, "core.worktree",
relative_path(path, sm_gitdir, &rel_path));
+
+ /* setup alternateLocation and alternateErrorStrategy in the cloned submodule if needed */
+ git_config_get_string("submodule.alternateLocation", &sm_alternate);
+ if (sm_alternate)
+ git_config_set_in_file(p, "submodule.alternateLocation",
+ sm_alternate);
+ git_config_get_string("submodule.alternateErrorStrategy", &error_strategy);
+ if (error_strategy)
+ git_config_set_in_file(p, "submodule.alternateErrorStrategy",
+ error_strategy);
+
+ free(sm_alternate);
+ free(error_strategy);
+
strbuf_release(&sb);
strbuf_release(&rel_path);
free(sm_gitdir);
diff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh
index 1c1e289..9325297 100755
--- a/t/t7408-submodule-reference.sh
+++ b/t/t7408-submodule-reference.sh
@@ -125,4 +125,70 @@ test_expect_success 'ignoring missing submodule alternates passes clone and subm
)
'
+test_expect_success 'preparing second superproject with a nested submodule plus partial clone' '
+ test_create_repo supersuper &&
+ (
+ cd supersuper &&
+ echo "I am super super." >file &&
+ git add file &&
+ git commit -m B-super-super-initial
+ git submodule add "file://$base_dir/super" subwithsub &&
+ git commit -m B-super-super-added &&
+ git submodule update --init --recursive &&
+ git repack -ad
+ ) &&
+ git clone supersuper supersuper2 &&
+ (
+ cd supersuper2 &&
+ git submodule update --init
+ )
+'
+
+# At this point there are three root-level positories: A, B, super and super2
+
+test_expect_success 'nested submodule alternate in works and is actually used' '
+ test_when_finished "rm -rf supersuper-clone" &&
+ git clone --recursive --reference supersuper supersuper supersuper-clone &&
+ (
+ cd supersuper-clone &&
+ # test superproject has alternates setup correctly
+ test_alternate_is_used .git/objects/info/alternates . &&
+ # immediate submodule has alternate:
+ test_alternate_is_used .git/modules/subwithsub/objects/info/alternates subwithsub
+ # nested submodule also has alternate:
+ test_alternate_is_used .git/modules/subwithsub/modules/sub/objects/info/alternates subwithsub/sub
+ )
+'
+
+check_that_two_of_three_alternates_are_used() {
+ test_alternate_is_used .git/objects/info/alternates . &&
+ # immediate submodule has alternate:
+ test_alternate_is_used .git/modules/subwithsub/objects/info/alternates subwithsub &&
+ # but nested submodule has no alternate:
+ test_must_fail test_alternate_is_used .git/modules/subwithsub/modules/sub/objects/info/alternates subwithsub/sub
+}
+
+
+test_expect_success 'missing nested submodule alternate fails clone and submodule update' '
+ test_when_finished "rm -rf supersuper-clone" &&
+ test_must_fail git clone --recursive --reference supersuper2 supersuper2 supersuper-clone &&
+ (
+ cd supersuper-clone &&
+ check_that_two_of_three_alternates_are_used &&
+ # update of the submodule fails
+ test_must_fail git submodule update --init --recursive
+ )
+'
+
+test_expect_success 'missing nested submodule alternate in --reference-if-able mode' '
+ test_when_finished "rm -rf supersuper-clone" &&
+ git clone --recursive --reference-if-able supersuper2 supersuper2 supersuper-clone &&
+ (
+ cd supersuper-clone &&
+ check_that_two_of_three_alternates_are_used &&
+ # update of the submodule succeeds
+ git submodule update --init --recursive
+ )
+'
+
test_done
--
2.10.2
^ permalink raw reply related
* Re: [PATCHv5 5/5] submodule: add embed-git-dir function
From: Brandon Williams @ 2016-12-08 1:09 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, pclouds, gitster
In-Reply-To: <20161207210157.18932-6-sbeller@google.com>
On 12/07, Stefan Beller wrote:
> + argv_array_pushl(&cp.args, "--super-prefix", sb.buf,
> + "submodule--helper",
> + "embed-git-dirs", NULL);
check the spacing on these lines, looks like there is an extra space
before "submodule--helper"
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH 2/2] describe: add support for multiple match patterns
From: Jacob Keller @ 2016-12-08 1:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jacob Keller, Git mailing list
In-Reply-To: <xmqq60mvuv8v.fsf@gitster.mtv.corp.google.com>
On Wed, Dec 7, 2016 at 4:20 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jacob Keller <jacob.keller@gmail.com> writes:
>
>> Basically, this started as a script to try each pattern in sequence,
>> but this is slow, cumbersome and easy to mess up.
>>
>> You're suggesting just add a single second pattern that we will do
>> matches and discard any tag that matches that first?
>
> I am not suggesting anything. I was just trying to see how well what
> was designed and implemented supports the use case that motivated
> the feature. Think of it as a sanity check and review of the design.
>
Makes sense.
>> I think I can implement that pretty easily, and it should have simpler
>> semantics. We can discard first, and then match what remains easily.
>
> I actually think "multiple" and "negative" are orthogonal and both
> are good things. If we are enhancing the filtering by refname
> patterns to allow multiple patterns (i.e. your patch), that is good,
> and it would be ideal if we can also have support for negative ones.
I can add support for negative matches pretty easily. I personally
don't see the value of "logical and" filters, ie to match only tags
that match all the given filters, though that does allow some other
forms of expression.
I do like the idea of negative filters, and I'll go ahead and work on
adding that as another extension.
Thanks,
Jake
^ permalink raw reply
* Re: [PATCHv5 4/5] worktree: get worktrees from submodules
From: Brandon Williams @ 2016-12-08 1:14 UTC (permalink / raw)
To: Stefan Beller; +Cc: Junio C Hamano, git@vger.kernel.org, Duy Nguyen
In-Reply-To: <CAGZ79kZiS9dx6QUOcFYh8sSNoVsrv2eNLXJd6X54UekzUiC8VQ@mail.gmail.com>
On 12/07, Stefan Beller wrote:
> On Wed, Dec 7, 2016 at 2:45 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > Stefan Beller <sbeller@google.com> writes:
> >
> >> + submodule_common_dir = strbuf_detach(&sb, NULL);
> >> + ret = get_worktrees_internal(submodule_common_dir, flags);
> >> +
> >> + free(submodule_gitdir);
> >
> > This sequence felt somewhat unusual. I would have written this
> > without an extra variable, i.e.
> >
> > ret = get_worktrees_internal(sb.buf, flags);
> > strbuf_release(&sb);
>
> Yours is cleaner; I don't remember what I was thinking.
>
> Feel free to squash it in; in case a resend is needed I will do that.
Just make sure to leave that free in as it refers to another variable
(submodule_gitdir). It actually turns out there is a memory leak in the
original code because submodule_common_dir is never freed after being
detached from the strbuf.
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH] submodule--helper: set alternateLocation for cloned submodules
From: Stefan Beller @ 2016-12-08 1:22 UTC (permalink / raw)
To: vi0oss, Jeff King; +Cc: git@vger.kernel.org, Stefan Beller
In-Reply-To: <20161208003940.28794-1-vi0oss@gmail.com>
On Wed, Dec 7, 2016 at 4:39 PM, <vi0oss@gmail.com> wrote:
>
> Previously test contained errorneous
> test_must_fail, which was masked by
> missing &&.
I wonder if we could make either
the test_must_fail intelligent to detect such a broken && call chain
or the test_expect_success macro to see for those broken chains.
Patch looks good to me except one very minor nit. :)
> +test_expect_success 'nested submodule alternate in works and is actually used' '
> + test_when_finished "rm -rf supersuper-clone" &&
> + git clone --recursive --reference supersuper supersuper supersuper-clone &&
> + (
> + cd supersuper-clone &&
> + # test superproject has alternates setup correctly
> + test_alternate_is_used .git/objects/info/alternates . &&
> + # immediate submodule has alternate:
> + test_alternate_is_used .git/modules/subwithsub/objects/info/alternates subwithsub
here is a && missing ;)
^ permalink raw reply
* [PATCH] submodule--helper: set alternateLocation for cloned submodules
From: vi0oss @ 2016-12-08 1:38 UTC (permalink / raw)
To: git; +Cc: stefanbeller, Vitaly "_Vi" Shukela
From: "Vitaly \"_Vi\" Shukela" <vi0oss@gmail.com>
In 31224cbdc7 (clone: recursive and reference option triggers
submodule alternates, 2016-08-17) a mechanism was added to
have submodules referenced. It did not address _nested_
submodules, however.
This patch makes all not just the root repository, but also
all submodules (recursively) have submodule.alternateLocation
and submodule.alternateErrorStrategy configured, making Git
search for possible alternates for nested submodules as well.
As submodule's alternate target does not end in .git/objects
(rather .git/modules/qqqqqq/objects), this alternate target
path restriction for in add_possible_reference_from_superproject
relates from "*.git/objects" to just */objects".
New tests have been added to t7408-submodule-reference.
Signed-off-by: Vitaly _Vi Shukela <vi0oss@gmail.com>
---
Notes:
Third review: missing && in test fixed.
Mailmap change not included.
builtin/submodule--helper.c | 19 ++++++++++--
t/t7408-submodule-reference.sh | 66 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 83 insertions(+), 2 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 4beeda5..92fd676 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -498,9 +498,9 @@ static int add_possible_reference_from_superproject(
/*
* If the alternate object store is another repository, try the
- * standard layout with .git/modules/<name>/objects
+ * standard layout with .git/(modules/<name>)+/objects
*/
- if (ends_with(alt->path, ".git/objects")) {
+ if (ends_with(alt->path, "/objects")) {
char *sm_alternate;
struct strbuf sb = STRBUF_INIT;
struct strbuf err = STRBUF_INIT;
@@ -583,6 +583,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
struct strbuf rel_path = STRBUF_INIT;
struct strbuf sb = STRBUF_INIT;
struct string_list reference = STRING_LIST_INIT_NODUP;
+ char *sm_alternate = NULL, *error_strategy = NULL;
struct option module_clone_options[] = {
OPT_STRING(0, "prefix", &prefix,
@@ -672,6 +673,20 @@ static int module_clone(int argc, const char **argv, const char *prefix)
die(_("could not get submodule directory for '%s'"), path);
git_config_set_in_file(p, "core.worktree",
relative_path(path, sm_gitdir, &rel_path));
+
+ /* setup alternateLocation and alternateErrorStrategy in the cloned submodule if needed */
+ git_config_get_string("submodule.alternateLocation", &sm_alternate);
+ if (sm_alternate)
+ git_config_set_in_file(p, "submodule.alternateLocation",
+ sm_alternate);
+ git_config_get_string("submodule.alternateErrorStrategy", &error_strategy);
+ if (error_strategy)
+ git_config_set_in_file(p, "submodule.alternateErrorStrategy",
+ error_strategy);
+
+ free(sm_alternate);
+ free(error_strategy);
+
strbuf_release(&sb);
strbuf_release(&rel_path);
free(sm_gitdir);
diff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh
index 1c1e289..e159fc5 100755
--- a/t/t7408-submodule-reference.sh
+++ b/t/t7408-submodule-reference.sh
@@ -125,4 +125,70 @@ test_expect_success 'ignoring missing submodule alternates passes clone and subm
)
'
+test_expect_success 'preparing second superproject with a nested submodule plus partial clone' '
+ test_create_repo supersuper &&
+ (
+ cd supersuper &&
+ echo "I am super super." >file &&
+ git add file &&
+ git commit -m B-super-super-initial
+ git submodule add "file://$base_dir/super" subwithsub &&
+ git commit -m B-super-super-added &&
+ git submodule update --init --recursive &&
+ git repack -ad
+ ) &&
+ git clone supersuper supersuper2 &&
+ (
+ cd supersuper2 &&
+ git submodule update --init
+ )
+'
+
+# At this point there are three root-level positories: A, B, super and super2
+
+test_expect_success 'nested submodule alternate in works and is actually used' '
+ test_when_finished "rm -rf supersuper-clone" &&
+ git clone --recursive --reference supersuper supersuper supersuper-clone &&
+ (
+ cd supersuper-clone &&
+ # test superproject has alternates setup correctly
+ test_alternate_is_used .git/objects/info/alternates . &&
+ # immediate submodule has alternate:
+ test_alternate_is_used .git/modules/subwithsub/objects/info/alternates subwithsub &&
+ # nested submodule also has alternate:
+ test_alternate_is_used .git/modules/subwithsub/modules/sub/objects/info/alternates subwithsub/sub
+ )
+'
+
+check_that_two_of_three_alternates_are_used() {
+ test_alternate_is_used .git/objects/info/alternates . &&
+ # immediate submodule has alternate:
+ test_alternate_is_used .git/modules/subwithsub/objects/info/alternates subwithsub &&
+ # but nested submodule has no alternate:
+ test_must_fail test_alternate_is_used .git/modules/subwithsub/modules/sub/objects/info/alternates subwithsub/sub
+}
+
+
+test_expect_success 'missing nested submodule alternate fails clone and submodule update' '
+ test_when_finished "rm -rf supersuper-clone" &&
+ test_must_fail git clone --recursive --reference supersuper2 supersuper2 supersuper-clone &&
+ (
+ cd supersuper-clone &&
+ check_that_two_of_three_alternates_are_used &&
+ # update of the submodule fails
+ test_must_fail git submodule update --init --recursive
+ )
+'
+
+test_expect_success 'missing nested submodule alternate in --reference-if-able mode' '
+ test_when_finished "rm -rf supersuper-clone" &&
+ git clone --recursive --reference-if-able supersuper2 supersuper2 supersuper-clone &&
+ (
+ cd supersuper-clone &&
+ check_that_two_of_three_alternates_are_used &&
+ # update of the submodule succeeds
+ git submodule update --init --recursive
+ )
+'
+
test_done
--
2.10.2
^ permalink raw reply related
* [PATCHv6 0/7] submodule embedgitdirs
From: Stefan Beller @ 2016-12-08 1:46 UTC (permalink / raw)
To: bmwill; +Cc: git, pclouds, gitster, Stefan Beller
v6:
* renamed embedgitdirs to absorbgitdirs embedding may be interpreted as
embedding the git dir into the working directory, whereas absorbing sounds
more like the submodule is absorbed by the superproject, making the
submodule less independent
* Worktrees API offer uses_worktrees(void) and submodule_uses_worktree(path).
* moved the printing to stderr and one layer up (out of the pure
relocate_git_dir function).
* connect_... is in dir.h now.
v5:
* Add another layer of abstraction, i.e. the relocate_git_dir is only about
moving a git dir of one repository. The submodule specific stuff (e.g.
recursion into nested submodules) is in submodule.{c,h}
This was motivated by reviews on the series of checkout aware of submodules
building on top of this series, as we want to directly call the embed-git-dirs
function without the overhead of spawning a child process.
v4:
* rebuilt on top of nd/worktree-list-fixup
* fix and test behavior for un-init submodules (don't crash, rather do nothing)
* incorporated a "static" as pointed out by Ramsay
* use internal functions instead of duplicating code in worktree.c
(use get_common_dir_noenv for the submodule to actually get the common dir)
* fixed a memory leak in relocate_gitdir
v3:
* have a slightly more generic function "relocate_gitdir".
The recursion is strictly related to submodules, though.
* bail out if a submodule is using worktrees.
This also lays the groundwork for later doing the proper thing,
as worktree.h offers a function `get_submodule_worktrees(path)`
* nit by duy: use git_path instead of git_common_dir
v2:
* fixed commit message for patch:
"submodule: use absolute path for computing relative path connecting"
* a new patch "submodule helper: support super prefix"
* redid the final patch with more tests and fixing bugs along the way
* "test-lib-functions.sh: teach test_commit -C <dir>" unchanged
v1:
The discussion of the submodule checkout series revealed to me that a command
is needed to move the git directory from the submodules working tree to be
embedded into the superprojects git directory.
So I wrote the code to intern the submodules git dir into the superproject,
but whilst writing the code I realized this could be valueable for our use
in testing too. So I exposed it via the submodule--helper. But as the
submodule helper ought to be just an internal API, we could also
offer it via the proper submodule command.
The command as it is has little value to the end user for now, but
breaking it out of the submodule checkout series hopefully makes review easier.
Thanks,
Stefan
Stefan Beller (7):
submodule: use absolute path for computing relative path connecting
submodule helper: support super prefix
test-lib-functions.sh: teach test_commit -C <dir>
worktree: get worktrees from submodules
worktree: add function to check if worktrees are in use
move connect_work_tree_and_git_dir to dir.h
submodule: add absorb-git-dir function
Documentation/git-submodule.txt | 15 +++++
builtin/submodule--helper.c | 69 ++++++++++++++++----
dir.c | 38 +++++++++++
dir.h | 4 ++
git-submodule.sh | 7 +-
git.c | 2 +-
submodule.c | 127 ++++++++++++++++++++++++++++++-------
submodule.h | 5 +-
t/t7412-submodule-absorbgitdirs.sh | 101 +++++++++++++++++++++++++++++
t/test-lib-functions.sh | 20 ++++--
worktree.c | 70 +++++++++++++++++---
worktree.h | 13 ++++
12 files changed, 418 insertions(+), 53 deletions(-)
create mode 100755 t/t7412-submodule-absorbgitdirs.sh
--
2.11.0.rc2.30.gc512cbd.dirty
^ permalink raw reply
* [PATCHv6 1/7] submodule: use absolute path for computing relative path connecting
From: Stefan Beller @ 2016-12-08 1:46 UTC (permalink / raw)
To: bmwill; +Cc: git, pclouds, gitster, Stefan Beller
In-Reply-To: <20161208014623.7588-1-sbeller@google.com>
The current caller of connect_work_tree_and_git_dir passes
an absolute path for the `git_dir` parameter. In the future patch
we will also pass in relative path for `git_dir`. Extend the functionality
of connect_work_tree_and_git_dir to take relative paths for parameters.
We could work around this in the future patch by computing the absolute
path for the git_dir in the calling site, however accepting relative
paths for either parameter makes the API for this function much harder
to misuse.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
submodule.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/submodule.c b/submodule.c
index 6f7d883de9..66c5ce5a24 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1227,23 +1227,25 @@ 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));
+ char *real_git_dir = xstrdup(real_path(git_dir));
+ char *real_work_tree = xstrdup(real_path(work_tree));
/* Update gitfile */
strbuf_addf(&file_name, "%s/.git", work_tree);
write_file(file_name.buf, "gitdir: %s",
- relative_path(git_dir, real_work_tree, &rel_path));
+ relative_path(real_git_dir, real_work_tree, &rel_path));
/* Update core.worktree setting */
strbuf_reset(&file_name);
- strbuf_addf(&file_name, "%s/config", git_dir);
+ strbuf_addf(&file_name, "%s/config", real_git_dir);
git_config_set_in_file(file_name.buf, "core.worktree",
- relative_path(real_work_tree, git_dir,
+ relative_path(real_work_tree, real_git_dir,
&rel_path));
strbuf_release(&file_name);
strbuf_release(&rel_path);
- free((void *)real_work_tree);
+ free(real_work_tree);
+ free(real_git_dir);
}
int parallel_submodules(void)
--
2.11.0.rc2.30.gc512cbd.dirty
^ permalink raw reply related
* [PATCHv6 2/7] submodule helper: support super prefix
From: Stefan Beller @ 2016-12-08 1:46 UTC (permalink / raw)
To: bmwill; +Cc: git, pclouds, gitster, Stefan Beller
In-Reply-To: <20161208014623.7588-1-sbeller@google.com>
Just like main commands in Git, the submodule helper needs
access to the superproject prefix. Enable this in the git.c
but have its own fuse in the helper code by having a flag to
turn on the super prefix.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/submodule--helper.c | 31 ++++++++++++++++++++-----------
git.c | 2 +-
2 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 4beeda5f9f..33676a57cf 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1076,21 +1076,24 @@ static int resolve_remote_submodule_branch(int argc, const char **argv,
return 0;
}
+#define SUPPORT_SUPER_PREFIX (1<<0)
+
struct cmd_struct {
const char *cmd;
int (*fn)(int, const char **, const char *);
+ int option;
};
static struct cmd_struct commands[] = {
- {"list", module_list},
- {"name", module_name},
- {"clone", module_clone},
- {"update-clone", update_clone},
- {"relative-path", resolve_relative_path},
- {"resolve-relative-url", resolve_relative_url},
- {"resolve-relative-url-test", resolve_relative_url_test},
- {"init", module_init},
- {"remote-branch", resolve_remote_submodule_branch}
+ {"list", module_list, 0},
+ {"name", module_name, 0},
+ {"clone", module_clone, 0},
+ {"update-clone", update_clone, 0},
+ {"relative-path", resolve_relative_path, 0},
+ {"resolve-relative-url", resolve_relative_url, 0},
+ {"resolve-relative-url-test", resolve_relative_url_test, 0},
+ {"init", module_init, 0},
+ {"remote-branch", resolve_remote_submodule_branch, 0},
};
int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
@@ -1100,9 +1103,15 @@ int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
die(_("submodule--helper subcommand must be "
"called with a subcommand"));
- for (i = 0; i < ARRAY_SIZE(commands); i++)
- if (!strcmp(argv[1], commands[i].cmd))
+ for (i = 0; i < ARRAY_SIZE(commands); i++) {
+ if (!strcmp(argv[1], commands[i].cmd)) {
+ if (get_super_prefix() &&
+ !(commands[i].option & SUPPORT_SUPER_PREFIX))
+ die("%s doesn't support --super-prefix",
+ commands[i].cmd);
return commands[i].fn(argc - 1, argv + 1, prefix);
+ }
+ }
die(_("'%s' is not a valid submodule--helper "
"subcommand"), argv[1]);
diff --git a/git.c b/git.c
index efa1059fe0..98dcf6c518 100644
--- a/git.c
+++ b/git.c
@@ -493,7 +493,7 @@ static struct cmd_struct commands[] = {
{ "stage", cmd_add, RUN_SETUP | NEED_WORK_TREE },
{ "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
{ "stripspace", cmd_stripspace },
- { "submodule--helper", cmd_submodule__helper, RUN_SETUP },
+ { "submodule--helper", cmd_submodule__helper, RUN_SETUP | SUPPORT_SUPER_PREFIX},
{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },
{ "tag", cmd_tag, RUN_SETUP },
{ "unpack-file", cmd_unpack_file, RUN_SETUP },
--
2.11.0.rc2.30.gc512cbd.dirty
^ permalink raw reply related
* [PATCHv6 4/7] worktree: get worktrees from submodules
From: Stefan Beller @ 2016-12-08 1:46 UTC (permalink / raw)
To: bmwill; +Cc: git, pclouds, gitster, Stefan Beller
In-Reply-To: <20161208014623.7588-1-sbeller@google.com>
In a later patch we want to move around the the git directory of
a submodule. Both submodules as well as worktrees are involved in
placing git directories at unusual places, so their functionality
may collide. To react appropriately to situations where worktrees
in submodules are in use, offer a new function to query the
worktrees for submodules.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
worktree.c | 46 ++++++++++++++++++++++++++++++++++++----------
worktree.h | 6 ++++++
2 files changed, 42 insertions(+), 10 deletions(-)
diff --git a/worktree.c b/worktree.c
index eb6121263b..75db689672 100644
--- a/worktree.c
+++ b/worktree.c
@@ -72,7 +72,7 @@ static void add_head_info(struct strbuf *head_ref, struct worktree *worktree)
/**
* get the main worktree
*/
-static struct worktree *get_main_worktree(void)
+static struct worktree *get_main_worktree(const char *git_common_dir)
{
struct worktree *worktree = NULL;
struct strbuf path = STRBUF_INIT;
@@ -81,12 +81,12 @@ static struct worktree *get_main_worktree(void)
int is_bare = 0;
int is_detached = 0;
- strbuf_add_absolute_path(&worktree_path, get_git_common_dir());
+ strbuf_add_absolute_path(&worktree_path, git_common_dir);
is_bare = !strbuf_strip_suffix(&worktree_path, "/.git");
if (is_bare)
strbuf_strip_suffix(&worktree_path, "/.");
- strbuf_addf(&path, "%s/HEAD", get_git_common_dir());
+ strbuf_addf(&path, "%s/HEAD", git_common_dir);
worktree = xcalloc(1, sizeof(*worktree));
worktree->path = strbuf_detach(&worktree_path, NULL);
@@ -101,7 +101,8 @@ static struct worktree *get_main_worktree(void)
return worktree;
}
-static struct worktree *get_linked_worktree(const char *id)
+static struct worktree *get_linked_worktree(const char *git_common_dir,
+ const char *id)
{
struct worktree *worktree = NULL;
struct strbuf path = STRBUF_INIT;
@@ -112,7 +113,7 @@ static struct worktree *get_linked_worktree(const char *id)
if (!id)
die("Missing linked worktree name");
- strbuf_git_common_path(&path, "worktrees/%s/gitdir", id);
+ strbuf_addf(&path, "%s/worktrees/%s/gitdir", git_common_dir, id);
if (strbuf_read_file(&worktree_path, path.buf, 0) <= 0)
/* invalid gitdir file */
goto done;
@@ -125,7 +126,7 @@ static struct worktree *get_linked_worktree(const char *id)
}
strbuf_reset(&path);
- strbuf_addf(&path, "%s/worktrees/%s/HEAD", get_git_common_dir(), id);
+ strbuf_addf(&path, "%s/worktrees/%s/HEAD", git_common_dir, id);
if (parse_ref(path.buf, &head_ref, &is_detached) < 0)
goto done;
@@ -167,7 +168,8 @@ static int compare_worktree(const void *a_, const void *b_)
return fspathcmp((*a)->path, (*b)->path);
}
-struct worktree **get_worktrees(unsigned flags)
+static struct worktree **get_worktrees_internal(const char *git_common_dir,
+ unsigned flags)
{
struct worktree **list = NULL;
struct strbuf path = STRBUF_INIT;
@@ -177,9 +179,9 @@ struct worktree **get_worktrees(unsigned flags)
list = xmalloc(alloc * sizeof(struct worktree *));
- list[counter++] = get_main_worktree();
+ list[counter++] = get_main_worktree(git_common_dir);
- strbuf_addf(&path, "%s/worktrees", get_git_common_dir());
+ strbuf_addf(&path, "%s/worktrees", git_common_dir);
dir = opendir(path.buf);
strbuf_release(&path);
if (dir) {
@@ -188,7 +190,7 @@ struct worktree **get_worktrees(unsigned flags)
if (is_dot_or_dotdot(d->d_name))
continue;
- if ((linked = get_linked_worktree(d->d_name))) {
+ if ((linked = get_linked_worktree(git_common_dir, d->d_name))) {
ALLOC_GROW(list, counter + 1, alloc);
list[counter++] = linked;
}
@@ -209,6 +211,30 @@ struct worktree **get_worktrees(unsigned flags)
return list;
}
+struct worktree **get_worktrees(unsigned flags)
+{
+ return get_worktrees_internal(get_git_common_dir(), flags);
+}
+
+struct worktree **get_submodule_worktrees(const char *path, unsigned flags)
+{
+ char *submodule_gitdir;
+ struct strbuf sb = STRBUF_INIT;
+ struct worktree **ret;
+
+ submodule_gitdir = git_pathdup_submodule(path, "%s", "");
+ if (!submodule_gitdir)
+ return NULL;
+
+ /* the env would be set for the superproject */
+ get_common_dir_noenv(&sb, submodule_gitdir);
+ ret = get_worktrees_internal(sb.buf, flags);
+
+ strbuf_release(&sb);
+ free(submodule_gitdir);
+ return ret;
+}
+
const char *get_worktree_git_dir(const struct worktree *wt)
{
if (!wt)
diff --git a/worktree.h b/worktree.h
index d59ce1fee8..157fbc4a66 100644
--- a/worktree.h
+++ b/worktree.h
@@ -27,6 +27,12 @@ struct worktree {
*/
extern struct worktree **get_worktrees(unsigned flags);
+/*
+ * Get the worktrees of a submodule named by `path`.
+ */
+extern struct worktree **get_submodule_worktrees(const char *path,
+ unsigned flags);
+
/*
* Return git dir of the worktree. Note that the path may be relative.
* If wt is NULL, git dir of current worktree is returned.
--
2.11.0.rc2.30.gc512cbd.dirty
^ permalink raw reply related
* [PATCHv6 7/7] submodule: add absorb-git-dir function
From: Stefan Beller @ 2016-12-08 1:46 UTC (permalink / raw)
To: bmwill; +Cc: git, pclouds, gitster, Stefan Beller
In-Reply-To: <20161208014623.7588-1-sbeller@google.com>
When a submodule has its git dir inside the working dir, the submodule
support for checkout that we plan to add in a later patch will fail.
Add functionality to migrate the git directory to be absorbed
into the superprojects git directory.
The newly added code in this patch is structured such that other areas of
Git can also make use of it. The code in the submodule--helper is a mere
wrapper and option parser for the function
`absorb_git_dir_into_superproject`, that takes care of embedding the
submodules git directory into the superprojects git dir. That function
makes use of the more abstract function for this use case
`relocate_gitdir`, which can be used by e.g. the worktree code eventually
to move around a git directory.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
Documentation/git-submodule.txt | 15 ++++++
builtin/submodule--helper.c | 38 ++++++++++++++
dir.c | 12 +++++
dir.h | 3 ++
git-submodule.sh | 7 ++-
submodule.c | 103 +++++++++++++++++++++++++++++++++++++
submodule.h | 4 ++
t/t7412-submodule-absorbgitdirs.sh | 101 ++++++++++++++++++++++++++++++++++++
8 files changed, 282 insertions(+), 1 deletion(-)
create mode 100755 t/t7412-submodule-absorbgitdirs.sh
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index d841573475..918bd1d1bd 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -22,6 +22,7 @@ SYNOPSIS
[commit] [--] [<path>...]
'git submodule' [--quiet] foreach [--recursive] <command>
'git submodule' [--quiet] sync [--recursive] [--] [<path>...]
+'git submodule' [--quiet] absorbgitdirs [--] [<path>...]
DESCRIPTION
@@ -245,6 +246,20 @@ sync::
If `--recursive` is specified, this command will recurse into the
registered submodules, and sync any nested submodules within.
+absorbgitdirs::
+ If a git directory of a submodule is inside the submodule,
+ move the git directory of the submodule into its superprojects
+ `$GIT_DIR/modules` path and then connect the git directory and
+ its working directory by setting the `core.worktree` and adding
+ a .git file pointing to the git directory embedded in the
+ superprojects git directory.
++
+A repository that was cloned independently and later added as a submodule or
+old setups have the submodules git directory inside the submodule instead of
+embedded into the superprojects git directory.
++
+This command is recursive by default.
+
OPTIONS
-------
-q::
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 33676a57cf..0108afac93 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1076,6 +1076,43 @@ static int resolve_remote_submodule_branch(int argc, const char **argv,
return 0;
}
+static int absorb_git_dirs(int argc, const char **argv, const char *prefix)
+{
+ int i;
+ struct pathspec pathspec;
+ struct module_list list = MODULE_LIST_INIT;
+ unsigned flags = ABSORB_GITDIR_RECURSE_SUBMODULES;
+
+ struct option embed_gitdir_options[] = {
+ OPT_STRING(0, "prefix", &prefix,
+ N_("path"),
+ N_("path into the working tree")),
+ OPT_BIT(0, "--recursive", &flags, N_("recurse into submodules"),
+ ABSORB_GITDIR_RECURSE_SUBMODULES),
+ OPT_END()
+ };
+
+ const char *const git_submodule_helper_usage[] = {
+ N_("git submodule--helper embed-git-dir [<path>...]"),
+ NULL
+ };
+
+ argc = parse_options(argc, argv, prefix, embed_gitdir_options,
+ git_submodule_helper_usage, 0);
+
+ gitmodules_config();
+ git_config(submodule_config, NULL);
+
+ if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
+ return 1;
+
+ for (i = 0; i < list.nr; i++)
+ absorb_git_dir_into_superproject(prefix,
+ list.entries[i]->name, flags);
+
+ return 0;
+}
+
#define SUPPORT_SUPER_PREFIX (1<<0)
struct cmd_struct {
@@ -1094,6 +1131,7 @@ static struct cmd_struct commands[] = {
{"resolve-relative-url-test", resolve_relative_url_test, 0},
{"init", module_init, 0},
{"remote-branch", resolve_remote_submodule_branch, 0},
+ {"absorb-git-dirs", absorb_git_dirs, SUPPORT_SUPER_PREFIX},
};
int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
diff --git a/dir.c b/dir.c
index 8b74997c66..cc5729f733 100644
--- a/dir.c
+++ b/dir.c
@@ -2774,3 +2774,15 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)
free(real_work_tree);
free(real_git_dir);
}
+
+/*
+ * Migrate the git directory of the given path from old_git_dir to new_git_dir.
+ */
+void relocate_gitdir(const char *path, const char *old_git_dir, const char *new_git_dir)
+{
+ if (rename(old_git_dir, new_git_dir) < 0)
+ die_errno(_("could not migrate git directory from '%s' to '%s'"),
+ old_git_dir, new_git_dir);
+
+ connect_work_tree_and_git_dir(path, new_git_dir);
+}
diff --git a/dir.h b/dir.h
index 051674a431..bf23a470af 100644
--- a/dir.h
+++ b/dir.h
@@ -336,4 +336,7 @@ void write_untracked_extension(struct strbuf *out, struct untracked_cache *untra
void add_untracked_cache(struct index_state *istate);
void remove_untracked_cache(struct index_state *istate);
extern void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
+extern void relocate_gitdir(const char *path,
+ const char *old_git_dir,
+ const char *new_git_dir);
#endif
diff --git a/git-submodule.sh b/git-submodule.sh
index a024a135d6..9285b5c43d 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -1131,6 +1131,11 @@ cmd_sync()
done
}
+cmd_absorbgitdirs()
+{
+ git submodule--helper absorb-git-dirs --prefix "$wt_prefix" "$@"
+}
+
# This loop parses the command line arguments to find the
# subcommand name to dispatch. Parsing of the subcommand specific
# options are primarily done by the subcommand implementations.
@@ -1140,7 +1145,7 @@ cmd_sync()
while test $# != 0 && test -z "$command"
do
case "$1" in
- add | foreach | init | deinit | update | status | summary | sync)
+ add | foreach | init | deinit | update | status | summary | sync | absorbgitdirs)
command=$1
;;
-q|--quiet)
diff --git a/submodule.c b/submodule.c
index 0bb50b4b62..6477746ce4 100644
--- a/submodule.c
+++ b/submodule.c
@@ -14,6 +14,7 @@
#include "blob.h"
#include "thread-utils.h"
#include "quote.h"
+#include "worktree.h"
static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;
static int parallel_jobs = 1;
@@ -1237,3 +1238,105 @@ void prepare_submodule_repo_env(struct argv_array *out)
}
argv_array_push(out, "GIT_DIR=.git");
}
+
+/*
+ * Embeds a single submodules git directory into the superprojects git dir,
+ * non recursively.
+ */
+static void relocate_single_git_dir_into_superproject(const char *prefix,
+ const char *path)
+{
+ char *old_git_dir = NULL, *real_old_git_dir = NULL, *real_new_git_dir = NULL;
+ const char *new_git_dir;
+ const struct submodule *sub;
+
+ if (submodule_uses_worktrees(path))
+ die(_("relocate_gitdir for submodule '%s' with "
+ "more than one worktree not supported"), path);
+
+ old_git_dir = xstrfmt("%s/.git", path);
+ if (read_gitfile(old_git_dir))
+ /* If it is an actual gitfile, it doesn't need migration. */
+ return;
+
+ real_old_git_dir = xstrdup(real_path(old_git_dir));
+
+ sub = submodule_from_path(null_sha1, path);
+ if (!sub)
+ die(_("could not lookup name for submodule '%s'"), path);
+
+ new_git_dir = git_path("modules/%s", sub->name);
+ if (safe_create_leading_directories_const(new_git_dir) < 0)
+ die(_("could not create directory '%s'"), new_git_dir);
+ real_new_git_dir = xstrdup(real_path(new_git_dir));
+
+ if (!prefix)
+ prefix = get_super_prefix();
+
+ fprintf(stderr, "Migrating git directory of '%s%s' from\n'%s' to\n'%s'\n",
+ prefix ? prefix : "", path,
+ real_old_git_dir, real_new_git_dir);
+
+ relocate_gitdir(path, real_old_git_dir, real_new_git_dir);
+
+ free(old_git_dir);
+ free(real_old_git_dir);
+ free(real_new_git_dir);
+}
+
+/*
+ * Migrate the git directory of the submodule given by path from
+ * having its git directory within the working tree to the git dir nested
+ * in its superprojects git dir under modules/.
+ */
+void absorb_git_dir_into_superproject(const char *prefix,
+ const char *path,
+ unsigned flags)
+{
+ const char *sub_git_dir, *v;
+ char *real_sub_git_dir = NULL, *real_common_git_dir = NULL;
+ struct strbuf gitdir = STRBUF_INIT;
+
+ strbuf_addf(&gitdir, "%s/.git", path);
+ sub_git_dir = resolve_gitdir(gitdir.buf);
+
+ /* Not populated? */
+ if (!sub_git_dir)
+ goto out;
+
+ /* Is it already absorbed into the superprojects git dir? */
+ real_sub_git_dir = xstrdup(real_path(sub_git_dir));
+ real_common_git_dir = xstrdup(real_path(get_git_common_dir()));
+ if (!skip_prefix(real_sub_git_dir, real_common_git_dir, &v))
+ relocate_single_git_dir_into_superproject(prefix, path);
+
+ if (flags & ABSORB_GITDIR_RECURSE_SUBMODULES) {
+ struct child_process cp = CHILD_PROCESS_INIT;
+ struct strbuf sb = STRBUF_INIT;
+
+ if (flags & ~ABSORB_GITDIR_RECURSE_SUBMODULES)
+ die("BUG: we don't know how to pass the flags down?");
+
+ if (get_super_prefix())
+ strbuf_addstr(&sb, get_super_prefix());
+ strbuf_addstr(&sb, path);
+ strbuf_addch(&sb, '/');
+
+ cp.dir = path;
+ cp.git_cmd = 1;
+ cp.no_stdin = 1;
+ argv_array_pushl(&cp.args, "--super-prefix", sb.buf,
+ "submodule--helper",
+ "absorb-git-dirs", NULL);
+ prepare_submodule_repo_env(&cp.env_array);
+ if (run_command(&cp))
+ die(_("could not recurse into submodule '%s'"), path);
+
+ strbuf_release(&sb);
+ }
+
+out:
+ strbuf_release(&gitdir);
+ free(real_sub_git_dir);
+ free(real_common_git_dir);
+}
diff --git a/submodule.h b/submodule.h
index 4e3bf469b4..6229054b99 100644
--- a/submodule.h
+++ b/submodule.h
@@ -74,4 +74,8 @@ int parallel_submodules(void);
*/
void prepare_submodule_repo_env(struct argv_array *out);
+#define ABSORB_GITDIR_RECURSE_SUBMODULES (1<<0)
+extern void absorb_git_dir_into_superproject(const char *prefix,
+ const char *path,
+ unsigned flags);
#endif
diff --git a/t/t7412-submodule-absorbgitdirs.sh b/t/t7412-submodule-absorbgitdirs.sh
new file mode 100755
index 0000000000..7c4e8b8d79
--- /dev/null
+++ b/t/t7412-submodule-absorbgitdirs.sh
@@ -0,0 +1,101 @@
+#!/bin/sh
+
+test_description='Test submodule absorbgitdirs
+
+This test verifies that `git submodue absorbgitdirs` moves a submodules git
+directory into the superproject.
+'
+
+. ./test-lib.sh
+
+test_expect_success 'setup a real submodule' '
+ git init sub1 &&
+ test_commit -C sub1 first &&
+ git submodule add ./sub1 &&
+ test_tick &&
+ git commit -m superproject
+'
+
+test_expect_success 'absorb the git dir' '
+ >expect.1 &&
+ >expect.2 &&
+ >actual.1 &&
+ >actual.2 &&
+ git status >expect.1 &&
+ git -C sub1 rev-parse HEAD >expect.2 &&
+ git submodule absorbgitdirs &&
+ git fsck &&
+ test -f sub1/.git &&
+ test -d .git/modules/sub1 &&
+ git status >actual.1 &&
+ git -C sub1 rev-parse HEAD >actual.2 &&
+ test_cmp expect.1 actual.1 &&
+ test_cmp expect.2 actual.2
+'
+
+test_expect_success 'absorbing does not fail for deinitalized submodules' '
+ test_when_finished "git submodule update --init" &&
+ git submodule deinit --all &&
+ git submodule absorbgitdirs &&
+ test -d .git/modules/sub1 &&
+ test -d sub1 &&
+ ! test -e sub1/.git
+'
+
+test_expect_success 'setup nested submodule' '
+ git init sub1/nested &&
+ test_commit -C sub1/nested first_nested &&
+ git -C sub1 submodule add ./nested &&
+ test_tick &&
+ git -C sub1 commit -m "add nested" &&
+ git add sub1 &&
+ git commit -m "sub1 to include nested submodule"
+'
+
+test_expect_success 'absorb the git dir in a nested submodule' '
+ git status >expect.1 &&
+ git -C sub1/nested rev-parse HEAD >expect.2 &&
+ git submodule absorbgitdirs &&
+ test -f sub1/nested/.git &&
+ test -d .git/modules/sub1/modules/nested &&
+ git status >actual.1 &&
+ git -C sub1/nested rev-parse HEAD >actual.2 &&
+ test_cmp expect.1 actual.1 &&
+ test_cmp expect.2 actual.2
+'
+
+test_expect_success 'setup a gitlink with missing .gitmodules entry' '
+ git init sub2 &&
+ test_commit -C sub2 first &&
+ git add sub2 &&
+ git commit -m superproject
+'
+
+test_expect_success 'absorbing the git dir fails for incomplete submodules' '
+ git status >expect.1 &&
+ git -C sub2 rev-parse HEAD >expect.2 &&
+ test_must_fail git submodule absorbgitdirs &&
+ git -C sub2 fsck &&
+ test -d sub2/.git &&
+ git status >actual &&
+ git -C sub2 rev-parse HEAD >actual.2 &&
+ test_cmp expect.1 actual.1 &&
+ test_cmp expect.2 actual.2
+'
+
+test_expect_success 'setup a submodule with multiple worktrees' '
+ # first create another unembedded git dir in a new submodule
+ git init sub3 &&
+ test_commit -C sub3 first &&
+ git submodule add ./sub3 &&
+ test_tick &&
+ git commit -m "add another submodule" &&
+ git -C sub3 worktree add ../sub3_second_work_tree
+'
+
+test_expect_success 'absorb a submodule with multiple worktrees' '
+ test_must_fail git submodule absorbgitdirs sub3 2>error &&
+ test_i18ngrep "not supported" error
+'
+
+test_done
--
2.11.0.rc2.30.gc512cbd.dirty
^ permalink raw reply related
* [PATCHv6 5/7] worktree: add function to check if worktrees are in use
From: Stefan Beller @ 2016-12-08 1:46 UTC (permalink / raw)
To: bmwill; +Cc: git, pclouds, gitster, Stefan Beller
In-Reply-To: <20161208014623.7588-1-sbeller@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
worktree.c | 24 ++++++++++++++++++++++++
worktree.h | 7 +++++++
2 files changed, 31 insertions(+)
diff --git a/worktree.c b/worktree.c
index 75db689672..2559f33846 100644
--- a/worktree.c
+++ b/worktree.c
@@ -406,3 +406,27 @@ const struct worktree *find_shared_symref(const char *symref,
return existing;
}
+
+static int uses_worktree_internal(struct worktree **worktrees)
+{
+ int i;
+ for (i = 0; worktrees[i]; i++)
+ ; /* nothing */
+
+ free_worktrees(worktrees);
+ return i > 1;
+}
+
+int uses_worktrees(void)
+{
+ return uses_worktree_internal(get_worktrees(0));
+}
+
+int submodule_uses_worktrees(const char *path)
+{
+ struct worktree **worktrees = get_submodule_worktrees(path, 0);
+ if (!worktrees)
+ return 0;
+
+ return uses_worktree_internal(worktrees);
+}
diff --git a/worktree.h b/worktree.h
index 157fbc4a66..76027b1fd2 100644
--- a/worktree.h
+++ b/worktree.h
@@ -33,6 +33,13 @@ extern struct worktree **get_worktrees(unsigned flags);
extern struct worktree **get_submodule_worktrees(const char *path,
unsigned flags);
+/*
+ * Returns 1 if more than one worktree exists.
+ * Returns 0 if only the main worktree exists.
+ */
+extern int uses_worktrees(void);
+extern int submodule_uses_worktrees(const char *path);
+
/*
* Return git dir of the worktree. Note that the path may be relative.
* If wt is NULL, git dir of current worktree is returned.
--
2.11.0.rc2.30.gc512cbd.dirty
^ permalink raw reply related
* [PATCHv6 3/7] test-lib-functions.sh: teach test_commit -C <dir>
From: Stefan Beller @ 2016-12-08 1:46 UTC (permalink / raw)
To: bmwill; +Cc: git, pclouds, gitster, Stefan Beller
In-Reply-To: <20161208014623.7588-1-sbeller@google.com>
Specifically when setting up submodule tests, it comes in handy if
we can create commits in repositories that are not at the root of
the tested trash dir. Add "-C <dir>" similar to gits -C parameter
that will perform the operation in the given directory.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/test-lib-functions.sh | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index fdaeb3a96b..579e812506 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -157,16 +157,21 @@ debug () {
GIT_TEST_GDB=1 "$@"
}
-# Call test_commit with the arguments "<message> [<file> [<contents> [<tag>]]]"
+# Call test_commit with the arguments
+# [-C <directory>] <message> [<file> [<contents> [<tag>]]]"
#
# This will commit a file with the given contents and the given commit
# message, and tag the resulting commit with the given tag name.
#
# <file>, <contents>, and <tag> all default to <message>.
+#
+# If the first argument is "-C", the second argument is used as a path for
+# the git invocations.
test_commit () {
notick= &&
signoff= &&
+ indir= &&
while test $# != 0
do
case "$1" in
@@ -176,21 +181,26 @@ test_commit () {
--signoff)
signoff="$1"
;;
+ -C)
+ indir="$2"
+ shift
+ ;;
*)
break
;;
esac
shift
done &&
+ indir=${indir:+"$indir"/} &&
file=${2:-"$1.t"} &&
- echo "${3-$1}" > "$file" &&
- git add "$file" &&
+ echo "${3-$1}" > "$indir$file" &&
+ git ${indir:+ -C "$indir"} add "$file" &&
if test -z "$notick"
then
test_tick
fi &&
- git commit $signoff -m "$1" &&
- git tag "${4:-$1}"
+ git ${indir:+ -C "$indir"} commit $signoff -m "$1" &&
+ git ${indir:+ -C "$indir"} tag "${4:-$1}"
}
# Call test_merge with the arguments "<message> <commit>", where <commit>
--
2.11.0.rc2.30.gc512cbd.dirty
^ permalink raw reply related
* [PATCHv6 6/7] move connect_work_tree_and_git_dir to dir.h
From: Stefan Beller @ 2016-12-08 1:46 UTC (permalink / raw)
To: bmwill; +Cc: git, pclouds, gitster, Stefan Beller
In-Reply-To: <20161208014623.7588-1-sbeller@google.com>
That function was primarily used by submodule code, but the function
itself is not inherently about submodules. In the next patch we'll
introduce relocate_git_dir, which can be used by worktrees as well,
so find a neutral middle ground in dir.h.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
dir.c | 26 ++++++++++++++++++++++++++
dir.h | 1 +
submodule.c | 26 --------------------------
submodule.h | 1 -
4 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/dir.c b/dir.c
index bfa8c8a9a5..8b74997c66 100644
--- a/dir.c
+++ b/dir.c
@@ -2748,3 +2748,29 @@ void untracked_cache_add_to_index(struct index_state *istate,
{
untracked_cache_invalidate_path(istate, path);
}
+
+/* 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;
+ char *real_git_dir = xstrdup(real_path(git_dir));
+ char *real_work_tree = xstrdup(real_path(work_tree));
+
+ /* Update gitfile */
+ strbuf_addf(&file_name, "%s/.git", work_tree);
+ write_file(file_name.buf, "gitdir: %s",
+ relative_path(real_git_dir, real_work_tree, &rel_path));
+
+ /* Update core.worktree setting */
+ strbuf_reset(&file_name);
+ strbuf_addf(&file_name, "%s/config", real_git_dir);
+ git_config_set_in_file(file_name.buf, "core.worktree",
+ relative_path(real_work_tree, real_git_dir,
+ &rel_path));
+
+ strbuf_release(&file_name);
+ strbuf_release(&rel_path);
+ free(real_work_tree);
+ free(real_git_dir);
+}
diff --git a/dir.h b/dir.h
index 97c83bb383..051674a431 100644
--- a/dir.h
+++ b/dir.h
@@ -335,4 +335,5 @@ struct untracked_cache *read_untracked_extension(const void *data, unsigned long
void write_untracked_extension(struct strbuf *out, struct untracked_cache *untracked);
void add_untracked_cache(struct index_state *istate);
void remove_untracked_cache(struct index_state *istate);
+extern void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
#endif
diff --git a/submodule.c b/submodule.c
index 66c5ce5a24..0bb50b4b62 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1222,32 +1222,6 @@ int merge_submodule(unsigned char result[20], const char *path,
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;
- char *real_git_dir = xstrdup(real_path(git_dir));
- char *real_work_tree = xstrdup(real_path(work_tree));
-
- /* Update gitfile */
- strbuf_addf(&file_name, "%s/.git", work_tree);
- write_file(file_name.buf, "gitdir: %s",
- relative_path(real_git_dir, real_work_tree, &rel_path));
-
- /* Update core.worktree setting */
- strbuf_reset(&file_name);
- strbuf_addf(&file_name, "%s/config", real_git_dir);
- git_config_set_in_file(file_name.buf, "core.worktree",
- relative_path(real_work_tree, real_git_dir,
- &rel_path));
-
- strbuf_release(&file_name);
- strbuf_release(&rel_path);
- free(real_work_tree);
- free(real_git_dir);
-}
-
int parallel_submodules(void)
{
return parallel_jobs;
diff --git a/submodule.h b/submodule.h
index d9e197a948..4e3bf469b4 100644
--- a/submodule.h
+++ b/submodule.h
@@ -65,7 +65,6 @@ int merge_submodule(unsigned char result[20], const char *path, const unsigned c
int 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);
int parallel_submodules(void);
/*
--
2.11.0.rc2.30.gc512cbd.dirty
^ permalink raw reply related
* Re: [PATCH v15 19/27] bisect--helper: `bisect_state` & `bisect_head` shell function in C
From: Pranit Bauva @ 2016-12-08 6:43 UTC (permalink / raw)
To: Stephan Beyer; +Cc: Git List
In-Reply-To: <14b9ba3a-4568-2e4b-0b1a-f0ee75a7c677@gmx.net>
Hey Stephan,
On Wed, Dec 7, 2016 at 5:24 AM, Stephan Beyer <s-beyer@gmx.net> wrote:
> Hi Pranit,
>
> On 12/06/2016 11:40 PM, Pranit Bauva wrote:
>> On Tue, Nov 22, 2016 at 5:42 AM, Stephan Beyer <s-beyer@gmx.net> wrote:
>>> On 10/14/2016 04:14 PM, Pranit Bauva wrote:
>>>> +static int bisect_state(struct bisect_terms *terms, const char **argv,
>>>> + int argc)
>>>> +{
>>>> + const char *state = argv[0];
>>>> +
>>>> + get_terms(terms);
>>>> + if (check_and_set_terms(terms, state))
>>>> + return -1;
>>>> +
>>>> + if (!argc)
>>>> + die(_("Please call `--bisect-state` with at least one argument"));
>>>
>>> I think this check should move to cmd_bisect__helper. There are also the
>>> other argument number checks.
>>
>> Not really. After the whole conversion, the cmdmode will cease to
>> exists while bisect_state will be called directly, thus it is
>> important to check it here.
>
> Okay, that's a point.
> In that case, you should probably use "return error()" again and the
> message (mentioning "--bisect-state") does not make sense when
> --bisect-state ceases to exist.
True. Will change accordingly.
>>>> +
>>>> + if (argc == 1 && one_of(state, terms->term_good,
>>>> + terms->term_bad, "skip", NULL)) {
>>>> + const char *bisected_head = xstrdup(bisect_head());
>>>> + const char *hex[1];
>>>
>>> Maybe:
>>> const char *hex;
>>>
>>>> + unsigned char sha1[20];
>>>> +
>>>> + if (get_sha1(bisected_head, sha1))
>>>> + die(_("Bad rev input: %s"), bisected_head);
>>>
>>> And instead of...
>>>
>>>> + if (bisect_write(state, sha1_to_hex(sha1), terms, 0))
>>>> + return -1;
>>>> +
>>>> + *hex = xstrdup(sha1_to_hex(sha1));
>>>> + if (check_expected_revs(hex, 1))
>>>> + return -1;
>>>
>>> ... simply:
>>>
>>> hex = xstrdup(sha1_to_hex(sha1));
>>> if (set_state(terms, state, hex)) {
>>> free(hex);
>>> return -1;
>>> }
>>> free(hex);
>>>
>>> where:
>>
>> Yes I am planning to convert all places with hex rather than the sha1
>> but not yet, maybe in an another patch series because currently a lot
>> of things revolve around sha1 and changing its behaviour wouldn't
>> really be a part of a porting patch series.
>
> I was not suggesting a change of behavior, I was suggesting a simple
> helper function set_state() to get rid of code duplication above and
> some lines below:
>
>>> ... And replace this:
>>>
>>>> + const char **hex_string = (const char **) &hex.items[i].string;
>>>> + if(bisect_write(state, *hex_string, terms, 0)) {
>>>> + string_list_clear(&hex, 0);
>>>> + return -1;
>>>> + }
>>>> + if (check_expected_revs(hex_string, 1)) {
>>>> + string_list_clear(&hex, 0);
>>>> + return -1;
>>>> + }
>>>
>>> by:
>>>
>>> const char *hex_str = hex.items[i].string;
>>> if (set_state(terms, state, hex_string)) {
>>> string_list_clear(&hex, 0);
>>> return -1;
>>> }
>
> See?
I can do this change of using set_state() keeping aside the sha1 to
hex and such change.
>>>> @@ -184,8 +137,8 @@ exit code \$res from '\$command' is < 0 or >= 128" >&2
>>>> state="$TERM_GOOD"
>>>> fi
>>>>
>>>> - # We have to use a subshell because "bisect_state" can exit.
>>>> - ( bisect_state $state >"$GIT_DIR/BISECT_RUN" )
>>>> + # We have to use a subshell because "--bisect-state" can exit.
>>>> + ( git bisect--helper --bisect-state $state >"$GIT_DIR/BISECT_RUN" )
>>>
>>> The new comment is funny, but you don't need a subshell here any
>>> longer.
>>
>> True, but right now I didn't want to modify that part of the source
>> code to remove the comment. I will remove the comment all together
>> when I port bisect_run()
> For most of the patches, I was commenting on the current state, not on
> the big picture.
>
> Still I think that it is better to remove the comment and the subshell
> instead of making the comment weird ("yes the builtin can exit, but why
> do we need a subshell?" vs "yes the shell function calls exit, so we
> need a subshell because we do not want to exit this shell script")
Sure I will remove the comment.
Regards,
Pranit Bauva
^ permalink raw reply
* Re: BUG: "cherry-pick A..B || git reset --hard OTHER"
From: Christian Couder @ 2016-12-08 7:50 UTC (permalink / raw)
To: Stephan Beyer; +Cc: Junio C Hamano, git, SZEDER Gábor
In-Reply-To: <6facca6e-622a-ea8f-89d8-a18b7faee3cc@gmx.net>
Hi,
On Wed, Dec 7, 2016 at 7:36 PM, Stephan Beyer <s-beyer@gmx.net> wrote:
> Hi,
>
> On 12/06/2016 07:58 PM, Junio C Hamano wrote:
>
>> (1) The third invocation of "cherry-pick" with "--abort" to get rid
>> of the state from the unfinished cherry-pick we did previously
>> is necessary, but the command does not notice that we resetted
>> to a new branch AND we even did some other work there. This
>> loses end-user's work.
>>
>> "git cherry-pick --abort" should learn from "git am --abort"
>> that has an extra safety to deal with the above workflow. The
>> state from the unfinished "am" is removed, but the head is not
>> rewound to avoid losing end-user's work.
>>
>> You can try by replacing two instances of
>>
>> $ git cherry-pick 0582a34f52..a94bb68397
>>
>> with
>>
>> $ git format-patch --stdout 0582a34f52..a94bb68397 | git am
>>
>> in the above sequence, and conclude with "git am--abort" to see
>> how much more pleasant and safe "git am --abort" is.
> Definitely. I'd volunteer to add that safety guard. (But (2) remains.)
That would be great if you could take care of that!
Thanks,
Christian.
^ permalink raw reply
* Re: [PATCH] real_path: make real_path thread-safe
From: Torsten Bögershausen @ 2016-12-08 7:55 UTC (permalink / raw)
To: Brandon Williams
Cc: Junio C Hamano, Ramsay Jones, git, sbeller, peff, jacob.keller
In-Reply-To: <20161207221335.GA116201@google.com>
On Wed, Dec 07, 2016 at 02:13:35PM -0800, Brandon Williams wrote:
> On 12/07, Junio C Hamano wrote:
> > Torsten Bögershausen <tboegi@web.de> writes:
> >
> > > But in any case it seems that e.g.
> > > //SEFVER/SHARE/DIR1/DIR2/..
> > > must be converted into
> > > //SEFVER/SHARE/DIR1
> > >
> > > and
> > > \\SEFVER\SHARE\DIR1\DIR2\..
> > > must be converted into
> > > \\SEFVER\SHARE\DIR1
> >
> > Additional questions that may be interesting are:
> >
> > //A/B/../C is it //A/C? is it an error?
Yes, at least under Windows.
If I have e.g. a Raspi with SAMBA, I can put a git Repository here:
//raspi/torsten/projects/git
If I use
git push //raspi/torsten/../junio/projects/git
that should be an error.
> > //A/B/../../C/D is it //C/D? is it an error?
> >
Same for
git push /raspi/../raspi2/torsten//projects/git
>
>
> Also is //.. the same as //? I would assume so since /.. is /
>
Under Windows //.. is simply illegal, I would say.
The documentation here
https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx
Mentions these 2 examples, what to feed into the WIN32 file API:
a)
\\?\D:\very-long-path
b)
\\server\share\path\file"
c)
"\\?\UNC\server\share\path"
So whatever we do, the ".." resoltion is only allowed to look at
"very-long-path" or "path".
Some conversion may be done in mingw.c:
https://github.com/github/git-msysgit/blob/master/compat/mingw.c
So what I understand, '/' in Git are already converted into '\' if needed ?
It seams that we may wnat a function get_start_of_path(uncpath),
which returns:
get_start_of_path_win("//?/D:/very-long-path") "/very-long-path"
get_start_of_path_win("//server/share/path/file") "/path/file"
get_start_of_path_win("//?/UNC/server/share/path") "/path"
(I don't know if we need the variant with '\', but is would'n hurt):
get_start_of_path_win("\\\\?\\D:\\very-long-path") "\\very-long-path"
get_start_of_path_win("\\\\server\\share\\path\\file") "\\path\\file"
get_start_of_path_win("\\\\?\\UNC\\server\\share\\path") "\\path"
Then the non-windows version could simply return
get_start_of_path_non_win(something) something
Does this make sense ?
^ permalink raw reply
* Re: [PATCH 11/17] pathspec: factor global magic into its own function
From: Duy Nguyen @ 2016-12-08 9:20 UTC (permalink / raw)
To: Brandon Williams; +Cc: Git Mailing List, Stefan Beller, Junio C Hamano
In-Reply-To: <20161207223936.GD116201@google.com>
On Thu, Dec 8, 2016 at 5:39 AM, Brandon Williams <bmwill@google.com> wrote:
> On 12/07, Duy Nguyen wrote:
>> On Wed, Dec 7, 2016 at 4:51 AM, Brandon Williams <bmwill@google.com> wrote:
>> > Create helper functions to read the global magic environment variables
>> > in additon to factoring out the global magic gathering logic into its
>> > own function.
>> >
>> > Signed-off-by: Brandon Williams <bmwill@google.com>
>> > ---
>> > pathspec.c | 120 +++++++++++++++++++++++++++++++++++++------------------------
>> > 1 file changed, 74 insertions(+), 46 deletions(-)
>> >
>> > diff --git a/pathspec.c b/pathspec.c
>> > index 5afebd3..08e76f6 100644
>> > --- a/pathspec.c
>> > +++ b/pathspec.c
>> > @@ -87,6 +87,74 @@ static void prefix_magic(struct strbuf *sb, int prefixlen, unsigned magic)
>> > strbuf_addf(sb, ",prefix:%d)", prefixlen);
>> > }
>> >
>> > +static inline int get_literal_global(void)
>> > +{
>> > + static int literal_global = -1;
>> > +
>> > + if (literal_global < 0)
>> > + literal_global = git_env_bool(GIT_LITERAL_PATHSPECS_ENVIRONMENT,
>> > + 0);
>>
>> These zeros look so lonely. I know it would exceed 80 columns if we
>> put it on the previous line. But I think it's ok for occasional
>> exceptions. Or you could rename noglob_global to noglob.
>
> I was thinking the same thing but was so torn between the char limit. I
> think it's probably ok to rename these vars by drooping the global since
> the function name themselves indicate they are global.
Exactly. I almost suggested just "ret" for that reason, but it was a
bit on the extreme side, relying entirely on the function's name for
context.
--
Duy
^ permalink raw reply
* Re: [PATCH 16/17] pathspec: small readability changes
From: Duy Nguyen @ 2016-12-08 9:23 UTC (permalink / raw)
To: Brandon Williams; +Cc: Git Mailing List, Stefan Beller, Junio C Hamano
In-Reply-To: <20161207232724.GI116201@google.com>
On Thu, Dec 8, 2016 at 6:27 AM, Brandon Williams <bmwill@google.com> wrote:
>> On Wed, Dec 7, 2016 at 4:51 AM, Brandon Williams <bmwill@google.com> wrote:
>> > @@ -362,8 +368,6 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
>> > } else {
>> > item->original = xstrdup(elt);
>> > }
>> > - item->len = strlen(item->match);
>> > - item->prefix = prefixlen;
>> >
>> > if (flags & PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP)
>> > strip_submodule_slash_cheap(item);
>> > @@ -371,13 +375,14 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
>> > if (flags & PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE)
>> > strip_submodule_slash_expensive(item);
>> >
>> > - if (magic & PATHSPEC_LITERAL)
>> > + if (magic & PATHSPEC_LITERAL) {
>> > item->nowildcard_len = item->len;
>> > - else {
>> > + } else {
>> > item->nowildcard_len = simple_length(item->match);
>> > if (item->nowildcard_len < prefixlen)
>> > item->nowildcard_len = prefixlen;
>> > }
>> > +
>> > item->flags = 0;
>>
>> You probably can move this line up with the others too.
>
> I didn't move the item->flags assignment up since the code immediately
> following this assignment deal with setting item->flags. I made more
> sense to keep them grouped.
It's probably why I put it there in the beginning :) Yes let's leave
it where it is then.
--
Duy
^ permalink raw reply
* Re: [PATCH] real_path: make real_path thread-safe
From: Duy Nguyen @ 2016-12-08 9:41 UTC (permalink / raw)
To: Brandon Williams
Cc: Stefan Beller, git@vger.kernel.org, Jeff King, Jacob Keller
In-Reply-To: <20161205201619.GE68588@google.com>
On Tue, Dec 6, 2016 at 3:16 AM, Brandon Williams <bmwill@google.com> wrote:
> On 12/05, Stefan Beller wrote:
>> > static const char *real_path_internal(const char *path, int die_on_error)
>> > {
>> > - static struct strbuf sb = STRBUF_INIT;
>> > + static struct strbuf resolved = STRBUF_INIT;
>>
>> Also by having this static here, it is not quite thread safe, yet.
>>
>> By removing the static here we cannot do the early cheap check as:
>>
>> > /* We've already done it */
>> > - if (path == sb.buf)
>> > + if (path == resolved.buf)
>> > return path;
>>
>> I wonder how often we run into this case; are there some callers explicitly
>> relying on real_path_internal being cheap for repeated calls?
>> (Maybe run the test suite with this early return instrumented? Not sure how
>> to assess the impact of removing the cheap out return optimization)
>>
>> The long tail (i.e. the actual functionality) should actually be
>> faster, I'd imagine
>> as we do less than with using chdir.
>
> Depends on how expensive the chdir calls were. And I'm working to get
> rid of the static buffer. Just need have the callers own the memory
> first.
I suggest you turn this real_path_internal into strbuf_real_path. In
other words, it takes a strbuf and writes the result there, allocating
memory if needed.
This function can replace the two strbuf_addstr(..., real_path(..));
we have in setup.c and sha1_file.c. real_path() can own a static
strbuf buffer to retain current behavior. We could also have a new
wrapper real_pathdup() around strbuf_real_path(), which can replace 9
instances of xstrdup(real_path(...)) (and Stefan is adding a few more;
that's what led me back to these mails)
--
Duy
^ permalink raw reply
* Re: [PATCHv6 1/7] submodule: use absolute path for computing relative path connecting
From: Duy Nguyen @ 2016-12-08 9:45 UTC (permalink / raw)
To: Stefan Beller; +Cc: Brandon Williams, Git Mailing List, Junio C Hamano
In-Reply-To: <20161208014623.7588-2-sbeller@google.com>
On Thu, Dec 8, 2016 at 8:46 AM, Stefan Beller <sbeller@google.com> wrote:
> The current caller of connect_work_tree_and_git_dir passes
> an absolute path for the `git_dir` parameter. In the future patch
> we will also pass in relative path for `git_dir`. Extend the functionality
> of connect_work_tree_and_git_dir to take relative paths for parameters.
>
> We could work around this in the future patch by computing the absolute
> path for the git_dir in the calling site, however accepting relative
> paths for either parameter makes the API for this function much harder
> to misuse.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> submodule.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/submodule.c b/submodule.c
> index 6f7d883de9..66c5ce5a24 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1227,23 +1227,25 @@ 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));
> + char *real_git_dir = xstrdup(real_path(git_dir));
Is it a bad idea to rename the argument git_dir to git_dir_input, or
something, then have a new "git_dir" variable here? It certainly helps
reduce diff size, but not sure if the end result looks better or
worse.
> + char *real_work_tree = xstrdup(real_path(work_tree));
>
> /* Update gitfile */
> strbuf_addf(&file_name, "%s/.git", work_tree);
> write_file(file_name.buf, "gitdir: %s",
> - relative_path(git_dir, real_work_tree, &rel_path));
> + relative_path(real_git_dir, real_work_tree, &rel_path));
>
> /* Update core.worktree setting */
> strbuf_reset(&file_name);
> - strbuf_addf(&file_name, "%s/config", git_dir);
> + strbuf_addf(&file_name, "%s/config", real_git_dir);
> git_config_set_in_file(file_name.buf, "core.worktree",
> - relative_path(real_work_tree, git_dir,
> + relative_path(real_work_tree, real_git_dir,
> &rel_path));
>
> strbuf_release(&file_name);
> strbuf_release(&rel_path);
> - free((void *)real_work_tree);
> + free(real_work_tree);
> + free(real_git_dir);
> }
>
> int parallel_submodules(void)
> --
> 2.11.0.rc2.30.gc512cbd.dirty
>
--
Duy
^ permalink raw reply
* Re: [PATCHv6 2/7] submodule helper: support super prefix
From: Duy Nguyen @ 2016-12-08 9:52 UTC (permalink / raw)
To: Stefan Beller; +Cc: Brandon Williams, Git Mailing List, Junio C Hamano
In-Reply-To: <20161208014623.7588-3-sbeller@google.com>
On Thu, Dec 8, 2016 at 8:46 AM, Stefan Beller <sbeller@google.com> wrote:
> Just like main commands in Git, the submodule helper needs
> access to the superproject prefix. Enable this in the git.c
> but have its own fuse in the helper code by having a flag to
> turn on the super prefix.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> builtin/submodule--helper.c | 31 ++++++++++++++++++++-----------
> git.c | 2 +-
> 2 files changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 4beeda5f9f..33676a57cf 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -1076,21 +1076,24 @@ static int resolve_remote_submodule_branch(int argc, const char **argv,
> return 0;
> }
>
> +#define SUPPORT_SUPER_PREFIX (1<<0)
> +
> struct cmd_struct {
> const char *cmd;
> int (*fn)(int, const char **, const char *);
> + int option;
unsigned int is probably safer for variables that are used as bit-flags.
> int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
> @@ -1100,9 +1103,15 @@ int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
> die(_("submodule--helper subcommand must be "
> "called with a subcommand"));
>
> - for (i = 0; i < ARRAY_SIZE(commands); i++)
> - if (!strcmp(argv[1], commands[i].cmd))
> + for (i = 0; i < ARRAY_SIZE(commands); i++) {
> + if (!strcmp(argv[1], commands[i].cmd)) {
> + if (get_super_prefix() &&
> + !(commands[i].option & SUPPORT_SUPER_PREFIX))
> + die("%s doesn't support --super-prefix",
> + commands[i].cmd);
If it's meant for users to see, please _() the string.
> return commands[i].fn(argc - 1, argv + 1, prefix);
> + }
> + }
>
> die(_("'%s' is not a valid submodule--helper "
> "subcommand"), argv[1]);
> diff --git a/git.c b/git.c
> index efa1059fe0..98dcf6c518 100644
> --- a/git.c
> +++ b/git.c
> @@ -493,7 +493,7 @@ static struct cmd_struct commands[] = {
> { "stage", cmd_add, RUN_SETUP | NEED_WORK_TREE },
> { "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
> { "stripspace", cmd_stripspace },
> - { "submodule--helper", cmd_submodule__helper, RUN_SETUP },
> + { "submodule--helper", cmd_submodule__helper, RUN_SETUP | SUPPORT_SUPER_PREFIX},
The same macro defined twice in two separate .c files? Hmm.. it
confused me a bit because i thought there was a connection.. I guess
it's ok.
--
Duy
^ permalink raw reply
* Re: [PATCHv6 4/7] worktree: get worktrees from submodules
From: Duy Nguyen @ 2016-12-08 10:09 UTC (permalink / raw)
To: Stefan Beller; +Cc: Brandon Williams, Git Mailing List, Junio C Hamano
In-Reply-To: <20161208014623.7588-5-sbeller@google.com>
On Thu, Dec 8, 2016 at 8:46 AM, Stefan Beller <sbeller@google.com> wrote:
>
> worktree = xcalloc(1, sizeof(*worktree));
> worktree->path = strbuf_detach(&worktree_path, NULL);
> @@ -101,7 +101,8 @@ static struct worktree *get_main_worktree(void)
All the good stuff is outside context lines again.. Somewhere between
here we call add_head_info() which calls resolve_ref_unsafe(), which
always uses data from current repo, not the submodule we want it to
look at.
> @@ -167,7 +168,8 @@ static int compare_worktree(const void *a_, const void *b_)
> return fspathcmp((*a)->path, (*b)->path);
fspathcmp() reads core.ignorecase from current repo. I guess it's
insane to have this key different between repos on the same machine,
so it should be ok. But I want to point this out just in case I'm
missing something.
> @@ -188,7 +190,7 @@ struct worktree **get_worktrees(unsigned flags)
> if (is_dot_or_dotdot(d->d_name))
> continue;
>
> - if ((linked = get_linked_worktree(d->d_name))) {
> + if ((linked = get_linked_worktree(git_common_dir, d->d_name))) {
> ALLOC_GROW(list, counter + 1, alloc);
> list[counter++] = linked;
> }
> @@ -209,6 +211,30 @@ struct worktree **get_worktrees(unsigned flags)
> return list;
Right before this line is mark_current_worktree(), which in turn calls
get_git_dir() and not suitable for peeking into another repository the
way submodule code does. get_worktree_git_dir() called within that
function shares the same problem.
> }
>
> +struct worktree **get_worktrees(unsigned flags)
> +{
> + return get_worktrees_internal(get_git_common_dir(), flags);
> +}
> +
> +struct worktree **get_submodule_worktrees(const char *path, unsigned flags)
> +{
> + char *submodule_gitdir;
> + struct strbuf sb = STRBUF_INIT;
> + struct worktree **ret;
> +
> + submodule_gitdir = git_pathdup_submodule(path, "%s", "");
> + if (!submodule_gitdir)
> + return NULL;
> +
> + /* the env would be set for the superproject */
> + get_common_dir_noenv(&sb, submodule_gitdir);
Technically we need to read submodule_gitdir/.config and see if we can
understand core.repositoryformatversion, or find any unrecognized
extensions. But the problem is not specific to this code. And fixing
it is no small task. But perhaps we could call a dummy
validate_submodule_gitdir() here? Then when we implement that function
for real, we don't have to search the entire code base to see where to
put it.
Kinda off-topic though. Feel free to ignore the above comment.
> + ret = get_worktrees_internal(sb.buf, flags);
> +
> + strbuf_release(&sb);
> + free(submodule_gitdir);
> + return ret;
> +}
--
Duy
^ permalink raw reply
* Re: [PATCH 1/3] wt-status: implement opportunisitc index update correctly
From: Paul Tan @ 2016-12-08 10:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stefan Beller, git@vger.kernel.org
In-Reply-To: <CAGZ79kZHGqU2y19_uKhtVuE6vhspzPNpw-nVDnm8gLQ8u528kQ@mail.gmail.com>
Hi Junio,
On Thu, Dec 8, 2016 at 4:48 AM, Stefan Beller <sbeller@google.com> wrote:
> On Wed, Dec 7, 2016 at 11:41 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> The require_clean_work_tree() function calls hold_locked_index()
>> with die_on_error=0 to signal that it is OK if it fails to obtain
>> the lock, but unconditionally calls update_index_if_able(), which
>> will try to write into fd=-1.
>>
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> ---
Ah, sorry about this. I was indeed misled by the function naming and
its comment ("do not complain if we can't"). Should have looked more
closely at the other call sites.
> However I think the promise of that function is
> to take care of the fd == -1?
Hmm, to add on, looking at the three other call sites of this
function, two of them (builtin/commit.c and builtin/describe.c)
basically do:
if (0 <= fd)
update_index_if_able(...)
with that 0 <= fd conditional. With this patch it becomes three out of
four. Perhaps the repeated use of this conditional is a sign that the
0 <= fd check could be built into update_index_if_able()? I think
there is precedent for building in these kind of checks --
rollback_lock_file() also does not fail if the lock file was not
successfully opened.
That said, the number of call sites is quite low so it's probably not
worth doing this.
Thanks,
Paul
^ permalink raw reply
* Re: [PATCH 1/5] am: Fix filename in safe_to_abort() error message
From: Paul Tan @ 2016-12-08 10:21 UTC (permalink / raw)
To: Stephan Beyer
Cc: Git List, Christian Couder, Junio C Hamano, SZEDER Gábor
In-Reply-To: <20161207215133.13433-1-s-beyer@gmx.net>
Hi Stephan,
On Thu, Dec 8, 2016 at 5:51 AM, Stephan Beyer <s-beyer@gmx.net> wrote:
> diff --git a/builtin/am.c b/builtin/am.c
> index 6981f42ce..7cf40e6f2 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -2124,7 +2124,7 @@ static int safe_to_abort(const struct am_state *state)
>
> if (read_state_file(&sb, state, "abort-safety", 1) > 0) {
> if (get_oid_hex(sb.buf, &abort_safety))
> - die(_("could not parse %s"), am_path(state, "abort_safety"));
> + die(_("could not parse %s"), am_path(state, "abort-safety"));
Ah, this is obviously correct. Sorry for the oversight.
> } else
> oidclr(&abort_safety);
>
> --
> 2.11.0.27.g4eed97c
Thanks,
Paul
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox