Git development
 help / color / mirror / Atom feed
* Re: SHA1 collisions found
From: Junio C Hamano @ 2017-02-24 19:20 UTC (permalink / raw)
  To: Stefan Beller; +Cc: David Lang, Ian Jackson, Joey Hess, git@vger.kernel.org
In-Reply-To: <CAGZ79kaZWe-8pMZnQv7uZtr8wXWawFeJjUa68-b0oa4yFo-HcA@mail.gmail.com>

Stefan Beller <sbeller@google.com> writes:

> On Fri, Feb 24, 2017 at 10:14 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
>> you are inviting people to start using
>>
>>     md5,54ddf8d47340e048166c45f439ce65fd
>>
>> as object names.
>
> which might even be okay for specific subsets of operations.
> (e.g. all local work including staging things, making local "fixup" commits)
>
> The addressing scheme should not be too hardcoded, we should rather
> treat it similar to the cipher schemes in pgp. The additional complexity that
> we have is the longevity of existence of things, though.

The not-so-well-hidden agenda was exactly that we _SHOULD_ not
mimick PGP.  They do not have a requirement to encourage everybody
to use the same thing because each message is encrypted/signed
independently, i.e. they do not have to chain things like we do.


^ permalink raw reply

* Re: [PATCH 10/15] update submodules: add submodule_move_head
From: Stefan Beller @ 2017-02-24 19:08 UTC (permalink / raw)
  To: Ramsay Jones
  Cc: git@vger.kernel.org, brian m. carlson, Jonathan Nieder,
	Brandon Williams, Junio C Hamano, David Turner
In-Reply-To: <7e920dd5-3638-57d9-0ca1-3f8aed4d700b@ramsayjones.plus.com>

On Thu, Feb 23, 2017 at 5:21 PM, Ramsay Jones
<ramsay@ramsayjones.plus.com> wrote:
>
>
> 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.]
>

good catch. Yes submodule_reset_index
ought to be static.

fixed in a reroll.

^ permalink raw reply

* Re: git email From: parsing (was Re: [GIT PULL] Staging/IIO driver patches for 4.11-rc1)
From: Jeff King @ 2017-02-24 18:56 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Greg KH, Linus Torvalds, Simon Sandström, Andrew Morton,
	Git Mailing List, Linux Kernel Mailing List, Linux Driver Project
In-Reply-To: <CAMuHMdX+xGUD+K16VCE=ywRgN1Zd4MzSr=NJ=2xz+8e_ixyGKQ@mail.gmail.com>

On Fri, Feb 24, 2017 at 12:03:45PM +0100, Geert Uytterhoeven wrote:

> > 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

Sorry, I don't have a clue on that one.

If you have UTF-8 or other non-ASCII characters in your subject,
format-patch will correctly do the rfc2047 encoding (and it should
always use QP). And that would kick in here because of the UTF-8 quotes.

But that weird "\x" encoding is not in any mail standard I know of (and
certainly Git would never do it).

The odd thing is that the quotes themselves _aren't_ encoded. Just
everything else.

One other feature is that subject line is long enough (especially
QP-encoded) that it spans two lines:

  $ git format-patch --stdout -1 b401f34314d | grep -A1 ^Subject
  Subject: [PATCH] =?UTF-8?q?drm/tinydrm:=20mipi-dbi:=20Silence:=20=E2=80=98?=
   =?UTF-8?q?cmd=E2=80=99=20may=20be=20used=20uninitialized?=

It's possible that something along the way is mis-handling subjects with
line-continuation (though why it would escape those characters, I don't
know).

> 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?=

That might be related to the whitespace continuation (the first line
after the break is the second line of the subject).

-Peff

^ permalink raw reply

* Re: SHA1 collisions found
From: Stefan Beller @ 2017-02-24 18:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: David Lang, Ian Jackson, Joey Hess, git@vger.kernel.org
In-Reply-To: <xmqqk28f4fti.fsf@gitster.mtv.corp.google.com>

On Fri, Feb 24, 2017 at 10:14 AM, Junio C Hamano <gitster@pobox.com> wrote:

> you are inviting people to start using
>
>     md5,54ddf8d47340e048166c45f439ce65fd
>
> as object names.

which might even be okay for specific subsets of operations.
(e.g. all local work including staging things, making local "fixup" commits)

