Git development
 help / color / mirror / Atom feed
* Re: [PATCH 5/6 (v2)] upload-pack: send the HEAD information
From: Shawn O. Pearce @ 2008-12-01 16:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <1228140775-29212-6-git-send-email-gitster@pobox.com>

Junio C Hamano <gitster@pobox.com> wrote:
> This implements the server side of protocol extension to show which branch
> the HEAD points at.  The information is sent after the terminating NUL
> that comes after the server capabilities list, to cause older clients to
> ignore it, while allowing newer clients to make use of that information

Ok, not to paint the bikeshed another color or anything ... but
can we do something to make this slightly more extensible?  I like
the "lets hide it behind the NUL" bit a lot better than the prior
iteration, but I wonder if we shouldn't do something slightly
different.

Maybe we put on the first capability line a flag that lets the
client know we have symref data in the advertised list, and then
instead of sticking only HEAD into that first ref we put the names
of the symrefs after the ref they point to.

So we might see something like:

  xxxx......................... refs/heads/boo\0with-symref\0
  xxxx......................... refs/heads/master\0HEAD\0
  xxxx......................... refs/remotes/origin/HEAD\0refs/remotes/origin/master\0

etc.  Its probably harder to produce the output for, but it permits
advertising all of the symrefs on the remote side, which may be good
for --mirror, among other uses.  It also should make it easier to put
multiple symrefs down pointing at the same real ref, they could just
be a space delimited list stored after the ref name, and if its the
first ref in the stream, after the other capability advertisement.

Actually, since the capability line is space delimited and space is
not valid in a ref name, we could just include into the capability
line like "symref=HEAD", but I still like the idea of listing it
after each ref, to reduce the risk of running into pkt-line length
limitations.

-- 
Shawn.

^ permalink raw reply

* Re: SPEC files for Git
From: Michael J Gruber @ 2008-12-01 16:00 UTC (permalink / raw)
  To: devel; +Cc: git
In-Reply-To: <4933FFC6.8080306@morey-chaisemartin.com>

Nicolas Morey-Chaisemartin venit, vidit, dixit 01.12.2008 16:16:
> Hi,
> 
> I'm trying to recompile gitweb1.6.0.4-1 for RHEL5, but I'm missing the
> spec file.
> I've checked the git SRP. It generates a lot of things but no RPM for
> gitweb.

RHEL5 probably comes with git 1.5.something, and its spec file won't
work nicely with git 1.6.x (because of new install locations).

> Is there any place with an official spec file? Or has anyone made one
> which he could share?

git source comes with a spec file template. If you have a git checkout
(or source tree) then "make git.spec" will generate it.

It does not generate a gitweb subpackage, but you only need to add a few
lines, following the example of the archimport package, say.

Or, even simpler: Just take the srpm from Fedora 10 and rebuild (gitweb
is a subpackage of git in F10, so you need the git srpm (e.g.
'yumdownloader --source gitweb' on an F10 instance). Installing the srpm
(as non-root) gives you the spec, I'm sure you know ;)

Michael

^ permalink raw reply

* Re: RPM for gitweb (was: SPEC files for git)
From: Jakub Narebski @ 2008-12-01 15:57 UTC (permalink / raw)
  To: Nicolas Morey-Chaisemartin; +Cc: git
In-Reply-To: <4933FFC6.8080306@morey-chaisemartin.com>

Nicolas Morey-Chaisemartin <devel@morey-chaisemartin.com> writes:

> I'm trying to recompile gitweb-1.6.0.4-1 for RHEL5, but I'm missing
> the spec file.  I've checked the git SRPM. 

Which SRPM? The one for RHEL or Fedora Core, or the one from
http://www.kernel.org/pub/software/scm/git/RPMS/SRPMS/ ?

> It generates a lot of things but no RPM for gitweb.
> 
> Is there any place with an official spec file? Or has anyone made one
> which he could share?

The .spec file for various git subpackages (git, git-all, git-svn,
gitk, git-gui, git-email, perl-Git, etc.) is in git src.rpm file, and
source for it can be found as git.spec.in file in git.git repository.

There is no gitweb RPM subpackage in generic .spec file from git.git
repository (from kernel.org) because 1.) installation of CGI scripts
(or mod_perl legacy scripts) depends on distribution (what web server
is installed, where and how config for this web server is managed);
2.) there is no standard place where to put git repositories to be
hosted by gitweb like /var/www/html/ or /var/www/cgi-bin/

