Git development
 help / color / mirror / Atom feed
* Re: Something weird is happening...
From: Junio C Hamano @ 2009-01-29  6:56 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: Git Mailing List, Ingo Molnar
In-Reply-To: <49814BA4.6030705@zytor.com>

"H. Peter Anvin" <hpa@zytor.com> writes:

> I was investigating a problem that Ingo Molnar reported on the
> linux-2.6-tip.git repository on kernel.org.  Unfortunately I was not
> able to reproduce his problem (which is a problem in itself) but I did
> run into another oddity:
>
> : hera 4 ; git fsck
>
> [lots of dangling commits deleted]
> missing blob af0e01d4c663a101f48614e40d006ed6272d5c36
>
> : hera 5 ; git cat-file blob af0e01d4c663a101f48614e40d006ed6272d5c36
> /*
>  *  debugfs.h - a tiny little debug file system
>  *
>  *  Copyright (C) 2004 Greg Kroah-Hartman <greg@kroah.com>
>  *  Copyright (C) 2004 IBM Inc.
>  *
> [... rest of blob deleted ...]
>
> Okay, what is going on here?

Does "git fsck --full" report the blob missing?

^ permalink raw reply

* Re: How to install and use a custom merge driver
From: Alec Clews @ 2009-01-29  7:00 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20090129050348.GA31202@coredump.intra.peff.net>

Jeff King wrote:
> On Thu, Jan 29, 2009 at 04:47:19AM +0000, Alec Clews wrote:
>
>   
> What git version are you using? Can you post a shell snippet that breaks
> reliably?
>   

I used 1.5.6.3 on Ubuntu and 1.6.1.1 on Cygwin.

The problem appears to be spaces around '=' in the attributes file. This 
fails for me


----------8<----------------------------------
commit() {
 echo $1 >file && git add file && git commit -m $1
}

rm -rf repo

mkdir repo && cd repo && git init
commit base
commit branch-master
git checkout -b other HEAD^
commit branch-other

echo '* merge = overwrite' >.git/info/attributes
cat >>.git/config <<'EOF'
[merge "overwrite"]
 name = overwrite using cp
 driver= cp %B %A
EOF

git merge master
----------8<---------------------------------



-- 
Alec Clews
Personal <alec.clews@gmail.com>			Melbourne, Australia.
Jabber:  alecclews@jabber.org.au		PGPKey ID: 0x9BBBFC7C
Blog  http://alecthegeek.wordpress.com/

^ permalink raw reply

* Re: How to install and use a custom merge driver
From: Jeff King @ 2009-01-29  7:11 UTC (permalink / raw)
  To: Alec Clews; +Cc: git
In-Reply-To: <49815423.8000902@gmail.com>

On Thu, Jan 29, 2009 at 06:00:51PM +1100, Alec Clews wrote:

> The problem appears to be spaces around '=' in the attributes file. This  
> fails for me
> [...]
> echo '* merge = overwrite' >.git/info/attributes

Ah, OK. That isn't supposed to work. The attributes are whitespace
separated, and some don't even have an equals at all (e.g., "merge",
"-merge", and "merge=foo" are all valid). So that parses to "please use
the ordinary text merge driver", and some attributes that nobody uses
called "=" and "overwrite".

-Peff

^ permalink raw reply

* Re: [PATCH] symbolic ref: refuse non-ref targets in HEAD
From: Junio C Hamano @ 2009-01-29  7:53 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20090129045205.GA31183@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> I know that using symbolic-ref manually is rare, but both I and the
> original poster have been bitten by this (and figuring out what is going
> on and fixing it is quite painful). But most importantly, I don't think
> this can possibly hurt anyone trying to use this legitimately, since the
> exact thing it is protecting against corrupts your repo. :)

I generally do not like adding artificial limitation to plumbing like this
patch does, because the end user making silly mistake using plumbing is a
sign that there was something lacking in the Porcelain.

But for this particular case, I do not think any future usage of
symbolic-ref plubming will get inconvenienced with the change.  I would
even suggest making the check tighter to insist on refs/heads/ (not just
refs/) and tighten validate_headref() in path.c to match.

> Please beware that running the test script on the current "master" will
> actually hose your git repo (test 3 kills the trash directory's
> .git/HEAD, which means test 4 thinks your parent .git/ is its current
> repo). Maybe it makes sense to do a precautionary reset in between.

In addition, perhaps it may make sense to use test_create_repo to go one
level deeper before starting to play around, so that trash directory's
repository will prevent you from going any further up.

^ permalink raw reply

* Re: "malloc failed"
From: Junio C Hamano @ 2009-01-29  7:53 UTC (permalink / raw)
  To: Jeff King; +Cc: David Abrahams, git
In-Reply-To: <20090129055633.GA32609@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Subject: [PATCH] avoid 31-bit truncation in write_loose_object
>
> The size of the content we are adding may be larger than
> 2.1G (i.e., "git add gigantic-file"). Most of the code-path
> to do so uses size_t or unsigned long to record the size,
> but write_loose_object uses a signed int.

Thanks.

I wonder if some analysis tool like sparse can help us spot these...

^ permalink raw reply

* Re: [PATCH] symbolic ref: refuse non-ref targets in HEAD
From: Jeff King @ 2009-01-29  8:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vljsuh7kf.fsf@gitster.siamese.dyndns.org>

On Wed, Jan 28, 2009 at 11:53:20PM -0800, Junio C Hamano wrote:

> I generally do not like adding artificial limitation to plumbing like this
> patch does, because the end user making silly mistake using plumbing is a
> sign that there was something lacking in the Porcelain.

Nor do I. I only propose it in this case because

  1. We cannot possibly be hurting somebody's workflow, since it
     produces nonsensical results currently.

  2. The damage it does is so annoying to recover from.

I have no problem with symbolic-ref eventually learning to handle these
situations in some more sane manner; in the meantime, I think it makes
sense to prevent nasty breakage. And I don't think we're closing any
doors for the future; the behavior will switch from "you aren't allowed
to do this" to "does something sensible".

> But for this particular case, I do not think any future usage of
> symbolic-ref plubming will get inconvenienced with the change.  I would
> even suggest making the check tighter to insist on refs/heads/ (not just
> refs/) and tighten validate_headref() in path.c to match.

Reasonable. Updated series to follow.

> > Please beware that running the test script on the current "master" will
> > actually hose your git repo (test 3 kills the trash directory's
> > .git/HEAD, which means test 4 thinks your parent .git/ is its current
> > repo). Maybe it makes sense to do a precautionary reset in between.
> 
> In addition, perhaps it may make sense to use test_create_repo to go one
> level deeper before starting to play around, so that trash directory's
> repository will prevent you from going any further up.

That sort of helps, but only by luck. Each test kills off one layer of
repo. So the first one kills the test_create_repo, and the second one
kills the trash directory. Adding another test would kill off the main
repo. :) So you have to do something per-test. I'll do that in the
re-roll.

-Peff

^ permalink raw reply

* Re: What's cooking in git.git (Jan 2009, #07; Wed, 28)
From: Charles Bailey @ 2009-01-29  8:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwscej26i.fsf@gitster.siamese.dyndns.org>

On Wed, Jan 28, 2009 at 06:06:45PM -0800, Junio C Hamano wrote:
> * cb/mergetool (Wed Jan 21 22:57:48 2009 +0000) 1 commit
>  + mergetool: respect autocrlf by using checkout-index
> 

Can you hold off on merging this one? I now think that there's a
cleaner way of doing this and I would like the opportunity for a
rethink.

-- 
Charles Bailey
http://ccgi.hashpling.plus.com/blog/

^ permalink raw reply

* Re: [PATCH/RFC v1 5/6] combine-diff.c: remove a call to fstat() inside show_patch_diff()
From: Kjetil Barvik @ 2009-01-29  8:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vab9akj2p.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Kjetil Barvik <barvik@broadpark.no> writes:
>>
>>> Currently inside show_patch_diff() we have and fstat() call after an
>>> ok lstat() call.  Since we before the call to fstat() have already
>>> test for the link case with S_ISLNK() the fstat() can be removed.
>>
>> Good eyes.  Thanks.
>
> Heh, I noticed you will update the commit log message, so I'll dequeue
> this and wait for an update.

  Yes, I am planing a v2 in 1 or 3 days (it takes some time to run long-
  running tests).  And then hopefully I have addressed all comments so
  far.  Thanks for comments!

  -- kjetil

^ permalink raw reply

* Re: What's cooking in git.git (Jan 2009, #07; Wed, 28)
From: Junio C Hamano @ 2009-01-29  8:26 UTC (permalink / raw)
  To: Charles Bailey; +Cc: git
In-Reply-To: <20090129081438.GA10490@hashpling.org>

Charles Bailey <charles@hashpling.org> writes:

> On Wed, Jan 28, 2009 at 06:06:45PM -0800, Junio C Hamano wrote:
>> * cb/mergetool (Wed Jan 21 22:57:48 2009 +0000) 1 commit
>>  + mergetool: respect autocrlf by using checkout-index
>> 
>
> Can you hold off on merging this one? I now think that there's a
> cleaner way of doing this and I would like the opportunity for a
> rethink.

Sure, it is not in 'master' yet.

But it's in 'next', so incremental updates from now on, please.

^ permalink raw reply

* [PATCH 1/2] validate_headref: tighten ref-matching to just branches
From: Jeff King @ 2009-01-29  8:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20090129080145.GA777@coredump.intra.peff.net>

When we are trying to determine whether a directory contains
a git repository, one of the tests we do is to check whether
HEAD is either a symlink or a symref into the "refs/"
hierarchy, or a detached HEAD.

We can tighten this a little more, though: a non-detached
HEAD should always point to a branch (since checking out
anything else should result in detachment), so it is safe to
check for "refs/heads/".

Signed-off-by: Jeff King <peff@peff.net>
---
When I was writing the commit message, my spider sense tingled a little
bit. I don't think this is an unreasonable thing to do, but I also don't
know that it helps in any meaningful way. There is no evidence of people
having other stuff in their HEADs anyway.

 path.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/path.c b/path.c
index a074aea..108d9e9 100644
--- a/path.c
+++ b/path.c
@@ -154,7 +154,7 @@ int validate_headref(const char *path)
 	/* Make sure it is a "refs/.." symlink */
 	if (S_ISLNK(st.st_mode)) {
 		len = readlink(path, buffer, sizeof(buffer)-1);
-		if (len >= 5 && !memcmp("refs/", buffer, 5))
+		if (len >= 11 && !memcmp("refs/heads/", buffer, 11))
 			return 0;
 		return -1;
 	}
