Git development
 help / color / mirror / Atom feed
* Re: [PATCH] fast-import.c: Silence build warning
From: Michael Wookey @ 2009-08-31 23:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vfxb7y2h3.fsf@alter.siamese.dyndns.org>

2009/9/1 Junio C Hamano <gitster@pobox.com>:
> Michael Wookey <michaelwookey@gmail.com> writes:
>
>> gcc 4.3.3 (Ubuntu 9.04) warns that the return value of strtoul() was not
>> checked by issuing the following notice:
>>
>>   warning: ignoring return value of ‘strtoul’, declared with attribute
>> warn_unused_result
>>
>> Provide a dummy variable to keep the compiler happy.
>>
>> Signed-off-by: Michael Wookey <michaelwookey@gmail.com>
>> ---
>>  fast-import.c |    5 +++--
>>  1 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/fast-import.c b/fast-import.c
>> index 7ef9865..1386e75 100644
>> --- a/fast-import.c
>> +++ b/fast-import.c
>> @@ -1744,10 +1744,11 @@ static int validate_raw_date(const char *src,
>> char *result, int maxlen)
>>  {
>>       const char *orig_src = src;
>>       char *endp;
>> +     unsigned long int unused;
>>
>>       errno = 0;
>>
>> -     strtoul(src, &endp, 10);
>> +     unused = strtoul(src, &endp, 10);
>
> Isn't this typically done by casting the expression to (void)?

I originally tried that - the compiler still complains.

> Otherwise a clever compiler has every right to complain "the variable
> unused is assigned but never used."

 I get no other warnings, so does that make gcc less than clever? ;-)

^ permalink raw reply

* Re: [PATCH] git-submodule and --upload-pack
From: Junio C Hamano @ 2009-08-31 23:51 UTC (permalink / raw)
  To: Giulio Eulisse; +Cc: git
In-Reply-To: <9D7140EC-EAFD-4408-93E3-0E756BA363DA@cern.ch>

Giulio Eulisse <Giulio.Eulisse@cern.ch> writes:

> There was a thread a while ago aboyt having --upload-pack support for
> git-submodule.
>
> Given that there was no followup (as far as I can tell) and I needed
> pretty much
> the same functionality I ported Jason's patch to work on top of 1.6.4.2.

Thanks.

Can you point at the original patch with a usable commit log message, in
the gmane archive (i.e. http://thread.gmane.org/...) if possible?  I
do not think we have that patch queued anywhere even in 'pu'.

Given that it looks like a new feature, I do not think it would be
appropriate for any of the future 1.6.4.X series, but if it is useful we
may want to have it in the upcoming 1.6.5 release.

> Comments?

See below.

> diff --git a/Documentation/git-submodule.txt b/Documentation/git- 
> submodule.txt

The patch is linewrapped and will not be applicable.  But I'll comment on
the contents to save a round-trip.

> diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
> index 5daf750..bf982a6 100644
> --- a/Documentation/gitmodules.txt
> +++ b/Documentation/gitmodules.txt
> @@ -30,6 +30,14 @@ submodule.<name>.path::
>  submodule.<name>.url::
>  	Defines an url from where the submodule repository can be cloned.
>
> +submodule.<name>.receivepack::
> +	The default program to execute on the remote side when pushing.  See
> +	option \--receive-pack of linkgit:git-push[1].
> +
> +submodule.<name>.uploadpack::
> +	The default program to execute on the remote side when fetching.  See
> +	option \--upload-pack of linkgit:git-fetch-pack[1].

This placement of description in the documentation and variables in the
namespace quite sane, as these are properties of the remote site, and they
belong together with submodule.<name>.url.

> @@ -53,12 +60,16 @@ Consider the following .gitmodules file:
>
>  	[submodule "libbar"]
>  		path = include/bar
> -		url = git://bar.com/git/lib.git
> +		url = ssh://bar.com/~/git/lib.git
> +		uploadpack = /home/you/bin/git-upload-pack-wrapper
> +		receivepack = /home/you/bin/git-receive-pack-wrapper
> ...
> +For `libbar`, packs are retrieved and stored via the upload and receive
> +wrappers, respectively.

Using a custom wrapper in this example feels very misleading.  The option
is primarily meant as a workaround for a broken or hard-to-modify sshd
settings that does not allow you to include the directory you installed
upload-pack/receive-pack to the PATH environment when the ssh session is
not interactive.

> @@ -97,13 +99,30 @@ module_clone()
>  	test -e "$path" &&
>  	die "A file already exist at path '$path'"
>
> +        uploadpackCmd=""
> +
> +        if test "$uploadpack"
> +        then
> +          uploadpackCmd="--upload-pack $uploadpack"

Can the value of uploadpack contain a shell IFS?  This is a rhetorical
question---read on.

> +        fi
> +
>  	if test -n "$reference"
>  	then
> -		git-clone "$reference" -n "$url" "$path"
> +		git-clone $uploadpackCmd "$reference" -n "$url" "$path"

Without using uploadpackCmd and risking to trash IFS characters in the
variable, you can do something like:

    git clone ${uploadpack+--upload-pack "$uploadpack"} ...

I would prefer the code not to set variable "uploadpack" when nothing is
specified, instead of setting it to an empty string like this patch does,
but if you are going to use an empty string as a signal that no uploadpack
is specified, then you would need a colon between 'k'and '+' in the above.

> @@ -738,6 +801,18 @@ cmd_sync()
>  			remote=$(get_default_remote)
>  			say "Synchronizing submodule url for '$name'"
>  			git config remote."$remote".url "$url"
> +			uploadpack=$(git config -f .gitmodules submodule."$name".uploadpack)
> +			receivepack=$(git config -f .gitmodules
> submodule."$name".receivepack)
> +			if test "$uploadpack"
> +			then
> +			    git config submodule."$name".uploadpack "$uploadpack" ||
> +			    echo "  Warn: Failed to set uploadpack for
> $url' in submodule  path '$name'."
> +			fi
> +			if test "$receivepack"
> +			then
> +			    git config submodule."$name".receivepack "$receivepack" ||
> +			    echo "  Warn: Failed to set receivepack
> for '$url' in  submodule path '$name'."
> +			fi
>  		)