Read gitweb/INSTALL[1] from git repository on how to install gitweb.

[1] http://repo.or.cz/w/git.git/:/gitweb/README
-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [PATCH 0/6 (v2)] Detecting HEAD more reliably while cloning
From: Johannes Sixt @ 2008-12-01 15:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <1228140775-29212-1-git-send-email-gitster@pobox.com>

Junio C Hamano schrieb:
> Instead of introducing a full-fledged protocol extension, this round hides
> the new information in the same place as the server capabilities list that
> is used to implement protocol extension is hidden from older clients.

Not that it makes a lot of difference, but why do you want to *hide* the
information? Can't we just have a capability-with-parameter:

 ... shallow no-progress include-tag head=refs/heads/foo\ bar ...

(with spaces and backslashes escaped)?

-- Hannes

^ permalink raw reply

* Re: [PATCH 5/6 (v2)] upload-pack: send the HEAD information
From: Jakub Narebski @ 2008-12-01 15:40 UTC (permalink / raw)
  To: git
In-Reply-To: <1228140775-29212-6-git-send-email-gitster@pobox.com>

Junio C Hamano wrote:

> This implements the server side of protocol extension to show which branch
> the HEAD points at.  The information is sent after the terminating NUL
> that comes after the server capabilities list, to cause older clients to
> ignore it, while allowing newer clients to make use of that information

By the way, is negotiating protocol extensions, and supported protocol
extensions documented somewhere in Documentation/technical/ ?

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* Re: two questions about the format of loose object
From: Shawn O. Pearce @ 2008-12-01 15:32 UTC (permalink / raw)
  To: Liu Yubao; +Cc: git list
In-Reply-To: <493399B7.5000505@gmail.com>

Liu Yubao <yubao.liu@gmail.com> wrote:
> 
> In current implementation the loose objects are compressed:
> 
>      loose object = deflate(typename + <space> + size + '\0' + data)
...
> * Question 1:
> 
> Why not use the format below for loose object?
>     loose object = typename + <space> + size + '\0' + deflate(data)

Historical accident.  We really should have used a format more
like what you are asking here, because it makes inflation easier.
The pack file format uses a header structure sort of like this,
for exactly that reason.  IOW we did learn our mistakes and fix them.

If you look up the new style loose object code you'll see that it
has a format like this (sort of), the header is actually the same
format that is used in the pack files, making it smaller than what
you propose but also easier to unpack as the code can be reused
with the pack reading code.

Unfortunately the new style loose object was phased out; it never
really took off and it made the code much more complex.  So it was
pulled in commit 726f852b0ed7e03e88c419a9996c3815911c9db1:

 Author: Nicolas Pitre <nico@cam.org>:
 >  deprecate the new loose object header format
 >
 >  Now that we encourage and actively preserve objects in a packed form
 >  more agressively than we did at the time the new loose object format and
 >  core.legacyheaders were introduced, that extra loose object format
 >  doesn't appear to be worth it anymore.
 >
 >  Because the packing of loose objects has to go through the delta match
 >  loop anyway, and since most of them should end up being deltified in
 >  most cases, there is really little advantage to have this parallel loose
 >  object format as the CPU savings it might provide is rather lost in the
 >  noise in the end.
 >
 >  This patch gets rid of core.legacyheaders, preserve the legacy format as
 >  the only writable loose object format and deprecate the other one to
 >  keep things simpler.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH 1/8] generate-cmdlist.sh: avoid selecting synopsis at wrong place
From: Johannes Schindelin @ 2008-12-01 15:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy, git
In-Reply-To: <7vtz9oq92i.fsf@gitster.siamese.dyndns.org>

Hi,

On Mon, 1 Dec 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > As it is, the patch series is _already_ hard to review (as it is large 
> > not only in term of number of patches, but also individual patch 
> > size), _especially_ given the fact that there is no clear, precise and 
> > short description of why/how the sparse checkout is implemented.
> 
> Hmm, can you really tell the lack of such description without reading 
> the series, I have to wonder...

Okay, I thought it was obvious, but here is a template for the BLURP of 
the cover letter that would at least get me started:

-- snip --
A "sparse checkout" is an index/working directory pair where not all 
files/directories of the HEAD commit are actually checked out in the 
working directory.  Instead, they are marked as "not being checked out" in 
the index.

The real meat of this series is patch *** M/N *** which teaches Git to 
understand the *** XYZ flag *** for index entries.

The following operations are affected by sparse checkout: *** X, Y, Z ***

