Re: [PATCH] add `ignore_missing_links` mode to revwalk

2014-03-31 Thread Siddharth Agarwal

On 03/28/2014 03:00 AM, Jeff King wrote:

From: Vicent Marti tan...@gmail.com

When pack-objects is computing the reachability bitmap to
serve a fetch request, it can erroneously die() if some of
the UNINTERESTING objects are not present. Upload-pack
throws away HAVE lines from the client for objects we do not
have, but we may have a tip object without all of its
ancestors (e.g., if the tip is no longer reachable and was
new enough to survive a `git prune`, but some of its
reachable objects did get pruned).


Thanks for this patch. It looks pretty sensible.

Unfortunately, I can't provide feedback on running it in production 
because we've decided to set aside experimenting with bitmaps for a bit. 
I hope to get back to it in a couple of months.






In the non-bitmap case, we do a revision walk with the HAVE
objects marked as UNINTERESTING. The revision walker
explicitly ignores errors in accessing UNINTERESTING commits
to handle this case (and we do not bother looking at
UNINTERESTING trees or blobs at all).

When we have bitmaps, however, the process is quite
different.  The bitmap index for a pack-objects run is
calculated in two separate steps:

First, we perform an extensive walk from all the HAVEs to
find the full set of objects reachable from them. This walk
is usually optimized away because we are expected to hit an
object with a bitmap during the traversal, which allows us
to terminate early.

Secondly, we perform an extensive walk from all the WANTs,
which usually also terminates early because we hit a commit
with an existing bitmap.

Once we have the resulting bitmaps from the two walks, we
AND-NOT them together to obtain the resulting set of objects
we need to pack.

When we are walking the HAVE objects, the revision walker
does not know that we are walking it only to mark the
results as uninteresting. We strip out the UNINTERESTING flag,
because those objects _are_ interesting to us during the
first walk. We want to keep going to get a complete set of
reachable objects if we can.

We need some way to tell the revision walker that it's OK to
silently truncate the HAVE walk, just like it does for the
UNINTERESTING case. This patch introduces a new
`ignore_missing_links` flag to the `rev_info` struct, which
we set only for the HAVE walk.

It also adds tests to cover UNINTERESTING objects missing
from several positions: a missing blob, a missing tree, and
a missing parent commit. The missing blob already worked (as
we do not care about its contents at all), but the other two
cases caused us to die().

Note that there are a few cases we do not need to test:

   1. We do not need to test a missing tree, with the blob
  still present. Without the tree that refers to it, we
  would not know that the blob is relevant to our walk.

   2. We do not need to test a tip commit that is missing.
  Upload-pack omits these for us (and in fact, we
  complain even in the non-bitmap case if it fails to do
  so).

Reported-by: Siddharth Agarwal s...@fb.com
Signed-off-by: Vicent Marti tan...@gmail.com
Signed-off-by: Jeff King p...@peff.net
---
I believe this should solve the problem you're seeing, and I think any
solution is going to be along these lines.

This covers all code paths that can be triggered by pack-objects.  But
it does not necessarily cover all code paths that a revision walker
might use (e.g., it is still possible to die in try_to_simplify_commit,
but we would never hit that in pack-objects, because we do not do
pathspec limiting).

So it's a tradeoff. On the one hand, leaving it like this creates a flag
in rev_info that may surprise somebody later by not being as generally
useful. On the other hand, covering every die() is extra code churn, and
creates complexity for cases that cannot actually be triggered in
practice (complexity because each site has to decide how to handle a
failure to access the object).

  list-objects.c  |  5 -
  pack-bitmap.c   |  2 ++
  revision.c  |  8 +---
  revision.h  |  3 ++-
  t/t5310-pack-bitmaps.sh | 31 +++
  5 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/list-objects.c b/list-objects.c
