All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tay Ray Chuan <rctay89@gmail.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: Sverre Rabbelier <srabbelier@gmail.com>,
	git@vger.kernel.org, "Shawn O. Pearce" <spearce@spearce.org>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH 2/2] remote-curl.c: fix rpc_out()
Date: Tue, 24 Nov 2009 01:54:15 +0800	[thread overview]
Message-ID: <20091124015415.c22d07c1.rctay89@gmail.com> (raw)
In-Reply-To: <alpine.DEB.1.00.0911231137170.4985@pacific.mpi-cbg.de>

Hi,

On Mon, Nov 23, 2009 at 6:39 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> I guess you meant "not be enough", as an int can hold a pretty large
> number until it turns negative.

I really did mean 'enough' - enough to trigger the use of chunked
encoding. I think the most important fix here is forcing rpc_out to
(according to curl) memcpy at most size_t max bytes (the removal of the
extraneous ';' addresses this). Pushing with chunked transfer would
fail with this extra semicolon.

Removing the possibility of a negative size_t was a preventive measure,
and, like you mentioned, requires a larger repository, so it's harder
to test for.

I probably should separate these issues into separate patches.

> So I think in this case it is more harm- than helpful to have a test case.
>
> For future reference: if you need a repository with special featurs
> for testing, it is best to generate it in a test script (see the many test
> cases labeled 'setup' in our test suite for examples).

Here's what I came up with: use the git repository which fetched the
test suite, and use the environment variable GIT_REMOTE_REFSPEC to specify
the remote refspec which the tester fetches git from.


  if test -z "$GIT_REMOTE_REFSPEC"; then
  	say 'skipping test, the remote for git is not specified'
  else
  	test_expect_success 'push with chunked encoding' '
  		OWD=$(pwd) &&
  		cd $TEST_DIRECTORY/../.git/ &&
  		REPO=$(pwd) &&
  		cd "$OWD" &&
  		echo "$REPO"/objects > .git/objects/info/alternates &&
  		git fetch "$REPO" "$GIT_REMOTE_REFSPEC"/*:refs/remotes/git/* &&
  		git push -v -v origin "refs/remotes/git/*:refs/remotes/git/*" \
  			>out 2>&1 &&
  		grep "POST git-receive-pack (chunked)" out
  	'
  fi

Thoughts?

-- 
Cheers,
Ray Chuan

  reply	other threads:[~2009-11-23 17:54 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-23  3:03 [PATCH 2/2] remote-curl.c: fix rpc_out() Tay Ray Chuan
2009-11-23  5:53 ` Sverre Rabbelier
2009-11-23  8:38   ` Tay Ray Chuan
2009-11-23 10:39     ` Johannes Schindelin
2009-11-23 17:54       ` Tay Ray Chuan [this message]
2009-11-23 21:04 ` Shawn O. Pearce
2009-11-24  1:35   ` Tay Ray Chuan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20091124015415.c22d07c1.rctay89@gmail.com \
    --to=rctay89@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=spearce@spearce.org \
    --cc=srabbelier@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.