* Re: [PATCH v5 22/27] sequencer: teach write_message() to append an optional LF
From: Johannes Schindelin @ 2016-10-23 9:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jakub Narębski, Johannes Sixt, Ramsay Jones
In-Reply-To: <xmqqmvhxbljm.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Fri, 21 Oct 2016, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
> > This commit prepares for future callers that will have a pointer/length
> > to some text to be written that lacks an LF, yet an LF is desired.
> > Instead of requiring the caller to append an LF to the buffer (and
> > potentially allocate memory to do so), the write_message() function
> > learns to append an LF at the end of the file.
>
> As no existing callers need this, it probably is better left out and
> added to the series that actually needs the new feature as a
> preparatory step.
Apart from this patch series being semantically the right place
("prepare-sequencer"), there is also the following consideration:
The next patch series is already quite long. Taking this current patch
series into account, which started out as a 22-patch series and needed to
bloat by 25% through four subsequent iterations, it is probably not a wise
idea to move this patch to a patch series that already weighs 34 patches.
So I respectfully, and forcefully, disagree,
Dscho
^ permalink raw reply
* Re: Stash pop/apply conflict and --theirs and --ours
From: Jeff King @ 2016-10-23 9:30 UTC (permalink / raw)
To: Sven Strickroth; +Cc: Git List, Junio C Hamano
In-Reply-To: <169639c9-054f-8f4c-26bd-3f130fa4c1ee@cs-ware.de>
On Sun, Oct 23, 2016 at 12:58:12AM +0200, Sven Strickroth wrote:
> I regularly experience that beginners have problems unterstanding that
> --ours and --theirs are swapped when a conflict occurrs on git stash
> apply or stash pop.
>
> From the HCI perspective this is really counter intuitive.
I know that people have complained about "rebase" swapping the two, but
I don't think anybody has ever mentioned it for stash. I'm not sure I
that they are swapped, though.
The "ours" content is generally what was in the HEAD before the
operation started, and "theirs" is what the operation is bringing into
that history. That is true of "merge" and "cherry-pick". And AFAICT, it
is true of "stash", too (I basically think of "stash apply" as a
cherry-pick).
So with a setup like:
git init
echo base >file
git add file
git commit -m file
echo stash >file
git stash
echo master >file
git commit -am master
git checkout -b branch HEAD^
echo branch >file
git commit -am branch
if we merge, then --theirs is the branch we are merging:
git checkout master
git merge branch
git checkout --theirs file
cat file
# "branch"
Likewise, if we cherry-pick:
git reset --hard
git cherry-pick branch
git checkout --theirs file
cat file
# "branch"
And likewise if we apply the stash:
git reset --hard
git stash apply
git checkout --theirs file
cat file
# "stash"
So that seems consistent to me.
I guess if you are stashing in order to pull somebody else's work, like:
git stash
git pull
git stash pop
then conceptually the stash is "ours" and HEAD is "theirs". This is
exactly like the rebase case. E.g., if you instead did:
git commit -m 'tmp stash'
git pull --rebase
So I sympathize, but I don't think that having "stash" flip the order
would be the right thing to do in all cases. In theory there could be
some kind of option (and things like pull autostash could use it), but I
suspect it may be hard to implement in practice. The unpack-trees code
does not treat "ours" and "theirs" entirely symmetrically (the "ours"
side represents the current working tree, so we might do things like
check whether the index is fresh). I guess you could flip the "1" and
"2" bits in the index after the conflicted merge completes.
I'm still not convinced it's a good idea, though.
-Peff
^ permalink raw reply
* Re: [PATCH v4 20/25] sequencer: refactor write_message()
From: Johannes Schindelin @ 2016-10-23 9:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jakub Narębski, Johannes Sixt, Ramsay Jones
In-Reply-To: <xmqqr379d82b.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Fri, 21 Oct 2016, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> >> Ah, make that four steps. The final one is:
> >>
> >> - add append_eol parameter that nobody uses at this step in the
> >> series.
> >>
> >> This is a new feature to the helper. While it is OK to have it as a
> >> preparatory step in this series, it is easier to understand if it
> >> kept as a separate step. It is even more preferrable if it is made
> >> as a preparatory step in a series that adds a caller that passes
> >> true to append_eol to this helper...
> >
> > Done,
> > Dscho
>
> Hmm, what has been done exactly? I still see append_eol in v5 where
> nobody uses it yet. Confused...
Your bullet point suggests that this change should be a separate patch. I
did that.
And since this patch series' purpose is to prepare the sequencer for the
upcoming series that teaches it to understand git-rebase-todo scripts,
this patch falls fair and square into the preparatory phase.
Ciao,
Dscho
^ permalink raw reply
* [PATCH v1 06/19] t1700: add tests 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>
---
t/t1700-split-index.sh | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index 292a072..db8c39f 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -200,4 +200,41 @@ 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 &&
+ git ls-files --stage >ls-files.actual &&
+ cat >ls-files.expect <<EOF &&
+100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 one
+100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 three
+100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 two
+EOF
+ test_cmp ls-files.expect ls-files.actual &&
+ 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_expect_success 'set core.splitIndex config variable to false' '
+ git config core.splitIndex false &&
+ git update-index --force-remove three &&
+ git ls-files --stage >ls-files.actual &&
+ cat >ls-files.expect <<EOF &&
+100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 one
+100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 two
+EOF
+ test_cmp ls-files.expect ls-files.actual &&
+ test-dump-split-index .git/index | sed "/^own/d" >actual &&
+ cat >expect <<EOF &&
+not a split index
+EOF
+ test_cmp expect actual
+'
+
test_done
--
2.10.1.462.g7e1e03a
^ permalink raw reply related
* [PATCH v1 14/19] read-cache: touch shared index files when used
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 a split-index file is created, let's update the mtime of the
shared index file that the split-index file is referencing.
In a following commit we will make shared index file expire
depending on their mtime, so updating the mtime makes sure that
the shared index file will not be deleted soon.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
read-cache.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/read-cache.c b/read-cache.c
index a91fabe..3aeff77 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2268,6 +2268,12 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,
int ret = write_shared_index(istate, lock, flags);
if (ret)
return ret;
+ } else {
+ /* Signal that the shared index is used */
+ const char *shared_index = git_path("sharedindex.%s",
+ sha1_to_hex(si->base_sha1));
+ if (!check_and_freshen_file(shared_index, 1))
+ warning("could not freshen '%s'", shared_index);
}
return write_split_index(istate, lock, flags);
--
2.10.1.462.g7e1e03a
^ permalink raw reply related
* [PATCH v1 08/19] Documentation/git-update-index: talk about core.splitIndex config var
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/git-update-index.txt | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt
index 7386c93..e091b2a 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -171,6 +171,12 @@ may not support it yet.
given again, all changes in $GIT_DIR/index are pushed back to
the shared index file. This mode is designed for very large
indexes that take a significant amount of time to read or write.
++
+These options take effect whatever the value of the `core.splitIndex`
+configuration variable (see linkgit:git-config[1]). But a warning is
+emitted when the change goes against the configured value, as the
+configured value will take effect next time the index is read and this
+will remove the intended effect of the option.
--untracked-cache::
--no-untracked-cache::
--
2.10.1.462.g7e1e03a
^ permalink raw reply related
* [PATCH v1 16/19] read-cache: unlink old sharedindex files
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>
Everytime split index is turned on, it creates a "sharedindex.XXXX"
file in the git directory. This change makes sure that shared index
files that haven't been used for a long time are removed when a new
shared index file is created.
The new "splitIndex.sharedIndexExpire" config variable is created
to tell the delay after which an unused shared index file can be
deleted. It defaults to "1.week.ago".
A previous commit made sure that each time a split index file is
created the mtime of the shared index file it references is updated.
This makes sure that recently used shared index file will not be
deleted.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
read-cache.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 62 insertions(+), 1 deletion(-)
diff --git a/read-cache.c b/read-cache.c
index 3aeff77..65ceb29 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2190,6 +2190,64 @@ static int write_split_index(struct index_state *istate,
return ret;
}
+static const char *shared_index_expire = "1.week.ago";
+
+static unsigned long get_shared_index_expire_date(void)
+{
+ static unsigned long shared_index_expire_date;
+ static int shared_index_expire_date_prepared;
+
+ if (!shared_index_expire_date_prepared) {
+ git_config_get_date_string("splitindex.sharedindexexpire",
+ &shared_index_expire);
+ shared_index_expire_date = approxidate(shared_index_expire);
+ shared_index_expire_date_prepared = 1;
+ }
+
+ return shared_index_expire_date;
+}
+
+static int can_delete_shared_index(const char *shared_sha1_hex)
+{
+ struct stat st;
+ unsigned long expiration;
+ const char *shared_index = git_path("sharedindex.%s", shared_sha1_hex);
+
+ /* Check timestamp */
+ expiration = get_shared_index_expire_date();
+ if (!expiration)
+ return 0;
+ if (stat(shared_index, &st))
+ return error_errno("could not stat '%s", shared_index);
+ if (st.st_mtime > expiration)
+ return 0;
+
+ return 1;
+}
+
+static void clean_shared_index_files(const char *current_hex)
+{
+ struct dirent *de;
+ DIR *dir = opendir(get_git_dir());
+
+ if (!dir) {
+ error_errno("unable to open git dir: %s", get_git_dir());
+ return;
+ }
+
+ while ((de = readdir(dir)) != NULL) {
+ const char *sha1_hex;
+ if (!skip_prefix(de->d_name, "sharedindex.", &sha1_hex))
+ continue;
+ if (!strcmp(sha1_hex, current_hex))
+ continue;
+ if (can_delete_shared_index(sha1_hex) > 0 &&
+ unlink(git_path("%s", de->d_name)))
+ error_errno("unable to unlink: %s", git_path("%s", de->d_name));
+ }
+ closedir(dir);
+}
+
static struct tempfile temporary_sharedindex;
static int write_shared_index(struct index_state *istate,
@@ -2211,8 +2269,11 @@ static int write_shared_index(struct index_state *istate,
}
ret = rename_tempfile(&temporary_sharedindex,
git_path("sharedindex.%s", sha1_to_hex(si->base->sha1)));
- if (!ret)
+ if (!ret) {
hashcpy(si->base_sha1, si->base->sha1);
+ clean_shared_index_files(sha1_to_hex(si->base->sha1));
+ }
+
return ret;
}
--
2.10.1.462.g7e1e03a
^ permalink raw reply related
* [PATCH v1 19/19] Documentation/git-update-index: explain 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/git-update-index.txt | 33 +++++++++++++++++++++++++--------
1 file changed, 25 insertions(+), 8 deletions(-)
diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt
index e091b2a..635d157 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -163,14 +163,10 @@ may not support it yet.
--split-index::
--no-split-index::
- Enable or disable split index mode. If enabled, the index is
- split into two files, $GIT_DIR/index and $GIT_DIR/sharedindex.<SHA-1>.
- Changes are accumulated in $GIT_DIR/index while the shared
- index file contains all index entries stays unchanged. 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. This mode is designed for very large
- indexes that take a significant amount of time to read or write.
+ Enable or disable split index mode. 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.
+
These options take effect whatever the value of the `core.splitIndex`
configuration variable (see linkgit:git-config[1]). But a warning is
@@ -394,6 +390,27 @@ Although this bit looks similar to assume-unchanged bit, its goal is
different from assume-unchanged bit's. Skip-worktree also takes
precedence over assume-unchanged bit when both are set.
+Split index
+-----------
+
+This mode is designed for very large indexes that take a significant
+amount of time to read or write.
+
+In this mode, the index is split into two files, $GIT_DIR/index and
+$GIT_DIR/sharedindex.<SHA-1>. Changes are accumulated in
+$GIT_DIR/index, the split index, while the shared index file contains
+all index entries and stays unchanged.
+
+All changes in the split index are pushed back to the shared index
+file when the number of entries in the split index reaches a level
+specified by the splitIndex.maxPercentChange config variable (see
+linkgit:git-config[1]).
+
+Each time a new shared index file is created, the old shared index
+files are deleted if they are older than what is specified by the
+splitIndex.sharedIndexExpire config variable (see
+linkgit:git-config[1]).
+
Untracked cache
---------------
--
2.10.1.462.g7e1e03a
^ permalink raw reply related
* [PATCH v1 18/19] Documentation/config: add splitIndex.sharedIndexExpire
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 | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 380eeb8..5212a97 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2776,6 +2776,17 @@ splitIndex.maxPercentChange::
than 20 percent of the total number of entries.
See linkgit:git-update-index[1].
+splitIndex.sharedIndexExpire::
+ When the split index feature is used, shared index files with
+ a mtime older than this time will be removed when a new shared
+ index file is created. The value "now" expires all entries
+ immediately, and "never" suppresses expiration altogether.
+ The default value is "one.week.ago".
+ Note that each time a new split-index file is created, the
+ mtime of the related shared index file is updated to the
+ current time.
+ 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 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
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