I do not agree with this part, nor what the "cmd_init" does.

Having URL/uploadpack/receivepack 3-tuple in the tracked .gitmodules is
sensible, as that is how the project expresses its recommendations to
people who clone the toplevel project.

However, after the top-level project is cloned and the submodules are
populated in the work tree of the person who cloned, I think these values
should be propagated to $path/.git/config, i.e. the configuration file of
the submodule checkout.

Inside cmd_update(), there is this code snippet:

	if test -z "$nofetch"
	then
		(unset GIT_DIR; cd "$path" &&
			git-fetch) ||
		die "Unable to fetch in submodule path '$path'"
	fi

And the patch does not touch it.  For this git-fetch to honour the custom
uploadpack your user configured, remote.origin.uploadpack variable in the
configuration file of the submodule checkout needs to be updated, because
this fetch will not (and should not) look at the configuration file of the
superproject.

You _could_ also copy them to submodule.$name.$var of the top-level
project if you really wanted to, but I do not think doing so serves any
useful purpose.

^ permalink raw reply

* Re: [PATCH] fast-import.c: Silence build warning
From: Junio C Hamano @ 2009-08-31 23:42 UTC (permalink / raw)
  To: Michael Wookey; +Cc: git
In-Reply-To: <d2e97e800908310421u7de8ae58o361bd64a026384bf@mail.gmail.com>

Michael Wookey <michaelwookey@gmail.com> writes:

