* Re: [PATCH v4] diff: handle --no-abbrev in no-index case
From: Junio C Hamano @ 2016-12-08 22:53 UTC (permalink / raw)
To: Jack Bates; +Cc: git, Jeff King, Jack Bates
In-Reply-To: <20161206165614.22921-1-jack@nottheoilrig.com>
Jack Bates <bk874k@nottheoilrig.com> writes:
> There are two different places where the --no-abbrev option is parsed,
> and two different places where SHA-1s are abbreviated. We normally parse
> --no-abbrev with setup_revisions(), but in the no-index case, "git diff"
> calls diff_opt_parse() directly, and diff_opt_parse() didn't handle
> --no-abbrev until now. (It did handle --abbrev, however.) We normally
> abbreviate SHA-1s with find_unique_abbrev(), but commit 4f03666 ("diff:
> handle sha1 abbreviations outside of repository, 2016-10-20) recently
> introduced a special case when you run "git diff" outside of a
> repository.
>
> setup_revisions() does also call diff_opt_parse(), but not for --abbrev
> or --no-abbrev, which it handles itself. setup_revisions() sets
> rev_info->abbrev, and later copies that to diff_options->abbrev. It
> handles --no-abbrev by setting abbrev to zero. (This change doesn't
> touch that.)
>
> Setting abbrev to zero was broken in the outside-of-a-repository special
> case, which until now resulted in a truly zero-length SHA-1, rather than
> taking zero to mean do not abbreviate. The only way to trigger this bug,
> however, was by running "git diff --raw" without either the --abbrev or
> --no-abbrev options, because 1) without --raw it doesn't respect abbrev
> (which is bizarre, but has been that way forever), 2) we silently clamp
> --abbrev=0 to MINIMUM_ABBREV, and 3) --no-abbrev wasn't handled until
> now.
>
> The outside-of-a-repository case is one of three no-index cases. The
> other two are when one of the files you're comparing is outside of the
> repository you're in, and the --no-index option.
Nicely described.
> diff --git a/diff.c b/diff.c
> index ec87283..84dba60 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -3106,7 +3106,8 @@ static const char *diff_abbrev_oid(const struct object_id *oid, int abbrev)
> abbrev = FALLBACK_DEFAULT_ABBREV;
> if (abbrev > GIT_SHA1_HEXSZ)
> die("BUG: oid abbreviation out of range: %d", abbrev);
> - hex[abbrev] = '\0';
> + if (abbrev)
> + hex[abbrev] = '\0';
> return hex;
> }
> }
This is the same since your earlier round and it is correct. The
code before this part clamps abbrev to be between 0 and 40.
> @@ -3364,6 +3365,7 @@ void diff_setup(struct diff_options *options)
>
> options->file = stdout;
>
> + options->abbrev = DEFAULT_ABBREV;
This is a new change relative to your earlier one.
I looked at all the callers of diff_setup() and noticed that many of
them were initializing "struct diff_options" that is on-stack that
is totally uninitialized, which means they were using a completely
random value that happened to be on the stack.
Which was surprising and made me wonder how the entire "diff" code
could have ever worked correctly for the past 10 years, as it's not
like all the users always passed --[no-]abbrev[=<value>] from the
command line.
In any case, this cannot possibly be introducing a regression; these
callsites of diff_setup() were starting from a random garbage---now
they start with -1 in this field. If they were doing the right
thing by assigning their own abbrev to the field after diff_setup()
returned, they will continue to do the same, and otherwise they will
keep doing whatever random things they have been doing when the
uninitialized field happened to contain -1 the same way.
I didn't look carefully at the additional tests, but the code change
looks good.
Thanks.
^ permalink raw reply
* [PATCHv7 6/6] submodule: add absorb-git-dir function
From: Stefan Beller @ 2016-12-08 21:03 UTC (permalink / raw)
To: bmwill; +Cc: git, pclouds, gitster, Stefan Beller
In-Reply-To: <20161208210329.12919-1-sbeller@google.com>
When a submodule has its git dir inside the working dir, the submodule
support for checkout that we plan to add in a later patch will fail.
Add functionality to migrate the git directory to be absorbed
into the superprojects git directory.
The newly added code in this patch is structured such that other areas of
Git can also make use of it. The code in the submodule--helper is a mere
wrapper and option parser for the function
`absorb_git_dir_into_superproject`, that takes care of embedding the
submodules git directory into the superprojects git dir. That function
makes use of the more abstract function for this use case
`relocate_gitdir`, which can be used by e.g. the worktree code eventually
to move around a git directory.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
Documentation/git-submodule.txt | 15 ++++++
builtin/submodule--helper.c | 38 ++++++++++++++
dir.c | 12 +++++
dir.h | 3 ++
git-submodule.sh | 7 ++-
submodule.c | 103 +++++++++++++++++++++++++++++++++++++
submodule.h | 4 ++
t/t7412-submodule-absorbgitdirs.sh | 101 ++++++++++++++++++++++++++++++++++++
8 files changed, 282 insertions(+), 1 deletion(-)
create mode 100755 t/t7412-submodule-absorbgitdirs.sh
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index d841573475..918bd1d1bd 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -22,6 +22,7 @@ SYNOPSIS
[commit] [--] [<path>...]
'git submodule' [--quiet] foreach [--recursive] <command>
'git submodule' [--quiet] sync [--recursive] [--] [<path>...]
+'git submodule' [--quiet] absorbgitdirs [--] [<path>...]
DESCRIPTION
@@ -245,6 +246,20 @@ sync::
If `--recursive` is specified, this command will recurse into the
registered submodules, and sync any nested submodules within.
+absorbgitdirs::
+ If a git directory of a submodule is inside the submodule,
+ move the git directory of the submodule into its superprojects
+ `$GIT_DIR/modules` path and then connect the git directory and
+ its working directory by setting the `core.worktree` and adding
+ a .git file pointing to the git directory embedded in the
+ superprojects git directory.
++
+A repository that was cloned independently and later added as a submodule or
+old setups have the submodules git directory inside the submodule instead of
+embedded into the superprojects git directory.
++
+This command is recursive by default.
+
OPTIONS
-------
-q::
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 5643848667..242d9911a6 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1076,6 +1076,43 @@ static int resolve_remote_submodule_branch(int argc, const char **argv,
return 0;
}
+static int absorb_git_dirs(int argc, const char **argv, const char *prefix)
+{
+ int i;
+ struct pathspec pathspec;
+ struct module_list list = MODULE_LIST_INIT;
+ unsigned flags = ABSORB_GITDIR_RECURSE_SUBMODULES;
+
+ struct option embed_gitdir_options[] = {
+ OPT_STRING(0, "prefix", &prefix,
+ N_("path"),
+ N_("path into the working tree")),
+ OPT_BIT(0, "--recursive", &flags, N_("recurse into submodules"),
+ ABSORB_GITDIR_RECURSE_SUBMODULES),
+ OPT_END()
+ };
+
+ const char *const git_submodule_helper_usage[] = {
+ N_("git submodule--helper embed-git-dir [<path>...]"),
+ NULL
+ };
+
+ argc = parse_options(argc, argv, prefix, embed_gitdir_options,
+ git_submodule_helper_usage, 0);
+
+ gitmodules_config();
+ git_config(submodule_config, NULL);
+
+ if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
+ return 1;
+
+ for (i = 0; i < list.nr; i++)
+ absorb_git_dir_into_superproject(prefix,
+ list.entries[i]->name, flags);
+
+ return 0;
+}
+
#define SUPPORT_SUPER_PREFIX (1<<0)
struct cmd_struct {
@@ -1094,6 +1131,7 @@ static struct cmd_struct commands[] = {
{"resolve-relative-url-test", resolve_relative_url_test, 0},
{"init", module_init, 0},
{"remote-branch", resolve_remote_submodule_branch, 0},
+ {"absorb-git-dirs", absorb_git_dirs, SUPPORT_SUPER_PREFIX},
};
int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
diff --git a/dir.c b/dir.c
index e0efd3c2c3..d872cc1570 100644
--- a/dir.c
+++ b/dir.c
@@ -2773,3 +2773,15 @@ void connect_work_tree_and_git_dir(const char *work_tree_, const char *git_dir_)
free(work_tree);
free(git_dir);
}
+
+/*
+ * Migrate the git directory of the given path from old_git_dir to new_git_dir.
+ */
+void relocate_gitdir(const char *path, const char *old_git_dir, const char *new_git_dir)
+{
+ if (rename(old_git_dir, new_git_dir) < 0)
+ die_errno(_("could not migrate git directory from '%s' to '%s'"),
+ old_git_dir, new_git_dir);
+
+ connect_work_tree_and_git_dir(path, new_git_dir);
+}
diff --git a/dir.h b/dir.h
index 051674a431..bf23a470af 100644
--- a/dir.h
+++ b/dir.h
@@ -336,4 +336,7 @@ void write_untracked_extension(struct strbuf *out, struct untracked_cache *untra
void add_untracked_cache(struct index_state *istate);
void remove_untracked_cache(struct index_state *istate);
extern void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
+extern void relocate_gitdir(const char *path,
+ const char *old_git_dir,
+ const char *new_git_dir);
#endif
diff --git a/git-submodule.sh b/git-submodule.sh
index a024a135d6..9285b5c43d 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -1131,6 +1131,11 @@ cmd_sync()
done
}
+cmd_absorbgitdirs()
+{
+ git submodule--helper absorb-git-dirs --prefix "$wt_prefix" "$@"
+}
+
# This loop parses the command line arguments to find the
# subcommand name to dispatch. Parsing of the subcommand specific
# options are primarily done by the subcommand implementations.
@@ -1140,7 +1145,7 @@ cmd_sync()
while test $# != 0 && test -z "$command"
do
case "$1" in
- add | foreach | init | deinit | update | status | summary | sync)
+ add | foreach | init | deinit | update | status | summary | sync | absorbgitdirs)
command=$1
;;
-q|--quiet)
diff --git a/submodule.c b/submodule.c
index 0bb50b4b62..45ccfb7ab4 100644
--- a/submodule.c
+++ b/submodule.c
@@ -14,6 +14,7 @@
#include "blob.h"
#include "thread-utils.h"
#include "quote.h"
+#include "worktree.h"
static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;
static int parallel_jobs = 1;
@@ -1237,3 +1238,105 @@ void prepare_submodule_repo_env(struct argv_array *out)
}
argv_array_push(out, "GIT_DIR=.git");
}
+
+/*
+ * Embeds a single submodules git directory into the superprojects git dir,
+ * non recursively.
+ */
+static void relocate_single_git_dir_into_superproject(const char *prefix,
+ const char *path)
+{
+ char *old_git_dir = NULL, *real_old_git_dir = NULL, *real_new_git_dir = NULL;
+ const char *new_git_dir;
+ const struct submodule *sub;
+
+ if (submodule_uses_worktrees(path))
+ die(_("relocate_gitdir for submodule '%s' with "
+ "more than one worktree not supported"), path);
+
+ old_git_dir = xstrfmt("%s/.git", path);
+ if (read_gitfile(old_git_dir))
+ /* If it is an actual gitfile, it doesn't need migration. */
+ return;
+
+ real_old_git_dir = xstrdup(real_path(old_git_dir));
+
+ sub = submodule_from_path(null_sha1, path);
+ if (!sub)
+ die(_("could not lookup name for submodule '%s'"), path);
+
+ new_git_dir = git_path("modules/%s", sub->name);
+ if (safe_create_leading_directories_const(new_git_dir) < 0)
+ die(_("could not create directory '%s'"), new_git_dir);
+ real_new_git_dir = xstrdup(real_path(new_git_dir));
+
+ if (!prefix)
+ prefix = get_super_prefix();
+
+ fprintf(stderr, _("Migrating git directory of '%s%s' from\n'%s' to\n'%s'\n"),
+ prefix ? prefix : "", path,
+ real_old_git_dir, real_new_git_dir);
+
+ relocate_gitdir(path, real_old_git_dir, real_new_git_dir);
+
+ free(old_git_dir);
+ free(real_old_git_dir);
+ free(real_new_git_dir);
+}
+
+/*
+ * Migrate the git directory of the submodule given by path from
+ * having its git directory within the working tree to the git dir nested
+ * in its superprojects git dir under modules/.
+ */
+void absorb_git_dir_into_superproject(const char *prefix,
+ const char *path,
+ unsigned flags)
+{
+ const char *sub_git_dir, *v;
+ char *real_sub_git_dir = NULL, *real_common_git_dir = NULL;
+ struct strbuf gitdir = STRBUF_INIT;
+
+ strbuf_addf(&gitdir, "%s/.git", path);
+ sub_git_dir = resolve_gitdir(gitdir.buf);
+
+ /* Not populated? */
+ if (!sub_git_dir)
+ goto out;
+
+ /* Is it already absorbed into the superprojects git dir? */
+ real_sub_git_dir = xstrdup(real_path(sub_git_dir));
+ real_common_git_dir = xstrdup(real_path(get_git_common_dir()));
+ if (!skip_prefix(real_sub_git_dir, real_common_git_dir, &v))
+ relocate_single_git_dir_into_superproject(prefix, path);
+
+ if (flags & ABSORB_GITDIR_RECURSE_SUBMODULES) {
+ struct child_process cp = CHILD_PROCESS_INIT;
+ struct strbuf sb = STRBUF_INIT;
+
+ if (flags & ~ABSORB_GITDIR_RECURSE_SUBMODULES)
+ die("BUG: we don't know how to pass the flags down?");
+
+ if (get_super_prefix())
+ strbuf_addstr(&sb, get_super_prefix());
+ strbuf_addstr(&sb, path);
+ strbuf_addch(&sb, '/');
+
+ cp.dir = path;
+ cp.git_cmd = 1;
+ cp.no_stdin = 1;
+ argv_array_pushl(&cp.args, "--super-prefix", sb.buf,
+ "submodule--helper",
+ "absorb-git-dirs", NULL);
+ prepare_submodule_repo_env(&cp.env_array);
+ if (run_command(&cp))
+ die(_("could not recurse into submodule '%s'"), path);
+
+ strbuf_release(&sb);
+ }
+
+out:
+ strbuf_release(&gitdir);
+ free(real_sub_git_dir);
+ free(real_common_git_dir);
+}
diff --git a/submodule.h b/submodule.h
index 4e3bf469b4..6229054b99 100644
--- a/submodule.h
+++ b/submodule.h
@@ -74,4 +74,8 @@ int parallel_submodules(void);
*/
void prepare_submodule_repo_env(struct argv_array *out);
+#define ABSORB_GITDIR_RECURSE_SUBMODULES (1<<0)
+extern void absorb_git_dir_into_superproject(const char *prefix,
+ const char *path,
+ unsigned flags);
#endif
diff --git a/t/t7412-submodule-absorbgitdirs.sh b/t/t7412-submodule-absorbgitdirs.sh
new file mode 100755
index 0000000000..7c4e8b8d79
--- /dev/null
+++ b/t/t7412-submodule-absorbgitdirs.sh
@@ -0,0 +1,101 @@
+#!/bin/sh
+
+test_description='Test submodule absorbgitdirs
+
+This test verifies that `git submodue absorbgitdirs` moves a submodules git
+directory into the superproject.
+'
+
+. ./test-lib.sh
+
+test_expect_success 'setup a real submodule' '
+ git init sub1 &&
+ test_commit -C sub1 first &&
+ git submodule add ./sub1 &&
+ test_tick &&
+ git commit -m superproject
+'
+
+test_expect_success 'absorb the git dir' '
+ >expect.1 &&
+ >expect.2 &&
+ >actual.1 &&
+ >actual.2 &&
+ git status >expect.1 &&
+ git -C sub1 rev-parse HEAD >expect.2 &&
+ git submodule absorbgitdirs &&
+ git fsck &&
+ test -f sub1/.git &&
+ test -d .git/modules/sub1 &&
+ git status >actual.1 &&
+ git -C sub1 rev-parse HEAD >actual.2 &&
+ test_cmp expect.1 actual.1 &&
+ test_cmp expect.2 actual.2
+'
+
+test_expect_success 'absorbing does not fail for deinitalized submodules' '
+ test_when_finished "git submodule update --init" &&
+ git submodule deinit --all &&
+ git submodule absorbgitdirs &&
+ test -d .git/modules/sub1 &&
+ test -d sub1 &&
+ ! test -e sub1/.git
+'
+
+test_expect_success 'setup nested submodule' '
+ git init sub1/nested &&
+ test_commit -C sub1/nested first_nested &&
+ git -C sub1 submodule add ./nested &&
+ test_tick &&
+ git -C sub1 commit -m "add nested" &&
+ git add sub1 &&
+ git commit -m "sub1 to include nested submodule"
+'
+
+test_expect_success 'absorb the git dir in a nested submodule' '
+ git status >expect.1 &&
+ git -C sub1/nested rev-parse HEAD >expect.2 &&
+ git submodule absorbgitdirs &&
+ test -f sub1/nested/.git &&
+ test -d .git/modules/sub1/modules/nested &&
+ git status >actual.1 &&
+ git -C sub1/nested rev-parse HEAD >actual.2 &&
+ test_cmp expect.1 actual.1 &&
+ test_cmp expect.2 actual.2
+'
+
+test_expect_success 'setup a gitlink with missing .gitmodules entry' '
+ git init sub2 &&
+ test_commit -C sub2 first &&
+ git add sub2 &&
+ git commit -m superproject
+'
+
+test_expect_success 'absorbing the git dir fails for incomplete submodules' '
+ git status >expect.1 &&
+ git -C sub2 rev-parse HEAD >expect.2 &&
+ test_must_fail git submodule absorbgitdirs &&
+ git -C sub2 fsck &&
+ test -d sub2/.git &&
+ git status >actual &&
+ git -C sub2 rev-parse HEAD >actual.2 &&
+ test_cmp expect.1 actual.1 &&
+ test_cmp expect.2 actual.2
+'
+
+test_expect_success 'setup a submodule with multiple worktrees' '
+ # first create another unembedded git dir in a new submodule
+ git init sub3 &&
+ test_commit -C sub3 first &&
+ git submodule add ./sub3 &&
+ test_tick &&
+ git commit -m "add another submodule" &&
+ git -C sub3 worktree add ../sub3_second_work_tree
+'
+
+test_expect_success 'absorb a submodule with multiple worktrees' '
+ test_must_fail git submodule absorbgitdirs sub3 2>error &&
+ test_i18ngrep "not supported" error
+'
+
+test_done
--
2.11.0.rc2.29.g7c00390.dirty
^ permalink raw reply related
* [PATCHv7 4/6] worktree: have a function to check if worktrees are in use
From: Stefan Beller @ 2016-12-08 21:03 UTC (permalink / raw)
To: bmwill; +Cc: git, pclouds, gitster, Stefan Beller
In-Reply-To: <20161208210329.12919-1-sbeller@google.com>
In a later patch we want to move around the the git directory of
a submodule. Both submodules as well as worktrees are involved in
placing git directories at unusual places, so their functionality
may collide. To react appropriately to situations where worktrees
in submodules are in use, offer a new function to query the
a submodule if it uses the worktree feature.
This can be done cheaply (both in new code to write as well as run time)
by obtaining the list of worktrees based off that submodules git
directory. However as we have loaded the variables for the current
repository, the values in the submodule worktree
can be wrong, e.g.
* core.ignorecase may differ between these two repositories
* the ref resolution is broken (refs/heads/branch in the submodule
resolves to the sha1 value of the `branch` in the current repository
that may not exist or have another sha1)
Signed-off-by: Stefan Beller <sbeller@google.com>
---
worktree.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++---------
worktree.h | 7 +++++++
2 files changed, 65 insertions(+), 10 deletions(-)
diff --git a/worktree.c b/worktree.c
index eb6121263b..1c46fcf25f 100644
--- a/worktree.c
+++ b/worktree.c
@@ -72,7 +72,7 @@ static void add_head_info(struct strbuf *head_ref, struct worktree *worktree)
/**
* get the main worktree
*/
-static struct worktree *get_main_worktree(void)
+static struct worktree *get_main_worktree(const char *git_common_dir)
{
struct worktree *worktree = NULL;
struct strbuf path = STRBUF_INIT;
@@ -81,12 +81,12 @@ static struct worktree *get_main_worktree(void)
int is_bare = 0;
int is_detached = 0;
- strbuf_add_absolute_path(&worktree_path, get_git_common_dir());
+ strbuf_add_absolute_path(&worktree_path, git_common_dir);
is_bare = !strbuf_strip_suffix(&worktree_path, "/.git");
if (is_bare)
strbuf_strip_suffix(&worktree_path, "/.");
- strbuf_addf(&path, "%s/HEAD", get_git_common_dir());
+ strbuf_addf(&path, "%s/HEAD", git_common_dir);
worktree = xcalloc(1, sizeof(*worktree));
worktree->path = strbuf_detach(&worktree_path, NULL);
@@ -101,7 +101,8 @@ static struct worktree *get_main_worktree(void)
return worktree;
}
-static struct worktree *get_linked_worktree(const char *id)
+static struct worktree *get_linked_worktree(const char *git_common_dir,
+ const char *id)
{
struct worktree *worktree = NULL;
struct strbuf path = STRBUF_INIT;
@@ -112,7 +113,7 @@ static struct worktree *get_linked_worktree(const char *id)
if (!id)
die("Missing linked worktree name");
- strbuf_git_common_path(&path, "worktrees/%s/gitdir", id);
+ strbuf_addf(&path, "%s/worktrees/%s/gitdir", git_common_dir, id);
if (strbuf_read_file(&worktree_path, path.buf, 0) <= 0)
/* invalid gitdir file */
goto done;
@@ -125,7 +126,7 @@ static struct worktree *get_linked_worktree(const char *id)
}
strbuf_reset(&path);
- strbuf_addf(&path, "%s/worktrees/%s/HEAD", get_git_common_dir(), id);
+ strbuf_addf(&path, "%s/worktrees/%s/HEAD", git_common_dir, id);
if (parse_ref(path.buf, &head_ref, &is_detached) < 0)
goto done;
@@ -167,7 +168,8 @@ static int compare_worktree(const void *a_, const void *b_)
return fspathcmp((*a)->path, (*b)->path);
}
-struct worktree **get_worktrees(unsigned flags)
+static struct worktree **get_worktrees_internal(const char *git_common_dir,
+ unsigned flags)
{
struct worktree **list = NULL;
struct strbuf path = STRBUF_INIT;
@@ -177,9 +179,10 @@ struct worktree **get_worktrees(unsigned flags)
list = xmalloc(alloc * sizeof(struct worktree *));
- list[counter++] = get_main_worktree();
+ if (!(flags & GWT_LINKED_ONLY))
+ list[counter++] = get_main_worktree(git_common_dir);
- strbuf_addf(&path, "%s/worktrees", get_git_common_dir());
+ strbuf_addf(&path, "%s/worktrees", git_common_dir);
dir = opendir(path.buf);
strbuf_release(&path);
if (dir) {
@@ -188,7 +191,7 @@ struct worktree **get_worktrees(unsigned flags)
if (is_dot_or_dotdot(d->d_name))
continue;
- if ((linked = get_linked_worktree(d->d_name))) {
+ if ((linked = get_linked_worktree(git_common_dir, d->d_name))) {
ALLOC_GROW(list, counter + 1, alloc);
list[counter++] = linked;
}
@@ -209,6 +212,34 @@ struct worktree **get_worktrees(unsigned flags)
return list;
}
+struct worktree **get_worktrees(unsigned flags)
+{
+ return get_worktrees_internal(get_git_common_dir(), flags);
+}
+
+/*
+ * NEEDSWORK: The values in the returned worktrees are broken, e.g.
+ * the refs or path resolution is influenced by the current repository.
+ */
+static struct worktree **get_submodule_worktrees(const char *path, unsigned flags)
+{
+ char *submodule_gitdir;
+ struct strbuf sb = STRBUF_INIT;
+ struct worktree **ret;
+
+ submodule_gitdir = git_pathdup_submodule(path, "%s", "");
+ if (!submodule_gitdir)
+ return NULL;
+
+ /* the env would be set for the superproject */
+ get_common_dir_noenv(&sb, submodule_gitdir);
+ ret = get_worktrees_internal(sb.buf, flags);
+
+ strbuf_release(&sb);
+ free(submodule_gitdir);
+ return ret;
+}
+
const char *get_worktree_git_dir(const struct worktree *wt)
{
if (!wt)
@@ -380,3 +411,20 @@ const struct worktree *find_shared_symref(const char *symref,
return existing;
}
+
+int uses_worktrees(void)
+{
+ struct worktree **worktrees = get_worktrees(GWT_LINKED_ONLY);
+ int retval = (worktrees != NULL && worktrees[0] != NULL);
+ free_worktrees(worktrees);
+ return retval;
+}
+
+int submodule_uses_worktrees(const char *path)
+{
+ struct worktree **worktrees =
+ get_submodule_worktrees(path, GWT_LINKED_ONLY);
+ int retval = (worktrees != NULL && worktrees[0] != NULL);
+ free_worktrees(worktrees);
+ return retval;
+}
diff --git a/worktree.h b/worktree.h
index d59ce1fee8..ebe0856330 100644
--- a/worktree.h
+++ b/worktree.h
@@ -16,6 +16,7 @@ struct worktree {
/* Functions for acting on the information about worktrees. */
#define GWT_SORT_LINKED (1 << 0) /* keeps linked worktrees sorted */
+#define GWT_LINKED_ONLY (1 << 1) /* do not include the main worktree */
/*
* Get the worktrees. The primary worktree will always be the first returned,
@@ -27,6 +28,12 @@ struct worktree {
*/
extern struct worktree **get_worktrees(unsigned flags);
+/*
+ * Returns 1 if linked worktrees exist, 0 otherwise.
+ */
+extern int uses_worktrees(void);
+extern int submodule_uses_worktrees(const char *path);
+
/*
* Return git dir of the worktree. Note that the path may be relative.
* If wt is NULL, git dir of current worktree is returned.
--
2.11.0.rc2.29.g7c00390.dirty
^ permalink raw reply related
* [PATCHv7 3/6] test-lib-functions.sh: teach test_commit -C <dir>
From: Stefan Beller @ 2016-12-08 21:03 UTC (permalink / raw)
To: bmwill; +Cc: git, pclouds, gitster, Stefan Beller
In-Reply-To: <20161208210329.12919-1-sbeller@google.com>
Specifically when setting up submodule tests, it comes in handy if
we can create commits in repositories that are not at the root of
the tested trash dir. Add "-C <dir>" similar to gits -C parameter
that will perform the operation in the given directory.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/test-lib-functions.sh | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index fdaeb3a96b..579e812506 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -157,16 +157,21 @@ debug () {
GIT_TEST_GDB=1 "$@"
}
-# Call test_commit with the arguments "<message> [<file> [<contents> [<tag>]]]"
+# Call test_commit with the arguments
+# [-C <directory>] <message> [<file> [<contents> [<tag>]]]"
#
# This will commit a file with the given contents and the given commit
# message, and tag the resulting commit with the given tag name.
#
# <file>, <contents>, and <tag> all default to <message>.
+#
+# If the first argument is "-C", the second argument is used as a path for
+# the git invocations.
test_commit () {
notick= &&
signoff= &&
+ indir= &&
while test $# != 0
do
case "$1" in
@@ -176,21 +181,26 @@ test_commit () {
--signoff)
signoff="$1"
;;
+ -C)
+ indir="$2"
+ shift
+ ;;
*)
break
;;
esac
shift
done &&
+ indir=${indir:+"$indir"/} &&
file=${2:-"$1.t"} &&
- echo "${3-$1}" > "$file" &&
- git add "$file" &&
+ echo "${3-$1}" > "$indir$file" &&
+ git ${indir:+ -C "$indir"} add "$file" &&
if test -z "$notick"
then
test_tick
fi &&
- git commit $signoff -m "$1" &&
- git tag "${4:-$1}"
+ git ${indir:+ -C "$indir"} commit $signoff -m "$1" &&
+ git ${indir:+ -C "$indir"} tag "${4:-$1}"
}
# Call test_merge with the arguments "<message> <commit>", where <commit>
--
2.11.0.rc2.29.g7c00390.dirty
^ permalink raw reply related
* [PATCHv7 5/6] move connect_work_tree_and_git_dir to dir.h
From: Stefan Beller @ 2016-12-08 21:03 UTC (permalink / raw)
To: bmwill; +Cc: git, pclouds, gitster, Stefan Beller
In-Reply-To: <20161208210329.12919-1-sbeller@google.com>
That function was primarily used by submodule code, but the function
itself is not inherently about submodules. In the next patch we'll
introduce relocate_git_dir, which can be used by worktrees as well,
so find a neutral middle ground in dir.h.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
dir.c | 25 +++++++++++++++++++++++++
dir.h | 1 +
submodule.c | 25 -------------------------
submodule.h | 1 -
4 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/dir.c b/dir.c
index bfa8c8a9a5..e0efd3c2c3 100644
--- a/dir.c
+++ b/dir.c
@@ -2748,3 +2748,28 @@ void untracked_cache_add_to_index(struct index_state *istate,
{
untracked_cache_invalidate_path(istate, path);
}
+
+/* Update gitfile and core.worktree setting to connect work tree and git dir */
+void connect_work_tree_and_git_dir(const char *work_tree_, const char *git_dir_)
+{
+ struct strbuf file_name = STRBUF_INIT;
+ struct strbuf rel_path = STRBUF_INIT;
+ char *git_dir = xstrdup(real_path(git_dir_));
+ char *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, work_tree, &rel_path));
+
+ /* Update core.worktree setting */
+ strbuf_reset(&file_name);
+ strbuf_addf(&file_name, "%s/config", git_dir);
+ git_config_set_in_file(file_name.buf, "core.worktree",
+ relative_path(work_tree, git_dir, &rel_path));
+
+ strbuf_release(&file_name);
+ strbuf_release(&rel_path);
+ free(work_tree);
+ free(git_dir);
+}
diff --git a/dir.h b/dir.h
index 97c83bb383..051674a431 100644
--- a/dir.h
+++ b/dir.h
@@ -335,4 +335,5 @@ struct untracked_cache *read_untracked_extension(const void *data, unsigned long
void write_untracked_extension(struct strbuf *out, struct untracked_cache *untracked);
void add_untracked_cache(struct index_state *istate);
void remove_untracked_cache(struct index_state *istate);
+extern void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
#endif
diff --git a/submodule.c b/submodule.c
index d4f7afe2f1..0bb50b4b62 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1222,31 +1222,6 @@ int merge_submodule(unsigned char result[20], const char *path,
return 0;
}
-/* Update gitfile and core.worktree setting to connect work tree and git dir */
-void connect_work_tree_and_git_dir(const char *work_tree_, const char *git_dir_)
-{
- struct strbuf file_name = STRBUF_INIT;
- struct strbuf rel_path = STRBUF_INIT;
- char *git_dir = xstrdup(real_path(git_dir_));
- char *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, work_tree, &rel_path));
-
- /* Update core.worktree setting */
- strbuf_reset(&file_name);
- strbuf_addf(&file_name, "%s/config", git_dir);
- git_config_set_in_file(file_name.buf, "core.worktree",
- relative_path(work_tree, git_dir, &rel_path));
-
- strbuf_release(&file_name);
- strbuf_release(&rel_path);
- free(work_tree);
- free(git_dir);
-}
-
int parallel_submodules(void)
{
return parallel_jobs;
diff --git a/submodule.h b/submodule.h
index d9e197a948..4e3bf469b4 100644
--- a/submodule.h
+++ b/submodule.h
@@ -65,7 +65,6 @@ int merge_submodule(unsigned char result[20], const char *path, const unsigned c
int find_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name,
struct string_list *needs_pushing);
int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name);
-void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
int parallel_submodules(void);
/*
--
2.11.0.rc2.29.g7c00390.dirty
^ permalink raw reply related
* [PATCHv7 2/6] submodule helper: support super prefix
From: Stefan Beller @ 2016-12-08 21:03 UTC (permalink / raw)
To: bmwill; +Cc: git, pclouds, gitster, Stefan Beller
In-Reply-To: <20161208210329.12919-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..5643848667 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 *);
+ unsigned 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.29.g7c00390.dirty
^ permalink raw reply related
* [PATCHv7 1/6] submodule: use absolute path for computing relative path connecting
From: Stefan Beller @ 2016-12-08 21:03 UTC (permalink / raw)
To: bmwill; +Cc: git, pclouds, gitster, Stefan Beller
In-Reply-To: <20161208210329.12919-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 | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/submodule.c b/submodule.c
index 6f7d883de9..d4f7afe2f1 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1223,27 +1223,28 @@ int merge_submodule(unsigned char result[20], const char *path,
}
/* Update gitfile and core.worktree setting to connect work tree and git dir */
-void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)
+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 *git_dir = xstrdup(real_path(git_dir_));
+ char *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(git_dir, work_tree, &rel_path));
/* Update core.worktree setting */
strbuf_reset(&file_name);
strbuf_addf(&file_name, "%s/config", git_dir);
git_config_set_in_file(file_name.buf, "core.worktree",
- relative_path(real_work_tree, git_dir,
- &rel_path));
+ relative_path(work_tree, git_dir, &rel_path));
strbuf_release(&file_name);
strbuf_release(&rel_path);
- free((void *)real_work_tree);
+ free(work_tree);
+ free(git_dir);
}
int parallel_submodules(void)
--
2.11.0.rc2.29.g7c00390.dirty
^ permalink raw reply related
* [PATCHv7 0/6] submodule absorbgitdirs
From: Stefan Beller @ 2016-12-08 21:03 UTC (permalink / raw)
To: bmwill; +Cc: git, pclouds, gitster, Stefan Beller
v7:
* do not expose submodule_get_worktrees. The values may be wrong internally,
but for our purpose we do not care about the actual values, only the count.
Document the wrong values!
* more strings are _(marked up)
* renamed variables in connect_work_tree_and_git_dir
* unsigned instead of int for options in the submodule helper.
*
v6:
* renamed embedgitdirs to absorbgitdirs embedding may be interpreted as
embedding the git dir into the working directory, whereas absorbing sounds
more like the submodule is absorbed by the superproject, making the
submodule less independent
* Worktrees API offer uses_worktrees(void) and submodule_uses_worktree(path).
* moved the printing to stderr and one layer up (out of the pure
relocate_git_dir function).
* connect_... is in dir.h now.
v5:
* Add another layer of abstraction, i.e. the relocate_git_dir is only about
moving a git dir of one repository. The submodule specific stuff (e.g.
recursion into nested submodules) is in submodule.{c,h}
This was motivated by reviews on the series of checkout aware of submodules
building on top of this series, as we want to directly call the embed-git-dirs
function without the overhead of spawning a child process.
v4:
* rebuilt on top of nd/worktree-list-fixup
* fix and test behavior for un-init submodules (don't crash, rather do nothing)
* incorporated a "static" as pointed out by Ramsay
* use internal functions instead of duplicating code in worktree.c
(use get_common_dir_noenv for the submodule to actually get the common dir)
* fixed a memory leak in relocate_gitdir
v3:
* have a slightly more generic function "relocate_gitdir".
The recursion is strictly related to submodules, though.
* bail out if a submodule is using worktrees.
This also lays the groundwork for later doing the proper thing,
as worktree.h offers a function `get_submodule_worktrees(path)`
* nit by duy: use git_path instead of git_common_dir
v2:
* fixed commit message for patch:
"submodule: use absolute path for computing relative path connecting"
* a new patch "submodule helper: support super prefix"
* redid the final patch with more tests and fixing bugs along the way
* "test-lib-functions.sh: teach test_commit -C <dir>" unchanged
v1:
The discussion of the submodule checkout series revealed to me that a command
is needed to move the git directory from the submodules working tree to be
embedded into the superprojects git directory.
So I wrote the code to intern the submodules git dir into the superproject,
but whilst writing the code I realized this could be valueable for our use
in testing too. So I exposed it via the submodule--helper. But as the
submodule helper ought to be just an internal API, we could also
offer it via the proper submodule command.
The command as it is has little value to the end user for now, but
breaking it out of the submodule checkout series hopefully makes review easier.
Thanks,
Stefan
Stefan Beller (6):
submodule: use absolute path for computing relative path connecting
submodule helper: support super prefix
test-lib-functions.sh: teach test_commit -C <dir>
worktree: have a function to check if worktrees are in use
move connect_work_tree_and_git_dir to dir.h
submodule: add absorb-git-dir function
Documentation/git-submodule.txt | 15 +++++
builtin/submodule--helper.c | 69 ++++++++++++++++----
dir.c | 37 +++++++++++
dir.h | 4 ++
git-submodule.sh | 7 +-
git.c | 2 +-
submodule.c | 127 ++++++++++++++++++++++++++++++-------
submodule.h | 5 +-
t/t7412-submodule-absorbgitdirs.sh | 101 +++++++++++++++++++++++++++++
t/test-lib-functions.sh | 20 ++++--
worktree.c | 68 +++++++++++++++++---
worktree.h | 7 ++
12 files changed, 409 insertions(+), 53 deletions(-)
create mode 100755 t/t7412-submodule-absorbgitdirs.sh
--
2.11.0.rc2.29.g7c00390.dirty
^ permalink raw reply
* Re: [PATCHv6 2/7] submodule helper: support super prefix
From: Stefan Beller @ 2016-12-08 20:19 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Brandon Williams, Git Mailing List, Junio C Hamano
In-Reply-To: <CACsJy8AohKMTm0inUyxu=kTwxNFcCbbBRO=O126SCzi5e=0tSQ@mail.gmail.com>
>
> unsigned int is probably safer for variables that are used as bit-flags.
done
> If it's meant for users to see, please _() the string.
done
>> + { "submodule--helper", cmd_submodule__helper, RUN_SETUP | SUPPORT_SUPER_PREFIX},
>
> The same macro defined twice in two separate .c files? Hmm.. it
> confused me a bit because i thought there was a connection.. I guess
> it's ok.
Its effect is exactly the same. just plain code duplication. I assume that
is okay here as it really is only one constant for now. Maybe we can
refactor that to
be defined in a header file (but which? There is no such thing as a git.h, where
I'd expect to put it.)
Thanks,
Stefan
^ permalink raw reply
* Feature request: read git config from parent directory
From: Dominique Dumont @ 2016-12-08 19:49 UTC (permalink / raw)
To: git
Hello
I use the same machine for work and open-source contribution. In both cases, I
deal with a lot of repositories. Depending on whether I commit for work or
open-source activities, I must use a different mail address. I used to setup
work address for each work repo in git local config, but this is no longer
practical.
Since I use different directories for work and open-source, would it be
possible for git to read irs config also from parent directories ? I.e. setup
git to read config from ./.git/config then ../.gitconfig, ../../gitconfig until
global ~/.gitconfig.
All the best
--
https://github.com/dod38fr/ -o- http://search.cpan.org/~ddumont/
http://ddumont.wordpress.com/ -o- irc: dod at irc.debian.org
^ permalink raw reply
* Re: [PATCHv2 7/7] versioncmp: generalize version sort suffix reordering
From: Junio C Hamano @ 2016-12-08 19:36 UTC (permalink / raw)
To: SZEDER Gábor
Cc: Jeff King, Nguyễn Thái Ngọc Duy, Leho Kraav, git
In-Reply-To: <20161208142401.1329-8-szeder.dev@gmail.com>
SZEDER Gábor <szeder.dev@gmail.com> writes:
> The 'versionsort.prereleaseSuffix' configuration variable, as its name
> suggests, is supposed to only deal with tagnames with prerelease
> suffixes, and allows sorting those prerelease tags in a user-defined
> order before the suffixless main release tag, instead of sorting them
> simply lexicographically.
>
> However, the previous changes in this series resulted in an
> interesting and useful property of version sort:
>
> - The empty string as a configured suffix matches all tagnames,
> including tagnames without any suffix, but
>
> - tagnames containing a "real" configured suffix are still ordered
> according to that real suffix, because any longer suffix takes
> precedence over the empty string.
>
> Exploiting this property we can easily generalize suffix reordering
> and specify the order of tags with given suffixes not only before but
> even after a main release tag by using the empty suffix to denote the
> position of the main release tag, without any algorithm changes:
>
> $ git -c versionsort.prereleaseSuffix=-alpha \
> -c versionsort.prereleaseSuffix=-beta \
> -c versionsort.prereleaseSuffix="" \
> -c versionsort.prereleaseSuffix=-gamma \
> -c versionsort.prereleaseSuffix=-delta \
> tag -l --sort=version:refname 'v3.0*'
> v3.0-alpha1
> v3.0-beta1
> v3.0
> v3.0-gamma1
> v3.0-delta1
>
> Since 'versionsort.prereleaseSuffix' is not a fitting name for a
> configuration variable to control this more general suffix reordering,
> introduce the new variable 'versionsort.suffix'. Still keep the old
> configuration variable name as a deprecated alias, though, to avoid
> suddenly breaking setups already using it. Ignore the old variable if
> both old and new configuration variables are set, but emit a warning
> so users will be aware of it and can fix their configuration. Extend
> the documentation to describe and add a test to check this more
> general behavior.
>
> Note: since the empty suffix matches all tagnames, tagnames with
> suffixes not included in the configuration are listed together with
> the suffixless main release tag, ordered lexicographically right after
> that, i.e. before tags with suffixes listed in the configuration
> following the empty suffix.
Thanks. Will comment on the individual patches later, but the end
result looks very nice.
^ permalink raw reply
* Re: [PATCHv6 5/7] worktree: add function to check if worktrees are in use
From: Stefan Beller @ 2016-12-08 19:32 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Brandon Williams, Git Mailing List, Junio C Hamano
In-Reply-To: <CACsJy8DMhC7Umnqf+-ViOBKn46Emtan4647=MDdLoYXVNB0=6A@mail.gmail.com>
On Thu, Dec 8, 2016 at 2:51 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Thu, Dec 8, 2016 at 5:40 PM, Duy Nguyen <pclouds@gmail.com> wrote:
>> Alternatively, we could add a new flag to get_worktrees() to tell it
>> to return all worktrees if there is a least one linked worktree, or
>> return NULL if there's only main worktree. I'm not sure if this is
>> clever or very stupid.
>
> No, this may be better. Add a flag to say "returns linked worktrees
> _only_". Which means when you're in a "normal" repo, get_worktrees()
> with this flag returns NULL. When you're in a multiple-worktree repo,
> it returns all linked worktrees (no main worktree). I think I might
> have a use for this flag in addition to this uses_worktrees() here.
> uses_worktrees() look quite simple with that flag
>
> int uses_worktrees(void)
> {
> struct worktree **worktrees = get_worktrees(WT_LINKED_ONLY);
> int retval = worktrees != NULL;
I am interested in the submodule case however, where we already return NULL
e.g. when the submodule git dir cannot be found. Actually that would
work out fine
as well:
/* NOTE on accuracy of result, hence not exposed. */
static worktree **submodule_get_worktrees(const char *path, unsigned flags)
..
int submodule_uses_worktrees(const char *path)
{
struct worktree **worktrees = submodule_get_worktrees(path,
WT_LINKED_ONLY);
int retval = worktrees != NULL;
free_worktrees(worktrees);
return retval;
}
Thanks for that inspiration!
^ permalink raw reply
* Re: [PATCH 4/5] Make sequencer abort safer
From: Stephan Beyer @ 2016-12-08 19:17 UTC (permalink / raw)
To: Junio C Hamano, Johannes Schindelin
Cc: git, Christian Couder, SZEDER Gábor
In-Reply-To: <xmqqr35itjor.fsf@gitster.mtv.corp.google.com>
Hi,
I'm a little afraid of feeding Parkinson's law of triviality here, but... ;)
On 12/08/2016 06:27 PM, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> On Wed, 7 Dec 2016, Stephan Beyer wrote:
>>
>>> diff --git a/sequencer.c b/sequencer.c
>>> index 30b10ba14..c9b560ac1 100644
>>> --- a/sequencer.c
>>> +++ b/sequencer.c
>>> @@ -27,6 +27,7 @@ GIT_PATH_FUNC(git_path_seq_dir, "sequencer")
>>> static GIT_PATH_FUNC(git_path_todo_file, "sequencer/todo")
>>> static GIT_PATH_FUNC(git_path_opts_file, "sequencer/opts")
>>> static GIT_PATH_FUNC(git_path_head_file, "sequencer/head")
>>> +static GIT_PATH_FUNC(git_path_curr_file, "sequencer/current")
>>
>> Is it required by law to have a four-letter infix, or can we have a nicer
>> variable name (e.g. git_path_current_file)?
>
> I agree with you that, as other git_path_*_file variables match the
> actual name on the filesystem, this one should too, together with
> the update_curr_file() function.
I totally agree with that (and I don't know why I used "curr", probably
just because it looked consistent and good...).
However:
> -static void update_curr_file()
> +static void update_current_file(void)
This function name could lead to the impression that there is some
current file (defined by a global state or whatever) that is updated.
So I'd rather rename the *file* to one of
* sequencer/abort-safety (consistent to am, describes its purpose)
* sequencer/safety (shorter, still describes the purpose)
* sequencer/current-head (describes what it contains)
* sequencer/last (a four-letter word, not totally unambiguous though)
> By the way, this step seems to be a fix to an existing problem, and
> the new test added in 3/5 seems to be a demonstration of the issue.
> If that is the case, shouldn't the new test initially expect failure
> and updated by this step to expect success?
That's usually a matter of taste that I sometimes also discuss with
colleagues in other projects... However, for the git test suite with its
"known breakage" behavior, your recommendation is surely the best way to
do it (aside from introducing the test and the fix in one commit... but
that does not show in the history that there actually was that breakage)
~Stephan
^ permalink raw reply
* Re: [PATCH] real_path: make real_path thread-safe
From: Brandon Williams @ 2016-12-08 19:02 UTC (permalink / raw)
To: Johannes Sixt
Cc: Torsten Bögershausen, Junio C Hamano, Ramsay Jones, git,
sbeller, peff, jacob.keller
In-Reply-To: <6cae7549-72f2-e591-ad48-28d449a62caf@kdbg.org>
On 12/08, Johannes Sixt wrote:
> Am 08.12.2016 um 08:55 schrieb Torsten Bögershausen:
> >Some conversion may be done in mingw.c:
> >https://github.com/github/git-msysgit/blob/master/compat/mingw.c
> >So what I understand, '/' in Git are already converted into '\' if needed ?
>
> Only if needed, and there are not many places where this is the
> case. (Actually, I can't find one place where we do.) In particular,
> typical file accesses does not require the conversion.
That's convenient, that way I don't have to worry about using '\' as a
directory separator instead of '/'.
>
> >It seams that we may wnat a function get_start_of_path(uncpath),
> >which returns:
> >
> >get_start_of_path_win("//?/D:/very-long-path") "/very-long-path"
>
> We have offset_1st_component().
Thanks for letting me know this function exists, that makes my job a bit
easier.
--
Brandon Williams
^ permalink raw reply
* [PATCH v2 07/16] pathspec: remove unused variable from unsupported_magic
From: Brandon Williams @ 2016-12-08 18:59 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, pclouds, gitster
In-Reply-To: <1481223550-65277-1-git-send-email-bmwill@google.com>
Removed unused variable 'n' from the 'unsupported_magic()' function.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
pathspec.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/pathspec.c b/pathspec.c
index 8f367f0..ec0d590 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -333,8 +333,8 @@ static void NORETURN unsupported_magic(const char *pattern,
unsigned short_magic)
{
struct strbuf sb = STRBUF_INIT;
- int i, n;
- for (n = i = 0; i < ARRAY_SIZE(pathspec_magic); i++) {
+ int i;
+ for (i = 0; i < ARRAY_SIZE(pathspec_magic); i++) {
const struct pathspec_magic *m = pathspec_magic + i;
if (!(magic & m->bit))
continue;
@@ -344,7 +344,6 @@ static void NORETURN unsupported_magic(const char *pattern,
strbuf_addf(&sb, "'%c'", m->mnemonic);
else
strbuf_addf(&sb, "'%s'", m->name);
- n++;
}
/*
* We may want to substitute "this command" with a command
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v2 05/16] pathspec: remove the deprecated get_pathspec function
From: Brandon Williams @ 2016-12-08 18:58 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, pclouds, gitster
In-Reply-To: <1481223550-65277-1-git-send-email-bmwill@google.com>
Now that all callers of the old 'get_pathspec' interface have been
migrated to use the new pathspec struct interface it can be removed
from the codebase.
Since there are no more users of the '_raw' field in the pathspec struct
it can also be removed. This patch also removes the old functionality
of modifying the const char **argv array that was passed into
parse_pathspec. Instead the constructed 'match' string (which is a
pathspec element with the prefix prepended) is only stored in its
corresponding pathspec_item entry.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
Documentation/technical/api-setup.txt | 2 --
cache.h | 1 -
pathspec.c | 42 +++--------------------------------
pathspec.h | 1 -
4 files changed, 3 insertions(+), 43 deletions(-)
diff --git a/Documentation/technical/api-setup.txt b/Documentation/technical/api-setup.txt
index 540e455..eb1fa98 100644
--- a/Documentation/technical/api-setup.txt
+++ b/Documentation/technical/api-setup.txt
@@ -27,8 +27,6 @@ parse_pathspec(). This function takes several arguments:
- prefix and args come from cmd_* functions
-get_pathspec() is obsolete and should never be used in new code.
-
parse_pathspec() helps catch unsupported features and reject them
politely. At a lower level, different pathspec-related functions may
not support the same set of features. Such pathspec-sensitive
diff --git a/cache.h b/cache.h
index a50a61a..0f80e01 100644
--- a/cache.h
+++ b/cache.h
@@ -514,7 +514,6 @@ extern void set_git_work_tree(const char *tree);
#define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES"
-extern const char **get_pathspec(const char *prefix, const char **pathspec);
extern void setup_work_tree(void);
extern const char *setup_git_directory_gently(int *);
extern const char *setup_git_directory(void);
diff --git a/pathspec.c b/pathspec.c
index 22ca74a..1f918cb 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -103,7 +103,7 @@ static void prefix_short_magic(struct strbuf *sb, int prefixlen,
*/
static unsigned prefix_pathspec(struct pathspec_item *item,
unsigned *p_short_magic,
- const char **raw, unsigned flags,
+ unsigned flags,
const char *prefix, int prefixlen,
const char *elt)
{
@@ -240,7 +240,7 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
if (!match)
die(_("%s: '%s' is outside repository"), elt, copyfrom);
}
- *raw = item->match = match;
+ item->match = match;
/*
* Prefix the pathspec (keep all magic) and assign to
* original. Useful for passing to another command.
@@ -381,8 +381,6 @@ void parse_pathspec(struct pathspec *pathspec,
/* No arguments with prefix -> prefix pathspec */
if (!entry) {
- static const char *raw[2];
-
if (flags & PATHSPEC_PREFER_FULL)
return;
@@ -394,10 +392,7 @@ void parse_pathspec(struct pathspec *pathspec,
item->original = prefix;
item->nowildcard_len = item->len = strlen(prefix);
item->prefix = item->len;
- raw[0] = prefix;
- raw[1] = NULL;
pathspec->nr = 1;
- pathspec->_raw = raw;
return;
}
@@ -415,7 +410,6 @@ void parse_pathspec(struct pathspec *pathspec,
pathspec->nr = n;
ALLOC_ARRAY(pathspec->items, n);
item = pathspec->items;
- pathspec->_raw = argv;
prefixlen = prefix ? strlen(prefix) : 0;
for (i = 0; i < n; i++) {
@@ -423,7 +417,7 @@ void parse_pathspec(struct pathspec *pathspec,
entry = argv[i];
item[i].magic = prefix_pathspec(item + i, &short_magic,
- argv + i, flags,
+ flags,
prefix, prefixlen, entry);
if ((flags & PATHSPEC_LITERAL_PATH) &&
!(magic_mask & PATHSPEC_LITERAL))
@@ -457,36 +451,6 @@ void parse_pathspec(struct pathspec *pathspec,
}
}
-/*
- * N.B. get_pathspec() is deprecated in favor of the "struct pathspec"
- * based interface - see pathspec.c:parse_pathspec().
- *
- * Arguments:
- * - prefix - a path relative to the root of the working tree
- * - pathspec - a list of paths underneath the prefix path
- *
- * Iterates over pathspec, prepending each path with prefix,
- * and return the resulting list.
- *
- * If pathspec is empty, return a singleton list containing prefix.
- *
- * If pathspec and prefix are both empty, return an empty list.
- *
- * This is typically used by built-in commands such as add.c, in order
- * to normalize argv arguments provided to the built-in into a list of
- * paths to process, all relative to the root of the working tree.
- */
-const char **get_pathspec(const char *prefix, const char **pathspec)
-{
- struct pathspec ps;
- parse_pathspec(&ps,
- PATHSPEC_ALL_MAGIC &
- ~(PATHSPEC_FROMTOP | PATHSPEC_LITERAL),
- PATHSPEC_PREFER_CWD,
- prefix, pathspec);
- return ps._raw;
-}
-
void copy_pathspec(struct pathspec *dst, const struct pathspec *src)
{
*dst = *src;
diff --git a/pathspec.h b/pathspec.h
index 59809e4..70a592e 100644
--- a/pathspec.h
+++ b/pathspec.h
@@ -19,7 +19,6 @@
#define PATHSPEC_ONESTAR 1 /* the pathspec pattern satisfies GFNM_ONESTAR */
struct pathspec {
- const char **_raw; /* get_pathspec() result, not freed by clear_pathspec() */
int nr;
unsigned int has_wildcard:1;
unsigned int recursive:1;
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v2 04/16] ls-tree: convert show_recursive to use the pathspec struct interface
From: Brandon Williams @ 2016-12-08 18:58 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, pclouds, gitster
In-Reply-To: <1481223550-65277-1-git-send-email-bmwill@google.com>
Convert 'show_recursive()' to use the pathspec struct interface from
using the '_raw' entry in the pathspec struct.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
builtin/ls-tree.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index 0e30d86..d7ebeb4 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -31,21 +31,18 @@ static const char * const ls_tree_usage[] = {
static int show_recursive(const char *base, int baselen, const char *pathname)
{
- const char **s;
+ int i;
if (ls_options & LS_RECURSIVE)
return 1;
- s = pathspec._raw;
- if (!s)
+ if (!pathspec.nr)
return 0;
- for (;;) {
- const char *spec = *s++;
+ for (i = 0; i < pathspec.nr; i++) {
+ const char *spec = pathspec.items[i].match;
int len, speclen;
- if (!spec)
- return 0;
if (strncmp(base, spec, baselen))
continue;
len = strlen(pathname);
@@ -59,6 +56,7 @@ static int show_recursive(const char *base, int baselen, const char *pathname)
continue;
return 1;
}
+ return 0;
}
static int show_tree(const unsigned char *sha1, struct strbuf *base,
@@ -175,8 +173,8 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
* cannot be lifted until it is converted to use
* match_pathspec() or tree_entry_interesting()
*/
- parse_pathspec(&pathspec, PATHSPEC_GLOB | PATHSPEC_ICASE |
- PATHSPEC_EXCLUDE,
+ parse_pathspec(&pathspec, PATHSPEC_ALL_MAGIC &
+ ~(PATHSPEC_FROMTOP | PATHSPEC_LITERAL),
PATHSPEC_PREFER_CWD,
prefix, argv + 1);
for (i = 0; i < pathspec.nr; i++)
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v2 08/16] pathspec: always show mnemonic and name in unsupported_magic
From: Brandon Williams @ 2016-12-08 18:59 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, pclouds, gitster
In-Reply-To: <1481223550-65277-1-git-send-email-bmwill@google.com>
For better clarity, always show the mnemonic and name of the unsupported
magic being used. This lets users have a more clear understanding of
what magic feature isn't supported. And if they supplied a mnemonic,
the user will be told what its corresponding name is which will allow
them to more easily search the man pages for that magic type.
This also avoids passing an extra parameter around the pathspec
initialization code.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
pathspec.c | 23 ++++++++---------------
1 file changed, 8 insertions(+), 15 deletions(-)
diff --git a/pathspec.c b/pathspec.c
index ec0d590..a360193 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -68,7 +68,7 @@ static struct pathspec_magic {
const char *name;
} pathspec_magic[] = {
{ PATHSPEC_FROMTOP, '/', "top" },
- { PATHSPEC_LITERAL, 0, "literal" },
+ { PATHSPEC_LITERAL,'\0', "literal" },
{ PATHSPEC_GLOB, '\0', "glob" },
{ PATHSPEC_ICASE, '\0', "icase" },
{ PATHSPEC_EXCLUDE, '!', "exclude" },
@@ -101,9 +101,7 @@ static void prefix_short_magic(struct strbuf *sb, int prefixlen,
* the prefix part must always match literally, and a single stupid
* string cannot express such a case.
*/
-static unsigned prefix_pathspec(struct pathspec_item *item,
- unsigned *p_short_magic,
- unsigned flags,
+static unsigned prefix_pathspec(struct pathspec_item *item, unsigned flags,
const char *prefix, int prefixlen,
const char *elt)
{
@@ -210,7 +208,6 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
}
magic |= short_magic;
- *p_short_magic = short_magic;
/* --noglob-pathspec adds :(literal) _unless_ :(glob) is specified */
if (noglob_global && !(magic & PATHSPEC_GLOB))
@@ -329,8 +326,7 @@ static int pathspec_item_cmp(const void *a_, const void *b_)
}
static void NORETURN unsupported_magic(const char *pattern,
- unsigned magic,
- unsigned short_magic)
+ unsigned magic)
{
struct strbuf sb = STRBUF_INIT;
int i;
@@ -340,8 +336,9 @@ static void NORETURN unsupported_magic(const char *pattern,
continue;
if (sb.len)
strbuf_addch(&sb, ' ');
- if (short_magic & m->bit)
- strbuf_addf(&sb, "'%c'", m->mnemonic);
+
+ if (m->mnemonic)
+ strbuf_addf(&sb, "'(%c)%s'", m->mnemonic, m->name);
else
strbuf_addf(&sb, "'%s'", m->name);
}
@@ -413,11 +410,9 @@ void parse_pathspec(struct pathspec *pathspec,
prefixlen = prefix ? strlen(prefix) : 0;
for (i = 0; i < n; i++) {
- unsigned short_magic;
entry = argv[i];
- item[i].magic = prefix_pathspec(item + i, &short_magic,
- flags,
+ item[i].magic = prefix_pathspec(item + i, flags,
prefix, prefixlen, entry);
if ((flags & PATHSPEC_LITERAL_PATH) &&
!(magic_mask & PATHSPEC_LITERAL))
@@ -425,9 +420,7 @@ void parse_pathspec(struct pathspec *pathspec,
if (item[i].magic & PATHSPEC_EXCLUDE)
nr_exclude++;
if (item[i].magic & magic_mask)
- unsupported_magic(entry,
- item[i].magic & magic_mask,
- short_magic);
+ unsupported_magic(entry, item[i].magic & magic_mask);
if ((flags & PATHSPEC_SYMLINK_LEADING_PATH) &&
has_symlink_leading_path(item[i].match, item[i].len)) {
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v2 14/16] pathspec: create strip submodule slash helpers
From: Brandon Williams @ 2016-12-08 18:59 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, pclouds, gitster
In-Reply-To: <1481223550-65277-1-git-send-email-bmwill@google.com>
Factor out the logic responsible for stripping the trailing slash on
pathspecs referencing submodules into its own function.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
pathspec.c | 68 ++++++++++++++++++++++++++++++++++++++------------------------
1 file changed, 42 insertions(+), 26 deletions(-)
diff --git a/pathspec.c b/pathspec.c
index f37f887..cf88390 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -258,6 +258,44 @@ static const char *parse_element_magic(unsigned *magic, int *prefix_len,
return parse_short_magic(magic, elem);
}
+static void strip_submodule_slash_cheap(struct pathspec_item *item)
+{
+ int i;
+
+ if ((item->len >= 1 && item->match[item->len - 1] == '/') &&
+ (i = cache_name_pos(item->match, item->len - 1)) >= 0 &&
+ S_ISGITLINK(active_cache[i]->ce_mode)) {
+ item->len--;
+ item->match[item->len] = '\0';
+ }
+}
+
+static void strip_submodule_slash_expensive(struct pathspec_item *item)
+{
+ int i;
+
+ for (i = 0; i < active_nr; i++) {
+ struct cache_entry *ce = active_cache[i];
+ int ce_len = ce_namelen(ce);
+
+ if (!S_ISGITLINK(ce->ce_mode))
+ continue;
+
+ if (item->len <= ce_len || item->match[ce_len] != '/' ||
+ memcmp(ce->name, item->match, ce_len))
+ continue;
+
+ if (item->len == ce_len + 1) {
+ /* strip trailing slash */
+ item->len--;
+ item->match[item->len] = '\0';
+ } else {
+ die (_("Pathspec '%s' is in submodule '%.*s'"),
+ item->original, ce_len, ce->name);
+ }
+ }
+}
+
/*
* Take an element of a pathspec and check for magic signatures.
* Append the result to the prefix. Return the magic bitmap.
@@ -278,7 +316,7 @@ static unsigned prefix_pathspec(struct pathspec_item *item, unsigned flags,
unsigned magic = 0, element_magic = 0;
const char *copyfrom = elt;
char *match;
- int i, pathspec_prefix = -1;
+ int pathspec_prefix = -1;
/* PATHSPEC_LITERAL_PATH ignores magic */
if (flags & PATHSPEC_LITERAL_PATH) {
@@ -329,33 +367,11 @@ static unsigned prefix_pathspec(struct pathspec_item *item, unsigned flags,
item->len = strlen(item->match);
item->prefix = prefixlen;
- if ((flags & PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP) &&
- (item->len >= 1 && item->match[item->len - 1] == '/') &&
- (i = cache_name_pos(item->match, item->len - 1)) >= 0 &&
- S_ISGITLINK(active_cache[i]->ce_mode)) {
- item->len--;
- match[item->len] = '\0';
- }
+ if (flags & PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP)
+ strip_submodule_slash_cheap(item);
if (flags & PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE)
- for (i = 0; i < active_nr; i++) {
- struct cache_entry *ce = active_cache[i];
- int ce_len = ce_namelen(ce);
-
- if (!S_ISGITLINK(ce->ce_mode))
- continue;
-
- if (item->len <= ce_len || match[ce_len] != '/' ||
- memcmp(ce->name, match, ce_len))
- continue;
- if (item->len == ce_len + 1) {
- /* strip trailing slash */
- item->len--;
- match[item->len] = '\0';
- } else
- die (_("Pathspec '%s' is in submodule '%.*s'"),
- elt, ce_len, ce->name);
- }
+ strip_submodule_slash_expensive(item);
if (magic & PATHSPEC_LITERAL)
item->nowildcard_len = item->len;
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v2 15/16] pathspec: small readability changes
From: Brandon Williams @ 2016-12-08 18:59 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, pclouds, gitster
In-Reply-To: <1481223550-65277-1-git-send-email-bmwill@google.com>
A few small changes to improve readability. This is done by grouping related
assignments, adding blank lines, ensuring lines are <80 characters, and
adding additional comments.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
pathspec.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/pathspec.c b/pathspec.c
index cf88390..4686298 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -336,6 +336,7 @@ static unsigned prefix_pathspec(struct pathspec_item *item, unsigned flags,
if ((magic & PATHSPEC_LITERAL) && (magic & PATHSPEC_GLOB))
die(_("%s: 'literal' and 'glob' are incompatible"), elt);
+ /* Create match string which will be used for pathspec matching */
if (pathspec_prefix >= 0) {
match = xstrdup(copyfrom);
prefixlen = pathspec_prefix;
@@ -343,11 +344,16 @@ static unsigned prefix_pathspec(struct pathspec_item *item, unsigned flags,
match = xstrdup(copyfrom);
prefixlen = 0;
} else {
- match = prefix_path_gently(prefix, prefixlen, &prefixlen, copyfrom);
+ match = prefix_path_gently(prefix, prefixlen,
+ &prefixlen, copyfrom);
if (!match)
die(_("%s: '%s' is outside repository"), elt, copyfrom);
}
+
item->match = match;
+ item->len = strlen(item->match);
+ item->prefix = prefixlen;
+
/*
* Prefix the pathspec (keep all magic) and assign to
* original. Useful for passing to another command.
@@ -364,8 +370,6 @@ static unsigned prefix_pathspec(struct pathspec_item *item, unsigned flags,
} else {
item->original = xstrdup(elt);
}
- item->len = strlen(item->match);
- item->prefix = prefixlen;
if (flags & PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP)
strip_submodule_slash_cheap(item);
@@ -373,13 +377,14 @@ static unsigned prefix_pathspec(struct pathspec_item *item, unsigned flags,
if (flags & PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE)
strip_submodule_slash_expensive(item);
- if (magic & PATHSPEC_LITERAL)
+ if (magic & PATHSPEC_LITERAL) {
item->nowildcard_len = item->len;
- else {
+ } else {
item->nowildcard_len = simple_length(item->match);
if (item->nowildcard_len < prefixlen)
item->nowildcard_len = prefixlen;
}
+
item->flags = 0;
if (magic & PATHSPEC_GLOB) {
/*
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v2 13/16] pathspec: create parse_element_magic helper
From: Brandon Williams @ 2016-12-08 18:59 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, pclouds, gitster
In-Reply-To: <1481223550-65277-1-git-send-email-bmwill@google.com>
Factor out the logic responsible for the magic in a pathspec element
into its own function.
Also avoid calling into the parsing functions when
`PATHSPEC_LITERAL_PATH` is specified since it causes magic to be
ignored and all paths to be treated as literals.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
pathspec.c | 37 ++++++++++++++++++++-----------------
1 file changed, 20 insertions(+), 17 deletions(-)
diff --git a/pathspec.c b/pathspec.c
index 6e9555e..f37f887 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -245,6 +245,19 @@ static const char *parse_short_magic(unsigned *magic, const char *elem)
return pos;
}
+static const char *parse_element_magic(unsigned *magic, int *prefix_len,
+ const char *elem)
+{
+ if (elem[0] != ':' || get_literal_global())
+ return elem; /* nothing to do */
+ else if (elem[1] == '(')
+ /* longhand */
+ return parse_long_magic(magic, prefix_len, elem);
+ else
+ /* shorthand */
+ return parse_short_magic(magic, elem);
+}
+
/*
* Take an element of a pathspec and check for magic signatures.
* Append the result to the prefix. Return the magic bitmap.
@@ -267,26 +280,16 @@ static unsigned prefix_pathspec(struct pathspec_item *item, unsigned flags,
char *match;
int i, pathspec_prefix = -1;
- if (elt[0] != ':' || get_literal_global() ||
- (flags & PATHSPEC_LITERAL_PATH)) {
- ; /* nothing to do */
- } else if (elt[1] == '(') {
- /* longhand */
- copyfrom = parse_long_magic(&element_magic,
- &pathspec_prefix,
- elt);
- } else {
- /* shorthand */
- copyfrom = parse_short_magic(&element_magic, elt);
- }
-
- magic |= element_magic;
-
/* PATHSPEC_LITERAL_PATH ignores magic */
- if (flags & PATHSPEC_LITERAL_PATH)
+ if (flags & PATHSPEC_LITERAL_PATH) {
magic = PATHSPEC_LITERAL;
- else
+ } else {
+ copyfrom = parse_element_magic(&element_magic,
+ &pathspec_prefix,
+ elt);
+ magic |= element_magic;
magic |= get_global_magic(element_magic);
+ }
if (pathspec_prefix >= 0 &&
(prefixlen || (prefix && *prefix)))
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v2 16/16] pathspec: rename prefix_pathspec to init_pathspec_item
From: Brandon Williams @ 2016-12-08 18:59 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, pclouds, gitster
In-Reply-To: <1481223550-65277-1-git-send-email-bmwill@google.com>
Give a more relevant name to the prefix_pathspec function as it does
more than just prefix a pathspec element.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
pathspec.c | 24 +++++++-----------------
1 file changed, 7 insertions(+), 17 deletions(-)
diff --git a/pathspec.c b/pathspec.c
index 4686298..08abdd3 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -297,21 +297,11 @@ static void strip_submodule_slash_expensive(struct pathspec_item *item)
}
/*
- * Take an element of a pathspec and check for magic signatures.
- * Append the result to the prefix. Return the magic bitmap.
- *
- * For now, we only parse the syntax and throw out anything other than
- * "top" magic.
- *
- * NEEDSWORK: This needs to be rewritten when we start migrating
- * get_pathspec() users to use the "struct pathspec" interface. For
- * example, a pathspec element may be marked as case-insensitive, but
- * the prefix part must always match literally, and a single stupid
- * string cannot express such a case.
+ * Perform the initialization of a pathspec_item based on a pathspec element.
*/
-static unsigned prefix_pathspec(struct pathspec_item *item, unsigned flags,
- const char *prefix, int prefixlen,
- const char *elt)
+static void init_pathspec_item(struct pathspec_item *item, unsigned flags,
+ const char *prefix, int prefixlen,
+ const char *elt)
{
unsigned magic = 0, element_magic = 0;
const char *copyfrom = elt;
@@ -329,6 +319,8 @@ static unsigned prefix_pathspec(struct pathspec_item *item, unsigned flags,
magic |= get_global_magic(element_magic);
}
+ item->magic = magic;
+
if (pathspec_prefix >= 0 &&
(prefixlen || (prefix && *prefix)))
die("BUG: 'prefix' magic is supposed to be used at worktree's root");
@@ -401,7 +393,6 @@ static unsigned prefix_pathspec(struct pathspec_item *item, unsigned flags,
/* sanity checks, pathspec matchers assume these are sane */
assert(item->nowildcard_len <= item->len &&
item->prefix <= item->len);
- return magic;
}
static int pathspec_item_cmp(const void *a_, const void *b_)
@@ -500,8 +491,7 @@ void parse_pathspec(struct pathspec *pathspec,
for (i = 0; i < n; i++) {
entry = argv[i];
- item[i].magic = prefix_pathspec(item + i, flags,
- prefix, prefixlen, entry);
+ init_pathspec_item(item + i, flags, prefix, prefixlen, entry);
if (item[i].magic & PATHSPEC_EXCLUDE)
nr_exclude++;
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v2 12/16] pathspec: create parse_long_magic function
From: Brandon Williams @ 2016-12-08 18:59 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, pclouds, gitster
In-Reply-To: <1481223550-65277-1-git-send-email-bmwill@google.com>
Factor out the logic responsible for parsing long magic into its own
function. As well as hoist the prefix check logic outside of the inner
loop as there isn't anything that needs to be done after matching
"prefix:".
Signed-off-by: Brandon Williams <bmwill@google.com>
---
pathspec.c | 92 ++++++++++++++++++++++++++++++++++++++------------------------
1 file changed, 57 insertions(+), 35 deletions(-)
diff --git a/pathspec.c b/pathspec.c
index 29054d2..6e9555e 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -157,6 +157,60 @@ static int get_global_magic(int element_magic)
}
/*
+ * Parse the pathspec element looking for long magic
+ *
+ * saves all magic in 'magic'
+ * if prefix magic is used, save the prefix length in 'prefix_len'
+ * returns the position in 'elem' after all magic has been parsed
+ */
+static const char *parse_long_magic(unsigned *magic, int *prefix_len,
+ const char *elem)
+{
+ const char *pos;
+ const char *nextat;
+
+ for (pos = elem + 2; *pos && *pos != ')'; pos = nextat) {
+ size_t len = strcspn(pos, ",)");
+ int i;
+
+ if (pos[len] == ',')
+ nextat = pos + len + 1; /* handle ',' */
+ else
+ nextat = pos + len; /* handle ')' and '\0' */
+
+ if (!len)
+ continue;
+
+ if (starts_with(pos, "prefix:")) {
+ char *endptr;
+ *prefix_len = strtol(pos + 7, &endptr, 10);
+ if (endptr - pos != len)
+ die(_("invalid parameter for pathspec magic 'prefix'"));
+ continue;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(pathspec_magic); i++) {
+ if (strlen(pathspec_magic[i].name) == len &&
+ !strncmp(pathspec_magic[i].name, pos, len)) {
+ *magic |= pathspec_magic[i].bit;
+ break;
+ }
+ }
+
+ if (ARRAY_SIZE(pathspec_magic) <= i)
+ die(_("Invalid pathspec magic '%.*s' in '%s'"),
+ (int) len, pos, elem);
+ }
+
+ if (*pos != ')')
+ die(_("Missing ')' at the end of pathspec magic in '%s'"),
+ elem);
+ pos++;
+
+ return pos;
+}
+
+/*
* Parse the pathspec element looking for short magic
*
* saves all magic in 'magic'
@@ -218,41 +272,9 @@ static unsigned prefix_pathspec(struct pathspec_item *item, unsigned flags,
; /* nothing to do */
} else if (elt[1] == '(') {
/* longhand */
- const char *nextat;
- for (copyfrom = elt + 2;
- *copyfrom && *copyfrom != ')';
- copyfrom = nextat) {
- size_t len = strcspn(copyfrom, ",)");
- if (copyfrom[len] == ',')
- nextat = copyfrom + len + 1;
- else
- /* handle ')' and '\0' */
- nextat = copyfrom + len;
- if (!len)
- continue;
- for (i = 0; i < ARRAY_SIZE(pathspec_magic); i++) {
- if (strlen(pathspec_magic[i].name) == len &&
- !strncmp(pathspec_magic[i].name, copyfrom, len)) {
- element_magic |= pathspec_magic[i].bit;
- break;
- }
- if (starts_with(copyfrom, "prefix:")) {
- char *endptr;
- pathspec_prefix = strtol(copyfrom + 7,
- &endptr, 10);
- if (endptr - copyfrom != len)
- die(_("invalid parameter for pathspec magic 'prefix'"));
- /* "i" would be wrong, but it does not matter */
- break;
- }
- }
- if (ARRAY_SIZE(pathspec_magic) <= i)
- die(_("Invalid pathspec magic '%.*s' in '%s'"),
- (int) len, copyfrom, elt);
- }
- if (*copyfrom != ')')
- die(_("Missing ')' at the end of pathspec magic in '%s'"), elt);
- copyfrom++;
+ copyfrom = parse_long_magic(&element_magic,
+ &pathspec_prefix,
+ elt);
} else {
/* shorthand */
copyfrom = parse_short_magic(&element_magic, elt);
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v2 09/16] pathspec: simpler logic to prefix original pathspec elements
From: Brandon Williams @ 2016-12-08 18:59 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, pclouds, gitster
In-Reply-To: <1481223550-65277-1-git-send-email-bmwill@google.com>
The logic used to prefix an original pathspec element with 'prefix'
magic is more general purpose and can be used for more than just short
magic. Remove the extra code paths and rename 'prefix_short_magic' to
'prefix_magic' to better indicate that it can be used in more general
situations.
Also, slightly change the logic which decides when to prefix the
original element in order to prevent a pathspec of "." from getting
converted to "" (empty string).
Signed-off-by: Brandon Williams <bmwill@google.com>
---
pathspec.c | 33 +++++++++++++--------------------
1 file changed, 13 insertions(+), 20 deletions(-)
diff --git a/pathspec.c b/pathspec.c
index a360193..49adea4 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -74,13 +74,12 @@ static struct pathspec_magic {
{ PATHSPEC_EXCLUDE, '!', "exclude" },
};
-static void prefix_short_magic(struct strbuf *sb, int prefixlen,
- unsigned short_magic)
+static void prefix_magic(struct strbuf *sb, int prefixlen, unsigned magic)
{
int i;
strbuf_addstr(sb, ":(");
for (i = 0; i < ARRAY_SIZE(pathspec_magic); i++)
- if (short_magic & pathspec_magic[i].bit) {
+ if (magic & pathspec_magic[i].bit) {
if (sb->buf[sb->len - 1] != '(')
strbuf_addch(sb, ',');
strbuf_addstr(sb, pathspec_magic[i].name);
@@ -109,8 +108,8 @@ static unsigned prefix_pathspec(struct pathspec_item *item, unsigned flags,
static int glob_global = -1;
static int noglob_global = -1;
static int icase_global = -1;
- unsigned magic = 0, short_magic = 0, global_magic = 0;
- const char *copyfrom = elt, *long_magic_end = NULL;
+ unsigned magic = 0, element_magic = 0, global_magic = 0;
+ const char *copyfrom = elt;
char *match;
int i, pathspec_prefix = -1;
@@ -164,7 +163,7 @@ static unsigned prefix_pathspec(struct pathspec_item *item, unsigned flags,
for (i = 0; i < ARRAY_SIZE(pathspec_magic); i++) {
if (strlen(pathspec_magic[i].name) == len &&
!strncmp(pathspec_magic[i].name, copyfrom, len)) {
- magic |= pathspec_magic[i].bit;
+ element_magic |= pathspec_magic[i].bit;
break;
}
if (starts_with(copyfrom, "prefix:")) {
@@ -183,7 +182,6 @@ static unsigned prefix_pathspec(struct pathspec_item *item, unsigned flags,
}
if (*copyfrom != ')')
die(_("Missing ')' at the end of pathspec magic in '%s'"), elt);
- long_magic_end = copyfrom;
copyfrom++;
} else {
/* shorthand */
@@ -196,7 +194,7 @@ static unsigned prefix_pathspec(struct pathspec_item *item, unsigned flags,
break;
for (i = 0; i < ARRAY_SIZE(pathspec_magic); i++)
if (pathspec_magic[i].mnemonic == ch) {
- short_magic |= pathspec_magic[i].bit;
+ element_magic |= pathspec_magic[i].bit;
break;
}
if (ARRAY_SIZE(pathspec_magic) <= i)
@@ -207,7 +205,7 @@ static unsigned prefix_pathspec(struct pathspec_item *item, unsigned flags,
copyfrom++;
}
- magic |= short_magic;
+ magic |= element_magic;
/* --noglob-pathspec adds :(literal) _unless_ :(glob) is specified */
if (noglob_global && !(magic & PATHSPEC_GLOB))
@@ -242,18 +240,13 @@ static unsigned prefix_pathspec(struct pathspec_item *item, unsigned flags,
* Prefix the pathspec (keep all magic) and assign to
* original. Useful for passing to another command.
*/
- if (flags & PATHSPEC_PREFIX_ORIGIN) {
+ if ((flags & PATHSPEC_PREFIX_ORIGIN) &&
+ prefixlen && !literal_global) {
struct strbuf sb = STRBUF_INIT;
- if (prefixlen && !literal_global) {
- /* Preserve the actual prefix length of each pattern */
- if (short_magic)
- prefix_short_magic(&sb, prefixlen, short_magic);
- else if (long_magic_end) {
- strbuf_add(&sb, elt, long_magic_end - elt);
- strbuf_addf(&sb, ",prefix:%d)", prefixlen);
- } else
- strbuf_addf(&sb, ":(prefix:%d)", prefixlen);
- }
+
+ /* Preserve the actual prefix length of each pattern */
+ prefix_magic(&sb, prefixlen, element_magic);
+
strbuf_addstr(&sb, match);
item->original = strbuf_detach(&sb, NULL);
} else {
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v2 10/16] pathspec: factor global magic into its own function
From: Brandon Williams @ 2016-12-08 18:59 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, pclouds, gitster
In-Reply-To: <1481223550-65277-1-git-send-email-bmwill@google.com>
Create helper functions to read the global magic environment variables
in additon to factoring out the global magic gathering logic into its
own function.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
pathspec.c | 127 +++++++++++++++++++++++++++++++++++++------------------------
1 file changed, 78 insertions(+), 49 deletions(-)
diff --git a/pathspec.c b/pathspec.c
index 49adea4..523d7bf 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -87,6 +87,75 @@ static void prefix_magic(struct strbuf *sb, int prefixlen, unsigned magic)
strbuf_addf(sb, ",prefix:%d)", prefixlen);
}
+static inline int get_literal_global(void)
+{
+ static int literal = -1;
+
+ if (literal < 0)
+ literal = git_env_bool(GIT_LITERAL_PATHSPECS_ENVIRONMENT, 0);
+
+ return literal;
+}
+
+static inline int get_glob_global(void)
+{
+ static int glob = -1;
+
+ if (glob < 0)
+ glob = git_env_bool(GIT_GLOB_PATHSPECS_ENVIRONMENT, 0);
+
+ return glob;
+}
+
+static inline int get_noglob_global(void)
+{
+ static int noglob = -1;
+
+ if (noglob < 0)
+ noglob = git_env_bool(GIT_NOGLOB_PATHSPECS_ENVIRONMENT, 0);
+
+ return noglob;
+}
+
+static inline int get_icase_global(void)
+{
+ static int icase = -1;
+
+ if (icase < 0)
+ icase = git_env_bool(GIT_ICASE_PATHSPECS_ENVIRONMENT, 0);
+
+ return icase;
+}
+
+static int get_global_magic(int element_magic)
+{
+ int global_magic = 0;
+
+ if (get_literal_global())
+ global_magic |= PATHSPEC_LITERAL;
+
+ /* --glob-pathspec is overridden by :(literal) */
+ if (get_glob_global() && !(element_magic & PATHSPEC_LITERAL))
+ global_magic |= PATHSPEC_GLOB;
+
+ if (get_glob_global() && get_noglob_global())
+ die(_("global 'glob' and 'noglob' pathspec settings are incompatible"));
+
+ if (get_icase_global())
+ global_magic |= PATHSPEC_ICASE;
+
+ if ((global_magic & PATHSPEC_LITERAL) &&
+ (global_magic & ~PATHSPEC_LITERAL))
+ die(_("global 'literal' pathspec setting is incompatible "
+ "with all other global pathspec settings"));
+
+ /* --noglob-pathspec adds :(literal) _unless_ :(glob) is specified */
+ if (get_noglob_global() && !(element_magic & PATHSPEC_GLOB))
+ global_magic |= PATHSPEC_LITERAL;
+
+ return global_magic;
+}
+
/*
* Take an element of a pathspec and check for magic signatures.
* Append the result to the prefix. Return the magic bitmap.
@@ -104,46 +173,12 @@ static unsigned prefix_pathspec(struct pathspec_item *item, unsigned flags,
const char *prefix, int prefixlen,
const char *elt)
{
- static int literal_global = -1;
- static int glob_global = -1;
- static int noglob_global = -1;
- static int icase_global = -1;
- unsigned magic = 0, element_magic = 0, global_magic = 0;
+ unsigned magic = 0, element_magic = 0;
const char *copyfrom = elt;
char *match;
int i, pathspec_prefix = -1;
- if (literal_global < 0)
- literal_global = git_env_bool(GIT_LITERAL_PATHSPECS_ENVIRONMENT, 0);
- if (literal_global)
- global_magic |= PATHSPEC_LITERAL;
-
- if (glob_global < 0)
- glob_global = git_env_bool(GIT_GLOB_PATHSPECS_ENVIRONMENT, 0);
- if (glob_global)
- global_magic |= PATHSPEC_GLOB;
-
- if (noglob_global < 0)
- noglob_global = git_env_bool(GIT_NOGLOB_PATHSPECS_ENVIRONMENT, 0);
-
- if (glob_global && noglob_global)
- die(_("global 'glob' and 'noglob' pathspec settings are incompatible"));
-
-
- if (icase_global < 0)
- icase_global = git_env_bool(GIT_ICASE_PATHSPECS_ENVIRONMENT, 0);
- if (icase_global)
- global_magic |= PATHSPEC_ICASE;
-
- if ((global_magic & PATHSPEC_LITERAL) &&
- (global_magic & ~PATHSPEC_LITERAL))
- die(_("global 'literal' pathspec setting is incompatible "
- "with all other global pathspec settings"));
-
- if (flags & PATHSPEC_LITERAL_PATH)
- global_magic = 0;
-
- if (elt[0] != ':' || literal_global ||
+ if (elt[0] != ':' || get_literal_global() ||
(flags & PATHSPEC_LITERAL_PATH)) {
; /* nothing to do */
} else if (elt[1] == '(') {
@@ -207,15 +242,11 @@ static unsigned prefix_pathspec(struct pathspec_item *item, unsigned flags,
magic |= element_magic;
- /* --noglob-pathspec adds :(literal) _unless_ :(glob) is specified */
- if (noglob_global && !(magic & PATHSPEC_GLOB))
- global_magic |= PATHSPEC_LITERAL;
-
- /* --glob-pathspec is overridden by :(literal) */
- if ((global_magic & PATHSPEC_GLOB) && (magic & PATHSPEC_LITERAL))
- global_magic &= ~PATHSPEC_GLOB;
-
- magic |= global_magic;
+ /* PATHSPEC_LITERAL_PATH ignores magic */
+ if (flags & PATHSPEC_LITERAL_PATH)
+ magic = PATHSPEC_LITERAL;
+ else
+ magic |= get_global_magic(element_magic);
if (pathspec_prefix >= 0 &&
(prefixlen || (prefix && *prefix)))
@@ -241,7 +272,7 @@ static unsigned prefix_pathspec(struct pathspec_item *item, unsigned flags,
* original. Useful for passing to another command.
*/
if ((flags & PATHSPEC_PREFIX_ORIGIN) &&
- prefixlen && !literal_global) {
+ prefixlen && !get_literal_global()) {
struct strbuf sb = STRBUF_INIT;
/* Preserve the actual prefix length of each pattern */
@@ -407,9 +438,7 @@ void parse_pathspec(struct pathspec *pathspec,
item[i].magic = prefix_pathspec(item + i, flags,
prefix, prefixlen, entry);
- if ((flags & PATHSPEC_LITERAL_PATH) &&
- !(magic_mask & PATHSPEC_LITERAL))
- item[i].magic |= PATHSPEC_LITERAL;
+
if (item[i].magic & PATHSPEC_EXCLUDE)
nr_exclude++;
if (item[i].magic & magic_mask)
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
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