@@ -178,7 +178,7 @@ int validate_headref(const char *path)
 		len -= 4;
 		while (len && isspace(*buf))
 			buf++, len--;
-		if (len >= 5 && !memcmp("refs/", buf, 5))
+		if (len >= 11 && !memcmp("refs/heads/", buf, 11))
 			return 0;
 	}
 
-- 
1.6.1.1.425.gdbb13

^ permalink raw reply related

* [PATCH 2/2] symbolic ref: refuse non-ref targets in HEAD
From: Jeff King @ 2009-01-29  8:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20090129080145.GA777@coredump.intra.peff.net>

When calling "git symbolic-ref" it is easy to forget that
the target must be a fully qualified ref. E.g., you might
accidentally do:

  $ git symbolic-ref HEAD master

Unfortunately, this is very difficult to recover from,
because the bogus contents of HEAD make git believe we are
no longer in a git repository (as is_git_dir explicitly
checks for "^refs/heads/" in the HEAD target). So
immediately trying to fix the situation doesn't work:

  $ git symbolic-ref HEAD refs/heads/master
  fatal: Not a git repository

and one is left editing the .git/HEAD file manually.

Furthermore, one might be tempted to use symbolic-ref to set
up a detached HEAD:

  $ git symbolic-ref HEAD `git rev-parse HEAD`