> gcc 4.3.3 (Ubuntu 9.04) warns that the return value of strtoul() was not
> checked by issuing the following notice:
>
>   warning: ignoring return value of ‘strtoul’, declared with attribute
> warn_unused_result
>
> Provide a dummy variable to keep the compiler happy.
>
> Signed-off-by: Michael Wookey <michaelwookey@gmail.com>
> ---
>  fast-import.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/fast-import.c b/fast-import.c
> index 7ef9865..1386e75 100644
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -1744,10 +1744,11 @@ static int validate_raw_date(const char *src,
> char *result, int maxlen)
>  {
>  	const char *orig_src = src;
>  	char *endp;
> +	unsigned long int unused;
>
>  	errno = 0;
>
> -	strtoul(src, &endp, 10);
> +	unused = strtoul(src, &endp, 10);

Isn't this typically done by casting the expression to (void)?

Otherwise a clever compiler has every right to complain "the variable
unused is assigned but never used."

^ permalink raw reply

* Re: [PATCH] Documentation/git-add.txt: Explain --patch option in layman terms
From: Junio C Hamano @ 2009-08-31 23:42 UTC (permalink / raw)
  To: Jari Aalto; +Cc: git
In-Reply-To: <87y6p08lz5.fsf@jondo.cante.net>

Jari Aalto <jari.aalto@cante.net> writes:

>     --patch:
>     -p::
>         In a modified work tree, choose interactively which patch hunks to
>         add. This gives a change to review the difference between the
>         index and the work before adding modified contents to the index.

Sounds sensible.  You may want to be even more direct and succinct, e.g.

    Interactively choose hunks of patch between the index and the work
    tree and add them to the index.

^ permalink raw reply

* Re: [PATCH] upload-pack: add a trigger for post-upload-pack hook
From: Junio C Hamano @ 2009-08-31 23:36 UTC (permalink / raw)
  To: Tom Werner
  Cc: Jakub Narebski, Johan Sorensen, Jeff King, Tom Preston-Werner,
	git
In-Reply-To: <12c267e40908311150n2aad598aw978c4691c27ac0fa@mail.gmail.com>

Tom Werner <mojombo@gmail.com> writes:

> ... I'd be happy
> with the previous incarnation of the post-upload-pack that simply
> sends the HAVEs and WANTs.

Thanks.  This settles the biggest worry I had.

The worry was not about "do we feed clone/fetch info?", but was about
getting a complaint "Now, these changes to feed info from standard input
does not help anybody but forces us to update our hooks for no good
reason" from you guys ;-).

^ permalink raw reply

* Re: What's cooking in git.git (Aug 2009, #05; Wed, 26)
From: Junio C Hamano @ 2009-08-31 23:35 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: git
In-Reply-To: <alpine.LNX.2.00.0908311130190.28290@iabervon.org>

Daniel Barkalow <barkalow@iabervon.org> writes:

> On Sat, 29 Aug 2009, Junio C Hamano wrote:
>
>> ..., if only to avoid confusion with our own earlier misdesigned
>> syntax git+ssh://), so the canonical syntax would be:
>
> (with the syntax <helper>+, "git+ssh://" would specify the helper "git", 
> which is as good an explicit identifier of the internal handling as any)

Sure but how would you explain "ssh+git://" then ;-)?

Luckily neither is advertised in our documentation set as far as I can
tell, so I do not think it is a huge deal between + vs ::, but as Peff
says in

    http://thread.gmane.org/gmane.comp.version-control.git/125615

I think the latter is probably less problematic.

With plus, a helper that talks with a Subversion repository whose native
URL is http://host/path would look like svn+http://host/path, which is
reasonable.  When talking the Subversion protocol over SSH, however, the
native URL for the repository would be svn+ssh://host/path, so the URL
with helper name on our side becomes svn+svn+ssh://host/path.  We could
recognize "svn+" part and implicitly pass the whole thing to svn helper
upon seeing svn+ssh://host/path, but I do not think we would want to make
the dispatcher too familiar with what the backends do.  Using something
other than plus sign would avoid this issue.

> If the policy is that we're going to have "traditionally supported" 
> schemes, where the internal code knows what helper supports them, I can 
> fix up the series so that the curl-based helper is named "curl", and we 
> can say that the check for "http://", "ftp://", and "https://" is
> recognizing traditionally-supported schemes, and we can defer coming up 
> with what the syntax for the explicit handler selection is. (For that 
> matter, if there's a // after the colon, it's obviously not a 
> handy ssh-style location, since the second slash would do nothing)

That sounds like a sane approach to first get the "eject curl from
builtin" out the door.  We might extend the dispatcher in the future by
changing "traditionally supported" criterion to "commonly used",
e.g. recognize "svn+ssh://" as something the svn helper would want to
handle, but that is a future extension we do not have to address right
now.

>> After you explained this in the thread (I think for the second time), I
>> see no problem with this, except that I think to support this we should
>> notice and raise an error when we see a remote has both vcs and url,
>> because the only reason we would want to say "vcs", if I recall your
>> explanation correctly, is that such a transport does not have the concept
>> of URL, i.e. a well defined single string that identifies the repository.
>
> A user who mostly uses Perforce as a foreign repositories but is using a 
> SVN repo on occasion might want to use "vcs" regardless, but I agree with 
> forcing the helper to use a different option for the case of a URL that 
> git isn't going to look at. That is, you ought to be able to use:
>
> [remote "origin"]
> 	vcs = svn
> 	(something) = http://svn.savannah.gnu.org/...
>
> But "(something)" shouldn't be "url".

I actuallly do not have a strong opinion on this one either way.  I said
"I think" when I suggested it, but it was actually without thinking too
deeply, hoping that you would come up with a good counter-argument.

