Git development
 help / color / mirror / Atom feed
* Re: [PATCH/RFC v4 3/5] sha1_name: support @{-N} syntax in get_sha1()
From: Johannes Schindelin @ 2009-01-17 17:55 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, Junio C Hamano, Johannes Sixt, Johan Herland
In-Reply-To: <1232208597-29249-4-git-send-email-trast@student.ethz.ch>

Hi,

On Sat, 17 Jan 2009, Thomas Rast wrote:

> diff --git a/t/t1505-rev-parse-last.sh b/t/t1505-rev-parse-last.sh
> new file mode 100755
> index 0000000..1e49dd2
> --- /dev/null
> +++ b/t/t1505-rev-parse-last.sh
> @@ -0,0 +1,71 @@
> +#!/bin/sh
> +
> +test_description='test @{-N} syntax'
> +
> +. ./test-lib.sh
> +
> +
> +make_commit () {
> +	echo "$1" > "$1" &&
> +	git add "$1" &&
> +	git commit -m "$1"
> +}
> +
> +
> +test_expect_success 'setup' '
> +
> +	make_commit 1 &&
> +	git branch side &&
> +	make_commit 2 &&
> +	make_commit 3 &&
> +	git checkout side &&
> +	make_commit 4 &&
> +	git merge master &&
> +	git checkout master
> +
> +'
> +
> +# 1 -- 2 -- 3 master
> +#  \         \
> +#   \         \
> +#    --- 4 --- 5 side
> +#
> +# and 'side' should be the last branch
> +
> +git log --graph --all --pretty=oneline --decorate
> +

Maybe you want to squash this in, so that the output of "make test" is 
not cluttered by the graph?

-- snipsnap --
diff --git a/t/t1505-rev-parse-last.sh b/t/t1505-rev-parse-last.sh
index 1e49dd2..72e8322 100755
--- a/t/t1505-rev-parse-last.sh
+++ b/t/t1505-rev-parse-last.sh
@@ -32,7 +32,11 @@ test_expect_success 'setup' '
 #
 # and 'side' should be the last branch
 
-git log --graph --all --pretty=oneline --decorate
+test_expect_success 'show a log (for debugging)' '
+
+	git log --graph --all --pretty=oneline --decorate
+
+'
 
 test_rev_equivalent () {
 
-- 
1.6.1.332.g9a59d

^ permalink raw reply related

* [PATCH 6/5] Fix parsing of @{-1}@{1}
From: Johannes Schindelin @ 2009-01-17 18:08 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, Junio C Hamano, Johannes Sixt, Johan Herland
In-Reply-To: <1232208597-29249-6-git-send-email-trast@student.ethz.ch>


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

 sha1_name.c               |   25 ++++++++++++++++++++++++-
 t/t1505-rev-parse-last.sh |    2 +-
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 306d04b..ee0c456 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -240,8 +240,28 @@ static int ambiguous_path(const char *path, int len)
 	return slash;
 }
 
+/*
+ * *string and *len will only be substituted, and *string returned (for
+ * later free()ing) if the string passed in is of the form @{-<n>}.
+ */
+static char *substitute_nth_last_branch(const char **string, int *len)
+{
+	struct strbuf buf = STRBUF_INIT;
+	int ret = interpret_nth_last_branch(*string, &buf);
+
+	if (ret == *len) {
+		size_t size;
+		*string = strbuf_detach(&buf, &size);
+		*len = size;
+		return (char *)*string;
+	}
+
+	return NULL;
+}
+
 int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
 {
+	char *last_branch = substitute_nth_last_branch(&str, &len);
 	const char **p, *r;
 	int refs_found = 0;
 
@@ -261,11 +281,13 @@ int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
 				break;
 		}
 	}
+	free(last_branch);
 	return refs_found;
 }
 
 int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
 {
+	char *last_branch = substitute_nth_last_branch(&str, &len);
 	const char **p;
 	int logs_found = 0;
 
@@ -296,6 +318,7 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
 		if (!warn_ambiguous_refs)
 			break;
 	}
+	free(last_branch);
 	return logs_found;
 }
 