index 206816f..3595ee7 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -81,8 +81,11 @@ static void process_tree(struct rev_info *revs,
die(bad tree object);
if (obj-flags  (UNINTERESTING | SEEN))
return;
-   if (parse_tree(tree)  0)
+   if (parse_tree(tree)  0) {
+   if (revs-ignore_missing_links)
+   return;
die(bad tree object %s, sha1_to_hex(obj-sha1));
+   }
obj-flags |= SEEN;
show(obj, path, name, cb_data);
me.up = path;
diff --git a/pack-bitmap.c b/pack-bitmap.c
index ae0b57b..91e4101 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -727,8 +727,10 @@ int prepare_bitmap_walk(struct rev_info *revs)
revs-pending.objects = NULL

Re: [PATCH/RFC 0/6] reuse deltas found by bitmaps

2014-03-27 Thread Siddharth Agarwal

On 03/26/2014 03:40 PM, Siddharth Agarwal wrote:

On 03/26/2014 12:22 AM, Jeff King wrote:

[tl;dr the patch is the same as before, but there is a script to measure
its effects; please try it out on your repos]


Here are the numbers from another, much larger repo:

Test origin HEAD
 


5311.3: server   (1 days)11.72(14.02+1.44) 6.33(5.87+0.75) -46.0%
5311.4: size (1 days) 19.4M 15.3M -21.3%
5311.5: client   (1 days)6.99(8.06+1.50) 10.60(10.34+1.83) +51.6%
5311.7: server   (2 days)39.82(40.56+3.05) 33.94(31.21+3.10) -14.8%
5311.8: size (2 days) 26.5M 22.8M -14.1%
5311.9: client   (2 days)15.81(16.48+4.29) 19.20(18.20+4.19) +21.4%
5311.11: server   (4 days)   37.21(39.75+3.73) 28.01(26.97+1.75) -24.7%
5311.12: size (4 days) 37.5M 34.1M -9.0%
5311.13: client   (4 days)   24.24(26.43+6.76) 31.14(30.75+5.96) +28.5%
5311.15: server   (8 days)   33.74(40.47+3.39) 22.42(22.25+1.51) -33.6%
5311.16: size (8 days) 81.9M 78.4M -4.2%
5311.17: client   (8 days)   81.96(91.07+22.35) 88.03(89.45+17.11) +7.4%
5311.19: server  (16 days)   30.87(34.57+2.78) 27.03(25.93+2.73) -12.4%
5311.20: size(16 days) 153.2M150.9M -1.5%
5311.21: client  (16 days)   169.99(183.57+46.96) 177.12(177.17+37.93) 
+4.2%

5311.23: server  (32 days)   51.00(75.49+4.62) 19.52(19.28+1.93) -61.7%
5311.24: size(32 days) 279.4M283.0M +1.3%
5311.25: client  (32 days)   272.43(296.17+76.48) 284.75(285.98+63.19) 
+4.5%

5311.27: server  (64 days)   51.73(97.88+6.40) 37.32(32.63+5.05) -27.9%
5311.28: size(64 days) 1.7G  1.8G +5.0%
5311.29: client  (64 days)   2600.42(2751.56+718.10) 
2429.06(2501.29+651.56) -6.6%

5311.31: server (128 days)   51.33(95.33+6.91) 37.73(32.98+5.09) -26.5%
5311.32: size   (128 days) 1.7G  1.8G +5.0%
5311.33: client (128 days)   2595.68(2739.45+729.07) 
2386.99(2524.54+583.07) -8.0%


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC 0/6] reuse deltas found by bitmaps

2014-03-26 Thread Siddharth Agarwal

On 03/26/2014 12:22 AM, Jeff King wrote:

[tl;dr the patch is the same as before, but there is a script to measure
its effects; please try it out on your repos]


Here are results from one of our repos:

Test origin HEAD
-
5311.3: server   (1 days)1.77(2.49+0.15) 0.76(0.65+0.12) -57.1%
5311.4: size (1 days)   7.8M3.1M -60.2%
5311.5: client   (1 days)0.56(0.48+0.03) 1.18(0.97+0.05) +110.7%
5311.7: server   (2 days)2.79(3.68+0.25) 0.77(0.65+0.14) -72.4%
5311.8: size (2 days)  24.2M6.8M -71.8%
5311.9: client   (2 days)1.14(0.95+0.10) 2.72(2.33+0.13) +138.6%
5311.11: server   (4 days)   3.70(4.76+0.28) 0.84(0.72+0.14) -77.3%
5311.12: size (4 days) 51.2M   18.9M -63.0%
5311.13: client   (4 days)   2.29(2.02+0.29) 4.58(3.91+0.30) +100.0%
5311.15: server   (8 days)   5.16(7.48+0.38) 1.20(1.18+0.21) -76.7%
5311.16: size (8 days) 78.3M   42.6M -45.5%
5311.17: client   (8 days)   3.73(3.67+0.40) 6.59(5.95+0.51) +76.7%
5311.19: server  (16 days)   6.48(10.27+0.52)1.60(1.85+0.27) -75.3%
5311.20: size(16 days) 97.5M   64.0M -34.4%
5311.21: client  (16 days)   5.31(5.76+0.57) 8.99(8.61+0.68) +69.3%
5311.23: server  (32 days)   8.56(14.00+0.67)2.31(2.91+0.37) -73.0%
5311.24: size(32 days)124.7M   92.0M -26.2%
5311.25: client  (32 days)   7.69(9.20+0.76) 12.80(13.07+0.91) +66.4%
5311.27: server  (64 days)   37.47(45.48+1.55)   29.75(29.58+0.96) -20.6%
5311.28: size(64 days)245.5M  207.1M -15.6%
5311.29: client  (64 days)   15.11(18.26+1.58)   25.02(25.90+2.03) +65.6%
5311.31: server (128 days)   43.85(57.02+2.57)   29.88(29.54+1.35) -31.9%
5311.32: size   (128 days)507.3M  461.6M -9.0%
5311.33: client (128 days)   27.13(31.54+3.32)   37.76(39.49+3.16) +39.2%

I'm currently running tests on a much larger repo than that. The full 
perf run will take several hours on that, so the soonest I can likely 
publish results for that is tomorrow.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


fetches with bitmaps enabled can cause accesses to already GC'd objects

2014-03-25 Thread Siddharth Agarwal

Hi,

We're still experimenting with bitmaps, and we've have run into issues 
where fetching from a repository with bitmaps enabled can lead to 
objects that used to be present on the server but have since been GC'd 
being accessed, and git pack-objects on the server failing because of that.


I can consistently reproduce this with a particular pair of repos, and 
tip of git master (3f09db0) with no patches on top running on both ends. 
git fetch fails with


remote: error: Could not read be7cbe440a7b9a34f53515af4075e971c811cfb2
remote: fatal: bad tree object be7cbe440a7b9a34f53515af4075e971c811cfb2
error: git upload-pack: git-pack-objects died with error.
fatal: git upload-pack: aborting due to possible repository corruption 
on the remote side.

remote: aborting due to possible repository corruption on the remote side.
fatal: protocol error: bad pack header

Removing the bitmap fixes this.

be7cbe440a7b9a34f53515af4075e971c811cfb2 is a tree object that is 
present on the client but not on the server. It used to be present on 
the server, but the any refs that it was reachable from have been 
removed and the object has since been garbage collected. One ref that 
this object was reachable from and that used to be on the server is 
still present on the client though, under refs/remotes/origin/.


This tree object seems to be reachable from exactly one other tree 
object, and so on, until I reach a commit object. Note that the commit 
and root tree pointing to be7cbe440a7b9a34f53515af4075e971c811cfb2 is 
still present as a loose object in the repo.


I dug into this a bit, and it looks like the bad access is inside 
https://github.com/git/git/blob/3f09db0/pack-bitmap.c#L730, and from 
there inside https://github.com/git/git/blob/3f09db0/pack-bitmap.c#L575. 
This ultimately calls traverse_commit_list at 
https://github.com/git/git/blob/3f09db0/list-objects.c#L195, which adds 
the tree that transitively points to 
be7cbe440a7b9a34f53515af4075e971c811cfb2 as pending. (Note again that 
the commit and root tree objects still exist in the repo as loose 
objects.) Further down in that function, process_tree is called, which 
traverses the tree and ultimately dies at 
https://github.com/git/git/blob/3f09db0/list-objects.c#L85.


Unfortunately, as before, I can't share the repo this is happening in.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: with reuse-delta patches, fetching with bitmaps segfaults due to possibly incomplete bitmap traverse

2014-03-23 Thread Siddharth Agarwal

On 03/22/2014 05:56 AM, Jeff King wrote:

On Fri, Mar 21, 2014 at 07:58:55PM -0700, Siddharth Agarwal wrote:

Is it also reproducible just with the tip of next? Note that the
patches in jk/bitmap-reuse-delta have not been widely deployed (in
particular, we are not yet using them at GitHub, and we track segfaults
on our servers closely and have not seen any related to this).


I cannot reproduce this with the tip of next (tested with 4443bfd). 
That's also --  unsurprisingly -- significantly slower in the 
compression phase and sends much more data (3x for the pair of repos in 
the OP) over the wire than a Git that doesn't use bitmaps.



Those patches allocate extra fake entries in the entry-delta fields,
which are not accounted for in to_pack.nr_objects. It's entirely
possible that those entries are related to the bug you are seeing.


That sounds like it could be the problem, yes.


Hmm, yeah, that confirms my suspicion. In the earlier loops, we call
add_to_write_order, which only adds the object in question, and can
never exceed to_pack.nr_objects. In this final loop, we call
add_family_to_write_order, which is going to add any deltas that were
not already included.

The patch below may fix your problem, but I have a feeling it is not the
right thing to do. The point of 81cdec28 is to try to point to a delta
entry as if it were a preferred base (i.e., something we know that the
other side has already). We perhaps want to add these entries to the
actual packing list, and skip them as we do with normal preferred_base
objects.


The patch does stop Git from segfaulting. I know too little to judge its 
correctness, though.




diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 9fc5321..ca1b0f7 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1437,6 +1437,7 @@ static void check_object(struct object_entry *entry)
entry-delta = xcalloc(1, sizeof(*entry-delta));
hashcpy(entry-delta-idx.sha1, base_ref);
entry-delta-preferred_base = 1;
+   entry-delta-filled = 1;
unuse_pack(w_curs);
return;
}

-Peff


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


with reuse-delta patches, fetching with bitmaps segfaults due to possibly incomplete bitmap traverse

2014-03-21 Thread Siddharth Agarwal

Hi all,

At Facebook we've found that fetch speed is a bottleneck for our Git 
repos, so we've been looking to deploy bitmaps to speed up fetches. 
We've been trying out git-next with the top two patches from 
https://github.com/peff/git/commits/jk/bitmap-reuse-delta, but the 
following is reproducible with tip of that branch, currently 81cdec2.


We're finding that git fetches with bitmaps enabled are malfunctioning 
under some circumstances. For at least one of our repositories, a 
particular git fetch that an engineer ran segfaulted on the rmeote git 
pack-objects because of writing to an array out of bounds. This is 
consistently reproducible with the particular pair of (remote, local) 
repositories.


The error looks like:

(1) $ git fetch
remote: Counting objects: 201614, done.
remote: Compressing objects: 100% (36262/36262), done.
error: pack-objects died of signal 11
error: git upload-pack: git-pack-objects died with error.
fatal: git upload-pack: aborting due to possible repository corruption 
on the remote side.

remote: aborting due to possible repository corruption on the remote side.
fatal: protocol error: bad pack header

In contrast, when I disable bitmaps by removing the .bitmap file on the 
server while using the same Git binary, I get


(2) $ git fetch
remote: Counting objects: 203466, done.
remote: Compressing objects: 100% (46014/46104), done.
Receiving objects: 100% (203466/203466), 55.52 MiB | 7.45 MiB/s, done.
remote: Total 203466 (delta 169894), reused 187613 (delta 156621)

