Git development
 help / color / mirror / Atom feed
* Re: [PATCH] Add a notice to the doc of git-ls-tree.
From: Petr Baudis @ 2008-07-21  0:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Steve Frécinaux, git
In-Reply-To: <7vtzek15b5.fsf@gitster.siamese.dyndns.org>

On Sun, Jul 20, 2008 at 04:53:02PM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Petr Baudis <pasky@suse.cz> writes:
> >
> >> I think that ls-tree simply shouldn't auto-fill its pathspec based on
> >> current prefix in case no pathspec was supplied. Patch to follow.
> >
> > Have you dug the list archive from mid-to-late December 2005 that prompted
> > the current behaviour (and introduction of --full-name)?  I haven't.
> 
> Now, I did:
> 
>     http://thread.gmane.org/gmane.comp.version-control.git/13028/focus=13135
> 
> I think the answer is --full-name (cf. a69dd58 (ls-tree: chomp leading
> directories when run from a subdirectory, 2005-12-23)).

I don't understand your point now.  --full-name cares only about the
displaying part; do you suggest that it should be extended to also turn
off prepending the prefix during the filtering phase? That would make a
lot of sense, if you are not worried about compatibility trouble.

-- 
			Petr "Pasky, missing something" Baudis
As in certain cults it is possible to kill a process if you know
its true name.  -- Ken Thompson and Dennis M. Ritchie

^ permalink raw reply

* Re: [PATCH] Add a notice to the doc of git-ls-tree.
From: Junio C Hamano @ 2008-07-21  0:14 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Steve Frécinaux, git
In-Reply-To: <20080721000824.GI10151@machine.or.cz>

Petr Baudis <pasky@suse.cz> writes:

> I don't understand your point now.  --full-name cares only about the
> displaying part; do you suggest that it should be extended to also turn
> off prepending the prefix...

Ah, sorry, I thought you were talking about the display part.

I never thought you would think "showing relative to tree-root" is even an
option.  That would make it inconsistent with not just the established
semantics of what the plumbing did, but also with what ls-files does.

^ permalink raw reply

* Re: [PATCH] Ensure that SSH runs in non-interactive mode
From: Jeff King @ 2008-07-21  0:14 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Johannes Schindelin, Fredrik Tolf, Keith Packard, git,
	Edward Z. Yang, Steffen Prohaska
In-Reply-To: <7v63r0bejy.fsf@gitster.siamese.dyndns.org>

On Sun, Jul 20, 2008 at 11:23:13AM -0700, Junio C Hamano wrote:

> I think that is a very sensible approach, but just like we have a few
> "built-in" function-header regexps with customization possibilities for
> the user, we might want to:
> 
>  * Have that "-x", "-T" in the command line we generate for OpenSSH;

I am slightly negative on this, because we are setting OpenSSH
preferences behind the user's back that they would not normally expect
git to be tampering with.

I think the expectation for this is that it impacts only the ssh session
used by git.  But because OpenSSH supports the concept of "master" and
"slave" sessions (i.e., it can multiplex many sessions over a single ssh
session, avoiding authentication and thus reducing latency until the
start of the session), what you do in one session can impact other
sessions. In particular, if the 'master' does not have x11 forwarding
(because it happens to be started by git), then slave connections do not
get it. So a user with X11Forwarding and ControlMaster set in his config
would usually have everything work, but bad timing with the
git-initiated session as the master would unexpectedly break his
X11Forwarding for other sessions.

I don't know how commonly the ControlMaster option for openssh is used.
I also don't know if this should simply be considered a bug in openssh,
since it silently ignores the request for X forwarding.  Personally, I
will not be affected because I don't do X forwarding by default, anyway.
But I thought I would raise the point.

-Peff

^ permalink raw reply

* Re: [PATCH 2/2] git-add -a: add all files
From: Jeff King @ 2008-07-21  0:17 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jay Soffian, Johannes Schindelin, Tarmigan, git, Michael J Gruber
In-Reply-To: <7vprp814oe.fsf@gitster.siamese.dyndns.org>

On Sun, Jul 20, 2008 at 05:06:41PM -0700, Junio C Hamano wrote:

> > But again, I haven ever felt the lack of this feature; such usage for me
> > always goes in scripts, where I am more than happy to write out "add .
> > && add -u && commit".
> 
> The reason we did not have such "feature" so far was not because somebody
> high in the git foodchain was opposed to the idea, but simply because
> nobody came up with a usable patch to do so.
> 
> I do not have anything fundamentally against "add -A" nor "commit -A".  To
> me, this is in "perhaps nice to have for some people, but I would not use
> it myself and I wouldn't bother" category, not in "I'm opposed -- it would
> promote bad workflow" cateogry.

I think I didn't make my point well; I am also not that I am opposed to
this feature. The paragraph you quoted meant to say "What I described
above with commit -A is what I think people who are asking for this
feature want. But _I_ don't actually want it, even as somebody who does
this workflow, so I might be wrong."

-Peff

^ permalink raw reply

* Re: [PATCH] Add a notice to the doc of git-ls-tree.
From: Petr Baudis @ 2008-07-21  0:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Steve Frécinaux, git
In-Reply-To: <7vljzw14br.fsf@gitster.siamese.dyndns.org>