which sets up an even more bogus HEAD:

  $ cat .git/HEAD
  ref: 1a9ace4f2ad4176148e61b5a85cd63d5604aac6d

This patch introduces a small safety valve to prevent the
specific case of anything not starting with refs/heads/ to
go into HEAD. The scope of the safety valve is intentionally
very limited, to make sure that we are not preventing any
behavior that would otherwise be valid (like pointing a
different symref than HEAD outside of refs/heads/).

Signed-off-by: Jeff King <peff@peff.net>
---
Changes from the original are:

  - s,refs/,refs/heads/, in the code and commit message

  - test non-ref and non-branch separately; under the current
    implementation it is obvious the former will work if the latter
    does, but I think by testing intent and not the implementation, the
    tests are more future-proof

  - tests make sure to restore validity of trash directory immediately
    after running

 builtin-symbolic-ref.c  |    3 +++
 t/t1401-symbolic-ref.sh |   41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 0 deletions(-)
 create mode 100755 t/t1401-symbolic-ref.sh

diff --git a/builtin-symbolic-ref.c b/builtin-symbolic-ref.c
index bfc78bb..cafc4eb 100644
--- a/builtin-symbolic-ref.c
+++ b/builtin-symbolic-ref.c
@@ -44,6 +44,9 @@ int cmd_symbolic_ref(int argc, const char **argv, const char *prefix)
 		check_symref(argv[0], quiet);
 		break;
 	case 2:
+		if (!strcmp(argv[0], "HEAD") &&
+		    prefixcmp(argv[1], "refs/heads/"))
+			die("Refusing to point HEAD outside of refs/heads/");
 		create_symref(argv[0], argv[1], msg);
 		break;
 	default:
diff --git a/t/t1401-symbolic-ref.sh b/t/t1401-symbolic-ref.sh
new file mode 100755
index 0000000..569f341
--- /dev/null
+++ b/t/t1401-symbolic-ref.sh
@@ -0,0 +1,41 @@
+#!/bin/sh
+
+test_description='basic symbolic-ref tests'
+. ./test-lib.sh
+
+# If the tests munging HEAD fail, they can break detection of
+# the git repo, meaning that further tests will operate on
+# the surrounding git repo instead of the trash directory.
+reset_to_sane() {
+	echo ref: refs/heads/foo >.git/HEAD
+}
+
+test_expect_success 'symbolic-ref writes HEAD' '
+	git symbolic-ref HEAD refs/heads/foo &&
+	echo ref: refs/heads/foo >expect &&
+	test_cmp expect .git/HEAD
+'
+
+test_expect_success 'symbolic-ref reads HEAD' '
+	echo refs/heads/foo >expect &&
+	git symbolic-ref HEAD >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'symbolic-ref refuses non-ref for HEAD' '
+	test_must_fail git symbolic-ref HEAD foo
+'
+reset_to_sane
+
+test_expect_success 'symbolic-ref refuses non-branch for HEAD' '
+	test_must_fail git symbolic-ref HEAD refs/foo
+'
+reset_to_sane
+
+test_expect_success 'symbolic-ref refuses bare sha1' '
+	echo content >file && git add file && git commit -m one
+	test_must_fail git symbolic-ref HEAD `git rev-parse HEAD`
+'
+reset_to_sane
+
+test_done
-- 
1.6.1.1.425.gdbb13

^ permalink raw reply related

* Re: [PATCH] symbolic ref: refuse non-ref targets in HEAD
From: Jeff King @ 2009-01-29  8:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20090129080145.GA777@coredump.intra.peff.net>

On Thu, Jan 29, 2009 at 03:01:45AM -0500, Jeff King wrote:

> > In addition, perhaps it may make sense to use test_create_repo to go one
> > level deeper before starting to play around, so that trash directory's
> > repository will prevent you from going any further up.
> 
> That sort of helps, but only by luck. Each test kills off one layer of
> repo. So the first one kills the test_create_repo, and the second one
> kills the trash directory. Adding another test would kill off the main
> repo. :) So you have to do something per-test. I'll do that in the
> re-roll.

It occurs to me you perhaps did mean to do something per-test, like:

  test_create_repo foo && (cd foo && do_the_actual_test)

That would work, too.

-Peff

^ permalink raw reply

* Re: What's cooking in git.git (Jan 2009, #07; Wed, 28)
From: Charles Bailey @ 2009-01-29  9:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vbptqh60w.fsf@gitster.siamese.dyndns.org>