For example, if we envision that for most of the helpers there will be one
primary string that identifies the repository, but the primary string
alone is not enough for the helper without some auxiliary information, it
would be natural to use remote.$name.url for that primary string.  I do
not know if that would be the case, but I was hoping that you would have a
better intuition[*1*], as you have thought this topic through a lot longer
and deeper than I have.  So I'd rather leave the decision on that "no
vcs/url at the same time restriction" up to you.  It is in general easier
to start more strict and then loosen the restiction later, than the other
way around, when we cannot decide, though.

Thanks.

[Footnote]

*1* What I mean by intuition is that you do not have to have the right
answer backed by research _now_, but have a good guess as to what the
right answer would be.

^ permalink raw reply

* Re: [PATCH] fast-import.c: Silence build warning
From: Michael Wookey @ 2009-08-31 23:31 UTC (permalink / raw)
  To: Alex Riesen, Junio C Hamano; +Cc: Sverre Rabbelier, git
In-Reply-To: <81b0412b0908311427t5b4a24ffg1d7d272669476117@mail.gmail.com>

2009/9/1 Alex Riesen <raa.lkml@gmail.com>:
> On Mon, Aug 31, 2009 at 14:29, Sverre Rabbelier<srabbelier@gmail.com> wrote:
>> On Mon, Aug 31, 2009 at 04:21, Michael Wookey<michaelwookey@gmail.com> wrote:
>>> Provide a dummy variable to keep the compiler happy.
>>
>> Should we not instead check the value?
>
> Why? It is endp (end of the parsed number) we're interested in.

Good point, perhaps the commit message should mention why we don't
bother checking the return value. Something like this maybe?

-- >8 --
gcc 4.3.3 (Ubuntu 9.04) warns that the return value of strtoul() was not
checked by issuing the following notice:

 warning: ignoring return value of ‘strtoul’, declared with attribute
warn_unused_result

The return value of strtoul() isn't used because we are only interested
in what is placed into endp.  As such, provide a dummy variable to keep
the compiler happy.

Signed-off-by: Michael Wookey <michaelwookey@gmail.com>
-- >8 --

^ permalink raw reply

* Re: clong an empty repo over ssh causes (harmless) fatal
From: Sverre Rabbelier @ 2009-08-31 22:50 UTC (permalink / raw)
  To: Jeff King; +Cc: Björn Steinbrink, Matthieu Moy, Sitaram Chamarty, git
In-Reply-To: <20090831224749.GA24190@sigill.intra.peff.net>

Heya,

2009/9/1 Jeff King <peff@peff.net>:
> AFAICT, this problem goes back to v1.6.2, the first version which
> handled empty clones. So I blame Sverre. ;)

Eep :(. Any idea what is going on?

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: clong an empty repo over ssh causes (harmless) fatal
From: Jeff King @ 2009-08-31 22:47 UTC (permalink / raw)
  To: Björn Steinbrink
  Cc: Sverre Rabbelier, Matthieu Moy, Sitaram Chamarty, git
In-Reply-To: <20090831201911.GA24989@atjola.homenet>

On Mon, Aug 31, 2009 at 10:19:11PM +0200, Björn Steinbrink wrote:

> I see the problem here, too.
> 
> doener@atjola:~ $ (mkdir a; cd a; git init)
> Initialized empty Git repository in /home/doener/a/.git/
> 
> doener@atjola:~ $ git clone localhost:a b
> Initialized empty Git repository in /home/doener/b/.git/
> warning: You appear to have cloned an empty repository.
> fatal: The remote end hung up unexpectedly
> 
> doener@atjola:~ $ ssh localhost git --version
> git version 1.6.4.2.236.gf324c

OK, it is definitely not about mixed versions, and it is definitely
reproducible, even without ssh. The local clone optimization manages to
avoid it, but you can see it with:

  git clone file://$PWD/a b

It also happens with git://, except that it is the _remote_ side
producing the message, so git-daemon gets "the remote end hung up
unexpectedly" on its stderr channel.

AFAICT, this problem goes back to v1.6.2, the first version which
handled empty clones. So I blame Sverre. ;)

-Peff

^ permalink raw reply

* Re: [PATCH] fast-import.c: Silence build warning
From: Sverre Rabbelier @ 2009-08-31 21:42 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Michael Wookey, git
In-Reply-To: <81b0412b0908311427t5b4a24ffg1d7d272669476117@mail.gmail.com>

Heya,

On Mon, Aug 31, 2009 at 23:27, Alex Riesen<raa.lkml@gmail.com> wrote:
> Why? It is endp (end of the parsed number) we're interested in.