These operations respect sparse checkouts by *** THIS, THIS AND THIS ***.

The first patch really should be independent, but patch *** M/N *** would 
fail without it.
-- snap --

And of course, the whole BLURP should not consist of 10-20 line 
paragraphs, but try to fit everything into 3-4 line paragraphs (I seem to 
remember that there was a mail on this list saying than more than 4 
lines/paragraph are too much for the average attention span...).

Ciao,
Dscho

^ permalink raw reply

* SPEC files for Git
From: Nicolas Morey-Chaisemartin @ 2008-12-01 15:16 UTC (permalink / raw)
  To: git

Hi,

I'm trying to recompile gitweb1.6.0.4-1 for RHEL5, but I'm missing the
spec file.
I've checked the git SRP. It generates a lot of things but no RPM for
gitweb.

Is there any place with an official spec file? Or has anyone made one
which he could share?

Best regards

Nicolas

^ permalink raw reply

* Re: two questions about the format of loose object
From: Shawn O. Pearce @ 2008-12-01 15:21 UTC (permalink / raw)
  To: Liu Yubao; +Cc: Junio C Hamano, git list
In-Reply-To: <4933AE55.2090007@gmail.com>

Liu Yubao <yubao.liu@gmail.com> wrote:
> Thanks for your explanation, but I doubt if it's too costly to change the
> format of loose object, after all this doesn't change the format of pack
> file and affect git-pull/fetch of old git client. 

It is too costly; Jakub pointed out the dumb protocol clients
would have issues with the new format.  Anyone copying a repository
between machines using scp or a USB memory stick may also run into
a problem.  Etc.
 