On Sun, Jul 20, 2008 at 05:14:16PM -0700, Junio C Hamano wrote:
> I never thought you would think "showing relative to tree-root" is even an
> option.

I assume you mean "not filtering relative to tree-root"?

> That would make it inconsistent with not just the established
> semantics of what the plumbing did, but also with what ls-files does.

But ls-files always works on the index; ls-tree can work on trees,
and when you're inspecting a non-root tree object from within
a subdirectory, this behaviour can be rather unexpected. But as I said,
I'm fine with just documenting it.

-- 
				Petr "Pasky" Baudis
As in certain cults it is possible to kill a process if you know
its true name.  -- Ken Thompson and Dennis M. Ritchie

^ permalink raw reply

* Re: [PATCH 2/2] git-add -a: add all files
From: Jeff King @ 2008-07-21  0:22 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jay Soffian, Johannes Schindelin, Tarmigan, git, Michael J Gruber
In-Reply-To: <20080721001731.GC12454@sigill.intra.peff.net>

On Sun, Jul 20, 2008 at 08:17:32PM -0400, Jeff King wrote:

> I think I didn't make my point well; I am also not that I am opposed to

Urgh, bad editing. "I am also not opposed" in case it was not clear.

-Peff

^ permalink raw reply

* Re: [PATCH] git-mv: Keep moved index entries inact
From: Petr Baudis @ 2008-07-21  0:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vej5pwhub.fsf@gitster.siamese.dyndns.org>

On Sat, Jul 19, 2008 at 04:54:20PM -0700, Junio C Hamano wrote:
> Petr Baudis <pasky@suse.cz> writes:
> 
> > diff --git a/read-cache.c b/read-cache.c
> > index 1648428..70e5f57 100644
> > --- a/read-cache.c
> > +++ b/read-cache.c
> > @@ -38,6 +38,21 @@ static void replace_index_entry(struct index_state *istate, int nr, struct cache
> >  	istate->cache_changed = 1;
> >  }
> >  
> > +void rename_index_entry_at(struct index_state *istate, int nr, const char *new_name)
> > +{
> > +	struct cache_entry *old = istate->cache[nr], *new;
> > +	int namelen = strlen(new_name);
> > +
> > +	new = xmalloc(cache_entry_size(namelen));
> > +	copy_cache_entry(new, old);
> > +	new->ce_flags = (new->ce_flags & ~CE_NAMEMASK) | namelen;
> > +	memcpy(new->name, new_name, namelen);
> > +
> > +	cache_tree_invalidate_path(istate->cache_tree, old->name);
> > +	remove_index_entry_at(istate, nr);
> > +	add_index_entry(istate, new, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);
> > +}
> 
> Hmm, would this use of copy_cache_entry() kosher, I have to wonder.  This
> new copy of cache entry begins its life unhashed, doesn't it?  Shouldn't
> we be not copying its hashed/unhashed bits from the old one?
> 
> Also setting of that ce_flags looks wrong when namelen does not fit within
> the width of CE_NAMEMASK.  Shouldn't it be doing the same thing as
> create_ce_flags()?

I have to admit that I don't clearly understand all the index entry
intricacies. It is good you didn't see my earlier misguided attempts.
:-) I have patched the two mistakes you pointed out. It is too bad I
cannot simply use existing functions for this, but I want to keep a
different set of traits that either copy_cache_entry() or
create_ce_flags() assumes.

> > diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
> > index 336cfaa..6b615f8 100755
> > --- a/t/t7001-mv.sh
> > +++ b/t/t7001-mv.sh
> > @@ -156,4 +156,61 @@ test_expect_success 'absolute pathname outside should fail' '(
> >  
> >  )'
> >  
> > +# git mv meets angry Git maintainer
> 
> What's this comment about?

Oh. Well, you sounded agitated in your original mail, but this actually
just slipped through. :-)

> > +test_expect_success 'git mv should not change sha1 of moved cache entry' '
> > +
> > +	rm -fr .git &&
> > +	git init &&
> > +	echo 1 >dirty &&
> > +	git add dirty &&
> > +	entry="$(git ls-files --stage dirty | cut -f 1)"
> 
> "rev-parse :dirty"?

I want to make sure the whole index entry is intact, not just the sha1.

-- 
				Petr "Pasky" Baudis
As in certain cults it is possible to kill a process if you know
its true name.  -- Ken Thompson and Dennis M. Ritchie

^ permalink raw reply

* [PATCHv2] git-mv: Keep moved index entries inact
From: Petr Baudis @ 2008-07-21  0:25 UTC (permalink / raw)
  To: gitster; +Cc: git
In-Reply-To: <20080721002354.GK10151@machine.or.cz>

The rewrite of git-mv from a shell script to a builtin was perhaps
a little too straightforward: the git add and git rm queues were
emulated directly, which resulted in a rather complicated code and
caused an inconsistent behaviour when moving dirty index entries;
git mv would update the entry based on working tree state,
except in case of overwrites, where the new entry would still have
sha1 of the old file.

This patch introduces rename_index_entry_at() into the index toolkit,
which will rename an entry while removing any entries the new entry
might render duplicate. This is then used in git mv instead
of all the file queues, resulting in a major simplification
of the code and an inevitable change in git mv -n output format.

