* Re: [PATCH 2/3] stripspace: respect repository config
From: Jeff King @ 2016-11-22 19:50 UTC (permalink / raw)
To: Junio C Hamano
Cc: René Scharfe, Johannes Schindelin, Duy Nguyen,
Git Mailing List, Ralf Thielow, Taufiq Hoven
In-Reply-To: <xmqqzikrpbz2.fsf@gitster.mtv.corp.google.com>
On Tue, Nov 22, 2016 at 11:10:25AM -0800, Junio C Hamano wrote:
> Archive & Upload-archive:
>
> "cd Documentation && git archive --remote=origin" immediately hits
> "BUG: setup_git_env called without repository" if your Git is built
> with b1ef400eec ("setup_git_env: avoid blind fall-back to ".git"",
> 2016-10-20), which will not be part of the upcoming release. And
> 'origin' will probably not be understood from the local config.
Yeah, I think "cd Documentation && git archive --remote" has always been
broken. It doesn't barf until b1ef400eec, but yes, v2.11-rc2 breaks
finding "origin" in the local config.
I agree that doing the "gently" setup will fix the config problem, but
there's more to do to make it work with b1ef400eec outside of a repo.
It looks like read_remotes_file() and read_branches_file() in remote.c
need to learn to silently return when we are not in a git repository (or
alternatively, remote_get_1() should learn not to call them in such a
case).
I doubt this is a critical case (it only kicks in if you are outside a
repo _and_ your name looks like a remote name and not a URL, _and_ it is
not defined by your config, and the result is that we die("BUG") rather
than reporting "you don't have such a remote"). But any time it is
possible to hit a die("BUG"), we should be fixing it.
> I think we can do the "gently" thing there, as we may be retrieving
> a remote archive outside a local repository. We'd need to tweak
> "output" with prefix to compensate for the case in which the command
> is run from a subdirectory, and probably we need to futz with the
> setup_prefix parameter to write_archive(), as a local caller now
> will know if we are in nongit situation.
Makes sense. I think OPT_FILENAME() would handle "output", but yeah, it
looks like write_archive() already tries to be clever about the setup.
> On the upload-archive side that serves "--remote" request, there is
> enter_repo() so we should be covered OK.
Right, that has to run in a repo, and enter_repo() should handle it.
> Mailinfo:
>
> We may want a "gently" thing there to pick up local configuration.
> i18n.commitencoding and mailinfo.scissors in local repository would
> be ignored otherwise.
Yeah, agreed.
> Verify-pack:
>
> This calls git_config() but these days farms out its operation to
> "index-pack", so we should be OK. We may even want to lose the call
> to git_config() which does not affect anything.
I'd be slightly worried that there is a core.* config option that
affects us in a subtle way (e.g., some portability flag for how we do
run_command()). I don't think there is one now, but it seems like a
potential maintenance pitfall. IOW, I think I'd generally prefer to err
on the side of having everything do a gentle setup and load at least the
default config, to avoid surprises.
I don't think it's a high priority, though.
So what do you want to do for v2.11? I think there are minor regressions
in stripspace, archive, and mailinfo with respect to reading
.git/config. The right solution in each case is to do a gentle repo
setup. We have patches for stripspace already, but not yet for the other
two. They _should_ be fairly trivial, but we are getting awfully close
to the release. Do you want to do another round of -rc3? Ship with the
minor regressions and fix them up in v2.11.1? Last-minute revert the
original config topic that introduced the regressions (I'm biased
against that as the author, but I also think it's dangerous; it's a big
enough topic that it might introduce new problems)?
-Peff
^ permalink raw reply
* [PATCH] SQUASH
From: Stefan Beller @ 2016-11-22 19:33 UTC (permalink / raw)
To: bmwill, gitster; +Cc: git, jrnieder, Jens.Lehmann, hvoigt, Stefan Beller
In-Reply-To: <20161122192235.6055-5-sbeller@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
On Tue, Nov 22, 2016 at 11:22 AM, Stefan Beller <sbeller@google.com> wrote:
> 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 embedded
> into the superprojects git directory.
>
I spoke too early, this needs to be squashed. :(
Stefan
builtin/submodule--helper.c | 3 +++
git-submodule.sh | 2 +-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index e94dd68a0e..75cdbf45b8 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1083,6 +1083,9 @@ static int embed_git_dir(int argc, const char **argv, const char *prefix)
struct module_list list = MODULE_LIST_INIT;
struct option embed_gitdir_options[] = {
+ OPT_STRING(0, "prefix", &prefix,
+ N_("path"),
+ N_("path into the working tree")),
OPT_END()
};
diff --git a/git-submodule.sh b/git-submodule.sh
index 2178248287..b7e124f340 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -1133,7 +1133,7 @@ cmd_sync()
cmd_embedgitdirs()
{
- git submodule--helper --prefix "$wt_prefix" embed-git-dirs "$@"
+ git submodule--helper embed-git-dirs --prefix "$wt_prefix" "$@"
}
# This loop parses the command line arguments to find the
--
2.11.0.rc2.4.g3396b6f.dirty
^ permalink raw reply related
* Re: Fwd: git diff with “--word-diff-regex” extremely slow compared to “--word-diff”?
From: Jeff King @ 2016-11-22 19:26 UTC (permalink / raw)
To: Matthieu S; +Cc: git
In-Reply-To: <CAEYvigLLQq2SK60UsiTPCxpptpjz85_rGtDVugjfu-sCT1juGQ@mail.gmail.com>
On Tue, Nov 22, 2016 at 10:08:33AM -0800, Matthieu S wrote:
> You are right, I should have compared with the same regex, and indeed,
> --word-diff-regex=[^[:space:]] is also much slower than just
> --word-diff, although they do the same job. Maybe this is a hint that
> the --word-diff-regex code could be made faster?
Maybe. If most of the time is spent in the regex engine, there may not
be much we can do. But perhaps there is something in the surrounding
code that can be improved. Looking at find_word_boundaries() (and this
is the first time I've done so), it does look like we regex-match the
whole buffer, and only then find the end-of-line. Now that we have
regexec_buf(), it might be possible to constrain the regex buffer more.
> I have a small understanding of git, but is git diff computing the
> diff value for the whole file, and then showing in the terminal the 10
> first values? In some cases, it seems to be a lot of unnecessary
> computation! Is there any possibility to ask git-diff to only compare
> say the first 100 lines? Or compute only when necessary, i.e.
> when"enter" is prompted in the console?
Git always computes the diff for the whole file. The paging is done by
an external program. So no, there's no easy way to do it incrementally
as the user interacts with the pager, as the pager does not communicate
back to git in any way. However, git should generally be streaming out
results (and the pager showing them) as they're computed, so in an ideal
world you get output immediately, and then the pager buffers the rest of
it while you're reading the first page.
Git does have to look at the whole file in order to do the initial
line-by-line diff, so it would be hard to make that incremental. It
could do the word-coloring for each hunk incrementally, though. I would
have assumed that is already how it is done, though I didn't dig into
it.
-Peff
^ permalink raw reply
* [PATCHv2 4/4] submodule: add embed-git-dir function
From: Stefan Beller @ 2016-11-22 19:22 UTC (permalink / raw)
To: bmwill, gitster; +Cc: git, jrnieder, Jens.Lehmann, hvoigt, Stefan Beller
In-Reply-To: <20161122192235.6055-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 embedded
into the superprojects git directory.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
Documentation/git-submodule.txt | 14 +++++++
builtin/submodule--helper.c | 33 ++++++++++++++++-
git-submodule.sh | 7 +++-
submodule.c | 65 ++++++++++++++++++++++++++++++++
submodule.h | 2 +
t/t7412-submodule-embedgitdirs.sh | 78 +++++++++++++++++++++++++++++++++++++++
6 files changed, 197 insertions(+), 2 deletions(-)
create mode 100755 t/t7412-submodule-embedgitdirs.sh
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index d841573475..34791cfc65 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] embedgitdirs [--] [<path>...]
DESCRIPTION
@@ -245,6 +246,19 @@ sync::
If `--recursive` is specified, this command will recurse into the
registered submodules, and sync any nested submodules within.
+embedgitdirs::
+ Move the git directory of submodules 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 interned into the
+ superproject.
++
+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 806e29ce4e..e94dd68a0e 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1076,6 +1076,36 @@ static int resolve_remote_submodule_branch(int argc, const char **argv,
return 0;
}
+static int embed_git_dir(int argc, const char **argv, const char *prefix)
+{
+ int i;
+ struct pathspec pathspec;
+ struct module_list list = MODULE_LIST_INIT;
+
+ struct option embed_gitdir_options[] = {
+ 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++)
+ migrate_submodule_gitdir(prefix, list.entries[i]->name, 1);
+
+ return 0;
+}
+
#define SUPPORT_SUPER_PREFIX (1<<0)
struct cmd_struct {
@@ -1093,7 +1123,8 @@ static struct cmd_struct commands[] = {
{"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}
+ {"remote-branch", resolve_remote_submodule_branch, 0},
+ {"embed-git-dirs", embed_git_dir, SUPPORT_SUPER_PREFIX}
};
int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
diff --git a/git-submodule.sh b/git-submodule.sh
index a024a135d6..2178248287 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -1131,6 +1131,11 @@ cmd_sync()
done
}
+cmd_embedgitdirs()
+{
+ git submodule--helper --prefix "$wt_prefix" embed-git-dirs "$@"
+}
+
# 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 | embedgitdirs)
command=$1
;;
-q|--quiet)
diff --git a/submodule.c b/submodule.c
index 66c5ce5a24..d330b567a3 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1263,3 +1263,68 @@ void prepare_submodule_repo_env(struct argv_array *out)
}
argv_array_push(out, "GIT_DIR=.git");
}
+
+/*
+ * Migrate the given submodule (and all its submodules recursively) from
+ * having its git directory within the working tree to the git dir nested
+ * in its superprojects git dir under modules/.
+ */
+void migrate_submodule_gitdir(const char *prefix, const char *path,
+ int recursive)
+{
+ char *old_git_dir;
+ const char *new_git_dir;
+ const struct submodule *sub;
+
+ old_git_dir = xstrfmt("%s/.git", path);
+ if (read_gitfile(old_git_dir))
+ /* If it is an actual gitfile, it doesn't need migration. */
+ goto out;
+
+ sub = submodule_from_path(null_sha1, path);
+ if (!sub)
+ die(_("Could not lookup name for submodule '%s'"),
+ path);
+
+ new_git_dir = git_common_path("modules/%s", sub->name);
+ if (safe_create_leading_directories_const(new_git_dir) < 0)
+ die(_("could not create directory '%s'"), new_git_dir);
+
+ if (!prefix)
+ prefix = get_super_prefix();
+ printf("Migrating git directory of %s%s from\n'%s' to\n'%s'\n",
+ prefix ? prefix : "", path,
+ real_path(old_git_dir), 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);
+
+out:
+ if (recursive) {
+ struct child_process cp = CHILD_PROCESS_INIT;
+ struct strbuf sb = STRBUF_INIT;
+
+ 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",
+ "embed-git-dirs", NULL);
+ prepare_submodule_repo_env(&cp.env_array);
+ if (run_command(&cp))
+ die(_("Could not migrate git directory in submodule '%s'"),
+ path);
+
+ strbuf_release(&sb);
+ }
+
+ free(old_git_dir);
+}
diff --git a/submodule.h b/submodule.h
index d9e197a948..e5975d1f3d 100644
--- a/submodule.h
+++ b/submodule.h
@@ -75,4 +75,6 @@ int parallel_submodules(void);
*/
void prepare_submodule_repo_env(struct argv_array *out);
+extern void migrate_submodule_gitdir(const char *prefix,
+ const char *path, int recursive);
#endif
diff --git a/t/t7412-submodule-embedgitdirs.sh b/t/t7412-submodule-embedgitdirs.sh
new file mode 100755
index 0000000000..3153dbc069
--- /dev/null
+++ b/t/t7412-submodule-embedgitdirs.sh
@@ -0,0 +1,78 @@
+#!/bin/sh
+
+test_description='Test submodule embedgitdirs
+
+This test verifies that `git submodue embedgitdirs` 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 'embed 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 embedgitdirs &&
+ 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 '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 'embed the git dir in a nested submodule' '
+ git status >expect.1 &&
+ git -C sub1/nested rev-parse HEAD >expect.2 &&
+ git submodule embedgitdirs &&
+ 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 'embedding the git dir fails for incomplete submodules' '
+ git status >expect.1 &&
+ git -C sub2 rev-parse HEAD >expect.2 &&
+ test_must_fail git submodule embedgitdirs &&
+ 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_done
+
--
2.11.0.rc2.4.g3396b6f.dirty
^ permalink raw reply related
* [PATCHv2 0/4] `submodule embedgitdirs` [was: Introduce `submodule interngitdirs`]
From: Stefan Beller @ 2016-11-22 19:22 UTC (permalink / raw)
To: bmwill, gitster; +Cc: git, jrnieder, Jens.Lehmann, hvoigt, Stefan Beller
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
diff to v1 below.
Stefan Beller (4):
submodule: use absolute path for computing relative path connecting
submodule helper: support super prefix
test-lib-functions.sh: teach test_commit -C <dir>
submodule: add embed-git-dir function
Documentation/git-submodule.txt | 14 +++++++
builtin/submodule--helper.c | 62 +++++++++++++++++++++++++------
git-submodule.sh | 7 +++-
git.c | 2 +-
submodule.c | 77 +++++++++++++++++++++++++++++++++++---
submodule.h | 2 +
t/t7412-submodule-embedgitdirs.sh | 78 +++++++++++++++++++++++++++++++++++++++
t/test-lib-functions.sh | 20 +++++++---
8 files changed, 239 insertions(+), 23 deletions(-)
create mode 100755 t/t7412-submodule-embedgitdirs.sh
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
diff to v1:
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 80d55350eb..34791cfc65 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -22,7 +22,8 @@ SYNOPSIS
[commit] [--] [<path>...]
'git submodule' [--quiet] foreach [--recursive] <command>
'git submodule' [--quiet] sync [--recursive] [--] [<path>...]
-'git submodule' [--quiet] interngitdirs [--] [<path>...]
+'git submodule' [--quiet] embedgitdirs [--] [<path>...]
+
DESCRIPTION
-----------
@@ -245,18 +246,18 @@ sync::
If `--recursive` is specified, this command will recurse into the
registered submodules, and sync any nested submodules within.
-interngitdirs::
+embedgitdirs::
Move the git directory of submodules 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 interned into the
superproject.
+
- 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 the
+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.
+This command is recursive by default.
OPTIONS
-------
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 256f8e9439..75cdbf45b8 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1076,22 +1076,25 @@ static int resolve_remote_submodule_branch(int argc, const char **argv,
return 0;
}
-static int intern_git_dir(int argc, const char **argv, const char *prefix)
+static int embed_git_dir(int argc, const char **argv, const char *prefix)
{
int i;
struct pathspec pathspec;
struct module_list list = MODULE_LIST_INIT;
- struct option intern_gitdir_options[] = {
+ struct option embed_gitdir_options[] = {
+ OPT_STRING(0, "prefix", &prefix,
+ N_("path"),
+ N_("path into the working tree")),
OPT_END()
};
const char *const git_submodule_helper_usage[] = {
- N_("git submodule--helper intern-git-dir [<path>...]"),
+ N_("git submodule--helper embed-git-dir [<path>...]"),
NULL
};
- argc = parse_options(argc, argv, prefix, intern_gitdir_options,
+ argc = parse_options(argc, argv, prefix, embed_gitdir_options,
git_submodule_helper_usage, 0);
gitmodules_config();
@@ -1101,27 +1104,30 @@ static int intern_git_dir(int argc, const char **argv, const char *prefix)
return 1;
for (i = 0; i < list.nr; i++)
- migrate_submodule_gitdir(list.entries[i]->name);
+ migrate_submodule_gitdir(prefix, list.entries[i]->name, 1);
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},
- {"intern-git-dir", intern_git_dir}
+ {"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},
+ {"embed-git-dirs", embed_git_dir, SUPPORT_SUPER_PREFIX}
};
int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
@@ -1131,9 +1137,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-submodule.sh b/git-submodule.sh
index 747e934df2..b7e124f340 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -1131,9 +1131,9 @@ cmd_sync()
done
}
-cmd_interngitdirs()
+cmd_embedgitdirs()
{
- git submodule--helper intern-git-dir "$@"
+ git submodule--helper embed-git-dirs --prefix "$wt_prefix" "$@"
}
# This loop parses the command line arguments to find the
@@ -1145,7 +1145,7 @@ cmd_interngitdirs()
while test $# != 0 && test -z "$command"
do
case "$1" in
- add | foreach | init | deinit | update | status | summary | sync | interngitdirs)
+ add | foreach | init | deinit | update | status | summary | sync | embedgitdirs)
command=$1
;;
-q|--quiet)
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 },
diff --git a/submodule.c b/submodule.c
index 99befdba85..d330b567a3 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1269,22 +1269,12 @@ void prepare_submodule_repo_env(struct argv_array *out)
* having its git directory within the working tree to the git dir nested
* in its superprojects git dir under modules/.
*/
-void migrate_submodule_gitdir(const char *path)
+void migrate_submodule_gitdir(const char *prefix, const char *path,
+ int recursive)
{
char *old_git_dir;
const char *new_git_dir;
const struct submodule *sub;
- struct child_process cp = CHILD_PROCESS_INIT;
-
- cp.git_cmd = 1;
- cp.no_stdin = 1;
- cp.dir = path;
- argv_array_pushl(&cp.args, "submodule", "foreach", "--recursive",
- "git", "submodule--helper" "intern-git-dir", NULL);
-
- if (run_command(&cp))
- die(_("Could not migrate git directory in submodule '%s'"),
- path);
old_git_dir = xstrfmt("%s/.git", path);
if (read_gitfile(old_git_dir))
@@ -1295,14 +1285,46 @@ void migrate_submodule_gitdir(const char *path)
if (!sub)
die(_("Could not lookup name for submodule '%s'"),
path);
+
new_git_dir = git_common_path("modules/%s", sub->name);
- mkdir_in_gitdir(".git/modules");
+ if (safe_create_leading_directories_const(new_git_dir) < 0)
+ die(_("could not create directory '%s'"), new_git_dir);
+
+ if (!prefix)
+ prefix = get_super_prefix();
+ printf("Migrating git directory of %s%s from\n'%s' to\n'%s'\n",
+ prefix ? prefix : "", path,
+ real_path(old_git_dir), 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);
+
out:
+ if (recursive) {
+ struct child_process cp = CHILD_PROCESS_INIT;
+ struct strbuf sb = STRBUF_INIT;
+
+ 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",
+ "embed-git-dirs", NULL);
+ prepare_submodule_repo_env(&cp.env_array);
+ if (run_command(&cp))
+ die(_("Could not migrate git directory in submodule '%s'"),
+ path);
+
+ strbuf_release(&sb);
+ }
+
free(old_git_dir);
}
diff --git a/submodule.h b/submodule.h
index 859026ecfa..e5975d1f3d 100644
--- a/submodule.h
+++ b/submodule.h
@@ -75,5 +75,6 @@ int parallel_submodules(void);
*/
void prepare_submodule_repo_env(struct argv_array *out);
-extern void migrate_submodule_gitdir(const char *path);
+extern void migrate_submodule_gitdir(const char *prefix,
+ const char *path, int recursive);
#endif
diff --git a/t/t7412-submodule-embedgitdirs.sh b/t/t7412-submodule-embedgitdirs.sh
new file mode 100755
index 0000000000..3153dbc069
--- /dev/null
+++ b/t/t7412-submodule-embedgitdirs.sh
@@ -0,0 +1,78 @@
+#!/bin/sh
+
+test_description='Test submodule embedgitdirs
+
+This test verifies that `git submodue embedgitdirs` 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 'embed 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 embedgitdirs &&
+ 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 '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 'embed the git dir in a nested submodule' '
+ git status >expect.1 &&
+ git -C sub1/nested rev-parse HEAD >expect.2 &&
+ git submodule embedgitdirs &&
+ 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 'embedding the git dir fails for incomplete submodules' '
+ git status >expect.1 &&
+ git -C sub2 rev-parse HEAD >expect.2 &&
+ test_must_fail git submodule embedgitdirs &&
+ 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_done
+
diff --git a/t/t7412-submodule-interngitdirs.sh b/t/t7412-submodule-interngitdirs.sh
deleted file mode 100755
index 709359155d..0000000000
--- a/t/t7412-submodule-interngitdirs.sh
+++ /dev/null
@@ -1,40 +0,0 @@
-#!/bin/sh
-
-test_description='Test submodule interngitdirs
-
-This test verifies that `git submodue interngitdirs` 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 'intern the git dir' '
- git submodule interngitdirs &&
- test -f sub1/.git &&
- test -d .git/modules/sub1 &&
- # check that we did not break the repository:
- git status
-'
-
-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 'intern the git dir fails for incomplete submodules' '
- test_must_fail git submodule interngitdirs &&
- # check that we did not break the repository:
- git status
-'
-
-test_done
--
2.11.0.rc2.4.g3396b6f.dirty
^ permalink raw reply related
* [PATCHv2 2/4] submodule helper: support super prefix
From: Stefan Beller @ 2016-11-22 19:22 UTC (permalink / raw)
To: bmwill, gitster; +Cc: git, jrnieder, Jens.Lehmann, hvoigt, Stefan Beller
In-Reply-To: <20161122192235.6055-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>
---
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..806e29ce4e 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.4.g3396b6f.dirty
^ permalink raw reply related
* [PATCHv2 3/4] test-lib-functions.sh: teach test_commit -C <dir>
From: Stefan Beller @ 2016-11-22 19:22 UTC (permalink / raw)
To: bmwill, gitster; +Cc: git, jrnieder, Jens.Lehmann, hvoigt, Stefan Beller
In-Reply-To: <20161122192235.6055-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>
---
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.4.g3396b6f.dirty
^ permalink raw reply related
* [PATCHv2 1/4] submodule: use absolute path for computing relative path connecting
From: Stefan Beller @ 2016-11-22 19:22 UTC (permalink / raw)
To: bmwill, gitster; +Cc: git, jrnieder, Jens.Lehmann, hvoigt, Stefan Beller
In-Reply-To: <20161122192235.6055-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>
---
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.4.g3396b6f.dirty
^ permalink raw reply related
* Re: [PATCH] git-gui: pass the branch name to git merge
From: Junio C Hamano @ 2016-11-22 19:16 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Pat Thoyts, René Scharfe, Git Mailing List
In-Reply-To: <6a6dd0b9-436d-327f-c33d-e5cce078b3a0@kdbg.org>
Johannes Sixt <j6t@kdbg.org> writes:
> The recent rewrite of the 'git merge' invocation in b5f325cb
> (git-gui: stop using deprecated merge syntax) replaced the
> subsidiary call of 'git fmt-merge-msg' to take advantage of
> the capability of 'git merge' that can construct a merge log
> message when the rev being merged is FETCH_HEAD.
>
> A disadvantage of this method is, though, that the conflict
> markers are augmented with a raw SHA1 instead of a symbolic
> branch name.
Can't this be handled on the "git merge FETCH_HEAD" codepath
instead? The codepath already does enough "magic" things to
FETCH_HEAD like skipping 'not-for-merge' entries and also knows
about the original refnames the commits to be merged came from.
^ permalink raw reply
* Re: [PATCH 2/3] stripspace: respect repository config
From: Junio C Hamano @ 2016-11-22 19:10 UTC (permalink / raw)
To: Jeff King, René Scharfe
Cc: Johannes Schindelin, Duy Nguyen, Git Mailing List, Ralf Thielow,
Taufiq Hoven
In-Reply-To: <xmqqshqjqw3z.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> I think we want to audit the ones without RUN_SETUP* in the command
> table in order to hunt for regression aka "a fix revealed a bug that
> was covered by .git/config accidentally getting read when run from
> the top-level of the working tree", though. We may find unreported
> breakages that we may have to fix.
So I took a brief look at the "PROGRAM_OBJS += ..." in the Makefile
for non-builtin commands, and commands[] array in git.c for builtin
commands to see how bad it looks.
Archive & Upload-archive:
"cd Documentation && git archive --remote=origin" immediately hits
"BUG: setup_git_env called without repository" if your Git is built
with b1ef400eec ("setup_git_env: avoid blind fall-back to ".git"",
2016-10-20), which will not be part of the upcoming release. And
'origin' will probably not be understood from the local config.
I think we can do the "gently" thing there, as we may be retrieving
a remote archive outside a local repository. We'd need to tweak
"output" with prefix to compensate for the case in which the command
is run from a subdirectory, and probably we need to futz with the
setup_prefix parameter to write_archive(), as a local caller now
will know if we are in nongit situation.
On the upload-archive side that serves "--remote" request, there is
enter_repo() so we should be covered OK.
Mailinfo:
We may want a "gently" thing there to pick up local configuration.
i18n.commitencoding and mailinfo.scissors in local repository would
be ignored otherwise.
Splitspace:
Dscho fixed this one.
Verify-pack:
This calls git_config() but these days farms out its operation to
"index-pack", so we should be OK. We may even want to lose the call
to git_config() which does not affect anything.
^ permalink raw reply
* [PATCH v5 4/6] grep: optionally recurse into submodules
From: Brandon Williams @ 2016-11-22 18:46 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, jonathantanmy, gitster
In-Reply-To: <1479840397-68264-1-git-send-email-bmwill@google.com>
Allow grep to recognize submodules and recursively search for patterns in
each submodule. This is done by forking off a process to recursively
call grep on each submodule. The top level --super-prefix option is
used to pass a path to the submodule which can in turn be used to
prepend to output or in pathspec matching logic.
Recursion only occurs for submodules which have been initialized and
checked out by the parent project. If a submodule hasn't been
initialized and checked out it is simply skipped.
In order to support the existing multi-threading infrastructure in grep,
output from each child process is captured in a strbuf so that it can be
later printed to the console in an ordered fashion.
To limit the number of theads that are created, each child process has
half the number of threads as its parents (minimum of 1), otherwise we
potentailly have a fork-bomb.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
Documentation/git-grep.txt | 5 +
builtin/grep.c | 300 ++++++++++++++++++++++++++++++++++---
git.c | 2 +-
t/t7814-grep-recurse-submodules.sh | 99 ++++++++++++
4 files changed, 386 insertions(+), 20 deletions(-)
create mode 100755 t/t7814-grep-recurse-submodules.sh
diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 0ecea6e..17aa1ba 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -26,6 +26,7 @@ SYNOPSIS
[--threads <num>]
[-f <file>] [-e] <pattern>
[--and|--or|--not|(|)|-e <pattern>...]
+ [--recurse-submodules]
[ [--[no-]exclude-standard] [--cached | --no-index | --untracked] | <tree>...]
[--] [<pathspec>...]
@@ -88,6 +89,10 @@ OPTIONS
mechanism. Only useful when searching files in the current directory
with `--no-index`.
+--recurse-submodules::
+ Recursively search in each submodule that has been initialized and
+ checked out in the repository.
+
-a::
--text::
Process binary files as if they were text.
diff --git a/builtin/grep.c b/builtin/grep.c
index 8887b6a..dca0be6 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -18,12 +18,20 @@
#include "quote.h"
#include "dir.h"
#include "pathspec.h"
+#include "submodule.h"
static char const * const grep_usage[] = {
N_("git grep [<options>] [-e] <pattern> [<rev>...] [[--] <path>...]"),
NULL
};
+static const char *super_prefix;
+static int recurse_submodules;
+static struct argv_array submodule_options = ARGV_ARRAY_INIT;
+
+static int grep_submodule_launch(struct grep_opt *opt,
+ const struct grep_source *gs);
+
#define GREP_NUM_THREADS_DEFAULT 8
static int num_threads;
@@ -174,7 +182,10 @@ static void *run(void *arg)
break;
opt->output_priv = w;
- hit |= grep_source(opt, &w->source);
+ if (w->source.type == GREP_SOURCE_SUBMODULE)
+ hit |= grep_submodule_launch(opt, &w->source);
+ else
+ hit |= grep_source(opt, &w->source);
grep_source_clear_data(&w->source);
work_done(w);
}
@@ -300,6 +311,10 @@ static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,
if (opt->relative && opt->prefix_length) {
quote_path_relative(filename + tree_name_len, opt->prefix, &pathbuf);
strbuf_insert(&pathbuf, 0, filename, tree_name_len);
+ } else if (super_prefix) {
+ strbuf_add(&pathbuf, filename, tree_name_len);
+ strbuf_addstr(&pathbuf, super_prefix);
+ strbuf_addstr(&pathbuf, filename + tree_name_len);
} else {
strbuf_addstr(&pathbuf, filename);
}
@@ -328,10 +343,13 @@ static int grep_file(struct grep_opt *opt, const char *filename)
{
struct strbuf buf = STRBUF_INIT;
- if (opt->relative && opt->prefix_length)
+ if (opt->relative && opt->prefix_length) {
quote_path_relative(filename, opt->prefix, &buf);
- else
+ } else {
+ if (super_prefix)
+ strbuf_addstr(&buf, super_prefix);
strbuf_addstr(&buf, filename);
+ }
#ifndef NO_PTHREADS
if (num_threads) {
@@ -378,31 +396,260 @@ static void run_pager(struct grep_opt *opt, const char *prefix)
exit(status);
}
-static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec, int cached)
+static void compile_submodule_options(const struct grep_opt *opt,
+ const struct pathspec *pathspec,
+ int cached, int untracked,
+ int opt_exclude, int use_index,
+ int pattern_type_arg)
+{
+ struct grep_pat *pattern;
+ int i;
+
+ if (recurse_submodules)
+ argv_array_push(&submodule_options, "--recurse-submodules");
+
+ if (cached)
+ argv_array_push(&submodule_options, "--cached");
+ if (!use_index)
+ argv_array_push(&submodule_options, "--no-index");
+ if (untracked)
+ argv_array_push(&submodule_options, "--untracked");
+ if (opt_exclude > 0)
+ argv_array_push(&submodule_options, "--exclude-standard");
+
+ if (opt->invert)
+ argv_array_push(&submodule_options, "-v");
+ if (opt->ignore_case)
+ argv_array_push(&submodule_options, "-i");
+ if (opt->word_regexp)
+ argv_array_push(&submodule_options, "-w");
+ switch (opt->binary) {
+ case GREP_BINARY_NOMATCH:
+ argv_array_push(&submodule_options, "-I");
+ break;
+ case GREP_BINARY_TEXT:
+ argv_array_push(&submodule_options, "-a");
+ break;
+ default:
+ break;
+ }
+ if (opt->allow_textconv)
+ argv_array_push(&submodule_options, "--textconv");
+ if (opt->max_depth != -1)
+ argv_array_pushf(&submodule_options, "--max-depth=%d",
+ opt->max_depth);
+ if (opt->linenum)
+ argv_array_push(&submodule_options, "-n");
+ if (!opt->pathname)
+ argv_array_push(&submodule_options, "-h");
+ if (!opt->relative)
+ argv_array_push(&submodule_options, "--full-name");
+ if (opt->name_only)
+ argv_array_push(&submodule_options, "-l");
+ if (opt->unmatch_name_only)
+ argv_array_push(&submodule_options, "-L");
+ if (opt->null_following_name)
+ argv_array_push(&submodule_options, "-z");
+ if (opt->count)
+ argv_array_push(&submodule_options, "-c");
+ if (opt->file_break)
+ argv_array_push(&submodule_options, "--break");
+ if (opt->heading)
+ argv_array_push(&submodule_options, "--heading");
+ if (opt->pre_context)
+ argv_array_pushf(&submodule_options, "--before-context=%d",
+ opt->pre_context);
+ if (opt->post_context)
+ argv_array_pushf(&submodule_options, "--after-context=%d",
+ opt->post_context);
+ if (opt->funcname)
+ argv_array_push(&submodule_options, "-p");
+ if (opt->funcbody)
+ argv_array_push(&submodule_options, "-W");
+ if (opt->all_match)
+ argv_array_push(&submodule_options, "--all-match");
+ if (opt->debug)
+ argv_array_push(&submodule_options, "--debug");
+ if (opt->status_only)
+ argv_array_push(&submodule_options, "-q");
+
+ switch (pattern_type_arg) {
+ case GREP_PATTERN_TYPE_BRE:
+ argv_array_push(&submodule_options, "-G");
+ break;
+ case GREP_PATTERN_TYPE_ERE:
+ argv_array_push(&submodule_options, "-E");
+ break;
+ case GREP_PATTERN_TYPE_FIXED:
+ argv_array_push(&submodule_options, "-F");
+ break;
+ case GREP_PATTERN_TYPE_PCRE:
+ argv_array_push(&submodule_options, "-P");
+ break;
+ case GREP_PATTERN_TYPE_UNSPECIFIED:
+ break;
+ }
+
+ for (pattern = opt->pattern_list; pattern != NULL;
+ pattern = pattern->next) {
+ switch (pattern->token) {
+ case GREP_PATTERN:
+ argv_array_pushf(&submodule_options, "-e%s",
+ pattern->pattern);
+ break;
+ case GREP_AND:
+ case GREP_OPEN_PAREN:
+ case GREP_CLOSE_PAREN:
+ case GREP_NOT:
+ case GREP_OR:
+ argv_array_push(&submodule_options, pattern->pattern);
+ break;
+ /* BODY and HEAD are not used by git-grep */
+ case GREP_PATTERN_BODY:
+ case GREP_PATTERN_HEAD:
+ break;
+ }
+ }
+
+ /*
+ * Limit number of threads for child process to use.
+ * This is to prevent potential fork-bomb behavior of git-grep as each
+ * submodule process has its own thread pool.
+ */
+ argv_array_pushf(&submodule_options, "--threads=%d",
+ (num_threads + 1) / 2);
+
+ /* Add Pathspecs */
+ argv_array_push(&submodule_options, "--");
+ for (i = 0; i < pathspec->nr; i++)
+ argv_array_push(&submodule_options,
+ pathspec->items[i].original);
+}
+
+/*
+ * Launch child process to grep contents of a submodule
+ */
+static int grep_submodule_launch(struct grep_opt *opt,
+ const struct grep_source *gs)
+{
+ struct child_process cp = CHILD_PROCESS_INIT;
+ int status, i;
+ struct work_item *w = opt->output_priv;
+
+ prepare_submodule_repo_env(&cp.env_array);
+
+ /* Add super prefix */
+ argv_array_pushf(&cp.args, "--super-prefix=%s%s/",
+ super_prefix ? super_prefix : "",
+ gs->name);
+ argv_array_push(&cp.args, "grep");
+
+ /* Add options */
+ for (i = 0; i < submodule_options.argc; i++)
+ argv_array_push(&cp.args, submodule_options.argv[i]);
+
+ cp.git_cmd = 1;
+ cp.dir = gs->path;
+
+ /*
+ * Capture output to output buffer and check the return code from the
+ * child process. A '0' indicates a hit, a '1' indicates no hit and
+ * anything else is an error.
+ */
+ status = capture_command(&cp, &w->out, 0);
+ if (status && (status != 1)) {
+ /* flush the buffer */
+ write_or_die(1, w->out.buf, w->out.len);
+ die("process for submodule '%s' failed with exit code: %d",
+ gs->name, status);
+ }
+
+ /* invert the return code to make a hit equal to 1 */
+ return !status;
+}
+
+/*
+ * Prep grep structures for a submodule grep
+ * sha1: the sha1 of the submodule or NULL if using the working tree
+ * filename: name of the submodule including tree name of parent
+ * path: location of the submodule
+ */
+static int grep_submodule(struct grep_opt *opt, const unsigned char *sha1,
+ const char *filename, const char *path)
+{
+ if (!is_submodule_initialized(path))
+ return 0;
+ if (!is_submodule_populated(path))
+ return 0;
+
+#ifndef NO_PTHREADS
+ if (num_threads) {
+ add_work(opt, GREP_SOURCE_SUBMODULE, filename, path, sha1);
+ return 0;
+ } else
+#endif
+ {
+ struct work_item w;
+ int hit;
+
+ grep_source_init(&w.source, GREP_SOURCE_SUBMODULE,
+ filename, path, sha1);
+ strbuf_init(&w.out, 0);
+ opt->output_priv = &w;
+ hit = grep_submodule_launch(opt, &w.source);
+
+ write_or_die(1, w.out.buf, w.out.len);
+
+ grep_source_clear(&w.source);
+ strbuf_release(&w.out);
+ return hit;
+ }
+}
+
+static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec,
+ int cached)
{
int hit = 0;
int nr;
+ struct strbuf name = STRBUF_INIT;
+ int name_base_len = 0;
+ if (super_prefix) {
+ name_base_len = strlen(super_prefix);
+ strbuf_addstr(&name, super_prefix);
+ }
+
read_cache();
for (nr = 0; nr < active_nr; nr++) {
const struct cache_entry *ce = active_cache[nr];
- if (!S_ISREG(ce->ce_mode))
- continue;
- if (!ce_path_match(ce, pathspec, NULL))
+ strbuf_setlen(&name, name_base_len);
+ strbuf_addstr(&name, ce->name);
+
+ if (S_ISREG(ce->ce_mode) &&
+ match_pathspec(pathspec, name.buf, name.len, 0, NULL,
+ S_ISDIR(ce->ce_mode) ||
+ S_ISGITLINK(ce->ce_mode))) {
+ /*
+ * If CE_VALID is on, we assume worktree file and its
+ * cache entry are identical, even if worktree file has
+ * been modified, so use cache version instead
+ */
+ if (cached || (ce->ce_flags & CE_VALID) ||
+ ce_skip_worktree(ce)) {
+ if (ce_stage(ce) || ce_intent_to_add(ce))
+ continue;
+ hit |= grep_sha1(opt, ce->oid.hash, ce->name,
+ 0, ce->name);
+ } else {
+ hit |= grep_file(opt, ce->name);
+ }
+ } else if (recurse_submodules && S_ISGITLINK(ce->ce_mode) &&
+ submodule_path_match(pathspec, name.buf, NULL)) {
+ hit |= grep_submodule(opt, NULL, ce->name, ce->name);
+ } else {
continue;
- /*
- * If CE_VALID is on, we assume worktree file and its cache entry
- * are identical, even if worktree file has been modified, so use
- * cache version instead
- */
- if (cached || (ce->ce_flags & CE_VALID) || ce_skip_worktree(ce)) {
- if (ce_stage(ce) || ce_intent_to_add(ce))
- continue;
- hit |= grep_sha1(opt, ce->oid.hash, ce->name, 0,
- ce->name);
}
- else
- hit |= grep_file(opt, ce->name);
+
if (ce_stage(ce)) {
do {
nr++;
@@ -413,6 +660,8 @@ static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec, int
if (hit && opt->status_only)
break;
}
+
+ strbuf_release(&name);
return hit;
}
@@ -651,6 +900,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
N_("search in both tracked and untracked files")),
OPT_SET_INT(0, "exclude-standard", &opt_exclude,
N_("ignore files specified via '.gitignore'"), 1),
+ OPT_BOOL(0, "recurse-submodules", &recurse_submodules,
+ N_("recursivley search in each submodule")),
OPT_GROUP(""),
OPT_BOOL('v', "invert-match", &opt.invert,
N_("show non-matching lines")),
@@ -755,6 +1006,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
init_grep_defaults();
git_config(grep_cmd_config, NULL);
grep_init(&opt, prefix);
+ super_prefix = get_super_prefix();
/*
* If there is no -- then the paths must exist in the working
@@ -872,6 +1124,13 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
pathspec.max_depth = opt.max_depth;
pathspec.recursive = 1;
+ if (recurse_submodules) {
+ gitmodules_config();
+ compile_submodule_options(&opt, &pathspec, cached, untracked,
+ opt_exclude, use_index,
+ pattern_type_arg);
+ }
+
if (show_in_pager && (cached || list.nr))
die(_("--open-files-in-pager only works on the worktree"));
@@ -895,6 +1154,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
}
}
+ if (recurse_submodules && (!use_index || untracked || list.nr))
+ die(_("option not supported with --recurse-submodules."));
+
if (!show_in_pager && !opt.status_only)
setup_pager();
diff --git a/git.c b/git.c
index efa1059..a156efd 100644
--- a/git.c
+++ b/git.c
@@ -434,7 +434,7 @@ static struct cmd_struct commands[] = {
{ "fsck-objects", cmd_fsck, RUN_SETUP },
{ "gc", cmd_gc, RUN_SETUP },
{ "get-tar-commit-id", cmd_get_tar_commit_id },
- { "grep", cmd_grep, RUN_SETUP_GENTLY },
+ { "grep", cmd_grep, RUN_SETUP_GENTLY | SUPPORT_SUPER_PREFIX },
{ "hash-object", cmd_hash_object },
{ "help", cmd_help },
{ "index-pack", cmd_index_pack, RUN_SETUP_GENTLY },
diff --git a/t/t7814-grep-recurse-submodules.sh b/t/t7814-grep-recurse-submodules.sh
new file mode 100755
index 0000000..1019125
--- /dev/null
+++ b/t/t7814-grep-recurse-submodules.sh
@@ -0,0 +1,99 @@
+#!/bin/sh
+
+test_description='Test grep recurse-submodules feature
+
+This test verifies the recurse-submodules feature correctly greps across
+submodules.
+'
+
+. ./test-lib.sh
+
+test_expect_success 'setup directory structure and submodule' '
+ echo "foobar" >a &&
+ mkdir b &&
+ echo "bar" >b/b &&
+ git add a b &&
+ git commit -m "add a and b" &&
+ git init submodule &&
+ echo "foobar" >submodule/a &&
+ git -C submodule add a &&
+ git -C submodule commit -m "add a" &&
+ git submodule add ./submodule &&
+ git commit -m "added submodule"
+'
+
+test_expect_success 'grep correctly finds patterns in a submodule' '
+ cat >expect <<-\EOF &&
+ a:foobar
+ b/b:bar
+ submodule/a:foobar
+ EOF
+
+ git grep -e "bar" --recurse-submodules >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'grep and basic pathspecs' '
+ cat >expect <<-\EOF &&
+ submodule/a:foobar
+ EOF
+
+ git grep -e. --recurse-submodules -- submodule >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'grep and nested submodules' '
+ git init submodule/sub &&
+ echo "foobar" >submodule/sub/a &&
+ git -C submodule/sub add a &&
+ git -C submodule/sub commit -m "add a" &&
+ git -C submodule submodule add ./sub &&
+ git -C submodule add sub &&
+ git -C submodule commit -m "added sub" &&
+ git add submodule &&
+ git commit -m "updated submodule" &&
+
+ cat >expect <<-\EOF &&
+ a:foobar
+ b/b:bar
+ submodule/a:foobar
+ submodule/sub/a:foobar
+ EOF
+
+ git grep -e "bar" --recurse-submodules >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'grep and multiple patterns' '
+ cat >expect <<-\EOF &&
+ a:foobar
+ submodule/a:foobar
+ submodule/sub/a:foobar
+ EOF
+
+ git grep -e "bar" --and -e "foo" --recurse-submodules >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'grep and multiple patterns' '
+ cat >expect <<-\EOF &&
+ b/b:bar
+ EOF
+
+ git grep -e "bar" --and --not -e "foo" --recurse-submodules >actual &&
+ test_cmp expect actual
+'
+
+test_incompatible_with_recurse_submodules ()
+{
+ test_expect_success "--recurse-submodules and $1 are incompatible" "
+ test_must_fail git grep -e. --recurse-submodules $1 2>actual &&
+ test_i18ngrep 'not supported with --recurse-submodules' actual
+ "
+}
+
+test_incompatible_with_recurse_submodules --untracked
+test_incompatible_with_recurse_submodules --no-index
+test_incompatible_with_recurse_submodules HEAD
+
+test_done
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v5 0/6] recursively grep across submodules
From: Brandon Williams @ 2016-11-22 18:46 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, jonathantanmy, gitster
In-Reply-To: <1479499135-64269-1-git-send-email-bmwill@google.com>
Major change in v5 is to use tree_is_interesting api instead of the vanilla
pathspec code for submodules. This is to fix the issue in the last seires
where I mix the two types. More tests were also added to ensure that the
changes to the pathspec code functioned properly.
Brandon Williams (6):
submodules: add helper functions to determine presence of submodules
submodules: load gitmodules file from commit sha1
grep: add submodules as a grep source type
grep: optionally recurse into submodules
grep: enable recurse-submodules to work on <tree> objects
grep: search history of moved submodules
Documentation/git-grep.txt | 14 ++
builtin/grep.c | 386 ++++++++++++++++++++++++++++++++++---
cache.h | 2 +
config.c | 8 +-
git.c | 2 +-
grep.c | 16 +-
grep.h | 1 +
submodule-config.c | 6 +-
submodule-config.h | 3 +
submodule.c | 50 +++++
submodule.h | 3 +
t/t7814-grep-recurse-submodules.sh | 241 +++++++++++++++++++++++
tree-walk.c | 28 +++
13 files changed, 729 insertions(+), 31 deletions(-)
create mode 100755 t/t7814-grep-recurse-submodules.sh
-- interdiff based on 'bw/grep-recurse-submodules'
diff --git a/builtin/grep.c b/builtin/grep.c
index 052f605..2c727ef 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -698,6 +698,8 @@ static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec,
} else if (recurse_submodules && S_ISGITLINK(ce->ce_mode) &&
submodule_path_match(pathspec, name.buf, NULL)) {
hit |= grep_submodule(opt, NULL, ce->name, ce->name);
+ } else {
+ continue;
}
if (ce_stage(ce)) {
@@ -734,17 +736,10 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
int te_len = tree_entry_len(&entry);
if (match != all_entries_interesting) {
- strbuf_setlen(&name, name_base_len);
strbuf_addstr(&name, base->buf + tn_len);
-
- if (recurse_submodules && S_ISGITLINK(entry.mode)) {
- strbuf_addstr(&name, entry.path);
- match = submodule_path_match(pathspec, name.buf,
- NULL);
- } else {
- match = tree_entry_interesting(&entry, &name,
- 0, pathspec);
- }
+ match = tree_entry_interesting(&entry, &name,
+ 0, pathspec);
+ strbuf_setlen(&name, name_base_len);
if (match == all_entries_not_interesting)
break;
diff --git a/t/t7814-grep-recurse-submodules.sh b/t/t7814-grep-recurse-submodules.sh
index 7d66716..0507771 100755
--- a/t/t7814-grep-recurse-submodules.sh
+++ b/t/t7814-grep-recurse-submodules.sh
@@ -127,6 +127,34 @@ test_expect_success 'grep tree and pathspecs' '
test_cmp expect actual
'
+test_expect_success 'grep tree and pathspecs' '
+ cat >expect <<-\EOF &&
+ HEAD:submodule/a:foobar
+ HEAD:submodule/sub/a:foobar
+ EOF
+
+ git grep -e "bar" --recurse-submodules HEAD -- "submodule*a" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'grep tree and more pathspecs' '
+ cat >expect <<-\EOF &&
+ HEAD:submodule/a:foobar
+ EOF
+
+ git grep -e "bar" --recurse-submodules HEAD -- "submodul?/a" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'grep tree and more pathspecs' '
+ cat >expect <<-\EOF &&
+ HEAD:submodule/sub/a:foobar
+ EOF
+
+ git grep -e "bar" --recurse-submodules HEAD -- "submodul*/sub/a" >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'grep recurse submodule colon in name' '
git init parent &&
test_when_finished "rm -rf parent" &&
diff --git a/tree-walk.c b/tree-walk.c
index 828f435..ff77605 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -1004,6 +1004,19 @@ static enum interesting do_match(const struct name_entry *entry,
*/
if (ps->recursive && S_ISDIR(entry->mode))
return entry_interesting;
+
+ /*
+ * When matching against submodules with
+ * wildcard characters, ensure that the entry
+ * at least matches up to the first wild
+ * character. More accurate matching can then
+ * be performed in the submodule itself.
+ */
+ if (ps->recursive && S_ISGITLINK(entry->mode) &&
+ !ps_strncmp(item, match + baselen,
+ entry->path,
+ item->nowildcard_len - baselen))
+ return entry_interesting;
}
continue;
@@ -1040,6 +1053,21 @@ static enum interesting do_match(const struct name_entry *entry,
strbuf_setlen(base, base_offset + baselen);
return entry_interesting;
}
+
+ /*
+ * When matching against submodules with
+ * wildcard characters, ensure that the entry
+ * at least matches up to the first wild
+ * character. More accurate matching can then
+ * be performed in the submodule itself.
+ */
+ if (ps->recursive && S_ISGITLINK(entry->mode) &&
+ !ps_strncmp(item, match, base->buf + base_offset,
+ item->nowildcard_len)) {
+ strbuf_setlen(base, base_offset + baselen);
+ return entry_interesting;
+ }
+
strbuf_setlen(base, base_offset + baselen);
/*
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v5 2/6] submodules: load gitmodules file from commit sha1
From: Brandon Williams @ 2016-11-22 18:46 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, jonathantanmy, gitster
In-Reply-To: <1479840397-68264-1-git-send-email-bmwill@google.com>
teach submodules to load a '.gitmodules' file from a commit sha1. This
enables the population of the submodule_cache to be based on the state
of the '.gitmodules' file from a particular commit.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
cache.h | 2 ++
config.c | 8 ++++----
submodule-config.c | 6 +++---
submodule-config.h | 3 +++
submodule.c | 12 ++++++++++++
submodule.h | 1 +
6 files changed, 25 insertions(+), 7 deletions(-)
diff --git a/cache.h b/cache.h
index 1be6526..559a461 100644
--- a/cache.h
+++ b/cache.h
@@ -1690,6 +1690,8 @@ extern int git_default_config(const char *, const char *, void *);
extern int git_config_from_file(config_fn_t fn, const char *, void *);
extern int git_config_from_mem(config_fn_t fn, const enum config_origin_type,
const char *name, const char *buf, size_t len, void *data);
+extern int git_config_from_blob_sha1(config_fn_t fn, const char *name,
+ const unsigned char *sha1, void *data);
extern void git_config_push_parameter(const char *text);
extern int git_config_from_parameters(config_fn_t fn, void *data);
extern void git_config(config_fn_t fn, void *);
diff --git a/config.c b/config.c
index 83fdecb..4d78e72 100644
--- a/config.c
+++ b/config.c
@@ -1214,10 +1214,10 @@ int git_config_from_mem(config_fn_t fn, const enum config_origin_type origin_typ
return do_config_from(&top, fn, data);
}
-static int git_config_from_blob_sha1(config_fn_t fn,
- const char *name,
- const unsigned char *sha1,
- void *data)
+int git_config_from_blob_sha1(config_fn_t fn,
+ const char *name,
+ const unsigned char *sha1,
+ void *data)
{
enum object_type type;
char *buf;
diff --git a/submodule-config.c b/submodule-config.c
index 098085b..8b9a2ef 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -379,9 +379,9 @@ static int parse_config(const char *var, const char *value, void *data)
return ret;
}
-static int gitmodule_sha1_from_commit(const unsigned char *commit_sha1,
- unsigned char *gitmodules_sha1,
- struct strbuf *rev)
+int gitmodule_sha1_from_commit(const unsigned char *commit_sha1,
+ unsigned char *gitmodules_sha1,
+ struct strbuf *rev)
{
int ret = 0;
diff --git a/submodule-config.h b/submodule-config.h
index d05c542..78584ba 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -29,6 +29,9 @@ const struct submodule *submodule_from_name(const unsigned char *commit_sha1,
const char *name);
const struct submodule *submodule_from_path(const unsigned char *commit_sha1,
const char *path);
+extern int gitmodule_sha1_from_commit(const unsigned char *commit_sha1,
+ unsigned char *gitmodules_sha1,
+ struct strbuf *rev);
void submodule_free(void);
#endif /* SUBMODULE_CONFIG_H */
diff --git a/submodule.c b/submodule.c
index f5107f0..062e58b 100644
--- a/submodule.c
+++ b/submodule.c
@@ -198,6 +198,18 @@ void gitmodules_config(void)
}
}
+void gitmodules_config_sha1(const unsigned char *commit_sha1)
+{
+ struct strbuf rev = STRBUF_INIT;
+ unsigned char sha1[20];
+
+ if (gitmodule_sha1_from_commit(commit_sha1, sha1, &rev)) {
+ git_config_from_blob_sha1(submodule_config, rev.buf,
+ sha1, NULL);
+ }
+ strbuf_release(&rev);
+}
+
/*
* Determine if a submodule has been initialized at a given 'path'
*/
diff --git a/submodule.h b/submodule.h
index 6ec5f2f..9203d89 100644
--- a/submodule.h
+++ b/submodule.h
@@ -37,6 +37,7 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
const char *path);
int submodule_config(const char *var, const char *value, void *cb);
void gitmodules_config(void);
+extern void gitmodules_config_sha1(const unsigned char *commit_sha1);
extern int is_submodule_initialized(const char *path);
extern int is_submodule_populated(const char *path);
int parse_submodule_update_strategy(const char *value,
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v5 6/6] grep: search history of moved submodules
From: Brandon Williams @ 2016-11-22 18:46 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, jonathantanmy, gitster
In-Reply-To: <1479840397-68264-1-git-send-email-bmwill@google.com>
If a submodule was renamed at any point since it's inception then if you
were to try and grep on a commit prior to the submodule being moved, you
wouldn't be able to find a working directory for the submodule since the
path in the past is different from the current path.
This patch teaches grep to find the .git directory for a submodule in
the parents .git/modules/ directory in the event the path to the
submodule in the commit that is being searched differs from the state of
the currently checked out commit. If found, the child process that is
spawned to grep the submodule will chdir into its gitdir instead of a
working directory.
In order to override the explicit setting of submodule child process's
gitdir environment variable (which was introduced in '10f5c526')
`GIT_DIR_ENVIORMENT` needs to be pushed onto child process's env_array.
This allows the searching of history from a submodule's gitdir, rather
than from a working directory.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
builtin/grep.c | 20 +++++++++++++++++--
t/t7814-grep-recurse-submodules.sh | 41 ++++++++++++++++++++++++++++++++++++++
2 files changed, 59 insertions(+), 2 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 5918a26..2c727ef 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -547,6 +547,7 @@ static int grep_submodule_launch(struct grep_opt *opt,
name = gs->name;
prepare_submodule_repo_env(&cp.env_array);
+ argv_array_push(&cp.env_array, GIT_DIR_ENVIRONMENT);
/* Add super prefix */
argv_array_pushf(&cp.args, "--super-prefix=%s%s/",
@@ -615,8 +616,23 @@ static int grep_submodule(struct grep_opt *opt, const unsigned char *sha1,
{
if (!is_submodule_initialized(path))
return 0;
- if (!is_submodule_populated(path))
- return 0;
+ if (!is_submodule_populated(path)) {
+ /*
+ * If searching history, check for the presense of the
+ * submodule's gitdir before skipping the submodule.
+ */
+ if (sha1) {
+ const struct submodule *sub =
+ submodule_from_path(null_sha1, path);
+ if (sub)
+ path = git_path("modules/%s", sub->name);
+
+ if (!(is_directory(path) && is_git_directory(path)))
+ return 0;
+ } else {
+ return 0;
+ }
+ }
#ifndef NO_PTHREADS
if (num_threads) {
diff --git a/t/t7814-grep-recurse-submodules.sh b/t/t7814-grep-recurse-submodules.sh
index 9e93fe7..0507771 100755
--- a/t/t7814-grep-recurse-submodules.sh
+++ b/t/t7814-grep-recurse-submodules.sh
@@ -186,6 +186,47 @@ test_expect_success 'grep recurse submodule colon in name' '
test_cmp expect actual
'
+test_expect_success 'grep history with moved submoules' '
+ git init parent &&
+ test_when_finished "rm -rf parent" &&
+ echo "foobar" >parent/file &&
+ git -C parent add file &&
+ git -C parent commit -m "add file" &&
+
+ git init sub &&
+ test_when_finished "rm -rf sub" &&
+ echo "foobar" >sub/file &&
+ git -C sub add file &&
+ git -C sub commit -m "add file" &&
+
+ git -C parent submodule add ../sub dir/sub &&
+ git -C parent commit -m "add submodule" &&
+
+ cat >expect <<-\EOF &&
+ dir/sub/file:foobar
+ file:foobar
+ EOF
+ git -C parent grep -e "foobar" --recurse-submodules >actual &&
+ test_cmp expect actual &&
+
+ git -C parent mv dir/sub sub-moved &&
+ git -C parent commit -m "moved submodule" &&
+
+ cat >expect <<-\EOF &&
+ file:foobar
+ sub-moved/file:foobar
+ EOF
+ git -C parent grep -e "foobar" --recurse-submodules >actual &&
+ test_cmp expect actual &&
+
+ cat >expect <<-\EOF &&
+ HEAD^:dir/sub/file:foobar
+ HEAD^:file:foobar
+ EOF
+ git -C parent grep -e "foobar" --recurse-submodules HEAD^ >actual &&
+ test_cmp expect actual
+'
+
test_incompatible_with_recurse_submodules ()
{
test_expect_success "--recurse-submodules and $1 are incompatible" "
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v5 5/6] grep: enable recurse-submodules to work on <tree> objects
From: Brandon Williams @ 2016-11-22 18:46 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, jonathantanmy, gitster
In-Reply-To: <1479840397-68264-1-git-send-email-bmwill@google.com>
Teach grep to recursively search in submodules when provided with a
<tree> object. This allows grep to search a submodule based on the state
of the submodule that is present in a commit of the super project.
When grep is provided with a <tree> object, the name of the object is
prefixed to all output. In order to provide uniformity of output
between the parent and child processes the option `--parent-basename`
has been added so that the child can preface all of it's output with the
name of the parent's object instead of the name of the commit SHA1 of
the submodule. This changes output from the command
`git grep -e. -l --recurse-submodules HEAD`
from:
HEAD:file
<commit sha1 of submodule>:sub/file
to:
HEAD:file
HEAD:sub/file
Signed-off-by: Brandon Williams <bmwill@google.com>
---
Documentation/git-grep.txt | 13 ++++-
builtin/grep.c | 76 ++++++++++++++++++++++++---
t/t7814-grep-recurse-submodules.sh | 103 ++++++++++++++++++++++++++++++++++++-
tree-walk.c | 28 ++++++++++
4 files changed, 211 insertions(+), 9 deletions(-)
diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 17aa1ba..71f32f3 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -26,7 +26,7 @@ SYNOPSIS
[--threads <num>]
[-f <file>] [-e] <pattern>
[--and|--or|--not|(|)|-e <pattern>...]
- [--recurse-submodules]
+ [--recurse-submodules] [--parent-basename <basename>]
[ [--[no-]exclude-standard] [--cached | --no-index | --untracked] | <tree>...]
[--] [<pathspec>...]
@@ -91,7 +91,16 @@ OPTIONS
--recurse-submodules::
Recursively search in each submodule that has been initialized and
- checked out in the repository.
+ checked out in the repository. When used in combination with the
+ <tree> option the prefix of all submodule output will be the name of
+ the parent project's <tree> object.
+
+--parent-basename <basename>::
+ For internal use only. In order to produce uniform output with the
+ --recurse-submodules option, this option can be used to provide the
+ basename of a parent's <tree> object to a submodule so the submodule
+ can prefix its output with the parent's name rather than the SHA1 of
+ the submodule.
-a::
--text::
diff --git a/builtin/grep.c b/builtin/grep.c
index dca0be6..5918a26 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -19,6 +19,7 @@
#include "dir.h"
#include "pathspec.h"
#include "submodule.h"
+#include "submodule-config.h"
static char const * const grep_usage[] = {
N_("git grep [<options>] [-e] <pattern> [<rev>...] [[--] <path>...]"),
@@ -28,6 +29,7 @@ static char const * const grep_usage[] = {
static const char *super_prefix;
static int recurse_submodules;
static struct argv_array submodule_options = ARGV_ARRAY_INIT;
+static const char *parent_basename;
static int grep_submodule_launch(struct grep_opt *opt,
const struct grep_source *gs);
@@ -534,19 +536,53 @@ static int grep_submodule_launch(struct grep_opt *opt,
{
struct child_process cp = CHILD_PROCESS_INIT;
int status, i;
+ const char *end_of_base;
+ const char *name;
struct work_item *w = opt->output_priv;
+ end_of_base = strchr(gs->name, ':');
+ if (gs->identifier && end_of_base)
+ name = end_of_base + 1;
+ else
+ name = gs->name;
+
prepare_submodule_repo_env(&cp.env_array);
/* Add super prefix */
argv_array_pushf(&cp.args, "--super-prefix=%s%s/",
super_prefix ? super_prefix : "",
- gs->name);
+ name);
argv_array_push(&cp.args, "grep");
+ /*
+ * Add basename of parent project
+ * When performing grep on a tree object the filename is prefixed
+ * with the object's name: 'tree-name:filename'. In order to
+ * provide uniformity of output we want to pass the name of the
+ * parent project's object name to the submodule so the submodule can
+ * prefix its output with the parent's name and not its own SHA1.
+ */
+ if (gs->identifier && end_of_base)
+ argv_array_pushf(&cp.args, "--parent-basename=%.*s",
+ (int) (end_of_base - gs->name),
+ gs->name);
+
/* Add options */
- for (i = 0; i < submodule_options.argc; i++)
+ for (i = 0; i < submodule_options.argc; i++) {
+ /*
+ * If there is a tree identifier for the submodule, add the
+ * rev after adding the submodule options but before the
+ * pathspecs. To do this we listen for the '--' and insert the
+ * sha1 before pushing the '--' onto the child process argv
+ * array.
+ */
+ if (gs->identifier &&
+ !strcmp("--", submodule_options.argv[i])) {
+ argv_array_push(&cp.args, sha1_to_hex(gs->identifier));
+ }
+
argv_array_push(&cp.args, submodule_options.argv[i]);
+ }
cp.git_cmd = 1;
cp.dir = gs->path;
@@ -673,12 +709,22 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
enum interesting match = entry_not_interesting;
struct name_entry entry;
int old_baselen = base->len;
+ struct strbuf name = STRBUF_INIT;
+ int name_base_len = 0;
+ if (super_prefix) {
+ strbuf_addstr(&name, super_prefix);
+ name_base_len = name.len;
+ }
while (tree_entry(tree, &entry)) {
int te_len = tree_entry_len(&entry);
if (match != all_entries_interesting) {
- match = tree_entry_interesting(&entry, base, tn_len, pathspec);
+ strbuf_addstr(&name, base->buf + tn_len);
+ match = tree_entry_interesting(&entry, &name,
+ 0, pathspec);
+ strbuf_setlen(&name, name_base_len);
+
if (match == all_entries_not_interesting)
break;
if (match == entry_not_interesting)
@@ -690,8 +736,7 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
if (S_ISREG(entry.mode)) {
hit |= grep_sha1(opt, entry.oid->hash, base->buf, tn_len,
check_attr ? base->buf + tn_len : NULL);
- }
- else if (S_ISDIR(entry.mode)) {
+ } else if (S_ISDIR(entry.mode)) {
enum object_type type;
struct tree_desc sub;
void *data;
@@ -707,12 +752,18 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
hit |= grep_tree(opt, pathspec, &sub, base, tn_len,
check_attr);
free(data);
+ } else if (recurse_submodules && S_ISGITLINK(entry.mode)) {
+ hit |= grep_submodule(opt, entry.oid->hash, base->buf,
+ base->buf + tn_len);
}
+
strbuf_setlen(base, old_baselen);
if (hit && opt->status_only)
break;
}
+
+ strbuf_release(&name);
return hit;
}
@@ -736,6 +787,10 @@ static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,
if (!data)
die(_("unable to read tree (%s)"), oid_to_hex(&obj->oid));
+ /* Use parent's name as base when recursing submodules */
+ if (recurse_submodules && parent_basename)
+ name = parent_basename;
+
len = name ? strlen(name) : 0;
strbuf_init(&base, PATH_MAX + len + 1);
if (len) {
@@ -762,6 +817,12 @@ static int grep_objects(struct grep_opt *opt, const struct pathspec *pathspec,
for (i = 0; i < nr; i++) {
struct object *real_obj;
real_obj = deref_tag(list->objects[i].item, NULL, 0);
+
+ /* load the gitmodules file for this rev */
+ if (recurse_submodules) {
+ submodule_free();
+ gitmodules_config_sha1(real_obj->oid.hash);
+ }
if (grep_object(opt, pathspec, real_obj, list->objects[i].name, list->objects[i].path)) {
hit = 1;
if (opt->status_only)
@@ -902,6 +963,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
N_("ignore files specified via '.gitignore'"), 1),
OPT_BOOL(0, "recurse-submodules", &recurse_submodules,
N_("recursivley search in each submodule")),
+ OPT_STRING(0, "parent-basename", &parent_basename,
+ N_("basename"),
+ N_("prepend parent project's basename to output")),
OPT_GROUP(""),
OPT_BOOL('v', "invert-match", &opt.invert,
N_("show non-matching lines")),
@@ -1154,7 +1218,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
}
}
- if (recurse_submodules && (!use_index || untracked || list.nr))
+ if (recurse_submodules && (!use_index || untracked))
die(_("option not supported with --recurse-submodules."));
if (!show_in_pager && !opt.status_only)
diff --git a/t/t7814-grep-recurse-submodules.sh b/t/t7814-grep-recurse-submodules.sh
index 1019125..9e93fe7 100755
--- a/t/t7814-grep-recurse-submodules.sh
+++ b/t/t7814-grep-recurse-submodules.sh
@@ -84,6 +84,108 @@ test_expect_success 'grep and multiple patterns' '
test_cmp expect actual
'
+test_expect_success 'basic grep tree' '
+ cat >expect <<-\EOF &&
+ HEAD:a:foobar
+ HEAD:b/b:bar
+ HEAD:submodule/a:foobar
+ HEAD:submodule/sub/a:foobar
+ EOF
+
+ git grep -e "bar" --recurse-submodules HEAD >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'grep tree HEAD^' '
+ cat >expect <<-\EOF &&
+ HEAD^:a:foobar
+ HEAD^:b/b:bar
+ HEAD^:submodule/a:foobar
+ EOF
+
+ git grep -e "bar" --recurse-submodules HEAD^ >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'grep tree HEAD^^' '
+ cat >expect <<-\EOF &&
+ HEAD^^:a:foobar
+ HEAD^^:b/b:bar
+ EOF
+
+ git grep -e "bar" --recurse-submodules HEAD^^ >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'grep tree and pathspecs' '
+ cat >expect <<-\EOF &&
+ HEAD:submodule/a:foobar
+ HEAD:submodule/sub/a:foobar
+ EOF
+
+ git grep -e "bar" --recurse-submodules HEAD -- submodule >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'grep tree and pathspecs' '
+ cat >expect <<-\EOF &&
+ HEAD:submodule/a:foobar
+ HEAD:submodule/sub/a:foobar
+ EOF
+
+ git grep -e "bar" --recurse-submodules HEAD -- "submodule*a" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'grep tree and more pathspecs' '
+ cat >expect <<-\EOF &&
+ HEAD:submodule/a:foobar
+ EOF
+
+ git grep -e "bar" --recurse-submodules HEAD -- "submodul?/a" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'grep tree and more pathspecs' '
+ cat >expect <<-\EOF &&
+ HEAD:submodule/sub/a:foobar
+ EOF
+
+ git grep -e "bar" --recurse-submodules HEAD -- "submodul*/sub/a" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'grep recurse submodule colon in name' '
+ git init parent &&
+ test_when_finished "rm -rf parent" &&
+ echo "foobar" >"parent/fi:le" &&
+ git -C parent add "fi:le" &&
+ git -C parent commit -m "add fi:le" &&
+
+ git init "su:b" &&
+ test_when_finished "rm -rf su:b" &&
+ echo "foobar" >"su:b/fi:le" &&
+ git -C "su:b" add "fi:le" &&
+ git -C "su:b" commit -m "add fi:le" &&
+
+ git -C parent submodule add "../su:b" "su:b" &&
+ git -C parent commit -m "add submodule" &&
+
+ cat >expect <<-\EOF &&
+ fi:le:foobar
+ su:b/fi:le:foobar
+ EOF
+ git -C parent grep -e "foobar" --recurse-submodules >actual &&
+ test_cmp expect actual &&
+
+ cat >expect <<-\EOF &&
+ HEAD:fi:le:foobar
+ HEAD:su:b/fi:le:foobar
+ EOF
+ git -C parent grep -e "foobar" --recurse-submodules HEAD >actual &&
+ test_cmp expect actual
+'
+
test_incompatible_with_recurse_submodules ()
{
test_expect_success "--recurse-submodules and $1 are incompatible" "
@@ -94,6 +196,5 @@ test_incompatible_with_recurse_submodules ()
test_incompatible_with_recurse_submodules --untracked
test_incompatible_with_recurse_submodules --no-index
-test_incompatible_with_recurse_submodules HEAD
test_done
diff --git a/tree-walk.c b/tree-walk.c
index 828f435..ff77605 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -1004,6 +1004,19 @@ static enum interesting do_match(const struct name_entry *entry,
*/
if (ps->recursive && S_ISDIR(entry->mode))
return entry_interesting;
+
+ /*
+ * When matching against submodules with
+ * wildcard characters, ensure that the entry
+ * at least matches up to the first wild
+ * character. More accurate matching can then
+ * be performed in the submodule itself.
+ */
+ if (ps->recursive && S_ISGITLINK(entry->mode) &&
+ !ps_strncmp(item, match + baselen,
+ entry->path,
+ item->nowildcard_len - baselen))
+ return entry_interesting;
}
continue;
@@ -1040,6 +1053,21 @@ static enum interesting do_match(const struct name_entry *entry,
strbuf_setlen(base, base_offset + baselen);
return entry_interesting;
}
+
+ /*
+ * When matching against submodules with
+ * wildcard characters, ensure that the entry
+ * at least matches up to the first wild
+ * character. More accurate matching can then
+ * be performed in the submodule itself.
+ */
+ if (ps->recursive && S_ISGITLINK(entry->mode) &&
+ !ps_strncmp(item, match, base->buf + base_offset,
+ item->nowildcard_len)) {
+ strbuf_setlen(base, base_offset + baselen);
+ return entry_interesting;
+ }
+
strbuf_setlen(base, base_offset + baselen);
/*
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v5 1/6] submodules: add helper functions to determine presence of submodules
From: Brandon Williams @ 2016-11-22 18:46 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, jonathantanmy, gitster
In-Reply-To: <1479840397-68264-1-git-send-email-bmwill@google.com>
Add two helper functions to submodules.c.
`is_submodule_initialized()` checks if a submodule has been initialized
at a given path and `is_submodule_populated()` check if a submodule
has been checked out at a given path.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
submodule.c | 38 ++++++++++++++++++++++++++++++++++++++
submodule.h | 2 ++
2 files changed, 40 insertions(+)
diff --git a/submodule.c b/submodule.c
index 6f7d883..f5107f0 100644
--- a/submodule.c
+++ b/submodule.c
@@ -198,6 +198,44 @@ void gitmodules_config(void)
}
}
+/*
+ * Determine if a submodule has been initialized at a given 'path'
+ */
+int is_submodule_initialized(const char *path)
+{
+ int ret = 0;
+ const struct submodule *module = NULL;
+
+ module = submodule_from_path(null_sha1, path);
+
+ if (module) {
+ char *key = xstrfmt("submodule.%s.url", module->name);
+ char *value = NULL;
+
+ ret = !git_config_get_string(key, &value);
+
+ free(value);
+ free(key);
+ }
+
+ return ret;
+}
+
+/*
+ * Determine if a submodule has been populated at a given 'path'
+ */
+int is_submodule_populated(const char *path)
+{
+ int ret = 0;
+ char *gitdir = xstrfmt("%s/.git", path);
+
+ if (resolve_gitdir(gitdir))
+ ret = 1;
+
+ free(gitdir);
+ return ret;
+}
+
int parse_submodule_update_strategy(const char *value,
struct submodule_update_strategy *dst)
{
diff --git a/submodule.h b/submodule.h
index d9e197a..6ec5f2f 100644
--- a/submodule.h
+++ b/submodule.h
@@ -37,6 +37,8 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
const char *path);
int submodule_config(const char *var, const char *value, void *cb);
void gitmodules_config(void);
+extern int is_submodule_initialized(const char *path);
+extern int is_submodule_populated(const char *path);
int parse_submodule_update_strategy(const char *value,
struct submodule_update_strategy *dst);
const char *submodule_strategy_to_string(const struct submodule_update_strategy *s);
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v5 3/6] grep: add submodules as a grep source type
From: Brandon Williams @ 2016-11-22 18:46 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, jonathantanmy, gitster
In-Reply-To: <1479840397-68264-1-git-send-email-bmwill@google.com>
Add `GREP_SOURCE_SUBMODULE` as a grep_source type and cases for this new
type in the various switch statements in grep.c.
When initializing a grep_source with type `GREP_SOURCE_SUBMODULE` the
identifier can either be NULL (to indicate that the working tree will be
used) or a SHA1 (the REV of the submodule to be grep'd). If the
identifier is a SHA1 then we want to fall through to the
`GREP_SOURCE_SHA1` case to handle the copying of the SHA1.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
grep.c | 16 +++++++++++++++-
grep.h | 1 +
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/grep.c b/grep.c
index 1194d35..0dbdc1d 100644
--- a/grep.c
+++ b/grep.c
@@ -1735,12 +1735,23 @@ void grep_source_init(struct grep_source *gs, enum grep_source_type type,
case GREP_SOURCE_FILE:
gs->identifier = xstrdup(identifier);
break;
+ case GREP_SOURCE_SUBMODULE:
+ if (!identifier) {
+ gs->identifier = NULL;
+ break;
+ }
+ /*
+ * FALL THROUGH
+ * If the identifier is non-NULL (in the submodule case) it
+ * will be a SHA1 that needs to be copied.
+ */
case GREP_SOURCE_SHA1:
gs->identifier = xmalloc(20);
hashcpy(gs->identifier, identifier);
break;
case GREP_SOURCE_BUF:
gs->identifier = NULL;
+ break;
}
}
@@ -1760,6 +1771,7 @@ void grep_source_clear_data(struct grep_source *gs)
switch (gs->type) {
case GREP_SOURCE_FILE:
case GREP_SOURCE_SHA1:
+ case GREP_SOURCE_SUBMODULE:
free(gs->buf);
gs->buf = NULL;
gs->size = 0;
@@ -1831,8 +1843,10 @@ static int grep_source_load(struct grep_source *gs)
return grep_source_load_sha1(gs);
case GREP_SOURCE_BUF:
return gs->buf ? 0 : -1;
+ case GREP_SOURCE_SUBMODULE:
+ break;
}
- die("BUG: invalid grep_source type");
+ die("BUG: invalid grep_source type to load");
}
void grep_source_load_driver(struct grep_source *gs)
diff --git a/grep.h b/grep.h
index 5856a23..267534c 100644
--- a/grep.h
+++ b/grep.h
@@ -161,6 +161,7 @@ struct grep_source {
GREP_SOURCE_SHA1,
GREP_SOURCE_FILE,
GREP_SOURCE_BUF,
+ GREP_SOURCE_SUBMODULE,
} type;
void *identifier;
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* Re: [PATCHv3 3/3] submodule-config: clarify parsing of null_sha1 element
From: Brandon Williams @ 2016-11-22 18:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stefan Beller, git, jacob.keller
In-Reply-To: <xmqqziksfae3.fsf@gitster.mtv.corp.google.com>
On 11/21, Junio C Hamano wrote:
> Stefan Beller <sbeller@google.com> writes:
>
> > +Whenever a submodule configuration is parsed in `parse_submodule_config_option`
> > +via e.g. `gitmodules_config()`, it will be overwrite the null_sha1 entry.
>
> It will overwrite? It will be overwritten? I guess it is the latter?
>
> > +So in the normal case, when HEAD:.gitmodules is parsed first and then overlayed
> > +with the repository configuration, the null_sha1 entry contains the local
> > +configuration of a submodule (e.g. consolidated values from local git
> > configuration and the .gitmodules file in the worktree).
> >
> > For an example usage see test-submodule-config.c.
I brought this up in v2, he must have just missed it for v3.
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH v7 13/17] ref-filter: add `:dir` and `:base` options for ref printing atoms
From: Karthik Nayak @ 2016-11-22 18:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jakub Narębski, Jacob Keller, Git mailing list
In-Reply-To: <xmqq4m32kqet.fsf@gitster.mtv.corp.google.com>
On Sun, Nov 20, 2016 at 11:02 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> We could have lstrip and rstrip as you suggested and perhaps make it work
>> together too. But I see this going off the scope of this series. Maybe
>> I'll follow up
>> with another series introducing these features. Since we can currently
>> make do with
>> 'strip=2' I'll drop this patch from v8 of this series and pursue this
>> idea after this.
>
> My primary point was that if we know we want to add "rstrip" later
> and still decide not to add it right now, it is OK, but we will
> regret it if we named the one we are going to add right now "strip".
> That will mean that future users, when "rstrip" is introduced, will
> end up having to choose between "strip" and "rstrip" (as opposed to
> "lstrip" and "rstrip"), wondering why left-variant is more important
> and named without left/right prefix.
>
That's a good point actually, I'll try and implement both and re-roll.
--
Regards,
Karthik Nayak
^ permalink raw reply
* Re: [PATCH v7 14/17] ref-filter: allow porcelain to translate messages in the output
From: Karthik Nayak @ 2016-11-22 18:33 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Jakub Narębski, Git List, Jacob Keller
In-Reply-To: <vpq1sy58bsl.fsf@anie.imag.fr>
On Mon, Nov 21, 2016 at 2:11 PM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> cc'in Matthieu since he wrote the patch.
>>
>> On Sat, Nov 19, 2016 at 4:16 AM, Jakub Narębski <jnareb@gmail.com> wrote:
>>> W dniu 08.11.2016 o 21:12, Karthik Nayak pisze:
>>>> From: Karthik Nayak <karthik.188@gmail.com>
>>>>
>>>> Introduce setup_ref_filter_porcelain_msg() so that the messages used in
>>>> the atom %(upstream:track) can be translated if needed. This is needed
>>>> as we port branch.c to use ref-filter's printing API's.
>>>>
>>>> Written-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
>>>> Mentored-by: Christian Couder <christian.couder@gmail.com>
>>>> Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
>>>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>>>> ---
>>>> ref-filter.c | 28 ++++++++++++++++++++++++----
>>>> ref-filter.h | 2 ++
>>>> 2 files changed, 26 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/ref-filter.c b/ref-filter.c
>>>> index b47b900..944671a 100644
>>>> --- a/ref-filter.c
>>>> +++ b/ref-filter.c
>>>> @@ -15,6 +15,26 @@
>>>> #include "version.h"
>>>> #include "wt-status.h"
>>>>
>>>> +static struct ref_msg {
>>>> + const char *gone;
>>>> + const char *ahead;
>>>> + const char *behind;
>>>> + const char *ahead_behind;
>>>> +} msgs = {
>>>> + "gone",
>>>> + "ahead %d",
>>>> + "behind %d",
>>>> + "ahead %d, behind %d"
>>>> +};
>>>> +
>>>> +void setup_ref_filter_porcelain_msg(void)
>>>> +{
>>>> + msgs.gone = _("gone");
>>>> + msgs.ahead = _("ahead %d");
>>>> + msgs.behind = _("behind %d");
>>>> + msgs.ahead_behind = _("ahead %d, behind %d");
>>>> +}
>>>
>>> Do I understand it correctly that this mechanism is here to avoid
>>> repeated calls into gettext, as those messages would get repeated
>>> over and over; otherwise one would use foo = N_("...") and _(foo),
>>> isn't it?
>
> That's not the primary goal. The primary goal is to keep untranslated,
> and immutable messages in plumbing commands. We may decide one day that
> _("gone") is not the best message for the end user and replace it with,
> say, _("vanished"), but the "gone" has to remain the same forever and
> regardless of the user's config for scripts using it.
>
> We could have written
>
> in_porcelain ? _("gone") : "gone"
>
> here and there in the code, but having a single place where we set all
> the messages seems simpler. Call setup_ref_filter_porcelain_msg() and
> get the porcelain messages, don't call it and keep the plumbing
> messages.
>
> Note that it's not the first place in the code where we do this, see
> setup_unpack_trees_porcelain in unpack-trees.c for another instance.
>
> Karthik: the commit message could be improved, for example:
>
> Introduce setup_ref_filter_porcelain_msg() so that the messages used in
> the atom %(upstream:track) can be translated if needed. By default, keep
> the messages untranslated, which is the right behavior for plumbing
> commands. This is needed as we port branch.c to use ref-filter's
> printing API's.
>
> Why not a comment right below "} msgs = {" saying e.g.:
>
> /* Untranslated plumbing messages: */
>
Will update the commit message and add the comment. Thanks :)
--
Regards,
Karthik Nayak
^ permalink raw reply
* Re: [PATCH v7 16/17] branch: use ref-filter printing APIs
From: Karthik Nayak @ 2016-11-22 18:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List, Jacob Keller
In-Reply-To: <xmqqa8cxoj7k.fsf@gitster.mtv.corp.google.com>
On Fri, Nov 18, 2016 at 3:35 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> One worry that I have is if the strings embedded in this function to
>> the final format are safe. As far as I can tell, the pieces of
>> strings that are literally inserted into the resulting format string
>> by this function are maxwidth, remote_prefix, and return values from
>> branch_get_color() calls.
>>
>> The maxwidth is inserted via "%d" and made into decimal constant,
>> and there is no risk for it being in the resulting format. Are
>> the return values of branch_get_color() calls safe? I do not think
>> they can have '%' in them, but if they do, they need to be quoted.
>> The same worry exists for remote_prefix. Currently it can either be
>> an empty string or "remotes/", and is safe to be embedded in a
>> format string.
>
> In case it was not clear, in short, I do not think there is anything
> broken in the code, but it is a longer-term improvement to introduce
> a helper that takes a string and returns a version of the string
> that is safely quoted to be used in the for-each-ref format string
> use it like so:
>
> strbuf_addf(&remote,
> "%s"
> "%%(align:%d,left)%s%%(refname:strip=2)%%(end)"
> ...
> "%%(else) %%(objectname:short=7) %%(contents:subject)%%(end)",
> quote_literal_for_format(branch_get_color(BRANCH_COLOR_REMOTE)),
> ...);
>
> and the implementation of the helper may look like:
>
> const char *quote_literal_for_format(const char *s)
> {
> static strbuf buf = STRBUF_INIT;
>
> strbuf_reset(&buf);
> while (*s) {
> const char *ep = strchrnul(s, '%');
> if (s < ep)
> strbuf_add(&buf, s, ep - s);
> if (*ep == '%') {
> strbuf_addstr(&buf, "%%");
> s = ep + 1;
> } else {
> s = ep;
> }
> }
> return buf.buf;
> }
>
Perfect. I get what you're saying, I'll add this in :)
--
Regards,
Karthik Nayak
^ permalink raw reply
* Re: [PATCH v2 2/2] push: fix --dry-run to not push submodules
From: Brandon Williams @ 2016-11-22 18:18 UTC (permalink / raw)
To: Junio C Hamano
Cc: Stefan Beller, git@vger.kernel.org, Heiko Voigt, Johannes Sixt
In-Reply-To: <xmqqk2bvquk1.fsf@gitster.mtv.corp.google.com>
On 11/22, Junio C Hamano wrote:
> Brandon Williams <bmwill@google.com> writes:
>
> > On 11/17, Stefan Beller wrote:
> >> On Thu, Nov 17, 2016 at 10:46 AM, Brandon Williams <bmwill@google.com> wrote:
> >>
> >> > sha1_array_clear(&commits);
> >> > - die("Failed to push all needed submodules!");
> >> > + die ("Failed to push all needed submodules!");
> >>
> >> huh? Is this a whitespace change?
> >
> > That's odd...I didn't mean to add that lone space.
>
> Is that the only glitch in this round? IOW, is the series OK to be
> picked up as long as I treak this out while queuing?
It looks that way. And I did fix this in my local series. Let me know
if you would rather I resend the series. Otherwise I think it looks
good.
I do also have a follow on series I'm planning on sending out later to
actually add in a feature which mimics what this bug does (as this
functionality could be desirable in some circumstances) but thought it
best to wait till heiko's and this series were more stable.
--
Brandon Williams
^ permalink raw reply
* Re: Fwd: git diff with “--word-diff-regex” extremely slow compared to “--word-diff”?
From: Matthieu S @ 2016-11-22 18:08 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20161120201744.7ym4gsmjoijw6oow@sigill.intra.peff.net>
Thanks Jeff for the answer!
You are right, I should have compared with the same regex, and indeed,
--word-diff-regex=[^[:space:]] is also much slower than just
--word-diff, although they do the same job. Maybe this is a hint that
the --word-diff-regex code could be made faster?
I have a small understanding of git, but is git diff computing the
diff value for the whole file, and then showing in the terminal the 10
first values? In some cases, it seems to be a lot of unnecessary
computation! Is there any possibility to ask git-diff to only compare
say the first 100 lines? Or compute only when necessary, i.e.
when"enter" is prompted in the console?
Thanks!
Matthieu
2016-11-20 12:17 GMT-08:00 Jeff King <peff@peff.net>:
> On Fri, Nov 18, 2016 at 03:40:22PM -0800, Matthieu S wrote:
>
>> Why is the speed so different if one uses --word-diff instead of
>> --word-diff-regex= ? Is it just because my expression is (slightly)
>> more complex than the default one (split on period instead of only
>> whitespace) ? Or is it that the default word-diff is implemented
>> differently/more efficiently? How can I overcome this speed slowdown?
>
> I think it's probably both.
>
> See diff.c:find_word_boundaries(). If there's no regex, we use a simple
> loop over isspace() to find the boundaries. I don't recall anybody
> measuring the performance before, but I'm not surprised to hear that
> matching a regex is slower.
>
> If I look at the output of "perf", though, it looks like we also spend a
> lot more time in xdl_clean_mmatch(). Which isn't surprising. Your regex
> treats commas as boundaries, which is going to generate a lot more
> matches for this particular data set (though the output is the same, I
> think, because of the nature of the change).
>
> I would have expected "--word-diff-regex=[^[:space:]]" to be faster than
> your regex, though, and it does not seem to be.
>
> -Peff
^ permalink raw reply
* Re: v2.11 new diff heuristic?
From: Jeff King @ 2016-11-22 17:55 UTC (permalink / raw)
To: Robert Dailey; +Cc: Git
In-Reply-To: <CAHd499AjXh1YnVgBj_8j0fgvOgOn53y+sPBBy6y7mSM-+dCyVw@mail.gmail.com>
On Tue, Nov 22, 2016 at 08:42:36AM -0600, Robert Dailey wrote:
> I dug into the git diff documentation here:
>
> https://git-scm.com/docs/git-diff
>
> It mentions a "--compaction-heuristic" option. Is this the new
> heuristic outlined by the release notes? If not, which is it?
No. The docs on git-scm.com are only for released versions, so that is
the "old" attempt at a similar feature from v2.9. The "new" one goes by
the name "--indent-heuristic" (and "diff.indentHeuristic" in the
config). It is not the default in v2.11, but it probably will become so
in a future version.
> Is the
> compaction heuristic compatible with the histogram diff algorithm? Is
> there a config option to turn this on all the time? For that matter,
> is this something I can keep on all the time or is it only useful in
> certain situations?
>
> There's still so much more about this feature I would like to know.
Yes, you can keep it all the time. Yes, it's compatible with histogram
(and patience); it happens as a separate post-processing step after the
actual diff.
You can find more discussion about how it works in the commit message of
433860f3d0beb0c6f205290bd16cda413148f098.
-Peff
^ permalink raw reply
* Re: [PATCH 3/3] submodule--helper: add intern-git-dir function
From: Junio C Hamano @ 2016-11-22 17:53 UTC (permalink / raw)
To: Stefan Beller
Cc: Brandon Williams, Jonathan Nieder, git@vger.kernel.org,
Jens Lehmann, Heiko Voigt
In-Reply-To: <CAGZ79kb7062abd6Cbf-ey3u0L6PXUcRf7m-og5EyCRZencOR6g@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> On Mon, Nov 21, 2016 at 11:07 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Stefan Beller <sbeller@google.com> writes:
>>
>>> So I guess we should test a bit more extensively, maybe
>>>
>>> git status >expect
>>> git submodule embedgitdirs
>>> git status >actual
>>> test_cmp expect actual
>>> # further testing via
>>> test -f ..
>>> test -d ..
>>
>> Something along that line. "status should succeed" does not tell
>> the readers what kind of breakage the test is expecting to protect
>> us from. If we are expecting a breakage in embed-git-dirs would
>> somehow corrupt an existing submodule, which would lead to "status"
>> that is run in the superproject report the submodule differently,
>> then comparing output before and after the operation may be a
>> reasonable test. Going there to the submodule working tree and
>> checking the health of the repository (of the submodule) may be
>> another sensible test.
>
> and by checking the health you mean just a status in there, or rather a
> more nuanced thing like `git rev-parse HEAD` ? I'll go with that for now.
Would fsck have caught the breakages you caused while developing it,
for example?
As the core of the "embed" operation is an non-atomic "move the .git
directory to .git/modules/$name in the superproject and then make a
gitfile pointing at it", verifying that there is no change in the
contents of .git/modules before and after the failed operation, and
verifying that the submodule working tree has the embedded .git/
directory would be good enough, I would think.
^ 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