Git development
 help / color / mirror / Atom feed
* Re: [1.8.0] Provide proper remote ref namespaces
From: Jeff King @ 2011-02-07  5:18 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Junio C Hamano, Johan Herland, git, Sverre Rabbelier,
	Nguyen Thai Ngoc Duy
In-Reply-To: <alpine.LFD.2.00.1102051449420.12104@xanadu.home>

On Sat, Feb 05, 2011 at 02:55:06PM -0500, Nicolas Pitre wrote:

> > The latter seems like a regression for the common case of fetching from
> > two upstreams. E.g., I usually pull from Junio, getting
> > remotes/origin/v1.7.0.  One day Shawn is the interim maintainer, and I
> > pull from him, getting remotes/spearce/v1.7.0, which he previously
> > fetched from Junio. Under the current code, I can still do "git show
> > v1.7.0"; under the scheme described above I now have to say
> > "origin/v1.7.0" to disambiguate.
> 
> Let's suppose that both tags are identical, as in your scenario above 
> they would be, then there is no need to call for any ambiguity in that 
> case.

Agreed, but...

> > The real issue, I think, is that we are claiming ambiguity even though
> > those tags almost certainly point to the same sha1. When handling
> > ambiguous tags, should we perhaps check to see if all of the ambiguities
> > point to the same sha1, and in that case, just pick one at random?
> 
> If they're identical then there is no randomness.  If they refer to 
> different tag objects, even if those tag objects do refer to the same 
> commit object, then I'd say there is an ambiguity only if the tag object 
> content matters i.e. when displaying the tag content.

My gut feeling is that they should point to the same tag object, for the
sake of simplicity (if you are re-tagging a commit under the same name,
wouldn't I want to know?) and efficiency (we can detect non-ambiguity
just by looking at the sha1 values without opening objects).