The addressing scheme should not be too hardcoded, we should rather
treat it similar to the cipher schemes in pgp. The additional complexity that
we have is the longevity of existence of things, though.

^ permalink raw reply

* Re: [PATCH 3/3] Makefile: add USE_SHA1DC knob
From: Jeff King @ 2017-02-24 18:57 UTC (permalink / raw)
  To: HW42; +Cc: Linus Torvalds, Joey Hess, Git Mailing List
In-Reply-To: <16c6d843-a516-9265-d3e7-61b110acbdcf@ipsumj.de>

On Fri, Feb 24, 2017 at 06:36:00PM +0000, HW42 wrote:

> > +ifdef USE_SHA1DC
> > +	SHA1_HEADER = "sha1dc/sha1.h"
> > +	LIB_OBJS += sha1dc/sha1.o
> > +	LIB_OBJS += sha1dc/ubc_check.o
> > +else
> >  ifdef BLK_SHA1
> >  	SHA1_HEADER = "block-sha1/sha1.h"
> >  	LIB_OBJS += block-sha1/sha1.o
> > @@ -1403,6 +1412,7 @@ else
> >  endif
> >  endif
> >  endif
> > +endif
> 
> This sets SHA1_MAX_BLOCK_SIZE and the compiler flags for Apple
> CommonCrypto even if the user selects USE_SHA1DC. The same happens for
> BLK_SHA1. Is this intended?

No, it's not. I suspect that setting BLK_SHA1 has the same problem in
the current code, then.

> > +void git_SHA1DCUpdate(SHA1_CTX *ctx, const void *vdata, unsigned long len)
> > +{
> > +	const char *data = vdata;
> > +	/* We expect an unsigned long, but sha1dc only takes an int */
> > +	while (len > INT_MAX) {
> > +		SHA1DCUpdate(ctx, data, INT_MAX);
> > +		data += INT_MAX;
> > +		len -= INT_MAX;
> > +	}
> > +	SHA1DCUpdate(ctx, data, len);
> > +}
> 
> I think you can simply change the len parameter from unsigned into
> size_t (or unsigned long) in SHA1DCUpdate().
> https://github.com/cr-marcstevens/sha1collisiondetection/pull/6

Yeah, I agree that is a cleaner solution. My focus was on changing the
(presumably tested) sha1dc code as little as possible.

-Peff

^ permalink raw reply

* Re: [PATCH v6 1/1] config: add conditional include
From: Junio C Hamano @ 2017-02-24 18:35 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: git, Jeff King, sschuberth, Matthieu Moy
In-Reply-To: <20170224131425.32409-2-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> +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.

s/match/&es/, I think.

> ++
> +The .git location which may be auto-discovered, or come from

s/ which//?

> +`$GIT_DIR` environment variable. If the repository auto discovered via

s/If the /In a/?

> +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.

OK.

