Git development
 help / color / mirror / Atom feed
* Re: [PATCH 6/5] Fix parsing of @{-1}@{1}
From: Junio C Hamano @ 2009-01-17 20:02 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Thomas Rast, git, Johannes Sixt, Johan Herland
In-Reply-To: <alpine.DEB.1.00.0901171907530.3586@pacific.mpi-cbg.de>

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

> To do that, Git no longer looks forward for the '@{' corresponding to the
> closing '}' but backward, and dwim_ref() as well as dwim_log() learnt
> about the @{-<N>} notation.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>
> 	The modifications of dwim_ref() and dwim_log() are probably
> 	more important than the issue I tried to fix...

Good, so we can say things like:

	git log -g @{-1}
        git show-branch -g @{-1}

with this?

By the way, I noticed that without these patch series we show something
when "git rev-parse --verify @{-1}" is asked for.  What is it trying to
show?

^ permalink raw reply

* [PATCH 1/2] http-push: send opaquelocktoken in If: for PUT requests
From: Ray Chuan @ 2009-01-17 20:10 UTC (permalink / raw)
  To: git

Currently, git PUTs to

  /repo.git/objects/1a/1a2b...9z_opaquelocktoken:1234-....

On some platforms, ':' isn't allowed in filenames so the PUT fails.
This seems to be an (faulty) implementation of the opaquelocktoken URI
scheme (http://www.webdav.org/specs/rfc4918.html#RFC2518).

PUT now sends the object to the url

  /repo.git/objects/1a/1a2b...9z

, without the trailing '_opaquelocktoken:1234-....', with an
additional "If: (<opaquelocktoken:1234-....>)" header.

Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
 http-push.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/http-push.c b/http-push.c
index c9ba07e..4517cf2 100644
--- a/http-push.c
+++ b/http-push.c
@@ -480,7 +480,9 @@ static void start_put(struct transfer_request *request)
 {
 	char *hex = sha1_to_hex(request->obj->sha1);
 	struct active_request_slot *slot;
+	struct curl_slist *dav_headers = NULL;
 	char *posn;
+	char *if_header;
 	enum object_type type;
 	char hdr[50];
 	void *unpacked;
@@ -535,6 +537,10 @@ static void start_put(struct transfer_request *request)
 	*(posn++) = '_';
 	strcpy(posn, request->lock->token);

+	if_header = xmalloc(strlen(request->lock->token) + 25);
+	sprintf(if_header, "If: <%s>", request->lock->token);
+	dav_headers = curl_slist_append(dav_headers, if_header);
+
 	slot = get_active_slot();
 	slot->callback_func = process_response;
 	slot->callback_data = request;
@@ -546,6 +552,7 @@ static void start_put(struct transfer_request *request)
 	curl_easy_setopt(slot->curl, CURLOPT_UPLOAD, 1);
 	curl_easy_setopt(slot->curl, CURLOPT_PUT, 1);
 	curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);
+	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, dav_headers);
 	curl_easy_setopt(slot->curl, CURLOPT_URL, request->url);

 	if (start_active_slot(slot)) {
-- 
1.5.6.3

^ permalink raw reply related

* Re: [PATCH 1/2] http-push: send opaquelocktoken in If: for PUT requests
From: Ray Chuan @ 2009-01-17 20:14 UTC (permalink / raw)
  To: git
In-Reply-To: <be6fef0d0901171210o97a24carf902e82848d48b5d@mail.gmail.com>

oops, i incorrectly stated in the commit message that the request URL
for PUT was modified; i apologize for that error.

On 1/17/09, Ray Chuan <rctay89@gmail.com> wrote:
> Currently, git PUTs to
>
>   /repo.git/objects/1a/1a2b...9z_opaquelocktoken:1234-....
>
> On some platforms, ':' isn't allowed in filenames so the PUT fails.
> This seems to be an (faulty) implementation of the opaquelocktoken URI
> scheme (http://www.webdav.org/specs/rfc4918.html#RFC2518).
>
> PUT now sends the object to the url
>
>   /repo.git/objects/1a/1a2b...9z
>
> , without the trailing '_opaquelocktoken:1234-....', with an
> additional "If: (<opaquelocktoken:1234-....>)" header.
>
> Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
> ---
>  http-push.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/http-push.c b/http-push.c
> index c9ba07e..4517cf2 100644
> --- a/http-push.c
> +++ b/http-push.c
> @@ -480,7 +480,9 @@ static void start_put(struct transfer_request *request)
>  {
>  	char *hex = sha1_to_hex(request->obj->sha1);
>  	struct active_request_slot *slot;
> +	struct curl_slist *dav_headers = NULL;
>  	char *posn;
> +	char *if_header;
>  	enum object_type type;
>  	char hdr[50];
>  	void *unpacked;
> @@ -535,6 +537,10 @@ static void start_put(struct transfer_request *request)
>  	*(posn++) = '_';
>  	strcpy(posn, request->lock->token);
>
> +	if_header = xmalloc(strlen(request->lock->token) + 25);
> +	sprintf(if_header, "If: <%s>", request->lock->token);
> +	dav_headers = curl_slist_append(dav_headers, if_header);
> +
>  	slot = get_active_slot();
>  	slot->callback_func = process_response;
>  	slot->callback_data = request;
> @@ -546,6 +552,7 @@ static void start_put(struct transfer_request *request)
>  	curl_easy_setopt(slot->curl, CURLOPT_UPLOAD, 1);
>  	curl_easy_setopt(slot->curl, CURLOPT_PUT, 1);
>  	curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);
> +	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, dav_headers);
>  	curl_easy_setopt(slot->curl, CURLOPT_URL, request->url);
>
>  	if (start_active_slot(slot)) {
> --
> 1.5.6.3
>


-- 
Cheers,
Ray Chuan

^ permalink raw reply

* [PATCH 2/2] http-push: remove MOVE step after PUT when sending objects to server
From: Ray Chuan @ 2009-01-17 20:24 UTC (permalink / raw)
  To: git

Currently, git PUTs to

 /repo.git/objects/1a/1a2b...9z_opaquelocktoken:1234-....

then MOVEs to

 /repo.git/objects/1a/1a2b...9z

This is needless. In fact, the only time MOVE requests are sent is for
this sole purpose (ie. of renaming an object).

A concern raised was repository corruption in the event of failure
during PUT. "put && move" won't afford any more protection than using
simply "put", since info/refs is not updated if a PUT fails, so there
is no cause for concern.

Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
 http-push.c |   45 +--------------------------------------------
 1 files changed, 1 insertions(+), 44 deletions(-)

diff --git a/http-push.c b/http-push.c
index 4517cf2..d21fd3b 100644
--- a/http-push.c
+++ b/http-push.c
@@ -31,7 +31,6 @@ enum XML_Status {
 /* DAV methods */
 #define DAV_LOCK "LOCK"
 #define DAV_MKCOL "MKCOL"
-#define DAV_MOVE "MOVE"
 #define DAV_PROPFIND "PROPFIND"
 #define DAV_PUT "PUT"
 #define DAV_UNLOCK "UNLOCK"
@@ -105,7 +104,6 @@ enum transfer_state {
 	NEED_PUSH,
 	RUN_MKCOL,
 	RUN_PUT,
-	RUN_MOVE,
 	ABORTED,
 	COMPLETE,
 };
@@ -531,11 +529,6 @@ static void start_put(struct transfer_request *request)
 	posn += 2;
 	*(posn++) = '/';
 	strcpy(posn, hex + 2);
-	request->dest = xmalloc(strlen(request->url) + 14);
-	sprintf(request->dest, "Destination: %s", request->url);
-	posn += 38;
-	*(posn++) = '_';
-	strcpy(posn, request->lock->token);

 	if_header = xmalloc(strlen(request->lock->token) + 25);
 	sprintf(if_header, "If: <%s>", request->lock->token);
@@ -565,32 +558,6 @@ static void start_put(struct transfer_request *request)
 	}
 }

-static void start_move(struct transfer_request *request)
-{
-	struct active_request_slot *slot;
-	struct curl_slist *dav_headers = NULL;
-
-	slot = get_active_slot();
-	slot->callback_func = process_response;
-	slot->callback_data = request;
-	curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1); /* undo PUT setup */
-	curl_easy_setopt(slot->curl, CURLOPT_CUSTOMREQUEST, DAV_MOVE);
-	dav_headers = curl_slist_append(dav_headers, request->dest);
-	dav_headers = curl_slist_append(dav_headers, "Overwrite: T");
-	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, dav_headers);
-	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_null);
-	curl_easy_setopt(slot->curl, CURLOPT_URL, request->url);
-
-	if (start_active_slot(slot)) {
-		request->slot = slot;
-		request->state = RUN_MOVE;
-	} else {
-		request->state = ABORTED;
-		free(request->url);
-		request->url = NULL;
-	}
-}
-
 static int refresh_lock(struct remote_lock *lock)
 {
 	struct active_request_slot *slot;
@@ -713,23 +680,13 @@ static void finish_request(struct
transfer_request *request)
 		}
 	} else if (request->state == RUN_PUT) {
 		if (request->curl_result == CURLE_OK) {
-			start_move(request);
-		} else {
-			fprintf(stderr,	"PUT %s failed, aborting (%d/%ld)\n",
-				sha1_to_hex(request->obj->sha1),
-				request->curl_result, request->http_code);
-			request->state = ABORTED;
-			aborted = 1;
-		}
-	} else if (request->state == RUN_MOVE) {
-		if (request->curl_result == CURLE_OK) {
 			if (push_verbosely)
 				fprintf(stderr, "    sent %s\n",
 					sha1_to_hex(request->obj->sha1));
 			request->obj->flags |= REMOTE;
 			release_request(request);
 		} else {
-			fprintf(stderr, "MOVE %s failed, aborting (%d/%ld)\n",
+			fprintf(stderr,	"PUT %s failed, aborting (%d/%ld)\n",
 				sha1_to_hex(request->obj->sha1),
 				request->curl_result, request->http_code);
 			request->state = ABORTED;
-- 
1.5.6.3

^ permalink raw reply related

* Re: [PATCH 3/3] http-push: update tests
From: Jakub Narebski @ 2009-01-17 20:21 UTC (permalink / raw)
  To: Ray Chuan; +Cc: Junio C Hamano, Johannes Schindelin, git
In-Reply-To: <be6fef0d0901171155p26e14aa1t90c0d7b8ec7925f3@mail.gmail.com>

"Ray Chuan" <rctay89@gmail.com> writes:

>>>>> -     git push &&
>>>>> -     [ -f "$HTTPD_DOCUMENT_ROOT_PATH/test_repo.git/refs/heads/master" ]
> 
> i modified the push arguments as there was no remote ref/branch
> specified. With a fixed "git push", that line says:
> 
>   No refs in common and none specified; doing nothing.
> 
> i'd like to take this chance to inquire, what does the -f, plus square
> brackets, really mean? i assumed it was to force push to go ahead even
> if "a remote ref that is not an ancestor of the local ref used to
> overwrite it" check fails.

Errr...

  git push &&
  [ -f "$HTTPD_DOCUMENT_ROOT_PATH/test_repo.git/refs/heads/master" ]

means, do "git push", and if it succeeds test (which should really be
written as 'test <cond>' and not '[ <cond> ]' I think) if the file
does exists and is regular file ('help test' or 'man test').

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [PATCH 3/3] http-push: update tests
From: Ray Chuan @ 2009-01-17 21:00 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Junio C Hamano, Johannes Schindelin, git
In-Reply-To: <m3zlhpy981.fsf@localhost.localdomain>

change tests to expect success.

Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
 t/t5540-http-push.sh |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t5540-http-push.sh b/t/t5540-http-push.sh
index da95886..4acc087 100755
--- a/t/t5540-http-push.sh
+++ b/t/t5540-http-push.sh
@@ -51,17 +51,17 @@ test_expect_success 'clone remote repository' '
 	git clone $HTTPD_URL/test_repo.git test_repo_clone
 '

-test_expect_failure 'push to remote repository' '
+test_expect_success 'push to remote repository' '
 	cd "$ROOT_PATH"/test_repo_clone &&
 	: >path2 &&
 	git add path2 &&
 	test_tick &&
 	git commit -m path2 &&
-	git push &&
+	git push origin master &&
 	[ -f "$HTTPD_DOCUMENT_ROOT_PATH/test_repo.git/refs/heads/master" ]
 '

-test_expect_failure 'create and delete remote branch' '
+test_expect_success 'create and delete remote branch' '
 	cd "$ROOT_PATH"/test_repo_clone &&
 	git checkout -b dev &&
 	: >path3 &&
-- 
1.5.6.3

^ permalink raw reply related

* Re: [PATCH 3/3] http-push: update tests
From: Jakub Narebski @ 2009-01-17 21:15 UTC (permalink / raw)
  To: Ray Chuan; +Cc: Junio C Hamano, Johannes Schindelin, git
In-Reply-To: <be6fef0d0901171300t44b60aedm801f6f18d88b654b@mail.gmail.com>

Ray Chuan wrote:

> Subject: [PATCH 3/3] http-push: update tests
>
> change tests to expect success.

Minor nit: I would use here

Subject: [PATCH 3/3] http-push: change tests to expect success

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH 6/5] Fix parsing of @{-1}@{1}
From: Johannes Schindelin @ 2009-01-17 21:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Thomas Rast, git, Johannes Sixt, Johan Herland
In-Reply-To: <7vsknh66qu.fsf@gitster.siamese.dyndns.org>

Hi,

On Sat, 17 Jan 2009, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > To do that, Git no longer looks forward for the '@{' corresponding to the
> > closing '}' but backward, and dwim_ref() as well as dwim_log() learnt
> > about the @{-<N>} notation.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >
> > 	The modifications of dwim_ref() and dwim_log() are probably
> > 	more important than the issue I tried to fix...
> 
> Good, so we can say things like:
> 
> 	git log -g @{-1}
>         git show-branch -g @{-1}
> 
> with this?

I _hope_ :-)

> By the way, I noticed that without these patch series we show something 
> when "git rev-parse --verify @{-1}" is asked for.  What is it trying to 
> show?

Apparently the same as @{1.Jan}: in get_sha1_basic(), we have this code:

                /* Is it asking for N-th entry, or approxidate? */
                for (i = nth = 0; 0 <= nth && i < reflog_len; i++) {
                        char ch = str[at+2+i];
                        if ('0' <= ch && ch <= '9')
                                nth = nth * 10 + ch - '0';
                        else
                                nth = -1;
                }
                if (100000000 <= nth) {
                        at_time = nth;
                        nth = -1;
                } else if (0 <= nth)
                        at_time = 0;
                else {
                        char *tmp = xstrndup(str + at + 2, reflog_len);
                        at_time = approxidate(tmp);
                        free(tmp);
                }

So in the loop, nth is set to -1 because of a non-digit, and later, 
approxidate is called for nth == -1, which does this:

	$ ./test-date now
	now -> bad -> Thu Jan  1 01:00:00 1970
	now -> Sat Jan 17 22:21:20 2009

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 3/3] http-push: update tests
From: Johannes Schindelin @ 2009-01-17 21:24 UTC (permalink / raw)
  To: Ray Chuan; +Cc: Jakub Narebski, Junio C Hamano, git