But more importantly, don't we sometimes care where the ref came from?
If I say "git push remote v1.7.4" we do some automagic on the
destination side of the refspec based on the fact that the source ref
was found in the refs/tags hierarchy. In the case we're talking about,
all of the ambiguous refs would presumably also be coming from
refs/remotes/*/tags/, so they would be functionally equivalent. But I
wanted to point it out because:

  1. It is an additional equivalent requirement for two refs to not be
     ambiguous. They must have the same sha1, _and_ they must have the
     same "type".

  2. I couldn't think of any other cases where the actual refname might
     matter and this would break. But I wanted to bring it up explicitly
     in case somebody else can think of one.

-Peff

^ permalink raw reply

* Re: "git add -u" broken in git 1.7.4?
From: Junio C Hamano @ 2011-02-07  5:50 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: Sebastian Pipping, Jeff King, Git ML
In-Reply-To: <vpq1v3kopn3.fsf@bauges.imag.fr>

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Sebastian Pipping <webmaster@hartwork.org> writes:
>
>> I was and I can confirm the different behaviour with 1.7.4 over here: it
>> does work on the root directory of the repo as you supposed.
>
> What do you mean by "it does not work"?
>
> "git add -u" adds files under the current directory, and it always
> did.

As it takes pathspecs (think "git add -u this-file"), it fundamentally
shouldn't be tree-wide.  I think the original implementation didn't take
pathspecs and was mistakenly done as tree-wide operation, but I think it
was fixed rather quickly.

^ permalink raw reply

* Re: "git add -u" broken in git 1.7.4?
From: Jeff King @ 2011-02-07  5:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Matthieu Moy, Sebastian Pipping, Git ML
In-Reply-To: <7vwrlcv1ea.fsf@alter.siamese.dyndns.org>

On Sun, Feb 06, 2011 at 09:50:37PM -0800, Junio C Hamano wrote:

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> 
> > Sebastian Pipping <webmaster@hartwork.org> writes:
> >
> >> I was and I can confirm the different behaviour with 1.7.4 over here: it
> >> does work on the root directory of the repo as you supposed.
> >
> > What do you mean by "it does not work"?
> >
> > "git add -u" adds files under the current directory, and it always
> > did.
> 
> As it takes pathspecs (think "git add -u this-file"), it fundamentally
> shouldn't be tree-wide.  I think the original implementation didn't take
> pathspecs and was mistakenly done as tree-wide operation, but I think it
> was fixed rather quickly.

Is "git add -p" broken, then? It takes pathspecs relative to the current
directory, but "git add -p" without arguments operates from the root,
not from the current subdirectory.

-Peff

^ permalink raw reply

* Re: [PATCH 1.8.0] add: make "add -u" update full tree without pathspec
From: Junio C Hamano @ 2011-02-07  5:58 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: git, Junio C Hamano, Sebastian Pipping, SZEDER Gábor,
	Matthieu, "Moy <Matthieu.Moy"
In-Reply-To: <1297045643-26697-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> When -u was introduced in dfdac5d (git-add -u: match the index with
> working tree., 2007-04-20), "add -u" (without pathspec) added
> everything. Shortly after, 2ed2c22 (git-add -u paths... now works from
> subdirectory, 2007-08-16) broke it while fixing something related.

As long as the command takes pathspecs, it should never be tree-wide.
Making it tree-wide when there is no pathspec is even worse.

If "add -p" does not limit operation to the current directory, probably
that is the inconsistency bug to be fixed.

^ permalink raw reply

* Re: [PATCH 1.8.0] add: make "add -u" update full tree without pathspec
From: Nguyen Thai Ngoc Duy @ 2011-02-07  6:14 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Sebastian Pipping, SZEDER Gábor, Matthieu, Matthieu.Moy
In-Reply-To: <7vsjw0v11p.fsf@alter.siamese.dyndns.org>

On Mon, Feb 7, 2011 at 12:58 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> When -u was introduced in dfdac5d (git-add -u: match the index with
>> working tree., 2007-04-20), "add -u" (without pathspec) added
>> everything. Shortly after, 2ed2c22 (git-add -u paths... now works from
>> subdirectory, 2007-08-16) broke it while fixing something related.
>
> As long as the command takes pathspecs, it should never be tree-wide.
> Making it tree-wide when there is no pathspec is even worse.

git log -p and diff family all can take pathspecs. All default to
tree-wide without pathspecs. This is what I'm doing all the time:

git diff
# checking, ok, looks good
git add -u
# ack, need to come to root dir first
-- 
Duy

^ permalink raw reply

* Re: [PATCH 1.8.0] add: make "add -u" update full tree without pathspec
From: Miles Bader @ 2011-02-07  6:26 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy
  Cc: Junio C Hamano, git, Sebastian Pipping, SZEDER Gábor,
	Matthieu, Matthieu.Moy
In-Reply-To: <AANLkTimG6s5vSCnisJCt-EXkcJGXfY4Y3WtdB_netU08@mail.gmail.com>

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> git log -p and diff family all can take pathspecs. All default to
> tree-wide without pathspecs. This is what I'm doing all the time:
>
> git diff
> # checking, ok, looks good
> git add -u
> # ack, need to come to root dir first

Not to mention "git status", which shows everything in the tree when
given no args, but will only show changes underneath the cwd if given
"." as an argument.

I agree that the "default to root of git dir when given no args"
behavior seems both more convenient -- it's annoying to specify the root
of the tree explicitly, but very easy to specify the cwd explicitly
(".") -- and more intuitive -- If I want "everything underneath cwd",
I'll naturually specify "." as an argument; if I give no args, I almost
certainly want the whole tree, no matter where I happen to be in it.

-Miles

-- 
Christian, n. One who follows the teachings of Christ so long as they are not
inconsistent with a life of sin.

^ permalink raw reply

* Re: "git add -u" broken in git 1.7.4?
From: Junio C Hamano @ 2011-02-07  6:46 UTC (permalink / raw)
  To: Jeff King; +Cc: Matthieu Moy, Sebastian Pipping, Git ML
In-Reply-To: <20110207055314.GA5511@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Is "git add -p" broken, then? It takes pathspecs relative to the current
> directory, but "git add -p" without arguments operates from the root,
> not from the current subdirectory.

I would say so; "add -p" was an ill-executed afterthought.  The codepath
was originally meant to be used from "-i" as the top-level interface that
was a fully interactive way to prepare for the next commit, which is an
operation that is inherently full-tree.

There are two schools of thought in previous threads discussing full-tree
vs current-directory-relative.  I think each side has merits.

If we defaulted to the current directory (i.e. "git grep"), that would
feel more natural as it is more consistent with how tools that are not git
aware (e.g. "GNU grep" run in the same directory) behave.  A downside is
when you are somewhere deep in a working tree, you have to know how deep
you are and repeat "../" that many times, i.e. "git grep pattern ../../"

If we defaulted to the root-level (i.e. "git diff"), you do not have that
downside (iow, "git diff" run from a deep directory is a full tree
operation), and you can limit the scope to the current directory by a
single dot, i.e. "git diff .".  A huge downside is that this may feel
awkward for new people who do not yet breath git [*1*], as no other git
aware tool would behave like this, limiting its scope to some directory
that is higher above.

In the past, I have took the third position, saying that tools that
semantically needs to be full-tree should be full-tree (i.e. ones that
make or format commits), and others should be relative to the current
directory (i.e. ones that are used to inspect your progress, such as
grep), but that is not a very understandable guideline that people can
easily follow.  If we have to choose between the two and make things
consistent, my personal preference is to make everything relative to the
current working directory.

I actually do not mind too much myself if all commands that can take
pathspecs consistently defaulted to "full-tree" pathspec given no
pathspec.  But if we were to go that route, everybody should join their
voice to defend that decision when outside people say "in 1.8.0 'git grep'
run from a subdirectory shows matches from all the irrelevant parts of the
tree; with all the cruft its output is unreadable". I won't be the sole
champion of such a behaviour when I do not fully believe in it.


[Footnote]

*1* In the case of "git diff", this is largely mitigated as its output is
always relative to the root of the working tree, but other tools may not
have that luxury.

^ permalink raw reply

* Re: "git add -u" broken in git 1.7.4?
From: Matthieu Moy @ 2011-02-07  6:48 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Sebastian Pipping, Git ML
In-Reply-To: <20110207055314.GA5511@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Sun, Feb 06, 2011 at 09:50:37PM -0800, Junio C Hamano wrote:
>
>> As it takes pathspecs (think "git add -u this-file"), it fundamentally
>> shouldn't be tree-wide.  I think the original implementation didn't take
>> pathspecs and was mistakenly done as tree-wide operation, but I think it
>> was fixed rather quickly.
>
> Is "git add -p" broken, then? It takes pathspecs relative to the current
> directory, but "git add -p" without arguments operates from the root,
> not from the current subdirectory.

It's not just "git add -p". Take "git log", "git status", "git
commit", "git diff" ... well, most Git commands taking pathspecs
optionally:

git foo   => tree-wide
git foo . => the . acts as a path limiter

and this is the right thing to do. Making "git foo" equivalent to "git
foo ." makes it hard to recover the tree-wide behavior from a
subdirectory (git foo ../../../).

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

^ permalink raw reply

* Re: [1.8.0] Provide proper remote ref namespaces
From: Matthieu Moy @ 2011-02-07  6:54 UTC (permalink / raw)
  To: Johan Herland
  Cc: git, Dmitry Potapov, Junio C Hamano, Sverre Rabbelier, Jeff King,
	Nguyen Thai Ngoc Duy, Nicolas Pitre
In-Reply-To: <201102070451.37370.johan@herland.net>

Johan Herland <johan@herland.net> writes:

> When you push to "remoteB", you would say "git push remoteB tag v1.7.4", and 
> it would resolve "v1.7.4" (via "refs/remotes/remoteA/tags/v1.7.4") into the 
> appropriate SHA-1, and then push the "v1.7.4" tag to the remote.

That means I can fetch git-gui/v1.0, gitk/v1.1 and git/v1.7, my
subproject tags are well sorted in my local repository, but if they
don't have the same shortname, they're not "ambiguous", and the
namespace will be completely flattened when I push (i.e. I'll push
v1.0, v1.1 and v1.7, and people accessing my repository will not be
able to say whether they refer to gitk, git-gui or git versions).

This defeats the point in having proper namespaces, and we'll still
see handmade namespaces like we already have now for git-gui tags in
git.git.

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

^ permalink raw reply

* Re: "git add -u" broken in git 1.7.4?
From: Nguyen Thai Ngoc Duy @ 2011-02-07  7:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Matthieu Moy, Sebastian Pipping, Git ML
In-Reply-To: <7vhbcguytf.fsf@alter.siamese.dyndns.org>

On Mon, Feb 7, 2011 at 1:46 PM, Junio C Hamano <gitster@pobox.com> wrote:
> I actually do not mind too much myself if all commands that can take
> pathspecs consistently defaulted to "full-tree" pathspec given no
> pathspec.  But if we were to go that route, everybody should join their
> voice to defend that decision when outside people say "in 1.8.0 'git grep'
> run from a subdirectory shows matches from all the irrelevant parts of the
> tree; with all the cruft its output is unreadable". I won't be the sole
> champion of such a behaviour when I do not fully believe in it.

That could be one more item for the next git survey (i.e. how do you
want the defaults to be?). Most of people in this list more or less
breath git already and therefore are bias (I think).

Personally "git add -u --full-tree" is good enough to me. It does have
the same problem that git --full-tree has: what *.h in "git grep
--full-tree -- '*.h'" means. But I'm OK with not supporting that case
until we agree on something.
-- 
Duy

^ permalink raw reply

* Re: [PATCH] git pull: Remove option handling done by fetch
From: Jonathan Nieder @ 2011-02-07  7:41 UTC (permalink / raw)
  To: Jens Lehmann; +Cc: Junio C Hamano, Johannes Sixt, Git Mailing List
In-Reply-To: <4D4F273C.8030003@web.de>

Jens Lehmann wrote:

> 2) Document "--[no-]recurse-submodules" as " as "git pull" options
>
> (And then I can later pass the same option to "git merge", which is
> much better than the solutions I came up with ;-)

Hmm. :)

[...]
> --- a/Documentation/git-pull.txt
> +++ b/Documentation/git-pull.txt
> @@ -84,6 +84,10 @@ must be given before the options meant for 'git fetch'.
>  --verbose::
>  	Pass --verbose to git-fetch and git-merge.
> 
> +--[no-]recurse-submodules::
> +	This option controls if new commits of all populated submodules should
> +	be fetched too (see linkgit:git-config[1] and linkgit:gitmodules[5]).
> +

Is it worth mentioning that this does not (yet) automatically check
out the new commits in submodules after a merge, or would such
documentation be too likely to be forgotten and left stale in the
future?

^ permalink raw reply

* [PATCH] correct type of EMPTY_TREE_SHA1_BIN
From: Jonathan Nieder @ 2011-02-07  8:17 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Nguyễn Thái Ngọc Duy, git, Ilari Liusvaara,
	Jakub Narebski, Dmitry S. Kravtsov, Shawn Pearce
In-Reply-To: <7v62swwq7s.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:

>> --- a/cache-tree.c
>> +++ b/cache-tree.c
>> @@ -621,9 +621,18 @@ static void prime_cache_tree_rec(struct cache_tree *it, struct tree *tree)
>>  			struct tree *subtree = lookup_tree(entry.sha1);
>>  			if (!subtree->object.parsed)
>>  				parse_tree(subtree);
>> +			if (!hashcmp(entry.sha1, (unsigned char *)EMPTY_TREE_SHA1_BIN)) {
>> +				warning("empty tree detected! Will be removed in new commits");
>> +				cnt = -1;
>> +				break;
>> +			}
>
> You shouldn't need the cast (if you did, then hashcmp() macro should be
> fixed so that you don't need to).

Isn't this a bug in the definition of EMPTY_TREE_SHA1_BIN rather than
the signature of hashcmp?

-- 8< --
Subject: correct type of EMPTY_TREE_SHA1_BIN

Functions such as hashcmp that expect a binary SHA-1 value take
parameters of type "unsigned char *" to avoid accepting a textual
SHA-1 passed by mistake.  Unfortunately, this means passing the string
literal EMPTY_TREE_SHA1_BIN requires an ugly cast.  Tweak the
definition of EMPTY_TREE_SHA1_BIN to produce a value of more
convenient type.

In the future the definition might change to

	extern const unsigned char empty_tree_sha1_bin[20];
	#define EMPTY_TREE_SHA1_BIN empty_tree_sha1_bin

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 builtin/checkout.c |    2 +-
 cache.h            |    4 +++-
 notes-merge.c      |    2 +-
 sha1_file.c        |    2 +-
 4 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 757f9a0..cd7f56e 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -404,7 +404,7 @@ static int merge_working_tree(struct checkout_opts *opts,
 		topts.dir->exclude_per_dir = ".gitignore";
 		tree = parse_tree_indirect(old->commit ?
 					   old->commit->object.sha1 :
-					   (unsigned char *)EMPTY_TREE_SHA1_BIN);
+					   EMPTY_TREE_SHA1_BIN);
 		init_tree_desc(&trees[0], tree->buffer, tree->size);
 		tree = parse_tree_indirect(new->commit->object.sha1);
 		init_tree_desc(&trees[1], tree->buffer, tree->size);
diff --git a/cache.h b/cache.h
index d83d68c..3abf895 100644
--- a/cache.h
+++ b/cache.h
@@ -676,9 +676,11 @@ static inline void hashclr(unsigned char *hash)
 
 #define EMPTY_TREE_SHA1_HEX \
 	"4b825dc642cb6eb9a060e54bf8d69288fbee4904"
-#define EMPTY_TREE_SHA1_BIN \
+#define EMPTY_TREE_SHA1_BIN_LITERAL \
 	 "\x4b\x82\x5d\xc6\x42\xcb\x6e\xb9\xa0\x60" \
 	 "\xe5\x4b\xf8\xd6\x92\x88\xfb\xee\x49\x04"
+#define EMPTY_TREE_SHA1_BIN \
+	 ((const unsigned char *) EMPTY_TREE_SHA1_BIN_LITERAL)
 
 int git_mkstemp(char *path, size_t n, const char *template);
 
diff --git a/notes-merge.c b/notes-merge.c
index 71c4d45..1467ad3 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -615,7 +615,7 @@ int notes_merge(struct notes_merge_options *o,
 	bases = get_merge_bases(local, remote, 1);
 	if (!bases) {
 		base_sha1 = null_sha1;
-		base_tree_sha1 = (unsigned char *)EMPTY_TREE_SHA1_BIN;
+		base_tree_sha1 = EMPTY_TREE_SHA1_BIN;
 		OUTPUT(o, 4, "No merge base found; doing history-less merge");
 	} else if (!bases->next) {
 		base_sha1 = bases->item->object.sha1;
diff --git a/sha1_file.c b/sha1_file.c
index d86a8db..b44fc95 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2048,7 +2048,7 @@ static struct cached_object {
 static int cached_object_nr, cached_object_alloc;
 
 static struct cached_object empty_tree = {
-	EMPTY_TREE_SHA1_BIN,
+	EMPTY_TREE_SHA1_BIN_LITERAL,
 	OBJ_TREE,
 	"",
 	0
-- 
1.7.4

^ permalink raw reply related

* Re: "git add -u" broken in git 1.7.4?
From: Michael J Gruber @ 2011-02-07  8:27 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: Jeff King, Junio C Hamano, Sebastian Pipping, Git ML
In-Reply-To: <vpqbp2ojq5x.fsf@bauges.imag.fr>

Matthieu Moy venit, vidit, dixit 07.02.2011 07:48:
> Jeff King <peff@peff.net> writes:
> 
>> On Sun, Feb 06, 2011 at 09:50:37PM -0800, Junio C Hamano wrote:
>>
>>> As it takes pathspecs (think "git add -u this-file"), it fundamentally
>>> shouldn't be tree-wide.  I think the original implementation didn't take
>>> pathspecs and was mistakenly done as tree-wide operation, but I think it
>>> was fixed rather quickly.
>>
>> Is "git add -p" broken, then? It takes pathspecs relative to the current
>> directory, but "git add -p" without arguments operates from the root,
>> not from the current subdirectory.
> 
> It's not just "git add -p". Take "git log", "git status", "git
> commit", "git diff" ... well, most Git commands taking pathspecs
> optionally:
> 
> git foo   => tree-wide
> git foo . => the . acts as a path limiter
> 
> and this is the right thing to do. Making "git foo" equivalent to "git
> foo ." makes it hard to recover the tree-wide behavior from a
> subdirectory (git foo ../../../).
> 

First of all, I'd vote for having this work the same way across all
commands - as Junio explained, the destinction we currently have is not
easy to grasp, and is violated by add -p.

Second, we have an established, natural syntax for "base on cwd", namely
".", but we do not have any for "base on worktree root". (I think we
discussed and discarded "/" at some point.)

So, if we go for "relative to cwd by default" we would need a simple way
to specify the root - and by simple I mean taking at most 2 chars in the
pathspec, not a long option!

In summary, I think going for "relative to worktree root by default" is
more in line with git's overall philosophy (so it teaches the right
concept), something the user is exposed to already in most places (but
not all), and limiting to "." already works in most (all?) places, even
with "status" and "status -s". We would only need to change the few
places where we still default to cwd, and make sure they accept "." when
we change their default to repo root.

Cheers,
Michael

^ permalink raw reply

* Re: [1.8.0] Provide proper remote ref namespaces
From: Johan Herland @ 2011-02-07  8:58 UTC (permalink / raw)
  To: Jeff King
  Cc: git, Matthieu Moy, Dmitry Potapov, Junio C Hamano,
	Sverre Rabbelier, Nguyen Thai Ngoc Duy, Nicolas Pitre
In-Reply-To: <20110207051123.GA4748@sigill.intra.peff.net>

On Monday 07 February 2011, Jeff King wrote:
> On Mon, Feb 07, 2011 at 04:51:37AM +0100, Johan Herland wrote:
> > > Take the example of the interim maintainer cited somewhere else in
> > > this thread. If Shawn fetches from Junio, he'll get a junio/v1.7.4
> > > tag, and on my side, I do not want to end up having
> > > shawn/junio/v1.7.4, especially if this means that people fetching
> > > from me would end up with a me/shawn/junio/v1.7.4 ...
> > 
> > You won't end up with "shawn/junio/v1.7.4". When Shawn fetches from
> > Junio, what he actually gets is "refs/remotes/junio/tags/v1.7.4"
> > ("junio/v1.7.4" is a shorthand; "v1.7.4" is an even better shorthand).
> 
> But keep in mind that this proposal will have to live alongside repos
> that are using older versions of git. So Shawn might very well have
> refs/tags/v1.7.4 from Junio if he is using (or has ever used) pre-1.8.0
> git.

Yes, but since they point to the same object, there's no ambiguity.

I'm also starting to wonder whether we should, in existing repos with 
existing remotes, keep using the old-style refspecs by default, thereby 
making new-style refspecs the default only for _new_ repos. It seems mixing 
the two styles in one repo would cause confusion. Another alternative would 
be to transform old-style remotes into new-style remotes, but I believe that 
was shot down elsewhere in this thread.

> No, that won't give you me/shawn/junio/v1.7.4, but it does mean we have
> to gracefully handle the case of ambiguous duplicate tags (that happen
> to point to the same thing).

Whoa, we use the "ambiguous" term differently here. In this whole thread I 
have used "ambiguous" exclusively about when the same (shorthand) tag name 
point to _different_ things. As long as they point to the same thing, there 
is no ambiguity, IMHO.

> Which I think you are implying here:
> > Next, you should never pull from Shawn's work repo, but rather from the
> > repo he has published. In that repo he will typically have pushed the
> > "v1.7.4" tag (as described above). When you pull from Shawn's public
> > repo, you will get the "v1.7.4" tag at
> > "refs/remotes/shawn/tags/v1.7.4" (but "v1.7.4" is an unambigious
> > shorthand).
> 
> But I wanted to point it out explicitly.

Yes, over its lifetime a tag name ("v1.7.4") might appear in several 
namespaces (refs/tags, refs/remotes/*/tags), but it's always identifiable 
with the shorthand "v1.7.4" name (assuming no other "v1.7.4" name point to a 
different object).

