Git development
 help / color / mirror / Atom feed
* Re: Commit changes to remote repository
From: Matthieu Moy @ 2012-01-14 14:27 UTC (permalink / raw)
  To: Carlos Martín Nieto; +Cc: ruperty, git
In-Reply-To: <20120114113141.GG2850@centaur.lab.cmartin.tk>

Carlos Martín Nieto <cmn@elego.de> writes:

> You're trying to push to a non-bare repository and change the
> currently active branch, which can cause problems, so git isn't
> letting you. There's an explanation of bare and non-bare at
> http://bare-vs-nonbare.gitrecipes.de/ but the short and sweet is that
> you should init the repo you want to use as the central point with
> --bare and do modifications locally and then push there.

An alternative is to push to a temporary, non-checked-out branch.

I sometimes do

  laptop$ git push desktop HEAD:incomming

and then

  desktop$ git merge incomming

The push does not disturb the worktree on the desktop, and the merge is
done manually on the receiving machine.

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

^ permalink raw reply

* git-upload-archive help was not shown correctly
From: devendra @ 2012-01-14 13:40 UTC (permalink / raw)
  To: git

Hi git folks,

the command git-upload-archive is not properly showing usage info when
ran barely with out any args.

it shows some kind of unwanted garbage instead of showing a nice help
message.

The output is pasted here.

root@devendra-Linux:/home/devendra/git/Documentation#
git-upload-archive 
0008ACK
00000026 usage: git upload-archive <repo>
0031 git upload-archive: archiver died with errorfatal: sent error to
the client: git upload-archive: archiver died with error

my git latest version shows commit sha1 number
6db5c6e43dccb380ca6e9947777985eb11248c31.

Devendra.

^ permalink raw reply

* Re: Zsh completion regression
From: SZEDER Gábor @ 2012-01-14 13:23 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: Stefan Haller, git
In-Reply-To: <vpq62ghj7fn.fsf@bauges.imag.fr>

Hi,


On Thu, Jan 12, 2012 at 03:56:28PM +0100, Matthieu Moy wrote:
> lists@haller-berlin.de (Stefan Haller) writes:
> 
> > I'm using zsh   4.3.11.
> >
> > When I type "git log mas<TAB>", it completes to "git log master\ " (a
> > backslash, a space, and then the cursor).

Stefan, thanks for reporting and bisecting.