Ah, my bad, I hadn't checked stroul's signature, sorry for the noise.

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [PATCH] fast-import.c: Silence build warning
From: Alex Riesen @ 2009-08-31 21:27 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: Michael Wookey, git
In-Reply-To: <fabb9a1e0908310529q4c601a73t671cc2813dfdb1a3@mail.gmail.com>

On Mon, Aug 31, 2009 at 14:29, Sverre Rabbelier<srabbelier@gmail.com> wrote:
> On Mon, Aug 31, 2009 at 04:21, Michael Wookey<michaelwookey@gmail.com> wrote:
>> Provide a dummy variable to keep the compiler happy.
>
> Should we not instead check the value?

Why? It is endp (end of the parsed number) we're interested in.

^ permalink raw reply

* from local to github
From: sigbackup @ 2009-08-31 20:46 UTC (permalink / raw)
  To: git

Hello guys,
I'm a git newbie and I'm looking for some good references about using
git locally (on Mac) and synchronize my repositories to my github
account and from there to a Win2003 production server.

Can anyone help me with this?


Thanks and have a great day.

Sig

^ permalink raw reply

* from local to github
From: sigbackup @ 2009-08-31 20:46 UTC (permalink / raw)
  To: git

Hello guys,
I'm a git newbie and I'm looking for some good references about using
git locally (on Mac) and synchronize my repositories to my github
account and from there to a Win2003 production server.

Can anyone help me with this?


Thanks and have a great day.

Sig

^ permalink raw reply

* `Git Status`-like output for two local branches
From: Tim Visher @ 2009-08-31 20:20 UTC (permalink / raw)
  To: Git Mailing List

Hello Everyone,

I'm interested in being able to get a message such as 'dev and master
have diverged, having 1 and 2 commits different respectively' or 'dev
is behind master by 3 commits and can be fast-forwarded', etc.  I'm
sure this is simple, but I can't figure out how to do it in the docs.
Sorry for the noobness of the question.

Thanks!

-- 

In Christ,

Timmy V.

http://burningones.com/
http://five.sentenc.es/ - Spend less time on e-mail

^ permalink raw reply

* Re: clong an empty repo over ssh causes (harmless) fatal
From: Björn Steinbrink @ 2009-08-31 20:19 UTC (permalink / raw)
  To: Jeff King; +Cc: Matthieu Moy, Sitaram Chamarty, git
In-Reply-To: <20090831191032.GB4876@sigill.intra.peff.net>

On 2009.08.31 15:10:32 -0400, Jeff King wrote:
> On Mon, Aug 31, 2009 at 07:25:22PM +0200, Matthieu Moy wrote:
> 
> > Since the client and server are the same machine:
> > 
> >     $ git clone ssh://sitaram@localhost/home/sitaram/t/a b
> > 
> > I'd bet Sitaram has two installations of git, and plain ssh to the
> > machine points to the old one (like a $PATH set in ~/.login and not
> > ~/.profile or something like that).
> 
> Oh, indeed. I didn't notice that his host was @localhost. :)
> 
> But yes, that would be my guess, as well. Trying "ssh sitaram@localhost
> git version" would be a good clue.

I see the problem here, too.

doener@atjola:~ $ (mkdir a; cd a; git init)
Initialized empty Git repository in /home/doener/a/.git/

doener@atjola:~ $ git clone localhost:a b
Initialized empty Git repository in /home/doener/b/.git/
warning: You appear to have cloned an empty repository.
fatal: The remote end hung up unexpectedly

doener@atjola:~ $ ssh localhost git --version
git version 1.6.4.2.236.gf324c

Björn

^ permalink raw reply

* Re: [PATCH 3/3] transport: don't show push status if --quiet is given
From: Sebastian Pipping @ 2009-08-31 19:39 UTC (permalink / raw)
  To: Jeff King
  Cc: Junio C Hamano, Nicolas Pitre, Shawn O. Pearce, Albert Astals Cid,
	Pau Garcia i Quiles, git
In-Reply-To: <20090831192834.GC4876@sigill.intra.peff.net>

Jeff King wrote:
> Junio applied the series, and it is in 'master' right now (and so should
> be part of the upcoming 1.6.5).
> 
> Using "git push -q" will do what you want,

That's great news.  Thanks for the quick reply.