On Thu, Jan 29, 2009 at 12:26:39AM -0800, Junio C Hamano wrote:
> Charles Bailey <charles@hashpling.org> writes:
> 
> > On Wed, Jan 28, 2009 at 06:06:45PM -0800, Junio C Hamano wrote:
> >> * cb/mergetool (Wed Jan 21 22:57:48 2009 +0000) 1 commit
> >>  + mergetool: respect autocrlf by using checkout-index
> >> 
> >
> > Can you hold off on merging this one? I now think that there's a
> > cleaner way of doing this and I would like the opportunity for a
> > rethink.
> 
> Sure, it is not in 'master' yet.
> 
> But it's in 'next', so incremental updates from now on, please.
> 

OK, I've thought again and I still think that this patch is good.

Just so you know what I was thinking...

I felt that the new shell function that calls git checkout-index was a
bit clunky. git checkout-index --temp creates its own temporary file
and then the git mergetool renames this file to the temporary filename
that it had already decided on.

An earlier patch to mergetool was careful to ensure that mergetool
temporaries maintained the file extension of the target file in order
to help syntax highlighting merge tools. For this reason, just using
checkout-index generated filenames is not a sufficient solution.

I had two ideas, the first was that perhaps git mergetool could choose
a temporary naming scheme that could be matched by the appropriate use
of checkout-index --prefix. This would obviously preserve the file
extension but it's fairly obvious that it would have surprising
behaviour for merging files in subfolders.

My last idea would be to add an explicit --to-path= to git
checkout-index. It would make the mergetool code simpler but I'm not
sure how useful it would be in any other circumstance.

-- 
Charles Bailey
http://ccgi.hashpling.plus.com/blog/

^ permalink raw reply

* [PATCH] Support various HTTP authentication methods
From: Moriyoshi Koizumi @ 2009-01-29  9:32 UTC (permalink / raw)
  To: git

Currently there is no way to specify the preferred authentication
method for the HTTP backend and it always ends up with the CURL's
default
settings.

This patch enables it if supported by CURL, adding a couple of new
settings
and config environment variables listed below (the names within the
parentheses indicate the latter.)

- http.auth (GIT_HTTP_AUTH)
  Specifies the preferred authentication method for HTTP.  This can
  be a method name or the combination of those separated by comma. Valid
  values are "basic", "digest", "gss" and "ntlm". You can also specify
  "any" (all of the above), "anysafe" (all of the above except "basic").

  Note that the strings are treated case-insensitive.

- http.proxy_auth (GIT_HTTP_PROXY_AUTH)
  Specifies the preferred authentication method method for HTTP proxy.
  The same thing as above applies to this setting.

Signed-off-by: Moriyoshi Koizumi <mozo@mozo.jp>
---
 http.c |  105
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 105 insertions(+), 0 deletions(-)

diff --git a/http.c b/http.c
index ee58799..889135f 100644
--- a/http.c
+++ b/http.c
@@ -25,6 +25,12 @@ static long curl_low_speed_limit = -1;
 static long curl_low_speed_time = -1;
 static int curl_ftp_no_epsv = 0;
 static const char *curl_http_proxy = NULL;
+#if LIBCURL_VERSION_NUM >= 0x070a06
+static const char *curl_http_auth = NULL;
+#endif
+#if LIBCURL_VERSION_NUM >= 0x070a07
+static const char *curl_http_proxy_auth = NULL;
+#endif
 
 static struct curl_slist *pragma_header;
 
@@ -153,11 +159,67 @@ static int http_options(const char *var, const
char *value, void *cb)
 			return git_config_string(&curl_http_proxy, var, value);
 		return 0;
 	}
+#if LIBCURL_VERSION_NUM >= 0x070a06
+	if (!strcmp("http.auth", var)) {
+		if (curl_http_auth == NULL)
+			return git_config_string(&curl_http_auth, var, value);
+		return 0;
+	}
+#endif
+#if LIBCURL_VERSION_NUM >= 0x070a07
+	if (!strcmp("http.proxy_auth", var)) {
+		if (curl_http_proxy_auth == NULL)
+			return git_config_string(&curl_http_proxy_auth, var, value);
+		return 0;
+	}
+#endif
 
 	/* Fall back on the default ones */
 	return git_default_config(var, value, cb);
 }
 
+#if LIBCURL_VERSION_NUM >= 0x070a06
+static long get_curl_auth_bitmask(const char* auth_method)
+{
+	char *buf = xmalloc(strlen(auth_method) + 1);
+	const unsigned char *p = (const unsigned char *)auth_method;
+	long mask = CURLAUTH_NONE;
+
+	for (;;) {
+		char *q = buf;
+		while (*p && isspace(*p))
+			++p;
+
+		while (*p && *p != ',')
+			*q++ = tolower(*p++);
+
+		while (--q >= buf && isspace(*(unsigned char *)q));
+		++q;
+
+		*q = '\0';
+
+		if (strcmp(buf, "basic") == 0)
+			mask |= CURLAUTH_BASIC;
+		else if (strcmp(buf, "digest") == 0)
+			mask |= CURLAUTH_DIGEST;
+		else if (strcmp(buf, "gss") == 0)
+			mask |= CURLAUTH_GSSNEGOTIATE;
+		else if (strcmp(buf, "ntlm") == 0)
+			mask |= CURLAUTH_NTLM;
+		else if (strcmp(buf, "any") == 0)
+			mask |= CURLAUTH_ANY;
+		else if (strcmp(buf, "anysafe") == 0)
+			mask |= CURLAUTH_ANYSAFE;
+
+		if (!*p)
+			break;
+		++p;
+	}
+
+	return mask;
+}
+#endif
+
 static CURL* get_curl_handle(void)
 {
 	CURL* result = curl_easy_init();
@@ -210,6 +272,20 @@ static CURL* get_curl_handle(void)
 	if (curl_http_proxy)
 		curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
 
+	if (curl_http_auth) {
+		long n = get_curl_auth_bitmask(curl_http_auth);
+		curl_easy_setopt(result, CURLOPT_HTTPAUTH, n);
+	}
+
+	if (curl_http_proxy) {
+		curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
+
+		if (curl_http_proxy_auth) {
+			long n = get_curl_auth_bitmask(curl_http_proxy_auth);
+			curl_easy_setopt(result, CURLOPT_PROXYAUTH, n);
+		}
+	}
+
 	return result;
 }
 