> Some cons and pros.
> 
> cons:
> 
> * old git client can't read loose objects in new format
>   (People degrade git rarely and old git can read pack files
>    generated by new git, so it's not a big problem)

That's a pretty big con.  We can also add slower performance on NFS,
as has been reported already by others.
 
> pros:
> 
> * avoid compressing and uncompressing loose objects that are likely
>   frequently used when you are coding/merging

True, loose objects are among the more frequently accessed items.

> * share loose objects among multipe git processes

Probably not a huge issue.  How many concurrent git processes are
you running on the same object store at once?  During development?
Its probably not more than 1.  So sharing the objects doesn't make
a very compelling argument.

> * the new code path is simpler although we will have more code paths for
>   compatibility

The new code path is more complex, because although one branch is
very simple (mmap and use) the other code paths have to stay for
backwards compatibility.  Every time you add a branch point the
code gets more complex.  It works well enough now, and is at least
one branch point simpler than what you are proposing.  So I'm not
really interested in seeing the change made.

-- 
Shawn.

^ permalink raw reply

* gitk: git bisect view doesn't work
From: Johannes Sixt @ 2008-12-01 14:43 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: Git Mailing List

gitk bails out like this if I do "git bisect view":

$ git bisect start HEAD HEAD~2
Bisecting: 0 revisions left to test after this
[9a61060c7e0d112d0742f5b845210ea8c41b6c6b] Added encoding

$ git bisect view
Error in startup script: can't read "notflag": no such variable
    while executing
"expr {!$notflag}"
    ("--not" arm line 2)
    invoked from within
"switch -glob -- $arg {
            "-d" -
            "--date-order" {
                set vdatemode($n) 1
                # remove from origargs in case we hit an unknown option
                set origarg..."
    (procedure "parseviewargs" line 21)
    invoked from within
"parseviewargs $view $args"
    (procedure "start_rev_list" line 27)
    invoked from within
"start_rev_list $curview"
    (procedure "getcommits" line 5)
    invoked from within
"getcommits {}"
    (file "/usr/local/bin/gitk" line 10897)

$ git version
git version 1.6.1.rc1

-- Hannes

^ permalink raw reply

* Re: [TOPGIT] Resolving conflicts
From: Uwe Kleine-König @ 2008-12-01 14:36 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: git, Petr Baudis, martin f krafft
In-Reply-To: <20081201121819.GB13495@pengutronix.de>

[added martin f krafft to Cc:]

Hi,

> Another thing is that the exported branch contains an empty commit
> resulting from t/whatever (and a corresponding empty patch when exported
> as a quilt series)
I fixed one half of this in with a patch sent to the ML:

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

it didn't made it upstream yet, though.

Best regards
Uwe

^ permalink raw reply

* [PATCH 6/6 (v2)] clone: test the new HEAD detection logic
From: Junio C Hamano @ 2008-12-01 14:12 UTC (permalink / raw)
  To: git
In-Reply-To: <1228140775-29212-6-git-send-email-gitster@pobox.com>

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5601-clone.sh |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 78a3fa6..6d4665b 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -125,4 +125,15 @@ test_expect_success 'clone to destination with extra trailing /' '
 
 '
 
+test_expect_success 'clone from a repository with two identical branches' '
+
+	(
+		cd src &&
+		git checkout -b another master
+	) &&
+	git clone src target-3 &&
+	test "z$( cd target-3 && git symbolic-ref HEAD )" = zrefs/heads/another
+
+'
+
 test_done
-- 
1.6.0.4.864.g0f47a

^ permalink raw reply related

* [PATCH 5/6 (v2)] upload-pack: send the HEAD information
From: Junio C Hamano @ 2008-12-01 14:12 UTC (permalink / raw)
  To: git
In-Reply-To: <1228140775-29212-5-git-send-email-gitster@pobox.com>

This implements the server side of protocol extension to show which branch
the HEAD points at.  The information is sent after the terminating NUL
that comes after the server capabilities list, to cause older clients to
ignore it, while allowing newer clients to make use of that information

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 upload-pack.c |   14 +++++++++++---
 1 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/upload-pack.c b/upload-pack.c
index e5adbc0..4aa444a 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -579,9 +579,17 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo
 	if (!o)
 		die("git upload-pack: cannot find object %s:", sha1_to_hex(sha1));
 
-	if (capabilities)
-		packet_write(1, "%s %s%c%s\n", sha1_to_hex(sha1), refname,
-			0, capabilities);
+	if (capabilities) {
+		unsigned char dummy[20];
+		const char *target = resolve_ref("HEAD", dummy, 0, NULL);
+
+		if (!target)
+			packet_write(1, "%s %s%c%s\n", sha1_to_hex(sha1), refname,
+				     0, capabilities);
+		else
+			packet_write(1, "%s %s%c%s%c%s\n", sha1_to_hex(sha1), refname,
+				     0, capabilities, 0, target);
+	}
 	else
 		packet_write(1, "%s %s\n", sha1_to_hex(sha1), refname);
 	capabilities = NULL;
-- 
1.6.0.4.864.g0f47a

^ permalink raw reply related

* [PATCH 4/6 (v2)] clone: find the current branch more explicitly
From: Junio C Hamano @ 2008-12-01 14:12 UTC (permalink / raw)
  To: git
In-Reply-To: <1228140775-29212-4-git-send-email-gitster@pobox.com>

This makes "git clone" use the HEAD information sent by an updated
uploader to point the resulting HEAD at the current branch that is not
'master' but happens to point at the same commit as 'master'.

IOW, immediately after doing:

    $ git checkout -b another master

a clone made out of that repository will check out 'another' branch, not
'master'.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-clone.c |   24 +++++++++++++++++++-----
 1 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/builtin-clone.c b/builtin-clone.c
index 2feac9c..a8b8d56 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -299,13 +299,27 @@ static const struct ref *locate_head(const struct ref *refs,
 	const struct ref *remote_head = NULL;
 	const struct ref *remote_master = NULL;
 	const struct ref *r;
-	for (r = refs; r; r = r->next)
-		if (!strcmp(r->name, "HEAD"))
+
+	for (r = refs; r; r = r->next) {
+		if (!strcmp(r->name, "HEAD")) {
 			remote_head = r;
+			break;
+		}
+	}
 
-	for (r = mapped_refs; r; r = r->next)
-		if (!strcmp(r->name, "refs/heads/master"))
-			remote_master = r;
+	if (remote_head && remote_head->symref) {
+		for (r = mapped_refs; r; r = r->next)
+			if (!strcmp(r->name, remote_head->symref)) {
+				remote_master = r;
+				break;
+			}
+	}
+
+	if (!remote_master) {
+		for (r = mapped_refs; r; r = r->next)
+			if (!strcmp(r->name, "refs/heads/master"))
+				remote_master = r;
+	}
 
 	if (remote_head_p)
 		*remote_head_p = remote_head;
-- 
1.6.0.4.864.g0f47a

^ permalink raw reply related

* [PATCH 3/6 (v2)] connect.c::read_extra_info(): find where HEAD points at
From: Junio C Hamano @ 2008-12-01 14:12 UTC (permalink / raw)
  To: git
In-Reply-To: <1228140775-29212-3-git-send-email-gitster@pobox.com>

This actually implements the protocol extension to receive the HEAD symref
information that is hidden after the server capabilities list.  Nobody
uses the information yet, though.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 connect.c |   27 +++++++++++++++++++++++++--
 1 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/connect.c b/connect.c
index 114d691..aa4757a 100644
--- a/connect.c
+++ b/connect.c
@@ -53,7 +53,7 @@ static void add_extra_have(struct extra_have_objects *extra, unsigned char *sha1
  * NUL".  "len" is the remaining number of bytes.  The caller knows
  * that the original packet contains more information than that.
  */
-static void read_extra_info(char *line, int len)
+static void read_extra_info(char *line, int len, char **hpa)
 {
 	/*
 	 * The first such "extra" piece of information is the list of
@@ -64,6 +64,20 @@ static void read_extra_info(char *line, int len)
 	free(server_capabilities);
 	server_capabilities = xmalloc(infolen);
 	memcpy(server_capabilities, line, infolen);
+	if (infolen == len)
+		return;
+	/*
+	 * The uploader can optionally tell us where the HEAD pointer
+	 * points at after that.
+	 */
+	line += infolen;
+	len -= infolen;
+
+	infolen = strlen(line) + 1;
+
+	free(*hpa);
+	*hpa = xmalloc(infolen);
+	memcpy(*hpa, line, infolen);
 }
 
 /*
@@ -74,6 +88,9 @@ struct ref **get_remote_heads(int in, struct ref **list,
 			      unsigned int flags,
 			      struct extra_have_objects *extra_have)
 {
+	struct ref **ref_list = list;
+	char *head_points_at = NULL;
+
 	*list = NULL;
 	for (;;) {
 		struct ref *ref;
@@ -97,7 +114,8 @@ struct ref **get_remote_heads(int in, struct ref **list,
 
 		name_len = strlen(name);
 		if (len != name_len + 41)
-			read_extra_info(name + name_len + 1, len - (name_len + 41));
+			read_extra_info(name + name_len + 1, len - (name_len + 41),
+					&head_points_at);
 
 		if (extra_have &&
 		    name_len == 5 && !memcmp(".have", name, 5)) {
@@ -114,6 +132,11 @@ struct ref **get_remote_heads(int in, struct ref **list,
 		*list = ref;
 		list = &ref->next;
 	}
+	if (head_points_at) {
+		struct ref *head = find_ref_by_name(*ref_list, "HEAD");
+		if (head)
+			head->symref = head_points_at;
+	}
 	return list;
 }
 
-- 
1.6.0.4.864.g0f47a

^ permalink raw reply related

* [PATCH 1/6 (v2)] get_remote_heads(): refactor code to read "server capabilities"
From: Junio C Hamano @ 2008-12-01 14:12 UTC (permalink / raw)
  To: git
In-Reply-To: <1228140775-29212-1-git-send-email-gitster@pobox.com>

This just moves the code to read the server capability list that is hidden
after "40-byte hex object name, SP, refname, NUL" on the information the
uploading end initially sends.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 connect.c |   21 +++++++++++++++++----
 1 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/connect.c b/connect.c
index 584e04c..932c503 100644
--- a/connect.c
+++ b/connect.c
@@ -49,6 +49,21 @@ static void add_extra_have(struct extra_have_objects *extra, unsigned char *sha1
 }
 
 /*
+ * "line" points at the byte just after "40-byte hex, SP, refname,
+ * NUL".  "len" is the remaining number of bytes.  The caller knows
+ * that the original packet contains more information than that.
+ */
+static void read_extra_info(char *line, int len)
+{
+	/*
+	 * The first such "extra" piece of information is the list of
+	 * server capabilities.
+	 */
+	free(server_capabilities);
+	server_capabilities = xstrdup(line);
+}
+
+/*
  * Read all the refs from the other end
  */
 struct ref **get_remote_heads(int in, struct ref **list,
@@ -78,10 +93,8 @@ struct ref **get_remote_heads(int in, struct ref **list,
 		name = buffer + 41;
 
 		name_len = strlen(name);
-		if (len != name_len + 41) {
-			free(server_capabilities);
-			server_capabilities = xstrdup(name + name_len + 1);
-		}
+		if (len != name_len + 41)
+			read_extra_info(name + name_len + 1, len - (name_len + 41));
 
 		if (extra_have &&
 		    name_len == 5 && !memcmp(".have", name, 5)) {
-- 
1.6.0.4.864.g0f47a

^ permalink raw reply related

* [PATCH 2/6 (v2)] connect.c::read_extra_info(): prepare to receive more than server capabilities
From: Junio C Hamano @ 2008-12-01 14:12 UTC (permalink / raw)
  To: git
In-Reply-To: <1228140775-29212-2-git-send-email-gitster@pobox.com>

This still does not actually parse and accept anything more than the list
of server capabilities, but prepares for the uploader that sends more
"extra" information than that.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 connect.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/connect.c b/connect.c
index 932c503..114d691 100644
--- a/connect.c
+++ b/connect.c
@@ -59,8 +59,11 @@ static void read_extra_info(char *line, int len)
 	 * The first such "extra" piece of information is the list of
 	 * server capabilities.
 	 */
+	int infolen = strlen(line) + 1;
+
 	free(server_capabilities);
-	server_capabilities = xstrdup(line);
+	server_capabilities = xmalloc(infolen);
+	memcpy(server_capabilities, line, infolen);
 }
 
 /*
-- 
1.6.0.4.864.g0f47a

^ permalink raw reply related

* [PATCH 0/6 (v2)] Detecting HEAD more reliably while cloning
From: Junio C Hamano @ 2008-12-01 14:12 UTC (permalink / raw)
  To: git

This is another approach to the same problem that a repository cloned from
another repository whose default branch is not 'master' can use 'master'
as the default.

The current code has to guess where the HEAD in the original repository
points at, because the original protocol tells what object each ref points
at but does not talk about which other ref a symbolic ref points at.  The
implication of this is that if you prepare another branch that points at
your master, like this:

	$ git checkout -b another master

and keep that other branch checked out (and in sync with 'master'), a
clone made from such a repository may incorrectly have its HEAD pointing
at 'master', not 'another'.

Instead of introducing a full-fledged protocol extension, this round hides
the new information in the same place as the server capabilities list that
is used to implement protocol extension is hidden from older clients.
This way, it does not have to work around the code structure imposed by
the transport API, does not have to introduce an extra round trip, and
does not have to trigger an annoying (but harmless) error message when an
older client contacts a new uploader.

  [1/6] get_remote_heads(): refactor code to read "server capabilities"
  [2/6] connect.c::read_extra_info(): prepare to receive more than server
        capabilities
  [3/6] connect.c::read_extra_info(): find where HEAD points at
  [4/6] clone: find the current branch more explicitly
  [5/6] upload-pack: send the HEAD information
  [6/6] clone: test the new HEAD detection logic

The first four are the client side, the fifth one is the uploader side,
and the last one is the test.  After storing these patches in separate
files, you would build this history (on top of 'master'):

    git am 1 2 3 4
    git reset --hard HEAD~4    5---------------M---6
    git am 5                  /               /
    git merge HEAD@{2}        ---1---2---3---4
    git am 6

 builtin-clone.c  |   24 +++++++++++++++++++-----
 connect.c        |   47 +++++++++++++++++++++++++++++++++++++++++++----
 t/t5601-clone.sh |   11 +++++++++++
 upload-pack.c    |   14 +++++++++++---
 4 files changed, 84 insertions(+), 12 deletions(-)

^ permalink raw reply

* Re: [PATCH 1/8] generate-cmdlist.sh: avoid selecting synopsis at wrong place
From: Junio C Hamano @ 2008-12-01 14:11 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Nguyễn Thái Ngọc Duy, git
In-Reply-To: <alpine.DEB.1.00.0812011505551.30769@pacific.mpi-cbg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Sun, 30 Nov 2008, Nguyễn Thái Ngọc Duy wrote:
>
>> In "common" man pages there is luckily no "NAME" anywhere except at 
>> beginning of documents. If there is another "NAME", sed could mis-select 
>> it and lead to common-cmds.h corruption. So better nail it at beginning 
>> of line, which would reduce corruption chance.
>
> I have no idea why you put this into the sparse checkout patch series.

That is because a documentation that triggers the misfortune this patch
fixes is introduced in the series.  I actually scratched head myself,
though, and come to think of it, I should have complained that this patch
should state that because it does not look like it has anything do with
the main topic of the series.

> As it is, the patch series is _already_ hard to review (as it is large not 
> only in term of number of patches, but also individual patch size), 
> _especially_ given the fact that there is no clear, precise and short 
> description of why/how the sparse checkout is implemented.

Hmm, can you really tell the lack of such description without reading the
series, I have to wonder...

^ permalink raw reply

* Re: [PATCH 1/8] generate-cmdlist.sh: avoid selecting synopsis at wrong place
From: Nguyen Thai Ngoc Duy @ 2008-12-01 14:11 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0812011505551.30769@pacific.mpi-cbg.de>

On 12/1/08, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
>
>  On Sun, 30 Nov 2008, Nguy­n Thái Ng÷c Duy wrote:
>
>  > In "common" man pages there is luckily no "NAME" anywhere except at
>  > beginning of documents. If there is another "NAME", sed could mis-select
>  > it and lead to common-cmds.h corruption. So better nail it at beginning
>  > of line, which would reduce corruption chance.
>
>
> I have no idea why you put this into the sparse checkout patch series.

It's more an independent bug fix. I posted this once but it was not
caught up. I intended to just put some comments in BLURB area to tell
people to get this patch from mail archive. But that way seems
inconvenient. Without this patch, you won't be able to compile the
series because of common-cmds.h corruption.
-- 
Duy

^ permalink raw reply

* Re: [PATCH 4/5] upload-pack: implement protocol extension "symbolic-ref"
From: Junio C Hamano @ 2008-12-01 14:03 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20081130180214.GA10375@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Sun, Nov 30, 2008 at 01:57:29AM -0800, Junio C Hamano wrote:
> ...
>> This is unfortunate because it forces an extra round trip (receiving end
>> sends a "please tell me symbolic-ref" packet, and then upload side sends
>> "here are the requested information" packet), but it has to be implemented
>> this way because (1) ls-remote may need to ask for this information, in
>> which case there is no "want" to be sent; and (2) the transport API
>> insists that transport_get_remote_refs() returns the final list, and does
>> not allow augmenting what was initially obtained from the call to it by
>> later calls to transport_fetch_refs() easily.
>
> Hrm. For (1), could we allow either interaction method? IOW, allow
> requesting a symref on the first want line, _or_ by separate "symbolic
> ref" packet? That would allow clients who are using "want" to
> piggy-back the symref request as an optimization, but not restrict those
> that just want to ask for it?

I think I found another hole in the protocol that we can use to carry the
"which branch does HEAD points at" information in a backward compatible
way, and it would be a much less intrusive although more sneaky way.  And
it would not have to suffer from the above issues at all.

A patchset follows shortly...

^ permalink raw reply

* Re: [PATCH 1/8] generate-cmdlist.sh: avoid selecting synopsis at wrong place
From: Johannes Schindelin @ 2008-12-01 14:10 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1228042478-1886-2-git-send-email-pclouds@gmail.com>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 972 bytes --]

Hi,

On Sun, 30 Nov 2008, Nguyễn Thái Ngọc Duy wrote:

> In "common" man pages there is luckily no "NAME" anywhere except at 
> beginning of documents. If there is another "NAME", sed could mis-select 
> it and lead to common-cmds.h corruption. So better nail it at beginning 
> of line, which would reduce corruption chance.

I have no idea why you put this into the sparse checkout patch series.

As it is, the patch series is _already_ hard to review (as it is large not 
only in term of number of patches, but also individual patch size), 
_especially_ given the fact that there is no clear, precise and short 
description of why/how the sparse checkout is implemented.

For example, instead of using the BLURP area of the cover letter to put 
forth convincing arguments why this is needed, and why it is implemented 
in the best possible manner, you wasted two lines stating the obvious.

IMO that is almost like _asking_ people not to have a look at it.

Ciao,
Dscho

^ permalink raw reply

* [PATCH 2/2] Documentation: describe how to "bisect skip" a range of commits
From: Christian Couder @ 2008-12-01 13:39 UTC (permalink / raw)
  To: Junio C Hamano, Johannes Schindelin, Johannes Sixt; +Cc: git, H. Peter Anvin

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/git-bisect.txt |   21 ++++++++++++++++++++-
 git-bisect.sh                |    2 +-
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index 39034ec..147ea38 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -19,7 +19,7 @@ on the subcommand:
  git bisect start [<bad> [<good>...]] [--] [<paths>...]
  git bisect bad [<rev>]
  git bisect good [<rev>...]
- git bisect skip [<rev>...]
+ git bisect skip [(<rev>|<range>)...]
  git bisect reset [<branch>]
  git bisect visualize
  git bisect replay <logfile>
@@ -164,6 +164,25 @@ But computing the commit to test may be slower afterwards and git may
 eventually not be able to tell the first bad among a bad and one or
 more "skip"ped commits.
 
+You can even skip a range of commits, instead of just one commit,
+using the "'<commit1>'..'<commit2>'" notation. For example:
+
+------------
+$ git bisect skip v2.5..v2.6
+------------
+
+would mean that no commit between `v2.5` excluded and `v2.6` included
+can be tested.
+
+Note that if you want to also skip the first commit of a range you can
+use something like:
+
+------------
+$ git bisect skip v2.5 v2.5..v2.6
+------------
+
+and the commit pointed to by `v2.5` will be skipped too.
+
 Cutting down bisection by giving more parameters to bisect start
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/git-bisect.sh b/git-bisect.sh
index 58d6276..da92d1e 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -9,7 +9,7 @@ git bisect bad [<rev>]
         mark <rev> a known-bad revision.
 git bisect good [<rev>...]
         mark <rev>... known-good revisions.
-git bisect skip [<rev>...]
+git bisect skip [(<rev>|<range>)...]
         mark <rev>... untestable revisions.
 git bisect next
         find next bisection to test and check it out.
-- 
1.6.0.4.837.g71a67

^ permalink raw reply related

* [PATCH 1/2] bisect: fix "git bisect skip <commit>" and add tests cases
From: Christian Couder @ 2008-12-01 13:39 UTC (permalink / raw)
  To: Junio C Hamano, Johannes Schindelin, Johannes Sixt; +Cc: git, H. Peter Anvin

The patch that allows "git bisect skip" to be passed a range of
commits using the "<commit1>..<commit2>" notation is flawed because
it introduces a regression when it was passed a simple rev or commit.

"git bisect skip <commit>" doesn't work any more, because <commit>
is quoted but not properly unquoted.

This patch fixes that and add tests cases to better check when it is
passed commits and range of commits.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 git-bisect.sh               |    2 +-
 t/t6030-bisect-porcelain.sh |   19 ++++++++++++++++++-
 2 files changed, 19 insertions(+), 2 deletions(-)

	Sorry I overlooked something in my previous patch (v2).

diff --git a/git-bisect.sh b/git-bisect.sh
index 6706bc1..58d6276 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -203,7 +203,7 @@ bisect_skip() {
 	    esac
             all="$all $revs"
         done
-        bisect_state 'skip' $all
+        eval bisect_state 'skip' $all
 }
 
 bisect_state() {
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 85fa39c..dd7eac8 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -313,8 +313,25 @@ test_expect_success 'bisect run & skip: find first bad' '
 	grep "$HASH6 is first bad commit" my_bisect_log.txt
 '
 
-test_expect_success 'bisect starting with a detached HEAD' '
+test_expect_success 'bisect skip only one range' '
+	git bisect reset &&
+	git bisect start $HASH7 $HASH1 &&
+	git bisect skip $HASH1..$HASH5 &&
+	test "$HASH6" = "$(git rev-parse --verify HEAD)" &&
+	test_must_fail git bisect bad > my_bisect_log.txt &&
+	grep "first bad commit could be any of" my_bisect_log.txt
+'
 
+test_expect_success 'bisect skip many ranges' '
+	git bisect start $HASH7 $HASH1 &&
+	test "$HASH4" = "$(git rev-parse --verify HEAD)" &&
+	git bisect skip $HASH2 $HASH2.. ..$HASH5 &&
+	test "$HASH6" = "$(git rev-parse --verify HEAD)" &&
+	test_must_fail git bisect bad > my_bisect_log.txt &&
+	grep "first bad commit could be any of" my_bisect_log.txt
+'
+
+test_expect_success 'bisect starting with a detached HEAD' '
 	git bisect reset &&
 	git checkout master^ &&
 	HEAD=$(git rev-parse --verify HEAD) &&
-- 
1.6.0.4.837.g71a67

^ permalink raw reply related

* Re: [PATCH] git-cvsimport: add support for cvs pserver password scrambling.
From: Johannes Schindelin @ 2008-12-01 13:43 UTC (permalink / raw)
  To: Dirk Hörner; +Cc: git
In-Reply-To: <5794AED2-43FF-4441-8292-0C9BFB3139A2@gmail.com>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 332 bytes --]

Hi,

On Fri, 28 Nov 2008, Dirk Hörner wrote:

> Instead of a cleartext password, the CVS pserver expects a scrambled one 
> in the authentication request. With this patch it is possible to import 
> CVS repositories only accessible via pserver and user/password.

The patch looks obvious enough; care to add a testcase?

Ciao,
Dscho

^ 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