> but playing with it a bit, I
> think there is one bit missing from the original series:
> 
> -- >8 --
> Subject: [PATCH] push: teach --quiet to suppress "Everything up-to-date"
> 
> This should have been part of 481c7a6, whose goal was to
> make "git push -q" silent unless there is an error.
> 
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  transport.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/transport.c b/transport.c
> index ce91387..f2bd998 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -908,7 +908,7 @@ int transport_push(struct transport *transport,
>  				update_tracking_ref(transport->remote, ref, verbose);
>  		}
>  
> -		if (!ret && !refs_pushed(remote_refs))
> +		if (!quiet && !ret && !refs_pushed(remote_refs))
>  			fprintf(stderr, "Everything up-to-date\n");
>  		return ret;
>  	}

Would be great to have that patch in too.



Sebastian

^ permalink raw reply

* Re: [PATCH 3/3] transport: don't show push status if --quiet is given
From: Jeff King @ 2009-08-31 19:28 UTC (permalink / raw)
  To: Sebastian Pipping
  Cc: Junio C Hamano, Nicolas Pitre, Shawn O. Pearce, Albert Astals Cid,
	Pau Garcia i Quiles, git
In-Reply-To: <4A9C175E.6020905@hartwork.org>

On Mon, Aug 31, 2009 at 08:33:02PM +0200, Sebastian Pipping wrote:

> I run git push in a cron job, too.  I want mails in error cases only
> so I need git push to print errors but _only_ errors to stderr.  That
> seems impossible with 1.6.4.* and related to what you're discussing here.
> 
> Does the patch you're building address that case already?  has it been
> applied to any branch already?  I got a bit lost in this thread, sorry.

Junio applied the series, and it is in 'master' right now (and so should
be part of the upcoming 1.6.5).

Using "git push -q" will do what you want, but playing with it a bit, I
think there is one bit missing from the original series:

-- >8 --
Subject: [PATCH] push: teach --quiet to suppress "Everything up-to-date"

This should have been part of 481c7a6, whose goal was to
make "git push -q" silent unless there is an error.

Signed-off-by: Jeff King <peff@peff.net>
---
 transport.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/transport.c b/transport.c
index ce91387..f2bd998 100644
--- a/transport.c
+++ b/transport.c
@@ -908,7 +908,7 @@ int transport_push(struct transport *transport,
 				update_tracking_ref(transport->remote, ref, verbose);
 		}
 
-		if (!ret && !refs_pushed(remote_refs))
+		if (!quiet && !ret && !refs_pushed(remote_refs))
 			fprintf(stderr, "Everything up-to-date\n");
 		return ret;
 	}
-- 
1.6.4.2.372.gf7961.dirty

^ permalink raw reply related

* Re: clong an empty repo over ssh causes (harmless) fatal
From: Jeff King @ 2009-08-31 19:10 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: Sitaram Chamarty, git
In-Reply-To: <vpqocpvevzx.fsf@bauges.imag.fr>

On Mon, Aug 31, 2009 at 07:25:22PM +0200, Matthieu Moy wrote:

> Since the client and server are the same machine:
> 
>     $ git clone ssh://sitaram@localhost/home/sitaram/t/a b
> 
> I'd bet Sitaram has two installations of git, and plain ssh to the
> machine points to the old one (like a $PATH set in ~/.login and not
> ~/.profile or something like that).

Oh, indeed. I didn't notice that his host was @localhost. :)

But yes, that would be my guess, as well. Trying "ssh sitaram@localhost
git version" would be a good clue.

-Peff

^ permalink raw reply

* Re: clong an empty repo over ssh causes (harmless) fatal
From: Sverre Rabbelier @ 2009-08-31 19:09 UTC (permalink / raw)
  To: Jeff King; +Cc: Sitaram Chamarty, Matthieu Moy, git
In-Reply-To: <20090831190827.GA4876@sigill.intra.peff.net>

Heya,

On Mon, Aug 31, 2009 at 21:08, Jeff King<peff@peff.net> wrote:
> I think the former. I thought it was discussed before, but the only
> reference I can find is this (see the end of the email):
>
>  http://article.gmane.org/gmane.comp.version-control.git/107626

Ah, yeup, I see.

> and I don't see any followup for that specific part of the mail.

I don't remember any follow up to that either, shame on me :(.

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: clong an empty repo over ssh causes (harmless) fatal
From: Jeff King @ 2009-08-31 19:08 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: Sitaram Chamarty, Matthieu Moy, git
In-Reply-To: <fabb9a1e0908311012q4cea2d51i2c2f0cbceac0cab@mail.gmail.com>