A new test has been added to the testsuite to reflect this change.
Also, based on suggestion by Junio about desired symlink behaviour
of git mv, I have added two tests for that; however, I do not have
need or desire to spend time fixing this, so they are expected
to fail for now until someone gets around to fixing that.

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 builtin-mv.c  |   62 ++++++++-------------------------------------------------
 cache.h       |    2 ++
 read-cache.c  |   17 ++++++++++++++++
 t/t7001-mv.sh |   55 +++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 83 insertions(+), 53 deletions(-)

diff --git a/builtin-mv.c b/builtin-mv.c
index 33ad082..28ebc9c 100644
--- a/builtin-mv.c
+++ b/builtin-mv.c
@@ -36,17 +36,6 @@ static const char **copy_pathspec(const char *prefix, const char **pathspec,
 	return get_pathspec(prefix, result);
 }
 
-static void show_list(const char *label, struct path_list *list)
-{
-	if (list->nr > 0) {
-		int i;
-		printf("%s", label);
-		for (i = 0; i < list->nr; i++)
-			printf("%s%s", i > 0 ? ", " : "", list->items[i].path);
-		putchar('\n');
-	}
-}
-
 static const char *add_slash(const char *path)
 {
 	int len = strlen(path);
@@ -75,11 +64,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 	const char **source, **destination, **dest_path;
 	enum update_mode { BOTH = 0, WORKING_DIRECTORY, INDEX } *modes;
 	struct stat st;
-	struct path_list overwritten = {NULL, 0, 0, 0};
 	struct path_list src_for_dst = {NULL, 0, 0, 0};
-	struct path_list added = {NULL, 0, 0, 0};
-	struct path_list deleted = {NULL, 0, 0, 0};
-	struct path_list changed = {NULL, 0, 0, 0};
 
 	git_config(git_default_config, NULL);
 
@@ -189,7 +174,6 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 							" will overwrite!\n",
 							bad);
 					bad = NULL;
-					path_list_insert(dst, &overwritten);
 				} else
 					bad = "Cannot overwrite";
 			}