> @@ -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)
> +{
> + ...
> +		/* TODO: escape wildcards */
> +		strbuf_add_absolute_path(&path, cf->path);

Is this still TODO?  As this one returns the prefix length (which
probably wants to be commented before the function) and this codepath
computes the prefix to cover the path to here, doesn't caller already
do the right thing?

> +static int include_condition_is_true(const char *cond, size_t cond_len)
> +{
> +	/* no condition (i.e., "include.path") is always true */

Does this want to say "includeIf.path" instead?  "include.path" is
done by handle_path_include() without involving a call to this
function.

> +	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;
> +}

Thanks.

^ permalink raw reply

* [PATCH] submodule init: warn about falling back to a local path
From: Stefan Beller @ 2017-02-24 18:22 UTC (permalink / raw)
  To: j6t; +Cc: git, philipoakley, gitster, sop, Stefan Beller
In-Reply-To: <ae377bda-0776-f98b-5b6f-afa198647400@kdbg.org>

When a submodule is initialized, the config variable 'submodule.<name>.url'
is set depending on the value of the same variable in the .gitmodules
file. When the URL indicates to be relative, then the url is computed
relative to its default remote. The default remote cannot be determined
accurately in all cases, such that it falls back to 'origin'.

The 'origin' remote may not exist, though. In that case we give up looking
for a suitable remote and we'll just assume it to be a local relative path.

This can be confusing to users as there is a lot of guessing involved,
which is not obvious to the user.

So in the corner case of assuming a local autoritative truth, warn the
user to lessen the confusion.

This behavior was introduced in 4d6893200 (submodule add: allow relative
repository path even when no url is set, 2011-06-06), which shared the
code with submodule-init and then ported to C in 3604242f080a (submodule:
port init from shell to C, 2016-04-15).

In case of submodule-add, this behavior makes sense in some use cases[1],
however for submodule-init there does not seem to be an immediate obvious
use case to fall back to a local submodule. However there might be, so
warn instead of die here.

While adding the warning, also clarify the behavior of relative URLs in
the documentation.

[1] e.g. http://stackoverflow.com/questions/8721984/git-ignore-files-for-public-repository-but-not-for-private
"store a secret locally in a submodule, with no intention to publish it"

Reported-by: Shawn Pearce <spearce@spearce.org>
Signed-off-by: Stefan Beller <sbeller@google.com>
---

* reworded sentence to not omit half of it.
* tested rendering to be correct
* warn before xgetcwd

Thanks,
Stefan

 Documentation/git-submodule.txt | 36 ++++++++++++++++++++++--------------
 builtin/submodule--helper.c     |  8 +++-----
 2 files changed, 25 insertions(+), 19 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 8acc72ebb8..b810d37aa2 100644
--- 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.
 +
 <repository> is the URL of the new submodule's origin repository.
 This may be either an absolute URL, or (if it begins with ./
-or ../), the location relative to the superproject's origin
+or ../), the location relative to the superproject's default remote
 repository (Please note that to specify a repository 'foo.git'
 which is located right next to a superproject 'bar.git', you'll
 have to use '../foo.git' instead of './foo.git' - as one might expect
 when following the rules for relative URLs - because the evaluation
 of relative URLs in Git is identical to that of relative directories).
