* Re: [PATCHv2 1/2] attr: convert to new threadsafe API
From: Stefan Beller @ 2016-10-27 0:22 UTC (permalink / raw)
To: Junio C Hamano
Cc: Duy Nguyen, git@vger.kernel.org, Johannes Schindelin,
Johannes Sixt, Jeff King, Brandon Williams, Simon Ruderich
In-Reply-To: <xmqqfuniabo6.fsf@gitster.mtv.corp.google.com>
On Wed, Oct 26, 2016 at 5:16 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> +* Allocate an array of `struct git_attr_result` either on the stack
>> + or via `git_attr_result_alloc` on the heap when the result size
>> + is not known at compile time. The call to initialize
>> the result is not thread safe, because different threads need their
>> own thread local result anyway.
>
> Do you want to keep the last sentence? "The call to initialize the
> result is not thread safe..."? Is that true?
I'll drop that sentence, as it overstates the situation.
To explain, you can either have:
struct git_attr_result result[2];
or
struct git_attr_result *result = git_attr_result_alloc(check);
and both are running just fine in a thread. However you should not
make that variable static. But maybe that is too much common sense
and hence confusing.
>
>> @@ -103,7 +105,7 @@ To see how attributes "crlf" and "ident" are set
>> for different paths.
>> const char *path;
>> struct git_attr_result result[2];
>>
>> - git_check_attr(path, check, result);
>> + git_check_attr(path, &check, result);
>
> What's the point of this change? Isn't check typically a pointer
> already?
This ought to go to
git_attr_check_initl(&check, "crlf", "ident", NULL);
instead.
>
^ permalink raw reply
* Re: [PATCHv2 1/2] attr: convert to new threadsafe API
From: Junio C Hamano @ 2016-10-27 0:16 UTC (permalink / raw)
To: Stefan Beller
Cc: Duy Nguyen, git@vger.kernel.org, Johannes Schindelin,
Johannes Sixt, Jeff King, Brandon Williams, Simon Ruderich
In-Reply-To: <CAGZ79kaR4DddoHQNUUvRAY=_PK5qqS=ws_Wkfa-EXT2seN5b=A@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> +* Allocate an array of `struct git_attr_result` either on the stack
> + or via `git_attr_result_alloc` on the heap when the result size
> + is not known at compile time. The call to initialize
> the result is not thread safe, because different threads need their
> own thread local result anyway.
Do you want to keep the last sentence? "The call to initialize the
result is not thread safe..."? Is that true?
> @@ -103,7 +105,7 @@ To see how attributes "crlf" and "ident" are set
> for different paths.
> const char *path;
> struct git_attr_result result[2];
>
> - git_check_attr(path, check, result);
> + git_check_attr(path, &check, result);
What's the point of this change? Isn't check typically a pointer
already?
^ permalink raw reply
* Re: [PATCHv2 1/2] attr: convert to new threadsafe API
From: Stefan Beller @ 2016-10-27 0:08 UTC (permalink / raw)
To: Junio C Hamano
Cc: Duy Nguyen, git@vger.kernel.org, Johannes Schindelin,
Johannes Sixt, Jeff King, Brandon Williams, Simon Ruderich
In-Reply-To: <xmqqoa26aek6.fsf@gitster.mtv.corp.google.com>
On Wed, Oct 26, 2016 at 4:14 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> @@ -53,19 +57,32 @@ value of the attribute for the path.
>> Querying Specific Attributes
>> ----------------------------
>>
>> -* Prepare `struct git_attr_check` using git_attr_check_initl()
>> +* Prepare a `struct git_attr_check` using `git_attr_check_initl()`
>> function, enumerating the names of attributes whose values you are
>> interested in, terminated with a NULL pointer. Alternatively, an
>> - empty `struct git_attr_check` can be prepared by calling
>> - `git_attr_check_alloc()` function and then attributes you want to
>> - ask about can be added to it with `git_attr_check_append()`
>> - function.
>> -
>> -* Call `git_check_attr()` to check the attributes for the path.
>> -
>> -* Inspect `git_attr_check` structure to see how each of the
>> - attribute in the array is defined for the path.
>> -
>> + empty `struct git_attr_check` as allocated by git_attr_check_alloc()
>> + can be prepared by calling `git_attr_check_alloc()` function and
>> + then attributes you want to ask about can be added to it with
>> + `git_attr_check_append()` function.
>
> I think that my version that was discarded forbade appending once
> you started to use the check for querying, because the check was
> meant to be used as a key for an attr-stack and the check-specific
> attr-stack was planned to keep only the attrs the check is
> interested in (and appending is to change the set of attrs the check
> is interested in, invalidating the attr-stack at that point).
>
> If you lost that restriction, that is good (I didn't check the
> implementation, though). Otherwise we'd need to say something here.
That restriction still applies. Though I think mentioning it in the
paragraph where we describe querying makes more sense.
> initialization?
done
>
> Grammo? "either on the stack, or dynamically in the heap"?
done
>
> Having result defined statically is not thread safe for that
> reason. It is not clear what you mean by "The call to initialize
> the result"; having it on the stack or have one dynamically on the
> heap ought to be thread safe.
done
>> -}
>> + static struct git_attr_check *check;
>> + git_attr_check_initl(check, "crlf", "ident", NULL);
>
> I think you are still missing "&" here.
done
>> + if (check)
>> + return; /* already done */
>> check = git_attr_check_alloc();
>
> You may want to say that this is thread-unsafe.
It is not; see the implementation:
void git_attr_check_append(struct git_attr_check *check,
const struct git_attr *attr)
{
int i;
if (check->finalized)
die("BUG: append after git_attr_check structure is finalized");
if (!attr_check_is_dynamic(check))
die("BUG: appending to a statically initialized git_attr_check");
attr_lock();
for (i = 0; i < check->check_nr; i++)
if (check->attr[i] == attr)
break;
if (i == check->check_nr) {
ALLOC_GROW(check->attr, check->check_nr + 1, check->check_alloc);
check->attr[check->check_nr++] = attr;
}
attr_unlock();
}
>> +* Call `git_all_attrs()`.
>
> Hmph, the caller does not know what attribute it is interested in,
> and it is unclear "how" the former needs to be set up from this
> description. Should it prepare an empty one that can be appended?
>
Good point on clarifying this one. It is just needed to have
NULL pointers around:
struct git_attr_check *check = NULL;
struct git_attr_result *result = NULL;
git_all_attrs(full_path, &local_check, &result);
// proceed as in the case above
All comments from above yield the following diff:
diff --git a/Documentation/technical/api-gitattributes.txt
b/Documentation/technical/api-gitattributes.txt
index 4d8ef93..d221736 100644
--- a/Documentation/technical/api-gitattributes.txt
+++ b/Documentation/technical/api-gitattributes.txt
@@ -67,19 +67,21 @@ Querying Specific Attributes
Both ways with `git_attr_check_initl()` as well as the
alloc and append route are thread safe, i.e. you can call it
from different threads at the same time; when check determines
- the initialzisation is still needed, the threads will use a
+ the initialization is still needed, the threads will use a
single global mutex to perform the initialization just once, the
others will wait on the the thread to actually perform the
initialization.
-* Allocate an array of `struct git_attr_result` either statically on the
- as a variable on the stack or dynamically via `git_attr_result_alloc`
- when the result size is not known at compile time. The call to initialize
+* Allocate an array of `struct git_attr_result` either on the stack
+ or via `git_attr_result_alloc` on the heap when the result size
+ is not known at compile time. The call to initialize
the result is not thread safe, because different threads need their
own thread local result anyway.
* Call `git_check_attr()` to check the attributes for the path,
the given `git_attr_result` will be filled with the result.
+ You must not change the `struct git_attr_check` after calling
+ `git_check_attr()`.
* Inspect each `git_attr_result` structure to see how
each of the attribute in the array is defined for the path.
@@ -103,7 +105,7 @@ To see how attributes "crlf" and "ident" are set
for different paths.
const char *path;
struct git_attr_result result[2];
- git_check_attr(path, check, result);
+ git_check_attr(path, &check, result);
------------
. Act on `result.value[]`:
@@ -155,10 +157,20 @@ To get the values of all attributes associated
with a file:
* Setup a local variables for the question
`struct git_attr_check` as well as a pointer where the result
- `struct git_attr_result` will be stored.
+ `struct git_attr_result` will be stored. Both should be initialized
+ to NULL.
+
+------------
+ struct git_attr_check *check = NULL;
+ struct git_attr_result *result = NULL;
+------------
* Call `git_all_attrs()`.
+------------
+ git_all_attrs(full_path, &check, &result);
+------------
+
* Iterate over the `git_attr_check.attr[]` array to examine the
attribute names. The name of the attribute described by a
`git_attr_check.attr[]` object can be retrieved via
^ permalink raw reply related
* Re: "git subtree --squash" interacts poorly with revert, merge, and rebase
From: Junio C Hamano @ 2016-10-26 23:59 UTC (permalink / raw)
To: Stefan Beller; +Cc: Matt McCutchen, git
In-Reply-To: <CAGZ79kaw0s_PC2AstRVwFT8N1CJVC_7yQfC19zPzRjAqkSpMDg@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
>> - We have to make separate commits and manage corresponding topic
>> branches for the superproject and subprojects.
>
> Well yeah, that is how submodule work on a conceptual level.
> While having multiple commits may seem like overhead, note
> the subtle difference for these commits. One if deep down in the
> stack patching one of the submodules, the other is a high level
> commit advancing the submodule pointer.
>
> Note that the target audience of these two commit messages
> might be vastly different, hence can be worded differently.
> (The submodule describing how you fixed e.g. a memleak or race condition
> and the superproject describes on why you needed to include that submodule,
> e.g. because you switched your toplevel application to use threads.)
Both good points.
Another thing to keep in mind is that in a well-organized project,
it is expected that you would have multiple commits in a submodule,
solving one single issue that is needed by the superproject in a
finer grained way, before the resulting submodule tip is recorded in
the tree of the superproject in one commit. IOW, between the time
the superproject's history moves by one commit, the submodule may
have multiple commits in order for the submodule to become ready to
be consumed by the superproject.
^ permalink raw reply
* Re: "git subtree --squash" interacts poorly with revert, merge, and rebase
From: Stefan Beller @ 2016-10-26 23:23 UTC (permalink / raw)
To: Matt McCutchen; +Cc: git
In-Reply-To: <1477523244.2764.114.camel@mattmccutchen.net>
On Wed, Oct 26, 2016 at 4:07 PM, Matt McCutchen <matt@mattmccutchen.net> wrote:
> I'm the lead developer of a research software application (https://bitb
> ucket.org/objsheets/objsheets) that uses modified versions of two
> third-party libraries, which we need to version and distribute along
> with our application. For better or for worse, we haven't made it a
> priority to upstream our changes, so for now we just want to optimize
> for ease of (1) making and reviewing changes and (2) upgrading to newer
> upstream versions.
>
> We've been using git submodules, but that's a pain for several reasons:
> - We have to run "git submodule update" manually.
That is true for now. :( But there are plans to revive a patch series
to include updating the submodule into git-checkout.
https://github.com/stefanbeller/git/commits/submodule-co
> - We have to make separate commits and manage corresponding topic
> branches for the superproject and subprojects.
Well yeah, that is how submodule work on a conceptual level.
While having multiple commits may seem like overhead, note
the subtle difference for these commits. One if deep down in the
stack patching one of the submodules, the other is a high level
commit advancing the submodule pointer.
Note that the target audience of these two commit messages
might be vastly different, hence can be worded differently.
(The submodule describing how you fixed e.g. a memleak or race condition
and the superproject describes on why you needed to include that submodule,
e.g. because you switched your toplevel application to use threads.)
> - A diff of the superproject doesn't include the content of
> subprojects.
A recent patch series by Jacob Keller (jk/diff-submodule-diff-inline)
taught git diff to show the diff in the submodule as well, see
commits that are merged in 305d7f133956a5f43c94d938beabbfbb0ac1753c
or:
https://kernel.googlesource.com/pub/scm/git/git/+/fd47ae6a5b9cc0cfc56c1f7c43db612d26ca4b75%5E%21/#F1
Although this is just Git, you probably also have a code review system that
would need that change as well. Also it is not part of any release yet, but
soon will be.
Is there anything else besides these 3 points that encourages you to
switch away from submodules?
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCHv2 1/2] attr: convert to new threadsafe API
From: Junio C Hamano @ 2016-10-26 23:14 UTC (permalink / raw)
To: Stefan Beller; +Cc: pclouds, git, Johannes.Schindelin, j6t, peff, bmwill, simon
In-Reply-To: <20161026224104.31844-1-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> @@ -53,19 +57,32 @@ value of the attribute for the path.
> Querying Specific Attributes
> ----------------------------
>
> -* Prepare `struct git_attr_check` using git_attr_check_initl()
> +* Prepare a `struct git_attr_check` using `git_attr_check_initl()`
> function, enumerating the names of attributes whose values you are
> interested in, terminated with a NULL pointer. Alternatively, an
> - empty `struct git_attr_check` can be prepared by calling
> - `git_attr_check_alloc()` function and then attributes you want to
> - ask about can be added to it with `git_attr_check_append()`
> - function.
> -
> -* Call `git_check_attr()` to check the attributes for the path.
> -
> -* Inspect `git_attr_check` structure to see how each of the
> - attribute in the array is defined for the path.
> -
> + empty `struct git_attr_check` as allocated by git_attr_check_alloc()
> + can be prepared by calling `git_attr_check_alloc()` function and
> + then attributes you want to ask about can be added to it with
> + `git_attr_check_append()` function.
I think that my version that was discarded forbade appending once
you started to use the check for querying, because the check was
meant to be used as a key for an attr-stack and the check-specific
attr-stack was planned to keep only the attrs the check is
interested in (and appending is to change the set of attrs the check
is interested in, invalidating the attr-stack at that point).
If you lost that restriction, that is good (I didn't check the
implementation, though). Otherwise we'd need to say something here.
> + Both ways with `git_attr_check_initl()` as well as the
> + alloc and append route are thread safe, i.e. you can call it
> + from different threads at the same time; when check determines
> + the initialzisation is still needed, the threads will use a
initialization?
> + single global mutex to perform the initialization just once, the
> + others will wait on the the thread to actually perform the
> + initialization.
> +
> +* Allocate an array of `struct git_attr_result` either statically on the
> + as a variable on the stack or dynamically via `git_attr_result_alloc`
Grammo? "either on the stack, or dynamically in the heap"?
> + when the result size is not known at compile time. The call to initialize
> + the result is not thread safe, because different threads need their
> + own thread local result anyway.
Having result defined statically is not thread safe for that
reason. It is not clear what you mean by "The call to initialize
the result"; having it on the stack or have one dynamically on the
heap ought to be thread safe.
> +* Call `git_check_attr()` to check the attributes for the path,
> + the given `git_attr_result` will be filled with the result.
> +
> +* Inspect each `git_attr_result` structure to see how
> + each of the attribute in the array is defined for the path.
>
> Example
> -------
> @@ -76,28 +93,23 @@ To see how attributes "crlf" and "ident" are set for different paths.
> we are checking two attributes):
>
> ------------
> -static struct git_attr_check *check;
> -static void setup_check(void)
> -{
> - if (check)
> - return; /* already done */
> - check = git_attr_check_initl("crlf", "ident", NULL);
> -}
> + static struct git_attr_check *check;
> + git_attr_check_initl(check, "crlf", "ident", NULL);
I think you are still missing "&" here.
> ------------
>
> . Call `git_check_attr()` with the prepared `struct git_attr_check`:
>
> ------------
> const char *path;
> + struct git_attr_result result[2];
>
> - setup_check();
> - git_check_attr(path, check);
> + git_check_attr(path, check, result);
> ------------
>
> -. Act on `.value` member of the result, left in `check->check[]`:
> +. Act on `result.value[]`:
>
> ------------
> - const char *value = check->check[0].value;
> + const char *value = result.value[0];
>
> if (ATTR_TRUE(value)) {
> The attribute is Set, by listing only the name of the
> @@ -123,12 +135,15 @@ the first step in the above would be different.
> static struct git_attr_check *check;
> static void setup_check(const char **argv)
> {
> + if (check)
> + return; /* already done */
> check = git_attr_check_alloc();
You may want to say that this is thread-unsafe.
> while (*argv) {
> struct git_attr *attr = git_attr(*argv);
> git_attr_check_append(check, attr);
> argv++;
> }
> + struct git_attr_result *result = git_attr_result_alloc(check);
> }
> ------------
>
> @@ -138,17 +153,20 @@ Querying All Attributes
>
> To get the values of all attributes associated with a file:
>
> +* Setup a local variables for the question
> + `struct git_attr_check` as well as a pointer where the result
> + `struct git_attr_result` will be stored.
> +* Call `git_all_attrs()`.
Hmph, the caller does not know what attribute it is interested in,
and it is unclear "how" the former needs to be set up from this
description. Should it prepare an empty one that can be appended?
For the same reason, the size of the result is also unknown at the
callsite.
For these reasons, I thought git-all-attrs would need to use a
different calling convention from the usual "query for these
attributes -- get result for them" API. Even if the same attr_check
and attr_result structures are used, they need to be initialized in
very different way from the normal calls, and this part of the doc
needs to explain how. Perhaps it is sufficient to say "just declare
two pointer variables that point at these structures and a pair of
newly allocated structures of these types will be stored there" or
something.
It does not even have to use the usual "struct attr_check" "struct
attr_result" pair (I am not suggesting to use different convention
only to be different; I am merely pointing out that you have that
lattitude, as the usage pattern for all-attrs is so different from
the usual query).
^ permalink raw reply
* Re: [PATCH 28/36] attr: keep attr stack for each check
From: Stefan Beller @ 2016-10-26 23:10 UTC (permalink / raw)
To: Ramsay Jones
Cc: Junio C Hamano, git@vger.kernel.org, Brandon Williams, Duy Nguyen
In-Reply-To: <8a91d287-5413-3f8f-9d0e-edd67fb36557@ramsayjones.plus.com>
On Sun, Oct 23, 2016 at 8:10 AM, Ramsay Jones
<ramsay@ramsayjones.plus.com> wrote:
>> +
>> +struct hashmap all_attr_stacks;
>> +int all_attr_stacks_init;
>
> Mark symbols 'all_attr_stacks' and 'all_attr_stacks_init' with
> the static keyword. (ie. these are file-local symbols).
>
> ATB,
> Ramsay Jones
>
This fix will appear in a resend of the series.
Thanks,
Stefan
^ permalink raw reply
* "git subtree --squash" interacts poorly with revert, merge, and rebase
From: Matt McCutchen @ 2016-10-26 23:07 UTC (permalink / raw)
To: git
I'm the lead developer of a research software application (https://bitb
ucket.org/objsheets/objsheets) that uses modified versions of two
third-party libraries, which we need to version and distribute along
with our application. For better or for worse, we haven't made it a
priority to upstream our changes, so for now we just want to optimize
for ease of (1) making and reviewing changes and (2) upgrading to newer
upstream versions.
We've been using git submodules, but that's a pain for several reasons:
- We have to run "git submodule update" manually.
- We have to make separate commits and manage corresponding topic
branches for the superproject and subprojects.
- A diff of the superproject doesn't include the content of
subprojects.
Recently I looked into switching to the "git subtree" contrib tool in
the --squash mode, but I identified a few drawbacks compared to
submodules:
1. The upstream commit on which the subtree is based is assumed to be
given by the latest squash commit in "git log". This means that (i) a
change to a different upstream commit can't be reverted with "git
revert" and (ii) a "git merge" of two superproject branches based on
different upstream commits may successfully merge the content of the
upstream commits but leave the tool thinking the subtree is based on an
arbitrary one of the two commits.
2. Rebasing messes up the merge commits generated by "git subtree --
squash". --preserve-merges worked in a simple test but supposedly
doesn't work if there are conflicts or I want to reorder commits with
--interactive.
Maybe we would never hit any of these problems in practice, but they
give me a bad enough feeling that I'm planning to write my own tool
that tracks the upstream commit ID in a file (like a submodule) and
doesn't generate any extra commits. Without generating extra commits,
the only place to store the upstream content in the superproject would
be in another subtree, which would take up disk space in every working
tree unless developers manually set skip-worktree. I think I prefer to
not store the upstream content and just have the tool fetch it from a
local subproject repository each time it's needed.
I'll of course post the tool on the web and would be happy to see it
integrated into "git subtree" if that makes sense, but I don't know how
much time I'd be willing to put into making that happen.
Any advice?
Thanks,
Matt
^ permalink raw reply
* Re: What's cooking in git.git (Oct 2016, #07; Wed, 26)
From: Junio C Hamano @ 2016-10-26 22:47 UTC (permalink / raw)
To: Jonathan Tan; +Cc: git, Stefan Beller
In-Reply-To: <50f4e0b0-c044-be57-4e08-08d9970fe4a4@google.com>
Jonathan Tan <jonathantanmy@google.com> writes:
> On 10/26/2016 03:29 PM, Junio C Hamano wrote:
>> * jt/trailer-with-cruft (2016-10-21) 8 commits
>> - trailer: support values folded to multiple lines
>> - trailer: forbid leading whitespace in trailers
>> - trailer: allow non-trailers in trailer block
>> - trailer: clarify failure modes in parse_trailer
>> - trailer: make args have their own struct
>> - trailer: streamline trailer item create and add
>> - trailer: use list.h for doubly-linked list
>> - trailer: improve const correctness
>>
>> Update "interpret-trailers" machinery and teaches it that people in
>> real world write all sorts of crufts in the "trailer" that was
>> originally designed to have the neat-o "Mail-Header: like thing"
>> and nothing else.
>>
>> Waiting for review.
>
> For this patch set, should I look for another reviewer? In the e-mail
> thread [1], you and Stefan seemed positive about this patch set.
Ah, I meant "waiting for the review to conclude". If nobody has any
more comments, I am OK to start cooking it in 'next'.
Thanks.
^ permalink raw reply
* Re: What's cooking in git.git (Oct 2016, #07; Wed, 26)
From: Jonathan Tan @ 2016-10-26 22:42 UTC (permalink / raw)
To: Junio C Hamano, git; +Cc: Stefan Beller
In-Reply-To: <xmqq4m3ybv7h.fsf@gitster.mtv.corp.google.com>
On 10/26/2016 03:29 PM, Junio C Hamano wrote:
> * jt/trailer-with-cruft (2016-10-21) 8 commits
> - trailer: support values folded to multiple lines
> - trailer: forbid leading whitespace in trailers
> - trailer: allow non-trailers in trailer block
> - trailer: clarify failure modes in parse_trailer
> - trailer: make args have their own struct
> - trailer: streamline trailer item create and add
> - trailer: use list.h for doubly-linked list
> - trailer: improve const correctness
>
> Update "interpret-trailers" machinery and teaches it that people in
> real world write all sorts of crufts in the "trailer" that was
> originally designed to have the neat-o "Mail-Header: like thing"
> and nothing else.
>
> Waiting for review.
For this patch set, should I look for another reviewer? In the e-mail
thread [1], you and Stefan seemed positive about this patch set.
(I've also checked [2] that this is not merged yet.)
[1] <cover.1477072247.git.jonathantanmy@google.com>
[2] git log --grep='jt/trailer'
^ permalink raw reply
* [PATCHv2 1/2] attr: convert to new threadsafe API
From: Stefan Beller @ 2016-10-26 22:41 UTC (permalink / raw)
To: gitster, pclouds
Cc: git, Johannes.Schindelin, j6t, peff, bmwill, simon, Stefan Beller
In-Reply-To: <CAGZ79kYhMVrKHhNGYcf_D9kWEYp+sC+tMGbuE+gnD8AU27dh8g@mail.gmail.com>
This revamps the API of the attr subsystem to be thread safe.
Before we had the question and its results in one struct type.
The typical usage of the API was
static struct git_attr_check *check;
if (!check)
check = git_attr_check_initl("text", NULL);
git_check_attr(path, check);
act_on(check->value[0]);
This has a couple of issues when it comes to thread safety:
* the initialization is racy in this implementation. To make it
thread safe, we need to acquire a mutex, such that only one
thread is executing the code in git_attr_check_initl.
As we do not want to introduce a mutex at each call site,
this is best done in the attr code. However to do so, we need
to have access to the `check` variable, i.e. the API has to
look like
git_attr_check_initl(struct git_attr_check*, ...);
Then one of the threads calling git_attr_check_initl will
acquire the mutex and init the `check`, while all other threads
will wait on the mutex just to realize they're late to the
party and they'll return with no work done.
* While the check for attributes to be questioned only need to
be initalized once as that part will be read only after its
initialisation, the answer may be different for each path.
Because of that we need to decouple the check and the answer,
such that each thread can obtain an answer for the path it
is currently processing.
This commit changes the API and adds locking in
git_attr_check_initl that provides the thread safety for constructing
`struct git_attr_check`.
The usage of the new API will be:
/*
* The initl call will thread-safely check whether the
* struct git_attr_check has been initialized. We only
* want to do the initialization work once, hence we do
* that work inside a thread safe environment.
*/
static struct git_attr_check *check;
git_attr_check_initl(&check, "text", NULL);
/* We're just asking for one attribute "text". */
git_attr_result myresult[1];
/* Perform the check and act on it: */
git_check_attr(path, check, myresult);
act_on(myresult[0].value);
/*
* No need to free the check as it is static, hence doesn't leak
* memory. The result is also static, so no need to free there either.
*/
Signed-off-by: Stefan Beller <sbeller@google.com>
---
In the reroll of this series, I plan to use the patch as-is below:
* Documentation/commit message matches actual implementation
* initialize the attr lock statically. It was uninitialized in the first
version, which works for me on Linux, but not so in Windows; I split off
the static initialized mutexes on Windows as a separate patch, see
https://public-inbox.org/git/CAGZ79kZryb-5jGif04BtK1V9tgFj-tnahqUk+1Lb7XeecU7cMQ@mail.gmail.com
which would be a requirement for this patch.
Documentation/technical/api-gitattributes.txt | 94 ++++++++++-------
archive.c | 11 +-
attr.c | 143 ++++++++++++++++++--------
attr.h | 71 ++++++++-----
builtin/check-attr.c | 35 ++++---
builtin/pack-objects.c | 16 +--
convert.c | 40 +++----
ll-merge.c | 24 +++--
userdiff.c | 16 +--
ws.c | 8 +-
10 files changed, 280 insertions(+), 178 deletions(-)
diff --git a/Documentation/technical/api-gitattributes.txt b/Documentation/technical/api-gitattributes.txt
index 92fc32a..4d8ef93 100644
--- a/Documentation/technical/api-gitattributes.txt
+++ b/Documentation/technical/api-gitattributes.txt
@@ -16,15 +16,19 @@ Data Structure
of no interest to the calling programs. The name of the
attribute can be retrieved by calling `git_attr_name()`.
-`struct git_attr_check_elem`::
-
- This structure represents one attribute and its value.
-
`struct git_attr_check`::
- This structure represents a collection of `git_attr_check_elem`.
+ This structure represents a collection of `struct git_attrs`.
It is passed to `git_check_attr()` function, specifying the
- attributes to check, and receives their values.
+ attributes to check, and receives their values into a corresponding
+ `struct git_attr_result`.
+
+`struct git_attr_result`::
+
+ This structure represents one results for a check, such that an
+ array of `struct git_attr_results` corresponds to a
+ `struct git_attr_check`. The answers given in that array are in
+ the the same order as the check struct.
Attribute Values
@@ -32,7 +36,7 @@ Attribute Values
An attribute for a path can be in one of four states: Set, Unset,
Unspecified or set to a string, and `.value` member of `struct
-git_attr_check` records it. There are three macros to check these:
+git_attr_result` records it. There are three macros to check these:
`ATTR_TRUE()`::
@@ -53,19 +57,32 @@ value of the attribute for the path.
Querying Specific Attributes
----------------------------
-* Prepare `struct git_attr_check` using git_attr_check_initl()
+* Prepare a `struct git_attr_check` using `git_attr_check_initl()`
function, enumerating the names of attributes whose values you are
interested in, terminated with a NULL pointer. Alternatively, an
- empty `struct git_attr_check` can be prepared by calling
- `git_attr_check_alloc()` function and then attributes you want to
- ask about can be added to it with `git_attr_check_append()`
- function.
-
-* Call `git_check_attr()` to check the attributes for the path.
-
-* Inspect `git_attr_check` structure to see how each of the
- attribute in the array is defined for the path.
-
+ empty `struct git_attr_check` as allocated by git_attr_check_alloc()
+ can be prepared by calling `git_attr_check_alloc()` function and
+ then attributes you want to ask about can be added to it with
+ `git_attr_check_append()` function.
+ Both ways with `git_attr_check_initl()` as well as the
+ alloc and append route are thread safe, i.e. you can call it
+ from different threads at the same time; when check determines
+ the initialzisation is still needed, the threads will use a
+ single global mutex to perform the initialization just once, the
+ others will wait on the the thread to actually perform the
+ initialization.
+
+* Allocate an array of `struct git_attr_result` either statically on the
+ as a variable on the stack or dynamically via `git_attr_result_alloc`
+ when the result size is not known at compile time. The call to initialize
+ the result is not thread safe, because different threads need their
+ own thread local result anyway.
+
+* Call `git_check_attr()` to check the attributes for the path,
+ the given `git_attr_result` will be filled with the result.
+
+* Inspect each `git_attr_result` structure to see how
+ each of the attribute in the array is defined for the path.
Example
-------
@@ -76,28 +93,23 @@ To see how attributes "crlf" and "ident" are set for different paths.
we are checking two attributes):
------------
-static struct git_attr_check *check;
-static void setup_check(void)
-{
- if (check)
- return; /* already done */
- check = git_attr_check_initl("crlf", "ident", NULL);
-}
+ static struct git_attr_check *check;
+ git_attr_check_initl(check, "crlf", "ident", NULL);
------------
. Call `git_check_attr()` with the prepared `struct git_attr_check`:
------------
const char *path;
+ struct git_attr_result result[2];
- setup_check();
- git_check_attr(path, check);
+ git_check_attr(path, check, result);
------------
-. Act on `.value` member of the result, left in `check->check[]`:
+. Act on `result.value[]`:
------------
- const char *value = check->check[0].value;
+ const char *value = result.value[0];
if (ATTR_TRUE(value)) {
The attribute is Set, by listing only the name of the
@@ -123,12 +135,15 @@ the first step in the above would be different.
static struct git_attr_check *check;
static void setup_check(const char **argv)
{
+ if (check)
+ return; /* already done */
check = git_attr_check_alloc();
while (*argv) {
struct git_attr *attr = git_attr(*argv);
git_attr_check_append(check, attr);
argv++;
}
+ struct git_attr_result *result = git_attr_result_alloc(check);
}
------------
@@ -138,17 +153,20 @@ Querying All Attributes
To get the values of all attributes associated with a file:
-* Prepare an empty `git_attr_check` structure by calling
- `git_attr_check_alloc()`.
+* Setup a local variables for the question
+ `struct git_attr_check` as well as a pointer where the result
+ `struct git_attr_result` will be stored.
-* Call `git_all_attrs()`, which populates the `git_attr_check`
- with the attributes attached to the path.
+* Call `git_all_attrs()`.
-* Iterate over the `git_attr_check.check[]` array to examine
- the attribute names and values. The name of the attribute
- described by a `git_attr_check.check[]` object can be retrieved via
- `git_attr_name(check->check[i].attr)`. (Please note that no items
+* Iterate over the `git_attr_check.attr[]` array to examine the
+ attribute names. The name of the attribute described by a
+ `git_attr_check.attr[]` object can be retrieved via
+ `git_attr_name(check->attr[i])`. (Please note that no items
will be returned for unset attributes, so `ATTR_UNSET()` will return
false for all returned `git_array_check` objects.)
+ The respective value for an attribute can be found in the same
+ index position in of `git_attr_result`.
-* Free the `git_array_check` by calling `git_attr_check_free()`.
+* Clear the variables by calling `git_attr_check_clear()` and
+ `git_attr_result_free()`.
diff --git a/archive.c b/archive.c
index 11e3951..65bc6b7 100644
--- a/archive.c
+++ b/archive.c
@@ -111,6 +111,7 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,
struct archiver_args *args = c->args;
write_archive_entry_fn_t write_entry = c->write_entry;
static struct git_attr_check *check;
+ struct git_attr_result result[2];
const char *path_without_prefix;
int err;
@@ -124,12 +125,12 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,
strbuf_addch(&path, '/');
path_without_prefix = path.buf + args->baselen;
- if (!check)
- check = git_attr_check_initl("export-ignore", "export-subst", NULL);
- if (!git_check_attr(path_without_prefix, check)) {
- if (ATTR_TRUE(check->check[0].value))
+ git_attr_check_initl(&check, "export-ignore", "export-subst", NULL);
+
+ if (!git_check_attr(path_without_prefix, check, result)) {
+ if (ATTR_TRUE(result[0].value))
return 0;
- args->convert = ATTR_TRUE(check->check[1].value);
+ args->convert = ATTR_TRUE(result[1].value);
}
if (S_ISDIR(mode) || S_ISGITLINK(mode)) {
diff --git a/attr.c b/attr.c
index abf734e..7ee9078 100644
--- a/attr.c
+++ b/attr.c
@@ -14,6 +14,7 @@
#include "dir.h"
#include "utf8.h"
#include "quote.h"
+#include "thread-utils.h"
const char git_attr__true[] = "(builtin)true";
const char git_attr__false[] = "\0(builtin)false";
@@ -46,6 +47,19 @@ struct git_attr {
static int attr_nr;
static struct git_attr *(git_attr_hash[HASHSIZE]);
+#ifndef NO_PTHREADS
+
+static pthread_mutex_t attr_mutex = PTHREAD_MUTEX_INITIALIZER;
+#define attr_lock() pthread_mutex_lock(&attr_mutex)
+#define attr_unlock() pthread_mutex_unlock(&attr_mutex)
+
+#else
+
+#define attr_lock() (void)0
+#define attr_unlock() (void)0
+
+#endif /* NO_PTHREADS */
+
/*
* NEEDSWORK: maybe-real, maybe-macro are not property of
* an attribute, as it depends on what .gitattributes are
@@ -55,6 +69,16 @@ static struct git_attr *(git_attr_hash[HASHSIZE]);
*/
static int cannot_trust_maybe_real;
+/*
+ * Send one or more git_attr_check to git_check_attrs(), and
+ * each 'value' member tells what its value is.
+ * Unset one is returned as NULL.
+ */
+struct git_attr_check_elem {
+ const struct git_attr *attr;
+ const char *value;
+};
+
/* NEEDSWORK: This will become per git_attr_check */
static struct git_attr_check_elem *check_all_attr;
@@ -781,7 +805,7 @@ static int macroexpand_one(int nr, int rem)
static int attr_check_is_dynamic(const struct git_attr_check *check)
{
- return (void *)(check->check) != (void *)(check + 1);
+ return (void *)(check->attr) != (void *)(check + 1);
}
static void empty_attr_check_elems(struct git_attr_check *check)
@@ -799,7 +823,9 @@ static void empty_attr_check_elems(struct git_attr_check *check)
* any and all attributes that are visible are collected in it.
*/
static void collect_some_attrs(const char *path, int pathlen,
- struct git_attr_check *check, int collect_all)
+ struct git_attr_check *check,
+ struct git_attr_result **result,
+ int collect_all)
{
struct attr_stack *stk;
@@ -825,13 +851,11 @@ static void collect_some_attrs(const char *path, int pathlen,
check_all_attr[i].value = ATTR__UNKNOWN;
if (!collect_all && !cannot_trust_maybe_real) {
- struct git_attr_check_elem *celem = check->check;
-
rem = 0;
for (i = 0; i < check->check_nr; i++) {
- if (!celem[i].attr->maybe_real) {
+ if (!check->attr[i]->maybe_real) {
struct git_attr_check_elem *c;
- c = check_all_attr + celem[i].attr->attr_nr;
+ c = check_all_attr + check->attr[i]->attr_nr;
c->value = ATTR__UNSET;
rem++;
}
@@ -845,38 +869,52 @@ static void collect_some_attrs(const char *path, int pathlen,
rem = fill(path, pathlen, basename_offset, stk, rem);
if (collect_all) {
- empty_attr_check_elems(check);
+ int check_nr = 0, check_alloc = 0;
+ const char **res = NULL;
+
for (i = 0; i < attr_nr; i++) {
const struct git_attr *attr = check_all_attr[i].attr;
const char *value = check_all_attr[i].value;
if (value == ATTR__UNSET || value == ATTR__UNKNOWN)
continue;
- git_attr_check_append(check, attr)->value = value;
+
+ git_attr_check_append(check, attr);
+
+ ALLOC_GROW(res, check_nr + 1, check_alloc);
+ res[check_nr++] = value;
}
+
+ *result = git_attr_result_alloc(check);
+ for (i = 0; i < check->check_nr; i++)
+ (*result)[i].value = res[i];
+
+ free(res);
}
}
static int git_check_attrs(const char *path, int pathlen,
- struct git_attr_check *check)
+ struct git_attr_check *check,
+ struct git_attr_result *result)
{
int i;
- struct git_attr_check_elem *celem = check->check;
- collect_some_attrs(path, pathlen, check, 0);
+ collect_some_attrs(path, pathlen, check, &result, 0);
for (i = 0; i < check->check_nr; i++) {
- const char *value = check_all_attr[celem[i].attr->attr_nr].value;
+ const char *value = check_all_attr[check->attr[i]->attr_nr].value;
if (value == ATTR__UNKNOWN)
value = ATTR__UNSET;
- celem[i].value = value;
+ result[i].value = value;
}
return 0;
}
-void git_all_attrs(const char *path, struct git_attr_check *check)
+void git_all_attrs(const char *path,
+ struct git_attr_check *check,
+ struct git_attr_result **result)
{
- collect_some_attrs(path, strlen(path), check, 1);
+ collect_some_attrs(path, strlen(path), check, result, 1);
}
void git_attr_set_direction(enum git_attr_direction new, struct index_state *istate)
@@ -892,36 +930,40 @@ void git_attr_set_direction(enum git_attr_direction new, struct index_state *ist
use_index = istate;
}
-static int git_check_attr_counted(const char *path, int pathlen,
- struct git_attr_check *check)
+int git_check_attr(const char *path,
+ struct git_attr_check *check,
+ struct git_attr_result *result)
{
check->finalized = 1;
- return git_check_attrs(path, pathlen, check);
+ return git_check_attrs(path, strlen(path), check, result);
}
-int git_check_attr(const char *path, struct git_attr_check *check)
+void git_attr_check_initl(struct git_attr_check **check_,
+ const char *one, ...)
{
- return git_check_attr_counted(path, strlen(path), check);
-}
-
-struct git_attr_check *git_attr_check_initl(const char *one, ...)
-{
- struct git_attr_check *check;
int cnt;
va_list params;
const char *param;
+ struct git_attr_check *check;
+
+ attr_lock();
+ if (*check_) {
+ attr_unlock();
+ return;
+ }
va_start(params, one);
for (cnt = 1; (param = va_arg(params, const char *)) != NULL; cnt++)
;
va_end(params);
+
check = xcalloc(1,
- sizeof(*check) + cnt * sizeof(*(check->check)));
+ sizeof(*check) + cnt * sizeof(*(check->attr)));
check->check_nr = cnt;
check->finalized = 1;
- check->check = (struct git_attr_check_elem *)(check + 1);
+ check->attr = (const struct git_attr **)(check + 1);
- check->check[0].attr = git_attr(one);
+ check->attr[0] = git_attr(one);
va_start(params, one);
for (cnt = 1; cnt < check->check_nr; cnt++) {
struct git_attr *attr;
@@ -932,29 +974,44 @@ struct git_attr_check *git_attr_check_initl(const char *one, ...)
attr = git_attr(param);
if (!attr)
die("BUG: %s: not a valid attribute name", param);
- check->check[cnt].attr = attr;
+ check->attr[cnt] = attr;
}
va_end(params);
- return check;
+ *check_ = check;
+ attr_unlock();
+}
+
+void git_attr_check_alloc(struct git_attr_check **check)
+{
+ attr_lock();
+ if (!*check)
+ *check = xcalloc(1, sizeof(struct git_attr_check));
+
+ attr_unlock();
}
-struct git_attr_check *git_attr_check_alloc(void)
+struct git_attr_result *git_attr_result_alloc(struct git_attr_check *check)
{
- return xcalloc(1, sizeof(struct git_attr_check));
+ return xcalloc(1, sizeof(struct git_attr_result) * check->check_nr);
}
-struct git_attr_check_elem *git_attr_check_append(struct git_attr_check *check,
- const struct git_attr *attr)
+void git_attr_check_append(struct git_attr_check *check,
+ const struct git_attr *attr)
{
- struct git_attr_check_elem *elem;
+ int i;
if (check->finalized)
die("BUG: append after git_attr_check structure is finalized");
if (!attr_check_is_dynamic(check))
die("BUG: appending to a statically initialized git_attr_check");
- ALLOC_GROW(check->check, check->check_nr + 1, check->check_alloc);
- elem = &check->check[check->check_nr++];
- elem->attr = attr;
- return elem;
+ attr_lock();
+ for (i = 0; i < check->check_nr; i++)
+ if (check->attr[i] == attr)
+ break;
+ if (i == check->check_nr) {
+ ALLOC_GROW(check->attr, check->check_nr + 1, check->check_alloc);
+ check->attr[check->check_nr++] = attr;
+ }
+ attr_unlock();
}
void git_attr_check_clear(struct git_attr_check *check)
@@ -962,12 +1019,12 @@ void git_attr_check_clear(struct git_attr_check *check)
empty_attr_check_elems(check);
if (!attr_check_is_dynamic(check))
die("BUG: clearing a statically initialized git_attr_check");
- free(check->check);
+ free(check->attr);
check->check_alloc = 0;
}
-void git_attr_check_free(struct git_attr_check *check)
+void git_attr_result_free(struct git_attr_result *result)
{
- git_attr_check_clear(check);
- free(check);
+ /* No need to free values as they are interned. */
+ free(result);
}
diff --git a/attr.h b/attr.h
index bb40967..607d44c 100644
--- a/attr.h
+++ b/attr.h
@@ -9,10 +9,16 @@ struct git_attr;
* corresponds to it.
*/
extern struct git_attr *git_attr(const char *);
-
/* The same, but with counted string */
extern struct git_attr *git_attr_counted(const char *, size_t);
+/*
+ * Return the name of the attribute represented by the argument. The
+ * return value is a pointer to a null-delimited string that is part
+ * of the internal data structure; it should not be modified or freed.
+ */
+extern const char *git_attr_name(const struct git_attr *);
+
extern void invalid_attr_name_message(struct strbuf *, const char *, int);
/* Internal use */
@@ -24,44 +30,53 @@ extern const char git_attr__false[];
#define ATTR_FALSE(v) ((v) == git_attr__false)
#define ATTR_UNSET(v) ((v) == NULL)
-/*
- * Send one or more git_attr_check to git_check_attrs(), and
- * each 'value' member tells what its value is.
- * Unset one is returned as NULL.
- */
-struct git_attr_check_elem {
- const struct git_attr *attr;
- const char *value;
-};
-
struct git_attr_check {
int finalized;
int check_nr;
int check_alloc;
- struct git_attr_check_elem *check;
+ const struct git_attr **attr;
+};
+#define GIT_ATTR_CHECK_INIT {0, 0, 0, NULL}
+
+struct git_attr_result {
+ const char *value;
};
-extern struct git_attr_check *git_attr_check_initl(const char *, ...);
-extern int git_check_attr(const char *path, struct git_attr_check *);
+/*
+ * Initialize the `git_attr_check` via one of the following three functions:
+ *
+ * git_attr_check_alloc allocates an empty check,
+ * git_attr_check_append add an attribute to the given git_attr_check
+ *
+ * git_all_attrs allocates a check and fills in all attributes that
+ * are set for the given path.
+ * git_attr_check_initl takes a pointer to where the check will be initialized,
+ * followed by all attributes that are to be checked.
+ * This makes it potentially thread safe as it could
+ * internally have a mutex for that memory location.
+ * Currently it is not thread safe!
+ */
+extern void git_attr_check_alloc(struct git_attr_check **);
+extern struct git_attr_result *git_attr_result_alloc(struct git_attr_check *check);
-extern struct git_attr_check *git_attr_check_alloc(void);
-extern struct git_attr_check_elem *git_attr_check_append(struct git_attr_check *, const struct git_attr *);
+extern void git_attr_check_append(struct git_attr_check *,
+ const struct git_attr *);
+extern void git_attr_check_initl(struct git_attr_check **,
+ const char *, ...);
+
+extern void git_all_attrs(const char *path,
+ struct git_attr_check *,
+ struct git_attr_result **);
+
+/* Query a path for its attributes */
+extern int git_check_attr(const char *path,
+ struct git_attr_check *,
+ struct git_attr_result *result);
extern void git_attr_check_clear(struct git_attr_check *);
-extern void git_attr_check_free(struct git_attr_check *);
-/*
- * Return the name of the attribute represented by the argument. The
- * return value is a pointer to a null-delimited string that is part
- * of the internal data structure; it should not be modified or freed.
- */
-extern const char *git_attr_name(const struct git_attr *);
+extern void git_attr_result_free(struct git_attr_result *);
-/*
- * Retrieve all attributes that apply to the specified path.
- * check holds the attributes and their values.
- */
-void git_all_attrs(const char *path, struct git_attr_check *check);
enum git_attr_direction {
GIT_ATTR_CHECKIN,
diff --git a/builtin/check-attr.c b/builtin/check-attr.c
index ec61476..c7c6c22 100644
--- a/builtin/check-attr.c
+++ b/builtin/check-attr.c
@@ -24,13 +24,14 @@ static const struct option check_attr_options[] = {
OPT_END()
};
-static void output_attr(struct git_attr_check *check, const char *file)
+static void output_attr(struct git_attr_check *check,
+ struct git_attr_result *result, const char *file)
{
int j;
int cnt = check->check_nr;
for (j = 0; j < cnt; j++) {
- const char *value = check->check[j].value;
+ const char *value = result[j].value;
if (ATTR_TRUE(value))
value = "set";
@@ -44,11 +45,11 @@ static void output_attr(struct git_attr_check *check, const char *file)
"%s%c" /* attrname */
"%s%c" /* attrvalue */,
file, 0,
- git_attr_name(check->check[j].attr), 0, value, 0);
+ git_attr_name(check->attr[j]), 0, value, 0);
} else {
quote_c_style(file, NULL, stdout, 0);
printf(": %s: %s\n",
- git_attr_name(check->check[j].attr), value);
+ git_attr_name(check->attr[j]), value);
}
}
}
@@ -59,16 +60,20 @@ static void check_attr(const char *prefix,
{
char *full_path =
prefix_path(prefix, prefix ? strlen(prefix) : 0, file);
+ struct git_attr_check local_check = GIT_ATTR_CHECK_INIT;
+ struct git_attr_result *result = NULL;
+
if (check != NULL) {
- if (git_check_attr(full_path, check))
- die("git_check_attr died");
- output_attr(check, file);
+ result = git_attr_result_alloc(check);
+ git_check_attr(full_path, check, result);
} else {
- check = git_attr_check_alloc();
- git_all_attrs(full_path, check);
- output_attr(check, file);
- git_attr_check_free(check);
+ git_all_attrs(full_path, &local_check, &result);
+ check = &local_check;
}
+ output_attr(check, result, file);
+ git_attr_check_clear(&local_check);
+
+ git_attr_result_free(result);
free(full_path);
}
@@ -102,7 +107,7 @@ static NORETURN void error_with_usage(const char *msg)
int cmd_check_attr(int argc, const char **argv, const char *prefix)
{
- struct git_attr_check *check;
+ struct git_attr_check *check = NULL;
int cnt, i, doubledash, filei;
if (!is_bare_repository())
@@ -162,10 +167,8 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix)
error_with_usage("No file specified");
}
- if (all_attrs) {
- check = NULL;
- } else {
- check = git_attr_check_alloc();
+ if (!all_attrs) {
+ git_attr_check_alloc(&check);
for (i = 0; i < cnt; i++) {
struct git_attr *a = git_attr(argv[i]);
if (!a)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 3918c07..3751836 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -899,14 +899,16 @@ static void write_pack_file(void)
static int no_try_delta(const char *path)
{
static struct git_attr_check *check;
+ int ret = 0;
+ struct git_attr_result result[1];
- if (!check)
- check = git_attr_check_initl("delta", NULL);
- if (git_check_attr(path, check))
- return 0;
- if (ATTR_FALSE(check->check[0].value))
- return 1;
- return 0;
+ git_attr_check_initl(&check, "delta", NULL);
+
+ if (!git_check_attr(path, check, result)) {
+ if (ATTR_FALSE(result[0].value))
+ ret = 1;
+ }
+ return ret;
}
/*
diff --git a/convert.c b/convert.c
index bb2435a..fe861ce 100644
--- a/convert.c
+++ b/convert.c
@@ -718,9 +718,9 @@ static int ident_to_worktree(const char *path, const char *src, size_t len,
return 1;
}
-static enum crlf_action git_path_check_crlf(struct git_attr_check_elem *check)
+static enum crlf_action git_path_check_crlf(struct git_attr_result *result)
{
- const char *value = check->value;
+ const char *value = result->value;
if (ATTR_TRUE(value))
return CRLF_TEXT;
@@ -735,9 +735,9 @@ static enum crlf_action git_path_check_crlf(struct git_attr_check_elem *check)
return CRLF_UNDEFINED;
}
-static enum eol git_path_check_eol(struct git_attr_check_elem *check)
+static enum eol git_path_check_eol(struct git_attr_result *result)
{
- const char *value = check->value;
+ const char *value = result->value;
if (ATTR_UNSET(value))
;
@@ -748,9 +748,9 @@ static enum eol git_path_check_eol(struct git_attr_check_elem *check)
return EOL_UNSET;
}
-static struct convert_driver *git_path_check_convert(struct git_attr_check_elem *check)
+static struct convert_driver *git_path_check_convert(struct git_attr_result *result)
{
- const char *value = check->value;
+ const char *value = result->value;
struct convert_driver *drv;
if (ATTR_TRUE(value) || ATTR_FALSE(value) || ATTR_UNSET(value))
@@ -761,9 +761,9 @@ static struct convert_driver *git_path_check_convert(struct git_attr_check_elem
return NULL;
}
-static int git_path_check_ident(struct git_attr_check_elem *check)
+static int git_path_check_ident(struct git_attr_result *result)
{
- const char *value = check->value;
+ const char *value = result->value;
return !!ATTR_TRUE(value);
}
@@ -778,25 +778,27 @@ struct conv_attrs {
static void convert_attrs(struct conv_attrs *ca, const char *path)
{
static struct git_attr_check *check;
+ static int init_user_convert_tail;
+ struct git_attr_result result[5];
- if (!check) {
- check = git_attr_check_initl("crlf", "ident",
- "filter", "eol", "text",
- NULL);
+ git_attr_check_initl(&check, "crlf", "ident", "filter",
+ "eol", "text", NULL);
+
+ if (!init_user_convert_tail) {
user_convert_tail = &user_convert;
git_config(read_convert_config, NULL);
+ init_user_convert_tail = 1;
}
- if (!git_check_attr(path, check)) {
- struct git_attr_check_elem *ccheck = check->check;
- ca->crlf_action = git_path_check_crlf(ccheck + 4);
+ if (!git_check_attr(path, check, result)) {
+ ca->crlf_action = git_path_check_crlf(&result[4]);
if (ca->crlf_action == CRLF_UNDEFINED)
- ca->crlf_action = git_path_check_crlf(ccheck + 0);
+ ca->crlf_action = git_path_check_crlf(&result[0]);
ca->attr_action = ca->crlf_action;
- ca->ident = git_path_check_ident(ccheck + 1);
- ca->drv = git_path_check_convert(ccheck + 2);
+ ca->ident = git_path_check_ident(&result[1]);
+ ca->drv = git_path_check_convert(&result[2]);
if (ca->crlf_action != CRLF_BINARY) {
- enum eol eol_attr = git_path_check_eol(ccheck + 3);
+ enum eol eol_attr = git_path_check_eol(&result[3]);
if (ca->crlf_action == CRLF_AUTO && eol_attr == EOL_LF)
ca->crlf_action = CRLF_AUTO_INPUT;
else if (ca->crlf_action == CRLF_AUTO && eol_attr == EOL_CRLF)
diff --git a/ll-merge.c b/ll-merge.c
index bc6479c..d71354a 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -353,12 +353,14 @@ int ll_merge(mmbuffer_t *result_buf,
mmfile_t *theirs, const char *their_label,
const struct ll_merge_options *opts)
{
- static struct git_attr_check *check;
static const struct ll_merge_options default_opts;
const char *ll_driver_name = NULL;
int marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
const struct ll_merge_driver *driver;
+ static struct git_attr_check *check;
+ struct git_attr_result result[2];
+
if (!opts)
opts = &default_opts;
@@ -368,13 +370,12 @@ int ll_merge(mmbuffer_t *result_buf,
normalize_file(theirs, path);
}
- if (!check)
- check = git_attr_check_initl("merge", "conflict-marker-size", NULL);
+ git_attr_check_initl(&check, "merge", "conflict-marker-size", NULL);
- if (!git_check_attr(path, check)) {
- ll_driver_name = check->check[0].value;
- if (check->check[1].value) {
- marker_size = atoi(check->check[1].value);
+ if (!git_check_attr(path, check, result)) {
+ ll_driver_name = result[0].value;
+ if (result[1].value) {
+ marker_size = atoi(result[1].value);
if (marker_size <= 0)
marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
}
@@ -395,11 +396,12 @@ int ll_merge_marker_size(const char *path)
{
static struct git_attr_check *check;
int marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
+ struct git_attr_result result[1];
+
+ git_attr_check_initl(&check, "conflict-marker-size", NULL);
- if (!check)
- check = git_attr_check_initl("conflict-marker-size", NULL);
- if (!git_check_attr(path, check) && check->check[0].value) {
- marker_size = atoi(check->check[0].value);
+ if (!git_check_attr(path, check, result) && !ATTR_UNSET(result[0].value)) {
+ marker_size = atoi(result[0].value);
if (marker_size <= 0)
marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
}
diff --git a/userdiff.c b/userdiff.c
index 46dfd32..1d6d363 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -263,21 +263,23 @@ struct userdiff_driver *userdiff_find_by_name(const char *name) {
struct userdiff_driver *userdiff_find_by_path(const char *path)
{
static struct git_attr_check *check;
+ struct git_attr_result result[1];
- if (!check)
- check = git_attr_check_initl("diff", NULL);
if (!path)
return NULL;
- if (git_check_attr(path, check))
+
+ git_attr_check_initl(&check, "diff", NULL);
+
+ if (git_check_attr(path, check, result))
return NULL;
- if (ATTR_TRUE(check->check[0].value))
+ if (ATTR_TRUE(result[0].value))
return &driver_true;
- if (ATTR_FALSE(check->check[0].value))
+ if (ATTR_FALSE(result[0].value))
return &driver_false;
- if (ATTR_UNSET(check->check[0].value))
+ if (ATTR_UNSET(result[0].value))
return NULL;
- return userdiff_find_by_name(check->check[0].value);
+ return userdiff_find_by_name(result[0].value);
}
struct userdiff_driver *userdiff_get_textconv(struct userdiff_driver *driver)
diff --git a/ws.c b/ws.c
index bb3270c..7e355c4 100644
--- a/ws.c
+++ b/ws.c
@@ -74,14 +74,14 @@ unsigned parse_whitespace_rule(const char *string)
unsigned whitespace_rule(const char *pathname)
{
static struct git_attr_check *attr_whitespace_rule;
+ struct git_attr_result result[1];
- if (!attr_whitespace_rule)
- attr_whitespace_rule = git_attr_check_initl("whitespace", NULL);
+ git_attr_check_initl(&attr_whitespace_rule, "whitespace", NULL);
- if (!git_check_attr(pathname, attr_whitespace_rule)) {
+ if (!git_check_attr(pathname, attr_whitespace_rule, result)) {
const char *value;
- value = attr_whitespace_rule->check[0].value;
+ value = result[0].value;
if (ATTR_TRUE(value)) {
/* true (whitespace) */
unsigned all_rule = ws_tab_width(whitespace_rule_cfg);
--
2.10.1.508.g6572022
^ permalink raw reply related
* What's cooking in git.git (Oct 2016, #07; Wed, 26)
From: Junio C Hamano @ 2016-10-26 22:29 UTC (permalink / raw)
To: git
Here are the topics that have been cooking. Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'. The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.
You can find the changes described here in the integration branches
of the repositories listed at
http://git-blame.blogspot.com/p/git-public-repositories.html
--------------------------------------------------
[Graduated to "master"]
* ab/gitweb-abbrev-links (2016-10-14) 3 commits
(merged to 'next' on 2016-10-17 at 4868def05e)
+ gitweb: link to "git describe"'d commits in log messages
+ gitweb: link to 7-char+ SHA-1s, not only 8-char+
+ gitweb: fix a typo in a comment
In addition to purely abbreviated commit object names, "gitweb"
learned to turn "git describe" output (e.g. v2.9.3-599-g2376d31787)
into clickable links in its output.
* bw/ls-files-recurse-submodules (2016-10-10) 4 commits
(merged to 'next' on 2016-10-17 at f0e398946a)
+ ls-files: add pathspec matching for submodules
+ ls-files: pass through safe options for --recurse-submodules
+ ls-files: optionally recurse into submodules
+ git: make super-prefix option
"git ls-files" learned "--recurse-submodules" option that can be
used to get a listing of tracked files across submodules (i.e. this
only works with "--cached" option, not for listing untracked or
ignored files). This would be a useful tool to sit on the upstream
side of a pipe that is read with xargs to work on all working tree
files from the top-level superproject.
* bw/submodule-branch-dot-doc (2016-10-19) 1 commit
(merged to 'next' on 2016-10-21 at 18aad25ba8)
+ submodules doc: update documentation for "." used for submodule branches
Recent git allows submodule.<name>.branch to use a special token
"." instead of the branch name; the documentation has been updated
to describe it.
* dk/worktree-dup-checkout-with-bare-is-ok (2016-10-14) 1 commit
(merged to 'next' on 2016-10-17 at 24594d3e56)
+ worktree: allow the main brach of a bare repository to be checked out
In a worktree connected to a repository elsewhere, created via "git
worktree", "git checkout" attempts to protect users from confusion
by refusing to check out a branch that is already checked out in
another worktree. However, this also prevented checking out a
branch, which is designated as the primary branch of a bare
reopsitory, in a worktree that is connected to the bare
repository. The check has been corrected to allow it.
* ex/deprecate-empty-pathspec-as-match-all (2016-06-22) 1 commit
(merged to 'next' on 2016-09-21 at e19148ea63)
+ pathspec: warn on empty strings as pathspec
Originally merged to 'next' on 2016-07-13
An empty string used as a pathspec element has always meant
'everything matches', but it is too easy to write a script that
finds a path to remove in $path and run 'git rm "$paht"', which
ends up removing everything. Start warning about this use of an
empty string used for 'everything matches' and ask users to use a
more explicit '.' for that instead.
The hope is that existing users will not mind this change, and
eventually the warning can be turned into a hard error, upgrading
the deprecation into removal of this (mis)feature.
* jc/cocci-xstrdup-or-null (2016-10-12) 1 commit
(merged to 'next' on 2016-10-17 at 55ceaa465a)
+ cocci: refactor common patterns to use xstrdup_or_null()
Code cleanup.
* jc/diff-unique-abbrev-comments (2016-09-30) 1 commit
(merged to 'next' on 2016-10-17 at c7fb286102)
+ diff_unique_abbrev(): document its assumption and limitation
(this branch is used by jk/no-looking-at-dotgit-outside-repo.)
A bit more comments in a tricky code.
* jc/ws-error-highlight (2016-10-04) 4 commits
(merged to 'next' on 2016-10-17 at ecbdc57d77)
+ diff: introduce diff.wsErrorHighlight option
+ diff.c: move ws-error-highlight parsing helpers up
+ diff.c: refactor parse_ws_error_highlight()
+ t4015: split out the "setup" part of ws-error-highlight test
"git diff/log --ws-error-highlight=<kind>" lacked the corresponding
configuration variable to set it by default.
* jk/ambiguous-short-object-names (2016-10-12) 1 commit
(merged to 'next' on 2016-10-19 at e7c55a9da5)
+ t1512: become resilient to GETTEXT_POISON build
A test fixup to recently graduated topic.
* jk/diff-submodule-diff-inline (2016-10-20) 1 commit
(merged to 'next' on 2016-10-21 at 13f300805e)
+ rev-list: use hdr_termination instead of a always using a newline
A recently graduated topic regressed "git rev-list --header"
output, breaking "gitweb". This has been fixed.
* jk/fetch-quick-tag-following (2016-10-14) 1 commit
(merged to 'next' on 2016-10-19 at d7718dcdf5)
+ fetch: use "quick" has_sha1_file for tag following
When fetching from a remote that has many tags that are irrelevant
to branches we are following, we used to waste way too many cycles
when checking if the object pointed at by a tag (that we are not
going to fetch!) exists in our repository too carefully.
* jk/merge-base-fork-point-without-reflog (2016-10-12) 1 commit
(merged to 'next' on 2016-10-19 at 00a6797f62)
+ merge-base: handle --fork-point without reflog
"git rebase" immediately after "git clone" failed to find the fork
point from the upstream.
* jk/tap-verbose-fix (2016-10-24) 4 commits
(merged to 'next' on 2016-10-24 at 5073a4de2d)
+ test-lib: bail out when "-v" used under "prove"
(merged to 'next' on 2016-10-21 at 592679411c)
+ travis: use --verbose-log test option
+ test-lib: add --verbose-log option
+ test-lib: handle TEST_OUTPUT_DIRECTORY with spaces
The Travis CI configuration we ship ran the tests with --verbose
option but this risks non-TAP output that happens to be "ok" to be
misinterpreted as TAP signalling a test that passed. This resulted
in unnecessary failure. This has been corrected by introducing a
new mode to run our tests in the test harness to send the verbose
output separately to the log file.
* jk/tighten-alloc (2016-10-17) 2 commits
(merged to 'next' on 2016-10-19 at 548522a520)
+ inline xalloc_flex() into FLEXPTR_ALLOC_MEM
+ avoid pointer arithmetic involving NULL in FLEX_ALLOC_MEM
Protect our code from over-eager compilers.
* jk/upload-pack-use-prio-queue (2016-10-11) 1 commit
(merged to 'next' on 2016-10-19 at 1d6efb07ac)
+ upload-pack: use priority queue in reachable() check
"git upload-pack" had its code cleaned-up and performance improved
by reducing use of timestamp-ordered commit-list, which was
replaced with a priority queue.
* js/libify-require-clean-work-tree (2016-10-07) 6 commits
(merged to 'next' on 2016-10-17 at f5c20df38b)
+ wt-status: begin error messages with lower-case
+ wt-status: teach has_{unstaged,uncommitted}_changes() about submodules
+ wt-status: export also the has_un{staged,committed}_changes() functions
+ wt-status: make the require_clean_work_tree() function reusable
+ pull: make code more similar to the shell script again
+ pull: drop confusing prefix parameter of die_on_unclean_work_tree()
The require_clean_work_tree() helper was recreated in C when "git
pull" was rewritten from shell; the helper is now made available to
other callers in preparation for upcoming "rebase -i" work.
* mg/gpg-richer-status (2016-10-12) 1 commit
(merged to 'next' on 2016-10-17 at 8843a6a8be)
+ gpg-interface: use more status letters
The GPG verification status shown in "%G?" pretty format specifier
was not rich enough to differentiate a signature made by an expired
key, a signature made by a revoked key, etc. New output letters
have been assigned to express them.
* mm/credential-libsecret (2016-10-11) 1 commit
(merged to 'next' on 2016-10-17 at 1b4af03ba4)
+ contrib: add credential helper for libsecret
A new credential helper that talks via "libsecret" with
implementations of XDG Secret Service API has been added to
contrib/credential/.
* mm/send-email-cc-cruft-after-address (2016-10-21) 3 commits
(merged to 'next' on 2016-10-21 at c7ec2b5025)
+ Git.pm: add comment pointing to t9000
+ t9000-addresses: update expected results after fix
(merged to 'next' on 2016-10-19 at 41e3f876cd)
+ parse_mailboxes: accept extra text after <...> address
"git send-email" attempts to pick up valid e-mails from the
trailers, but people in real world write non-addresses there, like
"Cc: Stable <add@re.ss> # 4.8+", which broke the output depending
on the availability and vintage of Mail::Address perl module.
* pb/test-parse-options-expect (2016-10-17) 1 commit
(merged to 'next' on 2016-10-19 at d3517d592f)
+ t0040: convert all possible tests to use `test-parse-options --expect`
Test clean-up.
* po/fix-doc-merge-base-illustration (2016-10-24) 2 commits
(merged to 'next' on 2016-10-24 at 6539e97fcf)
+ doc: fix the 'revert a faulty merge' ASCII art tab spacing
(merged to 'next' on 2016-10-21 at ac6f04a6c5)
+ doc: fix merge-base ASCII art tab spacing
Some AsciiDoc formatter mishandles a displayed illustration with
tabs in it. Adjust a few of them in merge-base documentation to
work around them.
* pt/gitgui-updates (2016-10-20) 35 commits
(merged to 'next' on 2016-10-21 at 4c8214095a)
+ Merge tag 'gitgui-0.21.0' of git://repo.or.cz/git-gui
+ git-gui: set version 0.21
+ Merge branch 'as/bulgarian' into pu
+ git-gui: Mark 'All' in remote.tcl for translation
+ git-gui i18n: Updated Bulgarian translation (565,0f,0u)
+ Merge branch 'os/preserve-author' into pu
+ git-gui: avoid persisting modified author identity
+ git-gui: Do not reset author details on amend
+ Merge branch 'kb/unicode' into pu
+ git-gui: handle the encoding of Git's output correctly
+ git-gui: unicode file name support on windows
+ Merge branch 'dr/ru' into pu
+ git-gui: Update Russian translation
+ git-gui: maintain backwards compatibility for merge syntax
+ Merge branch 'va/i18n_2' into pu
+ git-gui i18n: mark string in lib/error.tcl for translation
+ git-gui: fix incorrect use of Tcl append command
+ git-gui i18n: mark "usage:" strings for translation
+ git-gui i18n: internationalize use of colon punctuation
+ Merge branch 'pt/non-mouse-usage' into pu
+ Amend tab ordering and text widget border and highlighting.
+ Allow keyboard control to work in the staging widgets.
+ Merge branch 'pt/git4win-mods' into pu
+ git-gui (Windows): use git-gui.exe in `Create Desktop Shortcut`
+ git-gui: fix detection of Cygwin
+ Merge branch 'patches' into pu
+ git-gui: ensure the file in the diff pane is in the list of selected files
+ git-gui: support for $FILENAMES in tool definitions
+ git-gui: fix initial git gui message encoding
+ git-gui/po/glossary/txt-to-pot.sh: use the $( ... ) construct for command substitution
+ Merge branch 'va/i18n' into pu
+ Merge branch 'rs/use-modern-git-merge-syntax' into pu
+ Merge branch 'js/commit-gpgsign' into pu
+ Merge branch 'sy/i18n' into pu
+ git-gui: sort entries in tclIndex
A new version of git-gui, now at its 0.21.0 tag.
* tg/add-chmod+x-fix (2016-10-20) 1 commit
(merged to 'next' on 2016-10-21 at 1585ac7139)
+ t3700: fix broken test under !SANITY
A hot-fix for a test added by a recent topic that went to both
'master' and 'maint' already.
* va/i18n (2016-10-17) 7 commits
(merged to 'next' on 2016-10-19 at b7d733698b)
+ i18n: diff: mark warnings for translation
+ i18n: credential-cache--daemon: mark advice for translation
+ i18n: convert mark error messages for translation
+ i18n: apply: mark error message for translation
+ i18n: apply: mark error messages for translation
+ i18n: apply: mark info messages for translation
+ i18n: apply: mark plural string for translation
More i18n.
* yk/git-tag-remove-mention-of-old-layout-in-doc (2016-10-20) 1 commit
(merged to 'next' on 2016-10-21 at 8d9e23b023)
+ doc: remove reference to the traditional layout in git-tag.txt
Shorten description of auto-following in "git tag" by removing a
mention of historical remotes layout which is not relevant to the
main topic.
--------------------------------------------------
[New Topics]
* aw/numbered-stash (2016-10-26) 1 commit
(merged to 'next' on 2016-10-26 at 8d9325fa3a)
+ stash: allow stashes to be referenced by index only
The user always has to say "stash@{$N}" when naming a single
element in the default location of the stash, i.e. reflogs in
refs/stash. The "git stash" command learned to accept "git stash
apply 4" as a short-hand for "git stash apply stash@{4}".
Will merge to 'master'.
* jk/no-looking-at-dotgit-outside-repo-final (2016-10-26) 1 commit
(merged to 'next' on 2016-10-26 at 220e160451)
+ setup_git_env: avoid blind fall-back to ".git"
(this branch uses jk/no-looking-at-dotgit-outside-repo.)
This is the endgame of the topic to avoid blindly falling back to
".git" when the setup sequence said we are _not_ in Git repository.
A corner case that happens to work right now may be broken by a
call to die("BUG").
Will cook in 'next'.
* ew/svn-wt (2016-10-14) 2 commits
- git-svn: "git worktree" awareness
- git-svn: reduce scope of input record separator change
Will replace with a direct pulling from Eric to 'master'.
--------------------------------------------------
[Stalled]
* hv/submodule-not-yet-pushed-fix (2016-10-10) 3 commits
- batch check whether submodule needs pushing into one call
- serialize collection of refs that contain submodule changes
- serialize collection of changed submodules
The code in "git push" to compute if any commit being pushed in the
superproject binds a commit in a submodule that hasn't been pushed
out was overly inefficient, making it unusable even for a small
project that does not have any submodule but have a reasonable
number of refs.
Waiting for review.
cf. <cover.1475851621.git.hvoigt@hvoigt.net>
* sb/push-make-submodule-check-the-default (2016-10-10) 2 commits
- push: change submodule default to check when submodules exist
- submodule add: extend force flag to add existing repos
Turn the default of "push.recurseSubmodules" to "check" when
submodules seem to be in use.
Will hold to wait for hv/submodule-not-yet-pushed-fix
* jc/bundle (2016-03-03) 6 commits
- index-pack: --clone-bundle option
- Merge branch 'jc/index-pack' into jc/bundle
- bundle v3: the beginning
- bundle: keep a copy of bundle file name in the in-core bundle header
- bundle: plug resource leak
- bundle doc: 'verify' is not about verifying the bundle
The beginning of "split bundle", which could be one of the
ingredients to allow "git clone" traffic off of the core server
network to CDN.
While I think it would make it easier for people to experiment and
build on if the topic is merged to 'next', I am at the same time a
bit reluctant to merge an unproven new topic that introduces a new
file format, which we may end up having to support til the end of
time. It is likely that to support a "prime clone from CDN", it
would need a lot more than just "these are the heads and the pack
data is over there", so this may not be sufficient.
Will discard.
* mh/connect (2016-06-06) 10 commits
- connect: [host:port] is legacy for ssh
- connect: move ssh command line preparation to a separate function
- connect: actively reject git:// urls with a user part
- connect: change the --diag-url output to separate user and host
- connect: make parse_connect_url() return the user part of the url as a separate value
- connect: group CONNECT_DIAG_URL handling code
- connect: make parse_connect_url() return separated host and port
- connect: re-derive a host:port string from the separate host and port variables
- connect: call get_host_and_port() earlier
- connect: document why we sometimes call get_port after get_host_and_port
Rewrite Git-URL parsing routine (hopefully) without changing any
behaviour.
It has been two months without any support. We may want to discard
this.
* kn/ref-filter-branch-list (2016-05-17) 17 commits
- branch: implement '--format' option
- branch: use ref-filter printing APIs
- branch, tag: use porcelain output
- ref-filter: allow porcelain to translate messages in the output
- ref-filter: add `:dir` and `:base` options for ref printing atoms
- ref-filter: make remote_ref_atom_parser() use refname_atom_parser_internal()
- ref-filter: introduce symref_atom_parser() and refname_atom_parser()
- ref-filter: introduce refname_atom_parser_internal()
- ref-filter: make "%(symref)" atom work with the ':short' modifier
- ref-filter: add support for %(upstream:track,nobracket)
- ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreams
- ref-filter: introduce format_ref_array_item()
- ref-filter: move get_head_description() from branch.c
- ref-filter: modify "%(objectname:short)" to take length
- ref-filter: implement %(if:equals=<string>) and %(if:notequals=<string>)
- ref-filter: include reference to 'used_atom' within 'atom_value'
- ref-filter: implement %(if), %(then), and %(else) atoms
The code to list branches in "git branch" has been consolidated
with the more generic ref-filter API.
Rerolled.
Needs review.
* ec/annotate-deleted (2015-11-20) 1 commit
- annotate: skip checking working tree if a revision is provided
Usability fix for annotate-specific "<file> <rev>" syntax with deleted
files.
Has been waiting for a review for too long without seeing anything.
Will discard.
* dk/gc-more-wo-pack (2016-01-13) 4 commits
- gc: clean garbage .bitmap files from pack dir
- t5304: ensure non-garbage files are not deleted
- t5304: test .bitmap garbage files
- prepare_packed_git(): find more garbage
Follow-on to dk/gc-idx-wo-pack topic, to clean up stale
.bitmap and .keep files.
Has been waiting for a reroll for too long.
cf. <xmqq60ypbeng.fsf@gitster.mtv.corp.google.com>
Will discard.
* jc/diff-b-m (2015-02-23) 5 commits
. WIPWIP
. WIP: diff-b-m
- diffcore-rename: allow easier debugging
- diffcore-rename.c: add locate_rename_src()
- diffcore-break: allow debugging
"git diff -B -M" produced incorrect patch when the postimage of a
completely rewritten file is similar to the preimage of a removed
file; such a resulting file must not be expressed as a rename from
other place.
The fix in this patch is broken, unfortunately.
Will discard.
--------------------------------------------------
[Cooking]
* jc/abbrev-auto (2016-10-22) 4 commits
(merged to 'next' on 2016-10-26 at 92fdb66807)
+ transport: compute summary-width dynamically
+ transport: allow summary-width to be computed dynamically
+ fetch: pass summary_width down the callchain
+ transport: pass summary_width down the callchain
(this branch uses jk/abbrev-auto and lt/abbrev-auto.)
"git push" and "git fetch" reports from what old object to what new
object each ref was updated, using abbreviated refnames, and they
attempt to align the columns for this and other pieces of
information. The way these codepaths compute how many display
columns to allocate for the object names portion of this output has
been updated to match the recent "auto scale the default
abbreviation length" change.
Will merge to 'master'.
* jc/reset-unmerge (2016-10-24) 1 commit
- reset: --unmerge
After "git add" is run prematurely during a conflict resolution,
"git diff" can no longer be used as a way to sanity check by
looking at the combined diff. "git reset" learned a new
"--unmerge" option to recover from this situation.
* jk/daemon-path-ok-check-truncation (2016-10-24) 1 commit
(merged to 'next' on 2016-10-26 at 70c08241f6)
+ daemon: detect and reject too-long paths
"git daemon" used fixed-length buffers to turn URL to the
repository the client asked for into the server side directory
path, using snprintf() to avoid overflowing these buffers, but
allowed possibly truncated paths to the directory. This has been
tightened to reject such a request that causes overlong path to be
required to serve.
Will merge to 'master'.
* ls/git-open-cloexec (2016-10-25) 3 commits
(merged to 'next' on 2016-10-26 at f7259cbddb)
+ read-cache: make sure file handles are not inherited by child processes
+ sha1_file: open window into packfiles with O_CLOEXEC
+ sha1_file: rename git_open_noatime() to git_open()
Git generally does not explicitly close file descriptors that were
open in the parent process when spawning a child process, but most
of the time the child does not want to access them. As Windows does
not allow removing or renaming a file that has a file descriptor
open, a slow-to-exit child can even break the parent process by
holding onto them. Use O_CLOEXEC flag to open files in various
codepaths.
Will merge to 'master'.
* rs/ring-buffer-wraparound (2016-10-26) 1 commit
(merged to 'next' on 2016-10-26 at d2da68a14a)
+ hex: make wraparound of the index into ring-buffer explicit
The code that we have used for the past 10+ years to cycle
4-element ring buffers turns out to be not quite portable in
theoretical world.
Will merge to 'master'.
* jc/merge-base-fp-only (2016-10-19) 8 commits
. merge-base: fp experiment
- merge: allow to use only the fp-only merge bases
- merge-base: limit the output to bases that are on first-parent chain
- merge-base: mark bases that are on first-parent chain
- merge-base: expose get_merge_bases_many_0() a bit more
- merge-base: stop moving commits around in remove_redundant()
- sha1_name: remove ONELINE_SEEN bit
- commit: simplify fastpath of merge-base
An experiment of merge-base that ignores common ancestors that are
not on the first parent chain.
* jk/no-looking-at-dotgit-outside-repo (2016-10-26) 6 commits
(merged to 'next' on 2016-10-26 at 4aa877b578)
+ diff: handle sha1 abbreviations outside of repository
+ diff_aligned_abbrev: use "struct oid"
+ diff_unique_abbrev: rename to diff_aligned_abbrev
+ find_unique_abbrev: use 4-buffer ring
+ test-*-cache-tree: setup git dir
+ read info/{attributes,exclude} only when in repository
(this branch is used by jk/no-looking-at-dotgit-outside-repo-final.)
Update "git diff --no-index" codepath not to try to peek into .git/
directory that happens to be under the current directory, when we
know we are operating outside any repository.
Will merge to 'master'.
* tb/convert-stream-check (2016-10-12) 2 commits
- convert.c: stream and fast search for binary
- read-cache: factor out get_sha1_from_index() helper
End-of-line conversion sometimes needs to see if the current blob
in the index has NULs and CRs to base its decision. We used to
always get a full statistics over the blob, but in many cases we
can return early when we have seen "enough" (e.g. if we see a
single NUL, the blob will be handled as binary). The codepaths
have been optimized by using streaming interface.
Waiting for review.
* jt/trailer-with-cruft (2016-10-21) 8 commits
- trailer: support values folded to multiple lines
- trailer: forbid leading whitespace in trailers
- trailer: allow non-trailers in trailer block
- trailer: clarify failure modes in parse_trailer
- trailer: make args have their own struct
- trailer: streamline trailer item create and add
- trailer: use list.h for doubly-linked list
- trailer: improve const correctness
Update "interpret-trailers" machinery and teaches it that people in
real world write all sorts of crufts in the "trailer" that was
originally designed to have the neat-o "Mail-Header: like thing"
and nothing else.
Waiting for review.
* pb/bisect (2016-10-18) 27 commits
- bisect--helper: remove the dequote in bisect_start()
- bisect--helper: retire `--bisect-auto-next` subcommand
- bisect--helper: retire `--bisect-autostart` subcommand
- bisect--helper: retire `--bisect-write` subcommand
- bisect--helper: `bisect_replay` shell function in C
- bisect--helper: `bisect_log` shell function in C
- bisect--helper: retire `--write-terms` subcommand
- bisect--helper: retire `--check-expected-revs` subcommand
- bisect--helper: `bisect_state` & `bisect_head` shell function in C
- bisect--helper: `bisect_autostart` shell function in C
- bisect--helper: retire `--next-all` subcommand
- bisect--helper: retire `--bisect-clean-state` subcommand
- bisect--helper: `bisect_next` and `bisect_auto_next` shell function in C
- t6030: no cleanup with bad merge base
- bisect--helper: `bisect_start` shell function partially in C
- bisect--helper: `get_terms` & `bisect_terms` shell function in C
- bisect--helper: `bisect_next_check` & bisect_voc shell function in C
- bisect--helper: `check_and_set_terms` shell function in C
- bisect--helper: `bisect_write` shell function in C
- bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C
- bisect--helper: `bisect_reset` shell function in C
- wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()
- t6030: explicitly test for bisection cleanup
- bisect--helper: `bisect_clean_state` shell function in C
- bisect--helper: `write_terms` shell function in C
- bisect: rewrite `check_term_format` shell function in C
- bisect--helper: use OPT_CMDMODE instead of OPT_BOOL
Move more parts of "git bisect" to C.
Waiting for review.
* js/prepare-sequencer (2016-10-21) 27 commits
(merged to 'next' on 2016-10-26 at 12be8ebe90)
+ sequencer: mark all error messages for translation
+ sequencer: start error messages consistently with lower case
+ sequencer: quote filenames in error messages
+ sequencer: mark action_name() for translation
+ sequencer: remove overzealous assumption in rebase -i mode
+ sequencer: teach write_message() to append an optional LF
+ sequencer: refactor write_message() to take a pointer/length
+ sequencer: roll back lock file if write_message() failed
+ sequencer: stop releasing the strbuf in write_message()
+ sequencer: left-trim lines read from the script
+ sequencer: support cleaning up commit messages
+ sequencer: support amending commits
+ sequencer: allow editing the commit message on a case-by-case basis
+ sequencer: introduce a helper to read files written by scripts
+ sequencer: prepare for rebase -i's commit functionality
+ sequencer: remember the onelines when parsing the todo file
+ sequencer: get rid of the subcommand field
+ sequencer: avoid completely different messages for different actions
+ sequencer: strip CR from the todo script
+ sequencer: completely revamp the "todo" script parsing
+ sequencer: refactor the code to obtain a short commit name
+ sequencer: future-proof read_populate_todo()
+ sequencer: plug memory leaks for the option values
+ sequencer: future-proof remove_sequencer_state()
+ sequencer: avoid unnecessary indirection
+ sequencer: use memoized sequencer directory path
+ sequencer: use static initializers for replay_opts
Update of the sequencer codebase to make it reusable to reimplement
"rebase -i" continues.
Will merge to 'master'.
* sb/submodule-ignore-trailing-slash (2016-10-25) 3 commits
(merged to 'next' on 2016-10-26 at e56a8ebb38)
+ t0060: sidestep surprising path mangling results on Windows
(merged to 'next' on 2016-10-11 at e37425ed17)
+ submodule: ignore trailing slash in relative url
+ submodule: ignore trailing slash on superproject URL
A minor regression fix for "git submodule".
Will merge to 'master'.
Queued with a test breakage workaround on Windows from j6t.
* st/verify-tag (2016-10-10) 7 commits
- t/t7004-tag: Add --format specifier tests
- t/t7030-verify-tag: Add --format specifier tests
- builtin/tag: add --format argument for tag -v
- builtin/verify-tag: add --format to verify-tag
- tag: add format specifier to gpg_verify_tag
- ref-filter: add function to print single ref_array_item
- gpg-interface, tag: add GPG_VERIFY_QUIET flag
"git tag" and "git verify-tag" learned to put GPG verification
status in their "--format=<placeholders>" output format.
Waiting for review.
cf. <20161007210721.20437-1-santiago@nyu.edu>
* sb/attr (2016-10-24) 36 commits
- completion: clone can initialize specific submodules
- clone: add --init-submodule=<pathspec> switch
- submodule update: add `--init-default-path` switch
- pathspec: allow escaped query values
- pathspec: allow querying for attributes
- pathspec: move prefix check out of the inner loop
- pathspec: move long magic parsing out of prefix_pathspec
- Documentation: fix a typo
- attr: keep attr stack for each check
- attr: convert to new threadsafe API
- attr: make git_check_attr_counted static
- attr.c: outline the future plans by heavily commenting
- attr.c: always pass check[] to collect_some_attrs()
- attr.c: introduce empty_attr_check_elems()
- attr.c: correct ugly hack for git_all_attrs()
- attr.c: rename a local variable check
- attr.c: pass struct git_attr_check down the callchain
- attr.c: add push_stack() helper
- attr: support quoting pathname patterns in C style
- attr: expose validity check for attribute names
- attr: add counted string version of git_attr()
- attr: add counted string version of git_check_attr()
- attr: retire git_check_attrs() API
- attr: convert git_check_attrs() callers to use the new API
- attr: convert git_all_attrs() to use "struct git_attr_check"
- attr: (re)introduce git_check_attr() and struct git_attr_check
- attr: rename function and struct related to checking attributes
- attr.c: plug small leak in parse_attr_line()
- attr.c: tighten constness around "git_attr" structure
- attr.c: simplify macroexpand_one()
- attr.c: mark where #if DEBUG ends more clearly
- attr.c: complete a sentence in a comment
- attr.c: explain the lack of attr-name syntax check in parse_attr()
- attr.c: update a stale comment on "struct match_attr"
- attr.c: use strchrnul() to scan for one line
- commit.c: use strchrnul() to scan for one line
The attributes API has been updated so that it can later be
optimized using the knowledge of which attributes are queried.
Building on top of the updated API, the pathspec machinery learned
to select only paths with given attributes set.
Waiting for review.
* jk/abbrev-auto (2016-10-03) 1 commit
(merged to 'next' on 2016-10-21 at 8aa3d760d8)
+ find_unique_abbrev: move logic out of get_short_sha1()
(this branch is used by jc/abbrev-auto; uses lt/abbrev-auto.)
Updates the way approximate count of total objects is computed
while attempting to come up with a unique abbreviated object name,
which in turn needs to estimate how many hexdigits are necessary to
ensure uniqueness.
Will merge to 'master'.
* nd/ita-empty-commit (2016-10-24) 4 commits
(merged to 'next' on 2016-10-26 at fb007cdae1)
+ commit: don't be fooled by ita entries when creating initial commit
+ commit: fix empty commit creation when there's no changes but ita entries
+ diff: add --ita-[in]visible-in-index
+ diff-lib: allow ita entries treated as "not yet exist in index"
When new paths were added by "git add -N" to the index, it was
enough to circumvent the check by "git commit" to refrain from
making an empty commit without "--allow-empty". The same logic
prevented "git status" to show such a path as "new file" in the
"Changes not staged for commit" section.
Will merge to 'master'.
* lt/abbrev-auto (2016-10-03) 3 commits
(merged to 'next' on 2016-10-03 at bb188d00f7)
+ abbrev: auto size the default abbreviation
+ abbrev: prepare for new world order
+ abbrev: add FALLBACK_DEFAULT_ABBREV to prepare for auto sizing
(this branch is used by jc/abbrev-auto and jk/abbrev-auto.)
Allow the default abbreviation length, which has historically been
7, to scale as the repository grows. The logic suggests to use 12
hexdigits for the Linux kernel, and 9 to 10 for Git itself.
Will hold to see if people scream.
* va/i18n-perl-scripts (2016-10-20) 14 commits
- i18n: difftool: mark warnings for translation
- i18n: send-email: mark string with interpolation for translation
- i18n: send-email: mark warnings and errors for translation
- i18n: send-email: mark strings for translation
- i18n: add--interactive: mark status words for translation
- i18n: add--interactive: remove %patch_modes entries
- i18n: add--interactive: mark edit_hunk_manually message for translation
- i18n: add--interactive: i18n of help_patch_cmd
- i18n: add--interactive: mark patch prompt for translation
- i18n: add--interactive: mark plural strings
- i18n: clean.c: match string with git-add--interactive.perl
- i18n: add--interactive: mark strings with interpolation for translation
- i18n: add--interactive: mark simple here-documents for translation
- i18n: add--interactive: mark strings for translation
Porcelain scripts written in Perl are getting internationalized.
Waiting for review.
cf. <20161010125449.7929-1-vascomalmeida@sapo.pt>
* jc/latin-1 (2016-09-26) 2 commits
(merged to 'next' on 2016-09-28 at c8673e03c2)
+ utf8: accept "latin-1" as ISO-8859-1
+ utf8: refactor code to decide fallback encoding
Some platforms no longer understand "latin-1" that is still seen in
the wild in e-mail headers; replace them with "iso-8859-1" that is
more widely known when conversion fails from/to it.
Will hold to see if people scream.
* ls/filter-process (2016-10-17) 14 commits
(merged to 'next' on 2016-10-19 at ffd0de042c)
+ contrib/long-running-filter: add long running filter example
+ convert: add filter.<driver>.process option
+ convert: prepare filter.<driver>.process option
+ convert: make apply_filter() adhere to standard Git error handling
+ pkt-line: add functions to read/write flush terminated packet streams
+ pkt-line: add packet_write_gently()
+ pkt-line: add packet_flush_gently()
+ pkt-line: add packet_write_fmt_gently()
+ pkt-line: extract set_packet_header()
+ pkt-line: rename packet_write() to packet_write_fmt()
+ run-command: add clean_on_exit_handler
+ run-command: move check_pipe() from write_or_die to run_command
+ convert: modernize tests
+ convert: quote filter names in error messages
The smudge/clean filter API expect an external process is spawned
to filter the contents for each path that has a filter defined. A
new type of "process" filter API has been added to allow the first
request to run the filter for a path to spawn a single process, and
all filtering need is served by this single process for multiple
paths, reducing the process creation overhead.
Will merge to 'master'.
* sg/fix-versioncmp-with-common-suffix (2016-09-08) 5 commits
- versioncmp: cope with common leading parts in versionsort.prereleaseSuffix
- versioncmp: pass full tagnames to swap_prereleases()
- t7004-tag: add version sort tests to show prerelease reordering issues
- t7004-tag: use test_config helper
- t7004-tag: delete unnecessary tags with test_when_finished
The prereleaseSuffix feature of version comparison that is used in
"git tag -l" did not correctly when two or more prereleases for the
same release were present (e.g. when 2.0, 2.0-beta1, and 2.0-beta2
are there and the code needs to compare 2.0-beta1 and 2.0-beta2).
Waiting for a reroll.
cf. <20160908223727.Horde.jVOOJ278ssZ3qkyjkmyqZD-@webmail.informatik.kit.edu>
* jc/pull-rebase-ff (2016-07-28) 1 commit
- pull: fast-forward "pull --rebase=true"
"git pull --rebase", when there is no new commits on our side since
we forked from the upstream, should be able to fast-forward without
invoking "git rebase", but it didn't.
Needs a real log message and a few tests.
* jc/merge-drop-old-syntax (2015-04-29) 1 commit
(merged to 'next' on 2016-10-11 at 8928c8b9b3)
+ merge: drop 'git merge <message> HEAD <commit>' syntax
Stop supporting "git merge <message> HEAD <commit>" syntax that has
been deprecated since October 2007, and issues a deprecation
warning message since v2.5.0.
It has been reported that git-gui still uses the deprecated syntax,
which needs to be fixed before this final step can proceed.
cf. <5671DB28.8020901@kdbg.org>
Will cook in 'next'.
^ permalink raw reply
* Re: [PATCH] compat: Allow static initializer for pthreads on Windows
From: Stefan Beller @ 2016-10-26 22:14 UTC (permalink / raw)
To: Johannes Schindelin, Johannes Sixt
Cc: git@vger.kernel.org, Simon Ruderich, Jeff King, Stefan Beller
In-Reply-To: <20161026215732.16411-1-sbeller@google.com>
On Wed, Oct 26, 2016 at 2:57 PM, Stefan Beller <sbeller@google.com> wrote:
> In Windows it is not possible to have a static initialized mutex as of
> now, but that seems to be painful for the upcoming refactoring of the
> attribute subsystem, as we have no good place to put the initialization
> of the attr global lock.
>
> The trick is to get a named mutex as CreateMutex[1] will return the
> existing named mutex if it exists in a thread safe way, or return
> a newly created mutex with that name.
>
> Inside the critical section of the single named mutex, we need to double
> check if the mutex was already initialized because the outer check is
> not sufficient.
> (e.g. 2 threads enter the first condition `(!a)` at the same time, but
> only one of them will acquire the named mutex first and proceeds to
> initialize the given mutex a. The second thread shall not re-initialize
> the given mutex `a`, which is why we have the inner condition on `(!a)`.
>
> Due to the use of memory barriers inside the critical section the mutex
> `a` gets updated to other threads, such that any further invocation
> will skip the initialization check code altogether on the first condition.
>
> [1] https://msdn.microsoft.com/en-us/library/windows/desktop/ms682411(v=vs.85).aspx
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>
> Flying blind here, i.e. not compiled, not tested. For a system I do not
> have deep knowledge of. The only help was the online documentation.
This is of course a Double Check Locking Pattern, that Johannes warned about
a couple of days ago. However according to
https://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html
we can make it work by adding enough memory barriers, (See section
"Making it work with explicit memory barriers").
^ permalink raw reply
* [PATCH] compat: Allow static initializer for pthreads on Windows
From: Stefan Beller @ 2016-10-26 21:57 UTC (permalink / raw)
To: Johannes.Schindelin, j6t; +Cc: git, simon, peff, Stefan Beller
In Windows it is not possible to have a static initialized mutex as of
now, but that seems to be painful for the upcoming refactoring of the
attribute subsystem, as we have no good place to put the initialization
of the attr global lock.
The trick is to get a named mutex as CreateMutex[1] will return the
existing named mutex if it exists in a thread safe way, or return
a newly created mutex with that name.
Inside the critical section of the single named mutex, we need to double
check if the mutex was already initialized because the outer check is
not sufficient.
(e.g. 2 threads enter the first condition `(!a)` at the same time, but
only one of them will acquire the named mutex first and proceeds to
initialize the given mutex a. The second thread shall not re-initialize
the given mutex `a`, which is why we have the inner condition on `(!a)`.
Due to the use of memory barriers inside the critical section the mutex
`a` gets updated to other threads, such that any further invocation
will skip the initialization check code altogether on the first condition.
[1] https://msdn.microsoft.com/en-us/library/windows/desktop/ms682411(v=vs.85).aspx
Signed-off-by: Stefan Beller <sbeller@google.com>
---
Flying blind here, i.e. not compiled, not tested. For a system I do not
have deep knowledge of. The only help was the online documentation.
compat/win32/pthread.h | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h
index 1c16408..a900513 100644
--- a/compat/win32/pthread.h
+++ b/compat/win32/pthread.h
@@ -21,9 +21,25 @@
static inline int return_0(int i) {
return 0;
}
+
+#define PTHREAD_MUTEX_INITIALIZER NULL
#define pthread_mutex_init(a,b) return_0((InitializeCriticalSection((a)), 0))
#define pthread_mutex_destroy(a) DeleteCriticalSection((a))
-#define pthread_mutex_lock EnterCriticalSection
+#define pthread_mutex_lock(a) \
+{ \
+ if (!a) { \
+ HANDLE p = CreateMutex(NULL, FALSE, "Git-Global-Windows-Mutex"); \
+ EnterCriticalSection(p); \
+ MemoryBarrier(); \
+ if (!a)
+ pthread_mutex_init(a); \
+ MemoryBarrier(); \
+ ReleaseMutex(p); \
+ } \
+ EnterCriticalSection(a); \
+}
+
+
#define pthread_mutex_unlock LeaveCriticalSection
typedef int pthread_mutexattr_t;
--
2.10.1.508.g6572022
^ permalink raw reply related
* [PATCH] attr: expose error reporting function for invalid attribute names
From: Stefan Beller @ 2016-10-26 21:20 UTC (permalink / raw)
To: ramsay; +Cc: git, bmwill, gitster, pclouds, Stefan Beller
In-Reply-To: <0425fea3-3419-c265-b964-f5a309b867fa@ramsayjones.plus.com>
From: Junio C Hamano <gitster@pobox.com>
Export invalid_attr_name_message() function that returns the
message to be given when a given <name, len> pair
is not a good name for an attribute.
We could later update the message to exactly spell out what the
rules for a good attribute name are, etc.
We do not need to export the validity check 'attr_name_valid()' itself
as we will learn about the validity indirectly in a later patch
via calling 'git_attr_counted()'.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
Ramsay,
I intend to replace the previous
[PATCH 17/36] attr: expose validity check for attribute names
by this one in a reroll.
Thanks,
Stefan
attr.c | 39 +++++++++++++++++++++++++--------------
attr.h | 2 ++
2 files changed, 27 insertions(+), 14 deletions(-)
diff --git a/attr.c b/attr.c
index 90dbacd..ec878c3 100644
--- a/attr.c
+++ b/attr.c
@@ -59,23 +59,38 @@ static unsigned hash_name(const char *name, int namelen)
return val;
}
-static int invalid_attr_name(const char *name, int namelen)
+static int attr_name_valid(const char *name, size_t namelen)
{
/*
* Attribute name cannot begin with '-' and must consist of
* characters from [-A-Za-z0-9_.].
*/
if (namelen <= 0 || *name == '-')
- return -1;
+ return 0;
while (namelen--) {
char ch = *name++;
if (! (ch == '-' || ch == '.' || ch == '_' ||
('0' <= ch && ch <= '9') ||
('a' <= ch && ch <= 'z') ||
('A' <= ch && ch <= 'Z')) )
- return -1;
+ return 0;
}
- return 0;
+ return 1;
+}
+
+void invalid_attr_name_message(struct strbuf *err, const char *name, int len)
+{
+ strbuf_addf(err, _("%.*s is not a valid attribute name"),
+ len, name);
+}
+
+static void report_invalid_attr(const char *name, size_t len,
+ const char *src, int lineno)
+{
+ struct strbuf err = STRBUF_INIT;
+ invalid_attr_name_message(&err, name, len);
+ fprintf(stderr, "%s: %s:%d\n", err.buf, src, lineno);
+ strbuf_release(&err);
}
struct git_attr *git_attr_counted(const char *name, size_t len)
@@ -90,7 +105,7 @@ struct git_attr *git_attr_counted(const char *name, size_t len)
return a;
}
- if (invalid_attr_name(name, len))
+ if (!attr_name_valid(name, len))
return NULL;
FLEX_ALLOC_MEM(a, name, name, len);
@@ -176,17 +191,15 @@ static const char *parse_attr(const char *src, int lineno, const char *cp,
cp++;
len--;
}
- if (invalid_attr_name(cp, len)) {
- fprintf(stderr,
- "%.*s is not a valid attribute name: %s:%d\n",
- len, cp, src, lineno);
+ if (!attr_name_valid(cp, len)) {
+ report_invalid_attr(cp, len, src, lineno);
return NULL;
}
} else {
/*
* As this function is always called twice, once with
* e == NULL in the first pass and then e != NULL in
- * the second pass, no need for invalid_attr_name()
+ * the second pass, no need for attr_name_valid()
* check here.
*/
if (*cp == '-' || *cp == '!') {
@@ -229,10 +242,8 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
name += strlen(ATTRIBUTE_MACRO_PREFIX);
name += strspn(name, blank);
namelen = strcspn(name, blank);
- if (invalid_attr_name(name, namelen)) {
- fprintf(stderr,
- "%.*s is not a valid attribute name: %s:%d\n",
- namelen, name, src, lineno);
+ if (!attr_name_valid(name, namelen)) {
+ report_invalid_attr(name, namelen, src, lineno);
goto fail_return;
}
}
diff --git a/attr.h b/attr.h
index bcedf92..d39e327 100644
--- a/attr.h
+++ b/attr.h
@@ -13,6 +13,8 @@ extern struct git_attr *git_attr(const char *);
/* The same, but with counted string */
extern struct git_attr *git_attr_counted(const char *, size_t);
+extern void invalid_attr_name_message(struct strbuf *, const char *, int);
+
/* Internal use */
extern const char git_attr__true[];
extern const char git_attr__false[];
--
2.10.1.508.g6572022
^ permalink raw reply related
* Re: [PATCH 0/2] git-svn: implement "git worktree" awareness
From: Junio C Hamano @ 2016-10-26 21:17 UTC (permalink / raw)
To: Eric Wong
Cc: Jakub Narębski, git, Mathieu Arnold,
Nguyễn Thái Ngọc Duy, Stefan Beller
In-Reply-To: <20161026200248.GA28105@starla>
Eric Wong <e@80x24.org> writes:
> Eric Wong <e@80x24.org> wrote:
>> +Cc Jakub since gitweb could probably take advantage of get_record
>> from the first patch, too. I'm not completely sure about the API
>> for this, though.
>
> Jakub: ping?
>
> +Cc: Junio, too. I'm hoping to have this in 2.11.
I somehow was hoping that I can pull this as part of git-svn updates
for the upcoming release without having to even think about it (I
did read the patch when they were posted and did not find anything
wrong with them, fwiw).
>> The following changes since commit 3cdd5d19178a54d2e51b5098d43b57571241d0ab:
>>
>> Sync with maint (2016-10-11 14:55:48 -0700)
>>
>> are available in the git repository at:
>>
>> git://bogomips.org/git-svn.git svn-wt
>>
>> for you to fetch changes up to 112423eb905cf28c9445781a7647ba590d597ab3:
>>
>> git-svn: "git worktree" awareness (2016-10-14 01:36:12 +0000)
Thanks.
^ permalink raw reply
* Re: [PATCH v3 2/3] sha1_file: open window into packfiles with O_CLOEXEC
From: Junio C Hamano @ 2016-10-26 21:15 UTC (permalink / raw)
To: Jeff King; +Cc: git, Lars Schneider, Eric Wong, Johannes Schindelin
In-Reply-To: <20161026201721.2pw4slsuyhxhcwxj@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Wed, Oct 26, 2016 at 10:52:41AM -0700, Junio C Hamano wrote:
>
>> > I actually wonder if it is worth carrying around the O_NOATIME hack at
>> > all.
>>
>> Yes, I share the thought. We no longer have too many loose objects
>> to matter.
>>
>> I do not mind flipping the order, but I'd prefer to cook the result
>> even longer. I am tempted to suggest we take two step route:
>>
>> - ship 2.11 with the "atime has been there and we won't regress it"
>> shape, while cooking the "cloexec is semantically more
>> important" version in 'next' during the feature freeze
>>
>> - immediately after 2.11 merge it to 'master' for 2.12 to make sure
>> there is no fallout.
>
> That sounds reasonable, though I'd consider jumping straight to "NOATIME
> is not worth it; drop it" as the patch for post-2.11.
That endgame is fine by me too. Thanks for a sanity-check.
^ permalink raw reply
* Re: git-archive and submodules
From: Stefan Beller @ 2016-10-26 21:04 UTC (permalink / raw)
To: Anatoly Borodin; +Cc: git@vger.kernel.org
In-Reply-To: <nur45i$e9b$1@blaine.gmane.org>
On Wed, Oct 26, 2016 at 1:37 PM, Anatoly Borodin
<anatoly.borodin@gmail.com> wrote:
> are there plans to add submodules support to git-archive?
plans by whom?
Git is a project with contributors from all over the place. (different
time zones,
people motivated by different means, i.e. we have the hobbiest that
scratches their
itch, we have paid people working on Git because their employer wants
them to work on Git,
there are other people (who like to) use Git in their work environment
and hack on it
in their spare time to make it awesome.)
AFAICT there are currently not a lot of people actively working on
submodule features,
though there is some history, e.g. Jens Lehmann maintains a wiki
specialised on submodules
https://github.com/jlehmann/git-submod-enhancements/wiki
and archive is mentioned there as one of the many "Issues still to be tackled".
Maybe you want to give it a try as you need it? I'd be happy to review any
submodule related code.
How to get started:
* git clone https://github.com/git/git
* Read (at least skim) Documentation/SubmittingPatches)
* Look at builtin/archive.c as a starting point (cmd_archive is called
when you call "git archive ...")
* That leads to archive.c:write_archive, which calls parse_archive_args
There we'd want to add an option there for recursing into submodules.
* See write_archive_entry (still in archive.c) that mentions S_ISGITLINK
Somewhere there you need to add code. :)
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH 27/36] attr: convert to new threadsafe API
From: Stefan Beller @ 2016-10-26 20:46 UTC (permalink / raw)
To: Johannes Sixt
Cc: Jeff King, Simon Ruderich, Johannes Schindelin, Junio C Hamano,
git@vger.kernel.org, Brandon Williams, Duy Nguyen
In-Reply-To: <6f231a78-5d74-b93f-a644-a4700c9dfbe7@kdbg.org>
On Wed, Oct 26, 2016 at 1:40 PM, Johannes Sixt <j6t@kdbg.org> wrote:
> Am 26.10.2016 um 22:26 schrieb Jeff King:
>>
>> On Wed, Oct 26, 2016 at 10:25:38PM +0200, Johannes Sixt wrote:
>>
>>> Am 26.10.2016 um 21:51 schrieb Stefan Beller:
>>>>
>>>> it is
>>>> very convenient to not have to explicitly initialize mutexes?
>>>
>>>
>>> Not to initialize a mutex is still wrong for pthreads.
>>
>>
>> I think Stefan was being loose with his wording. There would still be an
>> initializer, but it would be a constant (and in the case of pthread
>> emulation on Windows, would just be NULL).
>
>
> And I was loose, too: Not to initialize a mutex with at least
> PTHREAD_MUTEX_INITILIZER (if not pthread_mutex_init) is still wrong.
>
My words were wrong, I meant statically initialized instead of the need to
call a function to initialize a mutex. (For the attribute subsystem, where would
that function go? We use attrs all over the place. My current thinking would
be in git.c to initialize the Big Single Attr Lock. I feel like that
is not very well
maintainable though).
Sorry for the confusion,
Stefan
^ permalink raw reply
* Re: [PATCH 27/36] attr: convert to new threadsafe API
From: Stefan Beller @ 2016-10-26 20:43 UTC (permalink / raw)
To: Jeff King
Cc: Johannes Sixt, Simon Ruderich, Johannes Schindelin,
Junio C Hamano, git@vger.kernel.org, Brandon Williams, Duy Nguyen
In-Reply-To: <20161026202652.kz63mxqqjan7swvl@sigill.intra.peff.net>
On Wed, Oct 26, 2016 at 1:26 PM, Jeff King <peff@peff.net> wrote:
> On Wed, Oct 26, 2016 at 10:25:38PM +0200, Johannes Sixt wrote:
>
>> Am 26.10.2016 um 21:51 schrieb Stefan Beller:
>> > it is
>> > very convenient to not have to explicitly initialize mutexes?
>>
>> Not to initialize a mutex is still wrong for pthreads.
>
> I think Stefan was being loose with his wording. There would still be an
> initializer, but it would be a constant (and in the case of pthread
> emulation on Windows, would just be NULL).
Exactly, so we would do
/* as per the man page of pthread_mutexes: */
pthread_mutex_t mymutex = PTHREAD_MUTEX_INITIALIZER;
int somefunction()
{
pthread_mutex_lock(&mymutex); /* threadsafely initialised on first use */
...
pthread_unlock(&mymutex);
}
and for the Windows compat we'd do
#define PTHREAD_MUTEX_INITIALIZER NULL
#define pthread_mutex_lock emulate_pthread_mutex_lock
int emulate_pthread_mutex_lock(volatile MUTEX_TYPE *mx)
{
if (*mx == NULL) /* static initializer? */
{ /* this stackoverflow magic to initialize threadsafely if not init'd */}
EnterCriticalSection(mx) /* as it currently is in compat/win32/pthread.h */
return 0;
}
>
> -Peff
^ permalink raw reply
* Re: [PATCH 27/36] attr: convert to new threadsafe API
From: Johannes Sixt @ 2016-10-26 20:40 UTC (permalink / raw)
To: Jeff King
Cc: Stefan Beller, Simon Ruderich, Johannes Schindelin,
Junio C Hamano, git@vger.kernel.org, Brandon Williams, Duy Nguyen
In-Reply-To: <20161026202652.kz63mxqqjan7swvl@sigill.intra.peff.net>
Am 26.10.2016 um 22:26 schrieb Jeff King:
> On Wed, Oct 26, 2016 at 10:25:38PM +0200, Johannes Sixt wrote:
>
>> Am 26.10.2016 um 21:51 schrieb Stefan Beller:
>>> it is
>>> very convenient to not have to explicitly initialize mutexes?
>>
>> Not to initialize a mutex is still wrong for pthreads.
>
> I think Stefan was being loose with his wording. There would still be an
> initializer, but it would be a constant (and in the case of pthread
> emulation on Windows, would just be NULL).
And I was loose, too: Not to initialize a mutex with at least
PTHREAD_MUTEX_INITILIZER (if not pthread_mutex_init) is still wrong.
-- Hannes
^ permalink raw reply
* git-archive and submodules
From: Anatoly Borodin @ 2016-10-26 20:37 UTC (permalink / raw)
To: git
Hi All,
are there plans to add submodules support to git-archive?
--
Mit freundlichen Grüßen,
Anatoly Borodin
^ permalink raw reply
* Re: [PATCH 27/36] attr: convert to new threadsafe API
From: Jeff King @ 2016-10-26 20:26 UTC (permalink / raw)
To: Johannes Sixt
Cc: Stefan Beller, Simon Ruderich, Johannes Schindelin,
Junio C Hamano, git@vger.kernel.org, Brandon Williams, Duy Nguyen
In-Reply-To: <e1f760f5-27a7-8266-5d6c-d61fab7e194d@kdbg.org>
On Wed, Oct 26, 2016 at 10:25:38PM +0200, Johannes Sixt wrote:
> Am 26.10.2016 um 21:51 schrieb Stefan Beller:
> > it is
> > very convenient to not have to explicitly initialize mutexes?
>
> Not to initialize a mutex is still wrong for pthreads.
I think Stefan was being loose with his wording. There would still be an
initializer, but it would be a constant (and in the case of pthread
emulation on Windows, would just be NULL).
-Peff
^ permalink raw reply
* Re: [PATCH 27/36] attr: convert to new threadsafe API
From: Johannes Sixt @ 2016-10-26 20:25 UTC (permalink / raw)
To: Stefan Beller
Cc: Jeff King, Simon Ruderich, Johannes Schindelin, Junio C Hamano,
git@vger.kernel.org, Brandon Williams, Duy Nguyen
In-Reply-To: <CAGZ79kYgk9rQDju0MT2uniaxhAWpzJ9f1T9czgNnxfq+Wz6m+A@mail.gmail.com>
Am 26.10.2016 um 21:51 schrieb Stefan Beller:
> it is
> very convenient to not have to explicitly initialize mutexes?
Not to initialize a mutex is still wrong for pthreads.
-- Hannes
^ permalink raw reply
* Re: [PATCH 27/36] attr: convert to new threadsafe API
From: Jeff King @ 2016-10-26 20:20 UTC (permalink / raw)
To: Stefan Beller
Cc: Simon Ruderich, Johannes Schindelin, Junio C Hamano,
git@vger.kernel.org, Brandon Williams, Duy Nguyen
In-Reply-To: <CAGZ79kYgk9rQDju0MT2uniaxhAWpzJ9f1T9czgNnxfq+Wz6m+A@mail.gmail.com>
On Wed, Oct 26, 2016 at 12:51:02PM -0700, Stefan Beller wrote:
> > I seem to recall this does not work on Windows, where the pthread
> > functions are thin wrappers over CRITICAL_SECTION. Other threaded code
> > in git does an explicit setup step before entering threaded sections.
> > E.g., see start_threads() in builtin/grep.c.
> >
>
> I wonder if we can have a similar thing as
> http://stackoverflow.com/a/9490113 in compat/win32/pthread.{h.c} as it is
> very convenient to not have to explicitly initialize mutexes?
I agree it would be much more convenient and get rid of some repetitive
boilerplate code. I'll leave it to Windows folks to decide if they are
OK with that approach or not (I do not offhand know of any reason it
would not work).
-Peff
^ 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