On Mon, Aug 31, 2009 at 07:12:44PM +0200, Sverre Rabbelier wrote:

> On Mon, Aug 31, 2009 at 18:41, Jeff King<peff@peff.net> wrote:
> > IIRC, the message you are seeing comes when the _server_ is an older
> > version of git. It is harmless, though.
> 
> Mhhhh, is it some weird interaction between 'empty repository' patch
> and old server versions, or did this happen too before my patch was
> applied?

I think the former. I thought it was discussed before, but the only
reference I can find is this (see the end of the email):

  http://article.gmane.org/gmane.comp.version-control.git/107626

and I don't see any followup for that specific part of the mail.

-Peff

^ permalink raw reply

* Re: [PATCH] upload-pack: add a trigger for post-upload-pack hook
From: Tom Werner @ 2009-08-31 18:50 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jakub Narebski, Johan Sorensen, Jeff King, Tom Preston-Werner,
	git
In-Reply-To: <7vljl3p2iw.fsf@alter.siamese.dyndns.org>

On Fri, Aug 28, 2009 at 11:17 PM, Junio C Hamano<gitster@pobox.com> wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>
>>> I'd like to suggest the following line from the original patch:
>>>
>>>    full-pack integer::
>>>         1 if the request was considered a full clone, 0 if it was a
>>> partial update (fetch)
>>
>> If it is all "want" and no "have", it is clone or fetch into empty
>> repository.  If additionaly "want"s cover all refs, it is a clone.
>> No need to pass this information: it can be derived.
>
> Well, not exactly.
>
> Here is an iffy RFC patch.  Iffy not in the sense that its implementation
> is questionable, but in the sense that I am not really convinced if the
> distinction between fetching some (or in the worst case, most) but not all
> refs, and fetching full set of refs, into an empty repository is something
> worth making.
>
> Does anybody from GitHub have any input?  Is there something that can
> still improved to suit GitHub's needs?

From GitHub's perspective, we'd treat any clone or fetch into an empty
repo as a clone operation, whether or not that included all of the
refs that were available. For us, the distinction between full and
partial clones is too nuanced to warrant additional code. I'd be happy
with the previous incarnation of the post-upload-pack that simply
sends the HAVEs and WANTs.

Tom

^ permalink raw reply

* Re: [PATCH 3/3] transport: don't show push status if --quiet is given
From: Sebastian Pipping @ 2009-08-31 18:33 UTC (permalink / raw)
  To: Jeff King
  Cc: Junio C Hamano, Nicolas Pitre, Shawn O. Pearce, Albert Astals Cid,
	Pau Garcia i Quiles, git
In-Reply-To: <20090805211700.GA24697@coredump.intra.peff.net>

Hello!


I run git push in a cron job, too.  I want mails in error cases only
so I need git push to print errors but _only_ errors to stderr.  That
seems impossible with 1.6.4.* and related to what you're discussing here.

Does the patch you're building address that case already?  has it been
applied to any branch already?  I got a bit lost in this thread, sorry.



Sebastian

^ permalink raw reply

* Re: clong an empty repo over ssh causes (harmless) fatal
From: Matthieu Moy @ 2009-08-31 17:25 UTC (permalink / raw)
  To: Jeff King; +Cc: Sitaram Chamarty, git
In-Reply-To: <20090831164146.GA23245@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Mon, Aug 31, 2009 at 08:00:41PM +0530, Sitaram Chamarty wrote:
>
>> > Maybe you have an older version of Git?
>> 
>> Had 1.6.4, just tried with 1.6.4.2 -- the error is still there, exactly so.
>> 
>> Anything I can do to provide more info?
>
> IIRC, the message you are seeing comes when the _server_ is an older
> version of git. It is harmless, though.

Since the client and server are the same machine:

    $ git clone ssh://sitaram@localhost/home/sitaram/t/a b

I'd bet Sitaram has two installations of git, and plain ssh to the
machine points to the old one (like a $PATH set in ~/.login and not
~/.profile or something like that).

-- 
Matthieu

^ permalink raw reply

* Re: clong an empty repo over ssh causes (harmless) fatal
From: Sverre Rabbelier @ 2009-08-31 17:12 UTC (permalink / raw)
  To: Jeff King; +Cc: Sitaram Chamarty, Matthieu Moy, git
In-Reply-To: <20090831164146.GA23245@coredump.intra.peff.net>

Heya,

On Mon, Aug 31, 2009 at 18:41, Jeff King<peff@peff.net> wrote:
> IIRC, the message you are seeing comes when the _server_ is an older
> version of git. It is harmless, though.