This is the same technique we use when talking about branch names: On this 
mailing list, nobody is confused when I refer to 'maint', 'master', 'next' 
and 'pu'. Still, in our own work repos (at least in mine), these branches 
are actually called "refs/remotes/origin/<name>" (commonly referred to by 
their shorthands "origin/<name>"). Here we are, juggling the same kind of 
namespaces that I propose for tags, and it seems to work well without 
causing much confusion.


...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

^ permalink raw reply

* Re: [1.8.0] Provide proper remote ref namespaces
From: Sverre Rabbelier @ 2011-02-07  9:01 UTC (permalink / raw)
  To: Johan Herland
  Cc: Jeff King, git, Matthieu Moy, Dmitry Potapov, Junio C Hamano,
	Nguyen Thai Ngoc Duy, Nicolas Pitre
In-Reply-To: <201102070958.11551.johan@herland.net>

Heya,

On Mon, Feb 7, 2011 at 09:58, Johan Herland <johan@herland.net> wrote:
> This is the same technique we use when talking about branch names: On this
> mailing list, nobody is confused when I refer to 'maint', 'master', 'next'
> and 'pu'. Still, in our own work repos (at least in mine), these branches
> are actually called "refs/remotes/origin/<name>" (commonly referred to by
> their shorthands "origin/<name>"). Here we are, juggling the same kind of
> namespaces that I propose for tags, and it seems to work well without
> causing much confusion.

With the difference that you can't refer to "maint" as just "maint"
unless you've created "refs/heads/maint" iff it is unambiguous.


-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [PATCH] cache-tree: do not cache empty trees
From: Jonathan Nieder @ 2011-02-07  9:17 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: git, Ilari Liusvaara, Jakub Narebski, Dmitry S. Kravtsov,
	Shawn Pearce
In-Reply-To: <1296914835-808-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy wrote:

> Let's do it in a consistent way, always disregard empty trees in
> index. If users choose to create empty trees their own way, they
> should not use index at all.

While this violates some seeming invariants, like

1.
	git reset --hard
	git commit --allow-empty
	git rev-parse HEAD^^{tree} >expect
	git rev-parse HEAD^{tree} >actual
	test_cmp expect actual

2.
	git reset --hard
	git revert HEAD
	if git rev-parse HEAD~2
	then
		git rev-parse HEAD~2^{tree} >expect
		git rev-parse HEAD^{tree} >actual
		test_cmp expect actual
	fi

, I think it's a good change.  Malformed modes in trees already break
those false invariants iiuc.

Thanks.

> --- a/cache-tree.c
> +++ b/cache-tree.c
> @@ -621,9 +621,18 @@ static void prime_cache_tree_rec(struct cache_tree *it, struct tree *tree)
>  			struct tree *subtree = lookup_tree(entry.sha1);
>  			if (!subtree->object.parsed)
>  				parse_tree(subtree);
> +			if (!hashcmp(entry.sha1, (unsigned char *)EMPTY_TREE_SHA1_BIN)) {
> +				warning("empty tree detected! Will be removed in new commits");
> +				cnt = -1;
> +				break;
> +			}

Aside from the warning, this part is an optimization, right?

>  			sub = cache_tree_sub(it, entry.path);
>  			sub->cache_tree = cache_tree();
>  			prime_cache_tree_rec(sub->cache_tree, subtree);
> +			if (sub->cache_tree->entry_count == -1) {
> +				cnt = -1;
> +				break;
> +			}

Would be nice to include a test for this, like so:

	subdir/
		empty1/
		subsubdir/
			empty2/
			empty3/

> --- /dev/null
> +++ b/t/t1013-read-tree-empty.sh
> @@ -0,0 +1,20 @@
> +#!/bin/sh
> +
> +test_description='read-tree with empty trees'
> +
> +. ./test-lib.sh
> +
> +EMPTY_TREE=4b825dc642cb6eb9a060e54bf8d69288fbee4904

If we _have_ to hard-code this (why?) then I'd prefer to do so
in test-lib.sh.

[...]
> +test_expect_success 'write-tree removes empty tree' '
> +	git read-tree `cat tree` &&
> +	git write-tree >actual
> +	echo $EMPTY_TREE >expected
> +	test_cmp expected actual

Broken &&-chain.  Not sure why this test relies on virtual objects
and another (independently nice) patch instead of adding the empty
tree to the object db --- is there something subtle I am missing?

