* Re: [PATCH v2] config: reject invalid VAR in 'git -c VAR=VAL command'
From: Jeff King @ 2017-02-24 0:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org, Stefan Beller
In-Reply-To: <xmqqy3ww5wbl.fsf@gitster.mtv.corp.google.com>
On Thu, Feb 23, 2017 at 03:19:58PM -0800, Junio C Hamano wrote:
> > But you are right. config-parse-key does have the simpler string
> > that can just be given to the canonicalize thing and we should be
> > able to reuse it.
>
> Actually, I think we can just use the existing config_parse_key()
> instead of adding the new function. It adds one allocation and
> deallocation, but it's not like this is a performance-critical
> codepath that we absolutely avoid extra allocations. After all, we
> are still using the strbuf-split thing :-/.
Yeah, you're right. This is much nicer, and everything else was
premature optimization.
> -- >8 --
> From: Junio C Hamano <gitster@pobox.com>
> Date: Thu, 23 Feb 2017 15:04:40 -0800
> Subject: [PATCH] config: reject invalid VAR in 'git -c VAR=VAL command' and
> keep subsection intact
Long subject. :)
I'd have just said:
config: pass variables through git_config_parse_parameter()
That is "what", but the "why" can come in the next paragraph.
> The parsing of one-shot assignments of configuration variables that
> come from the command line historically was quite loose and allowed
> anything to pass. It also downcased everything in the variable name,
> even a three-level <section>.<subsection>.<variable> name in which
> the <subsection> part must be treated in a case sensible manner.
>
> Existing git_config_parse_key() helper is used to parse the variable
> name that comes from the command line, i.e. "git config VAR VAL",
> and handles these details correctly. Replace the strbuf_tolower()
> call in git-config_parse_parameter() with a call to it to correct
> both issues. git_config_parse_key() does a bit more things that are
> not necessary for the purpose of this codepath (e.g. it allocates a
> separate buffer to return the canonicalized variable name because it
> takes a "const char *" input), but we are not in a performance-critical
> codepath here.
Nicely explained.
> diff --git a/config.c b/config.c
> index b8cce1dffa..39f20dcd2c 100644
> --- a/config.c
> +++ b/config.c
> @@ -295,7 +295,9 @@ int git_config_parse_parameter(const char *text,
> config_fn_t fn, void *data)
> {
> const char *value;
> + char *canonical_name;
> struct strbuf **pair;
> + int ret = 0;
>
> pair = strbuf_split_str(text, '=', 2);
> if (!pair[0])
Hmm. I suspect one cannot do:
git -c 'section.subsection with an = in it.key=foo' ...
Definitely not a new problem, nor something that should block your
patch. But if we want to fix it, I suspect the problem will ultimately
involve parsing left-to-right to get the key first, then confirming it
has an =, and then the value.
> @@ -313,13 +315,15 @@ int git_config_parse_parameter(const char *text,
> strbuf_list_free(pair);
> return error("bogus config parameter: %s", text);
> }
> - strbuf_tolower(pair[0]);
> - if (fn(pair[0]->buf, value, data) < 0) {
> - strbuf_list_free(pair);
> +
> + if (git_config_parse_key(pair[0]->buf, &canonical_name, NULL))
> return -1;
> - }
I think git_config_parse_key() will free canonical_name itself if it
returns failure. But do you need to strbuf_list_free(pair) here?
Or alternatively:
int ret = -1;
if (!parse(...))
ret = fn(...);
or use a "got out". Whatever. You don't need me to teach you about error
exits. :)
> + ret = (fn(canonical_name, value, data) < 0) ? -1 : 0;
> +
> + free(canonical_name);
> strbuf_list_free(pair);
> - return 0;
> + return ret;
Looks good.
> diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
> index 923bfc5a26..ea371020fa 100755
> --- a/t/t1300-repo-config.sh
> +++ b/t/t1300-repo-config.sh
I just skimmed these, as they look like the previous ones.
So overall I like it, modulo the minor error-leak.
-Peff
^ permalink raw reply
* Re: [PATCH 10/10] submodule--helper clone: check for configured submodules using helper
From: Stefan Beller @ 2017-02-24 0:58 UTC (permalink / raw)
To: Brandon Williams; +Cc: git@vger.kernel.org
In-Reply-To: <20170223234728.164111-11-bmwill@google.com>
On Thu, Feb 23, 2017 at 3:47 PM, Brandon Williams <bmwill@google.com> wrote:
> @@ -795,14 +794,11 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
> }
>
> /*
> - * Looking up the url in .git/config.
> + * Check if the submodule has been initialized.
> * We must not fall back to .gitmodules as we only want
> * to process configured submodules.
This sentence "we must not ..." is also no longer accurate,
as we do exactly that when using sub->url instead of just url
below.
^ permalink raw reply
* Re: [PATCH 4/4] ident: do not ignore empty config name/email
From: Jeff King @ 2017-02-24 1:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: bs.x.ttp, git
In-Reply-To: <xmqqfuj47hfk.fsf@gitster.mtv.corp.google.com>
On Thu, Feb 23, 2017 at 12:58:39PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > This one is perhaps questionable. Maybe somebody is relying on setting a
> > per-repo user.name to override a ~/.gitconfig value and enforce
> > auto-detection?
>
> Thanks for splitting this step out. 1/4 and 2/4 are obvious
> improvements, and 3/4 is a very sensible fix. Compared to those
> three, this one does smell questionable, because I do not quite see
> any other reasonable fallback other than the auto-detection if the
> user gives an empty ident on purpose.
The outcomes are basically:
1. In strict mode (making a commit, etc), we'll die with "empty name
not allowed". My thinking was that this is less confusing for the
user.
2. In non-strict mode, we'd use a blank name instead of trying your
username (or dying if you don't have an /etc/passwd entry).
> Erroring out to say "don't do that" is probably not too bad, but
> perhaps we are being run by a script that is doing a best-effort
> conversion from $ANOTHER_SCM using a list of known authors that is
> incomplete, ending up feeding empty ident and allowing us to fall
> back to attribute them to the user who runs the script. I do not
> see a point in breaking that user and having her or him update the
> script to stuff in a truly bogus "Unknown <unknown>" name.
Keep in mind this _only_ affects Git's config variables. So a script
feeding git via GIT_AUTHOR_NAME, etc, shouldn't change at all with this
code. If your script is doing "git -c user.name=whatever commit", I
think you should reconsider your script. :)
So I dunno. I could really go either way on it. Feel free to drop it, or
even move it into a separate topic to be cooked longer.
-Peff
^ permalink raw reply
* Re: [PATCH 05/10] submodule--helper: add is_active command
From: Stefan Beller @ 2017-02-24 1:15 UTC (permalink / raw)
To: Brandon Williams; +Cc: git@vger.kernel.org
In-Reply-To: <20170223234728.164111-6-bmwill@google.com>
On Thu, Feb 23, 2017 at 3:47 PM, Brandon Williams <bmwill@google.com> wrote:
> There are a lot of places where an explicit check for
> submodule."<name>".url is done to see if a submodule exists. In order
> to more easily facilitate the use of the submodule.active config option
> to indicate active submodules, add a helper which can be used to query
> if a submodule is active or not.
>
> Signed-off-by: Brandon Williams <bmwill@google.com>
> ---
> builtin/submodule--helper.c | 11 ++++++++
> t/t7413-submodule-is-active.sh | 63 ++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 74 insertions(+)
> create mode 100755 t/t7413-submodule-is-active.sh
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index df0d9c166..dac02604d 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -1128,6 +1128,16 @@ static int absorb_git_dirs(int argc, const char **argv, const char *prefix)
> return 0;
> }
>
> +static int is_active(int argc, const char **argv, const char *prefix)
> +{
> + if (argc != 2)
> + die("submodule--helper is-active takes exactly 1 arguments");
> +
> + gitmodules_config();
> +
> + return !is_submodule_initialized(argv[1]);
> +}
> +
> #define SUPPORT_SUPER_PREFIX (1<<0)
>
> struct cmd_struct {
> @@ -1147,6 +1157,7 @@ static struct cmd_struct commands[] = {
> {"init", module_init, 0},
> {"remote-branch", resolve_remote_submodule_branch, 0},
> {"absorb-git-dirs", absorb_git_dirs, SUPPORT_SUPER_PREFIX},
> + {"is-active", is_active, 0},
> };
>
> int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
> diff --git a/t/t7413-submodule-is-active.sh b/t/t7413-submodule-is-active.sh
> new file mode 100755
> index 000000000..683487020
> --- /dev/null
> +++ b/t/t7413-submodule-is-active.sh
> @@ -0,0 +1,63 @@
> +#!/bin/sh
> +
> +test_description='Test submodule--helper is-active
> +
> +This test verifies that `git submodue--helper is-active` correclty identifies
> +submodules which are "active" and interesting to the user.
> +'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'setup' '
> + git init sub &&
> + test_commit -C sub initial &&
> + git init super &&
> + test_commit -C super initial &&
> + git -C super submodule add ../sub sub1 &&
> + git -C super submodule add ../sub sub2 &&
> + git -C super commit -a -m "add 2 submodules at sub{1,2}"
> +'
> +
> +test_expect_success 'is-active works with urls' '
> + git -C super submodule--helper is-active sub1 &&
> + git -C super submodule--helper is-active sub2 &&
> +
> + git -C super config --unset submodule.sub1.URL &&
> + test_must_fail git -C super submodule--helper is-active sub1 &&
> + git -C super config submodule.sub1.URL ../sub &&
> + git -C super submodule--helper is-active sub1
> +'
> +
> +test_expect_success 'is-active works with basic submodule.active config' '
> + git -C super config --add submodule.active "." &&
> + git -C super config --unset submodule.sub1.URL &&
> + git -C super config --unset submodule.sub2.URL &&
I think we'd want to unset only one of them here
> +
> + git -C super submodule--helper is-active sub1 &&
> + git -C super submodule--helper is-active sub2 &&
to test 2 different cases of one being active by config setting only and
the other having both.
I could not spot test for having the URL set but the config setting set, not
including the submodule, e.g.
git -C super config submodule.sub1.URL ../sub &&
git -C super submodule.active ":(exclude)sub1" &&
which would be expected to not be active, as once the configuration
is there it takes precedence over any (no)URL setting?
^ permalink raw reply
* Re: [PATCH 10/15] update submodules: add submodule_move_head
From: Ramsay Jones @ 2017-02-24 1:21 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, sandals, jrnieder, bmwill, gitster, novalis
In-Reply-To: <20170223225735.10994-11-sbeller@google.com>
On 23/02/17 22:57, Stefan Beller wrote:
> In later patches we introduce the options and flag for commands
> that modify the working directory, e.g. git-checkout.
>
> This piece of code will be used universally for
> all these working tree modifications as it
> * supports dry run to answer the question:
> "Is it safe to change the submodule to this new state?"
> e.g. is it overwriting untracked files or are there local
> changes that would be overwritten?
> * supports a force flag that can be used for resetting
> the tree.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> submodule.c | 135 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> submodule.h | 7 ++++
> 2 files changed, 142 insertions(+)
>
> diff --git a/submodule.c b/submodule.c
> index 0b2596e88a..a2cf8c9376 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1239,6 +1239,141 @@ int bad_to_remove_submodule(const char *path, unsigned flags)
> return ret;
> }
>
> +static int submodule_has_dirty_index(const struct submodule *sub)
> +{
> + struct child_process cp = CHILD_PROCESS_INIT;
> +
> + prepare_submodule_repo_env_no_git_dir(&cp.env_array);
> +
> + cp.git_cmd = 1;
> + argv_array_pushl(&cp.args, "diff-index", "--quiet", \
> + "--cached", "HEAD", NULL);
> + cp.no_stdin = 1;
> + cp.no_stdout = 1;
> + cp.dir = sub->path;
> + if (start_command(&cp))
> + die("could not recurse into submodule '%s'", sub->path);
> +
> + return finish_command(&cp);
> +}
> +
> +void submodule_reset_index(const char *path)
I was just about to send a patch against the previous series
(in pu branch last night), but since you have sent another
version ...
In the last series this was called 'submodule_clean_index()'
and, since it is a file-local symbol, should be marked with
static. I haven't applied these patches to check, but the
interdiff in the cover letter leads me to believe that this
will also apply to the renamed function.
[The patch subject was also slightly different.]
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH 15/15] builtin/checkout: add --recurse-submodules switch
From: Ramsay Jones @ 2017-02-24 1:25 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, sandals, jrnieder, bmwill, gitster, novalis
In-Reply-To: <20170223225735.10994-16-sbeller@google.com>
On 23/02/17 22:57, Stefan Beller wrote:
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> Documentation/git-checkout.txt | 7 +++++++
> builtin/checkout.c | 28 ++++++++++++++++++++++++++++
> t/lib-submodule-update.sh | 33 ++++++++++++++++++++++++---------
> t/t2013-checkout-submodule.sh | 5 +++++
> 4 files changed, 64 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
> index 8e2c0662dd..d6399c0af8 100644
> --- a/Documentation/git-checkout.txt
> +++ b/Documentation/git-checkout.txt
> @@ -256,6 +256,13 @@ section of linkgit:git-add[1] to learn how to operate the `--patch` mode.
> out anyway. In other words, the ref can be held by more than one
> worktree.
>
> +--[no-]recurse-submodules::
> + Using --recurse-submodules will update the content of all initialized
> + submodules according to the commit recorded in the superproject. If
> + local modifications in a submodule would be overwritten the checkout
> + will fail unless `-f` is used. If nothing (or --no-recurse-submodules)
> + is used, the work trees of submodules will not be updated.
> +
> <branch>::
> Branch to checkout; if it refers to a branch (i.e., a name that,
> when prepended with "refs/heads/", is a valid ref), then that
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index f174f50303..207ce09771 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -21,12 +21,31 @@
> #include "submodule-config.h"
> #include "submodule.h"
>
> +static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
> +
> static const char * const checkout_usage[] = {
> N_("git checkout [<options>] <branch>"),
> N_("git checkout [<options>] [<branch>] -- <file>..."),
> NULL,
> };
>
> +int option_parse_recurse_submodules(const struct option *opt,
> + const char *arg, int unset)
Again, this function should be marked static.
[I also noted _two_ other local functions with the same name
in builtin/fetch.c and builtin/push.c]
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH 4/4] ident: do not ignore empty config name/email
From: Junio C Hamano @ 2017-02-24 4:11 UTC (permalink / raw)
To: Jeff King; +Cc: bs.x.ttp, git
In-Reply-To: <20170224010823.my4wmdyezjuqajfx@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Keep in mind this _only_ affects Git's config variables. So a script
> feeding git via GIT_AUTHOR_NAME, etc, shouldn't change at all with this
> code.
Ah, that changes the equation somewhat ;-)
> So I dunno. I could really go either way on it. Feel free to drop it, or
> even move it into a separate topic to be cooked longer.
If it were 5 years ago, it would have been different, but I do not
think cooking it longer in 'next' would smoke out breakages in
obscure scripts any longer. Git is used by too many people who have
never seen its source these days.
^ permalink raw reply
* Re: [PATCH v2] config: reject invalid VAR in 'git -c VAR=VAL command'
From: Junio C Hamano @ 2017-02-24 4:17 UTC (permalink / raw)
To: Jeff King; +Cc: git@vger.kernel.org, Stefan Beller
In-Reply-To: <20170224004105.ayddcwlnpmq7tifu@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
>> pair = strbuf_split_str(text, '=', 2);
>> if (!pair[0])
>
> Hmm. I suspect one cannot do:
>
> git -c 'section.subsection with an = in it.key=foo' ...
>
> Definitely not a new problem, nor something that should block your
> patch. But if we want to fix it, I suspect the problem will ultimately
> involve parsing left-to-right to get the key first, then confirming it
> has an =, and then the value.
Backtracking will not fundamentally "fix" parsing of
a.b=c=.d
between twhse two
[a "b="] c = ".d"
[a] b = "c=.d"
unfortunately, I think. I do not think it is worth doing the "best
effort" with erroring out when ambiguous, because there is no way
for the end user to disambiguate, unless we introduce a different
syntax, at which point we cannot use config_parse_key() anymore.
>> + if (git_config_parse_key(pair[0]->buf, &canonical_name, NULL))
>> return -1;
>> - }
>
> I think git_config_parse_key() will free canonical_name itself if it
> returns failure. But do you need to strbuf_list_free(pair) here?
Yeah, I missed that one. Thanks.
^ permalink raw reply
* Re: [PATCH 4/4] ident: do not ignore empty config name/email
From: Jeff King @ 2017-02-24 4:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: bs.x.ttp, git
In-Reply-To: <xmqqlgsw5iu8.fsf@gitster.mtv.corp.google.com>
On Thu, Feb 23, 2017 at 08:11:11PM -0800, Junio C Hamano wrote:
> > So I dunno. I could really go either way on it. Feel free to drop it, or
> > even move it into a separate topic to be cooked longer.
>
> If it were 5 years ago, it would have been different, but I do not
> think cooking it longer in 'next' would smoke out breakages in
> obscure scripts any longer. Git is used by too many people who have
> never seen its source these days.
Yeah, I have noticed that, too. I wonder if it would be interesting to
cut "weeklies" or something of "master" or even "next" that people could
install with a single click.
Of course it's not like we have a binary installer in the first place,
so I guess that's a prerequisite.
-Peff
^ permalink raw reply
* Re: [PATCH v2] config: reject invalid VAR in 'git -c VAR=VAL command'
From: Jeff King @ 2017-02-24 4:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org, Stefan Beller
In-Reply-To: <xmqqh93k5ijb.fsf@gitster.mtv.corp.google.com>
On Thu, Feb 23, 2017 at 08:17:44PM -0800, Junio C Hamano wrote:
> > Hmm. I suspect one cannot do:
> >
> > git -c 'section.subsection with an = in it.key=foo' ...
> >
> > Definitely not a new problem, nor something that should block your
> > patch. But if we want to fix it, I suspect the problem will ultimately
> > involve parsing left-to-right to get the key first, then confirming it
> > has an =, and then the value.
>
> Backtracking will not fundamentally "fix" parsing of
>
> a.b=c=.d
>
> between twhse two
>
> [a "b="] c = ".d"
> [a] b = "c=.d"
>
> unfortunately, I think. I do not think it is worth doing the "best
> effort" with erroring out when ambiguous, because there is no way
> for the end user to disambiguate, unless we introduce a different
> syntax, at which point we cannot use config_parse_key() anymore.
Ah, yeah, you're right. I thought the problem was just that the "split"
was too naive, but it really is that the whole syntax is badly
specified.
I guess "git config --list" suffers from the same problem. You can get
around it there with "-z", but that probably would not be very pleasant
here. :)
Probably not worth worrying too much about if nobody is complaining.
-Peff
^ permalink raw reply
* Re: [PATCH v2] config: reject invalid VAR in 'git -c VAR=VAL command'
From: Junio C Hamano @ 2017-02-24 6:08 UTC (permalink / raw)
To: Jeff King; +Cc: git@vger.kernel.org, Stefan Beller
In-Reply-To: <20170224042227.2rjgf4zbiadxbrtz@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
>> Backtracking will not fundamentally "fix" parsing of
>>
>> a.b=c=.d
>>
>> between twhse two
>>
>> [a "b="] c = ".d"
>> [a] b = "c=.d"
>>
>> unfortunately, I think. I do not think it is worth doing the "best
>> effort" with erroring out when ambiguous, because there is no way
>> for the end user to disambiguate, unless we introduce a different
>> syntax, at which point we cannot use config_parse_key() anymore.
>
> Ah, yeah, you're right. I thought the problem was just that the "split"
> was too naive, but it really is that the whole syntax is badly
> specified.
>
> I guess "git config --list" suffers from the same problem. You can get
> around it there with "-z", but that probably would not be very pleasant
> here. :)
>
> Probably not worth worrying too much about if nobody is complaining.
Yup.
Anyway, here is an updated one (the part of the patch to t/ is not
shown as it is unchanged).
-- >8 --
Subject: [PATCH] config: use git_config_parse_key() in git_config_parse_parameter()
The parsing of one-shot assignments of configuration variables that
come from the command line historically was quite loose and allowed
anything to pass. It also downcased everything in the variable name,
even a three-level <section>.<subsection>.<variable> name in which
the <subsection> part must be treated in a case sensitive manner.
Existing git_config_parse_key() helper is used to parse the variable
name that comes from the command line, i.e. "git config VAR VAL",
and handles these details correctly. Replace the strbuf_tolower()
call in git_config_parse_parameter() with a call to it to correct
both issues. git_config_parse_key() does a bit more things that are
not necessary for the purpose of this codepath (e.g. it allocates a
separate buffer to return the canonicalized variable name because it
takes a "const char *" input), but we are not in a performance-critical
codepath here.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
config.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/config.c b/config.c
index b8cce1dffa..1c1a1520ff 100644
--- a/config.c
+++ b/config.c
@@ -295,7 +295,9 @@ int git_config_parse_parameter(const char *text,
config_fn_t fn, void *data)
{
const char *value;
+ char *canonical_name;
struct strbuf **pair;
+ int ret;
pair = strbuf_split_str(text, '=', 2);
if (!pair[0])
@@ -313,13 +315,15 @@ int git_config_parse_parameter(const char *text,
strbuf_list_free(pair);
return error("bogus config parameter: %s", text);
}
- strbuf_tolower(pair[0]);
- if (fn(pair[0]->buf, value, data) < 0) {
- strbuf_list_free(pair);
- return -1;
+
+ if (git_config_parse_key(pair[0]->buf, &canonical_name, NULL)) {
+ ret = -1;
+ } else {
+ ret = (fn(canonical_name, value, data) < 0) ? -1 : 0;
+ free(canonical_name);
}
strbuf_list_free(pair);
- return 0;
+ return ret;
}
int git_config_from_parameters(config_fn_t fn, void *data)
--
2.12.0-rc2-308-gbf7e63c428
^ permalink raw reply related
* Re: [PATCH v2] config: reject invalid VAR in 'git -c VAR=VAL command'
From: Jeff King @ 2017-02-24 6:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org, Stefan Beller
In-Reply-To: <xmqqd1e85ddy.fsf@gitster.mtv.corp.google.com>
On Thu, Feb 23, 2017 at 10:08:57PM -0800, Junio C Hamano wrote:
> Anyway, here is an updated one (the part of the patch to t/ is not
> shown as it is unchanged).
>
> -- >8 --
> Subject: [PATCH] config: use git_config_parse_key() in git_config_parse_parameter()
Looks good. Nice and simple.
-Peff
^ permalink raw reply
* [PATCH] docs/git-gc: fix default value for `--aggressiveDepth`
From: Patrick Steinhardt @ 2017-02-24 8:46 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Jeff King
In commit 07e7dbf0d (gc: default aggressive depth to 50, 2016-08-11),
the default aggressive depth of git-gc has been changed to 50. While
git-config(1) has been updated to represent the new default value,
git-gc(1) still mentions the old value. This patch fixes it.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/git-gc.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt
index 852b72c67..571b5a7e3 100644
--- a/Documentation/git-gc.txt
+++ b/Documentation/git-gc.txt
@@ -127,7 +127,7 @@ the documentation for the --window' option in linkgit:git-repack[1] for
more details. This defaults to 250.
Similarly, the optional configuration variable `gc.aggressiveDepth`
-controls --depth option in linkgit:git-repack[1]. This defaults to 250.
+controls --depth option in linkgit:git-repack[1]. This defaults to 50.
The optional configuration variable `gc.pruneExpire` controls how old
the unreferenced loose objects have to be before they are pruned. The
--
2.11.1
^ permalink raw reply related
* Re: [PATCH] submodule init: warn about falling back to a local path
From: Johannes Sixt @ 2017-02-24 8:51 UTC (permalink / raw)
To: Stefan Beller; +Cc: philipoakley, git, gitster, sop
In-Reply-To: <20170224001704.23854-1-sbeller@google.com>
Am 24.02.2017 um 01:17 schrieb Stefan Beller:
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -73,13 +73,17 @@ configuration entries unless `--name` is used to specify a logical name.
> ...
> +The default remote is the remote of the remote tracking branch
> +of the current branch. If no such remote tracking branch exists or
> +the in detached HEAD mode, "origin" is assumed to be the default remote.
The part after "or" does not quite parse.
> +If the superproject doesn't have a default remote configured
> the superproject is its own authoritative upstream and the current
> working directory is used instead.
> +
> @@ -118,18 +122,22 @@ too (and can also report changes to a submodule's work tree).
>
> init [--] [<path>...]::
> Initialize the submodules recorded in the index (which were
> - added and committed elsewhere) by copying submodule
> - names and urls from .gitmodules to .git/config.
> + added and committed elsewhere) by copying `submodule.$name.url`
> + from .gitmodules to .git/config, resolving relative urls to be
> + relative to the default remote.
> ++
> Optional <path> arguments limit which submodules will be initialized.
> - It will also copy the value of `submodule.$name.update` into
> - .git/config.
> - The key used in .git/config is `submodule.$name.url`.
> + If no path is specified all submodules are initialized.
> ++
> + When present, it will also copy the value of `submodule.$name.update`.
> This command does not alter existing information in .git/config.
> You can then customize the submodule clone URLs in .git/config
> for your local setup and proceed to `git submodule update`;
> you can also just use `git submodule update --init` without
> the explicit 'init' step if you do not intend to customize
> any submodule locations.
> ++
> + See the add subcommand for the defintion of default remote.
To be rendered correctly, I think you must remove the indentation from
continuation paragraphs.
>
> deinit [-f|--force] (--all|[--] <path>...)::
> Unregister the given submodules, i.e. remove the whole
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 899dc334e3..44c11dd91e 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -356,12 +356,10 @@ static void init_submodule(const char *path, const char *prefix, int quiet)
> strbuf_addf(&remotesb, "remote.%s.url", remote);
> free(remote);
>
> - if (git_config_get_string(remotesb.buf, &remoteurl))
> - /*
> - * The repository is its own
> - * authoritative upstream
> - */
> + if (git_config_get_string(remotesb.buf, &remoteurl)) {
> remoteurl = xgetcwd();
> + warning(_("could not lookup configuration '%s'. Assuming this repository is its own authoritative upstream."), remotesb.buf);
> + }
If you re-roll this patch, please place the warning before xgetcwd,
which can potentially fail.
-- Hannes
^ permalink raw reply
* Re: [PATCH] docs/git-gc: fix default value for `--aggressiveDepth`
From: Jeff King @ 2017-02-24 9:00 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <952cf1f2cb37b746d823f3b917bfb44171cbc465.1487925983.git.ps@pks.im>
On Fri, Feb 24, 2017 at 09:46:45AM +0100, Patrick Steinhardt wrote:
> In commit 07e7dbf0d (gc: default aggressive depth to 50, 2016-08-11),
> the default aggressive depth of git-gc has been changed to 50. While
> git-config(1) has been updated to represent the new default value,
> git-gc(1) still mentions the old value. This patch fixes it.
Thanks, this is obviously an improvement.
(I also grepped for "250" in Documentation; the results are thankfully
short, and the only other mentions I saw were referring to the window
size).
-Peff
^ permalink raw reply
* Re: SHA1 collisions found
From: Duy Nguyen @ 2017-02-24 9:42 UTC (permalink / raw)
To: Joey Hess; +Cc: Git Mailing List
In-Reply-To: <20170223164306.spg2avxzukkggrpb@kitenet.net>
On Thu, Feb 23, 2017 at 11:43 PM, Joey Hess <id@joeyh.name> wrote:
> IIRC someone has been working on parameterizing git's SHA1 assumptions
> so a repository could eventually use a more secure hash. How far has
> that gotten? There are still many "40" constants in git.git HEAD.
Michael asked Brian (that "someone") the other day and he replied [1]
>> I'm curious; what fraction of the overall convert-to-object_id campaign
>> do you estimate is done so far? Are you getting close to the promised
>> land yet?
>
> So I think that the current scope left is best estimated by the
> following command:
>
> git grep -P 'unsigned char\s+(\*|.*20)' | grep -v '^Documentation'
>
> So there are approximately 1200 call sites left, which is quite a bit of
> work. I estimate between the work I've done and other people's
> refactoring work (such as the refs backend refactor), we're about 40%
> done.
[1] http://public-inbox.org/git/%3C20170217214513.giua5ksuiqqs2laj@genre.crustytoothpaste.net%3E/
--
Duy
^ permalink raw reply
* Re: [PATCH v5 1/1] config: add conditional include
From: Duy Nguyen @ 2017-02-24 9:37 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git Mailing List, Jeff King, Sebastian Schuberth, Matthieu Moy
In-Reply-To: <xmqqwpcg7k6r.fsf@gitster.mtv.corp.google.com>
On Fri, Feb 24, 2017 at 2:59 AM, Junio C Hamano <gitster@pobox.com> wrote:
> The variable is obviously not treated the same way as include.path ;-)
>
> When includeIf.<condition>.path variable is set in a
> configuration file, the configuration file named by that
> variable is included (in a way similar to how include.path
> works) only if the <condition> holds true.
Yeah. I was thinking "value" and writing "variable" instead (or
perhaps I meant to write "variable value" and accidentally'd the last
word again).
>> + /* TODO: maybe support ~user/ too */
>> + if (pat->buf[0] == '~' && is_dir_sep(pat->buf[1])) {
>> + const char *home = getenv("HOME");
>> +
>> + if (!home)
>> + return error(_("$HOME is not defined"));
>
> Instead of half-duplicating it here yourself, can't we let
> expand_user_path() do its thing?
This comment came up in previous iterations. I perform explicit
expansion to make sure we don't apply wildcards on the expanded "~".
But I guess going with expand_user_path() is better (less confusion
for future readers). We can come back to it when that "don't apply
wildcards" concern becomes real.
>> +static int include_condition_is_true(const char *cond, size_t cond_len)
>> +{
>> + /* no condition (i.e., "include.path") is always true */
>> + if (!cond)
>> + return 1;
>> +
>> + if (skip_prefix_mem(cond, cond_len, "gitdir:", &cond, &cond_len))
>> + return include_by_gitdir(cond, cond_len, 0);
>> + else if (skip_prefix_mem(cond, cond_len, "gitdir/i:", &cond, &cond_len))
>> + return include_by_gitdir(cond, cond_len, 1);
>
> This may be OK for now, but it should be trivial to start from a
> table with two entries, i.e.
>
> struct include_cond {
> const char *keyword;
> int (*fn)(const char *, size_t);
> };
>
> and will show a better way to do things to those who follow your
> footsteps.
Yeah I don't see a third include coming soon and did not go with that.
Let's way for it and refactor then.
--
Duy
^ permalink raw reply
* Re: git email From: parsing (was Re: [GIT PULL] Staging/IIO driver patches for 4.11-rc1)
From: Geert Uytterhoeven @ 2017-02-24 11:03 UTC (permalink / raw)
To: Jeff King
Cc: Greg KH, Linus Torvalds, Simon Sandström, Andrew Morton,
Git Mailing List, Linux Kernel Mailing List, Linux Driver Project
In-Reply-To: <20170223061702.bzzgrntotppvwdw6@sigill.intra.peff.net>
On Thu, Feb 23, 2017 at 7:17 AM, Jeff King <peff@peff.net> wrote:
> On Thu, Feb 23, 2017 at 07:04:44AM +0100, Greg KH wrote:
>> > Poor Simon Sandström.
>> >
>> > Funnily enough, this only exists for one commit. You've got several
>> > other commits from Simon that get his name right.
>> >
>> > What happened?
>>
>> I don't know what happened, I used git for this, I don't use quilt for
>> "normal" patches accepted into my trees anymore, only for stable kernel
>> work.
>>
>> So either the mail is malformed, or git couldn't figure it out, I've
>> attached the original message below, and cc:ed the git mailing list.
>>
>> Also, Simon emailed me after this was committed saying something went
>> wrong, but I couldn't go back and rebase my tree. Simon, did you ever
>> figure out if something was odd on your end?
>>
>> Git developers, any ideas?
>
> The problem isn't on the applying end, but rather on the generating end.
> The From header in the attached mbox is:
>
> From: =?us-ascii?B?PT9VVEYtOD9xP1NpbW9uPTIwU2FuZHN0cj1DMz1CNm0/PQ==?= <simon@nikanor.nu>
Slightly related, once in a while I get funny emails through
git-commits-head@vger.kernel.org, where the subject is completely screwed up:
Subject: \x64\x72\x6D\x2F\x74\x69\x6E\x79\x64\x72\x6D\x3A
\x6D\x69\x70\x69\x2D\x64\x62\x69\x3A \x53\x69\x6C\x65\x6E\x63\x65\x3A
‘\x63\x6D\x64’ \x6D\x61\x79 \x62\x65
and some of the mail headers end up in the body as well:
=?UTF-8?Q?\x75\x73\x65\x64_\x75\x6E\x69\x6E\x69\x74\x69\x61\x6C\x69\x7A\x65\x64?=
Return-Path: "Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
X-Git-Commit: b401f34314db7c60e6d23ee7771cd090b4ef56c1
X-Git-Parent: 1e8ad3d8da4763b238d09244d4d1177aa640c0d3
X-Git-Refname: refs/heads/master
Web:
https://git.kernel.org/torvalds/c/b401f34314db7c60e6d23ee7771cd090b4ef56c1
Commit: b401f34314db7c60e6d23ee7771cd090b4ef56c1
Parent: 1e8ad3d8da4763b238d09244d4d1177aa640c0d3
Refname: refs/heads/master
Author: Noralf Trønnes <noralf@tronnes.org>
AuthorDate: Thu Feb 23 14:29:55 2017 +0100
Committer: Dave Airlie <airlied@redhat.com>
CommitDate: Fri Feb 24 12:08:58 2017 +1000
drm/tinydrm: mipi-dbi: Silence: ‘cmd’ may be used uninitialized
My first guess was Noralf's UTF8 last name, but after looking at a few more,
they all seem to have UTF8 quotes from gcc output in the oneline summary.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* fatal error when diffing changed symlinks
From: Christophe Macabiau @ 2017-02-24 11:47 UTC (permalink / raw)
To: git
Hi,
with the commands below, you will get :
> fatal: bad object 0000000000000000000000000000000000000000
> show 0000000000000000000000000000000000000000: command returned error: 128
>
I am using version 2.5.5 fedora 23
cd /tmp
mkdir a
cd a
git init
touch b
ln -s b c
git add .
git commit -m 'first'
touch d
rm c
ln -s d c
git difftool --dir-diff
Thanks for your feedback,
Christophe
^ permalink raw reply
* [PATCH v6 1/1] config: add conditional include
From: Nguyễn Thái Ngọc Duy @ 2017-02-24 13:14 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, sschuberth, Matthieu Moy,
Nguyễn Thái Ngọc Duy
In-Reply-To: <20170224131425.32409-1-pclouds@gmail.com>
Sometimes a set of repositories want to share configuration settings
among themselves that are distinct from other such sets of repositories.
A user may work on two projects, each of which have multiple
repositories, and use one user.email for one project while using another
for the other.
Setting $GIT_DIR/.config works, but if the penalty of forgetting to
update $GIT_DIR/.config is high (especially when you end up cloning
often), it may not be the best way to go. Having the settings in
~/.gitconfig, which would work for just one set of repositories, would
not well in such a situation. Having separate ${HOME}s may add more
problems than it solves.
Extend the include.path mechanism that lets a config file include
another config file, so that the inclusion can be done only when some
conditions hold. Then ~/.gitconfig can say "include config-project-A
only when working on project-A" for each project A the user works on.
In this patch, the only supported grouping is based on $GIT_DIR (in
absolute path), so you would need to group repositories by directory, or
something like that to take advantage of it.
We already have include.path for unconditional includes. This patch goes
with includeIf.<condition>.path to make it clearer that a condition is
required. The new config has the same backward compatibility approach as
include.path: older git versions that don't understand includeIf will
simply ignore them.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/config.txt | 61 +++++++++++++++++++++++++++++
config.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++
t/t1305-config-include.sh | 56 +++++++++++++++++++++++++++
3 files changed, 214 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 015346c417..6c0cd2a273 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -91,6 +91,56 @@ found at the location of the include directive. If the value of the
relative to the configuration file in which the include directive was
found. See below for examples.
+Conditional includes
+~~~~~~~~~~~~~~~~~~~~
+
+You can include one config file from another conditionally by setting
+a `includeIf.<condition>.path` variable to the name of the file to be
+included. The variable's value is treated the same way as `include.path`.
+
+The condition starts with a keyword followed by a colon and some data
+whose format and meaning depends on the keyword. Supported keywords
+are:
+
+`gitdir`::
+
+ The data that follows the keyword `gitdir:` is used as a glob
+ pattern. If the location of the .git directory match the
+ pattern, the include condition is met.
++
+The .git location which may be auto-discovered, or come from
+`$GIT_DIR` environment variable. If the repository auto discovered via
+a .git file (e.g. from submodules, or a linked worktree), the .git
+location would be the final location, not where the .git file is.
++
+The pattern can contain standard globbing wildcards and two additional
+ones, `**/` and `/**`, that can match multiple path components. Please
+refer to linkgit:gitignore[5] for details. For convenience:
+
+ * If the pattern starts with `~/`, `~` will be substituted with the
+ content of the environment variable `HOME`.
+
+ * If the pattern starts with `./`, it is replaced with the directory
+ containing the current config file.
+
+ * If the pattern does not start with either `~/`, `./` or `/`, `**/`
+ will be automatically prepended. For example, the pattern `foo/bar`
+ becomes `**/foo/bar` and would match `/any/path/to/foo/bar`.
+
+ * If the pattern ends with `/`, `**` will be automatically added. For
+ example, the pattern `foo/` becomes `foo/**`. In other words, it
+ matches "foo" and everything inside, recursively.
+
+`gitdir/i`::
+ This is the same as `gitdir` except that matching is done
+ case-insensitively (e.g. on case-insensitive file sytems)
+
+A few more notes on matching with `gitdir` and `gitdir/i`:
+
+ * Symlinks in `$GIT_DIR` are not resolved before matching.
+
+ * Note that "../" is not special and will match literally, which is
+ unlikely what you want.
Example
~~~~~~~
@@ -119,6 +169,17 @@ Example
path = foo ; expand "foo" relative to the current file
path = ~/foo ; expand "foo" in your `$HOME` directory
+ ; include if $GIT_DIR is /path/to/foo/.git
+ [include-if "gitdir:/path/to/foo/.git"]
+ path = /path/to/foo.inc
+
+ ; include for all repositories inside /path/to/group
+ [include-if "gitdir:/path/to/group/"]
+ path = /path/to/foo.inc
+
+ ; include for all repositories inside $HOME/to/group
+ [include-if "gitdir:~/to/group/"]
+ path = /path/to/foo.inc
Values
~~~~~~
diff --git a/config.c b/config.c
index c6b874a7bf..ad16802c8a 100644
--- a/config.c
+++ b/config.c
@@ -13,6 +13,7 @@
#include "hashmap.h"
#include "string-list.h"
#include "utf8.h"
+#include "dir.h"
struct config_source {
struct config_source *prev;
@@ -170,9 +171,99 @@ static int handle_path_include(const char *path, struct config_include_data *inc
return ret;
}
+static int prepare_include_condition_pattern(struct strbuf *pat)
+{
+ struct strbuf path = STRBUF_INIT;
+ char *expanded;
+ int prefix = 0;
+
+ expanded = expand_user_path(pat->buf);
+ if (expanded) {
+ strbuf_reset(pat);
+ strbuf_addstr(pat, expanded);
+ free(expanded);
+ }
+
+ if (pat->buf[0] == '.' && is_dir_sep(pat->buf[1])) {
+ const char *slash;
+
+ if (!cf || !cf->path)
+ return error(_("relative config include "
+ "conditionals must come from files"));
+
+ /* TODO: escape wildcards */
+ strbuf_add_absolute_path(&path, cf->path);
+ slash = find_last_dir_sep(path.buf);
+ if (!slash)
+ die("BUG: how is this possible?");
+ strbuf_splice(pat, 0, 1, path.buf, slash - path.buf);
+ prefix = slash - path.buf + 1 /* slash */;
+ } else if (!is_absolute_path(pat->buf))
+ strbuf_insert(pat, 0, "**/", 3);
+
+ if (pat->len && is_dir_sep(pat->buf[pat->len - 1]))
+ strbuf_addstr(pat, "**");
+
+ strbuf_release(&path);
+ return prefix;
+}
+
+static int include_by_gitdir(const char *cond, size_t cond_len, int icase)
+{
+ struct strbuf text = STRBUF_INIT;
+ struct strbuf pattern = STRBUF_INIT;
+ int ret = 0, prefix;
+
+ strbuf_add_absolute_path(&text, get_git_dir());
+ strbuf_add(&pattern, cond, cond_len);
+ prefix = prepare_include_condition_pattern(&pattern);
+
+ if (prefix < 0)
+ goto done;
+
+ if (prefix > 0) {
+ /*
+ * perform literal matching on the prefix part so that
+ * any wildcard character in it can't create side effects.
+ */
+ if (text.len < prefix)
+ goto done;
+ if (!icase && strncmp(pattern.buf, text.buf, prefix))
+ goto done;
+ if (icase && strncasecmp(pattern.buf, text.buf, prefix))
+ goto done;
+ }
+
+ ret = !wildmatch(pattern.buf + prefix, text.buf + prefix,
+ icase ? WM_CASEFOLD : 0, NULL);
+
+done:
+ strbuf_release(&pattern);
+ strbuf_release(&text);
+ return ret;
+}
+
+static int include_condition_is_true(const char *cond, size_t cond_len)
+{
+ /* no condition (i.e., "include.path") is always true */
+ if (!cond)
+ return 1;
+
+ if (skip_prefix_mem(cond, cond_len, "gitdir:", &cond, &cond_len))
+ return include_by_gitdir(cond, cond_len, 0);
+ else if (skip_prefix_mem(cond, cond_len, "gitdir/i:", &cond, &cond_len))
+ return include_by_gitdir(cond, cond_len, 1);
+
+ error(_("unrecognized include condition: %.*s"), (int)cond_len, cond);
+ /* unknown conditionals are always false */
+ return 0;
+}
+
int git_config_include(const char *var, const char *value, void *data)
{
struct config_include_data *inc = data;
+ const char *cond, *key;
+ int cond_len;
int ret;
/*
@@ -185,6 +276,12 @@ int git_config_include(const char *var, const char *value, void *data)
if (!strcmp(var, "include.path"))
ret = handle_path_include(value, inc);
+
+ if (!parse_config_key(var, "includeif", &cond, &cond_len, &key) &&
+ include_condition_is_true(cond, cond_len) &&
+ !strcmp(key, "path"))
+ ret = handle_path_include(value, inc);
+
return ret;
}
diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh
index 9ba2ba11c3..f0cd2056ba 100755
--- a/t/t1305-config-include.sh
+++ b/t/t1305-config-include.sh
@@ -152,6 +152,62 @@ test_expect_success 'relative includes from stdin line fail' '
test_must_fail git config --file - test.one
'
+test_expect_success 'conditional include, both unanchored' '
+ git init foo &&
+ (
+ cd foo &&
+ echo "[includeIf \"gitdir:foo/\"]path=bar" >>.git/config &&
+ echo "[test]one=1" >.git/bar &&
+ echo 1 >expect &&
+ git config test.one >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'conditional include, $HOME expansion' '
+ (
+ cd foo &&
+ echo "[includeIf \"gitdir:~/foo/\"]path=bar2" >>.git/config &&
+ echo "[test]two=2" >.git/bar2 &&
+ echo 2 >expect &&
+ git config test.two >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'conditional include, full pattern' '
+ (
+ cd foo &&
+ echo "[includeIf \"gitdir:**/foo/**\"]path=bar3" >>.git/config &&
+ echo "[test]three=3" >.git/bar3 &&
+ echo 3 >expect &&
+ git config test.three >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'conditional include, relative path' '
+ echo "[includeIf \"gitdir:./foo/.git\"]path=bar4" >>.gitconfig &&
+ echo "[test]four=4" >bar4 &&
+ (
+ cd foo &&
+ echo 4 >expect &&
+ git config test.four >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'conditional include, both unanchored, icase' '
+ (
+ cd foo &&
+ echo "[includeIf \"gitdir/i:FOO/\"]path=bar5" >>.git/config &&
+ echo "[test]five=5" >.git/bar5 &&
+ echo 5 >expect &&
+ git config test.five >actual &&
+ test_cmp expect actual
+ )
+'
+
test_expect_success 'include cycles are detected' '
cat >.gitconfig <<-\EOF &&
[test]value = gitconfig
--
2.11.0.157.gd943d85
^ permalink raw reply related
* [PATCH v6 0/1] Conditional config include
From: Nguyễn Thái Ngọc Duy @ 2017-02-24 13:14 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, sschuberth, Matthieu Moy,
Nguyễn Thái Ngọc Duy
In-Reply-To: <20170224131425.32409-1-pclouds@gmail.com>
v6 has lots of text changes, most of which I shamelessly copied from
Junio's, with some minor edits. The biggest edit is the mention of .git
files, which is probably a good idea since .git files are used by
multi worktrees and submodules.
I could not move the "notes about matching" block into the gitdir
block though. I needed to indent it once (not twice like the "for
convenience" block) but my asciidoc-foo did not manage it so I settled
for "a few more notes on matching _with gitdir_" without moving it.
Code changes include renaming include-if to includeIf, and using
expand_user_path() instead of rolling my own.
Nguyễn Thái Ngọc Duy (1):
config: add conditional include
Documentation/config.txt | 61 +++++++++++++++++++++++++++++
config.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++
t/t1305-config-include.sh | 56 +++++++++++++++++++++++++++
3 files changed, 214 insertions(+)
--
2.11.0.157.gd943d85
^ permalink raw reply
* [PATCH v6 1/1] config: add conditional include
From: Nguyễn Thái Ngọc Duy @ 2017-02-24 13:14 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, sschuberth, Matthieu Moy,
Nguyễn Thái Ngọc Duy
In-Reply-To: <20170224131425.32409-1-pclouds@gmail.com>
Sometimes a set of repositories want to share configuration settings
among themselves that are distinct from other such sets of repositories.
A user may work on two projects, each of which have multiple
repositories, and use one user.email for one project while using another
for the other.
Setting $GIT_DIR/.config works, but if the penalty of forgetting to
update $GIT_DIR/.config is high (especially when you end up cloning
often), it may not be the best way to go. Having the settings in
~/.gitconfig, which would work for just one set of repositories, would
not well in such a situation. Having separate ${HOME}s may add more
problems than it solves.
Extend the include.path mechanism that lets a config file include
another config file, so that the inclusion can be done only when some
conditions hold. Then ~/.gitconfig can say "include config-project-A
only when working on project-A" for each project A the user works on.
In this patch, the only supported grouping is based on $GIT_DIR (in
absolute path), so you would need to group repositories by directory, or
something like that to take advantage of it.
We already have include.path for unconditional includes. This patch goes
with includeIf.<condition>.path to make it clearer that a condition is
required. The new config has the same backward compatibility approach as
include.path: older git versions that don't understand includeIf will
simply ignore them.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/config.txt | 61 +++++++++++++++++++++++++++++
config.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++
t/t1305-config-include.sh | 56 +++++++++++++++++++++++++++
3 files changed, 214 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 015346c417..6c0cd2a273 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -91,6 +91,56 @@ found at the location of the include directive. If the value of the
relative to the configuration file in which the include directive was
found. See below for examples.
+Conditional includes
+~~~~~~~~~~~~~~~~~~~~
+
+You can include one config file from another conditionally by setting
+a `includeIf.<condition>.path` variable to the name of the file to be
+included. The variable's value is treated the same way as `include.path`.
+
+The condition starts with a keyword followed by a colon and some data
+whose format and meaning depends on the keyword. Supported keywords
+are:
+
+`gitdir`::
+
+ The data that follows the keyword `gitdir:` is used as a glob
+ pattern. If the location of the .git directory match the
+ pattern, the include condition is met.
++
+The .git location which may be auto-discovered, or come from
+`$GIT_DIR` environment variable. If the repository auto discovered via
+a .git file (e.g. from submodules, or a linked worktree), the .git
+location would be the final location, not where the .git file is.
++
+The pattern can contain standard globbing wildcards and two additional
+ones, `**/` and `/**`, that can match multiple path components. Please
+refer to linkgit:gitignore[5] for details. For convenience:
+
+ * If the pattern starts with `~/`, `~` will be substituted with the
+ content of the environment variable `HOME`.
+
+ * If the pattern starts with `./`, it is replaced with the directory
+ containing the current config file.
+
+ * If the pattern does not start with either `~/`, `./` or `/`, `**/`
+ will be automatically prepended. For example, the pattern `foo/bar`
+ becomes `**/foo/bar` and would match `/any/path/to/foo/bar`.
+
+ * If the pattern ends with `/`, `**` will be automatically added. For
+ example, the pattern `foo/` becomes `foo/**`. In other words, it
+ matches "foo" and everything inside, recursively.
+
+`gitdir/i`::
+ This is the same as `gitdir` except that matching is done
+ case-insensitively (e.g. on case-insensitive file sytems)
+
+A few more notes on matching with `gitdir` and `gitdir/i`:
+
+ * Symlinks in `$GIT_DIR` are not resolved before matching.
+
+ * Note that "../" is not special and will match literally, which is
+ unlikely what you want.
Example
~~~~~~~
@@ -119,6 +169,17 @@ Example
path = foo ; expand "foo" relative to the current file
path = ~/foo ; expand "foo" in your `$HOME` directory
+ ; include if $GIT_DIR is /path/to/foo/.git
+ [include-if "gitdir:/path/to/foo/.git"]
+ path = /path/to/foo.inc
+
+ ; include for all repositories inside /path/to/group
+ [include-if "gitdir:/path/to/group/"]
+ path = /path/to/foo.inc
+
+ ; include for all repositories inside $HOME/to/group
+ [include-if "gitdir:~/to/group/"]
+ path = /path/to/foo.inc
Values
~~~~~~
diff --git a/config.c b/config.c
index c6b874a7bf..ad16802c8a 100644
--- a/config.c
+++ b/config.c
@@ -13,6 +13,7 @@
#include "hashmap.h"
#include "string-list.h"
#include "utf8.h"
+#include "dir.h"
struct config_source {
struct config_source *prev;
@@ -170,9 +171,99 @@ static int handle_path_include(const char *path, struct config_include_data *inc
return ret;
}
+static int prepare_include_condition_pattern(struct strbuf *pat)
+{
+ struct strbuf path = STRBUF_INIT;
+ char *expanded;
+ int prefix = 0;
+
+ expanded = expand_user_path(pat->buf);
+ if (expanded) {
+ strbuf_reset(pat);
+ strbuf_addstr(pat, expanded);
+ free(expanded);
+ }
+
+ if (pat->buf[0] == '.' && is_dir_sep(pat->buf[1])) {
+ const char *slash;
+
+ if (!cf || !cf->path)
+ return error(_("relative config include "
+ "conditionals must come from files"));
+
+ /* TODO: escape wildcards */
+ strbuf_add_absolute_path(&path, cf->path);
+ slash = find_last_dir_sep(path.buf);
+ if (!slash)
+ die("BUG: how is this possible?");
+ strbuf_splice(pat, 0, 1, path.buf, slash - path.buf);
+ prefix = slash - path.buf + 1 /* slash */;
+ } else if (!is_absolute_path(pat->buf))
+ strbuf_insert(pat, 0, "**/", 3);
+
+ if (pat->len && is_dir_sep(pat->buf[pat->len - 1]))
+ strbuf_addstr(pat, "**");
+
+ strbuf_release(&path);
+ return prefix;
+}
+
+static int include_by_gitdir(const char *cond, size_t cond_len, int icase)
+{
+ struct strbuf text = STRBUF_INIT;
+ struct strbuf pattern = STRBUF_INIT;
+ int ret = 0, prefix;
+
+ strbuf_add_absolute_path(&text, get_git_dir());
+ strbuf_add(&pattern, cond, cond_len);
+ prefix = prepare_include_condition_pattern(&pattern);
+
+ if (prefix < 0)
+ goto done;
+
+ if (prefix > 0) {
+ /*
+ * perform literal matching on the prefix part so that
+ * any wildcard character in it can't create side effects.
+ */
+ if (text.len < prefix)
+ goto done;
+ if (!icase && strncmp(pattern.buf, text.buf, prefix))
+ goto done;
+ if (icase && strncasecmp(pattern.buf, text.buf, prefix))
+ goto done;
+ }
+
+ ret = !wildmatch(pattern.buf + prefix, text.buf + prefix,
+ icase ? WM_CASEFOLD : 0, NULL);
+
+done:
+ strbuf_release(&pattern);
+ strbuf_release(&text);
+ return ret;
+}
+
+static int include_condition_is_true(const char *cond, size_t cond_len)
+{
+ /* no condition (i.e., "include.path") is always true */
+ if (!cond)
+ return 1;
+
+ if (skip_prefix_mem(cond, cond_len, "gitdir:", &cond, &cond_len))
+ return include_by_gitdir(cond, cond_len, 0);
+ else if (skip_prefix_mem(cond, cond_len, "gitdir/i:", &cond, &cond_len))
+ return include_by_gitdir(cond, cond_len, 1);
+
+ error(_("unrecognized include condition: %.*s"), (int)cond_len, cond);
+ /* unknown conditionals are always false */
+ return 0;
+}
+
int git_config_include(const char *var, const char *value, void *data)
{
struct config_include_data *inc = data;
+ const char *cond, *key;
+ int cond_len;
int ret;
/*
@@ -185,6 +276,12 @@ int git_config_include(const char *var, const char *value, void *data)
if (!strcmp(var, "include.path"))
ret = handle_path_include(value, inc);
+
+ if (!parse_config_key(var, "includeif", &cond, &cond_len, &key) &&
+ include_condition_is_true(cond, cond_len) &&
+ !strcmp(key, "path"))
+ ret = handle_path_include(value, inc);
+
return ret;
}
diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh
index 9ba2ba11c3..f0cd2056ba 100755
--- a/t/t1305-config-include.sh
+++ b/t/t1305-config-include.sh
@@ -152,6 +152,62 @@ test_expect_success 'relative includes from stdin line fail' '
test_must_fail git config --file - test.one
'
+test_expect_success 'conditional include, both unanchored' '
+ git init foo &&
+ (
+ cd foo &&
+ echo "[includeIf \"gitdir:foo/\"]path=bar" >>.git/config &&
+ echo "[test]one=1" >.git/bar &&
+ echo 1 >expect &&
+ git config test.one >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'conditional include, $HOME expansion' '
+ (
+ cd foo &&
+ echo "[includeIf \"gitdir:~/foo/\"]path=bar2" >>.git/config &&
+ echo "[test]two=2" >.git/bar2 &&
+ echo 2 >expect &&
+ git config test.two >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'conditional include, full pattern' '
+ (
+ cd foo &&
+ echo "[includeIf \"gitdir:**/foo/**\"]path=bar3" >>.git/config &&
+ echo "[test]three=3" >.git/bar3 &&
+ echo 3 >expect &&
+ git config test.three >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'conditional include, relative path' '
+ echo "[includeIf \"gitdir:./foo/.git\"]path=bar4" >>.gitconfig &&
+ echo "[test]four=4" >bar4 &&
+ (
+ cd foo &&
+ echo 4 >expect &&
+ git config test.four >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'conditional include, both unanchored, icase' '
+ (
+ cd foo &&
+ echo "[includeIf \"gitdir/i:FOO/\"]path=bar5" >>.git/config &&
+ echo "[test]five=5" >.git/bar5 &&
+ echo 5 >expect &&
+ git config test.five >actual &&
+ test_cmp expect actual
+ )
+'
+
test_expect_success 'include cycles are detected' '
cat >.gitconfig <<-\EOF &&
[test]value = gitconfig
--
2.11.0.157.gd943d85
^ permalink raw reply related
* Re: [PATCH v6 0/1] Conditional config include
From: Duy Nguyen @ 2017-02-24 13:18 UTC (permalink / raw)
To: Git Mailing List
Cc: Junio C Hamano, Jeff King, Sebastian Schuberth, Matthieu Moy,
Nguyễn Thái Ngọc Duy
In-Reply-To: <20170224131425.32409-3-pclouds@gmail.com>
And sorry for duplicates :P Somehow I managed to --dry-run correctly
the first time, then had _two_ 000*.patch in the command line. And
send-email happily let me shoot myself in the foot.
--
Duy
^ permalink raw reply
* [PATCH v6 0/1] Conditional config include
From: Nguyễn Thái Ngọc Duy @ 2017-02-24 13:14 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, sschuberth, Matthieu Moy,
Nguyễn Thái Ngọc Duy
In-Reply-To: <20170223122346.12222-1-pclouds@gmail.com>
v6 has lots of text changes, most of which I shamelessly copied from
Junio's, with some minor edits. The biggest edit is the mention of .git
files, which is probably a good idea since .git files are used by
multi worktrees and submodules.
I could not move the "notes about matching" block into the gitdir
block though. I needed to indent it once (not twice like the "for
convenience" block) but my asciidoc-foo did not manage it so I settled
for "a few more notes on matching _with gitdir_" without moving it.
Code changes include renaming include-if to includeIf, and using
expand_user_path() instead of rolling my own.
Nguyễn Thái Ngọc Duy (1):
config: add conditional include
Documentation/config.txt | 61 +++++++++++++++++++++++++++++
config.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++
t/t1305-config-include.sh | 56 +++++++++++++++++++++++++++
3 files changed, 214 insertions(+)
--
2.11.0.157.gd943d85
^ permalink raw reply
* Re: SHA1 collisions found
From: Geert Uytterhoeven @ 2017-02-24 15:52 UTC (permalink / raw)
To: Morten Welinder; +Cc: Joey Hess, Linus Torvalds, Git Mailing List
In-Reply-To: <CANv4PNmSjJUhFgC7GhpuBjiSQhwfAhrP8WxiP_siP2AjjXnrnw@mail.gmail.com>
On Thu, Feb 23, 2017 at 8:13 PM, Morten Welinder <mwelinder@gmail.com> wrote:
> The attack seems to generate two 64-bytes blocks, one quarter of which
> is repeated data. (Table-1 in the paper.)
>
> Assuming the result of that is evenly distributed and that bytes are
> independent, we can estimate the chances that the result is NUL-free
> as (255/256)^192 = 47% and the probability that the result is NUL and
> newline free as (254/256)^192 = 22%. Clearly one should not rely of
> NULs or newlines to save the day. On the other hand, the chances of
> an ascii result is something like (95/256)^192 = 10^-83.
Good. So they can replace linux/Documentation/logo.gif, but not actual source
files, not even if they contain hex arrays with "device parameters" ;-)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ 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