@@ -218,6 +202,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 	for (i = 0; i < argc; i++) {
 		const char *src = source[i], *dst = destination[i];
 		enum update_mode mode = modes[i];
+		int pos;
 		if (show_only || verbose)
 			printf("Renaming %s to %s\n", src, dst);
 		if (!show_only && mode != INDEX &&
@@ -227,45 +212,16 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 		if (mode == WORKING_DIRECTORY)
 			continue;
 
-		assert(cache_name_pos(src, strlen(src)) >= 0);
-
-		path_list_insert(src, &deleted);
-		/* destination can be a directory with 1 file inside */
-		if (path_list_has_path(&overwritten, dst))
-			path_list_insert(dst, &changed);
-		else
-			path_list_insert(dst, &added);
+		pos = cache_name_pos(src, strlen(src));
+		assert(pos >= 0);
+		if (!show_only)
+			rename_cache_entry_at(pos, dst);
 	}
 
-	if (show_only) {
-		show_list("Changed  : ", &changed);
-		show_list("Adding   : ", &added);
-		show_list("Deleting : ", &deleted);
-	} else {
-		for (i = 0; i < changed.nr; i++) {
-			const char *path = changed.items[i].path;
-			int j = cache_name_pos(path, strlen(path));
-			struct cache_entry *ce = active_cache[j];
-
-			if (j < 0)
-				die ("Huh? Cache entry for %s unknown?", path);
-			refresh_cache_entry(ce, 0);
-		}
-
-		for (i = 0; i < added.nr; i++) {
-			const char *path = added.items[i].path;
-			if (add_file_to_cache(path, verbose ? ADD_CACHE_VERBOSE : 0))
-				die("updating index entries failed");
-		}
-
-		for (i = 0; i < deleted.nr; i++)
-			remove_file_from_cache(deleted.items[i].path);
-
-		if (active_cache_changed) {
-			if (write_cache(newfd, active_cache, active_nr) ||
-			    commit_locked_index(&lock_file))
-				die("Unable to write new index file");
-		}
+	if (active_cache_changed) {
+		if (write_cache(newfd, active_cache, active_nr) ||
+		    commit_locked_index(&lock_file))
+			die("Unable to write new index file");
 	}
 
 	return 0;
diff --git a/cache.h b/cache.h
index a779d92..6f1d003 100644
--- a/cache.h
+++ b/cache.h
@@ -260,6 +260,7 @@ static inline void remove_name_hash(struct cache_entry *ce)
 #define unmerged_cache() unmerged_index(&the_index)
 #define cache_name_pos(name, namelen) index_name_pos(&the_index,(name),(namelen))
 #define add_cache_entry(ce, option) add_index_entry(&the_index, (ce), (option))
+#define rename_cache_entry_at(pos, new_name) rename_index_entry_at(&the_index, (pos), (new_name))
 #define remove_cache_entry_at(pos) remove_index_entry_at(&the_index, (pos))
 #define remove_file_from_cache(path) remove_file_from_index(&the_index, (path))
 #define add_to_cache(path, st, flags) add_to_index(&the_index, (path), (st), (flags))
@@ -370,6 +371,7 @@ extern int index_name_pos(const struct index_state *, const char *name, int name
 #define ADD_CACHE_JUST_APPEND 8		/* Append only; tree.c::read_tree() */
 extern int add_index_entry(struct index_state *, struct cache_entry *ce, int option);
 extern struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int really);
+extern void rename_index_entry_at(struct index_state *, int pos, const char *new_name);
 extern int remove_index_entry_at(struct index_state *, int pos);
 extern int remove_file_from_index(struct index_state *, const char *path);
 #define ADD_CACHE_VERBOSE 1
diff --git a/read-cache.c b/read-cache.c
index 1648428..e93ee3c 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -38,6 +38,23 @@ static void replace_index_entry(struct index_state *istate, int nr, struct cache
 	istate->cache_changed = 1;
 }
 
+void rename_index_entry_at(struct index_state *istate, int nr, const char *new_name)
+{
+	struct cache_entry *old = istate->cache[nr], *new;
+	int namelen = strlen(new_name);
+
+	new = xmalloc(cache_entry_size(namelen));
+	copy_cache_entry(new, old);
+	new->ce_flags = (new->ce_flags & ~CE_HASHED) | CE_UNHASHED;
+	new->ce_flags = (new->ce_flags & ~CE_NAMEMASK)
+	                | (namelen >= CE_NAMEMASK ? CE_NAMEMASK : namelen);
+	memcpy(new->name, new_name, namelen);
+
+	cache_tree_invalidate_path(istate->cache_tree, old->name);
+	remove_index_entry_at(istate, nr);
+	add_index_entry(istate, new, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);
+}
+
 /*
  * This only updates the "non-critical" parts of the directory
  * cache, ie the parts that aren't tracked by GIT, and only used
diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index 336cfaa..7e47931 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -156,4 +156,59 @@ test_expect_success 'absolute pathname outside should fail' '(
 
 )'
 
+test_expect_success 'git mv should not change sha1 of moved cache entry' '
+
+	rm -fr .git &&
+	git init &&
+	echo 1 >dirty &&
+	git add dirty &&
+	entry="$(git ls-files --stage dirty | cut -f 1)"
+	git mv dirty dirty2 &&
+	[ "$entry" = "$(git ls-files --stage dirty2 | cut -f 1)" ] &&
+	echo 2 >dirty2 &&
+	git mv dirty2 dirty &&
+	[ "$entry" = "$(git ls-files --stage dirty | cut -f 1)" ]
+
+'
+
+rm -f dirty dirty2
+
+test_expect_failure 'git mv should overwrite symlink to a file' '
+
+	rm -fr .git &&
+	git init &&
+	echo 1 >moved &&
+	ln -s moved symlink &&
+	git add moved symlink &&
+	! git mv moved symlink &&
+	git mv -f moved symlink &&
+	[ ! -e moved ] &&
+	[ -f symlink ] &&
+	[ $(cat symlink) = 1 ] &&
+	git diff-files --quiet
+
+'
+
+rm -f moved symlink
+
+test_expect_failure 'git mv should follow symlink to a directory' '
+
+	rm -fr .git &&
+	git init &&
+	echo 1 >moved &&
+	mkdir -p dir &&
+	touch dir/.keep &&
+	ln -s dir symlink &&
+	git add moved dir/.keep symlink &&
+	git mv moved symlink &&
+	[ ! -e moved ] &&
+	[ -f symlink/moved ] &&
+	[ $(cat symlink/moved) = 1 ] &&
+	[ "$(ls dir)" = "$(ls symlink)" ] &&
+	git diff-files --quiet
+
+'
+
+rm -rf moved symlink dir
+
 test_done

^ permalink raw reply related

* git-shell build error
From: SungHyun Nam @ 2008-07-21  0:29 UTC (permalink / raw)
  To: git

Hello,

If NO_SETENV is defined, git-shell cannot be built.
Simply adding the 'compat/setenv.o' to the make rule fixes the problem.

Regards,

[master] ~/srcs/git[30]$ LANG= make
     LINK git-shell
Undefined                       first referenced
  symbol                             in file
gitsetenv                           exec_cmd.o
ld: fatal: Symbol referencing errors. No output written to git-shell
collect2: ld returned 1 exit status
make: *** [git-shell] Error 1

^ permalink raw reply

* [PATCH] Fix git-shell build error when NO_SETENV is defined
From: Stephan Beyer @ 2008-07-21  0:43 UTC (permalink / raw)
  To: SungHyun Nam; +Cc: git, gitster, Stephan Beyer
In-Reply-To: <g60la4$diu$1@ger.gmane.org>

If NO_SETENV is defined, git-shell could not be built.

Thanks to SungHyun Nam for the hint.

Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
---

This was my mistake. I haven't tested with several build options.
Now I've tested with
	NO_SETENV=1 NO_EXPAT=1 NO_MEMMEM=1 NO_STRTOUMAX=1 NO_MKDTEMP=1
	NO_SYS_SELECT_H=1 NO_SYMLINK_HEAD=1
and compat/setenv.o seems to be the only one that was missing.

Regards.

 Makefile |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index 2b670d7..b650ee6 100644
--- a/Makefile
+++ b/Makefile
@@ -1203,7 +1203,8 @@ git-http-push$X: revision.o http.o http-push.o $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
 		$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
 
-git-shell$X: compat/strlcpy.o abspath.o ctype.o exec_cmd.o quote.o strbuf.o usage.o wrapper.o shell.o
+git-shell$X: compat/strlcpy.o compat/setenv.o abspath.o ctype.o exec_cmd.o \
+	     quote.o strbuf.o usage.o wrapper.o shell.o
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^)
 
 $(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H)
-- 
1.5.6.3.390.g7b30

^ permalink raw reply related

* Re: [PATCH v2 1/4] builtin-add.c: restructure the code for maintainability
From: Johannes Schindelin @ 2008-07-21  0:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <1216534144-23826-1-git-send-email-gitster@pobox.com>

Hi,

On Sat, 19 Jul 2008, Junio C Hamano wrote:


> diff --git a/builtin-add.c b/builtin-add.c
> index bf13aa3..9b2ee8c 100644
> --- a/builtin-add.c
> +++ b/builtin-add.c
> [...]
> +	/*
> +	 * If we are adding new files, we need to scan the working
> +	 * tree to find the ones that match pathspecs; this needs
> +	 * to be done before we read the index.
> +	 */

This comment left me scratching my head.  While I do see a breakage when 
reading the index first, I had the impression that it should not.

I can only imagine that the other users of read_directory() depend on some 
funny interaction between the index and treat_directory().

Ciao,
Dscho

^ permalink raw reply

* Re: [GSoC] What is status of Git's Google Summer of Code 2008 projects?
From: Sam Vilain @ 2008-07-21  0:55 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git, Joshua Roys
In-Reply-To: <200807210029.31543.jnareb@gmail.com>

On Mon, 2008-07-21 at 00:29 +0200, Jakub Narebski wrote:
> 1. GitTorrent
>  
> Student: Joshua Roys
> Mentor: Sam Vilain
> 
> I never got more response than "it is going slower than I would like, 
> [...] Other than that, it's going well, I think." from Joshua Roys.

> Mailing list archives for gittorrent mailing list doesn't show anything 
> interesting, either (last post is from 2007).
>   http://lists.utsl.gen.nz/pipermail/gittorrent/

That's a valid complaint.  I've posted a summary of the project status
there, and will keep as much related discussion as appropriate on-list
from here.

> Besides canonical repository gitweb
>   http://utsl.gen.nz/gitweb/?p=VCS-Git-Torrent
> there is also mirror at
>   http://repo.or.cz/w/VCS-Git-Torrent.git
> 
> There is some activity there... but no summary of it anywhere I could 
> find.

git-log | git-shortlog?  ;)

Sam.

^ permalink raw reply

* [PATCH/RFC] git add: do not add files from a submodule
From: Johannes Schindelin @ 2008-07-21  0:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0807201529150.3305@eeepc-johanness>


It comes quite as a surprise to an unsuspecting Git user that calling
"git add submodule/file" (which is a mistake, alright) _removes_
the submodule in the index, and adds the file.  Instead, complain loudly.

While at it, be nice when the user said "git add submodule/" which is
most likely the consequence of tab-completion, and stage the submodule,
instead of trying to add the contents of that directory.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	This would need a companion patch for "git diff submodule/",
	obviously.  However, revision.c does not need a valid cache,
	usually.  So I am hesitant.

	Oh, and I am sure somebody will come up with a more elegant 
	solution to this problem.  I sure do not, having smashed my head 
	against the wall for a few hours.

 builtin-add.c              |   42 +++++++++++++++++++++++++++++++++++++++---
 t/t7400-submodule-basic.sh |   18 ++++++++++++++++++
 2 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index 6f5672a..9453557 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -50,6 +50,33 @@ static void prune_directory(struct dir_struct *dir, const char **pathspec, int p
         free(seen);
 }
 
+static void treat_gitlinks(const char **pathspec, struct index_state *index)
+{
+	int i;
+
+	if (!pathspec || !*pathspec)
+		return;
+
+	for (i = 0; i < index->cache_nr; i++) {
+		struct cache_entry *ce = index->cache[i];
+		if (S_ISGITLINK(ce->ce_mode)) {
+			int len = ce_namelen(ce), j;
+			for (j = 0; pathspec[j]; j++) {
+				int len2 = strlen(pathspec[j]);
+				if (len2 <= len || pathspec[j][len] != '/' ||
+				    memcmp(ce->name, pathspec[j], len))
+					continue;
+				if (len2 == len + 1)
+					/* strip trailing slash */
+					pathspec[j] = xstrndup(ce->name, len);
+				else
+					die ("Path '%s' is in submodule '%.*s'",
+						pathspec[j], len, ce->name);
+			}
+		}
+	}
+}
+
 static void fill_directory(struct dir_struct *dir, const char **pathspec,
 		int ignored_too)
 {
@@ -245,6 +272,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 	int flags;
 	int add_new_files;
 	int require_pathspec;
+	struct index_state index;
 
 	argc = parse_options(argc, argv, builtin_add_options,
 			  builtin_add_usage, 0);
@@ -283,12 +311,20 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 	 * If we are adding new files, we need to scan the working
 	 * tree to find the ones that match pathspecs; this needs
 	 * to be done before we read the index.
+	 *
+	 * However, to avoid adding files from submodules, we have to
+	 * read the index first.  So read the index into a local
+	 * variable, and set the global index after fill_directory().
 	 */
-	if (add_new_files)
-		fill_directory(&dir, pathspec, ignored_too);
 
-	if (read_cache() < 0)
+	memset(&index, 0, sizeof(index));
+	if (read_index(&index) < 0)
 		die("index file corrupt");
+	treat_gitlinks(pathspec, &index);
+
+	if (add_new_files)
+		fill_directory(&dir, pathspec, ignored_too);
+	the_index = index;
 
 	if (refresh_only) {
 		refresh(verbose, pathspec);
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index cbc0c34..6da2545 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -209,4 +209,22 @@ test_expect_success 'update --init' '
 
 '
 
+test_expect_success 'do not add files from a submodule' '
+
+	git reset --hard &&
+	test_must_fail git add init/a
+
+'
+
+test_expect_success 'gracefully add submodule with a trailing slash' '
+
+	commit=$(cd init &&
+	 echo b > a &&
+	 git commit -m update a >/dev/null &&
+	 git rev-parse HEAD) &&
+	git add init/ &&
+	test_must_fail git diff --exit-code --cached init
+
+'
+
 test_done
-- 
1.5.6.2.516.g22071

^ permalink raw reply related

* Re: [GSoC] What is status of Git's Google Summer of Code 2008 projects?
From: Johannes Schindelin @ 2008-07-21  1:05 UTC (permalink / raw)
  To: Sam Vilain; +Cc: Jakub Narebski, git, Joshua Roys
In-Reply-To: <1216601739.6523.48.camel@maia.lan>

Hi,

On Mon, 21 Jul 2008, Sam Vilain wrote:

> On Mon, 2008-07-21 at 00:29 +0200, Jakub Narebski wrote:
> > 1. GitTorrent
> >  
> > [...]
> >
> > Besides canonical repository gitweb
> >   http://utsl.gen.nz/gitweb/?p=VCS-Git-Torrent
> > there is also mirror at
> >   http://repo.or.cz/w/VCS-Git-Torrent.git
> > 
> > There is some activity there... but no summary of it anywhere I could 
> > find.
> 
> git-log | git-shortlog?  ;)

Please note that one of the reasons why last year's libgit-thin project 
has not been merged, or even been useful to anybody else, because it has 
been developed in too private a manner.  Sad.

I would appreciate it, therefore, to keep the progress way more public 
than it is now.  That is, either you or your student should be not 
invisible.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH v2] Ensure that SSH runs in non-interactive mode
From: Johannes Schindelin @ 2008-07-21  1:15 UTC (permalink / raw)
  To: Fredrik Tolf; +Cc: git
In-Reply-To: <1216598432-18553-1-git-send-email-fredrik@dolda2000.com>

Hi,

On Mon, 21 Jul 2008, Fredrik Tolf wrote:

> I'm following my previous SSH patch up with this one, which should at
> least solve the problems discussed, and probably some more. If anything,
> it might be considered a bit overkill for the problem at hand.

I am not assuming it is overkill, but since you do not reuse functions 
such as strbuf_expand() and split_cmdline(), your patch ends up pretty 
large.

And since you use very short and undescriptive variable names, with ugly 
assignments inside arithmetic expressions, I will be less likely 
reviewing it in detail.

> I assume it might have to be documented as well, if people approve of it.

Catch 22.  Since you have not documented what %P should be useful for, 
people might not approve of the patch, because they do not understand what
it is supposed to do.

People like me,
Dscho

^ permalink raw reply

* Re: [PATCH] git-mv: Keep moved index entries inact
From: Johannes Schindelin @ 2008-07-21  1:20 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Junio C Hamano, git
In-Reply-To: <20080721002354.GK10151@machine.or.cz>

Hi,

On Mon, 21 Jul 2008, Petr Baudis wrote:

> On Sat, Jul 19, 2008 at 04:54:20PM -0700, Junio C Hamano wrote:
> > Petr Baudis <pasky@suse.cz> writes:
> > 
> > > +test_expect_success 'git mv should not change sha1 of moved cache entry' '
> > > +
> > > +	rm -fr .git &&
> > > +	git init &&
> > > +	echo 1 >dirty &&
> > > +	git add dirty &&
> > > +	entry="$(git ls-files --stage dirty | cut -f 1)"
> > 
> > "rev-parse :dirty"?
> 
> I want to make sure the whole index entry is intact, not just the sha1.

"rev-parse :dirty" will have to read the index to get at the object name 
of "dirty".  So there you have your index validation for you.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Fix git-shell build error when NO_SETENV is defined
From: Johannes Schindelin @ 2008-07-21  1:23 UTC (permalink / raw)
  To: Stephan Beyer; +Cc: SungHyun Nam, git, gitster
In-Reply-To: <1216601017-7871-1-git-send-email-s-beyer@gmx.net>

Hi,

On Mon, 21 Jul 2008, Stephan Beyer wrote:

> If NO_SETENV is defined, git-shell could not be built.
> 
> Thanks to SungHyun Nam for the hint.
> 
> Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
> ---
> 
> This was my mistake. I haven't tested with several build options.
> Now I've tested with
> 	NO_SETENV=1 NO_EXPAT=1 NO_MEMMEM=1 NO_STRTOUMAX=1 NO_MKDTEMP=1
> 	NO_SYS_SELECT_H=1 NO_SYMLINK_HEAD=1
> and compat/setenv.o seems to be the only one that was missing.

Funny.  It was not 24 hours ago that Hannes reported a related issue.  And 
he was testing with different options.

His fix to include COMPAT_OBJECTS made much more sense, too, than to pick 
selectively a file here and a file there and then hoping that you catch 
all.

Ciao,
Dscho

^ permalink raw reply

* Re: [HACK] t/test-lib.sh HACK: Add -s/--show-hack to test suite.
From: Stephan Beyer @ 2008-07-21  1:24 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0807131518370.4816@eeepc-johanness>

Hi,

Johannes Schindelin wrote:
> You will have to scroll back a few lines to see exactly what failed, but 
> you will see the exact commands (also of functions that were called), 
> together with their return values (i.e. what the function output, and what 
> was assigned to variables).

I've tested again and found it now. Thanks!

It's a littler harder to see, but now I know that I should look for
the last "+ eval_ret=" line, ...that makes it easier ;)

Puh, now I can finally move this thread to the archives :)

Regards.

-- 
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F

^ permalink raw reply

* Re: [PATCH] Fix git-shell build error when NO_SETENV is defined
From: Stephan Beyer @ 2008-07-21  1:29 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: SungHyun Nam, git, gitster
In-Reply-To: <alpine.DEB.1.00.0807210321190.3305@eeepc-johanness>

Johannes Schindelin wrote:
> Funny.  It was not 24 hours ago that Hannes reported a related issue.  And 
> he was testing with different options.

Oh, seems that I have missed that topic. Gna :)

But fine if everything is working again then.

Regards.

-- 
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F

^ permalink raw reply

* Re: [PATCH/rfc] git-svn.perl: workaround assertions in svn library 1.5.0
From: Eric Wong @ 2008-07-21  1:29 UTC (permalink / raw)
  To: Dmitry Potapov; +Cc: Junio C Hamano, Gerrit Pape, git
In-Reply-To: <20080720201407.GM2925@dpotapov.dyndns.org>

Dmitry Potapov <dpotapov@gmail.com> wrote:
> On Sat, Jul 19, 2008 at 06:27:36PM -0700, Junio C Hamano wrote:
> > 
> > So what's the conclusion of this issue?
> > 
> > I'll just revert 2fe403e (git-svn.perl: workaround assertions in svn
> > library 1.5.0, 2008-07-06) for 1.6.0-rc0 unless I hear better
> > suggestions.
> 
> I have tested the change that I proposed, and it seems to solve the
> problem and, as far as I can tell, no other correction is necessary.
> Yet, I don't really understand git-svn well, so I could be wrong.
> 
> Reverting 2fe403e will only help users of svn library 1.4, while all
> new linux distributives, which will include Git 1.6.0, are going to
> install svn library 1.5.0, and if you use svn library 1.5.0, reverting
> 2fe403e does not fix anything but only add one more bug. Thus, unless
> we are going to require to install git-svn only with svn library 1.4,
> reverting this change does not seem to be very helpful for most users.
> 
> So, I hope my patch is better solution...
> 
> Dmitry