> The following patch makes the situation better, but is not really a fix:
> 
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -525,7 +525,7 @@ __gitcomp ()
>  __gitcomp_nl ()
>  {
>         local s=$'\n' IFS=' '$'\t'$'\n'
> -       local cur_="$cur" suffix=" "
> +       local cur_="$cur" suffix=""
>  
>         if [ $# -gt 2 ]; then
>                 cur_="$3"
> 
> With this, the trailing space isn't added,

Yeah, this would be a regression for Bash users, because then they
will need to manually append that space.

> but e.g. "git checkout
> master<TAB>" does not add the trailing space, at all.

I'm not sure what you mean; did you got a trailing space after
'master<TAB>' before a31e6262 (completion: optimize refs completion,
2011-10-15)?
I'm not a zsh user myself, but just tried it and found that in
a31e626^ 'git checkout master<TAB>' doesn't add the trailing space,
and neither does 'git checkout mas<TAB>', which is in sync with what
Stefan reported ("Before this commit, I get "git log master" (with no
space at the end).").

> The problem is a little bit below:
> 
> 	IFS=$s
> 	COMPREPLY=($(compgen -P "${2-}" -S "$suffix" -W "$1" -- "$cur_"))
> 
> The -S "$suffix" adds a space to the completion, but ZSH escapes the
> space (which sounds sensible in general, but is not at all what we
> expect).

So it seems we have two issues here with zsh:

- Spaces appended with 'compgen -S " "' in __gitcomp_nl() are quoted.
  This is the regression caused by a31e6262.
- Spaces appended in __gitcomp_1() (by echo "$word ") are stripped.
  This is a long-standing issue; if those spaces weren't stripped, the
  quoting issue would have been noted earlier, I suspect.

We could fix the regression by not appending a space suffix to
completion words in __gitcomp_nl(), but only when the completion
script is running under zsh to avoid hurting bash users, like this:

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 2d02a7f3..49393243 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -601,6 +601,9 @@ __gitcomp_nl ()
 			suffix="$4"
 		fi
 	fi
+	if [ -n "${ZSH_VERSION-}" ] && [ "$suffix" = " " ]; then
+		suffix=""
+	fi
 
 	IFS=$s
 	COMPREPLY=($(compgen -P "${2-}" -S "$suffix" -W "$1" -- "$cur_"))


Although it wouldn't append a space at all for zsh users, that's
nothing new, it's just restoring old behavior.  And then later someone
more knowledgable about the quirks of zsh's completion and in
particular zsh's bash completion emulation can come up with a proper
fix to the issue of quoted spaces and stripped trailing spaces.


However.

While playing around with zsh, I noticed that git completion works
without even sourcing git's bash completion script.  As it turned out,
zsh ships its own git completion script[1], and from my cursory tests
it seems to be quite capable: it supports commands, aliases, options,
refs, ref1..ref2, config variables, ...  and I also saw a __git_ps1()
equivalent for zsh.

So, is there any reason why you are still using git's bash completion
under zsh (which has some quirks and lacks some features) instead of
zsh's own?  Perhaps it would make sense to point zsh users to zsh's
git completion and drop zsh compatibility from git's bash completion.
We did similar with vim config files: git included a vim syntax
highlight config file for commit messages under contrib/vim/, but
eventually we dropped it after vim started shipping more capable
git-specific config files (for git config files, rebase instruction
sheets, etc.).


[1] http://zsh.git.sourceforge.net/git/gitweb.cgi?p=zsh/zsh;a=blob;f=Completion/Unix/Command/_git;hb=HEAD


Best,
Gábor

^ permalink raw reply related

* [PATCH v3 3/3] index-pack: eliminate unlimited recursion in get_base_data()
From: Nguyễn Thái Ngọc Duy @ 2012-01-14 12:19 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Shawn O. Pearce,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <1326543595-28300-1-git-send-email-pclouds@gmail.com>

Revese the order of delta applying so that by the time a delta is
applied, its base is either non-delta or already inflated.
get_base_data() is still recursive, but because base's data is always
ready, the inner get_base_data() call never has any chance to call
itself again.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/index-pack.c |   53 +++++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 38ff03a..dc6a584 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -515,14 +515,52 @@ static int is_delta_type(enum object_type type)
 	return (type == OBJ_REF_DELTA || type == OBJ_OFS_DELTA);
 }
 
+/*
+ * This function is part of find_unresolved_deltas(). There are two
+ * walkers going in the opposite ways.
+ *
+ * The first one in find_unresolved_deltas() traverses down from
+ * parent node to children, deflating nodes along the way. However,
+ * memory for deflated nodes is limited by delta_base_cache_limit, so
+ * at some point parent node's deflated content may be freed.
+ *
+ * The second walker is this function, which goes from current node up
+ * to top parent if necessary to deflate the node. In normal
+ * situation, its parent node would be already deflated, so it just
+ * needs to apply delta.
+ *
+ * In worst case scenario, parent node is no longer deflated because
+ * we're running out of delta_base_cache_limit, then we need to
+ * re-deflate parents, possibly up to the top base.
+ *
+ * All deflated objecsts here are subject to be freed if we exceed
+ * delta_base_cache_limit, just like in find_unresolved_deltas(), we
+ * just need to make sure the last node is not freed.
+ */
 static void *get_base_data(struct base_data *c)
 {
 	if (!c->data) {
 		struct object_entry *obj = c->obj;
+		struct base_data **delta = NULL;
+		int delta_nr = 0, delta_alloc = 0;
 
-		if (is_delta_type(obj->type)) {
-			void *base = get_base_data(c->base);
-			void *raw = get_data_from_pack(obj);
+		while (is_delta_type(c->obj->type) && !c->data) {
+			ALLOC_GROW(delta, delta_nr + 1, delta_alloc);
+			delta[delta_nr++] = c;
+			c = c->base;
+		}
+		if (!delta_nr) {
+			c->data = get_data_from_pack(obj);
+			c->size = obj->size;
+			base_cache_used += c->size;
+			prune_base_data(c);
+		}
+		for (; delta_nr > 0; delta_nr--) {
+			void *base, *raw;
+			c = delta[delta_nr - 1];
+			obj = c->obj;
+			base = get_base_data(c->base);
+			raw = get_data_from_pack(obj);
 			c->data = patch_delta(
 				base, c->base->size,
 				raw, obj->size,
@@ -530,13 +568,10 @@ static void *get_base_data(struct base_data *c)
 			free(raw);
 			if (!c->data)
 				bad_object(obj->idx.offset, "failed to apply delta");
-		} else {
-			c->data = get_data_from_pack(obj);
-			c->size = obj->size;
+			base_cache_used += c->size;
+			prune_base_data(c);
 		}
-
-		base_cache_used += c->size;
-		prune_base_data(c);
+		free(delta);
 	}
 	return c->data;
 }
-- 
1.7.8.36.g69ee2

^ permalink raw reply related

* [PATCH v3 1/3] Eliminate recursion in setting/clearing marks in commit list
From: Nguyễn Thái Ngọc Duy @ 2012-01-14 12:19 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Shawn O. Pearce,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <1326543595-28300-1-git-send-email-pclouds@gmail.com>

Recursion in a DAG is generally a bad idea because it could be very
deep. Be defensive and avoid recursion in mark_parents_uninteresting()
and clear_commit_marks().

mark_parents_uninteresting() learns a trick from clear_commit_marks()
to avoid malloc() in (dorminant) single-parent case.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 commit.c   |   13 +++++++++++--
 revision.c |   45 +++++++++++++++++++++++++++++----------------
 2 files changed, 40 insertions(+), 18 deletions(-)

diff --git a/commit.c b/commit.c
index 44bc96d..c7aefbf 100644
--- a/commit.c
+++ b/commit.c
@@ -421,7 +421,8 @@ struct commit *pop_most_recent_commit(struct commit_list **list,
 	return ret;
 }
 
-void clear_commit_marks(struct commit *commit, unsigned int mark)
+static void clear_commit_marks_1(struct commit_list **plist,
+				 struct commit *commit, unsigned int mark)
 {
 	while (commit) {
 		struct commit_list *parents;
@@ -436,12 +437,20 @@ void clear_commit_marks(struct commit *commit, unsigned int mark)
 			return;
 
 		while ((parents = parents->next))
-			clear_commit_marks(parents->item, mark);
+			commit_list_insert(parents->item, plist);
 
 		commit = commit->parents->item;
 	}
 }
 
+void clear_commit_marks(struct commit *commit, unsigned int mark)
+{
+	struct commit_list *list = NULL;
+	commit_list_insert(commit, &list);
+	while (list)
+		clear_commit_marks_1(&list, pop_commit(&list), mark);
+}
+
 void clear_commit_marks_for_object_array(struct object_array *a, unsigned mark)
 {
 	struct object *object;
diff --git a/revision.c b/revision.c
index 8764dde..7cc72fc 100644
--- a/revision.c
+++ b/revision.c
@@ -139,11 +139,32 @@ void mark_tree_uninteresting(struct tree *tree)
 
 void mark_parents_uninteresting(struct commit *commit)
 {
-	struct commit_list *parents = commit->parents;
+	struct commit_list *parents = NULL, *l;
+
+	for (l = commit->parents; l; l = l->next)
+		commit_list_insert(l->item, &parents);
 
 	while (parents) {
 		struct commit *commit = parents->item;
-		if (!(commit->object.flags & UNINTERESTING)) {
+		l = parents;
+		parents = parents->next;
+		free(l);
+
+		while (commit) {
+			/*
+			 * A missing commit is ok iff its parent is marked
+			 * uninteresting.
+			 *
+			 * We just mark such a thing parsed, so that when
+			 * it is popped next time around, we won't be trying
+			 * to parse it and get an error.
+			 */
+			if (!has_sha1_file(commit->object.sha1))
+				commit->object.parsed = 1;
+
+			if (commit->object.flags & UNINTERESTING)
+				break;
+
 			commit->object.flags |= UNINTERESTING;
 
 			/*
@@ -154,21 +175,13 @@ void mark_parents_uninteresting(struct commit *commit)
 			 * wasn't uninteresting), in which case we need
 			 * to mark its parents recursively too..
 			 */
-			if (commit->parents)
-				mark_parents_uninteresting(commit);
-		}
+			if (!commit->parents)
+				break;
 
-		/*
-		 * A missing commit is ok iff its parent is marked
-		 * uninteresting.
-		 *
-		 * We just mark such a thing parsed, so that when
-		 * it is popped next time around, we won't be trying
-		 * to parse it and get an error.
-		 */
-		if (!has_sha1_file(commit->object.sha1))
-			commit->object.parsed = 1;
-		parents = parents->next;
+			for (l = commit->parents->next; l; l = l->next)
+				commit_list_insert(l->item, &parents);
+			commit = commit->parents->item;
+		}
 	}
 }
 
-- 
1.7.8.36.g69ee2

^ permalink raw reply related

* [PATCH v3 2/3] index-pack: eliminate recursion in find_unresolved_deltas
From: Nguyễn Thái Ngọc Duy @ 2012-01-14 12:19 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Shawn O. Pearce,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <1326543595-28300-1-git-send-email-pclouds@gmail.com>

Current find_unresolved_deltas() links all bases together in a form of
tree, using struct base_data, with prev_base pointer to point to
parent node. Then it traverses down from parent to children in
recursive manner with all base_data allocated on stack.

To eliminate recursion, we simply need to put all on heap
(parse_pack_objects and fix_unresolved_deltas). After that, it's
simple non-recursive depth-first traversal loop. Each node also
maintains its own state (ofs and ref indices) to iterate over all
children nodes.

So we process one node:

 - if it returns a new (child) node (a parent base), we link it to our
   tree, then process the new node.

 - if it returns nothing, the node is done, free it. We go back to
   parent node and resume whatever it's doing.

and do it until we have no nodes to process.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/index-pack.c |  111 +++++++++++++++++++++++++++++++------------------
 1 files changed, 70 insertions(+), 41 deletions(-)

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index af7dc37..38ff03a 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -34,6 +34,8 @@ struct base_data {
 	struct object_entry *obj;
 	void *data;
 	unsigned long size;
+	int ref_first, ref_last;
+	int ofs_first, ofs_last;
 };
 
 /*
@@ -221,6 +223,15 @@ static NORETURN void bad_object(unsigned long offset, const char *format, ...)
 	die("pack has bad object at offset %lu: %s", offset, buf);
 }
 
+static struct base_data *alloc_base_data(void)
+{
+	struct base_data *base = xmalloc(sizeof(struct base_data));
+	memset(base, 0, sizeof(*base));
+	base->ref_last = -1;
+	base->ofs_last = -1;
+	return base;
+}
+
 static void free_base_data(struct base_data *c)
 {
 	if (c->data) {
@@ -553,58 +564,76 @@ static void resolve_delta(struct object_entry *delta_obj,
 	nr_resolved_deltas++;
 }
 
-static void find_unresolved_deltas(struct base_data *base,
-				   struct base_data *prev_base)
+static struct base_data *find_unresolved_deltas_1(struct base_data *base,
+						  struct base_data *prev_base)
 {
-	int i, ref_first, ref_last, ofs_first, ofs_last;
-
-	/*
-	 * This is a recursive function. Those brackets should help reducing
-	 * stack usage by limiting the scope of the delta_base union.
-	 */
-	{
+	if (base->ref_last == -1 && base->ofs_last == -1) {
 		union delta_base base_spec;
 
 		hashcpy(base_spec.sha1, base->obj->idx.sha1);
 		find_delta_children(&base_spec,
-				    &ref_first, &ref_last, OBJ_REF_DELTA);
+				    &base->ref_first, &base->ref_last, OBJ_REF_DELTA);
 
 		memset(&base_spec, 0, sizeof(base_spec));
 		base_spec.offset = base->obj->idx.offset;
 		find_delta_children(&base_spec,
-				    &ofs_first, &ofs_last, OBJ_OFS_DELTA);
-	}
+				    &base->ofs_first, &base->ofs_last, OBJ_OFS_DELTA);
 
-	if (ref_last == -1 && ofs_last == -1) {
-		free(base->data);
-		return;
-	}
+		if (base->ref_last == -1 && base->ofs_last == -1) {
+			free(base->data);
+			return NULL;
+		}
 
-	link_base_data(prev_base, base);
+		link_base_data(prev_base, base);
+	}
 
-	for (i = ref_first; i <= ref_last; i++) {
-		struct object_entry *child = objects + deltas[i].obj_no;
-		struct base_data result;
+	if (base->ref_first <= base->ref_last) {
+		struct object_entry *child = objects + deltas[base->ref_first].obj_no;
+		struct base_data *result = alloc_base_data();
 
 		assert(child->real_type == OBJ_REF_DELTA);
-		resolve_delta(child, base, &result);
-		if (i == ref_last && ofs_last == -1)
+		resolve_delta(child, base, result);
+		if (base->ref_first == base->ref_last && base->ofs_last == -1)
 			free_base_data(base);
-		find_unresolved_deltas(&result, base);
+
+		base->ref_first++;
+		return result;
 	}
 
-	for (i = ofs_first; i <= ofs_last; i++) {
-		struct object_entry *child = objects + deltas[i].obj_no;
-		struct base_data result;
+	if (base->ofs_first <= base->ofs_last) {
+		struct object_entry *child = objects + deltas[base->ofs_first].obj_no;
+		struct base_data *result = alloc_base_data();
 
 		assert(child->real_type == OBJ_OFS_DELTA);
-		resolve_delta(child, base, &result);
-		if (i == ofs_last)
+		resolve_delta(child, base, result);
+		if (base->ofs_first == base->ofs_last)
 			free_base_data(base);
-		find_unresolved_deltas(&result, base);
+
+		base->ofs_first++;
+		return result;
 	}
 
 	unlink_base_data(base);
+	return NULL;
+}
+
+static void find_unresolved_deltas(struct base_data *base)
+{
+	struct base_data *new_base, *prev_base = NULL;
+	for (;;) {
+		new_base = find_unresolved_deltas_1(base, prev_base);
+
+		if (new_base) {
+			prev_base = base;
+			base = new_base;
+		} else {
+			free(base);
+			base = prev_base;
+			if (!base)
+				return;
+			prev_base = base->base;
+		}
+	}
 }
 
 static int compare_delta_entry(const void *a, const void *b)
@@ -684,13 +713,13 @@ static void parse_pack_objects(unsigned char *sha1)
 		progress = start_progress("Resolving deltas", nr_deltas);
 	for (i = 0; i < nr_objects; i++) {
 		struct object_entry *obj = &objects[i];
-		struct base_data base_obj;
+		struct base_data *base_obj = alloc_base_data();
 
 		if (is_delta_type(obj->type))
 			continue;
-		base_obj.obj = obj;
-		base_obj.data = NULL;
-		find_unresolved_deltas(&base_obj, NULL);
+		base_obj->obj = obj;
+		base_obj->data = NULL;
+		find_unresolved_deltas(base_obj);
 		display_progress(progress, nr_resolved_deltas);
 	}
 }
@@ -783,20 +812,20 @@ static void fix_unresolved_deltas(struct sha1file *f, int nr_unresolved)
 	for (i = 0; i < n; i++) {
 		struct delta_entry *d = sorted_by_pos[i];
 		enum object_type type;
-		struct base_data base_obj;
+		struct base_data *base_obj = alloc_base_data();
 
 		if (objects[d->obj_no].real_type != OBJ_REF_DELTA)
 			continue;
-		base_obj.data = read_sha1_file(d->base.sha1, &type, &base_obj.size);
-		if (!base_obj.data)
+		base_obj->data = read_sha1_file(d->base.sha1, &type, &base_obj->size);
+		if (!base_obj->data)
 			continue;
 
-		if (check_sha1_signature(d->base.sha1, base_obj.data,
-				base_obj.size, typename(type)))
+		if (check_sha1_signature(d->base.sha1, base_obj->data,
+				base_obj->size, typename(type)))
 			die("local object %s is corrupt", sha1_to_hex(d->base.sha1));
-		base_obj.obj = append_obj_to_pack(f, d->base.sha1,
-					base_obj.data, base_obj.size, type);
-		find_unresolved_deltas(&base_obj, NULL);
+		base_obj->obj = append_obj_to_pack(f, d->base.sha1,
+					base_obj->data, base_obj->size, type);
+		find_unresolved_deltas(base_obj);
 		display_progress(progress, nr_resolved_deltas);
 	}
 	free(sorted_by_pos);
-- 
1.7.8.36.g69ee2

^ permalink raw reply related

* [PATCH v3 0/3] nd/index-pack-no-recurse
From: Nguyễn Thái Ngọc Duy @ 2012-01-14 12:19 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Shawn O. Pearce,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <1326081546-29320-1-git-send-email-pclouds@gmail.com>

This round adds more explanation in commit message of 2/3 and comment
before get_base_data() in 3/3. Changes in 3/3 does not really address
Junio's concern regarding maintainability though.

It also fixes a regression in 3/3. In current code, get_base_data()
goes up as far as the first deflated parent. v2 of this series always
goes up to top parent. v3 fixes this.

Junio raised a point about depth-first vs breadth-first search in 1/3. I
have not addressed that either, but it makes me wonder if we may
benefit from using bfs in find_unresolved_deltas(), 2/3. If the delta
chains form a fork-like figure (e.g. long delta chains sharing common
base), then we may run out of cache doing dfs on one chain, by the
time we get back on the common base, we would need to deflate them
again.

Another observation is recursion in get_base_data() is unlikely to be
called in real life. With 16M default delta base cache, git.git does
not trigger it at all. Perhaps repos with large blobs have better chance..

Nguyễn Thái Ngọc Duy (3):
  Eliminate recursion in setting/clearing marks in commit list
  index-pack: eliminate recursion in find_unresolved_deltas
  index-pack: eliminate unlimited recursion in get_base_data()

 builtin/index-pack.c |  164 ++++++++++++++++++++++++++++++++++---------------
 commit.c             |   13 ++++-
 revision.c           |   45 +++++++++-----
 3 files changed, 154 insertions(+), 68 deletions(-)

-- 
1.7.8.36.g69ee2

^ permalink raw reply

* Re: [PATCH 2/2] git-gui: fix applying line/ranges when the selection ends at the begin of a line
From: Bert Wesarg @ 2012-01-14 12:08 UTC (permalink / raw)
  To: Pat Thoyts; +Cc: git, Bert Wesarg
In-Reply-To: <37339be035746797fcec7634e3560ffcd5b26cf3.1326116492.git.bert.wesarg@googlemail.com>

On Mon, Jan 9, 2012 at 14:43, Bert Wesarg <bert.wesarg@googlemail.com> wrote:
> Selecting also the trailing newline of a line for staging/unstaging would
> have resulted in also staging/unstaging of the next line.

The fix is not complete, this logic should only be applied if we have
actually a range. I will send a replacement patch in the coming days.

Bert

>
> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
> ---
>  lib/diff.tcl |    8 +++++++-
>  1 files changed, 7 insertions(+), 1 deletions(-)
>
> diff --git a/lib/diff.tcl b/lib/diff.tcl
> index 63f8742..a750ea7 100644
> --- a/lib/diff.tcl
> +++ b/lib/diff.tcl
> @@ -632,7 +632,13 @@ proc apply_range_or_line {x y} {
>        }
>
>        set first_l [$ui_diff index "$first linestart"]
> -       set last_l [$ui_diff index "$last lineend"]
> +       # don't include the next line if $last points to the start of a line
> +       # ie. <lno>.0
> +       if {[lindex [split $last .] 1] == 0} {
> +               set last_l [$ui_diff index "$last -1 line lineend"]
> +       } else {
> +               set last_l [$ui_diff index "$last lineend"]
> +       }
>
>        if {$current_diff_path eq {} || $current_diff_header eq {}} return
>        if {![lock_index apply_hunk]} return
> --
> 1.7.8.1.873.gfea665
>

^ permalink raw reply

* Re: [PATCH] git-gui: fix selection regression introduced in a8ca786991
From: Bert Wesarg @ 2012-01-14 11:58 UTC (permalink / raw)
  To: Pat Thoyts; +Cc: git, Bert Wesarg, Shawn O. Pearce
In-Reply-To: <CAKPyHN0xRD7qYyLYaCB9u7mhCZYFObuTdJGHq-rRST-cEhTtXA@mail.gmail.com>

Hi Pat,

On Mon, Jan 9, 2012 at 10:28, Bert Wesarg <bert.wesarg@googlemail.com> wrote:
> Hi,
>
> On Sat, Jan 7, 2012 at 20:43, Bert Wesarg <bert.wesarg@googlemail.com> wrote:
>> While fixing the problem from a8ca786991, it introduces a regression
>> regarding what happen after the multi selected file operation (ie.
>> one of Ctrl-{T,U,J}) because the next selected file could not be handled
>> by such a subsequent file operation.
>>
>> The right way is to move the fix from this commit down into the show_diff
>> function. So that all code path add the current diff path to the list of
>> selections.
>>
>> This also simplifies helper functions for these operatione which needed
>> to handle the case whether there is only the current diff path or also
>> a selction.
>
> I think we need to think this more through, especially with input from
> Shawn, please.
>
> I have now find out, that git-gui has two selections in the file
> lists. The first is that for the current path for what we show the
> diff (the tag for this is called 'in_diff') and the the second is that
> for the current list of paths which are selected ('in_sel'). The file
> list operations 'staging', 'reverting', 'unstaging', work either on
> 'in_sel'; if that is not empty, or on 'in_diff'. The problem I've now
> realized is, that these two selections share the same visual hints,
> ie. a lightgray background.
>
> The problem I tried to solve in a8ca786991 was, that adding paths to
> the selection with Ctrl-Button-1 or Shift-cutton-1, didn't included
> the current diff path in the subsequent file list operation. But I
> would have expected it, because it was visual in the 'selection'.
>
> My current 'workaround' is to make the two selections visually
> distinguishable (and reverting a8ca786991), by using a different
> background color for the 'in_sel' tag and also the italic font, so
> that it is still possible to see whether the current diff path is in
> the selection or not:
>
> @@ -717,11 +717,11 @@ proc tk_optionMenu {w varName args} {
>  proc rmsel_tag {text} {
>        $text tag conf sel \
>                -background [$text cget -background] \
>                -foreground [$text cget -foreground] \
>                -borderwidth 0
> -       $text tag conf in_sel -background lightgray
> +       $text tag conf in_sel -background SlateGray1 -font font_diffitalic
>        bind $text <Motion> break
>        return $text
>  }
>
>  wm withdraw .
> @@ -3557,11 +3557,11 @@ if {$use_ttk} {
>        .vpane.files add .vpane.files.index -sticky news
>  }
>
>  foreach i [list $ui_index $ui_workdir] {
>        rmsel_tag $i
> -       $i tag conf in_diff -background [$i tag cget in_sel -background]
> +       $i tag conf in_diff -background lightgray
>  }
>  unset i
>
>  set files_ctxm .vpane.files.ctxm
>  menu $files_ctxm -tearoff 0
>
> I'm not very pleased with this, but at least it is now possible to
> visual recognize what files will be handled by a subsequent file list
> operation.
>
> Any input is more than welcome.
>

I think we don't resolve this now, because a8ca786991 introduced a
regression introduced in 0.16, I propose to revert it for upcoming
1.7.9 release.

Thanks.

Bert

> Regards,
> Bert

^ permalink raw reply

* Re: Commit changes to remote repository
From: Carlos Martín Nieto @ 2012-01-14 11:31 UTC (permalink / raw)
  To: ruperty; +Cc: git
In-Reply-To: <1326486589088-7185551.post@n2.nabble.com>

[-- Attachment #1: Type: text/plain, Size: 1381 bytes --]

On Fri, Jan 13, 2012 at 12:29:49PM -0800, ruperty wrote:
> Being new to git I am probably not doing things correctly so pointers in the
> right direction would be useful.
> 
> What I want to do make changes on my laptop and commit them to a remote
> repository. Here is what I have done,
> 
> 1. Created a repository on my remote linux host, in a folder of cource code,
> by,
> 
>    git init
>    git add *
>    git commit
> 
> 2. On my laptop I did a git clone pointing by ssh to the remote repo which
> downloaded all the files to my local system.
> 
> 3. I changed a file locally and did a commit.
> 
> 4. I then wanted to update the remote repo with my change, which I did with
> a git push, but that didn't work, getting this error,
> 
>     remote: error: refusing to update checked out branch:
> refs/heads/master^[[K
>     remote: error: By default, updating the current branch in a non-bare
> repository^[[K.......
> 
> 
> What am I doing wrong?

You're trying to push to a non-bare repository and change the
currently active branch, which can cause problems, so git isn't
letting you. There's an explanation of bare and non-bare at
http://bare-vs-nonbare.gitrecipes.de/ but the short and sweet is that
you should init the repo you want to use as the central point with
--bare and do modifications locally and then push there.

   cmn

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply

* Re: [PATCH 2/2] git-gui: fix applying line/ranges when the selection ends at the begin of a line
From: Bert Wesarg @ 2012-01-14 11:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Pat Thoyts, git
In-Reply-To: <7vaa5qydj7.fsf@alter.siamese.dyndns.org>

On Sat, Jan 14, 2012 at 08:00, Junio C Hamano <gitster@pobox.com> wrote:
> Bert Wesarg <bert.wesarg@googlemail.com> writes:
>
>> On Mon, Jan 9, 2012 at 14:43, Bert Wesarg <bert.wesarg@googlemail.com> wrote:
>>> Selecting also the trailing newline of a line for staging/unstaging would
>>> have resulted in also staging/unstaging of the next line.
>>
>> same here, could you please consider pushing this into the 1.7.9
>> release. I see no point in waiting for the next release.
>
> I do not use git-gui myself, so I wasn't paying much attention to these
> two patches.
>
> If these two fixes are for a new feature that was not present in v1.7.8
> but has already been merged before v1.7.8-rc1, then do please make sure to
> push them forward.
>
> On the other hand, if they are fixes for an old feature that was already
> in v1.7.8, then it is a bit too late for the next release.
>

Thanks, considering this, than these two patches have to wait for the
next release.

Bert

> Thanks.

^ permalink raw reply

* Re: git grep doesn't follow symbolic link
From: Nguyen Thai Ngoc Duy @ 2012-01-14  9:50 UTC (permalink / raw)
  To: Junio C Hamano, Pang Yan Han
  Cc: Thomas Rast, Ramkumar Ramachandra, Bertrand BENOIT, git
In-Reply-To: <7vwr8za04q.fsf@alter.siamese.dyndns.org>

(Pang's patch [1] caught my attention so I returned to the original discussion)

[1] http://thread.gmane.org/gmane.comp.version-control.git/188552

On Wed, Jan 11, 2012 at 1:22 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Thomas Rast <trast@student.ethz.ch> writes:
>
>>> I'd imagine so: symbolic links are not portable across different file
>>> systems; Git's internal representation of a symbolic link is a file
>>> containing the path of the file to be linked to.
>>
>> I'd actually welcome a fix to this general area,...
>
> Even though some platforms may lack symbolic links, where they are
> supported, they have a clear and defined meaning and that is what Git
> tracks as contents: where the link points at.
>
> So we would want our "git diff" to tell us, even if you moved without
> content modification the symbolic link target that lives somewhere on your
> filesystem but is outside the control of Git, and updated a symbolic link
> that is tracked by Git to point to a new location, that you updated the
> link. On the other hand, if you did not update a tracked symbolic link,
> even if the location the link points at that may or may not be under the
> control of Git, we do not want "git diff" to show anything. As far as that
> link is concerned, nothing has changed.
>
> Changing this would not be a fix; it would be butchering.

That's a good default. But git should allow me to say "diff the files
that symlinks point to". Link target is content from git perspective,
not from user perspective.

So instead changing the default behavior specifically for git-grep as
Pang did, I think adding --follow-symlinks option, that could be
passed to grep or any of diff family, would be a better approach.
-- 
Duy

^ permalink raw reply

* [PATCH v2 2/2] tree_entry_interesting: make recursive mode default
From: Nguyễn Thái Ngọc Duy @ 2012-01-14  9:23 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jonathan Nieder, Linus Torvalds,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <1326533003-19686-1-git-send-email-pclouds@gmail.com>

There is a bit of history behind this. Some time ago, t_e_i() only
supported prefix matching. diff-tree supported recursive and
non-recursive mode but it did not make any difference to prefix
matching.

Later on, t_e_i() gained limited recursion support to unify a similar
matching code used by git-grep. It introduced two new fields in struct
pathspec: max_depth and recursive. "recursive" field functions as a
feature switch so that this feature is off by default.

Some time after that, t_e_i() further gained wildcard support. With
wildcard matching, recursive and non-recursive diff-tree
mattered. "recursive" field was reused to distinguish recursion in
diff-tree.

This choice has a side effect that by default wildcard matching is in
non-recursive mode, which is not true. All current call sites except
"diff-tree without -r" (grep, traverse_commit_list, tree-walk and
general tree diff) prefer recursive mode.

This patch decouples the use of recursive field. The max_depth feature
switch is now controlled by max_depth_valid field. diff-tree recursion
is controlled by onelevel_only, which makes it recursive by default.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 The ugly name "nonrecursive_diff_tree" is changed to "onelevel_only".

 builtin/grep.c           |    2 +-
 cache.h                  |    3 ++-
 dir.c                    |    4 ++--
 t/t4010-diff-pathspec.sh |    8 ++++++++
 tree-diff.c              |    3 +--
 tree-walk.c              |    8 ++++----
 6 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 9ce064a..c081bf4 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -1051,7 +1051,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	paths = get_pathspec(prefix, argv + i);
 	init_pathspec(&pathspec, paths);
 	pathspec.max_depth = opt.max_depth;
-	pathspec.recursive = 1;
+	pathspec.max_depth_valid = 1;
 
 	if (show_in_pager && (cached || list.nr))
 		die(_("--open-files-in-pager only works on the worktree"));
diff --git a/cache.h b/cache.h
index 10afd71..0c4067d 100644
--- a/cache.h
+++ b/cache.h
@@ -526,7 +526,8 @@ struct pathspec {
 	const char **raw; /* get_pathspec() result, not freed by free_pathspec() */
 	int nr;
 	unsigned int has_wildcard:1;
-	unsigned int recursive:1;
+	unsigned int max_depth_valid:1;
+	unsigned int onelevel_only:1;
 	int max_depth;
 	struct pathspec_item {
 		const char *match;
diff --git a/dir.c b/dir.c
index 0a78d00..5af3567 100644
--- a/dir.c
+++ b/dir.c
@@ -258,7 +258,7 @@ int match_pathspec_depth(const struct pathspec *ps,
 	int i, retval = 0;
 
 	if (!ps->nr) {
-		if (!ps->recursive || ps->max_depth == -1)
+		if (!ps->max_depth_valid || ps->max_depth == -1)
 			return MATCHED_RECURSIVELY;
 
 		if (within_depth(name, namelen, 0, ps->max_depth))
@@ -275,7 +275,7 @@ int match_pathspec_depth(const struct pathspec *ps,
 		if (seen && seen[i] == MATCHED_EXACTLY)
 			continue;
 		how = match_pathspec_item(ps->items+i, prefix, name, namelen);
-		if (ps->recursive && ps->max_depth != -1 &&
+		if (ps->max_depth_valid && ps->max_depth != -1 &&
 		    how && how != MATCHED_FNMATCH) {
 			int len = ps->items[i].len;
 			if (name[len] == '/')
diff --git a/t/t4010-diff-pathspec.sh b/t/t4010-diff-pathspec.sh
index fbc8cd8..af5134b 100755
--- a/t/t4010-diff-pathspec.sh
+++ b/t/t4010-diff-pathspec.sh
@@ -48,6 +48,14 @@ test_expect_success \
      compare_diff_raw current expected'
 
 cat >expected <<\EOF
+:100644 100644 766498d93a4b06057a8e49d23f4068f1170ff38f 0a41e115ab61be0328a19b29f18cdcb49338d516 M	path1/file1
+EOF
+test_expect_success \
+    '"*file1" should show path1/file1' \
+    'git diff-index --cached $tree -- "*file1" >current &&
+     compare_diff_raw current expected'
+
+cat >expected <<\EOF
 :100644 100644 766498d93a4b06057a8e49d23f4068f1170ff38f 0a41e115ab61be0328a19b29f18cdcb49338d516 M	file0
 EOF
 test_expect_success \
diff --git a/tree-diff.c b/tree-diff.c
index 28ad6db..fbc683c 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -137,8 +137,7 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2,
 	enum interesting t2_match = entry_not_interesting;
 
 	/* Enable recursion indefinitely */
-	opt->pathspec.recursive = DIFF_OPT_TST(opt, RECURSIVE);
-	opt->pathspec.max_depth = -1;
+	opt->pathspec.onelevel_only = !DIFF_OPT_TST(opt, RECURSIVE);
 
 	strbuf_init(&base, PATH_MAX);
 	strbuf_add(&base, base_str, baselen);
diff --git a/tree-walk.c b/tree-walk.c
index 492c7cd..fdecacc 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -588,7 +588,7 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,
 		entry_not_interesting : all_entries_not_interesting;
 
 	if (!ps->nr) {
-		if (!ps->recursive || ps->max_depth == -1)
+		if (!ps->max_depth_valid || ps->max_depth == -1)
 			return all_entries_interesting;
 		return within_depth(base->buf + base_offset, baselen,
 				    !!S_ISDIR(entry->mode),
@@ -609,7 +609,7 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,
 			if (!match_dir_prefix(base_str, match, matchlen))
 				goto match_wildcards;
 
-			if (!ps->recursive || ps->max_depth == -1)
+			if (!ps->max_depth_valid || ps->max_depth == -1)
 				return all_entries_interesting;
 
 			return within_depth(base_str + matchlen + 1,
@@ -634,7 +634,7 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,
 				 * Match all directories. We'll try to
 				 * match files later on.
 				 */
-				if (ps->recursive && S_ISDIR(entry->mode))
+				if (!ps->onelevel_only && S_ISDIR(entry->mode))
 					return entry_interesting;
 			}
 
@@ -665,7 +665,7 @@ match_wildcards:
 		 * in future, see
 		 * http://thread.gmane.org/gmane.comp.version-control.git/163757/focus=163840
 		 */
-		if (ps->recursive && S_ISDIR(entry->mode))
+		if (!ps->onelevel_only && S_ISDIR(entry->mode))
 			return entry_interesting;
 	}
 	return never_interesting; /* No matches */
-- 
1.7.8.36.g69ee2

^ permalink raw reply related

* [PATCH v2 1/2] Document limited recursion pathspec matching with wildcards
From: Nguyễn Thái Ngọc Duy @ 2012-01-14  9:23 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jonathan Nieder, Linus Torvalds,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <1326341371-16628-1-git-send-email-pclouds@gmail.com>

It's actually unlimited recursion if wildcards are active regardless
--max-depth

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Regarding Junio's question earlier:

 >  - Shouldn't "onelevel_only" be the same as limiting to a single depth
 >   with "max_depth"?

 Doing that would change the behavior of "git grep --max-depth=0 -- 'a*'"
 from unlimited recursion currently to limited. We did not come to agree
 how --max-depth should behave with wildcards last time it was discussed,
 so it's best separating two flags (in the next patch) for now.

 Documentation/git-grep.txt |    3 +++
 tree-walk.c                |    3 +++
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 15d6711..6a8b1e3 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -79,6 +79,9 @@ OPTIONS
 --max-depth <depth>::
 	For each <pathspec> given on command line, descend at most <depth>
 	levels of directories. A negative value means no limit.
+	This option is ignored if <pathspec> contains active wildcards.
+	In other words if "a*" matches a directory named "a*",
+	"*" is matched literally so --max-depth is still effective.
 
 -w::
 --word-regexp::
diff --git a/tree-walk.c b/tree-walk.c
index f82dba6..492c7cd 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -661,6 +661,9 @@ match_wildcards:
 		/*
 		 * Match all directories. We'll try to match files
 		 * later on.
+		 * max_depth is ignored but we may consider support it
+		 * in future, see
+		 * http://thread.gmane.org/gmane.comp.version-control.git/163757/focus=163840
 		 */
 		if (ps->recursive && S_ISDIR(entry->mode))
 			return entry_interesting;
-- 
1.7.8.36.g69ee2

^ permalink raw reply related

* Re: difftool / mergetool waiting
From: Andreas Schwab @ 2012-01-14  8:38 UTC (permalink / raw)
  To: Jonathan Seng; +Cc: git
In-Reply-To: <CAG=s6FnG=3hO5jykc8s40SrCPfvJSvtEMVNBSihX5Y7T3b9SMg@mail.gmail.com>

Jonathan Seng <nekenyu@gmail.com> writes:

> If wait is false, git would fire off the tool command and proceed to
> the next then exit cleanly.

That doesn't work for git mergetool, it wouldn't be able to postprocess
the result of calling the tool command.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply

* Re: [PATCH v4 00/10] nd/clone-detached
From: Nguyen Thai Ngoc Duy @ 2012-01-14  7:40 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vhazyyduw.fsf@alter.siamese.dyndns.org>

On Fri, Jan 13, 2012 at 10:53:11PM -0800, Junio C Hamano wrote:
> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> 
> > 2012/1/14 Junio C Hamano <gitster@pobox.com>:
> >> Thanks, replaced (and updated comment strings read much better).
> >>
> >> There were some conlicts I had to resolve while merging this to 'pu'.
> >> I would appreciate it if you can eyeball it to make sure I didn't make
> >> silly mistakes there.
> >
> > Right, the conflict with nd/clone-single-branch. I kept thinking there
> > would not be conflict because clone-single-branch's big change was in
> > wanted_peer_refs() and missed write_followtags() call. The merge looks
> > good.
> 
> Hmm, 'pu' seems to fail its selftest with this merge present, though.

The commit "refuse to clone if --branch points to bogus ref" from this
series changes clone's behavior that t5500.31, which is added in
nd/clone-single-branch, relies on. This makes "make test" pass for me
on pu:

-- 8< --
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 7e85c71..c4e675f 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -283,10 +283,7 @@ test_expect_success 'clone shallow object count' '
 '
 
 test_expect_success 'clone shallow with nonexistent --branch' '
-	git clone --depth 1 --branch Z "file://$(pwd)/." shallow4 &&
-	GIT_DIR=shallow4/.git git rev-parse HEAD >actual &&
-	git rev-parse HEAD >expected &&
-	test_cmp expected actual
+	test_must_fail git clone --depth 1 --branch Z "file://$(pwd)/." shallow4
 '
 
 test_expect_success 'clone shallow with detached HEAD' '
-- 8< --

I'd rather remove the test, but removing something in a merge does not
sound wise.

Another cleaner approach is to combine the two clone series into
one. If you want to go this way, pick one as base, I'll rebase the
other on top and resend.
-- 
Duy

^ permalink raw reply related

* Re: [PATCH] grep: resolve symlinks for --no-index and untracked symlinks for --untracked
From: Pang Yan Han @ 2012-01-14  7:14 UTC (permalink / raw)
  To: git
  Cc: Bertrand BENOIT, Ramkumar Ramachandra, Thomas Rast,
	Junio C Hamano, Pang Yan Han

Sorry, this should be a PATCH/RFC

On Sat, Jan 14, 2012 at 03:10:50PM +0800, Pang Yan Han wrote:
> For a tracked symbolic link, git tracks where the symbolic link points to
> and as such, git grep does not search for patterns in where the symbolic link
> points to.
> 
> However, git grep with --no-index is supposed to work similarly to GNU grep by
> pretending that the current directory is not a git repository and hence resolve
> symbolic links.
> 
> When used with the --untracked option, untracked symbolic links should also
> be resolved.
> 
> Teach git grep to resolve symbolic links for --no-index and untracked symbolic
> links for --untracked.
> ---
>  builtin/grep.c  |   49 +++++++++++++++++++++++++++++++++++--------------
>  t/t7810-grep.sh |   35 +++++++++++++++++++++++++++++++++++
>  2 files changed, 70 insertions(+), 14 deletions(-)
> 
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 9ce064a..c7883c3 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -29,9 +29,12 @@ static int use_threads = 1;
>  #define THREADS 8
>  static pthread_t threads[THREADS];
>  
> +#define UNTRACKED 0
> +#define TRACKED 1
> +
>  static void *load_sha1(const unsigned char *sha1, unsigned long *size,
>  		       const char *name);
> -static void *load_file(const char *filename, size_t *sz);
> +static void *load_file(const char *filename, size_t *sz, int tracked);
>  
>  enum work_type {WORK_SHA1, WORK_FILE};
>  
> @@ -50,6 +53,11 @@ struct work_item {
>  	void *identifier;
>  	char done;
>  	struct strbuf out;
> +	/* indicates whether file is tracked by git.
> +	 * with --no-index, resolve all symlinks.
> +	 * with --untracked, resolve only untracked symlinks.
> +	 */
> +	int tracked;
>  };
>  
>  /* In the range [todo_done, todo_start) in 'todo' we have work_items
> @@ -113,7 +121,7 @@ static pthread_cond_t cond_result;
>  
>  static int skip_first_line;
>  
> -static void add_work(enum work_type type, char *name, void *id)
> +static void add_work(enum work_type type, char *name, void *id, int tracked)
>  {
>  	grep_lock();
>  
> @@ -125,6 +133,7 @@ static void add_work(enum work_type type, char *name, void *id)
>  	todo[todo_end].name = name;
>  	todo[todo_end].identifier = id;
>  	todo[todo_end].done = 0;
> +	todo[todo_end].tracked = tracked;
>  	strbuf_reset(&todo[todo_end].out);
>  	todo_end = (todo_end + 1) % ARRAY_SIZE(todo);
>  
> @@ -157,13 +166,13 @@ static void grep_sha1_async(struct grep_opt *opt, char *name,
>  	unsigned char *s;
>  	s = xmalloc(20);
>  	memcpy(s, sha1, 20);
> -	add_work(WORK_SHA1, name, s);
> +	add_work(WORK_SHA1, name, s, TRACKED);
>  }
>  
>  static void grep_file_async(struct grep_opt *opt, char *name,
> -			    const char *filename)
> +			    const char *filename, int tracked)
>  {
> -	add_work(WORK_FILE, name, xstrdup(filename));
> +	add_work(WORK_FILE, name, xstrdup(filename), tracked);
>  }
>  
>  static void work_done(struct work_item *w)
> @@ -226,7 +235,7 @@ static void *run(void *arg)
>  			}
>  		} else if (w->type == WORK_FILE) {
>  			size_t sz;
> -			void* data = load_file(w->identifier, &sz);
> +			void* data = load_file(w->identifier, &sz, w->tracked);
>  			if (data) {
>  				hit |= grep_buffer(opt, w->name, data, sz);
>  				free(data);
> @@ -429,7 +438,7 @@ static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,
>  	}
>  }
>  
> -static void *load_file(const char *filename, size_t *sz)
> +static void *load_file(const char *filename, size_t *sz, int tracked)
>  {
>  	struct stat st;
>  	char *data;
> @@ -441,6 +450,12 @@ static void *load_file(const char *filename, size_t *sz)
>  			error(_("'%s': %s"), filename, strerror(errno));
>  		return NULL;
>  	}
> +	/* Resolve symlink if file is not tracked */
> +	if (S_ISLNK(st.st_mode) && !tracked) {
> +		memset(&st, 0, sizeof(st));
> +		if (stat(filename, &st) < 0)
> +			goto err_ret;
> +	}
>  	if (!S_ISREG(st.st_mode))
>  		return NULL;
>  	*sz = xsize_t(st.st_size);
> @@ -459,7 +474,8 @@ static void *load_file(const char *filename, size_t *sz)
>  	return data;
>  }
>  
> -static int grep_file(struct grep_opt *opt, const char *filename)
> +static int grep_file(struct grep_opt *opt, const char *filename,
> +		int tracked)
>  {
>  	struct strbuf buf = STRBUF_INIT;
>  	char *name;
> @@ -472,14 +488,14 @@ static int grep_file(struct grep_opt *opt, const char *filename)
>  
>  #ifndef NO_PTHREADS
>  	if (use_threads) {
> -		grep_file_async(opt, name, filename);
> +		grep_file_async(opt, name, filename, tracked);
>  		return 0;
>  	} else
>  #endif
>  	{
>  		int hit;
>  		size_t sz;
> -		void *data = load_file(filename, &sz);
> +		void *data = load_file(filename, &sz, tracked);
>  		if (!data)
>  			hit = 0;
>  		else
> @@ -541,7 +557,7 @@ static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec, int
>  			hit |= grep_sha1(opt, ce->sha1, ce->name, 0);
>  		}
>  		else
> -			hit |= grep_file(opt, ce->name);
> +			hit |= grep_file(opt, ce->name, TRACKED);
>  		if (ce_stage(ce)) {
>  			do {
>  				nr++;
> @@ -658,7 +674,7 @@ static int grep_objects(struct grep_opt *opt, const struct pathspec *pathspec,
>  }
>  
>  static int grep_directory(struct grep_opt *opt, const struct pathspec *pathspec,
> -			  int exc_std)
> +			  int exc_std, int use_index)
>  {
>  	struct dir_struct dir;
>  	int i, hit = 0;
> @@ -668,12 +684,17 @@ static int grep_directory(struct grep_opt *opt, const struct pathspec *pathspec,
>  		setup_standard_excludes(&dir);
>  
>  	fill_directory(&dir, pathspec->raw);
> +	if (use_index)
> +		read_cache();
>  	for (i = 0; i < dir.nr; i++) {
>  		const char *name = dir.entries[i]->name;
>  		int namelen = strlen(name);
>  		if (!match_pathspec_depth(pathspec, name, namelen, 0, NULL))
>  			continue;
> -		hit |= grep_file(opt, dir.entries[i]->name);
> +		if (use_index && cache_name_exists(name, namelen, ignore_case))
> +			hit |= grep_file(opt, dir.entries[i]->name, TRACKED);
> +		else
> +			hit |= grep_file(opt, dir.entries[i]->name, UNTRACKED);
>  		if (hit && opt->status_only)
>  			break;
>  	}
> @@ -1083,7 +1104,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
>  		int use_exclude = (opt_exclude < 0) ? use_index : !!opt_exclude;
>  		if (list.nr)
>  			die(_("--no-index or --untracked cannot be used with revs."));
> -		hit = grep_directory(&opt, &pathspec, use_exclude);
> +		hit = grep_directory(&opt, &pathspec, use_exclude, use_index);
>  	} else if (0 <= opt_exclude) {
>  		die(_("--[no-]exclude-standard cannot be used for tracked contents."));
>  	} else if (!list.nr) {
> diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
> index 7ba5b16..fe41095 100755
> --- a/t/t7810-grep.sh
> +++ b/t/t7810-grep.sh
> @@ -647,6 +647,41 @@ test_expect_success 'inside git repository but with --no-index' '
>  	)
>  '
>  
> +test_expect_success '--no-index greps contents of targets of symlinks' '
> +	mkdir -p repo/sub &&
> +	echo hello >repo/file &&
> +	echo hello there >repo/sub/file1 &&
> +	(cd repo/sub && ln -s ../file link1 && ln -s ../file link2 &&
> +	git init && git add link1 && git commit -m "first" &&
> +	test_must_fail git grep "hello" &&
> +	cat >../expected <<-EOF &&
> +	file1:hello there
> +	link1:hello
> +	link2:hello
> +	EOF
> +	git grep --no-index "hello" >../actual &&
> +	test_cmp ../expected ../actual
> +	) &&
> +	rm -rf repo
> +'
> +
> +test_expect_success '--untracked greps targets of untracked symlinks' '
> +	mkdir -p repo/sub &&
> +	echo hello >repo/file &&
> +	echo hello there > repo/sub/file1 &&
> +	(cd repo/sub && ln -s ../file link1 && ln -s ../file link2 &&
> +	git init && git add link1 && git commit -m "first" &&
> +	test_must_fail git grep "hello" &&
> +	cat >../expected <<-EOF &&
> +	file1:hello there
> +	link2:hello
> +	EOF
> +	git grep --untracked "hello" >../actual &&
> +	test_cmp ../expected ../actual
> +	) &&
> +	rm -rf repo
> +'
> +
>  test_expect_success 'setup double-dash tests' '
>  cat >double-dash <<EOF &&
>  --
> -- 
> 1.7.9.rc0.24.ga4351

^ permalink raw reply

* [PATCH] grep: resolve symlinks for --no-index and untracked symlinks for --untracked
From: Pang Yan Han @ 2012-01-14  7:10 UTC (permalink / raw)
  To: git
  Cc: Bertrand BENOIT, Ramkumar Ramachandra, Thomas Rast,
	Junio C Hamano, Pang Yan Han

For a tracked symbolic link, git tracks where the symbolic link points to
and as such, git grep does not search for patterns in where the symbolic link
points to.

However, git grep with --no-index is supposed to work similarly to GNU grep by
pretending that the current directory is not a git repository and hence resolve
symbolic links.

When used with the --untracked option, untracked symbolic links should also
be resolved.

Teach git grep to resolve symbolic links for --no-index and untracked symbolic
links for --untracked.
---
 builtin/grep.c  |   49 +++++++++++++++++++++++++++++++++++--------------
 t/t7810-grep.sh |   35 +++++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 14 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 9ce064a..c7883c3 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -29,9 +29,12 @@ static int use_threads = 1;
 #define THREADS 8
 static pthread_t threads[THREADS];
 
+#define UNTRACKED 0
+#define TRACKED 1
+
 static void *load_sha1(const unsigned char *sha1, unsigned long *size,
 		       const char *name);
-static void *load_file(const char *filename, size_t *sz);
+static void *load_file(const char *filename, size_t *sz, int tracked);
 
 enum work_type {WORK_SHA1, WORK_FILE};
 
@@ -50,6 +53,11 @@ struct work_item {
 	void *identifier;
 	char done;
 	struct strbuf out;
+	/* indicates whether file is tracked by git.
+	 * with --no-index, resolve all symlinks.
+	 * with --untracked, resolve only untracked symlinks.
+	 */
+	int tracked;
 };
 
 /* In the range [todo_done, todo_start) in 'todo' we have work_items
@@ -113,7 +121,7 @@ static pthread_cond_t cond_result;
 
 static int skip_first_line;
 
-static void add_work(enum work_type type, char *name, void *id)
+static void add_work(enum work_type type, char *name, void *id, int tracked)
 {
 	grep_lock();
 
@@ -125,6 +133,7 @@ static void add_work(enum work_type type, char *name, void *id)
 	todo[todo_end].name = name;
 	todo[todo_end].identifier = id;
 	todo[todo_end].done = 0;
+	todo[todo_end].tracked = tracked;
 	strbuf_reset(&todo[todo_end].out);
 	todo_end = (todo_end + 1) % ARRAY_SIZE(todo);
 
@@ -157,13 +166,13 @@ static void grep_sha1_async(struct grep_opt *opt, char *name,
 	unsigned char *s;
 	s = xmalloc(20);
 	memcpy(s, sha1, 20);
-	add_work(WORK_SHA1, name, s);
+	add_work(WORK_SHA1, name, s, TRACKED);
 }
 
 static void grep_file_async(struct grep_opt *opt, char *name,
-			    const char *filename)
+			    const char *filename, int tracked)
 {
-	add_work(WORK_FILE, name, xstrdup(filename));
+	add_work(WORK_FILE, name, xstrdup(filename), tracked);
 }
 
 static void work_done(struct work_item *w)
@@ -226,7 +235,7 @@ static void *run(void *arg)
 			}
 		} else if (w->type == WORK_FILE) {
 			size_t sz;
-			void* data = load_file(w->identifier, &sz);
+			void* data = load_file(w->identifier, &sz, w->tracked);
 			if (data) {
 				hit |= grep_buffer(opt, w->name, data, sz);
 				free(data);
@@ -429,7 +438,7 @@ static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,
 	}
 }
 
-static void *load_file(const char *filename, size_t *sz)
+static void *load_file(const char *filename, size_t *sz, int tracked)
 {
 	struct stat st;
 	char *data;
@@ -441,6 +450,12 @@ static void *load_file(const char *filename, size_t *sz)
 			error(_("'%s': %s"), filename, strerror(errno));
 		return NULL;
 	}
+	/* Resolve symlink if file is not tracked */
+	if (S_ISLNK(st.st_mode) && !tracked) {
+		memset(&st, 0, sizeof(st));
+		if (stat(filename, &st) < 0)
+			goto err_ret;
+	}
 	if (!S_ISREG(st.st_mode))
 		return NULL;
 	*sz = xsize_t(st.st_size);
@@ -459,7 +474,8 @@ static void *load_file(const char *filename, size_t *sz)
 	return data;
 }
 
-static int grep_file(struct grep_opt *opt, const char *filename)
+static int grep_file(struct grep_opt *opt, const char *filename,
+		int tracked)
 {
 	struct strbuf buf = STRBUF_INIT;
 	char *name;
@@ -472,14 +488,14 @@ static int grep_file(struct grep_opt *opt, const char *filename)
 
 #ifndef NO_PTHREADS
 	if (use_threads) {
-		grep_file_async(opt, name, filename);
+		grep_file_async(opt, name, filename, tracked);
 		return 0;
 	} else
 #endif
 	{
 		int hit;
 		size_t sz;
-		void *data = load_file(filename, &sz);
+		void *data = load_file(filename, &sz, tracked);
 		if (!data)
 			hit = 0;
 		else
@@ -541,7 +557,7 @@ static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec, int
 			hit |= grep_sha1(opt, ce->sha1, ce->name, 0);
 		}
 		else
-			hit |= grep_file(opt, ce->name);
+			hit |= grep_file(opt, ce->name, TRACKED);
 		if (ce_stage(ce)) {
 			do {
 				nr++;
@@ -658,7 +674,7 @@ static int grep_objects(struct grep_opt *opt, const struct pathspec *pathspec,
 }
 
 static int grep_directory(struct grep_opt *opt, const struct pathspec *pathspec,
-			  int exc_std)
+			  int exc_std, int use_index)
 {
 	struct dir_struct dir;
 	int i, hit = 0;
@@ -668,12 +684,17 @@ static int grep_directory(struct grep_opt *opt, const struct pathspec *pathspec,
 		setup_standard_excludes(&dir);
 
 	fill_directory(&dir, pathspec->raw);
+	if (use_index)
+		read_cache();
 	for (i = 0; i < dir.nr; i++) {
 		const char *name = dir.entries[i]->name;
 		int namelen = strlen(name);
 		if (!match_pathspec_depth(pathspec, name, namelen, 0, NULL))
 			continue;
-		hit |= grep_file(opt, dir.entries[i]->name);
+		if (use_index && cache_name_exists(name, namelen, ignore_case))
+			hit |= grep_file(opt, dir.entries[i]->name, TRACKED);
+		else
+			hit |= grep_file(opt, dir.entries[i]->name, UNTRACKED);
 		if (hit && opt->status_only)
 			break;
 	}
@@ -1083,7 +1104,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		int use_exclude = (opt_exclude < 0) ? use_index : !!opt_exclude;
 		if (list.nr)
 			die(_("--no-index or --untracked cannot be used with revs."));
-		hit = grep_directory(&opt, &pathspec, use_exclude);
+		hit = grep_directory(&opt, &pathspec, use_exclude, use_index);
 	} else if (0 <= opt_exclude) {
 		die(_("--[no-]exclude-standard cannot be used for tracked contents."));
 	} else if (!list.nr) {
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 7ba5b16..fe41095 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -647,6 +647,41 @@ test_expect_success 'inside git repository but with --no-index' '
 	)
 '
 
+test_expect_success '--no-index greps contents of targets of symlinks' '
+	mkdir -p repo/sub &&
+	echo hello >repo/file &&
+	echo hello there >repo/sub/file1 &&
+	(cd repo/sub && ln -s ../file link1 && ln -s ../file link2 &&
+	git init && git add link1 && git commit -m "first" &&
+	test_must_fail git grep "hello" &&
+	cat >../expected <<-EOF &&
+	file1:hello there
+	link1:hello
+	link2:hello
+	EOF
+	git grep --no-index "hello" >../actual &&
+	test_cmp ../expected ../actual
+	) &&
+	rm -rf repo
+'
+
+test_expect_success '--untracked greps targets of untracked symlinks' '
+	mkdir -p repo/sub &&
+	echo hello >repo/file &&
+	echo hello there > repo/sub/file1 &&
+	(cd repo/sub && ln -s ../file link1 && ln -s ../file link2 &&
+	git init && git add link1 && git commit -m "first" &&
+	test_must_fail git grep "hello" &&
+	cat >../expected <<-EOF &&
+	file1:hello there
+	link2:hello
+	EOF
+	git grep --untracked "hello" >../actual &&
+	test_cmp ../expected ../actual
+	) &&
+	rm -rf repo
+'
+
 test_expect_success 'setup double-dash tests' '
 cat >double-dash <<EOF &&
 --
-- 
1.7.9.rc0.24.ga4351

^ permalink raw reply related

* Re: [PATCH 2/2] git-gui: fix applying line/ranges when the selection ends at the begin of a line
From: Junio C Hamano @ 2012-01-14  7:00 UTC (permalink / raw)
  To: Bert Wesarg; +Cc: Pat Thoyts, git
In-Reply-To: <CAKPyHN0tqQKuPONj_F9MXbgoHxeoZ7pFVSLPNWHddnA8340MGA@mail.gmail.com>

Bert Wesarg <bert.wesarg@googlemail.com> writes:

> On Mon, Jan 9, 2012 at 14:43, Bert Wesarg <bert.wesarg@googlemail.com> wrote:
>> Selecting also the trailing newline of a line for staging/unstaging would
>> have resulted in also staging/unstaging of the next line.
>
> same here, could you please consider pushing this into the 1.7.9
> release. I see no point in waiting for the next release.

I do not use git-gui myself, so I wasn't paying much attention to these
two patches.

If these two fixes are for a new feature that was not present in v1.7.8
but has already been merged before v1.7.8-rc1, then do please make sure to
push them forward.

On the other hand, if they are fixes for an old feature that was already
in v1.7.8, then it is a bit too late for the next release.

Thanks.

^ permalink raw reply

* Re: [PATCH v4 00/10] nd/clone-detached
From: Junio C Hamano @ 2012-01-14  6:53 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <CACsJy8DggQdM1aoeL+u=3Wz+5f7hi4eG=6MHXPCJZ6pOmhQJ_w@mail.gmail.com>

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> 2012/1/14 Junio C Hamano <gitster@pobox.com>:
>> Thanks, replaced (and updated comment strings read much better).
>>
>> There were some conlicts I had to resolve while merging this to 'pu'.
>> I would appreciate it if you can eyeball it to make sure I didn't make
>> silly mistakes there.
>
> Right, the conflict with nd/clone-single-branch. I kept thinking there
> would not be conflict because clone-single-branch's big change was in
> wanted_peer_refs() and missed write_followtags() call. The merge looks
> good.

Hmm, 'pu' seems to fail its selftest with this merge present, though.

^ permalink raw reply

* Re: [PATCH 2/2] git-gui: fix applying line/ranges when the selection ends at the begin of a line
From: Bert Wesarg @ 2012-01-14  5:26 UTC (permalink / raw)
  To: Pat Thoyts; +Cc: git, Bert Wesarg
In-Reply-To: <37339be035746797fcec7634e3560ffcd5b26cf3.1326116492.git.bert.wesarg@googlemail.com>

Hi Pat,

On Mon, Jan 9, 2012 at 14:43, Bert Wesarg <bert.wesarg@googlemail.com> wrote:
> Selecting also the trailing newline of a line for staging/unstaging would
> have resulted in also staging/unstaging of the next line.
>

same here, could you please consider pushing this into the 1.7.9
release. I see no point in waiting for the next release.

Thanks.

Bert

^ permalink raw reply

* Re: [PATCH 1/1] git-gui: fix hunk parsing for corner case changes
From: Bert Wesarg @ 2012-01-14  5:25 UTC (permalink / raw)
  To: Pat Thoyts; +Cc: git, Bert Wesarg
In-Reply-To: <cccd6193cf3bfe170e14270204d735a842bb8563.1326116492.git.bert.wesarg@googlemail.com>

Hi Pat,

On Mon, Jan 9, 2012 at 14:43, Bert Wesarg <bert.wesarg@googlemail.com> wrote:
> The simple hunk parsing code did not recognize hunks when there is no
> second number after the comma. Like in these cases:
>
>  @@ -1 +0,0 @@
>  -1
>
> Which resulted in this hunk header:
>
>  @@ -1 +0,1 +1 +0,0 @@
>
> Or:
>
>  @@ -1 +1 @@
>  -1
>  +2
>
> Resulted in:
>
>  @@ -1 +1 @@
>  ,1 +1 +1 @@
>  ,0 @@
>
> While trying to stage only the '-1' line.
>

could you please consider pushing this into the 1.7.9 release. I see
no point in waiting for the next release.

Thanks.

Bert

^ permalink raw reply

* difftool / mergetool waiting
From: Jonathan Seng @ 2012-01-14  5:13 UTC (permalink / raw)
  To: git

Hello,

I would like an option to difftool and mergetool that would control
waiting for a return code from the difftool: wait.


Like prompt, wait would be configurable with the difftool.name.

Like prompt and for backwards compatibility, wait would default true.


If wait is false, prompt would be overridden to be false.

If wait is false, git would fire off the tool command and proceed to
the next then exit cleanly.

If wait is true, git would behave as now: it will wait for an exit
code of each tool execution before proceeding.


Thank you,

Jonathan

^ permalink raw reply

* Re: [PATCH v4 00/10] nd/clone-detached
From: Nguyen Thai Ngoc Duy @ 2012-01-14  4:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vvcofxtv8.fsf@alter.siamese.dyndns.org>

2012/1/14 Junio C Hamano <gitster@pobox.com>:
> Thanks, replaced (and updated comment strings read much better).
>
> There were some conlicts I had to resolve while merging this to 'pu'.
> I would appreciate it if you can eyeball it to make sure I didn't make
> silly mistakes there.

Right, the conflict with nd/clone-single-branch. I kept thinking there
would not be conflict because clone-single-branch's big change was in
wanted_peer_refs() and missed write_followtags() call. The merge looks
good.
-- 
Duy

^ permalink raw reply

* Re: SVN -> Git *but* with special changes
From: Abscissa @ 2012-01-14  3:43 UTC (permalink / raw)
  To: git
In-Reply-To: <1326405138283-7181897.post@n2.nabble.com>

Someone on a different message board noticed that it seemed like it was
trying to strip and already-stripped path in git-svn.perl, so I went in
there, changed the "die" to "print", and after a "make && sudo make install"
it seems to work fine now. Probably not the proper way to fix it, but it
seems to work for me.

It does now fail to delete directories once they actually *are* deleted in
the SVN repo, which is kinda sloppy, but it doesn't hurt anything, so I can
live with that.


--
View this message in context: http://git.661346.n2.nabble.com/SVN-Git-but-with-special-changes-tp6840904p7186699.html
Sent from the git mailing list archive at Nabble.com.

^ 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