@@ -314,7 +337,7 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 	/* basic@{time or number or -number} format to query ref-log */
 	reflog_len = at = 0;
 	if (str[len-1] == '}') {
-		for (at = 0; at < len - 1; at++) {
+		for (at = len-2; at >= 0; at--) {
 			if (str[at] == '@' && str[at+1] == '{') {
 				reflog_len = (len-1) - (at+2);
 				len = at;
diff --git a/t/t1505-rev-parse-last.sh b/t/t1505-rev-parse-last.sh
index 72e8322..2d6b31e 100755
--- a/t/t1505-rev-parse-last.sh
+++ b/t/t1505-rev-parse-last.sh
@@ -58,7 +58,7 @@ test_expect_success '@{-1}^2 works' '
 	test_rev_equivalent side^2 @{-1}^2
 '
 
-test_expect_failure '@{-1}@{1} works' '
+test_expect_success '@{-1}@{1} works' '
 	test_rev_equivalent side@{1} @{-1}@{1}
 '
 
-- 
1.6.1.332.g9a59d

^ permalink raw reply related

* Re: [PATCH] git-checkout(1) mention fate of extraneous files
From: jidanni @ 2009-01-17 18:35 UTC (permalink / raw)
  To: markus.heidelberg; +Cc: git
In-Reply-To: <200901171357.18005.markus.heidelberg@web.de>

MH> Why do you reformat the whole paragraph?

OK, glad to know that I don't need to!!

^ permalink raw reply

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

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

>>> -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 &&
>>> -     [ -f "$HTTPD_DOCUMENT_ROOT_PATH/test_repo.git/refs/heads/master" ]
>>> +     git push origin master
>>>  '
>>
>> ... this removal?  I do not think this is a good change, as it removes
>> a test that is actually pretty important.
>
> i'm sorry for the poor commit message, what i wanted to do was to
> change the tests to expect success rather than failure. no tests were
> removed; only their expected outcomes were modified. currently, the
> pushes fail, so the test 'fails as expected'; now the pushes succeed,
> so they shouldn't be expecting failed pushes (or anything else).

The original seems to want the push to succeed, and also it wants the file
refs/heads/master to be present after the push (presumably because there
should be that ref when the push succeeds).  If you fixed "push" that used
to fail to succeed, that is great, and s/failure/success/ is a good thing.

But you are removing something else without explanation.  Why do you need
to remove the part of the test that checks if refs/heads/master is present?
Is it looking for a file in a wrong place?

^ permalink raw reply

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

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

> Instead of traversing them twice, we just build a list of branch switches,
> pick the one we're interested in, and free the list again.

Isn't the code keeping them all in core, or am I reading the patch wrong?

If you know that you are interested in the nth-from-the-last switch, and
if you are reading from the beginning, you would need to keep at most n 
last switches you have seen in core, wouldn't you?  

^ permalink raw reply

* Re: [JGIT PATCH 8/8] Define a basic merge API, and a two-way tree merge strategy
From: Tomi Pakarinen @ 2009-01-17 19:16 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Robin Rosenberg, git
In-Reply-To: <20090115210936.GI10179@spearce.org>

testTrivialTwoWay_disjointhistories() failed because merge strategy
didn't handle missing base
version. Am'i right?

  Tomi.

>From 1ed694b55d307c640d29eeebfcd108e08681297b Mon Sep 17 00:00:00 2001
From: Tomi Pakarinen <tomi.pakarinen@iki.fi>
Date: Sat, 17 Jan 2009 20:56:04 +0200
Subject: [PATCH] If base version missing, we can merge version from
one of other trees.

Signed-off-by: Tomi Pakarinen <tomi.pakarinen@iki.fi>
---
 .../jgit/merge/StrategySimpleTwoWayInCore.java     |   28 +++++++++++++++-----
 1 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/merge/StrategySimpleTwoWayInCore.java
b/org.spearce.jgit/src/org/spearce/jgit/merge/StrategySimpleTwoWayInCore.java
index 893add9..eb718ab 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/merge/StrategySimpleTwoWayInCore.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/merge/StrategySimpleTwoWayInCore.java
@@ -43,6 +43,7 @@
 import org.spearce.jgit.dircache.DirCacheBuilder;
 import org.spearce.jgit.dircache.DirCacheEntry;
 import org.spearce.jgit.errors.UnmergedPathException;
+import org.spearce.jgit.lib.FileMode;
 import org.spearce.jgit.lib.ObjectId;
 import org.spearce.jgit.lib.Repository;
 import org.spearce.jgit.treewalk.AbstractTreeIterator;
@@ -119,13 +120,26 @@ protected boolean mergeImpl() throws IOException {
 				}

 				final int modeB = tw.getRawMode(T_BASE);
-				if (modeB == modeO && tw.idEqual(T_BASE, T_OURS))
-					add(T_THEIRS, DirCacheEntry.STAGE_0);
-				else if (modeB == modeT && tw.idEqual(T_BASE, T_THEIRS))
-					add(T_OURS, DirCacheEntry.STAGE_0);
-				else {
-					conflict();
-					hasConflict = true;
+				if (!FileMode.MISSING.equals(modeB)) {
+					if (modeB == modeO && tw.idEqual(T_BASE, T_OURS))
+						add(T_THEIRS, DirCacheEntry.STAGE_0);
+					else if (modeB == modeT && tw.idEqual(T_BASE, T_THEIRS))
+						add(T_OURS, DirCacheEntry.STAGE_0);
+					else {
+						conflict();
+						hasConflict = true;
+					}
+				} else {
+					if (!FileMode.MISSING.equals(modeO)
+							&& FileMode.MISSING.equals(modeT))
+						add(T_OURS, DirCacheEntry.STAGE_0);
+					else if (FileMode.MISSING.equals(modeO)
+							&& !FileMode.MISSING.equals(modeT))
+						add(T_THEIRS, DirCacheEntry.STAGE_0);
+					else {
+						conflict();
+						hasConflict = true;
+					}
 				}
 			}
 			builder.finish();
-- 
1.6.0.4

^ permalink raw reply related

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

Hi,

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

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Instead of traversing them twice, we just build a list of branch switches,
> > pick the one we're interested in, and free the list again.
> 
> Isn't the code keeping them all in core, or am I reading the patch wrong?
> 
> If you know that you are interested in the nth-from-the-last switch, and
> if you are reading from the beginning, you would need to keep at most n 
> last switches you have seen in core, wouldn't you?  

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

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 3/3] http-push: update tests
From: Johannes Schindelin @ 2009-01-17 19:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Ray Chuan, git
In-Reply-To: <7vsknh7og5.fsf@gitster.siamese.dyndns.org>

Hi,

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

> The original seems to want the push to succeed, and also it wants the 
> file refs/heads/master to be present after the push (presumably because 
> there should be that ref when the push succeeds).  If you fixed "push" 
> that used to fail to succeed, that is great, and s/failure/success/ is a 
> good thing.
> 
> But you are removing something else without explanation.  Why do you 
> need to remove the part of the test that checks if refs/heads/master is 
> present? Is it looking for a file in a wrong place?

As I mentioned with two other patches, the push does not succeed, and that 
is the reason for the "failure" in test_expect_failure.

It does not succeed for two reasons:

- due to an off-by-path_len bug, xmalloc() tries to allocate ~4GB of 
  memory, which is a bit much, so http-push die()s with an OOM.

- even with that fix, the push fails because it cannot find any common 
  refs.  It cannot find them because it does not download info/refs as it 
  is supposed to do, but it looks through refs/, missing the fact that the 
  refs are packed (which it cannot handle).

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH/RFC v4 3/5] sha1_name: support @{-N} syntax in get_sha1()
From: Junio C Hamano @ 2009-01-17 19:37 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, Johannes Schindelin, Johannes Sixt, Johan Herland
In-Reply-To: <1232208597-29249-4-git-send-email-trast@student.ethz.ch>

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

> Let get_sha1() parse the @{-N} syntax, with docs and tests.
>
> Note that while @{-1}^2, @{-2}~5 and such are supported, @{-1}@{1} is
> currently not allowed.
>
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> ...
> @@ -324,6 +326,16 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
>  		return -1;
>  
>  	if (!len && reflog_len) {
> +		struct strbuf buf = STRBUF_INIT;
> +		int ret;
> +		/* try the @{-N} syntax for n-th checkout */
> +		ret = interpret_nth_last_branch(str+at, &buf);
> +		if (ret > 0) {
> +			/* substitute this branch name and restart */
> +			return get_sha1_1(buf.buf, buf.len, sha1);
> +		} else if (ret == 0) {
> +			return -1;
> +		}

What are the possible failure cases, and what do we want to tell the
end-user?

 - You asked for 3rd but there weren't that many switches yet, and ask
   "git rev-parse --verify @{-3}".

   Are we Ok with "fatal: Needed a single revision" from rev-parse?  Do we
   want to show "fatal: @{-3}: not that many branch switches yet"?

   What happens to "git checkout @{-3}" in this case?  Having checkout say
   "fatal: invalid reference: @{-3}" would be fine in this case, I think.

 - You try "git checkout @{-3}", you were on "frotz" branch back then, but
   the branch does not exist anymore.

   I think you will get "fatal: invalid reference: frotz" from checkout,
   which should be fine.

There also is a case where nth_last_branch() may find something that is
not a branch (e.g. "git checkout HEAD^"), but I am hoping we can label
that as a bug in nth_last_branch() and fix it later.

^ permalink raw reply

* Re: [PATCH 3/3] http-push: update tests
From: Ray Chuan @ 2009-01-17 19:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7vsknh7og5.fsf@gitster.siamese.dyndns.org>

Hi,

On Sat, Jan 17, 2009 at 6:54 PM, Junio C Hamano <gitster@pobox.com> wrote:
> "Ray Chuan" <rctay89@gmail.com> writes:
>
>>>> -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 &&
>>>> -     [ -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.

-- 
Cheers,
Ray Chuan

^ permalink raw reply

* Re: [PATCH/RFC v4 4/5] checkout: implement "-" abbreviation, add docs and tests
From: Junio C Hamano @ 2009-01-17 19:57 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, Johannes Schindelin, Johannes Sixt, Johan Herland
In-Reply-To: <1232208597-29249-5-git-send-email-trast@student.ethz.ch>

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

> @@ -133,6 +133,10 @@ the conflicted merge in the specified paths.
>  +
>  When this parameter names a non-branch (but still a valid commit object),
>  your HEAD becomes 'detached'.
> ++
> +As a special case, the "`@\{-N\}`" syntax for the N-th last branch
> +checks out the branch (instead of detaching).  You may also specify
> +"`-`" which is synonymous with "`@\{-1\}`".

I mildly disagree with this wording.

The new syntax is supposed to be a new way to name a branch, not a random
non-branch committish that is special cased by "git checkout".  I would
further suggest that we should teach "git rev-parse --symbolic-full-name"
and "git rev-parse --symbolic" about the new syntax, so that scripts can
use the syntax to find out the same information.

The "-" thing deserves a mention here in the documentation.  That _is_ a
special case that only applies to the "git checkout" command.

> diff --git a/builtin-checkout.c b/builtin-checkout.c
> index dc1de06..b0a101b 100644
> --- a/builtin-checkout.c
> +++ b/builtin-checkout.c
> @@ -679,6 +679,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
>  		arg = argv[0];
>  		has_dash_dash = (argc > 1) && !strcmp(argv[1], "--");
>  
> +		if (!strcmp(arg, "-"))
> +			arg = "@{-1}";
> +

This is not quite nice as it could be, but it probably is Ok.  If the
interpretation of @{-1} errors out, the user won't see an error message
that talks about "-" but instead the user will see "@{-1}".

Also it will look somewhat inconsistent to the end user who does not know
the internals for "-" claim to be a synonym for @{-1} but it really isn't.
For example, "git checkout -^0" does not work as "git checkout @{-1}^0".

To avoid such confusion, we could instead make "git checkout - <ENTER>" a
synonym for "git checkout @{-1} <ENTER>", without claiming to make "-" a
synonym for "@{-1}".  In other words, "git checkout -" can become a very
narrow, focused special case that does not allow anything else, such as
pathspecs, "--" separator, nor --force and other options.

^ permalink raw reply

* 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


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