Git development
 help / color / mirror / Atom feed
* Re: What's cooking in git.git (Aug 2009, #05; Wed, 26)
From: Daniel Barkalow @ 2009-09-01  4:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vhbvnzhdq.fsf@alter.siamese.dyndns.org>

On Mon, 31 Aug 2009, Junio C Hamano wrote:

> 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 ;-)?

That's just weird, and looks wrong to me, like ssh tunnelled over the git 
native protocol. I think, if we support it, it would have to be another 
special case.

> 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.

I was thinking that the dispatcher would always pass the whole thing, and 
the svn helper would have to deal with svn+http:// and svn+svn:// as well 
as svn+ssh://. The dispatcher shouldn't be familiar with what the backends 
do, but the backends could stand to be familiar with what might be there 
just to get to them. On the other hand, I don't have a good intuition for 
what makes sense to users of such systems, since I've only extensively 
used cvs, git, and perforce.

> > 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.

Fair enough.

> >> 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.

Agreed.

> 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.

I don't really have a particularly good guess about the case of vcs and a 
URL, because I haven't used systems that are not git and use URLs.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: [PATCH] fast-import.c: Silence build warning
From: Stephen Boyd @ 2009-09-01  4:06 UTC (permalink / raw)
  To: Michael Wookey; +Cc: Junio C Hamano, git
In-Reply-To: <d2e97e800908311655t553d6c4bo6ed45fe37819c1d8@mail.gmail.com>

Michael Wookey wrote:
> 2009/9/1 Junio C Hamano <gitster@pobox.com>:
>> 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?  ;-) 

I noticed this warning recently too when I upgraded my box and a flurry
of fwrite() unused warnings came up. Looks like ubuntu patches that
issue[1] by arguing it's a valid programming style to
fwrite/fflush/ferror. Perhaps this programming style could follow a
similar reasoning?

It gets better though. Commit c55fae4 (fast-import.c: stricter strtoul
check, silence compiler warning, 2008-12-21) made this change already.
Then commit eb3a9dd (Remove unused function scope local variables,
2009-03-07) came by and removed it. Unless the definition of strtoul
drops the attribute I fear we'll keep going back and forth.

-- Footnotes --
[1] https://lists.ubuntu.com/archives/ubuntu-devel/2009-March/027832.html

^ permalink raw reply

* Re: Improve on 'approxidate'
From: Linus Torvalds @ 2009-09-01  3:27 UTC (permalink / raw)
  To: Jeff King; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <20090830223558.GA29807@coredump.intra.peff.net>



On Sun, 30 Aug 2009, Jeff King wrote:
> 
> This breaks relative dates like "3.months.ago", because
> approxidate_alpha needs to see the "current" date in tm (and now it sees
> -1, subtracts from it, and assumes we are just crossing a year boundary
> because of the negative).  3.years.ago is also broken, but I don't think
> 3.days.ago is.

Gaah. Thanks for noticing and the fixes. I had tested the relative modes, 
but only the "fixed offset" ones (days, hours, minutes, seconds), not the 
months and years cases.

			Linus

^ permalink raw reply

* Re: [PATCH v2 3/4] tests: add date printing and parsing tests
From: Jeff King @ 2009-09-01  3:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, Alex Riesen, git
In-Reply-To: <20090831023015.GC5507@coredump.intra.peff.net>

On Sun, Aug 30, 2009 at 10:30:15PM -0400, Jeff King wrote:

>   - We confirm that the improvements in b5373e9 and 1bddb25
>     work.

Ugh. I just realized (when explaining how awesome git resurrect was in
another mail) that I managed to bungle these commit hashes (and the one
mentioned in the following patch).

What happened is that I was building on the topic branch and lazily did
a "rebase -i origin" to fix up my patches. I left the first two patches
untouched, of course, but they still ended up with new committer
information.

As my patches are merged to 'next' already, I think it is too late to
fixup the commit message. But for posterity, the correct referenced
commits are 9029055 and 36e4986.

Caveat rebaser.

-Peff

^ permalink raw reply

* Re: A note from the maintainer: Follow-up questions (MaintNotes)
From: Jeff King @ 2009-09-01  2:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: David Chanters, git
In-Reply-To: <7v8wgzla02.fsf@alter.siamese.dyndns.org>

On Mon, Aug 31, 2009 at 06:38:37PM -0700, Junio C Hamano wrote:

>     a71f64a Merge branch 'pk/import-dirs' into pu
>     ce6cd39 Merge branch 'jh/cvs-helper' into pu
>     ...
>     2178d02 Merge branch 'jc/log-tz' into pu
>     ...
>     927d129 Merge branch 'lt/approxidate' into jch
>     35ada54 Merge branch 'tr/reset-checkout-patch' into jch
>     d82f86c Merge branch 'db/vcs-helper' (early part) into jch
> 
> So if you for example happen to be interested in jc/log-tz topic,
> you would do something like:
> 
>     $ git checkout -b jc/log-tz 2178d02^2
>     $ git log -p master..
> 
> to check out, and view what changes the topic introduces.

Another alternative: Thomas Rast wrote a handy script called
'git-resurrect' which does this for you. It's carried in the contrib/
section right now. I used it just the other day to pull out the
lt/approxidate topic branch, which I then built some follow-up patches
for. Usage is something like:

  # contrib scripts aren't installed by "make install",
  # so put it somewhere in your PATH
  $ cp contrib/git-resurrect.sh ~/bin/git-resurrect

  $ git resurrect -m lt/approxidate
  ** Candidates for lt/approxidate **
  931e8e2 [22 hours ago] fix approxidate parsing of relative months and years
  ** Restoring lt/approxidate to 931e8e2 fix approxidate parsing of relative months and years

-Peff

^ permalink raw reply

* Re: Troubles building man pages
From: Junio C Hamano @ 2009-09-01  2:44 UTC (permalink / raw)
  To: Brian S. Schang; +Cc: git
In-Reply-To: <4A9C8750.60308@lists.schang.net>

"Brian S. Schang" <git@lists.schang.net> writes:

>>     ASCIIDOC git-am.xml
>>     XMLTO git-am.1
>> I/O error : Attempt to load network entity http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd
>> /raida/packages/git/Documentation/git-am.xml:2: warning: failed to load external entity "http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd"
>> D DocBook XML V4.2//EN" "http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd"
>
> I have read through INSTALL and Documentation/README. I have tried to
> cover all dependencies, but I suspect that I'm missing something
> simple.
>
> Note that 'git' itself compiles fine and I can build the HTML pages
> fine also. It seems to be the man pages that cause the problem.
>
> Note that I am using openSUSE 11.1. I was able to build the man pages
> on my older openSUSE 11.0 machine, but not any longer.

The following was an ancient experience of mine with a different distro
but I am reasonably sure it would point you in the right direction.

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

^ permalink raw reply

* Troubles building man pages
From: Brian S. Schang @ 2009-09-01  2:30 UTC (permalink / raw)
  To: git

Hello:

I am having problem building the man pages for git v1.6.4.2. I am 
getting the follow stream of errors: (sorry for the long lines)

