* Re: [PATCH] Allow stashes to be referenced by index only
From: Jeff King @ 2016-10-23 9:02 UTC (permalink / raw)
To: Aaron M Watson
Cc: git, Jon Seymour, David Caldwell, Øystein Walle,
Ævar Arnfjörð Bjarmason, David Aguilar,
Alex Henrie
In-Reply-To: <1473378397-22453-1-git-send-email-watsona4@gmail.com>
On Thu, Sep 08, 2016 at 07:46:37PM -0400, Aaron M Watson wrote:
> Instead of referencing "stash@{n}" explicitly, it can simply be
> referenced as "n". Most users only reference stashes by their position
> in the stash stask (what I refer to as the "index"). The syntax for the
> typical stash (stash@{n}) is slightly annoying and easy to forget, and
> sometimes difficult to escape properly in a script. Because of this the
> capability to do things with the stash by simply referencing the index
> is desirable.
>
> This patch includes the superior implementation provided by Øsse Walle
> (thanks for that), with a slight change to fix a broken test in the test
> suite. I also merged the test scripts as suggested by Jeff King, and
> un-wrapped the documentation as suggested by Junio Hamano.
Just cleaning out my list of leftover bits, and it looks like this patch
fell through the cracks.
The last thing before this was generally people agreeing with the
approach that Øsse showed:
http://public-inbox.org/git/CAFaJEqu-JUcwLjrQBk_huSa3DZfCf8O4eAZ=UgcXHzN=CLgtpw@mail.gmail.com/
http://public-inbox.org/git/xmqqbmzzoazy.fsf@gitster.mtv.corp.google.com/
and we just needed to roll that up into a patch. Which this _mostly_ is,
but there are a few funny things still:
> diff --git a/git-stash.sh b/git-stash.sh
> index 826af18..d8d3b8d 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -384,9 +384,10 @@ parse_flags_and_rev()
> i_tree=
> u_tree=
>
> - REV=$(git rev-parse --no-flags --symbolic --sq "$@") || exit 1
> + REV=$(git rev-parse --no-flags --symbolic --sq "$@" 2> /dev/null)
Style: we don't put a space between ">" and the filename.
So here we assign REV, and we no longer exit (to handle something like
"1" by itself, which is good).
> FLAGS=
> + ARGV=
> for opt
> do
> case "$opt" in
> @@ -404,10 +405,13 @@ parse_flags_and_rev()
> die "$(eval_gettext "unknown option: \$opt")"
> FLAGS="${FLAGS}${FLAGS:+ }$opt"
> ;;
> + *)
> + ARGV="${ARGV}${ARGV:+ }'$opt'"
> + ;;
> esac
> done
>
> - eval set -- $REV
> + eval set -- $ARGV
But what's going on here? Why did we bother running rev-parse earlier if
we don't actually use the value of REV?
You mentioned tweaking it to fix a broken test, and indeed, just using
$REV here breaks a few tests in t3903.
Offhand, I do not see anything wrong with pulling the non-option values
out in the loop. But in that case I think the assignment of REV can just
go away completely.
> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> index 2142c1f..f82a8c4 100755
> --- a/t/t3903-stash.sh
> +++ b/t/t3903-stash.sh
> @@ -131,6 +131,26 @@ test_expect_success 'drop middle stash' '
> test 1 = $(git show HEAD:file)
> '
>
> +test_expect_success 'drop middle stash by index' '
> + git reset --hard &&
> + echo 8 > file &&
> + git stash &&
> + echo 9 > file &&
Ditto on the ">" style here and elsewhere.
The tests otherwise look good to me.
-Peff
^ permalink raw reply
* [PATCH] hex: use unsigned index for ring buffer
From: René Scharfe @ 2016-10-23 9:00 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jeff King
Overflow is defined for unsigned integers, but not for signed ones.
Make the ring buffer index in sha1_to_hex() unsigned to be on the
safe side.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
Hard to trigger, but probably even harder to diagnose once someone
somehow manages to do it on some uncommon architecture.
hex.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hex.c b/hex.c
index ab2610e..8c6c189 100644
--- a/hex.c
+++ b/hex.c
@@ -76,7 +76,7 @@ char *oid_to_hex_r(char *buffer, const struct object_id *oid)
char *sha1_to_hex(const unsigned char *sha1)
{
- static int bufno;
+ static unsigned int bufno;
static char hexbuffer[4][GIT_SHA1_HEXSZ + 1];
return sha1_to_hex_r(hexbuffer[3 & ++bufno], sha1);
}
--
2.10.1
^ permalink raw reply related
* revisiting zstd timings
From: Jeff King @ 2016-10-23 8:05 UTC (permalink / raw)
To: git
You may recall some "what if" timings I did recently, where I replaced
zlib with zstd:
http://public-inbox.org/git/20160914235843.nacr54ekvl6rjipk@sigill.intra.peff.net/
The aim was that it would give us about the same compression level, but
much faster inflating and deflating. My numbers there showed that yes,
inflate is faster (which speeds up normal operations like traversals and
diffs), but that compression seemed to be much slower.
Since then, zstd has released a new version of their zlib wrapper which
performs much better for our case. Basically, the prior version's
transparent zstd setup was not optimized well for our case of doing tons
of small deflates. I'm cc-ing Przemyslaw Skibinski, who wrote the
wrapper code, and who pointed me in the right direction (thanks!).
I'll include the new patch at the end, but it's basically the same as
the old with one minor symbol change:
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 7da2fd44b0..07f45aa171 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2287,7 +2287,7 @@ static int git_pack_config(const char *k, const char *v, void *cb)
return config_error_nonbool(k);
#ifdef USE_ZSTD
if (skip_prefix(v, "zstd", &v)) {
- useZSTD(1);
+ ZWRAP_useZSTDcompression(1);
/* XXX doesn't seem to be a constant? */
pack_compression_max = 22;
}
Here are the numbers I get with the newer version (see the prior email
for descriptions of each test, but I did run them all from scratch
again).
zlib
- pack
1127899484
- repack
real 4m55.099s
user 17m46.796s
sys 0m6.164s
- rev-list
real 0m27.402s
user 0m27.080s
sys 0m0.320s
- log -Sfoo
real 5m16.326s
user 5m13.860s
sys 0m2.448s
That gives us a baseline for the time of each operation, and the size of
the compressed pack. The rest of the values will be expressed as
percentages from this baseline.
zstd=disabled
- pack
1127884778
(+0%)
- repack
real 4m49.443s
user 17m39.468s
sys 0m5.996s
(-1.9%)
- rev-list
real 0m27.527s
user 0m27.316s
sys 0m0.208s
(+0.4%)
- log -Sfoo
real 5m21.029s
user 5m17.704s
sys 0m3.308s
(+1.4%)
Timing the wrapper making use of zlib, it produces the same output and
takes a slightly larger amount of time (the repack is quicker, but
there's quite a bit of run-to-run noise in that measurement). Nothing
surprising.
zstd=1
- pack
1191357029
(+5.6%)
- repack
real 4m8.323s
user 16m24.664s
sys 0m6.816s
(-15.8%)
- rev-list
real 0m25.780s
user 0m25.488s
sys 0m0.288s
(-5.9%)
- log -Sfoo
real 4m19.301s
user 4m16.488s
sys 0m2.796s
(-18%)
Here's where we get interesting; zstd turned down to its lowest setting.
As before, the inflate step shows a nice speedup for some common
read-only operations. The repack is much faster, and the resulting pack
is slightly larger. We'd probably want to tune the cpu/size tradeoff for
deflate a bit higher.
zstd=3
- pack
1163632679
(+3.1%)
- repack
real 4m11.800s
user 16m34.260s
sys 0m6.788s
(-14.6%)
- rev-list
real 0m25.678s
user 0m25.376s
sys 0m0.296s
(-6.2%)
- log -Sfoo
real 4m19.902s
user 4m16.604s
sys 0m3.280s
(-17.8%)
And here that is. Without spending much more CPU on deflate, we shaved
off a few percent. The reading side remains fast (I wondered if it might
get faster as we shrink the pack just because there are fewer bytes to
deal with on the input side, but it doesn't seem to really matter. And
anyway, we're only talking a few percent of the input size).
zstd=5
- pack
1137697830
(+0.8%)
- repack
real 4m20.930s
user 16m45.844s
sys 0m7.100s
(-11.5%)
Turning it up higher, we're starting to reach parity with zlib for the
pack size. And the deflate process is still faster. I stopped measuring
the read side, since it seemed to be remaining constant.
zstd=7
- pack
1130545314
(+0.2%)
- repack
real 4m28.960s
user 16m50.028s
sys 0m6.884s
(-8.8%)
zstd=9
- pack
1128847500
(+0%)
- repack
real 4m52.234s
user 17m22.644s
sys 0m7.080s
(-0.9%)
zstd=11
- pack
1128415787
(+0%)
- repack
real 5m33.187s
user 18m12.792s
sys 0m7.436s
(+12.9%)
And these numbers show we can pretty much dial in the cpu/size tradeoff
wherever we want. Going past 9 doesn't seem to really help. I'd
probably leave it at 5, as 0.8% is close enough.
Note that this repack scenario _isn't_ especially realistic. It's
finding new deltas for the whole pack from scratch, which is something
that isn't generally done. Instead, we'd generally deflate a small
number of new objects during a repack, or perhaps when packing objects
to serve a fetch or push. And in that case, compression would be a much
smaller part of the whole task (because we'd spend a lot of our time
writing out the already-compressed objects verbatim).
So saving 10% here really _isn't_ that interesting. I mostly wanted to
confirm that we could use zstd without increasing the CPU time used for
deflating, so that we could reap the benefits on the inflate side. Which
is definitely the case. With these numbers, there's basically no
downside at all to using zstd. It's just faster to read the objects
later.
If we were designing git today, it seems like a no-brainer to use zstd
over zlib. But given backwards-compatibility issues, I'm not sure.
10-20% speedup on reading is awfully nice, but I don't think there's a
good way to gracefully transition, because zlib is part of the
on-the-wire format for serving objects. We could re-compress on the fly,
but that gets expensive (in existing cases, we can quite often serve the
zlib content straight from disk, but this would require an extra
inflate/deflate. At least we wouldn't have to reconstitute objects from
deltas, though).
A transition would probably look something like:
0. The patch below, or something like it, to teach git to read both
zlib and zstd, and optionally write zstd. We'd probably want to
make this an unconditional requirement like zlib, because the point
is for it to be available everywhere (I assume the zstd code is
pretty portable, but I haven't put it to the test).
1. Another patch to add a "zstd" capability to the protocol. This
would require teaching pack-objects an option to convert zstd back
to zlib on the fly.
Servers which handle a limited number of updated clients can switch
to zstd immediately to get the benefit, and their clients can
handle it directly. Likewise, clients of older servers may wish to
repack locally using zstd to get the benefit. They'll have to
recompress on the fly during push, but pushes are rare than other
operations (and often limited by bandwidth anyway).
2. After a while, eventually flip the default to zstd=5.
3. If "a while" is long enough, perhaps add a patch to let servers
tell clients "go upgrade" rather than recompressing on the fly.
I don't have immediate plans for any of that, but maybe something to
think about.
Here's the patch which produce the numbers above. Build it with:
make ZSTD_PATH=/path/to/zstd
where /path/to/zstd is a built clone of
https://github.com/facebook/zstd.
---
diff --git a/Makefile b/Makefile
index 1aad150b34..7e7c8c73e1 100644
--- a/Makefile
+++ b/Makefile
@@ -1140,6 +1140,15 @@ else
endif
IMAP_SEND_LDFLAGS += $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)
+# for linking straight out of zstd build directory
+ifdef ZSTD_PATH
+ LIB_OBJS += $(ZSTD_PATH)/zlibWrapper/zstd_zlibwrapper.o
+ BASIC_CFLAGS += -I$(ZSTD_PATH)/zlibWrapper
+ BASIC_CFLAGS += -I$(ZSTD_PATH)/lib -I$(ZSTD_PATH)/lib/common
+ BASIC_CFLAGS += -DUSE_ZSTD
+ EXTLIBS += $(ZSTD_PATH)/lib/libzstd.a
+endif
+
ifdef ZLIB_PATH
BASIC_CFLAGS += -I$(ZLIB_PATH)/include
EXTLIBS += -L$(ZLIB_PATH)/$(lib) $(CC_LD_DYNPATH)$(ZLIB_PATH)/$(lib)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 166e52c700..07f45aa171 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -62,6 +62,7 @@ static int num_preferred_base;
static struct progress *progress_state;
static int pack_compression_level = Z_DEFAULT_COMPRESSION;
static int pack_compression_seen;
+static int pack_compression_max = Z_BEST_COMPRESSION;
static struct packed_git *reuse_packfile;
static uint32_t reuse_packfile_objects;
@@ -2281,11 +2282,17 @@ static int git_pack_config(const char *k, const char *v, void *cb)
return 0;
}
if (!strcmp(k, "pack.compression")) {
- int level = git_config_int(k, v);
- if (level == -1)
- level = Z_DEFAULT_COMPRESSION;
- else if (level < 0 || level > Z_BEST_COMPRESSION)
- die("bad pack compression level %d", level);
+ int level;
+ if (!v)
+ return config_error_nonbool(k);
+#ifdef USE_ZSTD
+ if (skip_prefix(v, "zstd", &v)) {
+ ZWRAP_useZSTDcompression(1);
+ /* XXX doesn't seem to be a constant? */
+ pack_compression_max = 22;
+ }
+#endif
+ level = git_config_int(k, v);
pack_compression_level = level;
pack_compression_seen = 1;
return 0;
@@ -2826,7 +2833,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
reuse_delta = 0;
if (pack_compression_level == -1)
pack_compression_level = Z_DEFAULT_COMPRESSION;
- else if (pack_compression_level < 0 || pack_compression_level > Z_BEST_COMPRESSION)
+ else if (pack_compression_level < 0 || pack_compression_level > pack_compression_max)
die("bad pack compression level %d", pack_compression_level);
if (!delta_search_threads) /* --threads=0 means autodetect */
diff --git a/cache.h b/cache.h
index ed3d5dfce1..acc05720a0 100644
--- a/cache.h
+++ b/cache.h
@@ -37,7 +37,11 @@
#define git_SHA1_Update git_SHA1_Update_Chunked
#endif
+#ifdef USE_ZSTD
+#include "zstd_zlibwrapper.h"
+#else
#include <zlib.h>
+#endif
typedef struct git_zstream {
z_stream z;
unsigned long avail_in;
^ permalink raw reply related
* Re: tools for easily "uncommitting" parts of a patch I just commited?
From: Jeff King @ 2016-10-23 1:38 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Lukas Fleischer, Git Mailing List
In-Reply-To: <CACsJy8Bz-DhE+CkJH+3zsrZJUQfGYDN072MKawJ6dx5begfnMw@mail.gmail.com>
On Sun, Oct 23, 2016 at 08:23:01AM +0700, Duy Nguyen wrote:
> I hit the same problem sometimes, but in my case sometimes I
> accidentally do "git add" after "git add -p" and a configuration in
> "git commit -a" won't help me. I'd prefer we could undo changes in
> index instead. Something like reflog but for index.
An index write always writes the whole file from scratch, so you really
just need to save a copy of the old file. Perhaps something like:
rm -f $GIT_DIR/index.old
ln $GIT_DIR/index.old $GIT_DIR/index
... and then open $GIT_DIR/index.tmp ...
... and then rename(index.tmp, index) ...
could do it cheaply. It's a little more complicated if you want to save
a sequence of versions, and eventually would take a lot of space, but
presumably a handful of saved indexes would be sufficient.
Another option would be an index format that journals, and you could
potentially walk back the journal to a point. That seems like a much
bigger change (and has weird layering, because deciding when to fold in
the journal is usually a performance thing, but obviously this would
have user-visible impact about how far back you could undo).
-Peff
^ permalink raw reply
* Re: tools for easily "uncommitting" parts of a patch I just commited?
From: Duy Nguyen @ 2016-10-23 1:23 UTC (permalink / raw)
To: Lukas Fleischer; +Cc: Git Mailing List, Jeff King
In-Reply-To: <147712794056.12237.1478296296628149440@typhoon.fritz.box>
On Sat, Oct 22, 2016 at 4:19 PM, Lukas Fleischer <lfleischer@lfos.de> wrote:
> On Thu, 20 Oct 2016 at 19:27:58, Jacob Keller wrote:
>> [...]
>> I still think we're misunderstanding. I want git commit to complain
>> *only* under the following circumstance:
>>
>> I run "git add -p" and put a partial change into the index in <file>.
>> There are still other parts which were not added to the index yet.
>> Thus, the index version of the file and the actual file differ.
>>
>> Then, I (accidentally) run "git commit <file>"
>> [...]
>
> This reminded me of something that bothered me for a while. It's not
> 100% on-topic but still quite related so I thought I'd bring it up.
>
> When working on a feature, I usually try to make atomic changes from the
> beginning and use `git commit -a` to commit them one after another. This
> works fine most of the time. Sometimes I notice only after making some
> changes that it might be better to split the working tree changes into
> several commits.
>
> In that case, I git-add the relevant hunks and then, unfortunately, I
> often run `git commit -a` instead of `git commit` (muscle memory bites
> me), so I need to do all the splitting work again.
>
> It's not much of an issue but would it be worthwhile to add an optional
> feature (configurable) that warns you when using --all with staged
> changes (which are not new files)? Are there others having the same
> issue? Do you think this should be implemented as part of an alias
> instead?
I hit the same problem sometimes, but in my case sometimes I
accidentally do "git add" after "git add -p" and a configuration in
"git commit -a" won't help me. I'd prefer we could undo changes in
index instead. Something like reflog but for index.
--
Duy
^ permalink raw reply
* Re: tools for easily "uncommitting" parts of a patch I just commited?
From: Jacob Keller @ 2016-10-23 1:07 UTC (permalink / raw)
To: Lukas Fleischer; +Cc: Git mailing list, Jeff King
In-Reply-To: <147712794056.12237.1478296296628149440@typhoon.fritz.box>
On Sat, Oct 22, 2016 at 2:19 AM, Lukas Fleischer <lfleischer@lfos.de> wrote:
> On Thu, 20 Oct 2016 at 19:27:58, Jacob Keller wrote:
>> [...]
>> I still think we're misunderstanding. I want git commit to complain
>> *only* under the following circumstance:
>>
>> I run "git add -p" and put a partial change into the index in <file>.
>> There are still other parts which were not added to the index yet.
>> Thus, the index version of the file and the actual file differ.
>>
>> Then, I (accidentally) run "git commit <file>"
>> [...]
>
> This reminded me of something that bothered me for a while. It's not
> 100% on-topic but still quite related so I thought I'd bring it up.
>
> When working on a feature, I usually try to make atomic changes from the
> beginning and use `git commit -a` to commit them one after another. This
> works fine most of the time. Sometimes I notice only after making some
> changes that it might be better to split the working tree changes into
> several commits.
>
> In that case, I git-add the relevant hunks and then, unfortunately, I
> often run `git commit -a` instead of `git commit` (muscle memory bites
> me), so I need to do all the splitting work again.
>
> It's not much of an issue but would it be worthwhile to add an optional
> feature (configurable) that warns you when using --all with staged
> changes (which are not new files)? Are there others having the same
> issue? Do you think this should be implemented as part of an alias
> instead?
>
> Regards,
> Lukas
This is (essentially) what I am asking for above. It's the same
overall problem of "muscle memory bites me" and I want the tool to
change to help avoiding because I don't think I can win the fight
against muscle memory every time. Being configurable would ensure that
only those that want the behavior opt in.
Thanks,
Jake
^ permalink raw reply
* [PATCH 13/36] attr: convert git_check_attrs() callers to use the new API
From: Stefan Beller @ 2016-10-22 23:32 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, pclouds, Stefan Beller
In-Reply-To: <20161022233225.8883-1-sbeller@google.com>
From: Junio C Hamano <gitster@pobox.com>
The remaining callers are all simple "I have N attributes I am
interested in. I'll ask about them with various paths one by one".
After this step, no caller to git_check_attrs() remains. After
removing it, we can extend "struct git_attr_check" struct with data
that can be used in optimizing the query for the specific N
attributes it contains.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
builtin/pack-objects.c | 19 +++++--------------
convert.c | 18 +++++++-----------
ll-merge.c | 33 ++++++++++++++-------------------
userdiff.c | 19 ++++++++-----------
ws.c | 19 ++++++-------------
5 files changed, 40 insertions(+), 68 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 3cb38ed..3918c07 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -896,24 +896,15 @@ static void write_pack_file(void)
written, nr_result);
}
-static void setup_delta_attr_check(struct git_attr_check_elem *check)
-{
- static struct git_attr *attr_delta;
-
- if (!attr_delta)
- attr_delta = git_attr("delta");
-
- check[0].attr = attr_delta;
-}
-
static int no_try_delta(const char *path)
{
- struct git_attr_check_elem check[1];
+ static struct git_attr_check *check;
- setup_delta_attr_check(check);
- if (git_check_attrs(path, ARRAY_SIZE(check), check))
+ if (!check)
+ check = git_attr_check_initl("delta", NULL);
+ if (git_check_attr(path, check))
return 0;
- if (ATTR_FALSE(check->value))
+ if (ATTR_FALSE(check->check[0].value))
return 1;
return 0;
}
diff --git a/convert.c b/convert.c
index c95ae71..bb2435a 100644
--- a/convert.c
+++ b/convert.c
@@ -775,24 +775,20 @@ struct conv_attrs {
int ident;
};
-static const char *conv_attr_name[] = {
- "crlf", "ident", "filter", "eol", "text",
-};
-#define NUM_CONV_ATTRS ARRAY_SIZE(conv_attr_name)
-
static void convert_attrs(struct conv_attrs *ca, const char *path)
{
- int i;
- static struct git_attr_check_elem ccheck[NUM_CONV_ATTRS];
+ static struct git_attr_check *check;
- if (!ccheck[0].attr) {
- for (i = 0; i < NUM_CONV_ATTRS; i++)
- ccheck[i].attr = git_attr(conv_attr_name[i]);
+ if (!check) {
+ check = git_attr_check_initl("crlf", "ident",
+ "filter", "eol", "text",
+ NULL);
user_convert_tail = &user_convert;
git_config(read_convert_config, NULL);
}
- if (!git_check_attrs(path, NUM_CONV_ATTRS, ccheck)) {
+ if (!git_check_attr(path, check)) {
+ struct git_attr_check_elem *ccheck = check->check;
ca->crlf_action = git_path_check_crlf(ccheck + 4);
if (ca->crlf_action == CRLF_UNDEFINED)
ca->crlf_action = git_path_check_crlf(ccheck + 0);
diff --git a/ll-merge.c b/ll-merge.c
index eb2c37e..bc6479c 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -336,15 +336,6 @@ static const struct ll_merge_driver *find_ll_merge_driver(const char *merge_attr
return &ll_merge_drv[LL_TEXT_MERGE];
}
-static int git_path_check_merge(const char *path, struct git_attr_check_elem check[2])
-{
- if (!check[0].attr) {
- check[0].attr = git_attr("merge");
- check[1].attr = git_attr("conflict-marker-size");
- }
- return git_check_attrs(path, 2, check);
-}
-
static void normalize_file(mmfile_t *mm, const char *path)
{
struct strbuf strbuf = STRBUF_INIT;
@@ -362,7 +353,7 @@ 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_elem check[2];
+ 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;
@@ -376,10 +367,14 @@ int ll_merge(mmbuffer_t *result_buf,
normalize_file(ours, path);
normalize_file(theirs, path);
}
- if (!git_path_check_merge(path, check)) {
- ll_driver_name = check[0].value;
- if (check[1].value) {
- marker_size = atoi(check[1].value);
+
+ if (!check)
+ check = git_attr_check_initl("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 (marker_size <= 0)
marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
}
@@ -398,13 +393,13 @@ int ll_merge(mmbuffer_t *result_buf,
int ll_merge_marker_size(const char *path)
{
- static struct git_attr_check_elem check;
+ static struct git_attr_check *check;
int marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
- if (!check.attr)
- check.attr = git_attr("conflict-marker-size");
- if (!git_check_attrs(path, 1, &check) && check.value) {
- marker_size = atoi(check.value);
+ 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 (marker_size <= 0)
marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
}
diff --git a/userdiff.c b/userdiff.c
index 4de3289..46dfd32 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -262,25 +262,22 @@ struct userdiff_driver *userdiff_find_by_name(const char *name) {
struct userdiff_driver *userdiff_find_by_path(const char *path)
{
- static struct git_attr *attr;
- struct git_attr_check_elem check;
-
- if (!attr)
- attr = git_attr("diff");
- check.attr = attr;
+ static struct git_attr_check *check;
+ if (!check)
+ check = git_attr_check_initl("diff", NULL);
if (!path)
return NULL;
- if (git_check_attrs(path, 1, &check))
+ if (git_check_attr(path, check))
return NULL;
- if (ATTR_TRUE(check.value))
+ if (ATTR_TRUE(check->check[0].value))
return &driver_true;
- if (ATTR_FALSE(check.value))
+ if (ATTR_FALSE(check->check[0].value))
return &driver_false;
- if (ATTR_UNSET(check.value))
+ if (ATTR_UNSET(check->check[0].value))
return NULL;
- return userdiff_find_by_name(check.value);
+ return userdiff_find_by_name(check->check[0].value);
}
struct userdiff_driver *userdiff_get_textconv(struct userdiff_driver *driver)
diff --git a/ws.c b/ws.c
index 7350905..bb3270c 100644
--- a/ws.c
+++ b/ws.c
@@ -71,24 +71,17 @@ unsigned parse_whitespace_rule(const char *string)
return rule;
}
-static void setup_whitespace_attr_check(struct git_attr_check_elem *check)
-{
- static struct git_attr *attr_whitespace;
-
- if (!attr_whitespace)
- attr_whitespace = git_attr("whitespace");
- check[0].attr = attr_whitespace;
-}
-
unsigned whitespace_rule(const char *pathname)
{
- struct git_attr_check_elem attr_whitespace_rule;
+ static struct git_attr_check *attr_whitespace_rule;
+
+ if (!attr_whitespace_rule)
+ attr_whitespace_rule = git_attr_check_initl("whitespace", NULL);
- setup_whitespace_attr_check(&attr_whitespace_rule);
- if (!git_check_attrs(pathname, 1, &attr_whitespace_rule)) {
+ if (!git_check_attr(pathname, attr_whitespace_rule)) {
const char *value;
- value = attr_whitespace_rule.value;
+ value = attr_whitespace_rule->check[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
* [PATCH 32/36] pathspec: allow querying for attributes
From: Stefan Beller @ 2016-10-22 23:32 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, pclouds, Stefan Beller
In-Reply-To: <20161022233225.8883-1-sbeller@google.com>
The pathspec mechanism is extended via the new
":(attr:eol=input)pattern/to/match" syntax to filter paths so that it
requires paths to not just match the given pattern but also have the
specified attrs attached for them to be chosen.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/glossary-content.txt | 20 +++++
dir.c | 35 ++++++++
pathspec.c | 103 +++++++++++++++++++++-
pathspec.h | 16 ++++
t/t6134-pathspec-with-labels.sh | 170 +++++++++++++++++++++++++++++++++++++
5 files changed, 340 insertions(+), 4 deletions(-)
create mode 100755 t/t6134-pathspec-with-labels.sh
diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt
index 8ad29e6..f90bd45 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -384,6 +384,26 @@ full pathname may have special meaning:
+
Glob magic is incompatible with literal magic.
+attr;;
+After `attr:` comes a space separated list of "attribute
+requirements", all of which must be met in order for the
+path to be considered a match; this is in addition to the
+usual non-magic pathspec pattern matching.
++
+Each of the attribute requirements for the path takes one of
+these forms:
+
+- "`ATTR`" requires that the attribute `ATTR` must be set.
+
+- "`-ATTR`" requires that the attribute `ATTR` must be unset.
+
+- "`ATTR=VALUE`" requires that the attribute `ATTR` must be
+ set to the string `VALUE`.
+
+- "`!ATTR`" requires that the attribute `ATTR` must be
+ unspecified.
++
+
exclude;;
After a path matches any non-exclude pathspec, it will be run
through all exclude pathspec (magic signature: `!`). If it
diff --git a/dir.c b/dir.c
index 3bad1ad..3ec9117 100644
--- a/dir.c
+++ b/dir.c
@@ -9,6 +9,7 @@
*/
#include "cache.h"
#include "dir.h"
+#include "attr.h"
#include "refs.h"
#include "wildmatch.h"
#include "pathspec.h"
@@ -207,6 +208,37 @@ int within_depth(const char *name, int namelen,
return 1;
}
+static int match_attrs(const char *name, int namelen,
+ const struct pathspec_item *item)
+{
+ int i;
+ struct git_attr_result *res = git_attr_result_alloc(item->attr_check);
+
+ git_check_attr(name, item->attr_check, res);
+ for (i = 0; i < item->attr_match_nr; i++) {
+ const char *value;
+ int matched;
+ enum attr_match_mode match_mode;
+
+ value = res[i].value;
+ match_mode = item->attr_match[i].match_mode;
+
+ if (ATTR_TRUE(value))
+ matched = (match_mode == MATCH_SET);
+ else if (ATTR_FALSE(value))
+ matched = (match_mode == MATCH_UNSET);
+ else if (ATTR_UNSET(value))
+ matched = (match_mode == MATCH_UNSPECIFIED);
+ else
+ matched = (match_mode == MATCH_VALUE &&
+ !strcmp(item->attr_match[i].value, value));
+ if (!matched)
+ return 0;
+ }
+
+ return 1;
+}
+
#define DO_MATCH_EXCLUDE 1
#define DO_MATCH_DIRECTORY 2
@@ -262,6 +294,9 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix,
strncmp(item->match, name - prefix, item->prefix))
return 0;
+ if (item->attr_match_nr && !match_attrs(name, namelen, item))
+ return 0;
+
/* If the match was just the prefix, we matched */
if (!*match)
return MATCHED_RECURSIVELY;
diff --git a/pathspec.c b/pathspec.c
index d44f8e7..0eee177 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -1,6 +1,7 @@
#include "cache.h"
#include "dir.h"
#include "pathspec.h"
+#include "attr.h"
/*
* Finds which of the given pathspecs match items in the index.
@@ -88,12 +89,78 @@ static void prefix_short_magic(struct strbuf *sb, int prefixlen,
strbuf_addf(sb, ",prefix:%d)", prefixlen);
}
+static void parse_pathspec_attr_match(struct pathspec_item *item, const char *value)
+{
+ struct string_list_item *si;
+ struct string_list list = STRING_LIST_INIT_DUP;
+
+
+ if (!value || !strlen(value))
+ die(_("attr spec must not be empty"));
+
+ string_list_split(&list, value, ' ', -1);
+ string_list_remove_empty_items(&list, 0);
+
+ if (!item->attr_check)
+ git_attr_check_alloc(&item->attr_check);
+ else
+ die(_("Only one 'attr:' specification is allowed."));
+
+ ALLOC_GROW(item->attr_match, item->attr_match_nr + list.nr, item->attr_match_alloc);
+
+ for_each_string_list_item(si, &list) {
+ size_t attr_len;
+
+ int j = item->attr_match_nr++;
+ const char *attr = si->string;
+ struct attr_match *am = &item->attr_match[j];
+
+ switch (*attr) {
+ case '!':
+ am->match_mode = MATCH_UNSPECIFIED;
+ attr++;
+ attr_len = strlen(attr);
+ break;
+ case '-':
+ am->match_mode = MATCH_UNSET;
+ attr++;
+ attr_len = strlen(attr);
+ break;
+ default:
+ attr_len = strcspn(attr, "=");
+ if (attr[attr_len] != '=')
+ am->match_mode = MATCH_SET;
+ else {
+ am->match_mode = MATCH_VALUE;
+ am->value = xstrdup(&attr[attr_len + 1]);
+ if (strchr(am->value, '\\'))
+ die(_("attr spec values must not contain backslashes"));
+ }
+ break;
+ }
+
+ am->attr = git_attr_counted(attr, attr_len);
+ if (!am->attr) {
+ struct strbuf sb = STRBUF_INIT;
+ am->match_mode = INVALID_ATTR;
+ invalid_attr_name_message(&sb, attr, attr_len);
+ die(_("invalid attribute in '%s': '%s'"), value, sb.buf);
+ }
+
+ git_attr_check_append(item->attr_check, am->attr);
+ }
+
+ string_list_clear(&list, 0);
+ return;
+}
+
static void eat_long_magic(struct pathspec_item *item, const char *elt,
unsigned *magic, int *pathspec_prefix,
const char **copyfrom_, const char **long_magic_end)
{
int i;
const char *copyfrom = *copyfrom_;
+ const char *body;
/* longhand */
const char *nextat;
for (copyfrom = elt + 2;
@@ -108,15 +175,21 @@ static void eat_long_magic(struct pathspec_item *item, const char *elt,
if (!len)
continue;
- if (starts_with(copyfrom, "prefix:")) {
+ if (skip_prefix(copyfrom, "prefix:", &body)) {
char *endptr;
- *pathspec_prefix = strtol(copyfrom + 7,
- &endptr, 10);
+ *pathspec_prefix = strtol(body, &endptr, 10);
if (endptr - copyfrom != len)
die(_("invalid parameter for pathspec magic 'prefix'"));
continue;
}
+ if (skip_prefix(copyfrom, "attr:", &body)) {
+ char *attr_body = xmemdupz(body, len - strlen("attr:"));
+ parse_pathspec_attr_match(item, attr_body);
+ free(attr_body);
+ continue;
+ }
+
for (i = 0; i < ARRAY_SIZE(pathspec_magic); i++) {
if (strlen(pathspec_magic[i].name) == len &&
!strncmp(pathspec_magic[i].name, copyfrom, len)) {
@@ -425,7 +498,10 @@ void parse_pathspec(struct pathspec *pathspec,
for (i = 0; i < n; i++) {
unsigned short_magic;
entry = argv[i];
-
+ item[i].attr_check = NULL;
+ item[i].attr_match = NULL;
+ item[i].attr_match_nr = 0;
+ item[i].attr_match_alloc = 0;
item[i].magic = prefix_pathspec(item + i, &short_magic,
argv + i, flags,
prefix, prefixlen, entry);
@@ -447,6 +523,13 @@ void parse_pathspec(struct pathspec *pathspec,
if (item[i].nowildcard_len < item[i].len)
pathspec->has_wildcard = 1;
pathspec->magic |= item[i].magic;
+
+ if (item[i].attr_match_nr) {
+ int j;
+ for (j = 0; j < item[i].attr_match_nr; j++)
+ if (item[i].attr_match[j].match_mode == INVALID_ATTR)
+ die(_("attribute spec in the wrong syntax are prohibited."));
+ }
}
if (nr_exclude == n)
@@ -500,6 +583,18 @@ void copy_pathspec(struct pathspec *dst, const struct pathspec *src)
void clear_pathspec(struct pathspec *pathspec)
{
+ int i, j;
+ for (i = 0; i < pathspec->nr; i++) {
+ if (!pathspec->items[i].attr_match_nr)
+ continue;
+ for (j = 0; j < pathspec->items[j].attr_match_nr; j++)
+ free(pathspec->items[i].attr_match[j].value);
+ free(pathspec->items[i].attr_match);
+ if (pathspec->items[i].attr_check)
+ git_attr_check_clear(pathspec->items[i].attr_check);
+ free(pathspec->items[i].attr_check);
+ }
+
free(pathspec->items);
pathspec->items = NULL;
}
diff --git a/pathspec.h b/pathspec.h
index 59809e4..aebe6ea 100644
--- a/pathspec.h
+++ b/pathspec.h
@@ -32,6 +32,22 @@ struct pathspec {
int len, prefix;
int nowildcard_len;
int flags;
+ int attr_match_nr;
+ int attr_match_alloc;
+ struct attr_match {
+ struct git_attr *attr;
+ char *value;
+ enum attr_match_mode {
+ MATCH_SET,
+ MATCH_UNSET,
+ MATCH_VALUE,
+ MATCH_UNSPECIFIED,
+ MATCH_NOT_UNSPECIFIED,
+ MATCH_SET_OR_VALUE,
+ INVALID_ATTR
+ } match_mode;
+ } *attr_match;
+ struct git_attr_check *attr_check;
} *items;
};
diff --git a/t/t6134-pathspec-with-labels.sh b/t/t6134-pathspec-with-labels.sh
new file mode 100755
index 0000000..1c9323c
--- /dev/null
+++ b/t/t6134-pathspec-with-labels.sh
@@ -0,0 +1,170 @@
+#!/bin/sh
+
+test_description='test labels in pathspecs'
+. ./test-lib.sh
+
+test_expect_success 'setup a tree' '
+ cat <<-EOF >expect &&
+ fileA
+ fileAB
+ fileAC
+ fileB
+ fileBC
+ fileC
+ fileNoLabel
+ fileSetLabel
+ fileUnsetLabel
+ fileValue
+ fileWrongLabel
+ sub/fileA
+ sub/fileAB
+ sub/fileAC
+ sub/fileB
+ sub/fileBC
+ sub/fileC
+ sub/fileNoLabel
+ sub/fileSetLabel
+ sub/fileUnsetLabel
+ sub/fileValue
+ sub/fileWrongLabel
+ EOF
+ mkdir sub &&
+ while read path
+ do
+ : >$path &&
+ git add $path || return 1
+ done <expect &&
+ git commit -m "initial commit" &&
+ git ls-files >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'pathspec with no attr' '
+ test_must_fail git ls-files ":(attr:)"
+'
+
+test_expect_success 'pathspec with labels and non existent .gitattributes' '
+ git ls-files ":(attr:label)" >actual &&
+ test_must_be_empty actual
+'
+
+test_expect_success 'setup .gitattributes' '
+ cat <<-EOF >.gitattributes &&
+ fileA labelA
+ fileB labelB
+ fileC labelC
+ fileAB labelA labelB
+ fileAC labelA labelC
+ fileBC labelB labelC
+ fileUnsetLabel -label
+ fileSetLabel label
+ fileValue label=foo
+ fileWrongLabel label☺
+ EOF
+ git add .gitattributes &&
+ git commit -m "add attributes"
+'
+
+test_expect_success 'check specific set attr' '
+ cat <<-EOF >expect &&
+ fileSetLabel
+ sub/fileSetLabel
+ EOF
+ git ls-files ":(attr:label)" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'check specific unset attr' '
+ cat <<-EOF >expect &&
+ fileUnsetLabel
+ sub/fileUnsetLabel
+ EOF
+ git ls-files ":(attr:-label)" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'check specific value attr' '
+ cat <<-EOF >expect &&
+ fileValue
+ sub/fileValue
+ EOF
+ git ls-files ":(attr:label=foo)" >actual &&
+ test_cmp expect actual &&
+ git ls-files ":(attr:label=bar)" >actual &&
+ test_must_be_empty actual
+'
+
+test_expect_success 'check unspecified attr' '
+ cat <<-EOF >expect &&
+ .gitattributes
+ fileA
+ fileAB
+ fileAC
+ fileB
+ fileBC
+ fileC
+ fileNoLabel
+ fileWrongLabel
+ sub/fileA
+ sub/fileAB
+ sub/fileAC
+ sub/fileB
+ sub/fileBC
+ sub/fileC
+ sub/fileNoLabel
+ sub/fileWrongLabel
+ EOF
+ git ls-files ":(attr:!label)" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'check multiple unspecified attr' '
+ cat <<-EOF >expect &&
+ .gitattributes
+ fileC
+ fileNoLabel
+ fileWrongLabel
+ sub/fileC
+ sub/fileNoLabel
+ sub/fileWrongLabel
+ EOF
+ git ls-files ":(attr:!labelB !labelA !label)" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'check label with more labels but excluded path' '
+ cat <<-EOF >expect &&
+ fileAB
+ fileB
+ fileBC
+ EOF
+ git ls-files ":(attr:labelB)" ":(exclude)sub/" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'check label excluding other labels' '
+ cat <<-EOF >expect &&
+ fileAB
+ fileB
+ fileBC
+ sub/fileAB
+ sub/fileB
+ EOF
+ git ls-files ":(attr:labelB)" ":(exclude,attr:labelC)sub/" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'checking attributes in a multithreaded process' '
+ git status ":(attr:labelB)"
+'
+
+test_expect_success 'abort on giving invalid label on the command line' '
+ test_must_fail git ls-files . ":(attr:☺)"
+'
+
+test_expect_success 'abort on asking for wrong magic' '
+ test_must_fail git ls-files . ":(attr:-label=foo)" &&
+ test_must_fail git ls-files . ":(attr:!label=foo)"
+'
+
+test_done
--
2.10.1.508.g6572022
^ permalink raw reply related
* [PATCH 33/36] pathspec: allow escaped query values
From: Stefan Beller @ 2016-10-22 23:32 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, pclouds, Stefan Beller
In-Reply-To: <20161022233225.8883-1-sbeller@google.com>
In our own .gitattributes file we have attributes such as:
*.[ch] whitespace=indent,trail,space
When querying for attributes we want to be able to ask for the exact
value, i.e.
git ls-files :(attr:whitespace=indent,trail,space)
should work, but the commas are used in the attr magic to introduce
the next attr, such that this query currently fails with
fatal: Invalid pathspec magic 'trail' in ':(attr:whitespace=indent,trail,space)'
This change allows escaping characters by a backslash, such that the query
git ls-files :(attr:whitespace=indent\,trail\,space)
will match all path that have the value "indent,trail,space" for the
whitespace attribute. To accomplish this, we need to modify two places.
First `eat_long_magic` needs to not stop early upon seeing a comma or
closing paren that is escaped. As a second step we need to remove any
escaping from the attr value.
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
pathspec.c | 53 +++++++++++++++++++++++++++++++++++++----
t/t6134-pathspec-with-labels.sh | 10 ++++++++
2 files changed, 58 insertions(+), 5 deletions(-)
diff --git a/pathspec.c b/pathspec.c
index 0eee177..3832e03 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -89,12 +89,56 @@ static void prefix_short_magic(struct strbuf *sb, int prefixlen,
strbuf_addf(sb, ",prefix:%d)", prefixlen);
}
+static size_t strcspn_escaped(const char *s, const char *stop)
+{
+ const char *i;
+
+ for (i = s; *i; i++) {
+ /* skip the escaped character */
+ if (i[0] == '\\' && i[1]) {
+ i++;
+ continue;
+ }
+
+ if (strchr(stop, *i))
+ break;
+ }
+ return i - s;
+}
+
+static inline int invalid_value_char(const char ch)
+{
+ if (isalnum(ch) || strchr(",-_", ch))
+ return 0;
+ return -1;
+}
+
+static char *attr_value_unescape(const char *value)
+{
+ const char *src;
+ char *dst, *ret;
+
+ ret = xmallocz(strlen(value));
+ for (src = value, dst = ret; *src; src++, dst++) {
+ if (*src == '\\') {
+ if (!src[1])
+ die(_("Escape character '\\' not allowed as "
+ "last character in attr value"));
+ src++;
+ }
+ if (invalid_value_char(*src))
+ die("cannot use '%c' for value matching", *src);
+ *dst = *src;
+ }
+ *dst = '\0';
+ return ret;
+}
+
static void parse_pathspec_attr_match(struct pathspec_item *item, const char *value)
{
struct string_list_item *si;
struct string_list list = STRING_LIST_INIT_DUP;
-
if (!value || !strlen(value))
die(_("attr spec must not be empty"));
@@ -131,10 +175,9 @@ static void parse_pathspec_attr_match(struct pathspec_item *item, const char *va
if (attr[attr_len] != '=')
am->match_mode = MATCH_SET;
else {
+ const char *v = &attr[attr_len + 1];
am->match_mode = MATCH_VALUE;
- am->value = xstrdup(&attr[attr_len + 1]);
- if (strchr(am->value, '\\'))
- die(_("attr spec values must not contain backslashes"));
+ am->value = attr_value_unescape(v);
}
break;
}
@@ -166,7 +209,7 @@ static void eat_long_magic(struct pathspec_item *item, const char *elt,
for (copyfrom = elt + 2;
*copyfrom && *copyfrom != ')';
copyfrom = nextat) {
- size_t len = strcspn(copyfrom, ",)");
+ size_t len = strcspn_escaped(copyfrom, ",)");
if (copyfrom[len] == ',')
nextat = copyfrom + len + 1;
else
diff --git a/t/t6134-pathspec-with-labels.sh b/t/t6134-pathspec-with-labels.sh
index 1c9323c..f5f8413 100755
--- a/t/t6134-pathspec-with-labels.sh
+++ b/t/t6134-pathspec-with-labels.sh
@@ -167,4 +167,14 @@ test_expect_success 'abort on asking for wrong magic' '
test_must_fail git ls-files . ":(attr:!label=foo)"
'
+test_expect_success 'check attribute list' '
+ cat <<-EOF >>.gitattributes &&
+ * whitespace=indent,trail,space
+ EOF
+ cat .gitattributes &&
+ git ls-files ":(attr:whitespace=indent\,trail\,space)" >actual &&
+ git ls-files >expect &&
+ test_cmp expect actual
+'
+
test_done
--
2.10.1.508.g6572022
^ permalink raw reply related
* [PATCH 34/36] submodule update: add `--init-default-path` switch
From: Stefan Beller @ 2016-10-22 23:32 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, pclouds, Stefan Beller
In-Reply-To: <20161022233225.8883-1-sbeller@google.com>
The new switch `--init-default-path` initializes the submodules which are
configured in `submodule.defaultUpdatePath` instead of those given as
command line arguments before updating. In the first implementation this
is made incompatible with further command line arguments as it is
unclear what the user means by
git submodule update --init --init-default-path <paths>
This new switch allows to record more complex patterns as it saves
retyping them whenever you invoke update.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/config.txt | 5 ++++
Documentation/git-submodule.txt | 17 +++++++++----
git-submodule.sh | 21 +++++++++++++---
t/t7400-submodule-basic.sh | 53 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 89 insertions(+), 7 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 27069ac..72901ef 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2886,6 +2886,11 @@ submodule.alternateErrorStrategy
as computed via `submodule.alternateLocation`. Possible values are
`ignore`, `info`, `die`. Default is `die`.
+submodule.defaultUpdatePath::
+ Specifies a set of submodules to initialize when calling
+ `git submodule --init-default-group` by using the pathspec
+ syntax.
+
tag.forceSignAnnotated::
A boolean to specify whether annotated tags created should be GPG signed.
If `--annotate` is specified on the command line, it takes
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index bf3bb37..503fec8 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -14,10 +14,10 @@ SYNOPSIS
'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
'git submodule' [--quiet] init [--] [<path>...]
'git submodule' [--quiet] deinit [-f|--force] (--all|[--] <path>...)
-'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch]
- [--[no-]recommend-shallow] [-f|--force] [--rebase|--merge]
- [--reference <repository>] [--depth <depth>] [--recursive]
- [--jobs <n>] [--] [<path>...]
+'git submodule' [--quiet] update [--init[-default-path]] [--remote] [-N|--no-fetch]
+ [--[no-]recommend-shallow]
+ [-f|--force] [--rebase|--merge] [--reference <repository>]
+ [--depth <depth>] [--recursive] [--jobs <n>] [--] [<path>...]
'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]
[commit] [--] [<path>...]
'git submodule' [--quiet] foreach [--recursive] <command>
@@ -194,6 +194,10 @@ If the submodule is not yet initialized, and you just want to use the
setting as stored in .gitmodules, you can automatically initialize the
submodule with the `--init` option.
+You can configure a set of submodules using pathspec syntax in
+submodule.defaultUpdatePath you can use `--init-default-path` to initialize
+those before updating.
+
If `--recursive` is specified, this command will recurse into the
registered submodules, and update any nested submodules within.
--
@@ -361,6 +365,11 @@ the submodule itself.
Initialize all submodules for which "git submodule init" has not been
called so far before updating.
+--init-default-path::
+ This option is only valid for the update command.
+ Initialize all submodules configured in "`submodule.defaultUpdatePath`"
+ that have not been updated before.
+
--name::
This option is only valid for the add command. It sets the submodule's
name to the given string instead of defaulting to its path. The name
diff --git a/git-submodule.sh b/git-submodule.sh
index a024a13..334cecc 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -9,7 +9,7 @@ USAGE="[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <re
or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
or: $dashless [--quiet] init [--] [<path>...]
or: $dashless [--quiet] deinit [-f|--force] (--all| [--] <path>...)
- or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--checkout|--merge|--rebase] [--[no-]recommend-shallow] [--reference <repository>] [--recursive] [--] [<path>...]
+ or: $dashless [--quiet] update [--init[-default-path]] [--remote] [-N|--no-fetch] [-f|--force] [--checkout|--merge|--rebase] [--[no-]recommend-shallow] [--reference <repository>] [--recursive] [--] [<path>...]
or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
or: $dashless [--quiet] foreach [--recursive] <command>
or: $dashless [--quiet] sync [--recursive] [--] [<path>...]"
@@ -503,7 +503,12 @@ cmd_update()
progress="--progress"
;;
-i|--init)
- init=1
+ test -z $init || test $init = by_args || die "$(gettext "Only one of --init or --init-default-path may be used.")"
+ init=by_args
+ ;;
+ --init-default-path)
+ test -z $init || test $init = by_config || die "$(gettext "Only one of --init or --init-default-path may be used.")"
+ init=by_config
;;
--remote)
remote=1
@@ -572,7 +577,17 @@ cmd_update()
if test -n "$init"
then
- cmd_init "--" "$@" || return
+ if test "$init" = "by_config"
+ then
+ if test $# -gt 0
+ then
+ die "$(gettext "path arguments are incompatible with --init-default-path")"
+ fi
+ cmd_init "--" $(git config --get-all submodule.defaultUpdatePath) || return
+ else
+ cmd_init "--" "$@" || return
+ fi
+
fi
{
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index b77cce8..4699b1c 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -1116,5 +1116,58 @@ test_expect_success 'submodule helper list is not confused by common prefixes' '
test_cmp expect actual
'
+test_expect_success 'setup superproject with submodules' '
+ mkdir sub1 &&
+ (
+ cd sub1 &&
+ git init &&
+ test_commit test &&
+ test_commit test2
+ ) &&
+ mkdir multisuper &&
+ (
+ cd multisuper &&
+ git init &&
+ git submodule add ../sub1 sub0 &&
+ git submodule add ../sub1 sub1 &&
+ git submodule add ../sub1 sub2 &&
+ git submodule add ../sub1 sub3 &&
+ git commit -m "add some submodules"
+ )
+'
+
+cat >expect <<-EOF
+-sub0
+ sub1 (test2)
+ sub2 (test2)
+ sub3 (test2)
+EOF
+
+test_expect_success 'submodule update --init with a specification' '
+ test_when_finished "rm -rf multisuper_clone" &&
+ pwd=$(pwd) &&
+ git clone file://"$pwd"/multisuper multisuper_clone &&
+ (
+ cd multisuper_clone &&
+ git submodule update --init . ":(exclude)sub0" &&
+ git submodule status |cut -c 1,43- >../actual
+ ) &&
+ test_cmp expect actual
+'
+
+test_expect_success 'submodule update --init-default-path' '
+ test_when_finished "rm -rf multisuper_clone" &&
+ pwd=$(pwd) &&
+ git clone file://"$pwd"/multisuper multisuper_clone &&
+ (
+ cd multisuper_clone &&
+ git config submodule.defaultUpdatePath "." &&
+ git config --add submodule.defaultUpdatePath ":(exclude)sub0" &&
+ git submodule update --init-default-path &&
+ git submodule status |cut -c 1,43- >../actual &&
+ test_must_fail git submodule update --init-default-path sub0
+ ) &&
+ test_cmp expect actual
+'
test_done
--
2.10.1.508.g6572022
^ permalink raw reply related
* [PATCH 35/36] clone: add --init-submodule=<pathspec> switch
From: Stefan Beller @ 2016-10-22 23:32 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, pclouds, Stefan Beller
In-Reply-To: <20161022233225.8883-1-sbeller@google.com>
The new switch passes the pathspec to `git submodule update --init`
which is called after the actual clone is done.
Additionally this configures the submodule.defaultUpdatePath to
be the given pathspec, such that any future invocation of
`git submodule update --init-default-paths` will keep up
with the pathspec.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/git-clone.txt | 23 +++++++++----
builtin/clone.c | 36 ++++++++++++++++++--
t/t7400-submodule-basic.sh | 81 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 131 insertions(+), 9 deletions(-)
diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 35cc34b..1089f38 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -15,7 +15,8 @@ SYNOPSIS
[--dissociate] [--separate-git-dir <git dir>]
[--depth <depth>] [--[no-]single-branch]
[--recursive | --recurse-submodules] [--[no-]shallow-submodules]
- [--jobs <n>] [--] <repository> [<directory>]
+ [--init-submodule <pathspec>] [--jobs <n>] [--]
+ <repository> [<directory>]
DESCRIPTION
-----------
@@ -217,12 +218,20 @@ objects from the source repository into a pack in the cloned repository.
--recursive::
--recurse-submodules::
- After the clone is created, initialize all submodules within,
- using their default settings. This is equivalent to running
- `git submodule update --init --recursive` immediately after
- the clone is finished. This option is ignored if the cloned
- repository does not have a worktree/checkout (i.e. if any of
- `--no-checkout`/`-n`, `--bare`, or `--mirror` is given)
+ After the clone is created, initialize and clone all submodules
+ within, using their default settings. This is equivalent to
+ running `git submodule update --recursive --init `
+ immediately after the clone is finished. This option is ignored
+ if the cloned repository does not have a worktree/checkout (i.e.
+ if any of `--no-checkout`/`-n`, `--bare`, or `--mirror` is given)
+
+--init-submodule::
+ After the clone is created, initialize and clone specified
+ submodules within, using their default settings. It is possible
+ to give multiple specifications by giving this argument multiple
+ times. This is equivalent to configure `submodule.defaultUpdateGroup`
+ and then running `git submodule update --init-default-path`
+ immediately after the clone is finished.
--[no-]shallow-submodules::
All submodules which are cloned will be shallow with a depth of 1.
diff --git a/builtin/clone.c b/builtin/clone.c
index 6c76a6e..748e7c0 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -56,6 +56,16 @@ static struct string_list option_required_reference = STRING_LIST_INIT_NODUP;
static struct string_list option_optional_reference = STRING_LIST_INIT_NODUP;
static int option_dissociate;
static int max_jobs = -1;
+static struct string_list init_submodules;
+
+static int init_submodules_cb(const struct option *opt, const char *arg, int unset)
+{
+ if (unset)
+ return -1;
+
+ string_list_append((struct string_list *)opt->value, arg);
+ return 0;
+}
static struct option builtin_clone_options[] = {
OPT__VERBOSITY(&option_verbosity),
@@ -112,6 +122,9 @@ static struct option builtin_clone_options[] = {
TRANSPORT_FAMILY_IPV4),
OPT_SET_INT('6', "ipv6", &family, N_("use IPv6 addresses only"),
TRANSPORT_FAMILY_IPV6),
+ OPT_CALLBACK(0, "init-submodule", &init_submodules, N_("<pathspec>"),
+ N_("clone specific submodules. Pass multiple times for complex pathspecs"),
+ init_submodules_cb),
OPT_END()
};
@@ -733,13 +746,21 @@ static int checkout(int submodule_progress)
err |= run_hook_le(NULL, "post-checkout", sha1_to_hex(null_sha1),
sha1_to_hex(sha1), "1", NULL);
- if (!err && option_recursive) {
+ if (!err && (option_recursive || init_submodules.nr > 0)) {
struct argv_array args = ARGV_ARRAY_INIT;
- argv_array_pushl(&args, "submodule", "update", "--init", "--recursive", NULL);
+ argv_array_pushl(&args, "submodule", "update", NULL);
+
+ if (init_submodules.nr > 0)
+ argv_array_pushf(&args, "--init-default-path");
+ else
+ argv_array_pushf(&args, "--init");
if (option_shallow_submodules == 1)
argv_array_push(&args, "--depth=1");
+ if (option_recursive)
+ argv_array_pushf(&args, "--recursive");
+
if (max_jobs != -1)
argv_array_pushf(&args, "--jobs=%d", max_jobs);
@@ -887,6 +908,17 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
option_no_checkout = 1;
}
+ if (init_submodules.nr > 0) {
+ struct string_list_item *item;
+ struct strbuf sb = STRBUF_INIT;
+ for_each_string_list_item(item, &init_submodules) {
+ strbuf_addf(&sb, "submodule.defaultUpdatePath=%s",
+ item->string);
+ string_list_append(&option_config,
+ strbuf_detach(&sb, NULL));
+ }
+ }
+
if (!option_origin)
option_origin = "origin";
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 4699b1c..90f9030 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -1170,4 +1170,85 @@ test_expect_success 'submodule update --init-default-path' '
test_cmp expect actual
'
+cat <<EOF > expected
+ sub0 (test2)
+-sub1
+-sub2
+-sub3
+EOF
+
+test_expect_success 'clone --init-submodule works' '
+ test_when_finished "rm -rf multisuper_clone" &&
+ git clone --recurse-submodules --init-submodule="sub0" multisuper multisuper_clone &&
+ (
+ cd multisuper_clone &&
+ git submodule status |cut -c 1,43- >../actual
+ ) &&
+ test_cmp actual expected
+'
+
+cat <<EOF > expect
+-sub0
+ sub1 (test2)
+-sub2
+ sub3 (test2)
+EOF
+test_expect_success 'clone with multiple --init-submodule options' '
+ test_when_finished "rm -rf multisuper_clone" &&
+ git clone --recurse-submodules \
+ --init-submodule="." \
+ --init-submodule ":(exclude)sub0" \
+ --init-submodule ":(exclude)sub2" \
+ multisuper multisuper_clone &&
+ (
+ cd multisuper_clone &&
+ git submodule status |cut -c1,43- >../actual
+ ) &&
+ test_cmp expect actual
+'
+
+cat <<EOF > expect
+-sub0
+ sub1 (test2)
+-sub2
+ sub3 (test2)
+EOF
+
+cat <<EOF > expect2
+-sub0
+ sub1 (test2)
+-sub2
+ sub3 (test2)
+-sub4
+ sub5 (test2)
+EOF
+
+test_expect_success 'clone and subsequent updates correctly auto-initialize submodules' '
+ test_when_finished "rm -rf multisuper_clone" &&
+ git clone --recurse-submodules --init-submodule="." \
+ --init-submodule ":(exclude)sub0" \
+ --init-submodule ":(exclude)sub2" \
+ --init-submodule ":(exclude)sub4" \
+ multisuper multisuper_clone &&
+ (
+ cd multisuper_clone &&
+ git submodule status |cut -c1,43- >../actual
+ ) &&
+ test_cmp expect actual &&
+ (
+ cd multisuper &&
+ git submodule add ../sub1 sub4 &&
+ git submodule add ../sub1 sub5 &&
+ git commit -m "add more submodules"
+ ) &&
+ (
+ cd multisuper_clone &&
+ # obtain the new superproject
+ git pull &&
+ git submodule update --init-default-path &&
+ git submodule status |cut -c1,43- >../actual
+ ) &&
+ test_cmp expect2 actual
+'
+
test_done
--
2.10.1.508.g6572022
^ permalink raw reply related
* [PATCH 36/36] completion: clone can initialize specific submodules
From: Stefan Beller @ 2016-10-22 23:32 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, pclouds, Stefan Beller
In-Reply-To: <20161022233225.8883-1-sbeller@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
contrib/completion/git-completion.bash | 1 +
1 file changed, 1 insertion(+)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 21016bf..90eb772 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1138,6 +1138,7 @@ _git_clone ()
--single-branch
--branch
--recurse-submodules
+ --init-submodule
"
return
;;
--
2.10.1.508.g6572022
^ permalink raw reply related
* [PATCH 24/36] attr.c: always pass check[] to collect_some_attrs()
From: Stefan Beller @ 2016-10-22 23:32 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, pclouds, Stefan Beller
In-Reply-To: <20161022233225.8883-1-sbeller@google.com>
From: Junio C Hamano <gitster@pobox.com>
This function used to be called with check=NULL to signal it to
collect all attributes in the global check_all_attr[] array.
Because the longer term plan is to allocate check_all_attr[] and
attr_stack data structures per git_attr_check instance (i.e. "check"
here) to make the attr subsystem thread-safe, it is unacceptable.
Pass "Are we grabbing all attributes defined in the system?" bit as
a separate argument and pass it from the callers.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
attr.c | 37 +++++++++++++++++++------------------
1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/attr.c b/attr.c
index 9f58cc0..673dc7a 100644
--- a/attr.c
+++ b/attr.c
@@ -756,11 +756,12 @@ static void empty_attr_check_elems(struct git_attr_check *check)
/*
* Collect attributes for path into the array pointed to by
- * check_all_attr. If check is not NULL, only attributes in
- * check[] are collected. Otherwise all attributes are collected.
+ * check_all_attr. If collect_all is zero, only attributes in
+ * check[] are collected. Otherwise, check[] is cleared and
+ * 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)
+ struct git_attr_check *check, int collect_all)
{
struct attr_stack *stk;
@@ -781,10 +782,11 @@ static void collect_some_attrs(const char *path, int pathlen,
}
prepare_attr_stack(path, dirlen);
+
for (i = 0; i < attr_nr; i++)
check_all_attr[i].value = ATTR__UNKNOWN;
- if (check && !cannot_trust_maybe_real) {
+ if (!collect_all && !cannot_trust_maybe_real) {
struct git_attr_check_elem *celem = check->check;
rem = 0;
@@ -803,6 +805,17 @@ static void collect_some_attrs(const char *path, int pathlen,
rem = attr_nr;
for (stk = attr_stack; 0 < rem && stk; stk = stk->prev)
rem = fill(path, pathlen, basename_offset, stk, rem);
+
+ if (collect_all) {
+ empty_attr_check_elems(check);
+ 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;
+ }
+ }
}
static int git_check_attrs(const char *path, int pathlen,
@@ -811,7 +824,7 @@ static int git_check_attrs(const char *path, int pathlen,
int i;
struct git_attr_check_elem *celem = check->check;
- collect_some_attrs(path, pathlen, check);
+ collect_some_attrs(path, pathlen, check, 0);
for (i = 0; i < check->check_nr; i++) {
const char *value = check_all_attr[celem[i].attr->attr_nr].value;
@@ -825,19 +838,7 @@ static int git_check_attrs(const char *path, int pathlen,
void git_all_attrs(const char *path, struct git_attr_check *check)
{
- int i;
-
- git_attr_check_clear(check);
- collect_some_attrs(path, strlen(path), NULL);
-
- for (i = 0; i < attr_nr; i++) {
- const char *name = check_all_attr[i].attr->name;
- const char *value = check_all_attr[i].value;
- if (value == ATTR__UNSET || value == ATTR__UNKNOWN)
- continue;
- git_attr_check_append(check, git_attr(name));
- check->check[check->check_nr - 1].value = value;
- }
+ collect_some_attrs(path, strlen(path), check, 1);
}
void git_attr_set_direction(enum git_attr_direction new, struct index_state *istate)
--
2.10.1.508.g6572022
^ permalink raw reply related
* [PATCH 25/36] attr.c: outline the future plans by heavily commenting
From: Stefan Beller @ 2016-10-22 23:32 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, pclouds, Stefan Beller
In-Reply-To: <20161022233225.8883-1-sbeller@google.com>
From: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
attr.c | 40 +++++++++++++++++++++++++++++++++++++++-
1 file changed, 39 insertions(+), 1 deletion(-)
diff --git a/attr.c b/attr.c
index 673dc7a..0f08ee6 100644
--- a/attr.c
+++ b/attr.c
@@ -30,6 +30,11 @@ static const char git_attr__unknown[] = "(builtin)unknown";
#define DEBUG_ATTR 0
#endif
+/*
+ * NEEDSWORK: the global dictionary of the interned attributes
+ * must stay a singleton even after we become thread-ready.
+ * Access to these must be surrounded with mutex when it happens.
+ */
struct git_attr {
struct git_attr *next;
unsigned h;
@@ -39,10 +44,19 @@ struct git_attr {
char name[FLEX_ARRAY];
};
static int attr_nr;
+static struct git_attr *(git_attr_hash[HASHSIZE]);
+
+/*
+ * NEEDSWORK: maybe-real, maybe-macro are not property of
+ * an attribute, as it depends on what .gitattributes are
+ * read. Once we introduce per git_attr_check attr_stack
+ * and check_all_attr, the optimization based on them will
+ * become unnecessary and can go away. So is this variable.
+ */
static int cannot_trust_maybe_real;
+/* NEEDSWORK: This will become per git_attr_check */
static struct git_attr_check_elem *check_all_attr;
-static struct git_attr *(git_attr_hash[HASHSIZE]);
const char *git_attr_name(const struct git_attr *attr)
{
@@ -117,6 +131,11 @@ struct git_attr *git_attr_counted(const char *name, size_t len)
a->maybe_real = 0;
git_attr_hash[pos] = a;
+ /*
+ * NEEDSWORK: per git_attr_check check_all_attr
+ * will be initialized a lot more lazily, not
+ * like this, and not here.
+ */
REALLOC_ARRAY(check_all_attr, attr_nr);
check_all_attr[a->attr_nr].attr = a;
check_all_attr[a->attr_nr].value = ATTR__UNKNOWN;
@@ -329,6 +348,7 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
* .gitignore file and info/excludes file as a fallback.
*/
+/* NEEDSWORK: This will become per git_attr_check */
static struct attr_stack {
struct attr_stack *prev;
char *origin;
@@ -393,6 +413,24 @@ static struct attr_stack *read_attr_from_array(const char **list)
return res;
}
+/*
+ * NEEDSWORK: these two are tricky. The callers assume there is a
+ * single, system-wide global state "where we read attributes from?"
+ * and when the state is flipped by calling git_attr_set_direction(),
+ * attr_stack is discarded so that subsequent attr_check will lazily
+ * read from the right place. And they do not know or care who called
+ * by them uses the attribute subsystem, hence have no knowledge of
+ * existing git_attr_check instances or future ones that will be
+ * created).
+ *
+ * Probably we need a thread_local that holds these two variables,
+ * and a list of git_attr_check instances (which need to be maintained
+ * by hooking into git_attr_check_alloc(), git_attr_check_initl(), and
+ * git_attr_check_clear(). Then git_attr_set_direction() updates the
+ * fields in that thread_local for these two variables, iterate over
+ * all the active git_attr_check instances and discard the attr_stack
+ * they hold. Yuck, but it sounds doable.
+ */
static enum git_attr_direction direction;
static struct index_state *use_index;
--
2.10.1.508.g6572022
^ permalink raw reply related
* [PATCH 28/36] attr: keep attr stack for each check
From: Stefan Beller @ 2016-10-22 23:32 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, pclouds, Stefan Beller
In-Reply-To: <20161022233225.8883-1-sbeller@google.com>
Instead of having a global attr stack, attach the stack to each check.
This allows to use the attr in a multithreaded way.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
attr.c | 101 +++++++++++++++++++++++++++++++++++++++-----------------------
attr.h | 4 ++-
hashmap.h | 2 ++
3 files changed, 69 insertions(+), 38 deletions(-)
diff --git a/attr.c b/attr.c
index 89ae155..b65437d 100644
--- a/attr.c
+++ b/attr.c
@@ -372,15 +372,17 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
* .gitignore file and info/excludes file as a fallback.
*/
-/* NEEDSWORK: This will become per git_attr_check */
-static struct attr_stack {
+struct attr_stack {
struct attr_stack *prev;
char *origin;
size_t originlen;
unsigned num_matches;
unsigned alloc;
struct match_attr **attrs;
-} *attr_stack;
+};
+
+struct hashmap all_attr_stacks;
+int all_attr_stacks_init;
static void free_attr_elem(struct attr_stack *e)
{
@@ -561,11 +563,23 @@ static void debug_set(const char *what, const char *match, struct git_attr *attr
static void drop_attr_stack(void)
{
- while (attr_stack) {
- struct attr_stack *elem = attr_stack;
- attr_stack = elem->prev;
- free_attr_elem(elem);
+ struct hashmap_iter iter;
+ struct git_attr_check *check;
+
+ attr_lock();
+ if (!all_attr_stacks_init) {
+ attr_unlock();
+ return;
}
+ hashmap_iter_init(&all_attr_stacks, &iter);
+ while ((check = hashmap_iter_next(&iter))) {
+ while (check->attr_stack) {
+ struct attr_stack *elem = check->attr_stack;
+ check->attr_stack = elem->prev;
+ free_attr_elem(elem);
+ }
+ }
+ attr_unlock();
}
static const char *git_etc_gitattributes(void)
@@ -595,40 +609,42 @@ static void push_stack(struct attr_stack **attr_stack_p,
}
}
-static void bootstrap_attr_stack(void)
+static void bootstrap_attr_stack(struct git_attr_check *check)
{
struct attr_stack *elem;
- if (attr_stack)
+ if (check->attr_stack)
return;
- push_stack(&attr_stack, read_attr_from_array(builtin_attr), NULL, 0);
+ push_stack(&check->attr_stack,
+ read_attr_from_array(builtin_attr), NULL, 0);
if (git_attr_system())
- push_stack(&attr_stack,
+ push_stack(&check->attr_stack,
read_attr_from_file(git_etc_gitattributes(), 1),
NULL, 0);
if (!git_attributes_file)
git_attributes_file = xdg_config_home("attributes");
if (git_attributes_file)
- push_stack(&attr_stack,
+ push_stack(&check->attr_stack,
read_attr_from_file(git_attributes_file, 1),
NULL, 0);
if (!is_bare_repository() || direction == GIT_ATTR_INDEX) {
elem = read_attr(GITATTRIBUTES_FILE, 1);
- push_stack(&attr_stack, elem, xstrdup(""), 0);
+ push_stack(&check->attr_stack, elem, xstrdup(""), 0);
debug_push(elem);
}
elem = read_attr_from_file(git_path_info_attributes(), 1);
if (!elem)
elem = xcalloc(1, sizeof(*elem));
- push_stack(&attr_stack, elem, NULL, 0);
+ push_stack(&check->attr_stack, elem, NULL, 0);
}
-static void prepare_attr_stack(const char *path, int dirlen)
+static void prepare_attr_stack(const char *path, int dirlen,
+ struct git_attr_check *check)
{
struct attr_stack *elem, *info;
const char *cp;
@@ -648,13 +664,13 @@ static void prepare_attr_stack(const char *path, int dirlen)
* .gitattributes in deeper directories to shallower ones,
* and finally use the built-in set as the default.
*/
- bootstrap_attr_stack();
+ bootstrap_attr_stack(check);
/*
* Pop the "info" one that is always at the top of the stack.
*/
- info = attr_stack;
- attr_stack = info->prev;
+ info = check->attr_stack;
+ check->attr_stack = info->prev;
/*
* Pop the ones from directories that are not the prefix of
@@ -662,17 +678,17 @@ static void prepare_attr_stack(const char *path, int dirlen)
* the root one (whose origin is an empty string "") or the builtin
* one (whose origin is NULL) without popping it.
*/
- while (attr_stack->origin) {
- int namelen = strlen(attr_stack->origin);
+ while (check->attr_stack->origin) {
+ int namelen = strlen(check->attr_stack->origin);
- elem = attr_stack;
+ elem = check->attr_stack;
if (namelen <= dirlen &&
!strncmp(elem->origin, path, namelen) &&
(!namelen || path[namelen] == '/'))
break;
debug_pop(elem);
- attr_stack = elem->prev;
+ check->attr_stack = elem->prev;
free_attr_elem(elem);
}
@@ -688,9 +704,9 @@ static void prepare_attr_stack(const char *path, int dirlen)
*/
struct strbuf pathbuf = STRBUF_INIT;
- assert(attr_stack->origin);
+ assert(check->attr_stack->origin);
while (1) {
- size_t len = strlen(attr_stack->origin);
+ size_t len = strlen(check->attr_stack->origin);
char *origin;
if (dirlen <= len)
@@ -704,7 +720,7 @@ static void prepare_attr_stack(const char *path, int dirlen)
elem = read_attr(pathbuf.buf, 0);
strbuf_setlen(&pathbuf, cp - path);
origin = strbuf_detach(&pathbuf, &len);
- push_stack(&attr_stack, elem, origin, len);
+ push_stack(&check->attr_stack, elem, origin, len);
debug_push(elem);
}
@@ -714,7 +730,13 @@ static void prepare_attr_stack(const char *path, int dirlen)
/*
* Finally push the "info" one at the top of the stack.
*/
- push_stack(&attr_stack, info, NULL, 0);
+ push_stack(&check->attr_stack, info, NULL, 0);
+ if (!all_attr_stacks_init) {
+ hashmap_init(&all_attr_stacks, NULL, 0);
+ all_attr_stacks_init = 1;
+ }
+ if (!hashmap_get(&all_attr_stacks, check, NULL))
+ hashmap_put(&all_attr_stacks, check);
}
static int path_matches(const char *pathname, int pathlen,
@@ -740,9 +762,10 @@ static int path_matches(const char *pathname, int pathlen,
pattern, prefix, pat->patternlen, pat->flags);
}
-static int macroexpand_one(int attr_nr, int rem);
+static int macroexpand_one(int attr_nr, int rem, struct git_attr_check *check);
-static int fill_one(const char *what, struct match_attr *a, int rem)
+static int fill_one(const char *what, struct match_attr *a, int rem,
+ struct git_attr_check *check)
{
struct git_attr_check_elem *celem = check_all_attr;
int i;
@@ -758,14 +781,14 @@ static int fill_one(const char *what, struct match_attr *a, int rem)
attr, v);
*n = v;
rem--;
- rem = macroexpand_one(attr->attr_nr, rem);
+ rem = macroexpand_one(attr->attr_nr, rem, check);
}
}
return rem;
}
static int fill(const char *path, int pathlen, int basename_offset,
- struct attr_stack *stk, int rem)
+ struct attr_stack *stk, int rem, struct git_attr_check *check)
{
int i;
const char *base = stk->origin ? stk->origin : "";
@@ -776,12 +799,12 @@ static int fill(const char *path, int pathlen, int basename_offset,
continue;
if (path_matches(path, pathlen, basename_offset,
&a->u.pat, base, stk->originlen))
- rem = fill_one("fill", a, rem);
+ rem = fill_one("fill", a, rem, check);
}
return rem;
}
-static int macroexpand_one(int nr, int rem)
+static int macroexpand_one(int nr, int rem, struct git_attr_check *check)
{
struct attr_stack *stk;
int i;
@@ -790,13 +813,13 @@ static int macroexpand_one(int nr, int rem)
!check_all_attr[nr].attr->maybe_macro)
return rem;
- for (stk = attr_stack; stk; stk = stk->prev) {
+ for (stk = check->attr_stack; stk; stk = stk->prev) {
for (i = stk->num_matches - 1; 0 <= i; i--) {
struct match_attr *ma = stk->attrs[i];
if (!ma->is_macro)
continue;
if (ma->u.attr->attr_nr == nr)
- return fill_one("expand", ma, rem);
+ return fill_one("expand", ma, rem, check);
}
}
@@ -845,7 +868,7 @@ static void collect_some_attrs(const char *path, int pathlen,
dirlen = 0;
}
- prepare_attr_stack(path, dirlen);
+ prepare_attr_stack(path, dirlen, check);
for (i = 0; i < attr_nr; i++)
check_all_attr[i].value = ATTR__UNKNOWN;
@@ -865,8 +888,8 @@ static void collect_some_attrs(const char *path, int pathlen,
}
rem = attr_nr;
- for (stk = attr_stack; 0 < rem && stk; stk = stk->prev)
- rem = fill(path, pathlen, basename_offset, stk, rem);
+ for (stk = check->attr_stack; 0 < rem && stk; stk = stk->prev)
+ rem = fill(path, pathlen, basename_offset, stk, rem, check);
if (collect_all) {
int check_nr = 0, check_alloc = 0;
@@ -898,6 +921,8 @@ static int git_check_attrs(const char *path, int pathlen,
{
int i;
+ attr_lock();
+
collect_some_attrs(path, pathlen, check, &result, 0);
for (i = 0; i < check->check_nr; i++) {
@@ -907,6 +932,8 @@ static int git_check_attrs(const char *path, int pathlen,
result[i].value = value;
}
+ attr_unlock();
+
return 0;
}
diff --git a/attr.h b/attr.h
index 219b8c7..60d90f2 100644
--- a/attr.h
+++ b/attr.h
@@ -32,12 +32,14 @@ extern const char git_attr__false[];
#define ATTR_UNSET(v) ((v) == NULL)
struct git_attr_check {
+ struct hashmap_entry entry;
int finalized;
int check_nr;
int check_alloc;
const struct git_attr **attr;
+ struct attr_stack *attr_stack;
};
-#define GIT_ATTR_CHECK_INIT {0, 0, 0, NULL}
+#define GIT_ATTR_CHECK_INIT {HASHMAP_ENTRY_INIT, 0, 0, 0, NULL, NULL}
struct git_attr_result {
const char *value;
diff --git a/hashmap.h b/hashmap.h
index ab7958a..d247c62 100644
--- a/hashmap.h
+++ b/hashmap.h
@@ -31,6 +31,8 @@ struct hashmap_entry {
unsigned int hash;
};
+#define HASHMAP_ENTRY_INIT {NULL, 0}
+
typedef int (*hashmap_cmp_fn)(const void *entry, const void *entry_or_key,
const void *keydata);
--
2.10.1.508.g6572022
^ permalink raw reply related
* [PATCH 29/36] Documentation: fix a typo
From: Stefan Beller @ 2016-10-22 23:32 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, pclouds, Stefan Beller
In-Reply-To: <20161022233225.8883-1-sbeller@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/gitattributes.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 8a061af..5b31797 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -88,7 +88,7 @@ is either not set or empty, $HOME/.config/git/attributes is used instead.
Attributes for all users on a system should be placed in the
`$(prefix)/etc/gitattributes` file.
-Sometimes you would need to override an setting of an attribute
+Sometimes you would need to override a setting of an attribute
for a path to `Unspecified` state. This can be done by listing
the name of the attribute prefixed with an exclamation point `!`.
--
2.10.1.508.g6572022
^ permalink raw reply related
* [PATCH 31/36] pathspec: move prefix check out of the inner loop
From: Stefan Beller @ 2016-10-22 23:32 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, pclouds, Stefan Beller
In-Reply-To: <20161022233225.8883-1-sbeller@google.com>
The prefix check is not related the check of pathspec magic; also there
is no code that is relevant after we'd break the loop on a match for
"prefix:". So move the check before the loop and shortcircuit the outer
loop.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
pathspec.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/pathspec.c b/pathspec.c
index 67678fc..d44f8e7 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -107,21 +107,22 @@ static void eat_long_magic(struct pathspec_item *item, const char *elt,
nextat = copyfrom + len;
if (!len)
continue;
+
+ if (starts_with(copyfrom, "prefix:")) {
+ char *endptr;
+ *pathspec_prefix = strtol(copyfrom + 7,
+ &endptr, 10);
+ if (endptr - copyfrom != len)
+ die(_("invalid parameter for pathspec magic 'prefix'"));
+ continue;
+ }
+
for (i = 0; i < ARRAY_SIZE(pathspec_magic); i++) {
if (strlen(pathspec_magic[i].name) == len &&
!strncmp(pathspec_magic[i].name, copyfrom, len)) {
*magic |= pathspec_magic[i].bit;
break;
}
- if (starts_with(copyfrom, "prefix:")) {
- char *endptr;
- *pathspec_prefix = strtol(copyfrom + 7,
- &endptr, 10);
- if (endptr - copyfrom != len)
- die(_("invalid parameter for pathspec magic 'prefix'"));
- /* "i" would be wrong, but it does not matter */
- break;
- }
}
if (ARRAY_SIZE(pathspec_magic) <= i)
die(_("Invalid pathspec magic '%.*s' in '%s'"),
--
2.10.1.508.g6572022
^ permalink raw reply related
* [PATCH 30/36] pathspec: move long magic parsing out of prefix_pathspec
From: Stefan Beller @ 2016-10-22 23:32 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, pclouds, Stefan Beller
In-Reply-To: <20161022233225.8883-1-sbeller@google.com>
`prefix_pathspec` is quite a lengthy function and we plan on adding more.
Split it up for better readability. As we want to add code into the
inner loop of the long magic parsing, we also benefit from lower
indentation.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
pathspec.c | 84 +++++++++++++++++++++++++++++++++++---------------------------
1 file changed, 47 insertions(+), 37 deletions(-)
diff --git a/pathspec.c b/pathspec.c
index 86f2b44..67678fc 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -88,6 +88,52 @@ static void prefix_short_magic(struct strbuf *sb, int prefixlen,
strbuf_addf(sb, ",prefix:%d)", prefixlen);
}
+static void eat_long_magic(struct pathspec_item *item, const char *elt,
+ unsigned *magic, int *pathspec_prefix,
+ const char **copyfrom_, const char **long_magic_end)
+{
+ int i;
+ const char *copyfrom = *copyfrom_;
+ /* longhand */
+ const char *nextat;
+ for (copyfrom = elt + 2;
+ *copyfrom && *copyfrom != ')';
+ copyfrom = nextat) {
+ size_t len = strcspn(copyfrom, ",)");
+ if (copyfrom[len] == ',')
+ nextat = copyfrom + len + 1;
+ else
+ /* handle ')' and '\0' */
+ nextat = copyfrom + len;
+ if (!len)
+ continue;
+ for (i = 0; i < ARRAY_SIZE(pathspec_magic); i++) {
+ if (strlen(pathspec_magic[i].name) == len &&
+ !strncmp(pathspec_magic[i].name, copyfrom, len)) {
+ *magic |= pathspec_magic[i].bit;
+ break;
+ }
+ if (starts_with(copyfrom, "prefix:")) {
+ char *endptr;
+ *pathspec_prefix = strtol(copyfrom + 7,
+ &endptr, 10);
+ if (endptr - copyfrom != len)
+ die(_("invalid parameter for pathspec magic 'prefix'"));
+ /* "i" would be wrong, but it does not matter */
+ break;
+ }
+ }
+ if (ARRAY_SIZE(pathspec_magic) <= i)
+ die(_("Invalid pathspec magic '%.*s' in '%s'"),
+ (int) len, copyfrom, elt);
+ }
+ if (*copyfrom != ')')
+ die(_("Missing ')' at the end of pathspec magic in '%s'"), elt);
+ *long_magic_end = copyfrom;
+ copyfrom++;
+ *copyfrom_ = copyfrom;
+}
+
/*
* Take an element of a pathspec and check for magic signatures.
* Append the result to the prefix. Return the magic bitmap.
@@ -150,43 +196,7 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
(flags & PATHSPEC_LITERAL_PATH)) {
; /* nothing to do */
} else if (elt[1] == '(') {
- /* longhand */
- const char *nextat;
- for (copyfrom = elt + 2;
- *copyfrom && *copyfrom != ')';
- copyfrom = nextat) {
- size_t len = strcspn(copyfrom, ",)");
- if (copyfrom[len] == ',')
- nextat = copyfrom + len + 1;
- else
- /* handle ')' and '\0' */
- nextat = copyfrom + len;
- if (!len)
- continue;
- for (i = 0; i < ARRAY_SIZE(pathspec_magic); i++) {
- if (strlen(pathspec_magic[i].name) == len &&
- !strncmp(pathspec_magic[i].name, copyfrom, len)) {
- magic |= pathspec_magic[i].bit;
- break;
- }
- if (starts_with(copyfrom, "prefix:")) {
- char *endptr;
- pathspec_prefix = strtol(copyfrom + 7,
- &endptr, 10);
- if (endptr - copyfrom != len)
- die(_("invalid parameter for pathspec magic 'prefix'"));
- /* "i" would be wrong, but it does not matter */
- break;
- }
- }
- if (ARRAY_SIZE(pathspec_magic) <= i)
- die(_("Invalid pathspec magic '%.*s' in '%s'"),
- (int) len, copyfrom, elt);
- }
- if (*copyfrom != ')')
- die(_("Missing ')' at the end of pathspec magic in '%s'"), elt);
- long_magic_end = copyfrom;
- copyfrom++;
+ eat_long_magic(item, elt, &magic, &pathspec_prefix, ©from, &long_magic_end);
} else {
/* shorthand */
for (copyfrom = elt + 1;
--
2.10.1.508.g6572022
^ permalink raw reply related
* [PATCH 26/36] attr: make git_check_attr_counted static
From: Stefan Beller @ 2016-10-22 23:32 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, pclouds, Stefan Beller
In-Reply-To: <20161022233225.8883-1-sbeller@google.com>
It's not used outside the attr code, so let's keep it private.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
attr.c | 4 ++--
attr.h | 1 -
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/attr.c b/attr.c
index 0f08ee6..881bdfa 100644
--- a/attr.c
+++ b/attr.c
@@ -892,8 +892,8 @@ void git_attr_set_direction(enum git_attr_direction new, struct index_state *ist
use_index = istate;
}
-int git_check_attr_counted(const char *path, int pathlen,
- struct git_attr_check *check)
+static int git_check_attr_counted(const char *path, int pathlen,
+ struct git_attr_check *check)
{
check->finalized = 1;
return git_check_attrs(path, pathlen, check);
diff --git a/attr.h b/attr.h
index 40abc16..06ac93b 100644
--- a/attr.h
+++ b/attr.h
@@ -44,7 +44,6 @@ struct git_attr_check {
extern struct git_attr_check *git_attr_check_initl(const char *, ...);
extern int git_check_attr(const char *path, struct git_attr_check *);
-extern int git_check_attr_counted(const char *, int, struct git_attr_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 *);
--
2.10.1.508.g6572022
^ permalink raw reply related
* [PATCH 27/36] attr: convert to new threadsafe API
From: Stefan Beller @ 2016-10-22 23:32 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, pclouds, Stefan Beller
In-Reply-To: <20161022233225.8883-1-sbeller@google.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);
/*
* Obtain a pointer to a correctly sized result
* statically allocated on the stack; this macro:
*/
GIT_ATTR_RESULT_INIT_FOR(myresult, 1);
/* Perform the check and act on it: */
git_check_attr(path, check, myresult);
act_on(myresult->value[0]);
/*
* 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>
---
Documentation/technical/api-gitattributes.txt | 91 +++++++++-------
archive.c | 14 +--
attr.c | 143 ++++++++++++++++++--------
attr.h | 71 ++++++++-----
builtin/check-attr.c | 35 ++++---
builtin/pack-objects.c | 16 +--
convert.c | 39 +++----
ll-merge.c | 25 +++--
userdiff.c | 16 +--
ws.c | 17 ++-
10 files changed, 279 insertions(+), 188 deletions(-)
diff --git a/Documentation/technical/api-gitattributes.txt b/Documentation/technical/api-gitattributes.txt
index 92fc32a..f3fc7bd 100644
--- a/Documentation/technical/api-gitattributes.txt
+++ b/Documentation/technical/api-gitattributes.txt
@@ -16,15 +16,17 @@ 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 a collection of results to its
+ corresponding `struct git_attr_check`, that has the same order.
Attribute Values
@@ -32,7 +34,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 +55,31 @@ 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.
+ `git_attr_check_initl()` is 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.
+
+* Prepare a `struct git_attr_result` using `GIT_ATTR_RESULT_INIT_FOR()`
+ for the result for static allocations. When the result size is not known
+ at compile time, use `git_attr_result_alloc`. 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 the returned `git_attr_result` structure to see how
+ each of the attribute in the array is defined for the path.
Example
-------
@@ -76,28 +90,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;
+ GIT_ATTR_RESULT_INIT_FOR(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 +132,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 +150,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..e027b3c 100644
--- a/archive.c
+++ b/archive.c
@@ -107,12 +107,14 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,
void *context)
{
static struct strbuf path = STRBUF_INIT;
+ static struct git_attr_check *check;
+
struct archiver_context *c = context;
struct archiver_args *args = c->args;
write_archive_entry_fn_t write_entry = c->write_entry;
- static struct git_attr_check *check;
const char *path_without_prefix;
int err;
+ struct git_attr_result result[2];
args->convert = 0;
strbuf_reset(&path);
@@ -124,12 +126,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 881bdfa..89ae155 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;
+#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 06ac93b..219b8c7 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 int attr_name_valid(const char *name, size_t namelen);
extern void invalid_attr_name_message(struct strbuf *, const char *, int);
@@ -25,44 +31,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..0d55742 100644
--- a/convert.c
+++ b/convert.c
@@ -718,10 +718,8 @@ 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(const char *value)
{
- const char *value = check->value;
-
if (ATTR_TRUE(value))
return CRLF_TEXT;
else if (ATTR_FALSE(value))
@@ -735,10 +733,8 @@ 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(const char *value)
{
- const char *value = check->value;
-
if (ATTR_UNSET(value))
;
else if (!strcmp(value, "lf"))
@@ -748,9 +744,8 @@ 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(const char *value)
{
- const char *value = check->value;
struct convert_driver *drv;
if (ATTR_TRUE(value) || ATTR_FALSE(value) || ATTR_UNSET(value))
@@ -761,10 +756,8 @@ 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(const char *value)
{
- const char *value = check->value;
-
return !!ATTR_TRUE(value);
}
@@ -778,25 +771,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];
+
+ git_attr_check_initl(&check, "crlf", "ident", "filter",
+ "eol", "text", NULL);
- if (!check) {
- check = git_attr_check_initl("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].value);
if (ca->crlf_action == CRLF_UNDEFINED)
- ca->crlf_action = git_path_check_crlf(ccheck + 0);
+ ca->crlf_action = git_path_check_crlf(result[0].value);
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].value);
+ ca->drv = git_path_check_convert(result[2].value);
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].value);
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..2c9b684 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,13 +396,15 @@ 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];
- 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);
+ git_attr_check_initl(&check, "conflict-marker-size", NULL);
+
+ 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;
}
+
return 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..59fb501 100644
--- a/ws.c
+++ b/ws.c
@@ -74,15 +74,12 @@ 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)) {
- const char *value;
-
- value = attr_whitespace_rule->check[0].value;
- if (ATTR_TRUE(value)) {
+ if (!git_check_attr(pathname, attr_whitespace_rule, result)) {
+ if (ATTR_TRUE(result[0].value)) {
/* true (whitespace) */
unsigned all_rule = ws_tab_width(whitespace_rule_cfg);
int i;
@@ -91,15 +88,15 @@ unsigned whitespace_rule(const char *pathname)
!whitespace_rule_names[i].exclude_default)
all_rule |= whitespace_rule_names[i].rule_bits;
return all_rule;
- } else if (ATTR_FALSE(value)) {
+ } else if (ATTR_FALSE(result[0].value)) {
/* false (-whitespace) */
return ws_tab_width(whitespace_rule_cfg);
- } else if (ATTR_UNSET(value)) {
+ } else if (ATTR_UNSET(result[0].value)) {
/* reset to default (!whitespace) */
return whitespace_rule_cfg;
} else {
/* string */
- return parse_whitespace_rule(value);
+ return parse_whitespace_rule(result[0].value);
}
} else {
return whitespace_rule_cfg;
--
2.10.1.508.g6572022
^ permalink raw reply related
* [PATCH 20/36] attr.c: pass struct git_attr_check down the callchain
From: Stefan Beller @ 2016-10-22 23:32 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, pclouds, Stefan Beller
In-Reply-To: <20161022233225.8883-1-sbeller@google.com>
From: Junio C Hamano <gitster@pobox.com>
The callchain that starts from git_check_attrs() down to
collect_some_attrs() used to take an array of git_attr_check_elem
as their parameters. Pass the enclosing git_attr_check instance
instead, so that they will have access to new fields we will add to
the data structure.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
attr.c | 36 ++++++++++++++++++++++++------------
1 file changed, 24 insertions(+), 12 deletions(-)
diff --git a/attr.c b/attr.c
index 34c297d..9ed4825 100644
--- a/attr.c
+++ b/attr.c
@@ -751,14 +751,25 @@ static int attr_check_is_dynamic(const struct git_attr_check *check)
* check_all_attr. If num is non-zero, only attributes in check[] are
* collected. Otherwise all attributes are collected.
*/
-static void collect_some_attrs(const char *path, int pathlen, int num,
- struct git_attr_check_elem *check)
+static void collect_some_attrs(const char *path, int pathlen,
+ struct git_attr_check *check)
{
struct attr_stack *stk;
int i, rem, dirlen;
const char *cp, *last_slash = NULL;
int basename_offset;
+ int num;
+ struct git_attr_check_elem *celem;
+
+ if (!check) {
+ /* Yuck - ugly git_all_attrs() hack! */
+ celem = NULL;
+ num = 0;
+ } else {
+ celem = check->check;
+ num = check->check_nr;
+ }
for (cp = path; cp < path + pathlen; cp++) {
if (*cp == '/' && cp[1])
@@ -778,9 +789,9 @@ static void collect_some_attrs(const char *path, int pathlen, int num,
if (num && !cannot_trust_maybe_real) {
rem = 0;
for (i = 0; i < num; i++) {
- if (!check[i].attr->maybe_real) {
+ if (!celem[i].attr->maybe_real) {
struct git_attr_check_elem *c;
- c = check_all_attr + check[i].attr->attr_nr;
+ c = check_all_attr + celem[i].attr->attr_nr;
c->value = ATTR__UNSET;
rem++;
}
@@ -794,18 +805,19 @@ static void collect_some_attrs(const char *path, int pathlen, int num,
rem = fill(path, pathlen, basename_offset, stk, rem);
}
-static int git_check_attrs(const char *path, int pathlen, int num,
- struct git_attr_check_elem *check)
+static int git_check_attrs(const char *path, int pathlen,
+ struct git_attr_check *check)
{
int i;
+ struct git_attr_check_elem *celem = check->check;
- collect_some_attrs(path, pathlen, num, check);
+ collect_some_attrs(path, pathlen, check);
- for (i = 0; i < num; i++) {
- const char *value = check_all_attr[check[i].attr->attr_nr].value;
+ for (i = 0; i < check->check_nr; i++) {
+ const char *value = check_all_attr[celem[i].attr->attr_nr].value;
if (value == ATTR__UNKNOWN)
value = ATTR__UNSET;
- check[i].value = value;
+ celem[i].value = value;
}
return 0;
@@ -816,7 +828,7 @@ void git_all_attrs(const char *path, struct git_attr_check *check)
int i;
git_attr_check_clear(check);
- collect_some_attrs(path, strlen(path), 0, NULL);
+ collect_some_attrs(path, strlen(path), NULL);
for (i = 0; i < attr_nr; i++) {
const char *name = check_all_attr[i].attr->name;
@@ -845,7 +857,7 @@ int git_check_attr_counted(const char *path, int pathlen,
struct git_attr_check *check)
{
check->finalized = 1;
- return git_check_attrs(path, pathlen, check->check_nr, check->check);
+ return git_check_attrs(path, pathlen, check);
}
int git_check_attr(const char *path, struct git_attr_check *check)
--
2.10.1.508.g6572022
^ permalink raw reply related
* [PATCH 23/36] attr.c: introduce empty_attr_check_elems()
From: Stefan Beller @ 2016-10-22 23:32 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, pclouds, Stefan Beller
In-Reply-To: <20161022233225.8883-1-sbeller@google.com>
From: Junio C Hamano <gitster@pobox.com>
One codepath needs to just empty the git_attr_check_elem array in
the git_attr_check structure, without releasing the entire resource.
Introduce a helper to do so and rewrite git_attr_check_clear() using
it.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
attr.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/attr.c b/attr.c
index 2d13441..9f58cc0 100644
--- a/attr.c
+++ b/attr.c
@@ -746,6 +746,14 @@ static int attr_check_is_dynamic(const struct git_attr_check *check)
return (void *)(check->check) != (void *)(check + 1);
}
+static void empty_attr_check_elems(struct git_attr_check *check)
+{
+ if (!attr_check_is_dynamic(check))
+ die("BUG: emptying a statically initialized git_attr_check");
+ check->check_nr = 0;
+ check->finalized = 0;
+}
+
/*
* Collect attributes for path into the array pointed to by
* check_all_attr. If check is not NULL, only attributes in
@@ -912,12 +920,11 @@ struct git_attr_check_elem *git_attr_check_append(struct git_attr_check *check,
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);
- check->check_nr = 0;
check->check_alloc = 0;
- check->finalized = 0;
}
void git_attr_check_free(struct git_attr_check *check)
--
2.10.1.508.g6572022
^ permalink raw reply related
* [PATCH 22/36] attr.c: correct ugly hack for git_all_attrs()
From: Stefan Beller @ 2016-10-22 23:32 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, pclouds, Stefan Beller
In-Reply-To: <20161022233225.8883-1-sbeller@google.com>
From: Junio C Hamano <gitster@pobox.com>
The collect_some_attrs() function has an ugly hack since
06a604e6 (attr: avoid heavy work when we know the specified attr is
not defined, 2014-12-28) added an optimization that relies on the
fact that the caller knows what attributes it is interested in, so
that we can leave once we know the final answer for all the
attributes the caller asked.
git_all_attrs() that asks "what attributes are on this path?"
however does not know what attributes it is interested in, other
than the vague "we are interested in all of them", which is not a
very useful thing to say. As a way to disable this optimization
for this caller, the said commit added a code to skip it when
the caller passes a NULL for the check structure.
However, it skipped the optimization not when check is NULL, but
when the number of attributes being checked is 0, which is
unnecessarily pessimistic.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
attr.c | 24 ++++++++----------------
1 file changed, 8 insertions(+), 16 deletions(-)
diff --git a/attr.c b/attr.c
index 7869277..2d13441 100644
--- a/attr.c
+++ b/attr.c
@@ -748,8 +748,8 @@ static int attr_check_is_dynamic(const struct git_attr_check *check)
/*
* Collect attributes for path into the array pointed to by
- * check_all_attr. If num is non-zero, only attributes in check[] are
- * collected. Otherwise all attributes are collected.
+ * check_all_attr. If check is not NULL, only attributes in
+ * check[] are collected. Otherwise all attributes are collected.
*/
static void collect_some_attrs(const char *path, int pathlen,
struct git_attr_check *check)
@@ -759,17 +759,6 @@ static void collect_some_attrs(const char *path, int pathlen,
int i, rem, dirlen;
const char *cp, *last_slash = NULL;
int basename_offset;
- int num;
- struct git_attr_check_elem *celem;
-
- if (!check) {
- /* Yuck - ugly git_all_attrs() hack! */
- celem = NULL;
- num = 0;
- } else {
- celem = check->check;
- num = check->check_nr;
- }
for (cp = path; cp < path + pathlen; cp++) {
if (*cp == '/' && cp[1])
@@ -786,9 +775,12 @@ static void collect_some_attrs(const char *path, int pathlen,
prepare_attr_stack(path, dirlen);
for (i = 0; i < attr_nr; i++)
check_all_attr[i].value = ATTR__UNKNOWN;
- if (num && !cannot_trust_maybe_real) {
+
+ if (check && !cannot_trust_maybe_real) {
+ struct git_attr_check_elem *celem = check->check;
+
rem = 0;
- for (i = 0; i < num; i++) {
+ for (i = 0; i < check->check_nr; i++) {
if (!celem[i].attr->maybe_real) {
struct git_attr_check_elem *c;
c = check_all_attr + celem[i].attr->attr_nr;
@@ -796,7 +788,7 @@ static void collect_some_attrs(const char *path, int pathlen,
rem++;
}
}
- if (rem == num)
+ if (rem == check->check_nr)
return;
}
--
2.10.1.508.g6572022
^ permalink raw reply related
* [PATCH 15/36] attr: add counted string version of git_check_attr()
From: Stefan Beller @ 2016-10-22 23:32 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, pclouds, Stefan Beller
In-Reply-To: <20161022233225.8883-1-sbeller@google.com>
From: Junio C Hamano <gitster@pobox.com>
Often a potential caller has <path, pathlen> pair that
represents the path it wants to ask attributes for; when
path[pathlen] is not NUL, the caller has to xmemdupz()
only to call git_check_attr().
Add git_check_attr_counted() that takes such a counted
string instead of "const char *path".
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
attr.c | 23 ++++++++++++++---------
attr.h | 1 +
2 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/attr.c b/attr.c
index d427798..9bec243 100644
--- a/attr.c
+++ b/attr.c
@@ -734,20 +734,19 @@ static int attr_check_is_dynamic(const struct git_attr_check *check)
* check_all_attr. If num is non-zero, only attributes in check[] are
* collected. Otherwise all attributes are collected.
*/
-static void collect_some_attrs(const char *path, int num,
+static void collect_some_attrs(const char *path, int pathlen, int num,
struct git_attr_check_elem *check)
{
struct attr_stack *stk;
- int i, pathlen, rem, dirlen;
+ int i, rem, dirlen;
const char *cp, *last_slash = NULL;
int basename_offset;
- for (cp = path; *cp; cp++) {
+ for (cp = path; cp < path + pathlen; cp++) {
if (*cp == '/' && cp[1])
last_slash = cp;
}
- pathlen = cp - path;
if (last_slash) {
basename_offset = last_slash + 1 - path;
dirlen = last_slash - path;
@@ -778,12 +777,12 @@ static void collect_some_attrs(const char *path, int num,
rem = fill(path, pathlen, basename_offset, stk, rem);
}
-static int git_check_attrs(const char *path, int num,
+static int git_check_attrs(const char *path, int pathlen, int num,
struct git_attr_check_elem *check)
{
int i;
- collect_some_attrs(path, num, check);
+ collect_some_attrs(path, pathlen, num, check);
for (i = 0; i < num; i++) {
const char *value = check_all_attr[check[i].attr->attr_nr].value;
@@ -800,7 +799,7 @@ void git_all_attrs(const char *path, struct git_attr_check *check)
int i;
git_attr_check_clear(check);
- collect_some_attrs(path, 0, NULL);
+ collect_some_attrs(path, strlen(path), 0, NULL);
for (i = 0; i < attr_nr; i++) {
const char *name = check_all_attr[i].attr->name;
@@ -825,10 +824,16 @@ void git_attr_set_direction(enum git_attr_direction new, struct index_state *ist
use_index = istate;
}
-int git_check_attr(const char *path, struct git_attr_check *check)
+int git_check_attr_counted(const char *path, int pathlen,
+ struct git_attr_check *check)
{
check->finalized = 1;
- return git_check_attrs(path, check->check_nr, check->check);
+ return git_check_attrs(path, pathlen, check->check_nr, check->check);
+}
+
+int git_check_attr(const char *path, struct git_attr_check *check)
+{
+ return git_check_attr_counted(path, strlen(path), check);
}
struct git_attr_check *git_attr_check_initl(const char *one, ...)
diff --git a/attr.h b/attr.h
index 506db0c..c84f164 100644
--- a/attr.h
+++ b/attr.h
@@ -38,6 +38,7 @@ struct git_attr_check {
extern struct git_attr_check *git_attr_check_initl(const char *, ...);
extern int git_check_attr(const char *path, struct git_attr_check *);
+extern int git_check_attr_counted(const char *, int, struct git_attr_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 *);
--
2.10.1.508.g6572022
^ permalink raw reply related
* [PATCH 21/36] attr.c: rename a local variable check
From: Stefan Beller @ 2016-10-22 23:32 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, pclouds, Stefan Beller
In-Reply-To: <20161022233225.8883-1-sbeller@google.com>
From: Junio C Hamano <gitster@pobox.com>
Throughout this series, we are trying to use "check" to name an
instance of "git_attr_check" structure; let's rename a "check" that
refers to an array whose elements are git_attr_check_elem to avoid
confusion.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
attr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/attr.c b/attr.c
index 9ed4825..7869277 100644
--- a/attr.c
+++ b/attr.c
@@ -682,12 +682,12 @@ static int macroexpand_one(int attr_nr, int rem);
static int fill_one(const char *what, struct match_attr *a, int rem)
{
- struct git_attr_check_elem *check = check_all_attr;
+ struct git_attr_check_elem *celem = check_all_attr;
int i;
for (i = a->num_attr - 1; 0 < rem && 0 <= i; i--) {
struct git_attr *attr = a->state[i].attr;
- const char **n = &(check[attr->attr_nr].value);
+ const char **n = &(celem[attr->attr_nr].value);
const char *v = a->state[i].setto;
if (*n == ATTR__UNKNOWN) {
--
2.10.1.508.g6572022
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox