Git development
 help / color / mirror / Atom feed
* Re: Strange merge failure (would be overwritten by merge / cannot merge)
From: Linus Torvalds @ 2009-09-06 22:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <alpine.LFD.2.01.0909061424160.8946@localhost.localdomain>



On Sun, 6 Sep 2009, Linus Torvalds wrote:
> 
> So here's a slightly updated version, and it passes all the tests.

.. and here's something with a bit more abstraction, a bit more cleanup, 
and making more sure that there's no semantic changes. So that I can feel 
happy signing off on it.

		Linus
---
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Sun, 6 Sep 2009 14:37:21 -0700
Subject: [PATCH] Prepare 'traverse_trees()' for D/F conflict lookahead

traverse_trees() used to always walk the trees in order, and used the
special (and fundamentally broken) 'df_name_compare()' function to
compare directory and file entries as equal.

That works fine for all the common cases, when the D/F conflicts are
immediately adjacent, and there are no other entries that could confuse
the ordering.  But if you have one tree with a file 'a', and another
tree with a file 'a-1' and a directory 'a/', then you would not see the
D/F conflict of 'a' and 'a/' without looking ahead past the 'a-1' file
in the other tree.

So this re-organizes the tree walking code so that we can start doing
look-ahead for those cases.  It doesn't actually _do_ that lookahead
yet, because it requires marking the conflicts we've used, but the code
is now organized to do so.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
 tree-walk.c |   90 +++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 85 insertions(+), 5 deletions(-)

diff --git a/tree-walk.c b/tree-walk.c
index 02e2aed..7251dd2 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -62,7 +62,7 @@ void *fill_tree_descriptor(struct tree_desc *desc, const unsigned char *sha1)
 
 static int entry_compare(struct name_entry *a, struct name_entry *b)
 {
-	return df_name_compare(
+	return base_name_compare(
 			a->path, tree_entry_len(a->path, a->sha1), a->mode,
 			b->path, tree_entry_len(b->path, b->sha1), b->mode);
 }
@@ -138,6 +138,80 @@ char *make_traverse_path(char *path, const struct traverse_info *info, const str
 	return path;
 }
 
+/*
+ * See if 'entry' may conflict with a later tree entry in 't': if so,
+ * fill in 'conflict' with the conflicting tree entry from 't'.
+ *
+ * NOTE! Right now we do _not_ create a create a private copy of the tree
+ * descriptor, so we can't actually walk it any further without losing
+ * our place. We should change it to a loop over a copy of the tree
+ * descriptor, but then we'd also have to remember the skipped entries,
+ * so this is a hacky simple case that only handles the case we used
+ * to handle historically (ie clash in the very first entry)
+ *
+ * Note that only a regular file 'entry' can conflict with a later
+ * directory, since a directory with the same name will sort later.
+ */
+static int find_df_conflict(struct tree_desc *t, struct name_entry *entry, struct name_entry *conflict)
+{
+	int len;
+
+	if (S_ISDIR(entry->mode))
+		return 0;
+	len = tree_entry_len(entry->path, entry->sha1);
+
+	while (t->size) {
+		int nlen;
+
+		entry_extract(t, conflict);
+		nlen = tree_entry_len(conflict->path, conflict->sha1);
+
+		/*
+		 * We can only have a future conflict if the entry matches the
+		 * beginning of the name exactly, and if the next character is
+		 * smaller than '/'.
+		 *
+		 * Break out otherwise.
+		 */
+		if (nlen < len)
+			break;
+		if (memcmp(conflict->path, entry->path, nlen))
+			break;
+		if (nlen == len)
+			return 1;
+
+		if (conflict->path[len] > '/')
+			break;
+		/*
+		 * FIXME! Here we'd really like to do 'update_tree_entry(&copy);'
+		 * but that requires us to remember the conflict position specially
+		 * so now we just punt and stop looking for conflicts
+		 */
+		break;
+	}
+	entry_clear(conflict);
+	return 0;
+}
+
+/*
+ * For now, the used entries are always at the head of the tree_desc
+ * (no look-ahead), so marking an entry used is always just a matter
+ * of doing an 'update_tree_entry()'
+ */
+static void used_entry(struct tree_desc *t, struct name_entry *entry)
+{
+	update_tree_entry(t);
+}
+
+static int get_entry(struct tree_desc *t, struct name_entry *entry)
+{
+	if (t->size) {
+		entry_extract(t, entry);
+		return 1;
+	}
+	return 0;
+}
+
 int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
 {
 	int ret = 0;
@@ -150,9 +224,8 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
 
 		last = -1;
 		for (i = 0; i < n; i++) {
-			if (!t[i].size)
+			if (!get_entry(t+i, entry+i))
 				continue;
-			entry_extract(t+i, entry+i);
 			if (last >= 0) {
 				int cmp = entry_compare(entry+i, entry+last);
 
@@ -179,13 +252,20 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
 		dirmask &= mask;
 
 		/*
-		 * Clear all the unused name-entries.
+		 * Clear all the unused name-entries, and look for
+		 * conflicts.
 		 */
 		for (i = 0; i < n; i++) {
 			if (mask & (1ul << i))
 				continue;
 			entry_clear(entry + i);
+			if (find_df_conflict(t+i, entry+last, entry+i))
+				dirmask |= (1ul << i);
 		}
+
+		/* Add in the DF conflict entries into the mask */
+		mask |= dirmask;
+
 		ret = info->fn(n, mask, dirmask, entry, info);
 		if (ret < 0)
 			break;
@@ -194,7 +274,7 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
 		ret = 0;
 		for (i = 0; i < n; i++) {
 			if (mask & (1ul << i))
-				update_tree_entry(t + i);
+				used_entry(t+i, entry+i);
 		}
 	}
 	free(entry);

^ permalink raw reply related

* Re: [PATCH 1/2] grep: accept relative paths outside current working directory
From: Junio C Hamano @ 2009-09-06 22:58 UTC (permalink / raw)
  To: Clemens Buchacher; +Cc: Jeff King, SZEDER Gábor, git
In-Reply-To: <20090905123117.GA3099@darc.dnsalias.org>

Clemens Buchacher <drizzd@aon.at> writes:

> On Sat, Sep 05, 2009 at 12:58:50AM -0700, Junio C Hamano wrote:
>> >> If "git add -u ../.." (I mean "the grand parent directory", not "an
>> >> unnamed subdirectory") did not work 
>
> In git.git:
>
> $ cd t
> $ git grep addremove -- ../
> fatal: git grep: cannot generate relative filenames containing '..'
>
> So here's a fix for that. And a configurable solution for add and grep's
> scope as a follow-up. I did not look at any other commands yet.

Thanks.  This was oa breakage I pointed out in an earlier message in this
discussion, and it is worth fixing.

Your patch is queued in 'pu', but it seems to break the exit status in a
strange way with my limited test.

Here is a non-broken behaviour without the "look in uplevel":

: git.git/cb/maint-1.6.3-grep-relative-up; ./git grep adddelete .
: git.git/cb/maint-1.6.3-grep-relative-up; ./git grep adddelete .; echo $?
1
: git.git/cb/maint-1.6.3-grep-relative-up; ./git grep adddelete . >/dev/null; echo $?
1

Now we go down, and grep from an uplevel:

: git.git/cb/maint-1.6.3-grep-relative-up; cd t
: t/cb/maint-1.6.3-grep-relative-up; ../git grep adddelete ..
: t/cb/maint-1.6.3-grep-relative-up; ../git grep adddelete .. ; echo $?
1
: t/cb/maint-1.6.3-grep-relative-up; ../git grep adddelete .. >/dev/null; echo $?
0

The command should not give different exit status depending on the
destination of standard output stream.

^ permalink raw reply

* Re: [BUG] 'add -u' doesn't work from untracked subdir
From: Nanako Shiraishi @ 2009-09-07  0:07 UTC (permalink / raw)
  To: Clemens Buchacher; +Cc: Junio C Hamano, Jeff King, SZEDER Gbor, git
In-Reply-To: <20090905084641.GA24865@darc.dnsalias.org>

Quoting Clemens Buchacher <drizzd@aon.at>

> Sorry for stating the obvious here, but the following commands affect the
> entire repository, even though they limit themselves to the current
> directory, if passed a '.'.
>
> 	git commit
> 	git log
> 	git diff
> 	git checkout
> 	git reset
>
> Due to the frequent use of these commands, I believe many users (myself
> included) expect "git add" and "git grep" to do the same. AFAICT the
> following commands are the only non-plumbing ones that behave differently:
>
> 	git add -u
> 	git add -A
> 	git grep
>
> So I argue that _that_ is the real inconsistency.

The default behavior for 'git-grep' has already been discussed in length, and I don't think it is likely to change. See 

  http://thread.gmane.org/gmane.comp.version-control.git/111519/focus=111717

The original design for the other two in your list was to be a whole tree operation. This commit broke it. 

  2ed2c22 "git-add -u paths... now works from subdirectory".

'git-add -u' in a subdirectory without any other argument used to work on the entire working tree before that commit, but it didn't prefix the current directory in front of the paths... arguments. 

That commit 2ed2c22 fixed 'git-add -u paths...' by prepending the prefix to the arguments, but it broke 'git-add -u' to always limit the updates to the current directory. 

I think it is a good idea to fix this as an old regression in the maint branch. You don't have to introduce "git add -a". In fact the -a option was explicitly rejected when "git add -A" option was added with this commit. 

  3ba1f11 "git-add --all: add all files"

because "git commit -a" will never include new files and it will be inconsistent if "git add -a" did so.

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

^ permalink raw reply

* Re: Use case I don't know how to address
From: Alan Chandler @ 2009-09-06 12:44 UTC (permalink / raw)
  Cc: git
In-Reply-To: <7vk50ccxfj.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
> Alan Chandler <alan@chandlerfamily.org.uk> writes:
> 
>>        2' - 2a - 3' - 4' ----------------- 6' SITE
>>       /         /    /                    /
>> 1 -  2  ------ 3  - 4  ------------6'''- 6a TEST
>>                      \            /
>>                        5  ------ 6  MASTER
>>                         \         \
>>                           5''- 5a- 6'' DEMO
>>
>>
>> What will happen is the changes made in 4->5 will get applied to the
>> (now) Test branch as part of the 6->6'' merge, and I will be left
>> having to add a new commit, 6a, to undo them all again.  Given this is
>> likely to be quite a substantial change I want to try and avoid it if
>> possible.
> 
> I presume 6'''-6a has the revert of 4-5?  If so, the next merge should
> work just fine.


I think you missed the issue - Yes 6'''-6a is the revert, but the 
problem is this could be large and complicated, and I wanted to find an 
automated way rather than manual

Sort of like doing a diff of 4-5 and somehow applying it backwards.


-- 
Alan Chandler
http://www.chandlerfamily.org.uk

^ permalink raw reply

* Re: [PATCH 3/4] push: make non-fast-forward help message configurable
From: Nanako Shiraishi @ 2009-09-07  0:44 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jeff King, Nanako Shiraishi, Matthieu Moy, Teemu Likonen, Git
In-Reply-To: <7vzl98fr22.fsf@alter.siamese.dyndns.org>

Quoting Junio C Hamano <gitster@pobox.com>

> Speaking of which, has anybody felt annoyed by this message?
>
>     $ git reset --hard HEAD^^
>     HEAD is now at 3fb9d58 Do not scramble password read from .cvspass
>
> This is not "maybe you should try this", but I would consider that it
> falls into the same "I see you are trying to be helpful, but I know what I
> am doing, and you are stealing screen real estate from me without helping
> me at all, thank you very much" category.

You may be fixated at the sha1 part of the message when you find this message annoying, but I disagree strongly. I always appreciate the assurance this message gives me that I counted the number of commits correctly, whether I say HEAD^^^^ or HEAD~7.

Showing the subject of the commit you are now at is very useful and I will be equally irritated as you do if it starts showing the subject of the commit I was at.

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

^ permalink raw reply

* Re: Issue 323 in msysgit: Can't clone over http
From: Junio C Hamano @ 2009-09-07  4:59 UTC (permalink / raw)
  To: Tay Ray Chuan; +Cc: git, msysgit
In-Reply-To: <20090904212956.f02b0c60.rctay89@gmail.com>

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

> Subject: [PATCH] http.c: clarify missing-pack-check
>
> Abort the pack transfer only if the pack is not available in the HTTP-
> served repository; otherwise, allow the transfer to continue, even if
> the check failed.
>
> This addresses an issue raised by bjelli:
>
>   http://code.google.com/p/msysgit/issues/detail?id=323
>
> Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
> ---
>  http.c |   10 +++++++---
>  1 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/http.c b/http.c
> index 5926c5b..cba7e9a 100644
> --- a/http.c
> +++ b/http.c
> @@ -864,6 +864,7 @@ int http_fetch_ref(const char *base, struct ref *ref)
>  static int fetch_pack_index(unsigned char *sha1, const char *base_url)
>  {
>  	int ret = 0;
> +	int result;
>  	char *hex = xstrdup(sha1_to_hex(sha1));
>  	char *filename;
>  	char *url;
> @@ -874,11 +875,14 @@ static int fetch_pack_index(unsigned char *sha1, const char *base_url)
>  	strbuf_addf(&buf, "objects/pack/pack-%s.pack", hex);
>  	url = strbuf_detach(&buf, 0);
>
> -	if (http_get_strbuf(url, NULL, 0)) {
> -		ret = error("Unable to verify pack %s is available",
> +	result = http_get_strbuf(url, NULL, 0);
> +	if (result == HTTP_MISSING_TARGET) {
> +		ret = error("Unable to find pack %s",
>  			    hex);
>  		goto cleanup;
> -	}
> +	} else if (result && http_is_verbose)
> +		fprintf(stderr, "Unable to verify pack %s is available\n",
> +			hex);
>
>  	if (has_pack_index(sha1)) {
>  		ret = 0;

You said:

> Releases including and after v1.6.4 will have this issue:
>
>>> error: Unable to verify pack 382c25c935b744e909c749532578112d72a4aff9 is
>>> available
>>> error: Unable to find 0a41ac04d56ccc96491989dc71d9875cd804fc6b under
>>> http://github.com/tekkub/addontemplate.git
>
> The issue at hand is due to git checking the http repository for the
> pack file before commencing the transfer; failing which, the transfer
> aborts.
>
> Right now, git chokes on the 500 error that github.com gives it, which
> shouldn't be the case, even though that's a weird response.

I am assuming that you meant 39dc52c was the culprit by "including and
after v1.6.4", but it is not quite clear how this patch helps if that is
the case.

Before 39dc52c (http: use new http API in fetch_index(), 2009-06-06), we
used to run the slot by hand and checked results.curl_request against
CURLE_OK.  Everything else was an error.

With the updated code, that all went to http_get_strbuf() which in turn
calls http_request() that does the same thing, and the function returns
HTTP_OK only when it gets CURLE_OK, but now it says MISSING_TARGET when we
ask for an idx file we think exists in the repository but the server says
it doesn't, and all other errors will be ignored with this patch.

If this codepath is what was broken by github returning 500 [*1*], the
client before 39dc52c would have failed the same way.  I do not think
loosening error checking like this is a real solution, but I may be
reading the patch incorrectly.

Do people on the github side see something strange in the log?  Perhaps we
think we are making a request to objects/pack/ of the repository but by
mistake the request is going to somewhere completely off (but then we
would get 401 not 500).


[Footnote]

*1* Which I do agree is somewhat strange thing to say for a request to a
file in the objects/pack directory in a public repository---I would
understand it if it were 404, but then it means the repository is
inconsistent, i.e. has a stale objects/info/packs relative to its set of
packs it has.

^ permalink raw reply

* Re: tracking branch for a rebase
From: Junio C Hamano @ 2009-09-07  5:05 UTC (permalink / raw)
  To: Jeff King; +Cc: Björn Steinbrink, Michael J Gruber, Pete Wyckoff, git
In-Reply-To: <20090905142841.GB15631@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

>> Hm, I'd prefer a shorthand for "upstream for this branch", instead of
>> magic defaults.
>
> The more I think about, the more I think that is the right solution.
> Because magic defaults for "rebase -i" don't help when you want to do
> "gitk $UPSTREAM..".
>
> The previous discussion on the topic seems to be here:
>
>   http://article.gmane.org/gmane.comp.version-control.git/113666
>
> And apparently you and I both participated in the discussion, which I
> totally forgot about.
>
> Looks like the discussion ended with people liking the idea but not
> knowing what the specifier should look like. Maybe tightening the ref
> syntax a bit to allow more extensible "special" refs is a good v1.7.0
> topic? I dunno.

At-mark currently is reserved for anything that uses reflog, but we can
say that it is to specify operations on refs (as opposed to caret and
tilde are to specify operations on object names).

It specifies what ref to work on with the operand on its left side (and an
empty string stands for "HEAD"), and what operation is done to it by what
is in {} on the right side of it.  This view is quite consistent with the
following existing uses of the notation:

	ref@{number}	-- nth reflog entry
        ref@{time}	-- ref back then
	@{-number}	-- nth branch switching

So perhaps ref@{upstream}, or any string that is not a number and cannot
be time, can trigger the magic operation on the ref with ref@{magic}
syntax?

^ permalink raw reply

* Re: [BUG] 'add -u' doesn't work from untracked subdir
From: Junio C Hamano @ 2009-09-07  5:07 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: Clemens Buchacher, Jeff King, SZEDER Gbor, git
In-Reply-To: <20090907090713.6117@nanako3.lavabit.com>

Nanako Shiraishi <nanako3@lavabit.com> writes:

[jc: sometimes but not always your messages have looooooooooooooong lines.
please line-wrap for readability. I learned to type W Q so often I do not
complain but other people would find them irritating.]

> The default behavior for 'git-grep' has already been discussed in
> length, and I don't think it is likely to change. See
>
>   http://thread.gmane.org/gmane.comp.version-control.git/111519/focus=111717

Interesting.

The first message in that thread lists things we have scheduled for 1.7.0
but it has another item.  It is an off-topic for the thread, but we might
want to resurrect the "core.quotepath defaults to false" proposal, before
it gets too late for 1.7.0.

As to "grep", I am open to the proposal to make git commands consistent by
letting them operate on everything when no path argument is given, if grep
is really the single odd-man out remaining after changing the "add -u" and
"add -A" to work on the whole tree from subdirectories when given no paths.

> The original design for the other two in your list was to be a whole
> tree operation. This commit broke it.
>
>   2ed2c22 "git-add -u paths... now works from subdirectory".
> ...
> I think it is a good idea to fix this as an old regression in the maint
> branch. You don't have to introduce "git add -a". In fact the -a option
> was explicitly rejected when "git add -A" option was added with this
> commit.

Geez, you are good at digging things up.

It is very tempting to follow the suggestion above, but I suspect that,
even though it may be a regression from historical point of view, some
people who are used to the current behaviour may look at the corrected
one as a regression.  We've had the change by 2ed2c22 for a long time.

I dunno.

^ permalink raw reply

* Re: Issue 323 in msysgit: Can't clone over http
From: Tay Ray Chuan @ 2009-09-07  5:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, msysgit
In-Reply-To: <7v8wgrbb9e.fsf@alter.siamese.dyndns.org>

Hi,

On Mon, Sep 7, 2009 at 12:59 PM, Junio C Hamano<gitster@pobox.com> wrote:
> Before 39dc52c (http: use new http API in fetch_index(), 2009-06-06), we
> used to run the slot by hand and checked results.curl_request against
> CURLE_OK.  Everything else was an error.
>
> With the updated code, that all went to http_get_strbuf() which in turn
> calls http_request() that does the same thing, and the function returns
> HTTP_OK only when it gets CURLE_OK, but now it says MISSING_TARGET when we
> ask for an idx file we think exists in the repository but the server says
> it doesn't, and all other errors will be ignored with this patch.

We should only be interested in the MISSING_TARGET error, because it
tells us that the pack file is indeed not available. We aren't
interested in other errors, like being unable to perform the request
(HTTP_START_FAILED), or, say, a 401 (Unauthorized) error, or even a
500; we simply move along and we tell the user we couldn't perform the
check.

> If this codepath is what was broken by github returning 500 [*1*], the
> client before 39dc52c would have failed the same way.  I do not think
> loosening error checking like this is a real solution, but I may be
> reading the patch incorrectly.

You're right to say that git before 39dc52c would have failed. It did,
but no one had the chance to break anything, because 39dc52c was part
of my http patch series that only went wild in v1.6.4.

We can trace this back to 48188c2 ("http-walker: verify remote
packs"), which copied the "feature" from http-push.c to http-walker.c.

Before that, if you tried fetching a pack with http-push.c from a
500-prone server, you would also experience this.

-- 
Cheers,
Ray Chuan

^ permalink raw reply

* Re: [BUG] 'add -u' doesn't work from untracked subdir
From: Matthieu Moy @ 2009-09-07  6:23 UTC (permalink / raw)
  To: Clemens Buchacher; +Cc: Junio C Hamano, Jeff King, SZEDER Gábor, git
In-Reply-To: <20090906181621.GA23412@localhost>

Clemens Buchacher <drizzd@aon.at> writes:

> On Sun, Sep 06, 2009 at 02:32:44PM +0200, Matthieu Moy wrote:
>> I think it has already been proposed to introduce "git add -a" doing
>> what "git add -u" do, but for the full tree.
>
> I like that, actually. AFAICT it's completely analogous to "git commit -a".
> We also need something for "git add -A" though.
>
> Do you feel the same way about changing the behavior of "git grep"? I don't
> really want to change the command's name.

I don't have particular feeling about "git grep", probably because I
don't use it much. One argument in favour of keeping the current
behavior is the consistancy with plain "grep".

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* [PATCH 0/2] Add url.<base>.pushInsteadOf: URL rewriting for push only
From: Josh Triplett @ 2009-09-07  6:59 UTC (permalink / raw)
  To: git, gitster

Many sites host repositories via both git:// for fetch-only anonymous
access and ssh:// for push-capable access.  The "insteadOf" mechanism
makes it straightforward to substitute the push-capable URLs for the
pull-only URLs, which proves convenient when the site hosts many
repositories using the same URL scheme.  However, if you use such a
substitution and you cannot use the ssh:// URLs (either because you
don't have SSH access or you don't have permission to a particular
repository), you cannot clone or fetch either, even though you could do
so via the git:// URLs.  A situation like this arises when sharing git
configuration files between systems, of which only a few have SSH access
to repositories.

"pushurl" provides a way to specify URLs used only for push, but this
requires configuring a pushurl for each such repository.  As in the
rationale for insteadOf, it makes sense to configure this for all
repositories hosted on a given system at once.

This patch series adds a new "pushInsteadOf" option to go with
"insteadOf".  pushInsteadOf allows systematically rewriting fetch-only
URLs to push-capable URLs when used with push.  For instance:

[url "ssh://example.org/"]
    pushInsteadOf = "git://example.org/"

This will allow clones of "git://example.org/path/to/repo" to
subsequently push to "ssh://example.org/path/to/repo", without manually
configuring pushurl for that remote.

Includes documentation for the new option, bash completion updates, and
test cases (both that pushInsteadOf applies to push and that it does
*not* apply to fetch).


Josh Triplett (2):
  Wrap rewrite globals in a struct in preparation for adding another set
  Add url.<base>.pushInsteadOf: URL rewriting for push only

 Documentation/config.txt               |   12 +++++
 Documentation/urls.txt                 |   18 +++++++
 contrib/completion/git-completion.bash |    2 +-
 remote.c                               |   80 +++++++++++++++++++------------
 t/t5516-fetch-push.sh                  |   31 ++++++++++++
 5 files changed, 111 insertions(+), 32 deletions(-)

^ permalink raw reply

* [PATCH 1/2] Wrap rewrite globals in a struct in preparation for adding another set
From: Josh Triplett @ 2009-09-07  7:00 UTC (permalink / raw)
  To: git, gitster
In-Reply-To: <cover.1252306396.git.josh@joshtriplett.org>

remote.c has a global set of URL rewrites, accessed by alias_url and
make_rewrite.  Wrap them in a new "struct rewrites", passed to alias_url
and make_rewrite.  This allows adding other sets of rewrites.

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
 remote.c |   53 ++++++++++++++++++++++++++++-------------------------
 1 files changed, 28 insertions(+), 25 deletions(-)

diff --git a/remote.c b/remote.c
index 4b5b905..ff8e71f 100644
--- a/remote.c
+++ b/remote.c
@@ -28,6 +28,11 @@ struct rewrite {
 	int instead_of_nr;
 	int instead_of_alloc;
 };
+struct rewrites {
+	struct rewrite **rewrite;
+	int rewrite_alloc;
+	int rewrite_nr;
+};
 
 static struct remote **remotes;
 static int remotes_alloc;
@@ -41,14 +46,12 @@ static struct branch *current_branch;
 static const char *default_remote_name;
 static int explicit_default_remote_name;
 
-static struct rewrite **rewrite;
-static int rewrite_alloc;
-static int rewrite_nr;
+static struct rewrites rewrites;
 
 #define BUF_SIZE (2048)
 static char buffer[BUF_SIZE];
 
-static const char *alias_url(const char *url)
+static const char *alias_url(const char *url, struct rewrites *r)
 {
 	int i, j;
 	char *ret;
@@ -57,14 +60,14 @@ static const char *alias_url(const char *url)
 
 	longest = NULL;
 	longest_i = -1;
-	for (i = 0; i < rewrite_nr; i++) {
-		if (!rewrite[i])
+	for (i = 0; i < r->rewrite_nr; i++) {
+		if (!r->rewrite[i])
 			continue;
-		for (j = 0; j < rewrite[i]->instead_of_nr; j++) {
-			if (!prefixcmp(url, rewrite[i]->instead_of[j].s) &&
+		for (j = 0; j < r->rewrite[i]->instead_of_nr; j++) {
+			if (!prefixcmp(url, r->rewrite[i]->instead_of[j].s) &&
 			    (!longest ||
-			     longest->len < rewrite[i]->instead_of[j].len)) {
-				longest = &(rewrite[i]->instead_of[j]);
+			     longest->len < r->rewrite[i]->instead_of[j].len)) {
+				longest = &(r->rewrite[i]->instead_of[j]);
 				longest_i = i;
 			}
 		}
@@ -72,10 +75,10 @@ static const char *alias_url(const char *url)
 	if (!longest)
 		return url;
 
-	ret = xmalloc(rewrite[longest_i]->baselen +
+	ret = xmalloc(r->rewrite[longest_i]->baselen +
 		     (strlen(url) - longest->len) + 1);
-	strcpy(ret, rewrite[longest_i]->base);
-	strcpy(ret + rewrite[longest_i]->baselen, url + longest->len);
+	strcpy(ret, r->rewrite[longest_i]->base);
+	strcpy(ret + r->rewrite[longest_i]->baselen, url + longest->len);
 	return ret;
 }
 
@@ -103,7 +106,7 @@ static void add_url(struct remote *remote, const char *url)
 
 static void add_url_alias(struct remote *remote, const char *url)
 {
-	add_url(remote, alias_url(url));
+	add_url(remote, alias_url(url, &rewrites));
 }
 
 static void add_pushurl(struct remote *remote, const char *pushurl)
@@ -169,22 +172,22 @@ static struct branch *make_branch(const char *name, int len)
 	return ret;
 }
 
-static struct rewrite *make_rewrite(const char *base, int len)
+static struct rewrite *make_rewrite(struct rewrites *r, const char *base, int len)
 {
 	struct rewrite *ret;
 	int i;
 
-	for (i = 0; i < rewrite_nr; i++) {
+	for (i = 0; i < r->rewrite_nr; i++) {
 		if (len
-		    ? (len == rewrite[i]->baselen &&
-		       !strncmp(base, rewrite[i]->base, len))
-		    : !strcmp(base, rewrite[i]->base))
-			return rewrite[i];
+		    ? (len == r->rewrite[i]->baselen &&
+		       !strncmp(base, r->rewrite[i]->base, len))
+		    : !strcmp(base, r->rewrite[i]->base))
+			return r->rewrite[i];
 	}
 
-	ALLOC_GROW(rewrite, rewrite_nr + 1, rewrite_alloc);
+	ALLOC_GROW(r->rewrite, r->rewrite_nr + 1, r->rewrite_alloc);
 	ret = xcalloc(1, sizeof(struct rewrite));
-	rewrite[rewrite_nr++] = ret;
+	r->rewrite[r->rewrite_nr++] = ret;
 	if (len) {
 		ret->base = xstrndup(base, len);
 		ret->baselen = len;
@@ -355,7 +358,7 @@ static int handle_config(const char *key, const char *value, void *cb)
 		subkey = strrchr(name, '.');
 		if (!subkey)
 			return 0;
-		rewrite = make_rewrite(name, subkey - name);
+		rewrite = make_rewrite(&rewrites, name, subkey - name);
 		if (!strcmp(subkey, ".insteadof")) {
 			if (!value)
 				return config_error_nonbool(key);
@@ -433,10 +436,10 @@ static void alias_all_urls(void)
 		if (!remotes[i])
 			continue;
 		for (j = 0; j < remotes[i]->url_nr; j++) {
-			remotes[i]->url[j] = alias_url(remotes[i]->url[j]);
+			remotes[i]->url[j] = alias_url(remotes[i]->url[j], &rewrites);
 		}
 		for (j = 0; j < remotes[i]->pushurl_nr; j++) {
-			remotes[i]->pushurl[j] = alias_url(remotes[i]->pushurl[j]);
+			remotes[i]->pushurl[j] = alias_url(remotes[i]->pushurl[j], &rewrites);
 		}
 	}
 }
-- 
1.6.3.3

^ permalink raw reply related

* [PATCH 2/2] Add url.<base>.pushInsteadOf: URL rewriting for push only
From: Josh Triplett @ 2009-09-07  7:00 UTC (permalink / raw)
  To: git, gitster
In-Reply-To: <cover.1252306396.git.josh@joshtriplett.org>

This configuration option allows systematically rewriting fetch-only
URLs to push-capable URLs when used with push.  For instance:

[url "ssh://example.org/"]
    pushInsteadOf = "git://example.org/"

This will allow clones of "git://example.org/path/to/repo" to
subsequently push to "ssh://example.org/path/to/repo", without manually
configuring pushurl for that remote.

Includes documentation for the new option, bash completion updates, and
test cases (both that pushInsteadOf applies to push and that it does
*not* apply to fetch).

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
 Documentation/config.txt               |   12 +++++++++++
 Documentation/urls.txt                 |   18 +++++++++++++++++
 contrib/completion/git-completion.bash |    2 +-
 remote.c                               |   33 +++++++++++++++++++++++--------
 t/t5516-fetch-push.sh                  |   31 ++++++++++++++++++++++++++++++
 5 files changed, 86 insertions(+), 10 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 5256c7f..726aa89 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1500,6 +1500,18 @@ url.<base>.insteadOf::
 	never-before-seen repository on the site.  When more than one
 	insteadOf strings match a given URL, the longest match is used.
 
+url.<base>.pushInsteadOf::
+	Any URL that starts with this value will not be pushed to;
+	instead, it will be rewritten to start with <base>, and the
+	resulting URL will be pushed to. In cases where some site serves
+	a large number of repositories, and serves them with multiple
+	access methods, some of which do not allow push, this feature
+	allows people to specify a pull-only URL and have git
+	automatically use an appropriate URL to push, even for a
+	never-before-seen repository on the site.  When more than one
+	pushInsteadOf strings match a given URL, the longest match is
+	used.
+
 user.email::
 	Your email address to be recorded in any newly created commits.
 	Can be overridden by the 'GIT_AUTHOR_EMAIL', 'GIT_COMMITTER_EMAIL', and
diff --git a/Documentation/urls.txt b/Documentation/urls.txt
index 5355ebc..d813ceb 100644
--- a/Documentation/urls.txt
+++ b/Documentation/urls.txt
@@ -67,3 +67,21 @@ For example, with this:
 a URL like "work:repo.git" or like "host.xz:/path/to/repo.git" will be
 rewritten in any context that takes a URL to be "git://git.host.xz/repo.git".
 
+If you want to rewrite URLs for push only, you can create a
+configuration section of the form:
+
+------------
+	[url "<actual url base>"]
+		pushInsteadOf = <other url base>
+------------
+
+For example, with this:
+
+------------
+	[url "ssh://example.org/"]
+		pushInsteadOf = git://example.org/
+------------
+
+a URL like "git://example.org/path/to/repo.git" will be rewritten to
+"ssh://example.org/path/to/repo.git" for pushes, but pulls will still
+use the original URL.
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index bf688e1..9859204 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1532,7 +1532,7 @@ _git_config ()
 	url.*.*)
 		local pfx="${cur%.*}."
 		cur="${cur##*.}"
-		__gitcomp "insteadof" "$pfx" "$cur"
+		__gitcomp "insteadOf pushInsteadOf" "$pfx" "$cur"
 		return
 		;;
 	esac
diff --git a/remote.c b/remote.c
index ff8e71f..6789786 100644
--- a/remote.c
+++ b/remote.c
@@ -47,6 +47,7 @@ static const char *default_remote_name;
 static int explicit_default_remote_name;
 
 static struct rewrites rewrites;
+static struct rewrites rewrites_push;
 
 #define BUF_SIZE (2048)
 static char buffer[BUF_SIZE];
@@ -104,17 +105,25 @@ static void add_url(struct remote *remote, const char *url)
 	remote->url[remote->url_nr++] = url;
 }
 
-static void add_url_alias(struct remote *remote, const char *url)
-{
-	add_url(remote, alias_url(url, &rewrites));
-}
-
 static void add_pushurl(struct remote *remote, const char *pushurl)
 {
 	ALLOC_GROW(remote->pushurl, remote->pushurl_nr + 1, remote->pushurl_alloc);
 	remote->pushurl[remote->pushurl_nr++] = pushurl;
 }
 
+static void add_pushurl_alias(struct remote *remote, const char *url)
+{
+	const char *pushurl = alias_url(url, &rewrites_push);
+	if (pushurl != url)
+		add_pushurl(remote, pushurl);
+}
+
+static void add_url_alias(struct remote *remote, const char *url)
+{
+	add_url(remote, alias_url(url, &rewrites));
+	add_pushurl_alias(remote, url);
+}
+
 static struct remote *make_remote(const char *name, int len)
 {
 	struct remote *ret;
@@ -358,8 +367,13 @@ static int handle_config(const char *key, const char *value, void *cb)
 		subkey = strrchr(name, '.');
 		if (!subkey)
 			return 0;
-		rewrite = make_rewrite(&rewrites, name, subkey - name);
 		if (!strcmp(subkey, ".insteadof")) {
+			rewrite = make_rewrite(&rewrites, name, subkey - name);
+			if (!value)
+				return config_error_nonbool(key);
+			add_instead_of(rewrite, xstrdup(value));
+		} else if (!strcmp(subkey, ".pushinsteadof")) {
+			rewrite = make_rewrite(&rewrites_push, name, subkey - name);
 			if (!value)
 				return config_error_nonbool(key);
 			add_instead_of(rewrite, xstrdup(value));
@@ -435,12 +449,13 @@ static void alias_all_urls(void)
 	for (i = 0; i < remotes_nr; i++) {
 		if (!remotes[i])
 			continue;
-		for (j = 0; j < remotes[i]->url_nr; j++) {
-			remotes[i]->url[j] = alias_url(remotes[i]->url[j], &rewrites);
-		}
 		for (j = 0; j < remotes[i]->pushurl_nr; j++) {
 			remotes[i]->pushurl[j] = alias_url(remotes[i]->pushurl[j], &rewrites);
 		}
+		for (j = 0; j < remotes[i]->url_nr; j++) {
+			remotes[i]->url[j] = alias_url(remotes[i]->url[j], &rewrites);
+			add_pushurl_alias(remotes[i], remotes[i]->url[j]);
+		}
 	}
 }
 
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 2d2633f..8f455c7 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -122,6 +122,23 @@ test_expect_success 'fetch with insteadOf' '
 	)
 '
 
+test_expect_success 'fetch with pushInsteadOf (should not rewrite)' '
+	mk_empty &&
+	(
+		TRASH=$(pwd)/ &&
+		cd testrepo &&
+		git config "url.trash/.pushInsteadOf" "$TRASH" &&
+		git config remote.up.url "$TRASH." &&
+		git config remote.up.fetch "refs/heads/*:refs/remotes/origin/*" &&
+		git fetch up &&
+
+		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
+		test "z$r" = "z$the_commit" &&
+
+		test 1 = $(git for-each-ref refs/remotes/origin | wc -l)
+	)
+'
+
 test_expect_success 'push without wildcard' '
 	mk_empty &&
 
@@ -162,6 +179,20 @@ test_expect_success 'push with insteadOf' '
 	)
 '
 
+test_expect_success 'push with pushInsteadOf' '
+	mk_empty &&
+	TRASH="$(pwd)/" &&
+	git config "url.$TRASH.pushInsteadOf" trash/ &&
+	git push trash/testrepo refs/heads/master:refs/remotes/origin/master &&
+	(
+		cd testrepo &&
+		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
+		test "z$r" = "z$the_commit" &&
+
+		test 1 = $(git for-each-ref refs/remotes/origin | wc -l)
+	)
+'
+
 test_expect_success 'push with matching heads' '
 
 	mk_test heads/master &&
-- 
1.6.3.3

^ permalink raw reply related

* Re: Issue 323 in msysgit: Can't clone over http
From: Junio C Hamano @ 2009-09-07  7:10 UTC (permalink / raw)
  To: Tay Ray Chuan; +Cc: git, msysgit, Tom Preston-Werner
In-Reply-To: <be6fef0d0909062253p1b86628et8a9f979952eebb00@mail.gmail.com>

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

> We should only be interested in the MISSING_TARGET error, because it
> tells us that the pack file is indeed not available. We aren't
> interested in other errors, like being unable to perform the request
> (HTTP_START_FAILED), or, say, a 401 (Unauthorized) error, or even a
> 500; we simply move along and we tell the user we couldn't perform the
> check.

We couldn't perform the check, and then what happens?  We continue as if
everything is peachy, assuming that the *.idx file we thought we were
going to get describe what objects are in the corresponding pack, and barf
when we try to read the *.idx file that we failed to download even though
the server said we should be able to get it?

> You're right to say that git before 39dc52c would have failed. It did,
> but no one had the chance to break anything, because 39dc52c was part
> of my http patch series that only went wild in v1.6.4.
>
> We can trace this back to 48188c2 ("http-walker: verify remote
> packs"), which copied the "feature" from http-push.c to http-walker.c.

Ahh, v1.6.3 ships with fetch_index() that checks CURLE_OK and returns an
error(), but that is about .idx file, and it did not have the "do they
have the corresponding .pack file?" check introduced by 48188c2
(http-walker: verify remote packs, 2009-06-06), which is what makes the
server give us 500 error.  Before that check, we ignored a request to
fetch_index() if we already had one.

Why do we even call fetch_index() when we have one?  After all, we are
talking about "git clone" here, so it is not about "we failed once and the
previous attempt left .idx files we fetched".  Why

should we even have .idx file to begin with, that would have protected
v1.6.3 clients from getting this error?

Unless we are calling fetch_index() on the same .idx file twice from our
own stupidity, that is.

The same logic now is in fetch_pack_index(), which is called from
fetch_and_setup_pack_index().  I do not still see how we end up calling
the function for the same .idx file twice, though.

The repository in question http://github.com/tekkub/addontemplate.git/
return one liner file when asked for $URL/objects/info/packs.

Which means that it is not like that the loop in http_get_info_packs() is
calling the fetch_and_setup_pack() twice because the server lists the same
pack twice.  But if your patch matters, somebody is causing us to call the
function twice for the same .idx file, and I do not see where it is.

There definitely is something else going on.

Having said all that, after digging some more, I came to the conclusion
that I'd rather not see us proceed with bug hunting, based on the failures
we see with the current github repositories over http.  Read on for the
reasons.

The github's URL responds to request for $URL/HEAD and tells us that it
points at refs/heads/master, but requests for $URL/packed-refs and
$URL/refs/heads/master go to somewhere completely unrelated to the
request, without giving any failure indication.

In order to support fetch/clone over HTTP, a server at least must respond
to requests to the follwoing locations sensibly, meaning that it gives us
back the data without frills if they exist, and give us Not found if they
don't.

 - $URL/objects/info/refs

   This must list all the refs available in the repository and must be
   up-to-date.  We do not run PROPFIND, nor parse $URL/refs/index.htm, but
   trust this file's contents and start from there.

 - $URL/objects/info/packs

   This must list all the packs in the repository and must be up-to-date.
   We do not run PROPFIND, nor parse $URL/objects/pack/index.htm, but
   trust this file's contents.  If the repository does not have any pack,
   request to this file must give us Not found.

 - $URL/packed-refs

   This may be a valid packed-refs file left after "git pack-refs".  If
   the repository's refs are not packed, the file may not exist, and that
   is Ok, but in that case, the request must give us Not found.

 - $URL/objects/pack/pack-*.pack and pack-*.idx

   If the repository lacks what we asked for , the request must result in Not
   found.

 - $URL/objects/[0-9a-f][0-9][a-f]/*

   Loose objects.  If the repository lacks what we asked for, the request
   must result in Not found.

It appears that github always redirects the request to some random project
page when Not found response is expected, which is very broken from the
point of view of git-fetch/git-clone.

I cannot tell offhand if it is just this "addontemplate.git" repository,
or if the way github arranges the URL space is fundamentally broken and
all of their repositories are unusable in exactly the same way (their URL
space seems to overlay UI pages meant for browsers over output meant to be
read by git-fetch/git-clone).

In either case, cloning over http from that "addontemplate" URL is not
expected to work right now, and the primary reason is that the server is
utterly misbehaving.

Hopefully that is a temporary breakage something github folks can promptly
fix.  Tom, could you have your server folks in touch with the git mailing
list, so that we can figure out what is going on?  Or are they already on
top of this issue and we can just wait (hopefully not for too long)?

In the meantime, I do not think it is a good idea to loosen the error
checking on our side to accomodate a server in such a (hopefully
temporary) broken state, however popular it is.

^ permalink raw reply

* Re: [BUG] 'add -u' doesn't work from untracked subdir
From: SZEDER Gábor @ 2009-09-07  7:33 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: Clemens Buchacher, Junio C Hamano, Jeff King, git
In-Reply-To: <vpqpra3p92g.fsf@bauges.imag.fr>

On Mon, Sep 07, 2009 at 08:23:19AM +0200, Matthieu Moy wrote:
> Clemens Buchacher <drizzd@aon.at> writes:
> 
> > On Sun, Sep 06, 2009 at 02:32:44PM +0200, Matthieu Moy wrote:
> >> I think it has already been proposed to introduce "git add -a" doing
> >> what "git add -u" do, but for the full tree.
> >
> > I like that, actually. AFAICT it's completely analogous to "git commit -a".
> > We also need something for "git add -A" though.
> >
> > Do you feel the same way about changing the behavior of "git grep"? I don't
> > really want to change the command's name.
> 
> I don't have particular feeling about "git grep", probably because I
> don't use it much. One argument in favour of keeping the current
> behavior is the consistancy with plain "grep".

I'm not sure how important should be the consistency with plain grep
in this case.  After all, plain grep does not work without pathspec at
all (ok, it searches stdin, but it's completely different thing),
while git grep does.  And plain grep is not recursive by default,
while git grep is.  And plain 'grep -r foo .' searches all files in
the current directory and below, while 'git grep foo .' does not
search in untracked files.


Regards,
Gábor

^ permalink raw reply

* Re: [PATCH 3/4] push: make non-fast-forward help message configurable
From: Johannes Sixt @ 2009-09-07  7:35 UTC (permalink / raw)
  To: Nanako Shiraishi
  Cc: Junio C Hamano, Jeff King, Matthieu Moy, Teemu Likonen, Git
In-Reply-To: <20090907094457.6117@nanako3.lavabit.com>

Nanako Shiraishi schrieb:
> Quoting Junio C Hamano <gitster@pobox.com>
> 
>> Speaking of which, has anybody felt annoyed by this message?
>>
>>     $ git reset --hard HEAD^^
>>     HEAD is now at 3fb9d58 Do not scramble password read from .cvspass
>>
>> This is not "maybe you should try this", but I would consider that it
>> falls into the same "I see you are trying to be helpful, but I know what I
>> am doing, and you are stealing screen real estate from me without helping
>> me at all, thank you very much" category.
> 
> You may be fixated at the sha1 part of the message when you find this
> message annoying, but I disagree strongly. I always appreciate the assurance
> this message gives me that I counted the number of commits correctly,
> whether I say HEAD^^^^ or HEAD~7.
> 
> Showing the subject of the commit you are now at is very useful and I will
> be equally irritated as you do if it starts showing the subject of the
> commit I was at.

I agree 100% with your reasoning.

-- Hannes

^ permalink raw reply

* Re: [BUG] 'add -u' doesn't work from untracked subdir
From: Clemens Buchacher @ 2009-09-07  7:50 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: Junio C Hamano, Jeff King, SZEDER Gbor, git
In-Reply-To: <20090907090713.6117@nanako3.lavabit.com>

On Mon, Sep 07, 2009 at 09:07:13AM +0900, Nanako Shiraishi wrote:

> The default behavior for 'git-grep' has already been discussed in length,
> and I don't think it is likely to change. See 
> 
>   http://thread.gmane.org/gmane.comp.version-control.git/111519/focus=111717

Actually, most responded with the request for a command or config option,
and did not refuse the idea outright. One was not even aware that this is
how "git grep" behaves. And neither was I until a few days ago.

And that is kind of dangerous with this command. You expect it to behave
analogously to other git commands, but it doesn't. And because grep simply
does not search superdirectories, you may think that there are in fact no
matches so you don't even notice that behavior!

> I think it is a good idea to fix this as an old regression in the maint
> branch. You don't have to introduce "git add -a". In fact the -a option
> was explicitly rejected when "git add -A" option was added with this
> commit. 
> 
>   3ba1f11 "git-add --all: add all files"
> 
> because "git commit -a" will never include new files and it will be inconsistent if "git add -a" did so.

I certainly don't mind fixing "git add -u". But I was not suggesting "git
add -a" instead of "git add -A". The idea was to introduce it instead of
"git add -u" (which can be deprecated later), so that the following are
exactly the same.

	"git add -a; git commit"
	"git commit -a"

That way, scripts are not silently broken.

OTOH, "git add --all" is already inconsistent with "git commit --all". And
we would still need a new command for 'global' "add -A". *sigh*

Clemens

^ permalink raw reply

* Re: [PATCH 2/2] Add url.<base>.pushInsteadOf: URL rewriting for push only
From: Junio C Hamano @ 2009-09-07  7:53 UTC (permalink / raw)
  To: Josh Triplett; +Cc: git
In-Reply-To: <0be9995dcd7d48c918fa75f4d9e557a6144a047c.1252306396.git.josh@joshtriplett.org>

Josh Triplett <josh@joshtriplett.org> writes:

> This configuration option allows systematically rewriting fetch-only
> URLs to push-capable URLs when used with push.  For instance:
>
> [url "ssh://example.org/"]
>     pushInsteadOf = "git://example.org/"
>
> This will allow clones of "git://example.org/path/to/repo" to
> subsequently push to "ssh://example.org/path/to/repo", without manually
> configuring pushurl for that remote.

Nice.

> @@ -435,12 +449,13 @@ static void alias_all_urls(void)
>  	for (i = 0; i < remotes_nr; i++) {
>  		if (!remotes[i])
>  			continue;
> -		for (j = 0; j < remotes[i]->url_nr; j++) {
> -			remotes[i]->url[j] = alias_url(remotes[i]->url[j], &rewrites);
> -		}
>  		for (j = 0; j < remotes[i]->pushurl_nr; j++) {
>  			remotes[i]->pushurl[j] = alias_url(remotes[i]->pushurl[j], &rewrites);
>  		}
> +		for (j = 0; j < remotes[i]->url_nr; j++) {
> +			remotes[i]->url[j] = alias_url(remotes[i]->url[j], &rewrites);
> +			add_pushurl_alias(remotes[i], remotes[i]->url[j]);
> +		}

Even if you have URL but not pushURL, now you get a corresponding pushURL
for free by just adding pushinsteadof mapping that covers the URL without
having to configue pushURL for each of them.

What happens if you already had a pair of concrete url and pushurl defined
for one of your repositories (say git://git.kernel.org/pub/scm/git/git.git
for fetch, ssh://x.kernel.org/pub/scm/git/git.git for push) at a site, and
then upon seeing this new feature, added a pushinsteadof pattern that also
covers the URL side of that pair (e.g. everything in git://git.kernel.org/
is mapped to x.kernel.org:/ namespsace)?

Do you end up pushing to both (e.g. ssh://x.kernel.org/pub/scm/git/git.git
and x.kernel.org:/pub/scm/git/git.git), or in such a case, the pushURL you
gave explicitly prevents the pushinsteadof to give unexpected duplicates?

^ permalink raw reply

* Re: [PATCH 3/4] push: make non-fast-forward help message configurable
From: Mike Hommey @ 2009-09-07  7:40 UTC (permalink / raw)
  To: Nanako Shiraishi
  Cc: Junio C Hamano, Jeff King, Matthieu Moy, Teemu Likonen, Git
In-Reply-To: <20090907094457.6117@nanako3.lavabit.com>

On Mon, Sep 07, 2009 at 09:44:57AM +0900, Nanako Shiraishi wrote:
> Quoting Junio C Hamano <gitster@pobox.com>
> 
> > Speaking of which, has anybody felt annoyed by this message?
> >
> >     $ git reset --hard HEAD^^
> >     HEAD is now at 3fb9d58 Do not scramble password read from .cvspass
> >
> > This is not "maybe you should try this", but I would consider that it
> > falls into the same "I see you are trying to be helpful, but I know what I
> > am doing, and you are stealing screen real estate from me without helping
> > me at all, thank you very much" category.
> 
> You may be fixated at the sha1 part of the message when you find this message annoying, but I disagree strongly. I always appreciate the assurance this message gives me that I counted the number of commits correctly, whether I say HEAD^^^^ or HEAD~7.
> 
> Showing the subject of the commit you are now at is very useful and I will be equally irritated as you do if it starts showing the subject of the commit I was at.

I'd say both are equally interesting information.

Mike

^ permalink raw reply

* Re: [BUG] 'add -u' doesn't work from untracked subdir
From: Matthieu Moy @ 2009-09-07  8:06 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: Clemens Buchacher, Junio C Hamano, Jeff King, git
In-Reply-To: <20090907073322.GA6021@neumann>

SZEDER Gábor <szeder@ira.uka.de> writes:

>> I don't have particular feeling about "git grep", probably because I
>> don't use it much. One argument in favour of keeping the current
>> behavior is the consistancy with plain "grep".
>
> I'm not sure how important should be the consistency with plain grep
> in this case.  After all, plain grep does not work without pathspec at
> all

Yes, right, forget what I wrote, except the "I don't have particular
feeling about 'git grep'" part ;-).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* Re: tracking branch for a rebase
From: Michael J Gruber @ 2009-09-07  8:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Björn Steinbrink, Pete Wyckoff, git
In-Reply-To: <7vfxaz9wfi.fsf@alter.siamese.dyndns.org>

Junio C Hamano venit, vidit, dixit 07.09.2009 07:05:
> Jeff King <peff@peff.net> writes:
> 
>>> Hm, I'd prefer a shorthand for "upstream for this branch", instead of
>>> magic defaults.
>>
>> The more I think about, the more I think that is the right solution.
>> Because magic defaults for "rebase -i" don't help when you want to do
>> "gitk $UPSTREAM..".
>>
>> The previous discussion on the topic seems to be here:
>>
>>   http://article.gmane.org/gmane.comp.version-control.git/113666
>>
>> And apparently you and I both participated in the discussion, which I
>> totally forgot about.
>>
>> Looks like the discussion ended with people liking the idea but not
>> knowing what the specifier should look like. Maybe tightening the ref
>> syntax a bit to allow more extensible "special" refs is a good v1.7.0
>> topic? I dunno.
> 
> At-mark currently is reserved for anything that uses reflog, but we can
> say that it is to specify operations on refs (as opposed to caret and
> tilde are to specify operations on object names).
> 
> It specifies what ref to work on with the operand on its left side (and an
> empty string stands for "HEAD"), and what operation is done to it by what
> is in {} on the right side of it.  This view is quite consistent with the
> following existing uses of the notation:
> 
> 	ref@{number}	-- nth reflog entry
>         ref@{time}	-- ref back then
> 	@{-number}	-- nth branch switching
> 
> So perhaps ref@{upstream}, or any string that is not a number and cannot
> be time, can trigger the magic operation on the ref with ref@{magic}
> syntax?

Even @{} is not taken so far... Alternatively, most people associate '^'
with 'up', just the way we use it for "upwards parentship" ref^ (and
somewhat the way we use it for upwards/backwards tag reference
relationship resolving ref^{type}), so
@^
or
@{^}
would be an option. Read "at upstream" :)

Michael

^ permalink raw reply

* Re: Deleted folder keeps showing up?
From: Benjamin Buch @ 2009-09-07  8:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Michael J Gruber, git
In-Reply-To: <7vfxb3yuma.fsf@alter.siamese.dyndns.org>

Hi Junio,

thanks for the explanation!

It made things a lot clearer for me.
And I think that could be exactly what happened
and the reason why I couldn't reproduce the 'error'.

-benjamin

Am 04.09.2009 um 10:36 schrieb Junio C Hamano:

> Benjamin Buch <benni.buch@gmx.de> writes:
>
>> Strangely, I can't reproduce the error  today.
>
> If a branch has dir/file tracked, and another branch does not have
> anything tracked in dir/ directory at all, then switching from the  
> former
> branch to the latter can remove dir/ only when you do not have any
> untracked files in there when you switch.  Otherwise dir/ must stay
> behind to keep the untracked files.
>
> You can see it by a simple experiment.
>
>    $ rm -fr trial
>    $ mkdir trial
>    $ cd trial
>    $ git init
>    $ >elif
>    $ git commit -m initial
>    $ git branch lacksdir
>    $ mkdir dir
>    $ >dir/file
>    $ git add dir/file
>    $ git commit -m add-dir-file
>
> Now, after this set-up, your 'master' has dir/file and 'lacksdir' does
> not have anything tracked in dir/ directory.
>
> Observe:
>
>    $ git checkout lacksdir
>    $ find ??*
>    elif
>    $ git checkout master
>    $ find ??*
>    dir
>    dir/file
>    elif
>    $ >dir/garbage
>    $ git checkout lacksdir
>    $ find ??*
>    dir
>    dir/garbage
>    elif
>
> If switching to 'lacksdir' removed the dir/ directory, whatever was  
> in the
> untracked file dir/garbage will be lost.  In the above exercise, I  
> named
> it garbage, so a casual reader might get a false impression that it  
> should
> be thrown away, but in real life workflow, it often happens that
>
> (1) you start doing some interesting experimental changes, while on
>     'master';
>
> (2) you realize that this change does not belong to 'master', but  
> belongs
>     to some other branch, perhaps 'lacksdir';
>
> (3) you switch to the branch, to keep working.
>
> Remember that, in git, your uncommitted changes to the index and the  
> work
> tree do not belong to the branch.  They try to follow you across  
> branch
> switching.  Since untracked new files are something potentially you  
> might
> want to add after branch switching, we do not remove them.  And  
> because we
> choose not to remove dir/file, even though the commit at the tip of  
> the
> lacksdir branch does not have anything tracked in dir/ directory, we
> cannot remove it from the work tree.
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: Issue 323 in msysgit: Can't clone over http
From: Junio C Hamano @ 2009-09-07  8:18 UTC (permalink / raw)
  To: Tay Ray Chuan; +Cc: Junio C Hamano, git, msysgit, Tom Preston-Werner
In-Reply-To: <7vocpn44dg.fsf@alter.siamese.dyndns.org>

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

> In order to support fetch/clone over HTTP, a server at least must respond
> to requests to the follwoing locations sensibly, meaning that it gives us
> back the data without frills if they exist, and give us Not found if they
> don't.
>
>  - $URL/objects/info/refs
>
>    This must list all the refs available in the repository and must be
>    up-to-date.  We do not run PROPFIND, nor parse $URL/refs/index.htm, but
>    trust this file's contents and start from there.

Correction.  This is $URL/info/refs and the github repository in question
does respond correctly.  Sorry about the confusion.

Recent code makes CURLOPT_NOBODY request, which will turn into a HEAD
request over HTTP, to packfiles, in order to see if they exist.  Perhaps
github is not prepared to handle that and returns 500, even though it will
give the pack correctly if asked with a GET request?

^ permalink raw reply

* Re: [PATCH 2/2] Add url.<base>.pushInsteadOf: URL rewriting for push only
From: Josh Triplett @ 2009-09-07  8:19 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vljkr2ntd.fsf@alter.siamese.dyndns.org>

On Mon, Sep 07, 2009 at 12:53:18AM -0700, Junio C Hamano wrote:
> Josh Triplett <josh@joshtriplett.org> writes:
> > This configuration option allows systematically rewriting fetch-only
> > URLs to push-capable URLs when used with push.  For instance:
> >
> > [url "ssh://example.org/"]
> >     pushInsteadOf = "git://example.org/"
> >
> > This will allow clones of "git://example.org/path/to/repo" to
> > subsequently push to "ssh://example.org/path/to/repo", without manually
> > configuring pushurl for that remote.
> 
> Nice.

Thanks.

> > @@ -435,12 +449,13 @@ static void alias_all_urls(void)
> >  	for (i = 0; i < remotes_nr; i++) {
> >  		if (!remotes[i])
> >  			continue;
> > -		for (j = 0; j < remotes[i]->url_nr; j++) {
> > -			remotes[i]->url[j] = alias_url(remotes[i]->url[j], &rewrites);
> > -		}
> >  		for (j = 0; j < remotes[i]->pushurl_nr; j++) {
> >  			remotes[i]->pushurl[j] = alias_url(remotes[i]->pushurl[j], &rewrites);
> >  		}
> > +		for (j = 0; j < remotes[i]->url_nr; j++) {
> > +			remotes[i]->url[j] = alias_url(remotes[i]->url[j], &rewrites);
> > +			add_pushurl_alias(remotes[i], remotes[i]->url[j]);
> > +		}
> 
> Even if you have URL but not pushURL, now you get a corresponding pushURL
> for free by just adding pushinsteadof mapping that covers the URL without
> having to configue pushURL for each of them.

Yes, exactly.

> What happens if you already had a pair of concrete url and pushurl defined
> for one of your repositories (say git://git.kernel.org/pub/scm/git/git.git
> for fetch, ssh://x.kernel.org/pub/scm/git/git.git for push) at a site, and
> then upon seeing this new feature, added a pushinsteadof pattern that also
> covers the URL side of that pair (e.g. everything in git://git.kernel.org/
> is mapped to x.kernel.org:/ namespsace)?
> 
> Do you end up pushing to both (e.g. ssh://x.kernel.org/pub/scm/git/git.git
> and x.kernel.org:/pub/scm/git/git.git), or in such a case, the pushURL you
> gave explicitly prevents the pushinsteadof to give unexpected duplicates?

You get a duplicate:

~$ grep -B1 steadOf .gitconfig
[url "ssh://joshtriplett.org/"]
        pushInsteadOf="git://joshtriplett.org/"
~$ grep -B1 url .git/config
[remote "origin"]
        url = git://joshtriplett.org/git/home.git
        pushurl = ssh://joshtriplett.org/git/home.git
~$ ~/src/git/git push
Everything up-to-date
Everything up-to-date

Initially, that behavior seemed pretty reasonable to me; nothing else in
the remotes handling attempts to remove duplicates, and they seem
harmless enough and easily resolved by removing one or the other.  Now
that I think about it, though, an explicit pushurl should definitely
disable pushInsteadOf's implicit pushurls.  If you explicitly configure
a *different* pushurl for a remote, you may not *want* the default
pushurl that corresponds to your url.  For instance, consider what would
happen if you configure url to point to the main public repository and
pushurl to point to a private repository.  In this case, you definitely
don't want "git push" to helpfully push to the public repository as
well; if you do, you can easily enough add a second pushurl for that.

I can easily change the patch to make an explicit pushurl disable
pushInsteadOf.  Expect v2 shortly.

- Josh Triplett

^ permalink raw reply

* Re: [PATCH 3/4] push: make non-fast-forward help message configurable
From: Jeff King @ 2009-09-07  8:24 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: Junio C Hamano, Matthieu Moy, Teemu Likonen, Git
In-Reply-To: <20090907094457.6117@nanako3.lavabit.com>

On Mon, Sep 07, 2009 at 09:44:57AM +0900, Nanako Shiraishi wrote:

> > Speaking of which, has anybody felt annoyed by this message?
> >
> >     $ git reset --hard HEAD^^
> >     HEAD is now at 3fb9d58 Do not scramble password read from .cvspass
> >
> > This is not "maybe you should try this", but I would consider that it
> > falls into the same "I see you are trying to be helpful, but I know what I
> > am doing, and you are stealing screen real estate from me without helping
> > me at all, thank you very much" category.
> 
> You may be fixated at the sha1 part of the message when you find this
> message annoying, but I disagree strongly. I always appreciate the
> assurance this message gives me that I counted the number of commits
> correctly, whether I say HEAD^^^^ or HEAD~7.

Let me add a "me too" to Nanako's comments. This assurance has actually
saved me in the past from accidentally going to the wrong commit (just
the other day I did a rebase followed by "git reset --hard HEAD@{1}",
when of course what I meant was "git reset --hard master@{1}".

I think this type of message is different from the other "advice"
messages.

In the case of the push non-fast-forward message and the status "here is
how you stage" comments, those messages are not specific to this exact
situation. They are general advice for "if you do not understand or need
a reminder of how git works, this is it." Experienced users know how git
works, so the messages are just clutter.

This message, on the other hand, tells you about this _specific_
instance. So even if you have mastered git, the information can reassure
you that you have gone to the intended commit (and yes, I have actually
gone to the wrong commit before, noticed it via this reset message, and
corrected the situation).

So really they are two different conceptual types of message. And while
I have no problem with an argument of "I _personally_ find this clutter
and would like to configure it off", I don't think such an option should
go under "advice.*". My patch had "message.all" (which will become
"advice.all") to turn off all advice messages, which can act as a sort
of "I am an expert" switch. But because this type of message is
conceptually different, it should not be lumped in with the others.

OTOH, I am open to arguments against "advice.all"; maybe it is a good
thing for users to manually say "this message is annoying me, and
therefore I am now an expert in this particular area". It's not like
there are more than two. ;)

-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