-If the superproject doesn't have an origin configured
++
+The default remote is the remote of the remote tracking branch
+of the current branch. If no such remote tracking branch exists or
+the HEAD is detached, "origin" is assumed to be the default remote.
+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.
-	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`.
-	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.
+	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.
+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.
 
 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..15a5430c00 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)) {
+				warning(_("could not lookup configuration '%s'. Assuming this repository is its own authoritative upstream."), remotesb.buf);
 				remoteurl = xgetcwd();
+			}
 			relurl = relative_url(remoteurl, url, NULL);
 			strbuf_release(&remotesb);
 			free(remoteurl);
-- 
2.12.0.rc1.16.ge4278d41a0.dirty


^ permalink raw reply related

* Re: SHA1 collisions found
From: Junio C Hamano @ 2017-02-24 18:14 UTC (permalink / raw)
  To: David Lang; +Cc: Ian Jackson, Joey Hess, git
In-Reply-To: <nycvar.QRO.7.75.62.1702240943540.6590@qynat-yncgbc>

David Lang <david@lang.hm> writes:

> On Fri, 24 Feb 2017, Junio C Hamano wrote:
>
>> *1* In the above toy example, length being 40 vs 64 is used as a
>>    sign between SHA-1 and the new hash, and careful readers may
>>    wonder if we should use sha-3,20769079d22... or something like
>>    that that more explicity identifies what hash is used, so that
>>    we can pick a hash whose length is 64 when we transition again.
>>
>>    I personally do not think such a prefix is necessary during the
>>    first transition; we will likely to adopt a new hash again, and
>>    at that point that third one can have a prefix to differenciate
>>    it from the second one.
>
> as the saying goes "in computer science the interesting numbers are 0,
> 1, and many", does it really simplify things much to support 2 hashes
> vs supporting more so that this issue doesn't have to be revisited?
> (other than selecting new hashes over time)

It seems that I wasn't clear enough, perhaps?  The scheme I outlined
does not have to revisit this issue at all.  It already declares what
you need to do when you add the third one.  

If it is not 40 or 64 bytes long, you just write it out.  If it is
one of these length, then you add some identifying prefix or
postfix.  IOW, if the second one is sha-3 and the third one is blake
(both used at 256-bit), then we would have three kinds of names,
written like so:

    20769079d22a9f8010232bdf6131918c33a1bf69
    20769079d22a9f8010232bdf6131918c33a1bf6910232bdf6131918c33a1bf69
    3,20769079d22a9f8010232bdf6131918c33a1bf6910232bdf6131918c33a1bf69

and the readers can well tell that the first one, being 40-chars
long, is SHA-1, the second one, being 64-chars long, is SHA-3, and
the last one, with the prefix '3' (only because that is the third
one officially supported by Git) and being 64-chars long, is blake,
for example.

I do not particularly care if it is prefix or postfix or something
else.  A not-so-well-hidden agenda is to avoid inviting people into
thinking that they can use their choice of random hash functions and
and claim that their hacked version is still a Git, as long as they
follow the object naming convention.  IOW, if you said something
like:

 * 40-hex is SHA-1 for historical reasons;
 * Others use hash-name, colon, and then N-hex.

you are inviting people to start using

    md5,54ddf8d47340e048166c45f439ce65fd

as object names.

^ permalink raw reply

* Re: [GIT PULL] l10n updates for 2.12.0
From: Junio C Hamano @ 2017-02-24 17:57 UTC (permalink / raw)
  To: Jiang Xin
  Cc: Alexander Shopov, Jordi Mas, Ralf Thielow, Jean-Noël Avila,
	Marco Paolone, Changwoo Ryu, Vasco Almeida, Dimitriy Ryazantcev,
	Peter Krefting, Trần Ngọc Quân, Git List
In-Reply-To: <CANYiYbHPahB6CAUMWTRL19SfG2eYqEq7vqH8SzAPjUSV_qiNiw@mail.gmail.com>

Jiang Xin <worldhello.net@gmail.com> writes:

> Hi Junio,
>
> Please pull l10n updates for Git 2.12.0:
>
> The following changes since commit 076c05393a047247ea723896289b48d6549ed7d0:
>
>   Hopefully the final batch of mini-topics before the final
> (2017-02-16 14:46:35 -0800)
>
> are available in the git repository at:
>
>   git://github.com/git-l10n/git-po tags/l10n-2.12.0-rnd2
>
> for you to fetch changes up to 1a79b2f1795a6ec4c70674ce930843aa59bff859:
>
>   l10n: zh_CN: for git v2.12.0 l10n round 2 (2017-02-25 00:19:14 +0800)

Will pull.  Thanks, all.

^ permalink raw reply

* Re: [PATCH v5 1/1] config: add conditional include
From: Junio C Hamano @ 2017-02-24 17:46 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Git Mailing List, Jeff King, Sebastian Schuberth, Matthieu Moy
In-Reply-To: <CACsJy8CerU-=PF6wqzUAM02jkrVVGJ5MA0NgL6z9bHn5KM6jiw@mail.gmail.com>

Duy Nguyen <pclouds@gmail.com> writes:

>>> +     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.

I would have said exactly that in my message if you already had
include_by_gitdir() and include_by_gitdir_i() as separate functions.

But I didn't, because the above code gives an excuse to the person
who adds the third one to be lazy and add another "else if" for
expediency, because making it table-driven would require an extra
work to add two wrapper functions that do not have anything to do
with the third one being added.


^ permalink raw reply

* Re: fatal error when diffing changed symlinks
From: Junio C Hamano @ 2017-02-24 17:53 UTC (permalink / raw)
  To: Christophe Macabiau; +Cc: git, Johannes Schindelin
In-Reply-To: <CAC0icTZ87yeYndPFyjD4nkJBcw5SC-bgUJYbEzYqH7YhSFZvuQ@mail.gmail.com>

Christophe Macabiau <christophemacabiau@gmail.com> writes:

> 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

A slightly worse is that the upcoming Git will ship with a rewritten
"difftool" that makes the above sequence segfault.  As either way is
bad, I do not particularly think the rewritten one needs to be
reverted (it would give "fatal: bad object" then, and would not fix
your problem), but it needs to be looked at.

Thanks for a report.


^ permalink raw reply

* Re: SHA1 collisions found
From: David Lang @ 2017-02-24 17:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Ian Jackson, Joey Hess, git
In-Reply-To: <xmqq60jz5wbm.fsf@gitster.mtv.corp.google.com>

On Fri, 24 Feb 2017, Junio C Hamano wrote:

> *1* In the above toy example, length being 40 vs 64 is used as a
>    sign between SHA-1 and the new hash, and careful readers may
>    wonder if we should use sha-3,20769079d22... or something like
>    that that more explicity identifies what hash is used, so that
>    we can pick a hash whose length is 64 when we transition again.
>
>    I personally do not think such a prefix is necessary during the
>    first transition; we will likely to adopt a new hash again, and
>    at that point that third one can have a prefix to differenciate
>    it from the second one.

as the saying goes "in computer science the interesting numbers are 0, 1, and 
many", does it really simplify things much to support 2 hashes vs supporting 
more so that this issue doesn't have to be revisited? (other than selecting new 
hashes over time)

David Lang

^ permalink raw reply

* Re: [PATCH] docs/git-gc: fix default value for `--aggressiveDepth`
From: Junio C Hamano @ 2017-02-24 17:49 UTC (permalink / raw)
  To: Jeff King; +Cc: Patrick Steinhardt, git
In-Reply-To: <20170224090035.ju7evvro7mixc52w@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> 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).

Thanks, both.  Will queue.

^ permalink raw reply

* Re: SHA1 collisions found
From: Jason Cooper @ 2017-02-24 17:23 UTC (permalink / raw)
  To: Ian Jackson; +Cc: Joey Hess, git
In-Reply-To: <22704.19873.860148.22472@chiark.greenend.org.uk>

Hi Ian,

On Fri, Feb 24, 2017 at 03:13:37PM +0000, Ian Jackson wrote:
> Joey Hess writes ("SHA1 collisions found"):
> > https://shattered.io/static/shattered.pdf
> > https://freedom-to-tinker.com/2017/02/23/rip-sha-1/
> > 
> > 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.
> 
> I have been thinking about how to do a transition from SHA1 to another
> hash function.
> 
> I have concluded that:
> 
>  * We can should avoid expecting everyone to rewrite all their
>    history.

Agreed.

>  * Unfortunately, because the data formats (particularly, the commit
>    header) are not in practice extensible (because of the way existing
>    code parses them), it is not useful to try generate new data (new
>    commits etc.) containing both new hashes and old hashes: old
>    clients will mishandle the new data.

My thought here is:

 a) re-hash blobs with sha256, hardlink to sha1 objects
 b) create new tree objects which are mirrors of each sha1 tree object,
    but purely sha256
 c) mirror commits, but they are also purely sha256
 d) future PGP signed tags would sign both hashes (or include both?)

Which would end up something like:

  .git/
    \... #usual files
    \objects
      \ef
        \3c39f7522dc55a24f64da9febcfac71e984366
    \objects-sha2_256
      \72
        \604fd2de5f25c89d692b01081af93bcf00d2af34549d8d1bdeb68bc048932
    \info
      \...
    \info-sha2_256
      \refs #uses sha256 commit identifiers

Basically, keep the sha256 stuff out of the way for legacy clients, and
new clients will still be able to use it.

There shouldn't be a need to re-sign old signed tags if the underlying
objects are counter-hashed.  There might need to be some transition
info, though.

Say a new client does 'git tag -v tags/v3.16' in the kernel tree.  I would
expect it to check the sha1 hashes, verify the PGP signed tag, and then
also check the sha256 counter-hashes of the relevant objects.

thx,

Jason.

^ permalink raw reply

* [PATCH] travis-ci: run scan-build every time
From: Samuel Lijin @ 2017-02-24 17:29 UTC (permalink / raw)
  To: git@vger.kernel.org

Introduces the scan-build static code analysis tool from the Clang
project to all Travis CI builds. Installs clang (since scan-build
needs clang as a dependency) to make this possible (on macOS, also
updates PATH to allow scan-build to be invoked without referencing the
full path).
---

A build with this patch can be found at [1]. Note that if reports *are*
generated, this doesn't allow us to access them, and if we dumped
the reports as build artifacts, I'm not sure where we would want to
dump them to.

It's worth noting that there seems to be a weird issue with scan-build
where it *will* generate a report for something locally, but won't do it
on Travis. See [2] for an example where I have a C program with a
very obvious memory leak but scan-build on Travis doesn't generate
a report (despite complaining about it in stdout), even though it does
on my local machine.

[1] https://travis-ci.org/sxlijin/git/builds/204853233
[2] https://travis-ci.org/sxlijin/travis-testing/jobs/205025319#L331-L342

 .travis.yml | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 9c63c8c3f..1038b1b3d 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -20,6 +20,7 @@ addons:
     - language-pack-is
     - git-svn
     - apache2
+    - clang

 env:
   global:
@@ -78,9 +79,8 @@ before_install:
       brew update --quiet
       # Uncomment this if you want to run perf tests:
       # brew install gnu-time
-      brew install git-lfs gettext
-      brew link --force gettext
-      brew install caskroom/cask/perforce
+      brew install git-lfs gettext caskroom/cask/perforce llvm
+      brew link --force gettext llvm
       ;;
     esac;
     echo "$(tput setaf 6)Perforce Server Version$(tput sgr0)";
@@ -92,9 +92,9 @@ before_install:
     mkdir -p $HOME/travis-cache;
     ln -s $HOME/travis-cache/.prove t/.prove;

-before_script: make --jobs=2
+before_script: scan-build make --jobs=2

-script: make --quiet test
+script: scan-build make --quiet test

 after_failure:
   - >
--
2.11.1

^ permalink raw reply related

* Re: SHA1 collisions found
From: Junio C Hamano @ 2017-02-24 17:32 UTC (permalink / raw)
  To: Ian Jackson; +Cc: Joey Hess, git
In-Reply-To: <22704.19873.860148.22472@chiark.greenend.org.uk>

Ian Jackson <ijackson@chiark.greenend.org.uk> writes:

> I have been thinking about how to do a transition from SHA1 to another
> hash function.

Good.  I think many of us have also been, too, not necessarily just
in the past few days in response to shattered, but over the last 10
years, yet without coming to a consensus design ;-)

> I have concluded that:
>
>  * We can should avoid expecting everyone to rewrite all their
>    history.

Yes.

>  * Unfortunately, because the data formats (particularly, the commit
>    header) are not in practice extensible (because of the way existing
>    code parses them), it is not useful to try generate new data (new
>    commits etc.) containing both new hashes and old hashes: old
>    clients will mishandle the new data.

Yes.

>  * Therefore the transition needs to be done by giving every object
>    two names (old and new hash function).  Objects may refer to each
>    other by either name, but must pick one.  The usual shape of

I do not think it is necessrily so.  Existing code may not be able
to read anything new, but you can make the new code understand
object names in both formats, and for a smooth transition, I think
the new code needs to.

For example, a new commit that records a merge of an old and a new
commit whose resulting tree happens to be the same as the tree of
the old commit may begin like so:

    tree 21b97d4c4f968d1335f16292f954dfdbb91353f0
    parent 20769079d22a9f8010232bdf6131918c33a1bf6910232bdf6131918c33a1bf69
    parent 22af6fef9b6538c9e87e147a920be9509acf1ddd

naming the only object whose name was done with new hash with the
new longer hash, while recording the names of the other existing
objects with SHA-1.  We would need to extend the object format for
tag (which would be trivial as the object reference is textual and
similar to a commit) and tree (much harder), of course.

As long as the reader can tell from the format of object names
stored in the "new object format" object from what era is being
referred to in some way [*1*], we can name new objects with only new
hash, I would think.  "new refers only to new" that stratifies
objects into older and newer may make things simpler, but I am not
convinced yet that it would give our users a smooth enough
transition path (but I am open to be educated and pursuaded the
other way).


[Footnote]

*1* In the above toy example, length being 40 vs 64 is used as a
    sign between SHA-1 and the new hash, and careful readers may
    wonder if we should use sha-3,20769079d22... or something like
    that that more explicity identifies what hash is used, so that
    we can pick a hash whose length is 64 when we transition again.

    I personally do not think such a prefix is necessary during the
    first transition; we will likely to adopt a new hash again, and
    at that point that third one can have a prefix to differenciate
    it from the second one.

^ permalink raw reply

* Re: SHA1 collisions found
From: ankostis @ 2017-02-24 17:04 UTC (permalink / raw)
  To: Ian Jackson; +Cc: git
In-Reply-To: <22704.19873.860148.22472@chiark.greenend.org.uk>

On 24 February 2017 at 16:13, Ian Jackson
<ijackson@chiark.greenend.org.uk> wrote:
>
> Joey Hess writes ("SHA1 collisions found"):
> > https://shattered.io/static/shattered.pdf
> > https://freedom-to-tinker.com/2017/02/23/rip-sha-1/
> >
> > 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.
>
> I have been thinking about how to do a transition from SHA1 to another
> hash function.
>
> I have concluded that:
>
>  * We can should avoid expecting everyone to rewrite all their
>    history.
>
>  * Unfortunately, because the data formats (particularly, the commit
>    header) are not in practice extensible (because of the way existing
>    code parses them), it is not useful to try generate new data (new
>    commits etc.) containing both new hashes and old hashes: old
>    clients will mishandle the new data.
>
>  * Therefore the transition needs to be done by giving every object
>    two names (old and new hash function).  Objects may refer to each
>    other by either name, but must pick one.  The usual shape of
>    project histories will be a pile of old commits referring to each
>    other by old names, surmounted by new commits referrring to each
>    other by new names.
>
>  * It is not possible to solve this problem without extending the
>    object name format.  Therefore all software which calls git and
>    expects to handle object names will need to be updated.
>
> I have been writing a more detailed transition plan.  I hope to post
> this within a few days.

It would be great to have a rough plan of the transition to a new hash
function.

We are writing a git-based application to store electronic-files for
legislative purposes for EU.
And one of the great questions we face is about git's SHA-1 validity in 5
or 20 years of time from now.

Is it possible to have an assessment of the situation for this transition?

Best regards for your efforts,
  Kostis

^ permalink raw reply

* Re: git bugs
From: Johannes Schindelin @ 2017-02-24 16:52 UTC (permalink / raw)
  To: Sean Hunt; +Cc: git
In-Reply-To: <CAHikyLoK-h4tY_rxGikaSSv6AmcrBAXiayDFTtLa44A9qMZDqA@mail.gmail.com>

Hi,


On Thu, 23 Feb 2017, Sean Hunt wrote:

> There are a few bugs I git I noticed when using mingw, mingw64,
> cygwin, and cygwin64. These bugs are the following:
> 
> if I do git ``rebase -i --root`` and tell it to edit every commit to
> gpg sign all my commits it bugs out and merges all of the commits into
> 1 commit instead of only appending the ``-S`` to each and every commit
> and keeping all of the commits. It is as if I told it to squash the
> commits but yet I did not. There is also another bug where if I clone
> a repo on Windows and not on github desktop and that I placed commits
> to the repo on github web and then when I rebase to squash the commits
> to 1 commit (some repos are doing it as a requirement for 1 commit
> PR's) that all of my commits on the remote (fork in this case) that is
> linked to an open pull request are discarded and then the pull request
> is somehow and oddly closed. It is super annoying.

It appears this was reported as
https://github.com/git-for-windows/git/issues/1072, too.

I asked for clarification there, as I am quite confused by the
description.

Ciao,
Johannes

^ permalink raw reply

* [GIT PULL] l10n updates for 2.12.0
From: Jiang Xin @ 2017-02-24 16:29 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Alexander Shopov, Jordi Mas, Ralf Thielow, Jean-Noël Avila,
	Marco Paolone, Changwoo Ryu, Vasco Almeida, Dimitriy Ryazantcev,
	Peter Krefting, Trần Ngọc Quân, Jiang Xin,
	Git List

Hi Junio,

Please pull l10n updates for Git 2.12.0:

The following changes since commit 076c05393a047247ea723896289b48d6549ed7d0:

  Hopefully the final batch of mini-topics before the final
(2017-02-16 14:46:35 -0800)

are available in the git repository at:

  git://github.com/git-l10n/git-po tags/l10n-2.12.0-rnd2

for you to fetch changes up to 1a79b2f1795a6ec4c70674ce930843aa59bff859:

  l10n: zh_CN: for git v2.12.0 l10n round 2 (2017-02-25 00:19:14 +0800)

----------------------------------------------------------------
l10n-2.12.0-rnd2

----------------------------------------------------------------
Alexander Shopov (1):
      l10n: bg:  Updated Bulgarian translation (2913t+0f+0u)

Anthony Ramine (1):
      l10n: fr.po: Fix a typo in the French translation

Changwoo Ryu (2):
      l10n: ko.po: Update Korean translation
      l10n: ko.po: Update Korean translation

Jean-Noel Avila (2):
      l10n: fr.po: v2.11-rc0 first round
      l10n: fr.po: v2.12.0 round 2 3139t

Jiang Xin (14):
      Merge branch 'master' of https://github.com/Softcatala/git-po
      Merge branch 'master' of https://github.com/Softcatala/git-po
      Merge branch 'master' of git://github.com/git-l10n/git-po
      l10n: git.pot: v2.12.0 round 1 (239 new, 15 removed)
      Merge branch 'fr_v2.11.0_rnd1' of git://github.com/jnavila/git
      Merge branch 'master' of git://github.com/nafmo/git-l10n-sv
      Merge branch 'master' of https://github.com/Softcatala/git-po
      Merge branch 'master' of https://github.com/vnwildman/git
      Merge branch 'master' of git://github.com/git-l10n/git-po
      l10n: git.pot: v2.12.0 round 2 (2 new)
      Merge branch 'master' of https://github.com/vnwildman/git
      Merge branch 'ko/merge-l10n' of https://github.com/changwoo/git-l10n-ko
      Merge branch 'master' of git://github.com/nafmo/git-l10n-sv
      l10n: zh_CN: for git v2.12.0 l10n round 2

Joachim Jablon (2):
      l10n: fr.po: Fix typos
      l10n: fr.po: Remove gender specific adjectives

Jordi Mas (4):
      l10n: New Catalan translation maintainer
      l10n: fixes to Catalan translation
      l10n: update Catalan translation
      l10n: Update Catalan translation

Peter Krefting (2):
      l10n: sv.po: Update Swedish translation (3137t0f0u)
      l10n: sv.po: Update Swedish translation (3139t0f0u)

Ralf Thielow (1):
      l10n: de.po: translate 241 messages

Ray Chen (1):
      l10n: zh_CN: review for git v2.11.0 l10n

Trần Ngọc Quân (2):
      l10n: vi.po: Updated Vietnamese translation (3137t)
      l10n: vi.po (3139t): Updated 2 new messages for rc1

Vasco Almeida (1):
      l10n: pt_PT: update Portuguese tranlation

 po/TEAMS    |     5 +-
 po/bg.po    | 10422 ++++++++++++++++++++++++++++++++++------------------------
 po/ca.po    |  4894 +++++++++++++++++----------
 po/de.po    |  4331 +++++++++++++++---------
 po/fr.po    |  4293 +++++++++++++++---------
 po/git.pot  |  3997 ++++++++++++++--------
 po/ko.po    |  4221 ++++++++++++++++--------
 po/pt_PT.po |  4562 ++++++++++++++++---------
 po/sv.po    |  4275 +++++++++++++++---------
 po/vi.po    |  4275 +++++++++++++++---------
 po/zh_CN.po |  4258 +++++++++++++++---------
 11 files changed, 31663 insertions(+), 17870 deletions(-)

^ permalink raw reply

* Re: SHA1 collisions found
From: Ian Jackson @ 2017-02-24 15:13 UTC (permalink / raw)
  To: Joey Hess; +Cc: git
In-Reply-To: <20170223164306.spg2avxzukkggrpb@kitenet.net>

Joey Hess writes ("SHA1 collisions found"):
> https://shattered.io/static/shattered.pdf
> https://freedom-to-tinker.com/2017/02/23/rip-sha-1/
> 
> 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.

I have been thinking about how to do a transition from SHA1 to another
hash function.

I have concluded that:

 * We can should avoid expecting everyone to rewrite all their
   history.

 * Unfortunately, because the data formats (particularly, the commit
   header) are not in practice extensible (because of the way existing
   code parses them), it is not useful to try generate new data (new
   commits etc.) containing both new hashes and old hashes: old
   clients will mishandle the new data.

 * Therefore the transition needs to be done by giving every object
   two names (old and new hash function).  Objects may refer to each
   other by either name, but must pick one.  The usual shape of
   project histories will be a pile of old commits referring to each
   other by old names, surmounted by new commits referrring to each
   other by new names.

 * It is not possible to solve this problem without extending the
   object name format.  Therefore all software which calls git and
   expects to handle object names will need to be updated.

I have been writing a more detailed transition plan.  I hope to post
this within a few days.

Ian.

^ 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

* [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: [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 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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox