Re: Did we break receive-pack recently?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Aug 5, 2012 at 6:37 PM, Brandon Casey <drafnel@xxxxxxxxx> wrote:
> On Sat, Aug 4, 2012 at 6:55 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> I just saw this:
>>
>>     $ git push ko
>>     ko: Counting objects: 332, done.
>>     Delta compression using up to 4 threads.
>>     Compressing objects: 100% (110/110), done.
>>     Writing objects: 100% (130/130), 32.27 KiB, done.
>>     Total 130 (delta 106), reused 21 (delta 20)
>>     Auto packing the repository for optimum performance.
>>     fatal: protocol error: bad line length character: Remo
>>     error: error in sideband demultiplexer
>>     To ra.kernel.org:/pub/scm/git/git.git
>>     ...
>>
>> What is unusual with this push is that it happened to trigger the
>> auto-gc on the receiving end and the message "Auto packing the
>> repository..." came back to the pusher just fine, but somebody
>> nearby seem to have tried to say "Remo"(te---probably) without
>> properly using the sideband.
>
> Or perhaps "Remo" is short for "Removing...".
>
> Perhaps this is the source:
>
>    $ grep Remo builtin/prune.c
>    printf("Removing stale temporary file %s\n", fullpath);

Verified...

test_path=`pwd` &&
git init test_repo1 &&
( cd test_repo1 &&
  echo D >file.txt &&
  git add . &&
  git commit -m 'Commit something that hashes to 17...' &&
  echo MN >file.txt &&
  git commit -a -m 'Commit something else that hashes to 17...'
) &&
git init --bare test_repo2.git &&
( cd test_repo2.git &&
  git config gc.auto 1 &&
  touch -d '2012-07-01' objects/tmp_test
) &&
( cd test_repo1 &&
  git push "file://$test_path/test_repo2.git" HEAD:refs/heads/master
)

It seems to have been broken since we added 'gc --auto' to receive
pack in 2009 (or maybe since I added that printf to prune.c in 2008
depending on how you look at it :b ).  Apparently it's something not
very likely to be triggered.  Probably because most servers running
git daemon frequently run 'git gc'.  Wonder what k.org's policy is?

I think the original thinking behind writing to stdout
indiscriminately was that removing a temporary object was something
that was considered unusual, so the user should always be informed.
This is unlike removing a stale object, which is something that is
expected during normal usage.  If a stale temporary object is removed,
then it means that some piece of git which should have removed it,
failed to do so.

We could write the message to stderr instead, but I think the full
path to a temporary stale object is not appropriate to communicate to
remote users over the wire.

So, I think it's best just to protect it with 'if (show_only ||
verbose)' like the other informational messages.

Patch forthcoming, hopefully with a test.  Doesn't look like we have
anything testing the auto-gc spawned from receive-pack.  I'll see if I
can come up with something.

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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]