@@ -258,6 +334,21 @@ void http_init(struct remote *remote)
 	if (low_speed_time != NULL)
 		curl_low_speed_time = strtol(low_speed_time, NULL, 10);
 
+#if LIBCURL_VERSION_NUM >= 0x070a06
+	{
+		char *http_auth = getenv("GIT_HTTP_AUTH");
+		if (http_auth)
+			curl_http_auth = xstrdup(http_auth);
+	}
+#endif
+#if LIBCURL_VERSION_NUM >= 0x070a07
+	{
+		char *http_proxy_auth = getenv("GIT_HTTP_PROXY_AUTH");
+		if (http_proxy_auth)
+			curl_http_proxy_auth = xstrdup(http_proxy_auth);
+	}
+#endif
+
 	git_config(http_options, NULL);
 
 	if (curl_ssl_verify == -1)
@@ -309,6 +400,20 @@ void http_cleanup(void)
 		free((void *)curl_http_proxy);
 		curl_http_proxy = NULL;
 	}
+
+#if LIBCURL_VERSION_NUM >= 0x070a06
+	if (curl_http_auth) {
+		free((void *)curl_http_auth);
+		curl_http_auth = NULL;
+	}
+#endif
+
+#if LIBCURL_VERSION_NUM >= 0x070a07
+	if (curl_http_proxy_auth) {
+		free((void *)curl_http_proxy_auth);
+		curl_http_proxy_auth = NULL;
+	}
+#endif
 }
 
 struct active_request_slot *get_active_slot(void)
-- 
1.5.6.3

^ permalink raw reply related

* Re: [PATCH] Support various HTTP authentication methods
From: Junio C Hamano @ 2009-01-29 10:08 UTC (permalink / raw)
  To: Moriyoshi Koizumi; +Cc: git
In-Reply-To: <1233221532.21518.1.camel@lena.gsc.riken.jp>

Moriyoshi Koizumi <mozo@mozo.jp> writes:

> Currently there is no way to specify the preferred authentication
> method for the HTTP backend and it always ends up with the CURL's
> default
> settings.
>
> This patch enables it if supported by CURL, adding a couple of new

"it" in "enables it" is a bit unclear...

> Signed-off-by: Moriyoshi Koizumi <mozo@mozo.jp>
> ---
>  http.c |  105
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 105 insertions(+), 0 deletions(-)

Linewrapped and whitespace damaged patch that would not apply.

> diff --git a/http.c b/http.c
> index ee58799..889135f 100644
> --- a/http.c
> +++ b/http.c
> @@ -25,6 +25,12 @@ static long curl_low_speed_limit = -1;
>  static long curl_low_speed_time = -1;
>  static int curl_ftp_no_epsv = 0;
>  static const char *curl_http_proxy = NULL;
> +#if LIBCURL_VERSION_NUM >= 0x070a06
> +static const char *curl_http_auth = NULL;
> +#endif
> +#if LIBCURL_VERSION_NUM >= 0x070a07
> +static const char *curl_http_proxy_auth = NULL;
> +#endif

I am not a cURL expert, so I'd take your word for these version
dependencies.

We do not initialize static scope pointers to "= NULL" nor variables to 0,
instead we let BSS take care of that for us.  ftp_no_epsv we can see in
the context is doing unnecessary initialization that should be fixed.