Mhhhh, is it some weird interaction between 'empty repository' patch
and old server versions, or did this happen too before my patch was
applied?

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: What's cooking in git.git (Aug 2009, #05; Wed, 26)
From: Daniel Barkalow @ 2009-08-31 17:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vskfat07h.fsf@alter.siamese.dyndns.org>

On Sat, 29 Aug 2009, Junio C Hamano wrote:

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> >> There was a discussion that suggests that the use of colon ':' before vcs
> >> helper name needs to be corrected.  Nothing happened since.
> >
> > I believe the outcome of that discussion was:
> >
> >  - We want to keep supporting using regular location URLs that are URLs of 
> >    git repositories (e.g., http://git.savannah.gnu.org/cgit/xboard.git), 
> >    and we probably want to do it with a helper which runs when 
> >    run_command() is given "remote-<scheme>". I think installing hardlinks 
> >    in EXECPATH ended up being the best implementation here.
> 
> That is different from what I recall.
> 
> I think you said <scheme> in the above to mean that in the general URL
> syntax, <scheme> refers to the token before the colon, and you would be
> feeding the rest (i.e. after the colon, and for many <scheme>'s it
> typically begins with //) to the scheme.
>
> A flaw with this that was pointed out was that this conflicts with the
> scp-like syntax.  A remote.$name.url of foo:bar/baz could name
> $HOME/bar/baz on host foo (perhaps a nickname in .ssh/config), or the
> source "foo" helper recognizes with the name bar/baz.
>
> If I recall correctly, suggestions made later in the discussion were to
> use either <helper>+ or <helper>:: as the prefix to avoid this issue, and
> use it to choose remote-<helper> (and I think I probably would vote for
> double-colon, if only to avoid confusion with our own earlier misdesigned
> syntax git+ssh://), so the canonical syntax would be:

(with the syntax <helper>+, "git+ssh://" would specify the helper "git", 
which is as good an explicit identifier of the internal handling as any)

> 
> 	<helper>::<whatever is fed to the helper, typicall a URL>
> 
> while we would support obvious short-hands for transports we traditionally
> supported without explicit "<helper>::" prefix when we choose to eject it
> from "built-in" set of transports.
>
> E.g. http://git.savannah.gnu.org/cgit/xboard.git would be handled by curl
> based walker, so if you spell it in the very canonical form, the url would
> be curl::http://git.savannah.gnu.org/cgit/xboard.git, but nobody has to
> use it.  Instead, the transport dispatcher internally recognizes http://
> and picks the curl based walker helper, which is remote-curl without any
> extra hardlinks.

If the policy is that we're going to have "traditionally supported" 
schemes, where the internal code knows what helper supports them, I can 
fix up the series so that the curl-based helper is named "curl", and we 
can say that the check for "http://", "ftp://", and "https://" is 
recognizing traditionally-supported schemes, and we can defer coming up 
with what the syntax for the explicit handler selection is. (For that 
matter, if there's a // after the colon, it's obviously not a 
handy ssh-style location, since the second slash would do nothing)

I think you're right that we decided that things we used to support 
internally are a special case, and there's no need to try to generalize 
them to be the general mechanism (even though I think we simultaneously 
worked out how to implement the design we were abandoning, which confused 
my memory).

> >  - We want to support a separate "vcs" option for cases where repositories 
> >    in the foreign system need to be addressed through the combination of a 
> >    bunch of options, which will be read from the configuration by the 
> >    helper. The helper which gets run is "remote-<value of vcs option>". 
> >    This is in pu.
> 
> After you explained this in the thread (I think for the second time), I
> see no problem with this, except that I think to support this we should
> notice and raise an error when we see a remote has both vcs and url,
> because the only reason we would want to say "vcs", if I recall your
> explanation correctly, is that such a transport does not have the concept
> of URL, i.e. a well defined single string that identifies the repository.

A user who mostly uses Perforce as a foreign repositories but is using a 
SVN repo on occasion might want to use "vcs" regardless, but I agree with 
forcing the helper to use a different option for the case of a URL that 
git isn't going to look at. That is, you ought to be able to use:

[remote "origin"]
	vcs = svn
	(something) = http://svn.savannah.gnu.org/...

But "(something)" shouldn't be "url".

So my changes will be:

 - name the curl-based helper executable "git-remote-curl", and run it for 
   traditionally supported schemes by special-case.
 - prohibit using both "vcs" and "url" in a remote.

Agreed?

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox