* [PATCH v1 17/19] t1700: test shared index file expiration
From: Christian Couder @ 2016-10-23 9:26 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Christian Couder
In-Reply-To: <20161023092648.12086-1-chriscool@tuxfamily.org>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
t/t1700-split-index.sh | 44 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index f03addf..f448fc1 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -310,4 +310,48 @@ EOF
test_cmp expect actual
'
+test_expect_success 'shared index files expire after 7 days by default' '
+ : >ten &&
+ git update-index --add ten &&
+ test $(ls .git/sharedindex.* | wc -l) -gt 1 &&
+ just_under_7_days_ago=$((1-7*86400)) &&
+ test-chmtime =$just_under_7_days_ago .git/sharedindex.* &&
+ : >eleven &&
+ git update-index --add eleven &&
+ test $(ls .git/sharedindex.* | wc -l) -gt 1 &&
+ just_over_7_days_ago=$((-1-7*86400)) &&
+ test-chmtime =$just_over_7_days_ago .git/sharedindex.* &&
+ : >twelve &&
+ git update-index --add twelve &&
+ test $(ls .git/sharedindex.* | wc -l) = 1
+'
+
+test_expect_success 'check splitIndex.sharedIndexExpire set to 8 days' '
+ git config splitIndex.sharedIndexExpire "8.days.ago" &&
+ test-chmtime =$just_over_7_days_ago .git/sharedindex.* &&
+ : >thirteen &&
+ git update-index --add thirteen &&
+ test $(ls .git/sharedindex.* | wc -l) -gt 1 &&
+ just_over_8_days_ago=$((-1-8*86400)) &&
+ test-chmtime =$just_over_8_days_ago .git/sharedindex.* &&
+ : >fourteen &&
+ git update-index --add fourteen &&
+ test $(ls .git/sharedindex.* | wc -l) = 1
+'
+
+test_expect_success 'check splitIndex.sharedIndexExpire set to "never" and "now"' '
+ git config splitIndex.sharedIndexExpire never &&
+ just_10_years_ago=$((-365*10*86400)) &&
+ test-chmtime =$just_10_years_ago .git/sharedindex.* &&
+ : >fifteen &&
+ git update-index --add fifteen &&
+ test $(ls .git/sharedindex.* | wc -l) -gt 1 &&
+ git config splitIndex.sharedIndexExpire now &&
+ just_1_second_ago=-1 &&
+ test-chmtime =$just_1_second_ago .git/sharedindex.* &&
+ : >sixteen &&
+ git update-index --add sixteen &&
+ test $(ls .git/sharedindex.* | wc -l) = 1
+'
+
test_done
--
2.10.1.462.g7e1e03a
^ permalink raw reply related
* [PATCH v1 15/19] config: add git_config_get_date_string() from gc.c
From: Christian Couder @ 2016-10-23 9:26 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Christian Couder
In-Reply-To: <20161023092648.12086-1-chriscool@tuxfamily.org>
This function will be used in a following commit to get the expiration
time of the shared index files from the config, and it is generic
enough to be put in "config.c".
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/gc.c | 15 ++-------------
cache.h | 1 +
config.c | 13 +++++++++++++
3 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index 069950d..c1e9602 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -62,17 +62,6 @@ static void report_pack_garbage(unsigned seen_bits, const char *path)
string_list_append(&pack_garbage, path);
}
-static void git_config_date_string(const char *key, const char **output)
-{
- if (git_config_get_string_const(key, output))
- return;
- if (strcmp(*output, "now")) {
- unsigned long now = approxidate("now");
- if (approxidate(*output) >= now)
- git_die_config(key, _("Invalid %s: '%s'"), key, *output);
- }
-}
-
static void process_log_file(void)
{
struct stat st;
@@ -111,8 +100,8 @@ static void gc_config(void)
git_config_get_int("gc.auto", &gc_auto_threshold);
git_config_get_int("gc.autopacklimit", &gc_auto_pack_limit);
git_config_get_bool("gc.autodetach", &detach_auto);
- git_config_date_string("gc.pruneexpire", &prune_expire);
- git_config_date_string("gc.worktreepruneexpire", &prune_worktrees_expire);
+ git_config_get_date_string("gc.pruneexpire", &prune_expire);
+ git_config_get_date_string("gc.worktreepruneexpire", &prune_worktrees_expire);
git_config(git_default_config, NULL);
}
diff --git a/cache.h b/cache.h
index a625b47..bcfc0f1 100644
--- a/cache.h
+++ b/cache.h
@@ -1811,6 +1811,7 @@ extern int git_config_get_bool(const char *key, int *dest);
extern int git_config_get_bool_or_int(const char *key, int *is_bool, int *dest);
extern int git_config_get_maybe_bool(const char *key, int *dest);
extern int git_config_get_pathname(const char *key, const char **dest);
+extern int git_config_get_date_string(const char *key, const char **output);
extern int git_config_get_untracked_cache(void);
extern int git_config_get_split_index(void);
extern int git_config_get_max_percent_split_change(void);
diff --git a/config.c b/config.c
index 5580f56..f88c61b 100644
--- a/config.c
+++ b/config.c
@@ -1685,6 +1685,19 @@ int git_config_get_pathname(const char *key, const char **dest)
return ret;
}
+int git_config_get_date_string(const char *key, const char **output)
+{
+ int ret = git_config_get_string_const(key, output);
+ if (ret)
+ return ret;
+ if (strcmp(*output, "now")) {
+ unsigned long now = approxidate("now");
+ if (approxidate(*output) >= now)
+ git_die_config(key, _("Invalid %s: '%s'"), key, *output);
+ }
+ return ret;
+}
+
int git_config_get_untracked_cache(void)
{
int val = -1;
--
2.10.1.462.g7e1e03a
^ permalink raw reply related
* [PATCH v1 10/19] read-cache: regenerate shared index if necessary
From: Christian Couder @ 2016-10-23 9:26 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Christian Couder
In-Reply-To: <20161023092648.12086-1-chriscool@tuxfamily.org>
When writing a new split-index and there is a big number of cache
entries in the split-index compared to the shared index, it is a
good idea to regenerate the shared index.
By default when the ratio reaches 20%, we will push back all
the entries from the split-index into a new shared index file
instead of just creating a new split-index file.
The threshold can be configured using the
"splitIndex.maxPercentChange" config variable.
We need to adjust the existing tests in t1700 by setting
"splitIndex.maxPercentChange" to 100 at the beginning of t1700,
as the existing tests are assuming that the shared index is
regenerated only when `git update-index --split-index` is used.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
read-cache.c | 33 ++++++++++++++++++++++++++++++++-
t/t1700-split-index.sh | 1 +
2 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/read-cache.c b/read-cache.c
index bb53823..a91fabe 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2216,6 +2216,36 @@ static int write_shared_index(struct index_state *istate,
return ret;
}
+static const int default_max_percent_split_change = 20;
+
+int too_many_not_shared_entries(struct index_state *istate)
+{
+ int i, not_shared = 0;
+ int max_split = git_config_get_max_percent_split_change();
+
+ switch (max_split) {
+ case -1:
+ /* not or badly configured: use the default value */
+ max_split = default_max_percent_split_change;
+ break;
+ case 0:
+ return 1; /* 0% means always write a new shared index */
+ case 100:
+ return 0; /* 100% means never write a new shared index */
+ default:
+ ; /* do nothing: just use the configured value */
+ }
+
+ /* Count not shared entries */
+ for (i = 0; i < istate->cache_nr; i++) {
+ struct cache_entry *ce = istate->cache[i];
+ if (!ce->index)
+ not_shared++;
+ }
+
+ return istate->cache_nr * max_split < not_shared * 100;
+}
+
int write_locked_index(struct index_state *istate, struct lock_file *lock,
unsigned flags)
{
@@ -2233,7 +2263,8 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,
if ((v & 15) < 6)
istate->cache_changed |= SPLIT_INDEX_ORDERED;
}
- if (istate->cache_changed & SPLIT_INDEX_ORDERED) {
+ if (istate->cache_changed & SPLIT_INDEX_ORDERED ||
+ too_many_not_shared_entries(istate)) {
int ret = write_shared_index(istate, lock, flags);
if (ret)
return ret;
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index db8c39f..507a1dd 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -8,6 +8,7 @@ test_description='split index mode tests'
sane_unset GIT_TEST_SPLIT_INDEX
test_expect_success 'enable split index' '
+ git config splitIndex.maxPercentChange 100 &&
git update-index --split-index &&
test-dump-split-index .git/index >actual &&
indexversion=$(test-index-version <.git/index) &&
--
2.10.1.462.g7e1e03a
^ permalink raw reply related
* [PATCH v1 11/19] t1700: add tests for splitIndex.maxPercentChange
From: Christian Couder @ 2016-10-23 9:26 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Christian Couder
In-Reply-To: <20161023092648.12086-1-chriscool@tuxfamily.org>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
t/t1700-split-index.sh | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 72 insertions(+)
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index 507a1dd..f03addf 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -238,4 +238,76 @@ EOF
test_cmp expect actual
'
+test_expect_success 'set core.splitIndex config variable to true' '
+ git config core.splitIndex true &&
+ : >three &&
+ git update-index --add three &&
+ BASE=$(test-dump-split-index .git/index | grep "^base") &&
+ test-dump-split-index .git/index | sed "/^own/d" >actual &&
+ cat >expect <<EOF &&
+$BASE
+replacements:
+deletions:
+EOF
+ test_cmp expect actual &&
+ : >four &&
+ git update-index --add four &&
+ test-dump-split-index .git/index | sed "/^own/d" >actual &&
+ cat >expect <<EOF &&
+$BASE
+100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 four
+replacements:
+deletions:
+EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'check behavior with splitIndex.maxPercentChange unset' '
+ git config --unset splitIndex.maxPercentChange &&
+ : >five &&
+ git update-index --add five &&
+ BASE=$(test-dump-split-index .git/index | grep "^base") &&
+ test-dump-split-index .git/index | sed "/^own/d" >actual &&
+ cat >expect <<EOF &&
+$BASE
+replacements:
+deletions:
+EOF
+ test_cmp expect actual &&
+ : >six &&
+ git update-index --add six &&
+ test-dump-split-index .git/index | sed "/^own/d" >actual &&
+ cat >expect <<EOF &&
+$BASE
+100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 six
+replacements:
+deletions:
+EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'check splitIndex.maxPercentChange set to 0' '
+ git config splitIndex.maxPercentChange 0 &&
+ : >seven &&
+ git update-index --add seven &&
+ BASE=$(test-dump-split-index .git/index | grep "^base") &&
+ test-dump-split-index .git/index | sed "/^own/d" >actual &&
+ cat >expect <<EOF &&
+$BASE
+replacements:
+deletions:
+EOF
+ test_cmp expect actual &&
+ : >eight &&
+ git update-index --add eight &&
+ BASE=$(test-dump-split-index .git/index | grep "^base") &&
+ test-dump-split-index .git/index | sed "/^own/d" >actual &&
+ cat >expect <<EOF &&
+$BASE
+replacements:
+deletions:
+EOF
+ test_cmp expect actual
+'
+
test_done
--
2.10.1.462.g7e1e03a
^ permalink raw reply related
* [PATCH v1 12/19] Documentation/config: add splitIndex.maxPercentChange
From: Christian Couder @ 2016-10-23 9:26 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Christian Couder
In-Reply-To: <20161023092648.12086-1-chriscool@tuxfamily.org>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/config.txt | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 96521a4..380eeb8 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2763,6 +2763,19 @@ showbranch.default::
The default set of branches for linkgit:git-show-branch[1].
See linkgit:git-show-branch[1].
+splitIndex.maxPercentChange::
+ When the split index feature is used, this specifies the
+ percent of entries the split index can contain compared to the
+ whole number of entries in both the split index and the shared
+ index before a new shared index is written.
+ The value should be between 0 and 100. If the value is 0 then
+ a new shared index is always written, if it is 100 a new
+ shared index is never written.
+ By default the value is 20, so a new shared index is written
+ if the number of entries in the split index would be greater
+ than 20 percent of the total number of entries.
+ See linkgit:git-update-index[1].
+
status.relativePaths::
By default, linkgit:git-status[1] shows paths relative to the
current directory. Setting this variable to `false` shows paths
--
2.10.1.462.g7e1e03a
^ permalink raw reply related
* [PATCH v1 13/19] sha1_file: make check_and_freshen_file() non static
From: Christian Couder @ 2016-10-23 9:26 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Christian Couder
In-Reply-To: <20161023092648.12086-1-chriscool@tuxfamily.org>
This function will be used in a commit soon, so let's make
it available globally.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
cache.h | 3 +++
sha1_file.c | 2 +-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/cache.h b/cache.h
index faceceb..a625b47 100644
--- a/cache.h
+++ b/cache.h
@@ -1167,6 +1167,9 @@ extern int has_pack_index(const unsigned char *sha1);
extern void assert_sha1_type(const unsigned char *sha1, enum object_type expect);
+/* Helper to check and "touch" a file */
+extern int check_and_freshen_file(const char *fn, int freshen);
+
extern const signed char hexval_table[256];
static inline unsigned int hexval(unsigned char c)
{
diff --git a/sha1_file.c b/sha1_file.c
index 266152d..29f927e 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -576,7 +576,7 @@ static int freshen_file(const char *fn)
* either does not exist on disk, or has a stale mtime and may be subject to
* pruning).
*/
-static int check_and_freshen_file(const char *fn, int freshen)
+int check_and_freshen_file(const char *fn, int freshen)
{
if (access(fn, F_OK))
return 0;
--
2.10.1.462.g7e1e03a
^ permalink raw reply related
* [PATCH v1 09/19] config: add git_config_get_max_percent_split_change()
From: Christian Couder @ 2016-10-23 9:26 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Christian Couder
In-Reply-To: <20161023092648.12086-1-chriscool@tuxfamily.org>
This new function will be used in a following commit to get the
value of the "splitIndex.maxPercentChange" config variable.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
cache.h | 1 +
config.c | 16 ++++++++++++++++
2 files changed, 17 insertions(+)
diff --git a/cache.h b/cache.h
index 394da60..faceceb 100644
--- a/cache.h
+++ b/cache.h
@@ -1810,6 +1810,7 @@ extern int git_config_get_maybe_bool(const char *key, int *dest);
extern int git_config_get_pathname(const char *key, const char **dest);
extern int git_config_get_untracked_cache(void);
extern int git_config_get_split_index(void);
+extern int git_config_get_max_percent_split_change(void);
/*
* This is a hack for test programs like test-dump-untracked-cache to
diff --git a/config.c b/config.c
index 036e29b..5580f56 100644
--- a/config.c
+++ b/config.c
@@ -1719,6 +1719,22 @@ int git_config_get_split_index(void)
return -1; /* default value */
}
+int git_config_get_max_percent_split_change(void)
+{
+ int val = -1;
+
+ if (!git_config_get_int("splitindex.maxpercentchange", &val)) {
+ if (0 <= val && val <= 100)
+ return val;
+
+ error("splitindex.maxpercentchange value '%d' "
+ "should be between 0 and 100", val);
+ return -1;
+ }
+
+ return -1; /* default value */
+}
+
NORETURN
void git_die_config_linenr(const char *key, const char *filename, int linenr)
{
--
2.10.1.462.g7e1e03a
^ permalink raw reply related
* [PATCH v1 07/19] Documentation/config: add information for core.splitIndex
From: Christian Couder @ 2016-10-23 9:26 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Christian Couder
In-Reply-To: <20161023092648.12086-1-chriscool@tuxfamily.org>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/config.txt | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 27069ac..96521a4 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -331,6 +331,10 @@ core.trustctime::
crawlers and some backup systems).
See linkgit:git-update-index[1]. True by default.
+core.splitIndex::
+ If true, the split-index feature of the index will be used.
+ See linkgit:git-update-index[1]. False by default.
+
core.untrackedCache::
Determines what to do about the untracked cache feature of the
index. It will be kept, if this variable is unset or set to
--
2.10.1.462.g7e1e03a
^ permalink raw reply related
* [PATCH v1 03/19] split-index: add {add,remove}_split_index() functions
From: Christian Couder @ 2016-10-23 9:26 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Christian Couder
In-Reply-To: <20161023092648.12086-1-chriscool@tuxfamily.org>
Also use the functions in cmd_update_index() in
builtin/update-index.c.
These functions will be used in a following commit to tweak
our use of the split-index feature depending on the setting
of a configuration variable.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/update-index.c | 18 ++++++------------
split-index.c | 22 ++++++++++++++++++++++
split-index.h | 2 ++
3 files changed, 30 insertions(+), 12 deletions(-)
diff --git a/builtin/update-index.c b/builtin/update-index.c
index f3f07e7..b75ea03 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1098,18 +1098,12 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
}
if (split_index > 0) {
- init_split_index(&the_index);
- the_index.cache_changed |= SPLIT_INDEX_ORDERED;
- } else if (!split_index && the_index.split_index) {
- /*
- * can't discard_split_index(&the_index); because that
- * will destroy split_index->base->cache[], which may
- * be shared with the_index.cache[]. So yeah we're
- * leaking a bit here.
- */
- the_index.split_index = NULL;
- the_index.cache_changed |= SOMETHING_CHANGED;
- }
+ if (the_index.split_index)
+ the_index.cache_changed |= SPLIT_INDEX_ORDERED;
+ else
+ add_split_index(&the_index);
+ } else if (!split_index)
+ remove_split_index(&the_index);
switch (untracked_cache) {
case UC_UNSPECIFIED:
diff --git a/split-index.c b/split-index.c
index 615f4ca..f519e60 100644
--- a/split-index.c
+++ b/split-index.c
@@ -317,3 +317,25 @@ void replace_index_entry_in_base(struct index_state *istate,
istate->split_index->base->cache[new->index - 1] = new;
}
}
+
+void add_split_index(struct index_state *istate)
+{
+ if (!istate->split_index) {
+ init_split_index(istate);
+ istate->cache_changed |= SPLIT_INDEX_ORDERED;
+ }
+}
+
+void remove_split_index(struct index_state *istate)
+{
+ if (istate->split_index) {
+ /*
+ * can't discard_split_index(&the_index); because that
+ * will destroy split_index->base->cache[], which may
+ * be shared with the_index.cache[]. So yeah we're
+ * leaking a bit here.
+ */
+ istate->split_index = NULL;
+ istate->cache_changed |= SOMETHING_CHANGED;
+ }
+}
diff --git a/split-index.h b/split-index.h
index c1324f5..df91c1b 100644
--- a/split-index.h
+++ b/split-index.h
@@ -31,5 +31,7 @@ void merge_base_index(struct index_state *istate);
void prepare_to_write_split_index(struct index_state *istate);
void finish_writing_split_index(struct index_state *istate);
void discard_split_index(struct index_state *istate);
+void add_split_index(struct index_state *istate);
+void remove_split_index(struct index_state *istate);
#endif
--
2.10.1.462.g7e1e03a
^ permalink raw reply related
* [PATCH v1 01/19] split-index: s/eith/with/ typo fix
From: Christian Couder @ 2016-10-23 9:26 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Christian Couder
In-Reply-To: <20161023092648.12086-1-chriscool@tuxfamily.org>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
split-index.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/split-index.c b/split-index.c
index 35da553..615f4ca 100644
--- a/split-index.c
+++ b/split-index.c
@@ -187,7 +187,7 @@ void prepare_to_write_split_index(struct index_state *istate)
/* Go through istate->cache[] and mark CE_MATCHED to
* entry with positive index. We'll go through
* base->cache[] later to delete all entries in base
- * that are not marked eith either CE_MATCHED or
+ * that are not marked with either CE_MATCHED or
* CE_UPDATE_IN_BASE. If istate->cache[i] is a
* duplicate, deduplicate it.
*/
--
2.10.1.462.g7e1e03a
^ permalink raw reply related
* [PATCH v1 04/19] read-cache: add and then use tweak_split_index()
From: Christian Couder @ 2016-10-23 9:26 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Christian Couder
In-Reply-To: <20161023092648.12086-1-chriscool@tuxfamily.org>
This will make us use the split-index feature or not depending
on the value of the "core.splitIndex" config variable.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
read-cache.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/read-cache.c b/read-cache.c
index 38d67fa..bb53823 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1562,10 +1562,27 @@ static void tweak_untracked_cache(struct index_state *istate)
}
}
+static void tweak_split_index(struct index_state *istate)
+{
+ switch (git_config_get_split_index()) {
+ case -1: /* unset: do nothing */
+ break;
+ case 0: /* false */
+ remove_split_index(istate);
+ break;
+ case 1: /* true */
+ add_split_index(istate);
+ break;
+ default: /* unknown value: do nothing */
+ break;
+ }
+}
+
static void post_read_index_from(struct index_state *istate)
{
check_ce_order(istate);
tweak_untracked_cache(istate);
+ tweak_split_index(istate);
}
/* remember to discard_cache() before reading a different cache! */
--
2.10.1.462.g7e1e03a
^ permalink raw reply related
* [PATCH v1 05/19] update-index: warn in case of split-index incoherency
From: Christian Couder @ 2016-10-23 9:26 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Christian Couder
In-Reply-To: <20161023092648.12086-1-chriscool@tuxfamily.org>
When users are using `git update-index --(no-)split-index`, they
may expect the split-index feature to be used or not according to
the option they just used, but this might not be the case if the
new "core.splitIndex" config variable has been set. In this case
let's warn about what will happen and why.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/update-index.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/builtin/update-index.c b/builtin/update-index.c
index b75ea03..a14dbf2 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1098,12 +1098,21 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
}
if (split_index > 0) {
+ if (git_config_get_split_index() == 0)
+ warning("core.splitIndex is set to false; "
+ "remove or change it, if you really want to "
+ "enable split index");
if (the_index.split_index)
the_index.cache_changed |= SPLIT_INDEX_ORDERED;
else
add_split_index(&the_index);
- } else if (!split_index)
+ } else if (!split_index) {
+ if (git_config_get_split_index() == 1)
+ warning("core.splitIndex is set to true; "
+ "remove or change it, if you really want to "
+ "disable split index");
remove_split_index(&the_index);
+ }
switch (untracked_cache) {
case UC_UNSPECIFIED:
--
2.10.1.462.g7e1e03a
^ permalink raw reply related
* [PATCH v1 02/19] config: add git_config_get_split_index()
From: Christian Couder @ 2016-10-23 9:26 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Christian Couder
In-Reply-To: <20161023092648.12086-1-chriscool@tuxfamily.org>
This new function will be used in a following commit to know
if we want to use the split index feature or not.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
cache.h | 1 +
config.c | 10 ++++++++++
2 files changed, 11 insertions(+)
diff --git a/cache.h b/cache.h
index 05ecb88..394da60 100644
--- a/cache.h
+++ b/cache.h
@@ -1809,6 +1809,7 @@ extern int git_config_get_bool_or_int(const char *key, int *is_bool, int *dest);
extern int git_config_get_maybe_bool(const char *key, int *dest);
extern int git_config_get_pathname(const char *key, const char **dest);
extern int git_config_get_untracked_cache(void);
+extern int git_config_get_split_index(void);
/*
* This is a hack for test programs like test-dump-untracked-cache to
diff --git a/config.c b/config.c
index 83fdecb..036e29b 100644
--- a/config.c
+++ b/config.c
@@ -1709,6 +1709,16 @@ int git_config_get_untracked_cache(void)
return -1; /* default value */
}
+int git_config_get_split_index(void)
+{
+ int val = -1;
+
+ if (!git_config_get_maybe_bool("core.splitindex", &val))
+ return val;
+
+ return -1; /* default value */
+}
+
NORETURN
void git_die_config_linenr(const char *key, const char *filename, int linenr)
{
--
2.10.1.462.g7e1e03a
^ permalink raw reply related
* [PATCH v1 00/19] Add configuration options for split-index
From: Christian Couder @ 2016-10-23 9:26 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Christian Couder
Goal
~~~~
We want to make it possible to use the split-index feature
automatically by just setting a new "core.splitIndex" configuration
variable to true.
This can be valuable as split-index can help significantly speed up
`git rebase` especially along with the work to libify `git apply`
that has been recently merged to master
(see https://github.com/git/git/commit/81358dc238372793b1590efa149cc1581d1fbd98).
Design
~~~~~~
The design is similar as the previous work that introduced
"core.untrackedCache".
The new "core.splitIndex" configuration option can be either true,
false or undefined which is the default.
When it is true, the split index is created, if it does not already
exists, when the index is read. When it is false, the split index is
removed if it exists, when the index is read. Otherwise it is left as
is.
Along with this new configuration variable, the two following options
are also introduced:
- splitIndex.maxPercentChange
This is to avoid having too many changes accumulating in the split
index while in split index mode. The git-update-index
documentation says:
If split-index mode is already enabled and `--split-index` is
given again, all changes in $GIT_DIR/index are pushed back to
the shared index file.
but it is probably better to not expect the user to think about it
and to have a mechanism that pushes back all changes to the shared
index file automatically when some threshold is reached.
The default threshold is when the number of entries in the split
index file reaches 20% (by default) of the number of entries in
the shared index file. The new "splitIndex.maxPercentChange"
config option lets people tweak this value.
- splitIndex.sharedIndexExpire
To make sure that old sharedindex files are eventually removed
when a new one has been created, we "touch" the shared index file
every time it is used by a new split index file. Then we can
delete shared indexes with an mtime older than one week (by
default), when we create a new shared index file. The new
"splitIndex.sharedIndexExpire" config option lets people tweak
this grace period.
This idea was suggested by Duy in:
https://public-inbox.org/git/CACsJy8BqMFASHf5kJgUh+bd7XG98CafNydE964VJyPXz-emEvA@mail.gmail.com/
and after some experiments, I agree that it is much simpler than
what I thought could be done during our discussion.
Highlevel view of the patches in the series
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Except for patch 1/19, there are 3 big steps, one for each new
configuration variable introduced.
The main difference between this patch series and the RFC patch series
sent last July is that the Step 2 and 3 are new and have been
implemented as suggested by Duy. Thanks Duy!
- Patch 1/19 is a typo fix in a comment that can be applied
separately.
Step 1 is:
- Patches 2/19 to 5/19 introduce the functions that are reading
the "core.splitIndex" configuration variable and tweaking the
split index depending on its value.
- Patch 6/19 adds a few tests for the new feature.
- Patches 7/19 and 8/19 add some documentation for the new
feature.
Step 2 is:
- Patches 9/19 and 10/19 introduce the functions that are reading
the "splitIndex.maxPercentChange" configuration variable and
regenerating a new shared index file depending on its value.
- Patch 11/19 adds a few tests for the new feature.
- Patch 12/19 add some documentation for the new feature.
Step 3 is:
- Patches 13/19 to 16/19 introduce the functions that are reading
the "splitIndex.sharedIndexExpire" configuration variable and
expiring old shared index files depending on its value.
- Patch 17/19 adds a few tests for the new feature.
- Patches 18/19 and 19/19 add some documentation for the new
feature.
Links
~~~~~
This patch series is also available here:
https://github.com/chriscool/git/commits/config-split-index
The previous RFC version was:
https://github.com/chriscool/git/commits/config-split-index7
On the mailing list the related patch series and discussions were:
https://public-inbox.org/git/20160711172254.13439-1-chriscool@tuxfamily.org/
Christian Couder (19):
split-index: s/eith/with/ typo fix
config: add git_config_get_split_index()
split-index: add {add,remove}_split_index() functions
read-cache: add and then use tweak_split_index()
update-index: warn in case of split-index incoherency
t1700: add tests for core.splitIndex
Documentation/config: add information for core.splitIndex
Documentation/git-update-index: talk about core.splitIndex config var
config: add git_config_get_max_percent_split_change()
read-cache: regenerate shared index if necessary
t1700: add tests for splitIndex.maxPercentChange
Documentation/config: add splitIndex.maxPercentChange
sha1_file: make check_and_freshen_file() non static
read-cache: touch shared index files when used
config: add git_config_get_date_string() from gc.c
read-cache: unlink old sharedindex files
t1700: test shared index file expiration
Documentation/config: add splitIndex.sharedIndexExpire
Documentation/git-update-index: explain splitIndex.*
Documentation/config.txt | 28 +++++++
Documentation/git-update-index.txt | 39 ++++++++--
builtin/gc.c | 15 +---
builtin/update-index.c | 25 +++---
cache.h | 6 ++
config.c | 39 ++++++++++
read-cache.c | 119 +++++++++++++++++++++++++++-
sha1_file.c | 2 +-
split-index.c | 24 +++++-
split-index.h | 2 +
t/t1700-split-index.sh | 154 +++++++++++++++++++++++++++++++++++++
11 files changed, 417 insertions(+), 36 deletions(-)
--
2.10.1.462.g7e1e03a
^ permalink raw reply
* Re: [PATCH] hex: use unsigned index for ring buffer
From: Jeff King @ 2016-10-23 9:11 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List, Junio C Hamano
In-Reply-To: <ebf769d4-771f-499e-c7fc-f0377d8df18e@web.de>
On Sun, Oct 23, 2016 at 11:00:48AM +0200, René Scharfe wrote:
> 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.
Indeed. If we are worried about overflow, we would also want to assume
that it wraps at a multiple of 4, but that is probably a sane
assumption.
> 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);
> }
I wonder if just truncating bufno would be conceptually simpler (albeit
longer):
bufno++;
bufno &= 3;
return sha1_to_hex_r(hexbuffer[bufno], sha1);
You could also write the second line like:
bufno %= ARRAY_SIZE(hexbuffer);
which is less magical (right now the set of buffers must be a power of
2). I expect the compiler could turn that into a bitmask itself.
I'm fine with any of the options. I guess you'd want a similar patch for
find_unique_abbrev on top of jk/no-looking-at-dotgit-outside-repo.
-Peff
^ permalink raw reply
* 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
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