In-Reply-To: <be6fef0d0901171300t44b60aedm801f6f18d88b654b@mail.gmail.com>

Hi,

On Sat, 17 Jan 2009, Ray Chuan wrote:

> -test_expect_failure 'push to remote repository' '
> +test_expect_success 'push to remote repository' '
>  	cd "$ROOT_PATH"/test_repo_clone &&
>  	: >path2 &&
>  	git add path2 &&
>  	test_tick &&
>  	git commit -m path2 &&
> -	git push &&
> +	git push origin master &&
>  	[ -f "$HTTPD_DOCUMENT_ROOT_PATH/test_repo.git/refs/heads/master" ]
>  '

But it _should_ succeed without <remote> <branch>.  It should find that 
there is a master locally and one remotely, and push that.

Ciao,
Dscho

^ permalink raw reply

* [PATCH] mergetool: put the cursor on the editable file for Vim
From: Markus Heidelberg @ 2009-01-17 21:28 UTC (permalink / raw)
  To: gitster; +Cc: git

When resolving conflicts, you only need to edit the $MERGED file. Put
the cursor automatically into its window for vimdiff and gvimdiff to
avoid doing <C-w>l every time.

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---
 git-mergetool.sh |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index b2d5375..00e1337 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -200,14 +200,19 @@ merge_file () {
 	    fi
 	    status=$?
 	    ;;
-	meld|vimdiff)
+	meld)
 	    touch "$BACKUP"
 	    "$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
 	    check_unchanged
 	    ;;
