* Re: RFC: Another proposed hash function transition plan
From: brian m. carlson @ 2017-03-06 0:26 UTC (permalink / raw)
To: Linus Torvalds
Cc: Jonathan Nieder, Git Mailing List, Stefan Beller, bmwill,
jonathantanmy, Jeff King
In-Reply-To: <CA+55aFz+gkAsDZ24zmePQuEs1XPS9BP_s8O7Q4wQ7LV7X5-oDA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1818 bytes --]
On Sat, Mar 04, 2017 at 06:35:38PM -0800, Linus Torvalds wrote:
> On Fri, Mar 3, 2017 at 5:12 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> >
> > This document is still in flux but I thought it best to send it out
> > early to start getting feedback.
>
> This actually looks very reasonable if you can implement it cleanly
> enough. In many ways the "convert entirely to a new 256-bit hash" is
> the cleanest model, and interoperability was at least my personal
> concern. Maybe your model solves it (devil in the details), in which
> case I really like it.
If you think you can do it, I'm all for it.
> Btw, I do think the particular choice of hash should still be on the
> table. sha-256 may be the obvious first choice, but there are
> definitely a few reasons to consider alternatives, especially if it's
> a complete switch-over like this.
>
> One is large-file behavior - a parallel (or tree) mode could improve
> on that noticeably. BLAKE2 does have special support for that, for
> example. And SHA-256 does have known attacks compared to SHA-3-256 or
> BLAKE2 - whether that is due to age or due to more effort, I can't
> really judge. But if we're switching away from SHA1 due to known
> attacks, it does feel like we should be careful.
I agree with Linus on this. SHA-256 is the slowest option, and it's the
one with the most advanced cryptanalysis. SHA-3-256 is faster on 64-bit
machines (which, as we've seen on the list, is the overwhelming majority
of machines using Git), and even BLAKE2b-256 is stronger.
Doing this all over again in another couple years should also be a
non-goal.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]
^ permalink raw reply
* Re: Delta compression not so effective
From: Linus Torvalds @ 2017-03-06 1:14 UTC (permalink / raw)
To: Marius Storm-Olsen; +Cc: Git Mailing List
In-Reply-To: <9961a973-0d5d-5ff9-ab78-eea07bdb5dbf@gmail.com>
On Sat, Mar 4, 2017 at 12:27 AM, Marius Storm-Olsen <mstormo@gmail.com> wrote:
>
> I reran the repack with the options above (dropping the zlib=9, as you
> suggested)
>
> $ time git -c pack.threads=4 repack -a -d -F \
> --window=350 --depth=250 --window-memory=30g
>
> and ended up with
> $ du -sh .
> 205G .
>
> In other words, going from 6G to 30G window didn't help a lick on finding
> deltas for those binaries.
Ok.
> I did
> fprintf(stderr, "%s %u %lu\n",
> sha1_to_hex(delta_list[i]->idx.sha1),
> delta_list[i]->hash,
> delta_list[i]->size);
>
> I assume that's correct?
Looks good.
> I've removed all commit messages, and "sanitized" some filepaths etc, so
> name hashes won't match what's reported, but that should be fine. (the
> object_entry->hash seems to be just a trivial uint32 hash for sorting
> anyways)
Yes. I see your name list and your pack-file index.
> BUT, if I look at the last 3 entries of the sorted git verify-pack output,
> and look for them in the 'git log --oneline --raw -R --abbrev=40' output, I
> get:
...
> while I cannot find ANY of them in the delta_list output?? \
Yes. You have a lot of of object names in that log file you sent in
private that aren't in the delta list.
Now, objects smaller than 50 bytes we don't ever try to even delta. I
can't see the object sizes when they don't show up in the delta list,
but looking at some of those filenames I'd expect them to not fall in
that category.
I guess you could do the printout a bit earlier (on the
"to_pack.objects[]" array - to_pack.nr_objects is the count there).
That should show all of them. But the small objects shouldn't matter.
But if you have a file like
extern/win/FlammableV3/x64/lib/FlameProxyLibD.lib
I would have assumed that it has a size that is > 50. Unless those
"extern" things are placeholders?
> You might get an idea for how to easily create a repo which reproduces the
> issue, and which would highlight it more easily for the ML.
Looking at your sorted object list ready for packing, it doesn't look
horrible. When sorting for size, it still shows a lot of those large
files with the same name hash, so they sorted together in that form
too.
I do wonder if your dll data just simply is absolutely horrible for
xdelta. We've also limited the delta finding a bit, simply because it
had some O(m*n) behavior that gets very expensive on some patterns.
Maybe your blobs trigger some of those case.
The diff-delta work all goes back to 2005 and 2006, so it's a long time ago.
What I'd ask you to do is try to find if you could make a reposity of
just one of the bigger DLL's with its history, particularly if you can
find some that you don't think is _that_ sensitive.
Looking at it, for example, I see that you have that file
extern/redhat-5/FlammableV3/x64/plugins/libFlameCUDA-3.0.703.so
that seems to have changed several times, and is a largish blob. Could
you try creating a repository with git fast-import that *only*
contains that file (or pick another one), and see if that delta's
well?
And if you find some case that doesn't xdelta well, and that you feel
you could make available outside, we could have a test-case...
Linus
^ permalink raw reply
* Re: [PATCH] pull: do not segfault when HEAD refers to missing object file
From: Jeff King @ 2017-03-06 3:51 UTC (permalink / raw)
To: brian m. carlson, André Laszlo, git,
Nguyễn Thái Ngọc Duy, René Scharfe,
Johannes Schindelin
In-Reply-To: <20170305235222.vxia7jw2n5uj2h2e@genre.crustytoothpaste.net>
On Sun, Mar 05, 2017 at 11:52:22PM +0000, brian m. carlson wrote:
> On Mon, Mar 06, 2017 at 12:42:22AM +0100, André Laszlo wrote:
> > +test_expect_success 'git pull --rebase with corrupt HEAD does not segfault' '
> > + mkdir corrupted &&
> > + (cd corrupted &&
> > + git init &&
> > + echo one >file && git add file &&
> > + git commit -m one &&
> > + REV=$(git rev-parse HEAD) &&
> > + rm -f .git/objects/${REV:0:2}/${REV:2} &&
>
> I think this is a bashism. On dash, I get the following:
>
> genre ok % dash -c 'foo=abcdefg; echo ${foo:0:2}; echo ${foo:2}'
> dash: 1: Bad substitution
Yeah, it is. You can do it easily with 'sed', of course, but if you want
to avoid the extra process and do it in pure shell, it's more like:
last38=${REV#??}
first2=${REV%$last38}
rm -f .git/objects/$first2/$last38
-Peff
^ permalink raw reply
* Re: Git download
From: Torsten Bögershausen @ 2017-03-06 5:33 UTC (permalink / raw)
To: Cory Kilpatrick, git
In-Reply-To: <6BB71432-FB8F-458B-A1FF-EBF93565E6D8@gmail.com>
On 03/05/2017 09:26 PM, Cory Kilpatrick wrote:
> I have downloaded Git and cannot find the application on my Mac. Should I try to download it again?
I don't think so.
It could be helpful if we can get some more information:
- Could you open the terminal application and type
which git
git --version
and post the results here ?
It may be worth to mention that Git is a command line tool, so that you
may not
see anything in the "Applications" folder.
^ permalink raw reply
* [PATCH v2] git svn: fix authenticaton with 'branch'
From: Hiroshi Shirosaki @ 2017-03-06 5:59 UTC (permalink / raw)
To: e; +Cc: git, Hiroshi Shirosaki
Authentication fails with svn branch while svn rebase and
svn dcommit work fine without authentication failures.
$ git svn branch v7_3
Copying https://xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx at r27519
to https://xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/v7_3...
Can't create session: Unable to connect to a repository at URL
'https://xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx': No more
credentials or we tried too many times.
Authentication failed at
C:\Program Files\Git\mingw64/libexec/git-core\git-svn line 1200.
We add auth configuration to SVN::Client->new() to fix the issue.
Signed-off-by: Hiroshi Shirosaki <h.shirosaki@gmail.com>
---
git-svn.perl | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index fa42364..d240418 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1175,10 +1175,10 @@ sub cmd_branch {
::_req_svn();
require SVN::Client;
+ my ($config, $baton, undef) = Git::SVN::Ra::prepare_config_once();
my $ctx = SVN::Client->new(
- config => SVN::Core::config_get_config(
- $Git::SVN::Ra::config_dir
- ),
+ auth => $baton,
+ config => $config,
log_msg => sub {
${ $_[0] } = defined $_message
? $_message
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] pull: do not segfault when HEAD refers to missing object file
From: Johannes Sixt @ 2017-03-06 6:52 UTC (permalink / raw)
To: Jeff King, André Laszlo
Cc: brian m. carlson, git, Nguyễn Thái Ngọc Duy,
René Scharfe, Johannes Schindelin
In-Reply-To: <20170306035152.c7bh5jiqrfncyudl@sigill.intra.peff.net>
Am 06.03.2017 um 04:51 schrieb Jeff King:
> On Sun, Mar 05, 2017 at 11:52:22PM +0000, brian m. carlson wrote:
>
>> On Mon, Mar 06, 2017 at 12:42:22AM +0100, André Laszlo wrote:
>>> +test_expect_success 'git pull --rebase with corrupt HEAD does not segfault' '
>>> + mkdir corrupted &&
>>> + (cd corrupted &&
We usally indent this like so:
(
cd corrupted &&
echo one >file &&
git add file &&
...
) &&
>>> + git init &&
>>> + echo one >file && git add file &&
>>> + git commit -m one &&
>>> + REV=$(git rev-parse HEAD) &&
>>> + rm -f .git/objects/${REV:0:2}/${REV:2} &&
>>
>> I think this is a bashism. On dash, I get the following:
>>
>> genre ok % dash -c 'foo=abcdefg; echo ${foo:0:2}; echo ${foo:2}'
>> dash: 1: Bad substitution
>
> Yeah, it is. You can do it easily with 'sed', of course, but if you want
> to avoid the extra process and do it in pure shell, it's more like:
>
> last38=${REV#??}
> first2=${REV%$last38}
> rm -f .git/objects/$first2/$last38
Is it "HEAD points to non-existent object" or ".git/HEAD contains junk"?
In both cases there are simpler solutions than to remove an object. For
example, `echo "$_x40" >.git/HEAD` or `echo "this is junk" >.git/HEAD`?
-- Hannes
^ permalink raw reply
* Re: [PATCH] pull: do not segfault when HEAD refers to missing object file
From: Jeff King @ 2017-03-06 3:46 UTC (permalink / raw)
To: André Laszlo
Cc: git, Nguyễn Thái Ngọc Duy, brian m . carlson,
René Scharfe, Johannes Schindelin
In-Reply-To: <20170305234222.4590-1-andre@laszlo.nu>
On Mon, Mar 06, 2017 at 12:42:22AM +0100, André Laszlo wrote:
> git pull --rebase on a corrupted HEAD used to segfault;it has been
> corrected to error out with a message. A test has also been added to
> verify this fix.
Your commit message mostly tells us the "what" that we can see from the
diff. But why are we segfaulting? What assumption is being violated, and
why is the fix the right thing?
You've stuck some of the details in your notes, and they should probably
make it into the commit message.
> When add_head_to_pending fails to add a pending object, git pull
> --rebase segfaults. This can happen if HEAD is referring to a corrupt
> or missing object.
The other obvious time when HEAD is not valid is when you are on an
unborn branch. So we should also consider how such a case interacts with
the callsites you are touching.
I think it is primarily this hunk:
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -2252,6 +2252,11 @@ int has_uncommitted_changes(int ignore_submodules)
> DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES);
> DIFF_OPT_SET(&rev_info.diffopt, QUICK);
> add_head_to_pending(&rev_info);
> +
> + /* The add_head_to_pending call might not have added anything. */
> + if (!rev_info.pending.nr)
> + die("bad object %s", "HEAD");
> +
> diff_setup_done(&rev_info.diffopt);
> result = run_diff_index(&rev_info, 1);
> return diff_result_code(&rev_info.diffopt, result);
that is the fix. We assume that add_head_to_pending() put something into
rev_info.pending, but it might not.
In the case of corruption, "bad object HEAD" is a reasonable outcome.
In the case of an unborn branch, we'd probably want to compare against
the empty tree. This trick is used elsewhere in wt-status.c, as in
wt_status_collect_changes_index().
I'm not sure if we need to deal with that here or not. I wasn't able to
trigger this code with an unborn branch. If you have no index, then the
is_cache_unborn() check triggers earlier in the function. If you do have
an index, then we have an earlier check:
if (is_null_sha1(orig_head) && !is_cache_unborn())
die(_("Updating an unborn branch with changes added to the index."));
that covers this case. I am not 100% sure that check is correct, though.
If I do:
git init
echo content >foo
git add foo
git rm -f foo
then I have an index which is not "unborn", but also does not contain
any changes. I think the conditional above should just be checking
"active_nr". If we were to fix that, then I think
has_uncommitted_changes() would need to be adjusted as well.
I admit that this is probably an absurd corner case. So I think I am OK
with your fix for now, and we can revisit the logic later if anybody
starts to care.
> I discovered this segfault on my machine after pulling a repo from
> GitHub, but I have been unable to reproduce the sequence of events
> that lead to the corrupted HEAD (I think it may have been caused by a
> lost network connection in my case).
Yikes. That should never lead to a corrupted HEAD (unless you are on a
networked filesystem). I can't think of another read add_head_to_pending
would fail, though, aside from a missing or corrupted object.
Arguably add_head_to_pending() should die loudly if it can't parse the
object pointed to by HEAD, rather than quietly returning without adding
anything.
> diff --git a/diff-lib.c b/diff-lib.c
> index 52447466b..9d26b18c3 100644
> --- a/diff-lib.c
> +++ b/diff-lib.c
> @@ -512,7 +512,7 @@ int run_diff_index(struct rev_info *revs, int cached)
> struct object_array_entry *ent;
>
> ent = revs->pending.objects;
> - if (diff_cache(revs, ent->item->oid.hash, ent->name, cached))
> + if (!ent || diff_cache(revs, ent->item->oid.hash, ent->name, cached))
> exit(128);
So if I understand correctly, this hunk should not trigger anymore,
because we would never call run_diff_index() without something in
pending.
I think it would be a programming error elsewhere to do so, and we
should treat this as an assertion by complaining loudly (for this and
any other confusing cases):
if (revs->pending.nr != 1)
die("BUG: run_diff_index requires exactly one rev (got %d)",
revs->pending.nr);
Lastly, I think this is your first patch. So welcome, and thank you. :)
I know there was a lot of discussion and critique above, but I think
overall your patch is going in the right direction.
-Peff
^ permalink raw reply
* Re: [PATCH] pull: do not segfault when HEAD refers to missing object file
From: Jeff King @ 2017-03-06 7:33 UTC (permalink / raw)
To: Johannes Sixt
Cc: André Laszlo, brian m. carlson, git,
Nguyễn Thái Ngọc Duy, René Scharfe,
Johannes Schindelin
In-Reply-To: <6726b36d-6f50-d258-12e3-8b7b56159631@kdbg.org>
On Mon, Mar 06, 2017 at 07:52:10AM +0100, Johannes Sixt wrote:
> > Yeah, it is. You can do it easily with 'sed', of course, but if you want
> > to avoid the extra process and do it in pure shell, it's more like:
> >
> > last38=${REV#??}
> > first2=${REV%$last38}
> > rm -f .git/objects/$first2/$last38
>
> Is it "HEAD points to non-existent object" or ".git/HEAD contains junk"? In
> both cases there are simpler solutions than to remove an object. For
> example, `echo "$_x40" >.git/HEAD` or `echo "this is junk" >.git/HEAD`?
Good point. Unfortunately, it's a little tricky. You can't use "this is
junk" because then git does not recognize it as a git repo at all. You
can't use $_x40 because the null sha1 is used internally to signal an
unborn branch, so we mix it up with that case.
Something like "111..." for 40 characters would work, though at that
point I think just simulating an actual corruption might be less
convoluted.
-Peff
^ permalink raw reply
* Re: RFC: Another proposed hash function transition plan
From: Jeff King @ 2017-03-06 8:43 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, sbeller, bmwill, jonathantanmy, Linus Torvalds
In-Reply-To: <20170304011251.GA26789@aiede.mtv.corp.google.com>
On Fri, Mar 03, 2017 at 05:12:51PM -0800, Jonathan Nieder wrote:
> This past week we came up with this idea for what a transition to a new
> hash function for Git would look like. I'd be interested in your
> thoughts (especially if you can make them as comments on the document,
> which makes it easier to address them and update the document).
Overall it's an interesting idea. I thought at first that you were
suggesting servers do on-the-fly conversion, but after a more careful
reading that isn't the case. And I don't think that would work, because
the conversion is expensive.
So this pushes the conversion cost onto the clients who decide to move
to SHA-256. That may be a problem for sites which have a lot of clients
(like CI hosts). But I guess they would just stick with SHA-1 as long as
possible, until the upstream repo switches (and that _is_ a per-repo
flag day, because the upstream host isn't going to convert back to SHA-1
on the fly to serve the old clients).
> You can use the doc URL
>
> https://goo.gl/gh2Mzc
I'd encourage anybody following along to follow that link. I almost
didn't, but there are a ton of comments there (I'm not sure how I feel
about splitting the discussion off the list, though).
> Goals
> -----
> 1. The transition to SHA256 can be done one local repository at a time.
> a. Requiring no action by any other party.
> b. A SHA256 repository can communicate with SHA-1 Git servers and
> clients (push/fetch).
> c. Users can use SHA-1 and SHA256 identifiers for objects
> interchangeably.
> d. New signed objects make use of a stronger hash function than
> SHA-1 for their security guarantees.
> 2. Allow a complete transition away from SHA-1.
> a. Local metadata for SHA-1 compatibility can be dropped in a
> repository if compatibility with SHA-1 is no longer needed.
I suspect we'll never get away from keeping the mapping table. You'll
need at least the sha1->sha256 table if you want to look up names found
in historic commit messages, mailing list posts, etc.
And you'll need the sha256->sha1 table if you want to verify the gpg
signatures on old tags and commits. That might be something people are
willing to drop, though.
> After negotiation, the server sends a packfile containing the
> requested objects. We convert the packfile to SHA-256 format using the
> following steps:
>
> 1. index-pack: inflate each object in the packfile and compute its
> SHA-1. Objects can contain deltas in OBJ_REF_DELTA format against
> objects the client has locally. These objects can be looked up using
> the translation table and their sha1-content read as described above
> to resolve the deltas.
> 2. topological sort: starting at the "want"s from the negotiation
> phase, walk through objects in the pack and emit a list of them in
> topologically sorted order. (This list only contains objects
> reachable from the "wants". If the pack from the server contained
> additional extraneous objects, then they will be discarded.)
I don't think we do this right now, but you can actually find the entry
(and exit) points of a pack during the index-pack step. Basically:
1. Keep a hashmap of objects mentioned in the pack.
2. When we process an object's content (i.e., compute its hash), also
parse it for any object references. Add entries in the hashmap for
any object mentioned this way. Mark the entry for the object we
processed with a "HAVE" bit, and mark any referenced object with a
"REF" bit.
3. After processing all objects, anything with a "HAVE" but no "REF"
is an entry point to the pack (i.e., something that we should have
asked for with a want). Anything with a "REF" but not a "HAVE" is
an exit point (i.e., an object that we are expected to already have
in our repo).
(I've thought about this before because we could possibly shortcut
the connectivity check using the exit points. It's complicated by
the fact that we don't assume the transitive presence of objects
unless they are reachable).
I don't think using the "want"s as the entry points is unreasonable,
though. The server _shouldn't_ generally be sending us other cruft.
I do wonder if you might be able to omit the extra object-graph walk
from your step 2, if you could assign "depths" to each object during
step 1 instead of HAVE/REF bits. The trouble, of course, is that you're
not visiting the nodes in the right order (so given two trees, you're
not sure if one might eventually be a child of the other; how do you
assign their depths?). I have a feeling there's a proof that it's
impossible, but I might just not be clever enough.
Overall the basics of the conversion seem sound to me. The "nohash"
things seems more complicated than I think it ought to be, which
probably just means I'm missing something. I left a few related
comments on the google doc, so I won't repeat them here.
-Peff
^ permalink raw reply
* Re: RFC: Another proposed hash function transition plan
From: Jeff King @ 2017-03-06 9:43 UTC (permalink / raw)
To: ankostis
Cc: David Lang, Jonathan Nieder, Git Mailing List, Stefan Beller,
bmwill, jonathantanmy, Linus Torvalds
In-Reply-To: <CA+dhYEXHbQfJ6KUB1tWS9u1MLEOJL81fTYkbxu4XO-i+379LPw@mail.gmail.com>
On Mon, Mar 06, 2017 at 10:29:33AM +0100, ankostis wrote:
> On 5 March 2017 at 12:02, David Lang <david@lang.hm> wrote:
> >> Translation table
> >> ~~~~~~~~~~~~~~~~~
> >> A fast bidirectional mapping between sha1-names and sha256-names of
> >> all local objects in the repository is kept on disk. The exact format
> >> of that mapping is to be determined.
> >>
> >> All operations that make new objects (e.g., "git commit") add the new
> >> objects to the translation table.
> >
> >
> > This seems like a rather nontrival thing to design. It will need to hold
> > millions of mappings, and be quickly searchable from either direction
> > (sha1->new and new->sha1) while still be fairly fast to insert new records
> > into.
> >
> > For Linux, just the list of hashes recording the commits is going to be in
> > the millions, whiel the list of hashes of individual files for all those
> > commits is going to be substantially larger.
>
> Apologies if it is a stupid idea, but could we avoid the mappings-table
> just by
> hard-linking to the same object from both (or more) hashes?
> So instead of creating a text-db format, just use the filesystem.
No, for a few reasons:
1. Most of these objects will not be in the filesystem at all, but
rather in a packfile.
2. It's not just a different hash over the same bytes. The sha256-name
is taken over the sha256-content (which refers to other objects
using sha256). So they really are different objects. You probably
wouldn't keep the sha1 version around separately, but rather
generate it on the fly during a push to a sha1 server.
3. You really need to be able to take a sha256 name and convert it to
a sha1 and vice versa. Hardlinks don't help with that, because they
only point in one direction. That get you to the same _content_,
but not the other name (and I guess this is where your "look up the
name and then compute the other digest comes in, but that's
probably too expensive to be workable).
I do think updating the mapping could potentially be deferred until
interacting with a sha1 server. But because it needs to be generated in
reverse-topological order, it's conceptually easier to do it one object
at a time.
-Peff
^ permalink raw reply
* [PATCH v5 11/22] read-cache: regenerate shared index if necessary
From: Christian Couder @ 2017-03-06 9:41 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Ramsay Jones, Jeff King,
Christian Couder
In-Reply-To: <20170306094203.28250-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 | 32 ++++++++++++++++++++++++++++++++
t/t1700-split-index.sh | 1 +
2 files changed, 33 insertions(+)
diff --git a/read-cache.c b/read-cache.c
index 99bc274b8d..aeb413a508 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2212,6 +2212,36 @@ static int write_shared_index(struct index_state *istate,
return ret;
}
+static const int default_max_percent_split_change = 20;
+
+static 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:
+ break; /* 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 (int64_t)istate->cache_nr * max_split < (int64_t)not_shared * 100;
+}
+
int write_locked_index(struct index_state *istate, struct lock_file *lock,
unsigned flags)
{
@@ -2229,6 +2259,8 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,
if ((v & 15) < 6)
istate->cache_changed |= SPLIT_INDEX_ORDERED;
}
+ if (too_many_not_shared_entries(istate))
+ istate->cache_changed |= SPLIT_INDEX_ORDERED;
if (istate->cache_changed & SPLIT_INDEX_ORDERED) {
int ret = write_shared_index(istate, lock, flags);
if (ret)
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index aa2aff1778..9d7c01c3e1 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.12.0.206.g74921e51d6.dirty
^ permalink raw reply related
* [PATCH v5 17/22] read-cache: unlink old sharedindex files
From: Christian Couder @ 2017-03-06 9:41 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Ramsay Jones, Jeff King,
Christian Couder
In-Reply-To: <20170306094203.28250-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 "2.weeks.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 | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 63 insertions(+), 1 deletion(-)
diff --git a/read-cache.c b/read-cache.c
index 13375fa0ff..16c05f359b 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2199,6 +2199,65 @@ static int write_split_index(struct index_state *istate,
return ret;
}
+static const char *shared_index_expire = "2.weeks.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_expiry("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 should_delete_shared_index(const char *shared_index_path)
+{
+ struct stat st;
+ unsigned long expiration;
+
+ /* Check timestamp */
+ expiration = get_shared_index_expire_date();
+ if (!expiration)
+ return 0;
+ if (stat(shared_index_path, &st))
+ return error_errno(_("could not stat '%s"), shared_index_path);
+ if (st.st_mtime > expiration)
+ return 0;
+
+ return 1;
+}
+
+static int clean_shared_index_files(const char *current_hex)
+{
+ struct dirent *de;
+ DIR *dir = opendir(get_git_dir());
+
+ if (!dir)
+ return error_errno(_("unable to open git dir: %s"), get_git_dir());
+
+ while ((de = readdir(dir)) != NULL) {
+ const char *sha1_hex;
+ const char *shared_index_path;
+ if (!skip_prefix(de->d_name, "sharedindex.", &sha1_hex))
+ continue;
+ if (!strcmp(sha1_hex, current_hex))
+ continue;
+ shared_index_path = git_path("%s", de->d_name);
+ if (should_delete_shared_index(shared_index_path) > 0 &&
+ unlink(shared_index_path))
+ warning_errno(_("unable to unlink: %s"), shared_index_path);
+ }
+ closedir(dir);
+
+ return 0;
+}
+
static struct tempfile temporary_sharedindex;
static int write_shared_index(struct index_state *istate,
@@ -2220,8 +2279,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.12.0.206.g74921e51d6.dirty
^ permalink raw reply related
* [PATCH v5 09/22] Documentation/git-update-index: talk about core.splitIndex config var
From: Christian Couder @ 2017-03-06 9:41 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Ramsay Jones, Jeff King,
Christian Couder
In-Reply-To: <20170306094203.28250-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 7386c93162..e091b2a409 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.12.0.206.g74921e51d6.dirty
^ permalink raw reply related
* [PATCH v5 22/22] Documentation/git-update-index: explain splitIndex.*
From: Christian Couder @ 2017-03-06 9:42 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Ramsay Jones, Jeff King,
Christian Couder
In-Reply-To: <20170306094203.28250-1-chriscool@tuxfamily.org>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/config.txt | 2 +-
Documentation/git-update-index.txt | 37 +++++++++++++++++++++++++++++--------
2 files changed, 30 insertions(+), 9 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 2afd5d982b..24ed9c476d 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2873,7 +2873,7 @@ splitIndex.sharedIndexExpire::
The default value is "2.weeks.ago".
Note that a shared index file is considered modified (for the
purpose of expiration) each time a new split-index file is
- created based on it.
+ either created based on it or read from it.
See linkgit:git-update-index[1].
status.relativePaths::
diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt
index e091b2a409..1579abf3c3 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,31 @@ 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 repositories with very large indexes, and
+aims at reducing the time it takes to repeatedly write these indexes.
+
+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 their modification time is older than what is
+specified by the splitIndex.sharedIndexExpire config variable (see
+linkgit:git-config[1]).
+
+To avoid deleting a shared index file that is still used, its
+modification time is updated to the current time everytime a new split
+index based on the shared index file is either created or read from.
+
Untracked cache
---------------
--
2.12.0.206.g74921e51d6.dirty
^ permalink raw reply related
* [PATCH v5 14/22] sha1_file: make check_and_freshen_file() non static
From: Christian Couder @ 2017-03-06 9:41 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Ramsay Jones, Jeff King,
Christian Couder
In-Reply-To: <20170306094203.28250-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 0bb9adcd31..a35e9d5187 100644
--- a/cache.h
+++ b/cache.h
@@ -1273,6 +1273,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 6628f06da3..f02f10189f 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -667,7 +667,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.12.0.206.g74921e51d6.dirty
^ permalink raw reply related
* [PATCH v5 20/22] read-cache: use freshen_shared_index() in read_index_from()
From: Christian Couder @ 2017-03-06 9:42 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Ramsay Jones, Jeff King,
Christian Couder
In-Reply-To: <20170306094203.28250-1-chriscool@tuxfamily.org>
This way a share index file will not be garbage collected if
we still read from an index it is based from.
As we need to read the current index before creating a new
one, the tests have to be adjusted, so that we don't expect
an old shared index file to be deleted right away when we
create a new one.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
read-cache.c | 1 +
t/t1700-split-index.sh | 14 +++++++-------
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index 89c95d59b3..e447751823 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1719,6 +1719,7 @@ int read_index_from(struct index_state *istate, const char *path)
base_sha1_hex, base_path,
sha1_to_hex(split_index->base->sha1));
+ freshen_shared_index(base_sha1_hex, 0);
merge_base_index(istate);
post_read_index_from(istate);
return ret;
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index f5a95a6c28..af3ec0da5a 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -329,17 +329,17 @@ test_expect_success 'check splitIndex.maxPercentChange set to 0' '
test_expect_success 'shared index files expire after 2 weeks by default' '
: >ten &&
git update-index --add ten &&
- test $(ls .git/sharedindex.* | wc -l) -gt 1 &&
+ test $(ls .git/sharedindex.* | wc -l) -gt 2 &&
just_under_2_weeks_ago=$((5-14*86400)) &&
test-chmtime =$just_under_2_weeks_ago .git/sharedindex.* &&
: >eleven &&
git update-index --add eleven &&
- test $(ls .git/sharedindex.* | wc -l) -gt 1 &&
+ test $(ls .git/sharedindex.* | wc -l) -gt 2 &&
just_over_2_weeks_ago=$((-1-14*86400)) &&
test-chmtime =$just_over_2_weeks_ago .git/sharedindex.* &&
: >twelve &&
git update-index --add twelve &&
- test $(ls .git/sharedindex.* | wc -l) = 1
+ test $(ls .git/sharedindex.* | wc -l) -le 2
'
test_expect_success 'check splitIndex.sharedIndexExpire set to 16 days' '
@@ -347,12 +347,12 @@ test_expect_success 'check splitIndex.sharedIndexExpire set to 16 days' '
test-chmtime =$just_over_2_weeks_ago .git/sharedindex.* &&
: >thirteen &&
git update-index --add thirteen &&
- test $(ls .git/sharedindex.* | wc -l) -gt 1 &&
+ test $(ls .git/sharedindex.* | wc -l) -gt 2 &&
just_over_16_days_ago=$((-1-16*86400)) &&
test-chmtime =$just_over_16_days_ago .git/sharedindex.* &&
: >fourteen &&
git update-index --add fourteen &&
- test $(ls .git/sharedindex.* | wc -l) = 1
+ test $(ls .git/sharedindex.* | wc -l) -le 2
'
test_expect_success 'check splitIndex.sharedIndexExpire set to "never" and "now"' '
@@ -361,13 +361,13 @@ test_expect_success 'check splitIndex.sharedIndexExpire set to "never" and "now"
test-chmtime =$just_10_years_ago .git/sharedindex.* &&
: >fifteen &&
git update-index --add fifteen &&
- test $(ls .git/sharedindex.* | wc -l) -gt 1 &&
+ test $(ls .git/sharedindex.* | wc -l) -gt 2 &&
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 $(ls .git/sharedindex.* | wc -l) -le 2
'
test_done
--
2.12.0.206.g74921e51d6.dirty
^ permalink raw reply related
* [PATCH v5 19/22] read-cache: refactor read_index_from()
From: Christian Couder @ 2017-03-06 9:42 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Ramsay Jones, Jeff King,
Christian Couder
In-Reply-To: <20170306094203.28250-1-chriscool@tuxfamily.org>
It looks better and is simpler to review when we don't compute
the same things many times in the function.
It will also help make the following commit simpler.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
read-cache.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index 16c05f359b..89c95d59b3 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1691,6 +1691,8 @@ int read_index_from(struct index_state *istate, const char *path)
{
struct split_index *split_index;
int ret;
+ char *base_sha1_hex;
+ const char *base_path;
/* istate->initialized covers both .git/index and .git/sharedindex.xxx */
if (istate->initialized)
@@ -1708,15 +1710,15 @@ int read_index_from(struct index_state *istate, const char *path)
discard_index(split_index->base);
else
split_index->base = xcalloc(1, sizeof(*split_index->base));
- ret = do_read_index(split_index->base,
- git_path("sharedindex.%s",
- sha1_to_hex(split_index->base_sha1)), 1);
+
+ base_sha1_hex = sha1_to_hex(split_index->base_sha1);
+ base_path = git_path("sharedindex.%s", base_sha1_hex);
+ ret = do_read_index(split_index->base, base_path, 1);
if (hashcmp(split_index->base_sha1, split_index->base->sha1))
die("broken index, expect %s in %s, got %s",
- sha1_to_hex(split_index->base_sha1),
- git_path("sharedindex.%s",
- sha1_to_hex(split_index->base_sha1)),
+ base_sha1_hex, base_path,
sha1_to_hex(split_index->base->sha1));
+
merge_base_index(istate);
post_read_index_from(istate);
return ret;
--
2.12.0.206.g74921e51d6.dirty
^ permalink raw reply related
* [PATCH v5 13/22] Documentation/config: add splitIndex.maxPercentChange
From: Christian Couder @ 2017-03-06 9:41 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Ramsay Jones, Jeff King,
Christian Couder
In-Reply-To: <20170306094203.28250-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 f102879261..b64aa7db2d 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2851,6 +2851,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
+ total 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.12.0.206.g74921e51d6.dirty
^ permalink raw reply related
* [PATCH v5 10/22] config: add git_config_get_max_percent_split_change()
From: Christian Couder @ 2017-03-06 9:41 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Ramsay Jones, Jeff King,
Christian Couder
In-Reply-To: <20170306094203.28250-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 | 15 +++++++++++++++
2 files changed, 16 insertions(+)
diff --git a/cache.h b/cache.h
index 03de80daae..0bb9adcd31 100644
--- a/cache.h
+++ b/cache.h
@@ -1927,6 +1927,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 2a97696be7..35b6f02960 100644
--- a/config.c
+++ b/config.c
@@ -1746,6 +1746,21 @@ 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;
+
+ return error(_("splitIndex.maxPercentChange value '%d' "
+ "should be between 0 and 100"), val);
+ }
+
+ return -1; /* default value */
+}
+
NORETURN
void git_die_config_linenr(const char *key, const char *filename, int linenr)
{
--
2.12.0.206.g74921e51d6.dirty
^ permalink raw reply related
* [PATCH v5 06/22] update-index: warn in case of split-index incoherency
From: Christian Couder @ 2017-03-06 9:41 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Ramsay Jones, Jeff King,
Christian Couder
In-Reply-To: <20170306094203.28250-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 24fdadfa4b..d74d72cc7f 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1099,12 +1099,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.12.0.206.g74921e51d6.dirty
^ permalink raw reply related
* [PATCH v5 18/22] t1700: test shared index file expiration
From: Christian Couder @ 2017-03-06 9:41 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Ramsay Jones, Jeff King,
Christian Couder
In-Reply-To: <20170306094203.28250-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 00a64bed97..f5a95a6c28 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -326,4 +326,48 @@ test_expect_success 'check splitIndex.maxPercentChange set to 0' '
test_cmp expect actual
'
+test_expect_success 'shared index files expire after 2 weeks by default' '
+ : >ten &&
+ git update-index --add ten &&
+ test $(ls .git/sharedindex.* | wc -l) -gt 1 &&
+ just_under_2_weeks_ago=$((5-14*86400)) &&
+ test-chmtime =$just_under_2_weeks_ago .git/sharedindex.* &&
+ : >eleven &&
+ git update-index --add eleven &&
+ test $(ls .git/sharedindex.* | wc -l) -gt 1 &&
+ just_over_2_weeks_ago=$((-1-14*86400)) &&
+ test-chmtime =$just_over_2_weeks_ago .git/sharedindex.* &&
+ : >twelve &&
+ git update-index --add twelve &&
+ test $(ls .git/sharedindex.* | wc -l) = 1
+'
+
+test_expect_success 'check splitIndex.sharedIndexExpire set to 16 days' '
+ git config splitIndex.sharedIndexExpire "16.days.ago" &&
+ test-chmtime =$just_over_2_weeks_ago .git/sharedindex.* &&
+ : >thirteen &&
+ git update-index --add thirteen &&
+ test $(ls .git/sharedindex.* | wc -l) -gt 1 &&
+ just_over_16_days_ago=$((-1-16*86400)) &&
+ test-chmtime =$just_over_16_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.12.0.206.g74921e51d6.dirty
^ permalink raw reply related
* [PATCH v5 15/22] read-cache: touch shared index files when used
From: Christian Couder @ 2017-03-06 9:41 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Ramsay Jones, Jeff King,
Christian Couder
In-Reply-To: <20170306094203.28250-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 | 29 ++++++++++++++++++++++++++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index aeb413a508..13375fa0ff 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1674,6 +1674,19 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)
die("index file corrupt");
}
+/*
+ * Signal that the shared index is used by updating its mtime.
+ *
+ * This way, shared index can be removed if they have not been used
+ * for some time.
+ */
+static void freshen_shared_index(char *base_sha1_hex, int warn)
+{
+ const char *shared_index = git_path("sharedindex.%s", base_sha1_hex);
+ if (!check_and_freshen_file(shared_index, 1) && warn)
+ warning("could not freshen shared index '%s'", shared_index);
+}
+
int read_index_from(struct index_state *istate, const char *path)
{
struct split_index *split_index;
@@ -2245,6 +2258,7 @@ static int too_many_not_shared_entries(struct index_state *istate)
int write_locked_index(struct index_state *istate, struct lock_file *lock,
unsigned flags)
{
+ int new_shared_index, ret;
struct split_index *si = istate->split_index;
if (!si || alternate_index_output ||
@@ -2261,13 +2275,22 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,
}
if (too_many_not_shared_entries(istate))
istate->cache_changed |= SPLIT_INDEX_ORDERED;
- if (istate->cache_changed & SPLIT_INDEX_ORDERED) {
- int ret = write_shared_index(istate, lock, flags);
+
+ new_shared_index = istate->cache_changed & SPLIT_INDEX_ORDERED;
+
+ if (new_shared_index) {
+ ret = write_shared_index(istate, lock, flags);
if (ret)
return ret;
}
- return write_split_index(istate, lock, flags);
+ ret = write_split_index(istate, lock, flags);
+
+ /* Freshen the shared index only if the split-index was written */
+ if (!ret && !new_shared_index)
+ freshen_shared_index(sha1_to_hex(si->base_sha1), 1);
+
+ return ret;
}
/*
--
2.12.0.206.g74921e51d6.dirty
^ permalink raw reply related
* [PATCH v5 07/22] t1700: add tests for core.splitIndex
From: Christian Couder @ 2017-03-06 9:41 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Ramsay Jones, Jeff King,
Christian Couder
In-Reply-To: <20170306094203.28250-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 5ea227e6a1..aa2aff1778 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -216,4 +216,41 @@ test_expect_success 'rev-parse --shared-index-path' '
)
'
+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.12.0.206.g74921e51d6.dirty
^ permalink raw reply related
* [PATCH v5 16/22] config: add git_config_get_expiry() from gc.c
From: Christian Couder @ 2017-03-06 9:41 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Ramsay Jones, Jeff King,
Christian Couder
In-Reply-To: <20170306094203.28250-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 | 18 +++---------------
cache.h | 3 +++
config.c | 13 +++++++++++++
3 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index a2b9e8924e..56ab74f6ba 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -64,17 +64,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;
@@ -131,10 +120,9 @@ 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_date_string("gc.logexpiry", &gc_log_expire);
-
+ git_config_get_expiry("gc.pruneexpire", &prune_expire);
+ git_config_get_expiry("gc.worktreepruneexpire", &prune_worktrees_expire);
+ git_config_get_expiry("gc.logexpiry", &gc_log_expire);
git_config(git_default_config, NULL);
}
diff --git a/cache.h b/cache.h
index a35e9d5187..65ab507a76 100644
--- a/cache.h
+++ b/cache.h
@@ -1932,6 +1932,9 @@ 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 dies if the configured or default date is in the future */
+extern int git_config_get_expiry(const char *key, const char **output);
+
/*
* This is a hack for test programs like test-dump-untracked-cache to
* ensure that they do not modify the untracked cache when reading it.
diff --git a/config.c b/config.c
index 35b6f02960..f20d7d88f7 100644
--- a/config.c
+++ b/config.c
@@ -1712,6 +1712,19 @@ int git_config_get_pathname(const char *key, const char **dest)
return ret;
}
+int git_config_get_expiry(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.12.0.206.g74921e51d6.dirty
^ permalink raw reply related
* [PATCH v5 12/22] t1700: add tests for splitIndex.maxPercentChange
From: Christian Couder @ 2017-03-06 9:41 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Ramsay Jones, Jeff King,
Christian Couder
In-Reply-To: <20170306094203.28250-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 9d7c01c3e1..00a64bed97 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -254,4 +254,76 @@ test_expect_success 'set core.splitIndex config variable to false' '
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.12.0.206.g74921e51d6.dirty
^ 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