Note the differences in the Counting objects and Compressing objects 
figures between (1) and (2). I'm not sure if they're relevant.


As a baseline, if I run the same test with an older git -- version 
1.8.1, I get


(3) $ git fetch
remote: Counting objects: 235298, done.
remote: Compressing objects: 100% (46104/46104), done.
remote: Total 203466 (delta 169894), reused 187613 (delta 156621)
Receiving objects: 100% (203466/203466), 55.52 MiB | 11.15 MiB/s, done.

As a control, I'm using the same version of Git on the client in all 
three cases above -- git 1.8.1. The client Git doesn't matter -- using 
81cdec2 has the same effect. The transport is ssh in all three cases.


I dug into this a bit and it looks like at this point:

https://github.com/peff/git/blob/81cdec28fa24fdc613ab7c3406c1c67975dbf22f/builtin/pack-objects.c#L700

at some object that add_family_to_write_order is called for, wo_end 
exceeds to_pack.nr_objects by over 1000 objects. More precisely, at the 
point it crashes, wo_end is 218081 while to_pack.nr_objects is 201614. 
(This means wo_end overshot to_pack.nr_objects some time ago.)


I bumped up the malloc at 
https://github.com/peff/git/blob/81cdec28fa24fdc613ab7c3406c1c67975dbf22f/builtin/pack-objects.c#L628 
in order to prevent segfaulting, and the remote process dies with:


(4) $ git fetch
remote: Counting objects: 201614, done.
remote: Compressing objects: 100% (36262/36262), done.
remote: fatal: ordered 226227 objects, expected 201614

In contrast, in case (2) above, at the end of compute_pack_order, wo_end 
and to_pack.nr_objects are both 235298. I found it interesting that this 
number is not reflected anywhere in the output of (2) but is the same as 
the output of the Counting objects step of (3). I'm not sure if this 
is a red herring or not.


I suspect that the traverse_bitmap_commit_list call at 
https://github.com/peff/git/blob/81cdec28fa24fdc613ab7c3406c1c67975dbf22f/builtin/pack-objects.c#L2476 
is somehow skipping objects.


Other notes:
- I cannot reproduce this with a plain old 'git clone remote' with 
bitmaps enabled and used on the remote. There's something particular 
about either thin packs or the client repository that's triggering this.
- There is exactly one pack containing slightly over 3.5 million 
objects, and three loose objects in the remote repo.

- The remote repo is isolated -- there are no concurrent writes going on.
- While generating the bitmap I did not reuse existing any existing 
bitmaps: I removed the existing bitmap and ran git repack -adb.


Unfortunately I do not have a reproducible test case with a repository 
that's open source, or with one that I can share. However, I'm happy to 
provide any other information about the structure of the repository, and 
to set up a debugging session over IRC or other means.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] pack-objects: turn off bitmaps when skipping objects

2014-01-23 Thread Siddharth Agarwal

On 01/23/2014 02:52 PM, Jeff King wrote:

Right, that's expected.

The bitmap format cannot represent objects that are not present in the
pack. So we cannot write a bitmap index if any object reachable from a
packed commit is omitted from the pack.

We could be nicer and downgrade it to a warning, though. The patch below
does that.


This makes sense.


In our case we have .keep files lying around from ages ago (possibly
due to kill -9s run on the server).

We ran into that problem at GitHub, too. We just turn off
`--honor-pack-keep` during our repacks, as we never want them on anyway
(and we would prefer to ignore the .keep than to abort the bitmap).


Yes, we'd prefer to do that too. How do you actually do this, though? I 
don't see a way to pass `--honor-pack-keep` (shouldn't I pass in its 
inverse?) down to `git-pack-objects`.



It also means that running repack -a with bitmap writing enabled on a
repo becomes problematic if a fetch is run concurrently.

For the most part, no. The .keep file should generally only be set
during the period between indexing the pack and updating the refs (so
while checking connectivity and running hooks). But pack-objects starts
from the ref tips and walks backwards. Until they are updated, it will
not try to pack the objects in the .keep files, as nobody references
them.


The worry is less certain objects not being packed and more the old 
packs being deleted by git repack, isn't it? From the man page for 
git-index-pack:


--keep
Before moving the index into its final destination create an empty .keep 
file for the associated pack file. This option is usually necessary with
--stdin to prevent a simultaneous git repack process from deleting the 
newly constructed pack and index before refs can be updated to use 
objects contained in the pack.


I could be misunderstanding things here, though. From the description in 
the man page it's not clear what the actual failure mode here is.



There are two loopholes, though:

   1. In some instances, a remote may send an object we already have
  (e.g., because it is a blob referenced in an old commit, but newly
  referenced again due to a revert; we do not do a full object
  difference during the protocol negotiation, for reasons of
  efficiency). If that is the case, we may omit it if pack-objects
  starts during the period that the .pack and .keep files exist.

   2. Once the fetch updates the refs, it removes the .keep file. But
  this isn't atomic. A repack which starts between the two may pick
  up the new ref values, but also see the .keep file.

These are both unlikely, but possible on a very busy repository. The
patch below will downgrade each to a warning, rather than aborting the
repack.

So this should just work out of the box with this patch.  But if bitmaps
are important to you (say, you are running a very busy site and want
to make sure you always have bitmaps turned on) and you do not otherwise
care about .keep files, you may want to disable them, too.


We need to make sure bitmaps are always turned on, but we need to be 
even more certain that pushes don't fail due to races.



-Peff

-- 8 --
Subject: pack-objects: turn off bitmaps when skipping objects

The pack bitmap format requires that we have a single bit
for each object in the pack, and that each object's bitmap
represents its complete set of reachable objects. Therefore
we have no way to represent the bitmap of an object which
references objects outside the pack.

We notice this problem while generating the bitmaps, as we
try to find the offset of a particular object and realize
that we do not have it. In this case we die, and neither the
bitmap nor the pack is generated. This is correct, but
perhaps a little unfriendly. If you have bitmaps turned on
in the config, many repacks will fail which would otherwise
succeed. E.g., incremental repacks, repacks with -l when
you have alternates, .keep files.

Instead, this patch notices early that we are omitting some
objects from the pack and turns off bitmaps (with a
warning). Note that this is not strictly correct, as it's
possible that the object being omitted is not reachable from
any other object in the pack. In practice, this is almost
never the case, and there are two advantages to doing it
this way:

   1. The code is much simpler, as we do not have to cleanly
  abort the bitmap-generation process midway through.

   2. We do not waste time partially generating bitmaps only
  to find out that some object deep in the history is not
  being packed.

Signed-off-by: Jeff King p...@peff.net
---
I tried to keep the warning to an 80-character line without making it
too confusing. Suggestions welcome if it doesn't make sense to people.

  builtin/pack-objects.c  | 12 +++-
  t/t5310-pack-bitmaps.sh |  5 -
  2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 8364fbd..76831d9 100644
--- 

Re: [PATCH] pack-objects: turn off bitmaps when skipping objects

2014-01-23 Thread Siddharth Agarwal

On 01/23/2014 03:45 PM, Siddharth Agarwal wrote:


The worry is less certain objects not being packed and more the old 
packs being deleted by git repack, isn't it? From the man page for 
git-index-pack:


This should probably be new pack and not old packs, I guess. Not 
knowing much about how this actually works, I'm assuming the scenario 
here is something like:


(1) git receive-pack receives a pack P.pack and writes it to disk
(2) git index-pack runs on P.pack
(3) git repack runs separately, finds pack P.pack with no refs pointing 
to it, and deletes it

(4) everything goes wrong

With a keep file, this would be averted because

(1) git receive-pack receives a pack P.pack and writes it to disk
(2) git index-pack writes a keep file for P.pack, called P.keep
(3) git repack runs separately, finds pack P.pack with a keep file, 
doesn't touch it
(4) git index-pack finishes, and something updates refs to point to 
P.pack and deletes P.keep

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] pack-objects: turn off bitmaps when skipping objects

2014-01-23 Thread Siddharth Agarwal

On 01/23/2014 06:28 PM, Jeff King wrote:

I think your understanding is accurate here. So we want repack to
respect keep files for deletion, but we _not_ necessarily want
pack-objects to avoid packing an object just because it's in a pack
marked by .keep (see my other email).


Yes, that makes sense and sounds pretty safe.

So the right solution for us probably is to apply the patch Vicent 
posted, set repack.honorpackkeep to false, and also have a cron job that 
cleans up stale .keep files so that subsequent repacks clean it up.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


git repack --max-pack-size broken in git-next

2014-01-21 Thread Siddharth Agarwal

With git-next, the --max-pack-size option to git repack doesn't work.

With git at b139ac2, `git repack --max-pack-size=1g` says

error: option `max-pack-size' expects a numerical value

while `git repack --max-pack-size=1073741824` says

error: unknown option `max_pack_size=1073741824'

I bisected this down to a1bbc6c, which rewrote git repack in C.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: with git-next, git pull --rebase can print out fatal: No such ref: '' for branches without an upstream

2014-01-17 Thread Siddharth Agarwal

On 01/17/2014 12:40 AM, John Keeping wrote:

On Thu, Jan 16, 2014 at 06:47:38PM -0800, Siddharth Agarwal wrote:

On 01/16/2014 06:21 PM, Jeff King wrote:

On Thu, Jan 16, 2014 at 05:08:14PM -0800, Siddharth Agarwal wrote:


With git-next, where git pull --rebase can print out fatal: No such
ref: '' if git pull --rebase is run on branches without an upstream.

This is already fixed in bb3f458 (rebase: fix fork-point with zero
arguments, 2014-01-09), I think.

If I'm reading the patch correctly, that only fixes it for git rebase,
not for git pull --rebase. git-pull.sh contains a separate invocation of
git merge-base --fork-point.

I'm pretty sure the invocation in git-pull.sh is OK.  The error then
comes out of git-rebase.sh when git-pull invokes it.


That doesn't square with 48059e4 being the culprit commit.


Are you running a version of git-next that includes bb3f458?


Yes, I am.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


with git-next, git pull --rebase can print out fatal: No such ref: '' for branches without an upstream

2014-01-16 Thread Siddharth Agarwal
With git-next, where git pull --rebase can print out fatal: No such 
ref: '' if git pull --rebase is run on branches without an upstream.


With git at b139ac2589b15d55cd9fa5c6957da44b150d0737, the following 
commands demonstrate the problem:


git init repo1
cd repo1
touch a; git add a; git commit -m a
cd ..
git clone repo1 repo2
cd repo2
git config remote.origin.fetch refs/heads/master:refs/remotes/origin/master
git checkout -b test
git pull --rebase

This results in the following output:

fatal: No such ref: ''
Current branch test is up to date.

So the pull --rebase looks like it works, but it prints out a spurious 
fatal error.


I've managed to bisect this down to 
https://github.com/gitster/git/commit/48059e405028ebf8a09c5a9aede89dfb460cce98. 
Looks like get_remote_merge_branch is called without arguments, and it 
returns an empty string. This string is passed as-is to git merge-base, 
which causes the error.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: with git-next, git pull --rebase can print out fatal: No such ref: '' for branches without an upstream

2014-01-16 Thread Siddharth Agarwal

On 01/16/2014 06:21 PM, Jeff King wrote:

On Thu, Jan 16, 2014 at 05:08:14PM -0800, Siddharth Agarwal wrote:


With git-next, where git pull --rebase can print out fatal: No such
ref: '' if git pull --rebase is run on branches without an upstream.

This is already fixed in bb3f458 (rebase: fix fork-point with zero
arguments, 2014-01-09), I think.


If I'm reading the patch correctly, that only fixes it for git rebase, 
not for git pull --rebase. git-pull.sh contains a separate invocation of 
git merge-base --fork-point.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html