+	vimdiff)
+	    touch "$BACKUP"
+	    "$merge_tool_path" -c "wincmd l" "$LOCAL" "$MERGED" "$REMOTE"
+	    check_unchanged
+	    ;;
 	gvimdiff)
 	    touch "$BACKUP"
-	    "$merge_tool_path" -f "$LOCAL" "$MERGED" "$REMOTE"
+	    "$merge_tool_path" -c "wincmd l" -f "$LOCAL" "$MERGED" "$REMOTE"
 	    check_unchanged
 	    ;;
 	xxdiff)
-- 
1.6.1.154.g97e2f

^ permalink raw reply related

* Re: log-tree.c: date hardwired
From: jidanni @ 2009-01-17 23:27 UTC (permalink / raw)
  To: git
In-Reply-To: <87r637oq41.fsf@jidanni.org>

Can I get a go-ahead from somebody for me to begin work on implementing a fix?
(Yes, trivial.)

>>>>> "j" == jidanni  <jidanni@jidanni.org> writes:

j> In log-tree.c: printf("From %s Mon Sep 17 00:00:00 2001\n", name);
j> Wouldn't it be more aesthetically pleasing to use current local or UTC time?
j> Or at least comment in the code that the date is hardwired like that
j> in the fear that otherwise people will think it is the actual commit time.
j> No, I can't think of any other tool that hardwires the From separators
j> they produce.