The test does not distinguish between success due to git read-tree
omitting empty trees and success due to git mktree omitting empty
trees.

Hope that helps,
Jonathan

^ permalink raw reply

* Re: [PATCH] Add support for merging from upstream by default.
From: Bert Wesarg @ 2011-02-07  9:36 UTC (permalink / raw)
  To: Jared Hance; +Cc: git
In-Reply-To: <1296912681-4161-1-git-send-email-jaredhance@gmail.com>

On Sat, Feb 5, 2011 at 14:31, Jared Hance <jaredhance@gmail.com> wrote:
> Adds the option merge.defaultupstream to add support for merging from the
> upstream branch by default. The upstream branch is found using
> branch.[name].upstream.

Why do we need a per branch config for this, isn't .origin/.merge
enough? Or, where is the connection between your new .upstream and the
@{upstream} ref?

Bert

^ permalink raw reply

* Re: [PATCH] Add support for merging from upstream by default.
From: Bert Wesarg @ 2011-02-07  9:43 UTC (permalink / raw)
  To: Jared Hance; +Cc: git
In-Reply-To: <AANLkTikii-fx5nYeBDc70eh_wj05sEZ6fH6=bF9GqHBK@mail.gmail.com>

On Mon, Feb 7, 2011 at 10:36, Bert Wesarg <bert.wesarg@googlemail.com> wrote:
> On Sat, Feb 5, 2011 at 14:31, Jared Hance <jaredhance@gmail.com> wrote:
>> Adds the option merge.defaultupstream to add support for merging from the
>> upstream branch by default. The upstream branch is found using
>> branch.[name].upstream.
>
> Why do we need a per branch config for this, isn't .origin/.merge

Why do we need a *new* per branch config for this, isn't .origin/.merge

> enough? Or, where is the connection between your new .upstream and the
> @{upstream} ref?
>
> Bert
>

^ permalink raw reply

* Re: [PATCH] cache-tree: do not cache empty trees
From: Nguyen Thai Ngoc Duy @ 2011-02-07  9:57 UTC (permalink / raw)
  To: Jonathan Nieder
  Cc: git, Ilari Liusvaara, Jakub Narebski, Dmitry S. Kravtsov,
	Shawn Pearce
In-Reply-To: <20110207091740.GA5391@elie>

On Mon, Feb 07, 2011 at 03:17:40AM -0600, Jonathan Nieder wrote:
> While this violates some seeming invariants, like
> 
> 1.
> 	git reset --hard
> 	git commit --allow-empty
> 	git rev-parse HEAD^^{tree} >expect
> 	git rev-parse HEAD^{tree} >actual
> 	test_cmp expect actual
> 
> 2.
> 	git reset --hard
> 	git revert HEAD
> 	if git rev-parse HEAD~2
> 	then
> 		git rev-parse HEAD~2^{tree} >expect
> 		git rev-parse HEAD^{tree} >actual
> 		test_cmp expect actual
> 	fi
> 
> , I think it's a good change.  Malformed modes in trees already break
> those false invariants iiuc.

Perhaps it's not a good approach after all. What I wanted was to make
pre-1.8.0 tolerate empty trees created by 1.8.0. Perhaps it's better
to just let pre-1.8.0 refuse to work with empty trees, forcing users
to upgrade to 1.8.0?

The (untested) patch below would make git refuse to create an index
from a tree that contains empty trees. Hmm?

diff --git a/cache-tree.c b/cache-tree.c
index f755590..e33998a 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -619,6 +619,8 @@ static void prime_cache_tree_rec(struct cache_tree *it, struct tree *tree)
 		else {
 			struct cache_tree_sub *sub;
 			struct tree *subtree = lookup_tree(entry.sha1);
+			if (!hashcmp(entry.sha1, EMPTY_TREE_SHA1_BIN))
+				die("empty tree .../%s detected!", entry.path);
 			if (!subtree->object.parsed)
 				parse_tree(subtree);
 			sub = cache_tree_sub(it, entry.path);
diff --git a/unpack-trees.c b/unpack-trees.c
index 1ca41b1..0e6738e 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -434,6 +434,7 @@ static int traverse_trees_recursive(int n, unsigned long dirmask, unsigned long
 	void *buf[MAX_UNPACK_TREES];
 	struct traverse_info newinfo;
 	struct name_entry *p;
+	struct unpack_trees_options *o = info->data;
 
 	p = names;
 	while (!p->mode)
@@ -447,8 +448,11 @@ static int traverse_trees_recursive(int n, unsigned long dirmask, unsigned long
 
 	for (i = 0; i < n; i++, dirmask >>= 1) {
 		const unsigned char *sha1 = NULL;
-		if (dirmask & 1)
+		if (dirmask & 1) {
 			sha1 = names[i].sha1;
+			if (o->merge && !hashcmp(sha1, EMPTY_TREE_SHA1_BIN))
+				return error("empty tree .../%s detected!", p->path);
+		}
 		buf[i] = fill_tree_descriptor(t+i, sha1);
 	}
 

-- 
Duy

^ permalink raw reply related

* Re: [1.8.0] Provide proper remote ref namespaces
From: Johan Herland @ 2011-02-07 10:06 UTC (permalink / raw)
  To: Sverre Rabbelier
  Cc: Jeff King, git, Matthieu Moy, Dmitry Potapov, Junio C Hamano,
	Nguyen Thai Ngoc Duy, Nicolas Pitre
In-Reply-To: <AANLkTiksUqVnWeZOm-9XN3BbfVcjc6fWdwPcPJ-PLb88@mail.gmail.com>

On Monday 07 February 2011, Sverre Rabbelier wrote:
> On Mon, Feb 7, 2011 at 09:58, Johan Herland <johan@herland.net> wrote:
> > This is the same technique we use when talking about branch names:
> > On this mailing list, nobody is confused when I refer to 'maint',
> > 'master', 'next' and 'pu'. Still, in our own work repos (at least
> > in mine), these branches are actually called
> > "refs/remotes/origin/<name>" (commonly referred to by their
> > shorthands "origin/<name>"). Here we are, juggling the same kind of
> > namespaces that I propose for tags, and it seems to work well
> > without causing much confusion.
>
> With the difference that you can't refer to "maint" as just "maint"
> unless you've created "refs/heads/maint" iff it is unambiguous.

Except that with 'git checkout', you can:

$ git clone git://git.kernel.org/pub/scm/git/git.git
$ cd git/
$ git checkout maint


...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

^ permalink raw reply

* Re: [1.8.0] Provide proper remote ref namespaces
From: Nguyen Thai Ngoc Duy @ 2011-02-07 10:22 UTC (permalink / raw)
  To: Johan Herland
  Cc: Sverre Rabbelier, Jeff King, git, Matthieu Moy, Dmitry Potapov,
	Junio C Hamano, Nicolas Pitre
In-Reply-To: <201102071106.17269.johan@herland.net>

On Mon, Feb 7, 2011 at 5:06 PM, Johan Herland <johan@herland.net> wrote:
> On Monday 07 February 2011, Sverre Rabbelier wrote:
>> On Mon, Feb 7, 2011 at 09:58, Johan Herland <johan@herland.net> wrote:
>> > This is the same technique we use when talking about branch names:
>> > On this mailing list, nobody is confused when I refer to 'maint',
>> > 'master', 'next' and 'pu'. Still, in our own work repos (at least
>> > in mine), these branches are actually called
>> > "refs/remotes/origin/<name>" (commonly referred to by their
>> > shorthands "origin/<name>"). Here we are, juggling the same kind of
>> > namespaces that I propose for tags, and it seems to work well
>> > without causing much confusion.
>>
>> With the difference that you can't refer to "maint" as just "maint"
>> unless you've created "refs/heads/maint" iff it is unambiguous.
>
> Except that with 'git checkout', you can:
>
> $ git clone git://git.kernel.org/pub/scm/git/git.git
> $ cd git/
> $ git checkout maint

That's some checkout magic kicking in. I cloned and there was only
refs/heads/master. After checkout, refs/heads/maint appeared.

$ git clone git://git.kernel.org/pub/scm/git/git.git git2
$ cd git2
$ git rev-parse maint
maint
fatal: ambiguous argument 'maint': unknown revision or path not in the
working tree.
Use '--' to separate paths from revisions
$ git rev-parse origin/maint
597a63054241c122515c93cbce45bc44eb231f18
$ git co maint
Branch maint set up to track remote branch maint from origin.
Switched to a new branch 'maint'
$ git rev-parse maint
597a63054241c122515c93cbce45bc44eb231f18
-- 
Duy

^ permalink raw reply

* [1.8.0] git checkout refs/heads/foo checks out branch foo
From: Martin von Zweigbergk @ 2011-02-07 11:01 UTC (permalink / raw)
  To: git

Proposal:

'git checkout refs/heads/foo' (or 'git checkout heads/foo' for that
matter) does not check out the branch, but instead detaches HEAD at
foo. This is quite counter-intuitive (at least to me) and the same
functionality can be achieved by using e.g. foo~0. Change the behavior
so that the branch is actually checked out. This also applies to
e.g. 'git rebase master refs/heads/topic', which currently rebases a
detached HEAD. There are probably other examples as well that I'm not
aware of.


Risks:

Existing scripts may depend on the current behavior. It seems unlikely
that many users depend on it. Most likely, they use foo~0 or foo^0
instead.


Migration plan:

Make 'git checkout refs/head/foo' emit a warning in the next 1.7.x
explaining that its semantics will change in 1.8.0. Then change the
behavior in 1.8.0 and remove the warning.

^ permalink raw reply

* Re: [PATCH 2/2] git-p4: Add copy detection support
From: Vitor Antunes @ 2011-02-07 11:11 UTC (permalink / raw)
  To: Pete Wyckoff; +Cc: git
In-Reply-To: <20110206220546.GA9024@mew.padd.com>

Hi Pete,

On Sun, Feb 6, 2011 at 10:05 PM, Pete Wyckoff <pw@padd.com> wrote:
>> (Copying help text:
>>       The -t flag makes the source file's filetype propagate to the target
>>       file.  Normally, the target file retains its previous filetype.
>>       Newly branched files always use the source file's filetype.  The
>>       filetype can still be changed before 'p4 submit' with 'p4 reopen'.
>> )
>>
>> Since in git we're only considering newly branched files, I think in
>> this case "-t" will not add anything. In fact, what is being done here
>> is detecting exec bit changes from source to target files - we're not
>> trying to force P4 to use the source's exec bit. Do you agree?
>
> That sounds fine to me.  The code seemed to indicate that
> sometimes the destination file exists.

I've basically copied the code from the rename detection part and
adapted it to copying. Nevertheless, even if git detects a copy to a
already existing file I think that the integrate command should behave
correctly. I should create a test case for this, though.

>> +            elif modifier == "C":
>> +                src, dest = diff['src'], diff['dst']
>> +                p4_system("integrate -Dt \"%s\" \"%s\"" % (src, dest))
>> +                if diff['src_sha1'] != diff['dst_sha1']:
>> +                    p4_system("edit \"%s\"" % (dest))
>> +                if isModeExecChanged(diff['src_mode'], diff['dst_mode']):
>> +                    filesToChangeExecBit[dest] = diff['dst_mode']
>> +                os.unlink(dest)
>> +                editedFiles.add(dest)
>
> If you're happy the dest never exists, you may be able to get rid
> of the edit step and the mode-change check entirely.  As long as
> you've tested this, you're the expert here.  The change makes
> sense overall.

I'm not that happy with this... and I'm no expert! I will really need
to test this possibility. Just need to understand how I can make git
detect a copy to an existing file... :)

Thanks for your feedback,
-- 
Vitor Antunes

^ permalink raw reply

* Re: "git add -u" broken in git 1.7.4?
From: SZEDER Gábor @ 2011-02-07 11:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Matthieu Moy, Sebastian Pipping, Jeff King, Git ML
In-Reply-To: <7vwrlcv1ea.fsf@alter.siamese.dyndns.org>

Hi,


On Sun, Feb 06, 2011 at 09:50:37PM -0800, Junio C Hamano wrote:
> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> 
> > Sebastian Pipping <webmaster@hartwork.org> writes:
> >
> >> I was and I can confirm the different behaviour with 1.7.4 over here: it
> >> does work on the root directory of the repo as you supposed.
> >
> > What do you mean by "it does not work"?
> >
> > "git add -u" adds files under the current directory, and it always
> > did.
> 
> As it takes pathspecs (think "git add -u this-file"), it fundamentally
> shouldn't be tree-wide.  I think the original implementation didn't take
> pathspecs and was mistakenly done as tree-wide operation, but I think it
> was fixed rather quickly.

Interesting, when I brought up this issue about one and a half years
ago, you of all people proposed that this change might be worth
addressing in 1.8.0.

  Message-ID: <7veiql1etz.fsf@alter.siamese.dyndns.org>
  http://thread.gmane.org/gmane.comp.version-control.git/127593/focus=127594

There was a longish discussion back then with arguments both for and
against tree-wide operation.


Best,
Gábor

^ permalink raw reply

* GREETINGS!!.2
From: Directors Dr Raymond @ 2011-02-06 23:51 UTC (permalink / raw)


Dear Friend                                                      
I am Vice Chairman, Board of Directors Dr Raymond Chien Kuo Fung,
I work with the Hang Seng Bank Hong Kong.l have a transaction of
$24,500,000 to bring to your attention if only you would be
interested, kindly respond to the below email for more details.

PLEASE, REPLY-      drhraymond@yahoo.com.hk

^ 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