Thanks Dmitry,

Your patch works for me on 1.4.3, so if it works with
1.5.0, consider it: Acked-by: Eric Wong <normalperson@yhbt.net>

-- 
Eric Wong

^ permalink raw reply

* Re: [PATCH] Fix git-shell build error when NO_SETENV is defined
From: Johannes Schindelin @ 2008-07-21  1:33 UTC (permalink / raw)
  To: Stephan Beyer; +Cc: SungHyun Nam, git, gitster
In-Reply-To: <20080721012928.GG5950@leksak.fem-net>

Hi,

On Mon, 21 Jul 2008, Stephan Beyer wrote:

> Johannes Schindelin wrote:
> > Funny.  It was not 24 hours ago that Hannes reported a related issue.  And 
> > he was testing with different options.
> 
> Oh, seems that I have missed that topic. Gna :)
> 
> But fine if everything is working again then.

No, it is not.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH v2] Ensure that SSH runs in non-interactive mode
From: Fredrik Tolf @ 2008-07-21  1:44 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0807210310330.3305@eeepc-johanness>

On Mon, 2008-07-21 at 03:15 +0200, Johannes Schindelin wrote:
> Hi,
> 
> On Mon, 21 Jul 2008, Fredrik Tolf wrote:
> 
> > I'm following my previous SSH patch up with this one, which should at
> > least solve the problems discussed, and probably some more. If anything,
> > it might be considered a bit overkill for the problem at hand.
> 
> I am not assuming it is overkill, but since you do not reuse functions 
> such as strbuf_expand() and split_cmdline(), your patch ends up pretty 
> large.

Thanks! I didn't know that there was a split_cmdline. I will use it and
resubmit.

> And since you use very short and undescriptive variable names, with ugly 
> assignments inside arithmetic expressions, I will be less likely 
> reviewing it in detail.

I actually use those assignments for clarity. IMO, it is more clear to
have one line which clearly initializes a buffer (and from which the
exact details of the buffer initialization can still be read), than to
litter an algorithm with lots of auxiliary lines that obfuscate what the
code really does.

Seeing how you disagree, though, I'll change that too when I'm at it.

> > I assume it might have to be documented as well, if people approve of it.
> 
> Catch 22.  Since you have not documented what %P should be useful for, 
> people might not approve of the patch, because they do not understand what
> it is supposed to do.

Yes, that would be a problem. Here's some makeshift documentation:

The string specified in core.sshcommand is first checked if it matches
any of the built-in templates, in which case it is expanded (I've added
the templates "openssh" and "plink" by default). When used, the string
is split into words, each of which is processed as follows:

* If a word is %p, it is replaced by the port number, if specified.
  If the port number is not specified, the word is deleted.
* If a word is %h, it is replaced by the remote host name.
* If a word begins with %P, it is deleted if no port number is
  specified. This is to allow for specifying different port number
  flags for different SSH implementations. The syntax is a bit ugly,
  but I cannot really think of anything that would look better.
  If a port number has been specified, the leading %P is simply deleted.

The result is used, along with the command to run on the remote side, as
the SSH command line.

Fredrik Tolf

^ permalink raw reply

* Re: [PATCH/rfc] git-svn.perl: workaround assertions in svn library 1.5.0
From: Junio C Hamano @ 2008-07-21  1:54 UTC (permalink / raw)
  To: Eric Wong; +Cc: Dmitry Potapov, Gerrit Pape, git
In-Reply-To: <20080721012955.GA14129@untitled>

Thanks, both.

^ permalink raw reply

* Re: [PATCH 2/2] git-add -a: add all files
From: Jay Soffian @ 2008-07-21  2:11 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, Tarmigan, git, Michael J Gruber
In-Reply-To: <7v4p6k8l36.fsf@gitster.siamese.dyndns.org>

On Sun, Jul 20, 2008 at 2:30 PM, Junio C Hamano <gitster@pobox.com> wrote:
> I do not understand either of you.  If for whatever reason "add -A" makes
> sense in your workflow, it's a sign that you are extremely disciplined
> that changes in your working tree at one point of time where you would
> issue "add -A" are concentrated on a single topic, and at one of such
> points you may want to commit.  For such a disciplined person, "commit -a"
> would make perfect sense there.
>
> So for such people who would find "add -A" useful, "commit -a" will not be
> "unrelated changes in the same commit".  And for such people, I would even
> say "commit -A" would be even more useful, too.

Hah, it's Sunday and my brain wasn't awake. You're right, "commit -a"
complements when I'd use "add -a" -- namely, when I have a branch that
is tracking a non-git source: either files I'm rsyncing from another
VCS or drops I'm getting as tarballs. (I'm aware of import-tars.perl.)

j.

^ permalink raw reply

* Re: [GSoC] What is status of Git's Google Summer of Code 2008 projects?
From: Shawn O. Pearce @ 2008-07-21  3:22 UTC (permalink / raw)
  To: Jakub Narebski
  Cc: git, Sam Vilain, Joshua Roys, Sverre Rabbelier, Sverre Rabbelier,
	David Symonds, Lea Wiemann, John Hawley, Marek Zawirski,
	Miklos Vajna, Johannes Schindelin, Stephan Beyer,
	Christian Couder, Daniel Barkalow
In-Reply-To: <200807210029.31543.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> wrote:
> 1. GitTorrent
>  
> Student: Joshua Roys
> Mentor: Sam Vilain
...
> There is some activity there... but no summary of it anywhere I could 
> find. (I wonder if this was the project Johannes and Shawn were talking 
> about of "going dark" in GSoC 2008 podcast 018...)

Yes, this is the project I referred to in the podcast about going dark.
 
> 4. Eclipse plugin push support
>  
> Student: Marek Zawirski
> Mentor: Shawn O. Pearce
> 
> [...] Marek and I simply decided that protocol support was more
> important than really tight network transport at this point in time.

Correction, the "I" in "Marek and I" isn't Jakub, its Shawn.  This
is just an editing mistake due to copy and paste from earlier thread.
Apparently my original paragraph here was already a nice summary of
the projects decisions thus far.

I'm likely to be offline much of the rest of this week (I got lucky
and found an open access point just now) but Marek is actively
working on user interface for push support, and I think if he finds
the time is considering adding delta generation.  That will be a
lot more time consuming as I think he needs to go back to original
academic papers to learn an LCS algorithm and then implement that.

Copyright and licenses around libxdiff and delta-diff.c won't allow
us to directly port the diff code to Java and our BSD license.  No,
I don't want to start a BSD-GPL license war.  Our decision to go
BSD in jgit may be a thorn in our side in areas such as this, but it
is probably better for our long-term goals of working more directly
with the Eclipse Foundation and perhaps also the NetBeans folks.

> SUMMARY:
> ========
> From those projects, "git-merge builtin" did what it was meant to do 
> already.  "Eclipse plugin push support" and "git-statistics" did 
> minimum what it was meant to do already, and it looks like it would be 
> finished before August 11.  "Gitweb caching" is after first round of 
> patches, "git-sequencer" looks like already done; I don't know what is 
> the state of "GitTorrent" project.
> 
> Please correct any mistakes in this summary / writeup.  Thanks in 
> advance.

I think this is a pretty good summary.  I want to go through the
mid-term evaluations and summarize those for the mailing list but
I have not had a chance to do that yet.  With network being spotty
for the rest of this week it probably won't happen until the weekend.

I think the quick summary is our students and our mentors think
their projects are going well.  Jakub's summary above suggests
very much the same thing.  Its hard to claim a GSoC project isn't
meeting its goals when the code is already merged, or is at least
under active patch review.  ;-)

-- 
Shawn.

^ 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