^ permalink raw reply

* Re: log-tree.c: date hardwired
From: Alex Riesen @ 2009-01-17 23:38 UTC (permalink / raw)
  To: jidanni; +Cc: git
In-Reply-To: <87fxjhcy3i.fsf@jidanni.org>

2009/1/18  <jidanni@jidanni.org>:
> Can I get a go-ahead from somebody for me to begin work on implementing a fix?

No. Try and guess why

^ permalink raw reply

* Re: [PATCH] do not drop commits before the merge base
From: Johannes Schindelin @ 2009-01-17 23:41 UTC (permalink / raw)
  To: Stephen Haberman; +Cc: git
In-Reply-To: <cover.1232233454.git.stephen@exigencecorp.com>

Hi,

On Sat, 17 Jan 2009, Stephen Haberman wrote:

> [... no patch, despite the subject...]

You probably wanted to use -n --cover-letter...

Ciao,
Dscho

^ permalink raw reply

* Re: log-tree.c: date hardwired
From: jidanni @ 2009-01-17 23:50 UTC (permalink / raw)
  To: raa.lkml; +Cc: git
In-Reply-To: <81b0412b0901171538l5e9fb66bh862c9b7a125fc98f@mail.gmail.com>

Perhaps a comment could be added to the code as to the reason a date
or that certain date is hardwired. Any reason would be satisfactory.

^ permalink raw reply

* Re: [PATCH] rebase -p: seed first commit in case it's before the merge bases.
From: Johannes Schindelin @ 2009-01-17 23:51 UTC (permalink / raw)
  To: Stephen Haberman; +Cc: git
In-Reply-To: <a524993b13ee586cf0e8fbd3b6459ccd6767c6d8.1232233454.git.stephen@exigencecorp.com>

Hi,

On Sat, 17 Jan 2009, Stephen Haberman wrote:

> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index c8b0861..e800e07 100755
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -604,11 +604,18 @@ first and then run 'git rebase --continue' again."
>  				echo $ONTO > "$REWRITTEN"/$c ||
>  					die "Could not init rewritten commits"
>  			done
> +			# Along with the merge bases, look at the first commit's
> +			# parent (which may be before the merge base) and mark it
> +			# as rewritten to ONTO
> +			FIRST="$(git rev-list --reverse --first-parent $UPSTREAM..$HEAD | head -n 1)"
> +			for p in $(git rev-list --parents -1 $FIRST | cut -d' ' -f2)
> +			do
> +				echo $ONTO > "$REWRITTEN/$p"
> +			done

AFAICT this is wrong.  You have no guarantee whatsoever that the output of

	$ git rev-list --reverse --first-parent $UPSTREAM..$HEAD | head -n 1

has any parents at all.  Take for example a coolest-merge-ever, i.e. a 
merge of an independent project.

Instead, what you _actually_ are looking for are the boundary objects of 
$UPSTREAM..$HEAD, which would be easy to get at.

However, I have a strong feeling that just piling onto the current code 
will not fix the underlying issues.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] rebase -p: seed first commit in case it's before the merge bases.
From: Stephen Haberman @ 2009-01-18  0:11 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0901180041540.3586@pacific.mpi-cbg.de>


> > +			# Along with the merge bases, look at the first commit's
> > +			# parent (which may be before the merge base) and mark it
> > +			# as rewritten to ONTO
> > +			FIRST="$(git rev-list --reverse --first-parent $UPSTREAM..$HEAD | head -n 1)"
> > +			for p in $(git rev-list --parents -1 $FIRST | cut -d' ' -f2)
> > +			do
> > +				echo $ONTO > "$REWRITTEN/$p"
> > +			done
> 
> AFAICT this is wrong.  You have no guarantee whatsoever that the output of
> 
> 	$ git rev-list --reverse --first-parent $UPSTREAM..$HEAD | head -n 1
> 
> has any parents at all.  Take for example a coolest-merge-ever, i.e. a 
> merge of an independent project.
> 
> Instead, what you _actually_ are looking for are the boundary objects
> of $UPSTREAM..$HEAD,

Agreed.

> which would be easy to get at.

That would be great, but I'm not seeing it, obviously. Suggestions
would be appreciated.

> However, I have a strong feeling that just piling onto the current
> code will not fix the underlying issues.

Also agreed.

So...not that it really matters, but did my patches go out to the git
list or not? It looks like both Johannes and I got them from the cc
entries.

I tried to use format-patch and the files looked great, cc's including
Michael, Stephan, and Sitaram. Then I ran send-email with the three
files as arguments and it stripped all the cc's but Johannes and
myself. Then I got all three delivered due to my cc entry, but I didn't
see any entries arrive from the list even though the cc-delivered
copies all had "To: git@vger.kernel.org" in them (and that is what I
had pasted into the send-email prompt). I guess I did something wrong
but it's frustrating to not know what it was.