>  static struct curl_slist *pragma_header;
>  
> @@ -153,11 +159,67 @@ static int http_options(const char *var, const
> char *value, void *cb)
>  			return git_config_string(&curl_http_proxy, var, value);
>  		return 0;
>  	}
> +#if LIBCURL_VERSION_NUM >= 0x070a06
> +	if (!strcmp("http.auth", var)) {
> +		if (curl_http_auth == NULL)

We tend to say "if (!pointer)".

I see you implemented "the first one wins" rule with this test, but I do
not think you want that.  We first read $HOME/.gitconfig and then
repository specific $GIT_DIR/config, so it is often more useful to use
"the last one wins" rule.

> +			return git_config_string(&curl_http_auth, var, value);
> +		return 0;
> +	}
> +#endif
> +#if LIBCURL_VERSION_NUM >= 0x070a07
> +	if (!strcmp("http.proxy_auth", var)) {
> +		if (curl_http_proxy_auth == NULL)
> +			return git_config_string(&curl_http_proxy_auth, var, value);
> +		return 0;
> +	}
> +#endif
>  
>  	/* Fall back on the default ones */
>  	return git_default_config(var, value, cb);
>  }
>  
> +#if LIBCURL_VERSION_NUM >= 0x070a06
> +static long get_curl_auth_bitmask(const char* auth_method)
> +{
> +	char *buf = xmalloc(strlen(auth_method) + 1);
> +	const unsigned char *p = (const unsigned char *)auth_method;
> +	long mask = CURLAUTH_NONE;

Our isspace() is a sane_isspace(), so you do not have to play casting
games between signed vs unsigned char.

> +	for (;;) {
> +		char *q = buf;
> +		while (*p && isspace(*p))
> +			++p;
> +
> +		while (*p && *p != ',')
> +			*q++ = tolower(*p++);
> +
> +		while (--q >= buf && isspace(*(unsigned char *)q));
> +		++q;
> +
> +		*q = '\0';
> +
> +		if (strcmp(buf, "basic") == 0)

Say !strcmp(buf, "literal") like you did in the configuration parsing part
earlier.

> +			mask |= CURLAUTH_BASIC;
> +		else if (strcmp(buf, "digest") == 0)
> +			mask |= CURLAUTH_DIGEST;
> +		else if (strcmp(buf, "gss") == 0)
> +			mask |= CURLAUTH_GSSNEGOTIATE;
> +		else if (strcmp(buf, "ntlm") == 0)
> +			mask |= CURLAUTH_NTLM;
> +		else if (strcmp(buf, "any") == 0)
> +			mask |= CURLAUTH_ANY;
> +		else if (strcmp(buf, "anysafe") == 0)
> +			mask |= CURLAUTH_ANYSAFE;
> +
> +		if (!*p)
> +			break;
> +		++p;
> +	}

You leak "buf" here you forgot to free.  The string you can possibly
accept is a known set with some maximum length, so you can use a on-stack
buf[] and reject any token longer than that maximum, right?

> +	return mask;
> +}
> +#endif
> +
>  static CURL* get_curl_handle(void)
>  {
>  	CURL* result = curl_easy_init();
> @@ -210,6 +272,20 @@ static CURL* get_curl_handle(void)
>  	if (curl_http_proxy)
>  		curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
>  
> +	if (curl_http_auth) {
> +		long n = get_curl_auth_bitmask(curl_http_auth);
> +		curl_easy_setopt(result, CURLOPT_HTTPAUTH, n);
> +	}
> +
> +	if (curl_http_proxy) {
> +		curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
> +
> +		if (curl_http_proxy_auth) {
> +			long n = get_curl_auth_bitmask(curl_http_proxy_auth);
> +			curl_easy_setopt(result, CURLOPT_PROXYAUTH, n);
> +		}
> +	}
> +

This part does not have to be protected with the LIBCURL_VERSION_NUM
conditional?  I somehow find it unlikely...

Instead of parsing the string every time a curl handle is asked for, how
about parsing them once and store the masks in two file scope static longs
in http_init() and use that value to easy_setopt() call here?

That way you can free the two strings much early without waiting for
http_cleanup(), too, right?

^ permalink raw reply

* [ANNOUNCE] GIT 1.6.1.2
From: Junio C Hamano @ 2009-01-29 10:16 UTC (permalink / raw)
  To: git; +Cc: linux-kernel

The latest maintenance release GIT 1.6.1.2 is available at the
usual places:

  http://www.kernel.org/pub/software/scm/git/

  git-1.6.1.2.tar.{gz,bz2}			(source tarball)
  git-htmldocs-1.6.1.2.tar.{gz,bz2}		(preformatted docs)
  git-manpages-1.6.1.2.tar.{gz,bz2}		(preformatted docs)

The RPM binary packages for a few architectures are also provided
as courtesy.

  RPMS/$arch/git-*-1.6.1.2-1.fc9.$arch.rpm	(RPM)

People with 1.6.1 or 1.6.1.1, who push into a repository that borrows
objects from other repositories via "alternates" mechanism (most of the
linux kernel subsystems hosted on k.org, and "forks" on various public
hosting site such as repo.or.cz and github fall into this category), may
want to upgrade to this version, as these two versions have a buggy "git
push" that does not like such a repository served by git 1.6.1 or newer.


GIT v1.6.1.2 Release Notes
==========================

Fixes since v1.6.1.1
--------------------

* The logic for rename detectin in internal diff used by commands like
  "git diff" and "git blame" have been optimized to avoid loading the same
  blob repeatedly.

* We did not allow writing out a blob that is larger than 2GB for no good
  reason.

* "git format-patch -o $dir", when $dir is a relative directory, used it
  as relative to the root of the work tree, not relative to the current
  directory.

* v1.6.1 introduced an optimization for "git push" into a repository (A)
  that borrows its objects from another repository (B) to avoid sending
  objects that are available in repository B, when they are not yet used
  by repository A.  However the code on the "git push" sender side was
  buggy and did not work when repository B had new objects that are not
  known by the sender.  This caused pushing into a "forked" repository
  served by v1.6.1 software using "git push" from v1.6.1 sometimes did not
  work.  The bug was purely on the "git push" sender side, and has been
  corrected.

* "git status -v" did not paint its diff output in colour even when
  color.ui configuration was set.

* "git ls-tree" learned --full-tree option to help Porcelain scripts that
  want to always see the full path regardless of the current working
  directory.

* "git grep" incorrectly searched in work tree paths even when they are
  marked as assume-unchanged.  It now searches in the index entries.

* "git gc" with no grace period needlessly ejected packed but unreachable
  objects in their loose form, only to delete them right away.

----------------------------------------------------------------

Changes since v1.6.1.1 are as follows:

Björn Steinbrink (1):
      Rename detection: Avoid repeated filespec population

Jeff King (1):
      avoid 31-bit truncation in write_loose_object

Johannes Schindelin (2):
      get_sha1_basic(): fix invalid memory access, found by valgrind
      test-path-utils: Fix off by one, found by valgrind

Junio C Hamano (4):
      ls-tree: add --full-tree option
      Teach format-patch to handle output directory relative to cwd
      send-pack: do not send unknown object name from ".have" to pack-objects
      GIT 1.6.1.2

Marcel M. Cary (1):
      git-sh-setup: Fix scripts whose PWD is a symlink to a work-dir on OS X

Markus Heidelberg (2):
      git-commit: color status output when color.ui is set
      git-status -v: color diff output when color.ui is set

Nanako Shiraishi (1):
      Document git-ls-tree --full-tree

Nguyễn Thái Ngọc Duy (2):
      grep: support --no-ext-grep to test builtin grep
      grep: grep cache entries if they are "assume unchanged"

Nicolas Pitre (1):
      objects to be pruned immediately don't have to be loosened

^ permalink raw reply

* Re: [PATCH] Support various HTTP authentication methods
From: Johannes Sixt @ 2009-01-29 10:18 UTC (permalink / raw)
  To: Moriyoshi Koizumi; +Cc: git
In-Reply-To: <1233221532.21518.1.camel@lena.gsc.riken.jp>

Moriyoshi Koizumi schrieb:
> @@ -210,6 +272,20 @@ static CURL* get_curl_handle(void)
>  	if (curl_http_proxy)
>  		curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);

CURLOPT_PROXY is set here...

>  
> +	if (curl_http_auth) {
> +		long n = get_curl_auth_bitmask(curl_http_auth);
> +		curl_easy_setopt(result, CURLOPT_HTTPAUTH, n);
> +	}
> +
> +	if (curl_http_proxy) {
> +		curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);

... and here again. Is that necessary?

-- Hannes

^ permalink raw reply

* Re: Emacs git-mode feature request: support fill-paragraph correctly
From: Alexandre Julliard @ 2009-01-29 10:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Peter Simons
In-Reply-To: <7vk58fktfy.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> Peter Simons <simons@cryp.to> writes:
>
>> that patch has the desired effect (tested with GNU Emacs 22.3.1 and
>> GIT 1.6.1). Thank you very much.
>>
>> Now, I'd be hugely in favor of applying that change to the repository
>> so that future versions of GIT have it.
>
> Alexandre?

Sure, I'll include it in my next pull request.

-- 
Alexandre Julliard
julliard@winehq.org

^ permalink raw reply

* Re: Something weird is happening...
From: Ingo Molnar @ 2009-01-29 10:50 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: Git Mailing List
In-Reply-To: <49814BA4.6030705@zytor.com>


* H. Peter Anvin <hpa@zytor.com> wrote:

> I was investigating a problem that Ingo Molnar reported on the 
> linux-2.6-tip.git repository on kernel.org.  Unfortunately I was not 
> able to reproduce his problem (which is a problem in itself) but I did 
> run into another oddity:
> 
> : hera 4 ; git fsck
> 
> [lots of dangling commits deleted]
> missing blob af0e01d4c663a101f48614e40d006ed6272d5c36

This problem went away as i downgraded my version of Git from the 'maint' 
branch to a distro 1.6.0 version.

	Ingo

^ permalink raw reply

* Re: Something weird is happening...
From: Ingo Molnar @ 2009-01-29 10:52 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: Git Mailing List
In-Reply-To: <20090129105003.GB10987@elte.hu>


* Ingo Molnar <mingo@elte.hu> wrote:

> 
> * H. Peter Anvin <hpa@zytor.com> wrote:
> 
> > I was investigating a problem that Ingo Molnar reported on the 
> > linux-2.6-tip.git repository on kernel.org.  Unfortunately I was not 
> > able to reproduce his problem (which is a problem in itself) but I did 
> > run into another oddity:
> > 
> > : hera 4 ; git fsck
> > 
> > [lots of dangling commits deleted]
> > missing blob af0e01d4c663a101f48614e40d006ed6272d5c36
> 
> This problem went away as i downgraded my version of Git from the 
> 'maint' branch to a distro 1.6.0 version.

the last 'bad' version of git that i tried was:

    v1.6.1.1-259-g8712b3c

the 'good' version is:

    git version 1.6.0.6

but i had a previous version as well, about 2 weeks older than 
v1.6.1.1-259-g8712b3c - and it only started triggering these problems once 
the kernel.org upgrade happened. I have no clean reproducer so you can 
ignore this - my use of versions was a bit messy.

	Ingo

^ permalink raw reply

* Re: What's cooking in git.git (Jan 2009, #07; Wed, 28)
From: Sverre Rabbelier @ 2009-01-29 11:27 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20090129035138.GC11836@coredump.intra.peff.net>

On Thu, Jan 29, 2009 at 04:51, Jeff King <peff@peff.net> wrote:
>  $ mkdir parent && (cd parent && git init)
>  Initialized empty Git repository in /home/peff/parent/.git/
>
>  $ git clone parent child
>  Initialized empty Git repository in /home/peff/child/.git/
>  warning: You appear to have cloned an empty repository.
>
> So far so good...
>
>  $ (cd parent && echo content >file && git add file && git commit -m one)
>  [normal commit output]
>  $ (cd child && git fetch)
>  [normal fetch output]

I thought instead we wanted to support the following workflow:

$ (cd child && echo content >file && git add file && git commit -m one)
[normal commit output]

Which is what the testcase tests. E.g., we want to support cloning an
empty repo so that the user can then _push_ to that repository to make
it non-empty, no?

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: What's cooking in git.git (Jan 2009, #07; Wed, 28)
From: Jeff King @ 2009-01-29 11:37 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: Junio C Hamano, git
In-Reply-To: <bd6139dc0901290327u572cc30ci9dc719c912fbf875@mail.gmail.com>

On Thu, Jan 29, 2009 at 12:27:23PM +0100, Sverre Rabbelier wrote:

> I thought instead we wanted to support the following workflow:
> 
> $ (cd child && echo content >file && git add file && git commit -m one)
> [normal commit output]
> 
> Which is what the testcase tests. E.g., we want to support cloning an
> empty repo so that the user can then _push_ to that repository to make
> it non-empty, no?

True, that is probably going to be more common (otherwise, why would the
person who is going to push into the empty repo advertise it to you
before they have put any content in it).

But it will probably still be surprising not to have the branch merging
setup:

  mkdir parent && (cd parent && git init) &&
  git clone parent child && cd child &&
  echo content >file && git add file && git commit -m one &&
  git push origin master ;# note we have to explicitly mention the branch

  ... time passes ...

  git pull

produces the "you haven't asked me which branch to merge" message.

Which does make some sense, given how tracking configuration is set up.
It's just that it's a little sad that cloning an empty repository and
then later getting commits out of it (whether commits you put in or
somebody else) does not behave the same as cloning a repository with
commits.

Which I thought was sort of the point, that this would work seamlessly.
Otherwise, there is not much advantage over:

  mkdir parent && (cd parent && git init) &&
  mkdir child && cd child && git init &&
  echo content >file && git add file && git commit -m one &&
  git push origin master ;# note we have to explicitly mention the branch

With the empty clone, you get your "origin" remote set up, but in both
cases you are missing the branch tracking.

I don't know if there is a good solution, though. Perhaps it's best to
just let what's there get released and see if people complain.

-Peff

^ permalink raw reply

* Re: What's cooking in git.git (Jan 2009, #07; Wed, 28)
From: Pieter de Bie @ 2009-01-29 11:40 UTC (permalink / raw)
  To: Jeff King; +Cc: Sverre Rabbelier, Junio C Hamano, git
In-Reply-To: <20090129113735.GA6505@coredump.intra.peff.net>


On 29 jan 2009, at 11:37, Jeff King wrote:

>  git push origin master ;# note we have to explicitly mention the  
> branch
>
>  ... time passes ...
>
>  git pull
>
> produces the "you haven't asked me which branch to merge" message.
>
> Which does make some sense, given how tracking configuration is set  
> up.
> It's just that it's a little sad that cloning an empty repository and
> then later getting commits out of it (whether commits you put in or
> somebody else) does not behave the same as cloning a repository with
> commits.

This is true in all cases. If you create a new branch in any repository,
push that, and later do a 'git pull', you get that message. I agree it's
not the nicest way to handle things, but this is not an issue with the  
clone,
it's an issue of pushing new branches in general.

^ permalink raw reply

* Re: What's cooking in git.git (Jan 2009, #07; Wed, 28)
From: Sverre Rabbelier @ 2009-01-29 11:45 UTC (permalink / raw)
  To: Pieter de Bie; +Cc: Jeff King, Junio C Hamano, git
In-Reply-To: <351A6988-32EB-473F-B6E5-8FBB38D91F88@ai.rug.nl>

On Thu, Jan 29, 2009 at 12:40, Pieter de Bie <pdebie@ai.rug.nl> wrote:
> This is true in all cases. If you create a new branch in any repository,
> push that, and later do a 'git pull', you get that message. I agree it's
> not the nicest way to handle things, but this is not an issue with the
> clone, it's an issue of pushing new branches in general.

Mhhh, so maybe we want a way to set up tracking branches when pushing,
yes? From what I've seen a patch to do that shouldn't be too hard, so
if there's interest in that I could look into that.

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: What's cooking in git.git (Jan 2009, #07; Wed, 28)
From: Jeff King @ 2009-01-29 11:48 UTC (permalink / raw)
  To: Pieter de Bie; +Cc: Sverre Rabbelier, Junio C Hamano, git
In-Reply-To: <351A6988-32EB-473F-B6E5-8FBB38D91F88@ai.rug.nl>

On Thu, Jan 29, 2009 at 11:40:42AM +0000, Pieter de Bie wrote:

> This is true in all cases. If you create a new branch in any
> repository, push that, and later do a 'git pull', you get that
> message. I agree it's not the nicest way to handle things, but this is
> not an issue with the  clone, it's an issue of pushing new branches in
> general.

Right. I guess I was hoping by cloning an existing repository, even one
with no commits on the branch, that we could somehow remember that we
are "on" the master branch. I think that is what people who ask for
empty cloning really want:

  1. make a bare upstream

  2. clone empty repo

  3. create commits

  4. git push / git pull, as if we had cloned non-empty repo

And I know that it is not very "git" to talk about empty branches, since
branches are pointers into the DAG. But we already do similar trickery
with "yet to be born" branches by putting a dangling symref into HEAD.
But I don't think there's any way currently to send those dangling
symrefs across the git protocol, which is what would be required to do
the above accurately.

-Peff

^ 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