> # make ASCIIDOC8=Yes man
> GIT_VERSION = 1.6.4.2
> make -C Documentation man
> make[1]: Entering directory `/raida/packages/git/Documentation'
>     GEN doc.dep
> make[2]: Entering directory `/raida/packages/git'
> make[2]: `GIT-VERSION-FILE' is up to date.
> make[2]: Leaving directory `/raida/packages/git'
> make[1]: Leaving directory `/raida/packages/git/Documentation'
> make[1]: Entering directory `/raida/packages/git/Documentation'
> make[2]: Entering directory `/raida/packages/git'
> make[2]: `GIT-VERSION-FILE' is up to date.
> make[2]: Leaving directory `/raida/packages/git'
>     ASCIIDOC git-add.xml
>     XMLTO git-add.1
> I/O error : Attempt to load network entity http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd
> /raida/packages/git/Documentation/git-add.xml:2: warning: failed to load external entity "http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd"
> D DocBook XML V4.2//EN" "http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd"
>                                                                                ^
>     ASCIIDOC git-am.xml
>     XMLTO git-am.1
> I/O error : Attempt to load network entity http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd
> /raida/packages/git/Documentation/git-am.xml:2: warning: failed to load external entity "http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd"
> D DocBook XML V4.2//EN" "http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd"

I have read through INSTALL and Documentation/README. I have tried to 
cover all dependencies, but I suspect that I'm missing something simple.

Note that 'git' itself compiles fine and I can build the HTML pages fine 
also. It seems to be the man pages that cause the problem.

Note that I am using openSUSE 11.1. I was able to build the man pages on 
my older openSUSE 11.0 machine, but not any longer.

I would appreciate any advice you are willing to share.

Thank you.

-- 
Brian Schang

^ permalink raw reply

* Re: A note from the maintainer: Follow-up questions (MaintNotes)
From: Junio C Hamano @ 2009-09-01  1:38 UTC (permalink / raw)
  To: David Chanters; +Cc: git
In-Reply-To: <ac3d41850908301519s2cf8a45auf11fb4c9285c0cb5@mail.gmail.com>

David Chanters <david.chanters@googlemail.com> writes:

> I'd often wondered when I have read various posts of the git mailing
> list on gmane, just how it is I am supposed to track:
>
> dc/some-topic-feature
>
> ... Junio, are these topic branches ones you actively have somewhere
> in your own private checkout?  Yes, I appreciate that when I read a
> given post to the mailing list, you or other people will sometimes
> make reference to these topic branches, but what do I do if I am
> interested in finding out about one of them?

    $ git log --oneline --first-parent origin/master..origin/pu

would be a handy way to view where the tip of each branch is.

    a71f64a Merge branch 'pk/import-dirs' into pu
    ce6cd39 Merge branch 'jh/cvs-helper' into pu
    ...
    2178d02 Merge branch 'jc/log-tz' into pu
    ...
    927d129 Merge branch 'lt/approxidate' into jch
    35ada54 Merge branch 'tr/reset-checkout-patch' into jch
    d82f86c Merge branch 'db/vcs-helper' (early part) into jch

So if you for example happen to be interested in jc/log-tz topic,
you would do something like:

    $ git checkout -b jc/log-tz 2178d02^2
    $ git log -p master..

to check out, and view what changes the topic introduces. 

Some hawk-eyed people may have already noticed this, but I recently
updated the script I used to maintain the "What's cooking" messages, and
the entries come with when and at what commit each part of the series has
been merged to 'next'.  For example, the lt/approxidate topic reads like
this:

  * lt/approxidate (2009-08-30) 6 commits
    (merged to 'next' on 2009-08-30 at e016e3d)
   + fix approxidate parsing of relative months and years
   + tests: add date printing and parsing tests
   + refactor test-date interface
   + Add date formatting and parsing functions relative to a given time
    (merged to 'next' on 2009-08-26 at 62853f9)
   + Further 'approxidate' improvements
   + Improve on 'approxidate'

Time flows from bottom to top in this output, so this tells us that the
first two patches in the series has been in 'next' for five days or so,
and the tests and a fix in 4 follow-up patches came later, merged to
'next' yesterday.  You can use

    $ git checkout -b lt/approxidate e016e3d^2

to get at its tip.

> ..., how do you go about creating and
> maintaining these topic branches -- are you making heavy use of "git
> am"

Save patches from the list in mbox, review them in the mbox while fixing
trivial breakages, and finally:

    $ git checkout -b ai/topic-name master
    $ git am -s that.mbox

where "ai" is typically the author's initial, and topic-name names the
topic just like you would name a function.  A topic typically forks from
the tip of master if it is a new feature, or a much older commit in maint
if it is a fix (and in such a case, topic-name typically begins with
a string "maint-").

> I ask because of the following snippet from "MaintNotes":
>
>     The two branches "master" and "maint" are never rewound, and
>     "next" usually will not be either (this automatically means the
>     topics that have been merged into "next" are usually not
>     rebased, and you can find the tip of topic branches you are
>     interested in from the output of "git log next"). You should be
>     able to safely track them.
>
> I am not sure if there's any real use-case for this, but I will ask
> anyway:  is the above saying that I am able to *checkout* one of these
> topic-branches just from their presence in "next" alone?  I appreciate
> that the point is somewhat moot since the topic branch has already
> been merged into "next", but I can surely see this as a really useful
> way for people to manage topic-branches in a shared environment:
> people can simply pick a topic branch out from the integrated one --
> in this case "next".

Surely, see above.  And by checking out the topic alone, you can test and
enhance it in isolation.

If you come up with a follow-up patch based on one particular topic, in
other words, building on ai/topic-name created like the above example, as
opposed to building on 'next', it would be easier for me to integrate it,
too, because the way I accept a follow-up patch to a particular topic is
by doing this:

    $ git checkout ai/topic-name
    $ git am -s followup-mail.mbox

The result will be tested in isolation and if it is good it would be
merged to 'next' again.

    $ git merge ai/topic-name

> I'm obviously missing something here -- but why is rebasing these
> existing topic branches (I assume on top of "pu") more useful than
> just merging them into "pu" -- like you do with "next"?

Somebody may send a few patches [PATCH 1/3] thru [PATCH 3/3] whose
quality is sub-par but tries to tackle a good problem.  I'd queue it in
'pu'.

    $ git checkout -b dc/cool-feature
    $ git am -s david-chanters.mbox
    $ git checkout pu
    $ git merge dc/cool-feature

In a few days, people would notice that the series has a lot of room for
improvement, and offer suggestions.  You would send replacement patches
based on the review.  Perhaps you squashed the first two patches from the
original into one, and added another patch for test suite, and the new
series is marked as [PATCH v2 1/3] thru [PATCH v2 3/3].

In general, the early attempts of a topic that have never been merged to
'next' are not worth keeping in the public history.  The original author
is free to keep them in his private tree, but there is no point in forever
keeping earlier mistakes, bugs, typos and implementation based on a flawed
design, all of which later were corrected in _my_ history.

So I would replace the whole topic with the new series.

    $ git checkout dc/cool-feature
    $ git reset --hard HEAD~3
    $ git am -s david-chanters-v2.mbox

The whole point of replacing the contents of this topic was to get rid of
the mistakes in your earlier round, so it does not make much sense to
merge this to 'pu' without rewinding 'pu' first.

After the topic is merged to 'next', obviously I cannot sanely rewind and
rebuild it.  Everything goes incremental from then on (see the earlier
example of applying followup-mail.mbox).

> If regular readers of the git mailing list wish to track this topic
> branch, can they do so from you only until it's merged into "next"?

Sorry, but I am not sure if I understand the question.

> And a related question:  If you decide a given topic in pu is declared
> to "be dropped", is this done by rebasing (as you mentioned earlier)
> so as to remove any trace of the topic branch ever having been in
> "pu", or am I reading too much into "dropping" here?  :)

Essentially, I keep a list of branches that are in 'next' and another list
of branches that are in 'pu'.  At the end of each integration cycle, I do
a rough equivalent of:

    $ git checkout next
    $ git merge ..a good topic that is not fully in next..
    $ git merge ..another good topic that is not fully in next..
    $ make test ;# make sure next is ok
    $ git branch -f pu ;# get rid of everything in pu
    $ git checkout pu
    $ for branch in ..the list of branches to be in pu..
      do
          git merge $branch || break
      done

Discarding a topic from 'pu' simply means I do not merge the topic in the
last loop.

^ permalink raw reply

* Re: clong an empty repo over ssh causes (harmless) fatal
From: Jeff King @ 2009-09-01  1:08 UTC (permalink / raw)
  To: Sverre Rabbelier
  Cc: Björn Steinbrink, Matthieu Moy, Sitaram Chamarty, git
In-Reply-To: <fabb9a1e0908311550r1b549eb2k2df65c188a0ea6a0@mail.gmail.com>

On Tue, Sep 01, 2009 at 12:50:25AM +0200, Sverre Rabbelier wrote:

> 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?

Yeah. We call upload-pack on the remote side, realize there are no refs,
and then we just stop talking. Meanwhile upload-pack is waiting for a
packet to say "these are the refs that I want". So the client really
needs to send an extra packet saying "list of refs is finished".

The patch below seems to work for me, but I'm a little concerned how it
might impact other transports. It actually calls the transport's
fetch method when we have no refs that we want. So each transport must
recognize that we want zero refs and do the appropriate thing. In this
case, for the git protocol, we want to:

  - do a packet_flush to signal "no more refs" to the remote side

  - be aware that we might have zero refs and avoid establishing a new
    connection in that case

Other transports might need to be tweaked similarly, but I don't have
time to test at the moment.

diff --git a/builtin-clone.c b/builtin-clone.c
index 0d2b4a8..f198c01 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -515,8 +515,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 					     option_upload_pack);
 
 		refs = transport_get_remote_refs(transport);
-		if(refs)
-			transport_fetch_refs(transport, refs);
+		transport_fetch_refs(transport, refs);
 	}
 
 	if (refs) {
diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index 629735f..04a3776 100644
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -803,6 +803,8 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
 		nr_heads = remove_duplicates(nr_heads, heads);
 	if (!ref) {
 		packet_flush(fd[1]);
+		if (!nr_heads)
+			return NULL;
 		die("no matching remote head");
 	}
 	ref_cpy = do_fetch_pack(fd, ref, nr_heads, heads, pack_lockfile);
diff --git a/transport.c b/transport.c
index f2bd998..25e8946 100644
--- a/transport.c
+++ b/transport.c
@@ -512,6 +512,8 @@ static int fetch_refs_via_pack(struct transport *transport,
 		origh[i] = heads[i] = xstrdup(to_fetch[i]->name);
 
 	if (!data->conn) {
+		if (!nr_heads)
+			return 0;
 		connect_setup(transport, 0, 0);
 		get_remote_heads(data->fd[0], &refs_tmp, 0, NULL, 0, NULL);
 	}

^ permalink raw reply related

* 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


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