- Stephen

^ permalink raw reply

* Re: [PATCH] rebase -p: seed first commit in case it's before the merge bases.
From: Johannes Schindelin @ 2009-01-18  0:19 UTC (permalink / raw)
  To: Stephen Haberman; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0901180041540.3586@pacific.mpi-cbg.de>

Hi,

On Sun, 18 Jan 2009, Johannes Schindelin wrote:

> However, I have a strong feeling that just piling onto the current code 
> will not fix the underlying issues.

BTW just to clarify what I mean by "underlying issues": if you say "git 
rebase -i" in Sitaram's test case, you will see the two commits -- as 
expected.

However, if you add "-p", all of a sudden you will only see "noop".  IMO 
there is no excuse that the code can hide them at all.  If the commits are 
reachable from HEAD but not from $UPSTREAM, they have to be in the list.  
As simple as that.

Another thing that I find horribly wrong: there is a "touch 
$REWRITTEN/sha1".  There was a simple design in the beginning: the files 
in $REWRITTEN are actually a mapping from old SHA-1 (file name) to new 
SHA-1 (content).  This was broken, without any good explanation.

Ciao,
Dscho

^ permalink raw reply

* Re: log-tree.c: date hardwired
From: Sitaram Chamarty @ 2009-01-18  0:40 UTC (permalink / raw)
  To: git
In-Reply-To: <81b0412b0901171538l5e9fb66bh862c9b7a125fc98f@mail.gmail.com>

On 2009-01-17, Alex Riesen <raa.lkml@gmail.com> wrote:
> 2009/1/18  <jidanni@jidanni.org>:
>> Can I get a go-ahead from somebody for me to begin work
>> on implementing a fix?
>
> No. Try and guess why

OK, I'll bite...

Because it's only used as the date for the mbox format
leader line, and not an actual email (RFC 822/2822) header
line?  And in that format it represents "delivery date",
which is irrelevant for stuff you're generating to be sent
out anyway, meaning this line doesn't actually get sent out
to anyone when you mail your patches, so we don't care what
it is.

I can't think of anything else, so if it isn't that please
do tell.  Not that I cared about it before, but once someone
says "try and guess why" it somehow becomes important :-)

^ permalink raw reply

* Re: [PATCH] interpret_nth_last_branch(): avoid traversing the reflogs twice
From: Junio C Hamano @ 2009-01-18  0:43 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Thomas Rast, git, Johannes Sixt, Johan Herland
In-Reply-To: <alpine.DEB.1.00.0901172028470.3586@pacific.mpi-cbg.de>

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

> That is correct.  But this is such a highly uncritical code path that I'd 
> like to keep this simple rather than fast.

I actually not worried about "fast" at all; it was more about unbounded
memory consumption.

^ permalink raw reply

* Re: [PATCH 2/2] http-push: remove MOVE step after PUT when sending objects to server
From: Junio C Hamano @ 2009-01-18  0:48 UTC (permalink / raw)
  To: Ray Chuan; +Cc: git
In-Reply-To: <be6fef0d0901171224y35c3d95cn2d38639ac03c3b8f@mail.gmail.com>

"Ray Chuan" <rctay89@gmail.com> writes:

> Currently, git PUTs to
>
>  /repo.git/objects/1a/1a2b...9z_opaquelocktoken:1234-....
>
> then MOVEs to
>
>  /repo.git/objects/1a/1a2b...9z
>
> This is needless. In fact, the only time MOVE requests are sent is for
> this sole purpose (ie. of renaming an object).
>
> A concern raised was repository corruption in the event of failure
> during PUT. "put && move" won't afford any more protection than using
> simply "put", since info/refs is not updated if a PUT fails, so there
> is no cause for concern.

That's a completely bogus reasoning.  Normal operation inside the
repository that was pushed into won't look at info/refs at all.

The true reason you want to avoid put-then-move is...?

^ permalink raw reply

* Re: log-tree.c: date hardwired
From: Junio C Hamano @ 2009-01-18  0:52 UTC (permalink / raw)
  To: jidanni; +Cc: git
In-Reply-To: <87fxjhcy3i.fsf@jidanni.org>

jidanni@jidanni.org writes:

> Can I get a go-ahead from somebody for me to begin work on implementing a fix?

No, don't "fix" it.

Please keep it as a known constant.  That way, you can come up with an
/etc/magic entry to teach find(1) that it is not just a random mailbox but
is an output from format-patch.

^ permalink raw reply

* Re: [PATCH/RFC v4 2/5] sha1_name: tweak @{-N} lookup
From: Junio C Hamano @ 2009-01-18  0:54 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, Johannes Schindelin, Johannes Sixt, Johan Herland
In-Reply-To: <1232208597-29249-3-git-send-email-trast@student.ethz.ch>

Thomas Rast <trast@student.ethz.ch> writes:

> Have the lookup only look at "interesting" checkouts, meaning those
> that tell you "Already on ..." don't count even though they also cause
> a reflog entry.
>
> Let interpret_nth_last_branch() return the number of characters
> parsed, so that git-checkout can verify that the branch spec was
> @{-N}, not @{-1}^2 or something like that.  (The latter will be added
> later.)

Thanks; you seem to have handled the issues I pointed out in response to
my own weatherbaloon patch.  I think it is probably better to squash the
first two (and you take the authorship).

> diff --git a/sha1_name.c b/sha1_name.c
> index 6377264..34e39db 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -685,29 +685,28 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
>  				  const char *message, void *cb_data)
>  {
>  	struct grab_nth_branch_switch_cbdata *cb = cb_data;
> -	const char *match = NULL;
> -
> -	if (!prefixcmp(message, "checkout: moving to "))
> -		match = message + strlen("checkout: moving to ");
> -	else if (!prefixcmp(message, "checkout: moving from ")) {
> -		const char *cp = message + strlen("checkout: moving from ");
> -		if ((cp = strstr(cp, " to ")) != NULL) {
> -			match = cp + 4;
> -		}
> +	const char *match = NULL, *target = NULL;
> +	size_t len;
> +
> +	if (!prefixcmp(message, "checkout: moving from ")) {
> +		match = message + strlen("checkout: moving from ");
> +		if ((target = strstr(match, " to ")) != NULL)
> +			target += 4;
>  	}

This drops support for older reflog records, but I think it would be Ok.
This "N-th" support is really meant to be for small number of N anyway.

> -	if (--cb->nth <= 0) {
> -		size_t len = strlen(match);
> -		while (match[len-1] == '\n')
> -			len--;
> +	if (cb->nth-- <= 0) {
>  		strbuf_reset(cb->buf);
>  		strbuf_add(cb->buf, match, len);
>  		return 1;

Hmm, did I have an off-by-one I did not notice? ;-)

>  int interpret_nth_last_branch(const char *name, struct strbuf *buf)
>  {
> -	int nth, i;
> +	int nth;
>  	struct grab_nth_branch_switch_cbdata cb;
> +	const char *brace;
> +	char *num_end;
>  
>  	if (name[0] != '@' || name[1] != '{' || name[2] != '-')
>  		return -1;
> -	for (i = 3, nth = 0; name[i] && name[i] != '}'; i++) {
> -		char ch = name[i];
> -		if ('0' <= ch && ch <= '9')
> -			nth = nth * 10 + ch - '0';
> -		else
> -			return -1;
> -	}
> -	if (nth < 0 || 10 <= nth)

The removal of "limit to reasonably small recent N" I somewhat have
reservations on, but I think we can later re-add something based on
configuration variable if we need to.

^ permalink raw reply

* [PATCH] bash: offer to show (un)staged changes
From: Thomas Rast @ 2009-01-18  0:56 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Shawn O. Pearce

Add a bit of code to __git_ps1 that lets it append '*' to the branch
name if there are any uncommitted changes, and '+' if there are any
staged changes.

Since this is a rather expensive operation and will force a lot of
data into the cache whenever you first enter a repository, you have to
enable it manually by setting GIT_PS1_EXPENSIVE to something nonempty.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

I came up with this after sending two incomplete patches on the same
night, and really like it.  Perhaps others might find it useful.

Of course it would be brilliant if there were a way to ask the kernel
if a certain directory is cached, but I couldn't find one, let alone
one accessible from the shell.


 contrib/completion/git-completion.bash |   21 +++++++++++++++++++--
 1 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index f8b845a..36ea528 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -34,6 +34,10 @@
 #       are currently in a git repository.  The %s token will be
 #       the name of the current branch.
 #
+#	In addition, if you set GIT_PS1_EXPENSIVE to a nonempty value,
+#	unstaged (*) and staged (+) changes will be shown next to the
+#	branch name.
+#
 # To submit patches:
 #
 #    *) Read Documentation/SubmittingPatches
@@ -116,10 +120,23 @@ __git_ps1 ()
 			fi
 		fi
 
+		local w
+		local i
+
+		if test ! -z "$GIT_PS1_EXPENSIVE"; then
+			git update-index --refresh >/dev/null 2>&1 || w="*"
+			if git rev-parse --quiet --verify HEAD >/dev/null; then
+				git diff-index --cached --quiet \
+					--ignore-submodules HEAD -- || i="+"
+			else
+				i="#"
+			fi
+		fi
+
 		if [ -n "${1-}" ]; then
-			printf "$1" "${b##refs/heads/}$r"
+			printf "$1" "${b##refs/heads/}$w$i$r"
 		else
-			printf " (%s)" "${b##refs/heads/}$r"
+			printf " (%s)" "${b##refs/heads/}$w$i$r"
 		fi
 	fi
 }
-- 
tg: (7bbd8d6..) t/ps1-dirty-state (depends on: origin/master)

^ permalink raw reply related

* [PATCH next resend] bash completion: refactor diff options
From: Thomas Rast @ 2009-01-18  1:03 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Shawn O. Pearce
In-Reply-To: <1231679663-31907-1-git-send-email-trast@student.ethz.ch>

diff, log and show all take the same diff options.  Refactor them from
__git_diff and __git_log into a variable, and complete them in
__git_show too.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>

---

Any news on this?


 contrib/completion/git-completion.bash |   38 ++++++++++++++++++-------------
 1 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 70a6b44..40e3a14 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -774,25 +774,30 @@ _git_describe ()
 	__gitcomp "$(__git_refs)"
 }
 
-_git_diff ()
-{
-	__git_has_doubledash && return
-
-	local cur="${COMP_WORDS[COMP_CWORD]}"
-	case "$cur" in
-	--*)
-		__gitcomp "--cached --stat --numstat --shortstat --summary
+__git_diff_common_options="--stat --numstat --shortstat --summary
 			--patch-with-stat --name-only --name-status --color
 			--no-color --color-words --no-renames --check
 			--full-index --binary --abbrev --diff-filter=
-			--find-copies-harder --pickaxe-all --pickaxe-regex
+			--find-copies-harder
 			--text --ignore-space-at-eol --ignore-space-change
 			--ignore-all-space --exit-code --quiet --ext-diff
 			--no-ext-diff
 			--no-prefix --src-prefix= --dst-prefix=
-			--base --ours --theirs
 			--inter-hunk-context=
 			--patience
+			--raw
+"
+
+_git_diff ()
+{
+	__git_has_doubledash && return
+
+	local cur="${COMP_WORDS[COMP_CWORD]}"
+	case "$cur" in
+	--*)
+		__gitcomp "--cached --pickaxe-all --pickaxe-regex
+			--base --ours --theirs
+			$__git_diff_common_options
 			"
 		return
 		;;
@@ -975,17 +980,16 @@ _git_log ()
 			--relative-date --date=
 			--author= --committer= --grep=
 			--all-match
-			--pretty= --name-status --name-only --raw
+			--pretty=
 			--not --all
 			--left-right --cherry-pick
 			--graph
-			--stat --numstat --shortstat
-			--decorate --diff-filter=
-			--color-words --walk-reflogs
+			--decorate
+			--walk-reflogs
 			--parents --children --full-history
 			--merge
 			--inter-hunk-context=
-			--patience
+			$__git_diff_common_options
 			"
 		return
 		;;
@@ -1490,7 +1494,9 @@ _git_show ()
 		return
 		;;
 	--*)
-		__gitcomp "--pretty="
+		__gitcomp "--pretty=
+			$__git_diff_common_options
+			"
 		return
 		;;
 	esac
-- 
tg: (7ff1443..) t/bash-complete-show (depends on: origin/next)

^ permalink raw reply related

* Re: [PATCH] interpret_nth_last_branch(): avoid traversing the reflogs twice
From: Johannes Schindelin @ 2009-01-18  1:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Thomas Rast, git, Johannes Sixt, Johan Herland
In-Reply-To: <7vmydp5tqj.fsf@gitster.siamese.dyndns.org>

Hi,

On Sat, 17 Jan 2009, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > That is correct.  But this is such a highly uncritical code path that 
> > I'd like to keep this simple rather than fast.
> 
> I actually not worried about "fast" at all; it was more about unbounded 
> memory consumption.

Let's just assume that I make a branch switch per minute, continuously, 
for 90 days (until the reflogs are expired), and let's say that all my 
branchnames have 20 characters.  Conservatively, the memory requirement 
would be 100 * 2000 * 30 = 3 megabyte.

Note: these are the memory requirements after some really unrealistically 
high activity, and the memory is free()d during parameter parsing.

A much more realistical expectation would be to switch branches maybe 20 
times a day, which would amount to something like 36 kilobyte.  And again, 
they are free()d before the action really starts.

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