Git development
 help / color / mirror / Atom feed
* Re: [PATCH] ls-files: fix overeager pathspec optimization
From: Junio C Hamano @ 2010-01-09  7:16 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Jeff King, Michael J Gruber, Jon Schewe, spearce, git
In-Reply-To: <alpine.LFD.2.00.1001081701570.7821@localhost.localdomain>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> I'd have to see both patches to be able to tell. I do admit that while I
> acked your patch, it sure ain't _pretty_ to do that special odd
> "has_leading_ignored_dir()" thing.

Revised patch (v4) series is coming shortly.

^ permalink raw reply

* Re: How to push changes from clone back to master
From: Junio C Hamano @ 2010-01-09  6:49 UTC (permalink / raw)
  To: Simon Chu; +Cc: Tomas Carnecky, git@vger.kernel.org
In-Reply-To: <2394EBAE-BA99-4817-93E3-D5441D6DA46B@gmail.com>

Simon Chu <simonchu.web@gmail.com> writes:

> On Jan 8, 2010, at 5:00 PM, Tomas Carnecky <tom@dbservice.com> wrote:
>
>> If you asked that question in the official IRC channel, the answer
>> would be 'faq non-bare', which would cause the bot to send you a
>> link to this entry in the Git FAQ:
>> http://git.or.cz/gitwiki/GitFaq#Whywon.27tIseechangesintheremoterepoafter.22gitpush.22.3F
>
> How do I push code to master then, do I do got reset - hard on the
> master??

(Please do not top post).

If you asked that question on the IRC, the answer would be 'faq
mothership-satellite', which would cause the bot to send you a link to
this entry in the Git FAQ:

    push is reverse of fetch. See
    http://git.or.cz/gitwiki/GitFaq#mothership-satellite

This link is recommended by the entry Tomas gave you, so I am having this
sad feeling that you wouldn't read it either...  Sigh.

^ permalink raw reply

* Re: Problem Using Git with Subversion Repository
From: Eric Wong @ 2010-01-09  6:31 UTC (permalink / raw)
  To: Bryan Richardson; +Cc: git
In-Reply-To: <3f81a4241001071453g24297atc1caab4a0a4ad176@mail.gmail.com>

Bryan Richardson <btricha@gmail.com> wrote:
> Hello all,
> 
> Has anyone come across a similar problem as this?
> 
> Item already exists in filesystem: File already exists: filesystem
> '/usr/local/svn/repos/my-apps/db', transaction '96-2v', path
> '/app/trunk/vendor/rails/actionpack/test/fixtures/layout_tests/layouts/symlinked'
> at /usr/lib/git-core/git-svn line 508
> 
> I *think* what happened is in a previous git-svn dcommit I removed the
> vendor/rails directory (unfroze rails from my app) and now I'm trying
> to freeze it again, in which case git-svn thinks a file needs to be
> added (instead of modified) and the Subversion repository says the
> file already exists.

Hi Bryan,

Which version of git svn are you using?  Any chance we can take a look
at the SVN/git history to know what's going on?

Thanks.

> Anyone know a way around it?!

Can you try manually removing that file with "svn rm" and then
doing "git svn rebase" to rebase your working HEAD before trying
to dcommit?

-- 
Eric Wong

^ permalink raw reply

* Re: How to push changes from clone back to master
From: Simon Chu @ 2010-01-09  5:53 UTC (permalink / raw)
  To: Tomas Carnecky; +Cc: git@vger.kernel.org
In-Reply-To: <4B47D52A.1050608@dbservice.com>

Tomas,

How do I push code to master then, do I do got reset - hard on the  
master??

Simon

On Jan 8, 2010, at 5:00 PM, Tomas Carnecky <tom@dbservice.com> wrote:

> On 1/8/10 10:28 PM, Simon C wrote:
> >
> >
> > I created master branch in git:
> >
> > mkdir git_master;
> > cd git_master
> > git init
> > git add .
> > git commit
> >
> > create a clone
> > git clone . ../git_clone1
> > create newfile
> > git add newfile
> > git commit .
> >
> > then try to propagate changes to master
> > git push
> >
> > it gave me some warning, and subsequent git push says everything  
> is up to
> > date.
> > but I do not see the new file under git_master
>
> If you asked that question in the official IRC channel, the answer  
> would be 'faq non-bare', which would cause the bot to send you a  
> link to this entry in the Git FAQ:
> http://git.or.cz/gitwiki/GitFaq#Whywon.27tIseechangesintheremoterepoafter.22gitpush.22.3F
>
> tom
>

^ permalink raw reply

* Re: [PATCH] ls-files: fix overeager pathspec optimization
From: Jeff King @ 2010-01-09  5:42 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Junio C Hamano, Michael J Gruber, Jon Schewe, spearce, git
In-Reply-To: <alpine.LFD.2.00.1001081701570.7821@localhost.localdomain>

On Fri, Jan 08, 2010 at 05:07:46PM -0800, Linus Torvalds wrote:

> > But I am starting to wonder if we might be better off restructuring
> > read_directory_recursive().  Currently it assumes that the path it was
> > given _must_ be of interest (i.e. not ignored) and runs excluded() on
> > subdirectories it finds to make that same decision before recursing into
> > them or skipping them.  It might make more sense if it first checked if
> > the path given by the caller should be ignored and act accordingly.
> 
> Hmm. I can't make myself care one way or the other, I have to admit.  I 
> assume you mean basically taking the path and using the first component of 
> it _instead_ of doing a readdir() - and getting rid of the simplification 
> up front?
> 
> I agree that that should work. Would it be simpler and cleaner? Perhaps. 
> I'd have to see both patches to be able to tell. I do admit that while I 
> acked your patch, it sure ain't _pretty_ to do that special odd 
> "has_leading_ignored_dir()" thing.

It would look something like this:

diff --git a/dir.c b/dir.c
index 3a8d3e6..306d354 100644
--- a/dir.c
+++ b/dir.c
@@ -811,12 +811,19 @@ static void free_simplify(struct path_simplify *simplify)
 int read_directory(struct dir_struct *dir, const char *path, int len, const char **pathspec)
 {
 	struct path_simplify *simplify;
+	int d_type = DT_DIR;
+	int exclude;
 
 	if (has_symlink_leading_path(path, len))
 		return dir->nr;
 
 	simplify = create_simplify(pathspec);
-	read_directory_recursive(dir, path, len, 0, simplify);
+	exclude = excluded(dir, path, &d_type);
+	if (exclude && (dir->flags & DIR_COLLECT_IGNORED) &&
+	    in_pathspec(path, len, simplify))
+		dir_add_ignored(dir, path, len);
+	if (!exclude || (dir->flags & DIR_SHOW_IGNORED))
+		read_directory_recursive(dir, path, len, 0, simplify);
 	free_simplify(simplify);
 	qsort(dir->entries, dir->nr, sizeof(struct dir_entry *), cmp_name);
 	qsort(dir->ignored, dir->ignored_nr, sizeof(struct dir_entry *), cmp_name);

But unfortunately excluded() is not happy with the trailing slash on the
path given to read_directory, so we also need on top:

diff --git a/dir.c b/dir.c
index 306d354..6045a84 100644
--- a/dir.c
+++ b/dir.c
@@ -813,12 +813,17 @@ int read_directory(struct dir_struct *dir, const char *path, int len, const char
 	struct path_simplify *simplify;
 	int d_type = DT_DIR;
 	int exclude;
+	char *path_without_slash;
 
 	if (has_symlink_leading_path(path, len))
 		return dir->nr;
 
 	simplify = create_simplify(pathspec);
-	exclude = excluded(dir, path, &d_type);
+	path_without_slash = xstrdup(path);
+	if (path_without_slash[strlen(path_without_slash)-1] == '/')
+		path_without_slash[strlen(path_without_slash)-1] = '\0';
+	exclude = excluded(dir, path_without_slash, &d_type);
+	free(path_without_slash);
 	if (exclude && (dir->flags & DIR_COLLECT_IGNORED) &&
 	    in_pathspec(path, len, simplify))
 		dir_add_ignored(dir, path, len);

And that does fix the case that triggered this whole discussion, but I
haven't tested thoroughly to make sure we are not adversely affecting
other cases.

-Peff

^ permalink raw reply related

* Re: Linking multiple Git repositories for version tracking
From: Michael Witten @ 2010-01-09  5:25 UTC (permalink / raw)
  To: James Beck; +Cc: git@vger.kernel.org
In-Reply-To: <op.u574cwxqn3qeew@klee>

On Fri, Jan 8, 2010 at 11:03 AM, James Beck <james@jmbeck.com> wrote:
> I'm starting to move into alpha with this code, so I really need to have a
> system that keeps track of compatible firmware versions. The best I can
> come up with is a plain text file (or Excel spreadsheet) that lists the
> overall firmware version, and the Git hash for each individual project.
> That way, if I want to load up a particular firmware version, I can
> checkout each hash for that version. Seems foolproof, but not terribly
> easy, and somewhat annoying.

The Xorg project is currently composed of 197 separate `modules', and
I believe that (practically) each has its own Git repo.

Their solution seems to be the exact same as yours, though it involves
translating between official release numbers and commits from what I
recall, all of which uses this hand-written list:

    http://cgit.freedesktop.org/xorg/util/modular/tree/module-list.txt

So, perhaps you just need to write a small script to do the checkouts
automatically for you.

Also, you could use Git tags rather than raw Git hashes. If you have
"releases", you could just make the same tag name in each Git repo, so
that checking out any particular "release" just amounts to having a
script loop through the repos and checkout the necesarry commit using
the associated tag. In fact, I suggested this to the Xorg guys not
long ago:

    http://www.mail-archive.com/xorg-devel@lists.x.org/msg03037.html

See:

    http://cgit.freedesktop.org/xorg/util/modular/tree/
    http://www.x.org/wiki/Development/Documentation/ReleaseHOWTO

^ permalink raw reply

* [PATCH] help: fix configured help format taking over command line one
From: Christian Couder @ 2010-01-09  5:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Johannes Schindelin, Gerrit Pape, David Aguilar

Since commit 7c3baa9 (help -a: do not unnecessarily look for a
repository, 2009-09-04), the help format that is passed as a
command line option is not used if an help format has been
configured. This patch fixes that.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin-help.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/builtin-help.c b/builtin-help.c
index 09ad4b0..3182a2b 100644
--- a/builtin-help.c
+++ b/builtin-help.c
@@ -23,13 +23,14 @@ static struct man_viewer_info_list {
 } *man_viewer_info_list;
 
 enum help_format {
+	HELP_FORMAT_NONE,
 	HELP_FORMAT_MAN,
 	HELP_FORMAT_INFO,
 	HELP_FORMAT_WEB,
 };
 
 static int show_all = 0;
-static enum help_format help_format = HELP_FORMAT_MAN;
+static enum help_format help_format = HELP_FORMAT_NONE;
 static struct option builtin_help_options[] = {
 	OPT_BOOLEAN('a', "all", &show_all, "print all available commands"),
 	OPT_SET_INT('m', "man", &help_format, "show man page", HELP_FORMAT_MAN),
@@ -415,10 +416,12 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 {
 	int nongit;
 	const char *alias;
+	enum help_format parsed_help_format;
 	load_command_list("git-", &main_cmds, &other_cmds);
 
 	argc = parse_options(argc, argv, prefix, builtin_help_options,
 			builtin_help_usage, 0);
+	parsed_help_format = help_format;
 
 	if (show_all) {
 		printf("usage: %s\n\n", git_usage_string);
@@ -437,6 +440,9 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 	setup_git_directory_gently(&nongit);
 	git_config(git_help_config, NULL);
 
+	if (parsed_help_format != HELP_FORMAT_NONE)
+		help_format = parsed_help_format;
+
 	alias = alias_lookup(argv[0]);
 	if (alias && !is_git_command(argv[0])) {
 		printf("`git %s' is aliased to `%s'\n", argv[0], alias);
@@ -444,6 +450,7 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 	}
 
 	switch (help_format) {
+	case HELP_FORMAT_NONE:
 	case HELP_FORMAT_MAN:
 		show_man_page(argv[0]);
 		break;
-- 
1.6.6.271.gc8799

^ permalink raw reply related

* [PATCH v2 2/2] Handle relative paths in submodule .git files
From: Brad King @ 2010-01-09  3:36 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Avery Pennarun, Lars Hjemli
In-Reply-To: <32541b131001081524g43d54a44i582dd286c1dfe7a5@mail.gmail.com>

Commit 842abf06f36b5b31050db6406265972e3e1cc189 taught
resolve_gitlink_ref() to call read_gitfile_gently() to resolve .git
files.  In this commit teach read_gitfile_gently() to interpret a
relative path in a .git file with respect to the file location.

This change allows update-index to recognize a submodule that uses a
relative path in its .git file.  It previously failed because the
relative path was wrongly interpreted with respect to the superproject
directory.

Signed-off-by: Brad King <brad.king@kitware.com>
---
 setup.c                         |   22 +++++++++++++++++++---
 t/t2104-update-index-gitfile.sh |    2 +-
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/setup.c b/setup.c
index 2cf0f19..f10e2dd 100644
--- a/setup.c
+++ b/setup.c
@@ -252,6 +252,8 @@ static int check_repository_format_gently(int *nongit_ok)
 const char *read_gitfile_gently(const char *path)
 {
 	char *buf;
+	char* dir;
+	const char *slash;
 	struct stat st;
 	int fd;
 	size_t len;
@@ -276,9 +278,23 @@ const char *read_gitfile_gently(const char *path)
 	if (len < 9)
 		die("No path in gitfile: %s", path);
 	buf[len] = '\0';
-	if (!is_git_directory(buf + 8))
-		die("Not a git repository: %s", buf + 8);
-	path = make_absolute_path(buf + 8);
+	dir = buf + 8;
+
+	if (!is_absolute_path(dir) && (slash = strrchr(path, '/'))) {
+		size_t pathlen = slash+1 - path;
+		size_t dirlen = pathlen + len - 8;
+		dir = xmalloc(dirlen + 1);
+		strncpy(dir, path, pathlen);
+		strncpy(dir + pathlen, buf + 8, len - 8);
+		dir[dirlen] = '\0';
+		free(buf);
+		buf = dir;
+	}
+
+	if (!is_git_directory(dir))
+		die("Not a git repository: %s", dir);
+	path = make_absolute_path(dir);
+
 	free(buf);
 	return path;
 }
diff --git a/t/t2104-update-index-gitfile.sh b/t/t2104-update-index-gitfile.sh
index ba71984..641607d 100755
--- a/t/t2104-update-index-gitfile.sh
+++ b/t/t2104-update-index-gitfile.sh
@@ -31,7 +31,7 @@ test_expect_success 'submodule with relative .git file' '
 	 test_commit first)
 '
 
-test_expect_failure 'add gitlink to relative .git file' '
+test_expect_success 'add gitlink to relative .git file' '
 	git update-index --add -- sub2
 '
 
-- 
1.6.5

^ permalink raw reply related

* [PATCH v2 0/2] Support relative .git file in a submodule
From: Brad King @ 2010-01-09  3:36 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Avery Pennarun, Lars Hjemli
In-Reply-To: <32541b131001081524g43d54a44i582dd286c1dfe7a5@mail.gmail.com>

Junio C Hamano wrote:
> then I've always thought that is simply a misconfiguration (t0002
> seems to use full path for this exact reason).

Everything in that test works with REAL=.real except the line

  test "$REAL" = "$(git rev-parse --git-dir)"

because --git-dir returns an absolute path.

> Is there a reason why relative path should be used/usable here, other
> than "being able to is better than not being able to"???

Similar configurations already work:

 - A .git file with a relative path works inside its work tree
 - A .git symlink with a relative path works inside its work tree
 - A submodule whose .git is a real symlink with a relative path works

My patch just fixes an intuitive combination of these cases.

> I don't like my process randomly chdir'ing around assuming they can
> chdir back safely very much, and would prefer not to add such
> codepaths unless absolutely necessary.

Here is a new patch series.  Patch 1/2 is unchanged.  Patch 2/2 has been
re-written to avoid chdir.

Avery Pennarun wrote:
> This problem seems especially true with submodules.  If the
> submodule's repo is something like supermodule/.git/submodule.git, a
> relative path would almost always be a appropriate, no?

Exactly.  In fact the experiment I was doing involved creating submodule
repos inside the main .git and linking to them from the work tree
subdirectories.  I'm looking into combining the approach with that of
git-new-workdir to keep submodules in the same object database.

Brad King (2):
  Test update-index for a gitlink to a .git file
  Handle relative paths in submodule .git files

 setup.c                         |   22 +++++++++++++++++++---
 t/t2104-update-index-gitfile.sh |   38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 3 deletions(-)
 create mode 100755 t/t2104-update-index-gitfile.sh

^ permalink raw reply

* [PATCH v2 1/2] Test update-index for a gitlink to a .git file
From: Brad King @ 2010-01-09  3:36 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Avery Pennarun, Lars Hjemli
In-Reply-To: <32541b131001081524g43d54a44i582dd286c1dfe7a5@mail.gmail.com>

Check that update-index recognizes a submodule that uses a .git file.
Currently it works when the .git file specifies an absolute path, but
not when it specifies a relative path.

Signed-off-by: Brad King <brad.king@kitware.com>
---
 t/t2104-update-index-gitfile.sh |   38 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 38 insertions(+), 0 deletions(-)
 create mode 100755 t/t2104-update-index-gitfile.sh

diff --git a/t/t2104-update-index-gitfile.sh b/t/t2104-update-index-gitfile.sh
new file mode 100755
index 0000000..ba71984
--- /dev/null
+++ b/t/t2104-update-index-gitfile.sh
@@ -0,0 +1,38 @@
+#!/bin/sh
+#
+# Copyright (c) 2010 Brad King
+#
+
+test_description='git update-index for gitlink to .git file.
+'
+
+. ./test-lib.sh
+
+test_expect_success 'submodule with absolute .git file' '
+	mkdir sub1 &&
+	(cd sub1 &&
+	 git init &&
+	 REAL="$(pwd)/.real" &&
+	 mv .git "$REAL"
+	 echo "gitdir: $REAL" >.git &&
+	 test_commit first)
+'
+
+test_expect_success 'add gitlink to absolute .git file' '
+	git update-index --add -- sub1
+'
+
+test_expect_success 'submodule with relative .git file' '
+	mkdir sub2 &&
+	(cd sub2 &&
+	 git init &&
+	 mv .git .real &&
+	 echo "gitdir: .real" >.git &&
+	 test_commit first)
+'
+
+test_expect_failure 'add gitlink to relative .git file' '
+	git update-index --add -- sub2
+'
+
+test_done
-- 
1.6.5

^ permalink raw reply related

* Re: git-svn: git svn dcommit doesn't handle network failure
From: Eric Wong @ 2010-01-09  2:13 UTC (permalink / raw)
  To: Jonathan Nieder
  Cc: Brice Goglin, 557477-forwarded, 557477, git, Alex Vandiver,
	Pedro Melo, Sam Vilain
In-Reply-To: <20100109010058.GA11922@progeny.tock>

Jonathan Nieder <jrnieder@gmail.com> wrote:
> Hi Eric,
> 
> 'git svn dcommit' does not seem to have any error recovery built in,
> so the user is put in a not-great situation if the network cable is
> pulled in the middle, as in the report below.
> 
> See also <http://thread.gmane.org/gmane.comp.version-control.git/70458>
> 
> Ideally, it would be nice if users could pretend that each push of
> a single commit is an atomic operation that rewrites the corresponding
> local commit at the same time.  In other words, to clean up we might
> unapply the unpushed patch, then run rebase_cmd().
> 
> What do you think?

Hi Jonathan,

I agree that error recovery for dcommit sucks right now.

I think Brice's idea (creating a temporary branch to dcommit from)
is easier to implement and less likely to break than automatically
unapplying patches.

Not sure when I'll have time to implement this, but I'll accept
patches in the mean time.

> Brice Goglin wrote:
> 
> > Package: git-svn
> > Version: 1:1.6.5-1
> > Severity: important
> > 
> > Hello,
> > 
> > If the network fails during dcommit, git svn gets totally screwed up.
> > It will give you the prompt back with HEAD pointing to the last SVN-pushed
> > commit, and the next one applied and not git-committed.
> > 
> > $ git svn dcommit
> > Committing to svn+ssh://bgoglin@scm.gforge.inria.fr/svn/knem/branches/rma ...
> > 	M	common/knem_io.h
> > Committed r237
> > 	M	common/knem_io.h
> > r237 = f839e16b94052f8b71aa4b66124b37a8337182e0 (refs/remotes/rma)
> > No changes between current HEAD and refs/remotes/rma
> > Resetting to the latest refs/remotes/rma
> > Unstaged changes after reset:
> > M	common/knem_io.h
> > M	driver/linux/knem_main.c
> > ssh: connect to host scm.gforge.inria.fr port 22: Connection timed out
> > Use of uninitialized value in concatenation (.) or string at /usr/lib/perl5/SVN/Core.pm line 584.
> > Network connection closed unexpectedly:  at /usr/lib/git-core/git-svn line 557
> > 
> > 
> > Now the only way I found to solve this mess is to git-reset --hard,
> > git checkout the old HEAD again, and git svn dcommit the remaining stuff.
> > 
> > The big problem is that I don't see any easy way to find the old HEAD.
> > I had to look back in my terminal history in case there's still the output
> > of a git log showing the old HEAD commitid.
> > 
> > I don't know how dcommit works internally, but my feeling is that it
> > should go to a new temporary branch during dcommit. This way, we would
> > just have to checkout the non-temporary branch on failure. For now,
> > I do it manually in case the SVN server fails again, but that's really
> > annoying.
> > 
> > thanks,
> > Brice
> --

^ permalink raw reply

* Re: For real now: bug tracking and secretary tasks in git
From: J.H. @ 2010-01-09  1:54 UTC (permalink / raw)
  To: Jan Krüger; +Cc: Git ML
In-Reply-To: <20100109013850.16f82412@perceptron>

> The proposal goes like this:
> 
> * Set up bug tracker (done; it's at http://gitbugs.jk.gs/).
> * Optionally make it an official public bug tracker.

Is there a reason that the bug tracker should live outside of
kernel.org?  I mean pretty much everything official, the official source
tree for instance, already lives on kernel.org - wouldn't having the bug
tracker under the same domain make more sense?

I also thought there was some discussion about a distributed bug tracker
a while back for this, what ever came of that?  If I've been living
under a rock about those issues please pardon my ignorance.

- John 'Warthog9' Hawley

^ permalink raw reply

* Re: [PATCH (v2) 2/2] rebase -i: teach --onto A...B syntax
From: Johannes Schindelin @ 2010-01-09  1:36 UTC (permalink / raw)
  To: A Large Angry SCM
  Cc: Sverre Rabbelier, Avery Pennarun, Junio C Hamano,
	Nanako Shiraishi, git
In-Reply-To: <4B47BE13.7070509@gmail.com>

Hi,

On Fri, 8 Jan 2010, A Large Angry SCM wrote:

> Sverre Rabbelier wrote:
> > 
> > On Fri, Jan 8, 2010 at 15:31, Avery Pennarun <apenwarr@gmail.com> 
> > wrote:
> > > Thanks, I didn't know about that one.  But my general point is 
> > > still: we seem to have two implementations when the functionality of 
> > > one is actually a superset of the other.  As far as I can see, 
> > > anyway.  So the obvious way to reduce the duplicated code is to 
> > > simply eliminate the less-featureful implementation.
> > 
> > *cough* git sequencer *cough*
> 
> *cough* not in my ${PATH} *cough*

*cough* because that GSoC project failed in all but writing? *cough*

^ permalink raw reply

* Re: [PATCH] ls-files: fix overeager pathspec optimization
From: Linus Torvalds @ 2010-01-09  1:07 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Michael J Gruber, Jon Schewe, spearce, git
In-Reply-To: <7v8wc8kshh.fsf@alter.siamese.dyndns.org>



On Fri, 8 Jan 2010, Junio C Hamano wrote:
> 
> Yes, and the previous patch wasn't adding what is ignored to the array, so
> here is a re-roll to fix that in addition to the fix to "should the loop
> start from checking an empty path?" issue you noticed.

Ack. Looks ok to me, and I think it's a lot more obvious.

> But I am starting to wonder if we might be better off restructuring
> read_directory_recursive().  Currently it assumes that the path it was
> given _must_ be of interest (i.e. not ignored) and runs excluded() on
> subdirectories it finds to make that same decision before recursing into
> them or skipping them.  It might make more sense if it first checked if
> the path given by the caller should be ignored and act accordingly.

Hmm. I can't make myself care one way or the other, I have to admit.  I 
assume you mean basically taking the path and using the first component of 
it _instead_ of doing a readdir() - and getting rid of the simplification 
up front?

I agree that that should work. Would it be simpler and cleaner? Perhaps. 
I'd have to see both patches to be able to tell. I do admit that while I 
acked your patch, it sure ain't _pretty_ to do that special odd 
"has_leading_ignored_dir()" thing.

		Linus

^ permalink raw reply

* [PATCH] hg-to-git: fix COMMITTER type-o
From: Bart Trojanowski @ 2010-01-09  0:54 UTC (permalink / raw)
  To: git; +Cc: bart, Stelian Pop

This script passes the author and committer to git-commit via environment
variables, but it was missing the seccond T of COMMITTER in a few places.

Signed-off-by: Bart Trojanowski <bart@jukie.net>
---
 contrib/hg-to-git/hg-to-git.py |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/contrib/hg-to-git/hg-to-git.py b/contrib/hg-to-git/hg-to-git.py
index 2a6839d..854cd94 100755
--- a/contrib/hg-to-git/hg-to-git.py
+++ b/contrib/hg-to-git/hg-to-git.py
@@ -59,14 +59,14 @@ def getgitenv(user, date):
     elems = re.compile('(.*?)\s+<(.*)>').match(user)
     if elems:
         env += 'export GIT_AUTHOR_NAME="%s" ;' % elems.group(1)
-        env += 'export GIT_COMMITER_NAME="%s" ;' % elems.group(1)
+        env += 'export GIT_COMMITTER_NAME="%s" ;' % elems.group(1)
         env += 'export GIT_AUTHOR_EMAIL="%s" ;' % elems.group(2)
-        env += 'export GIT_COMMITER_EMAIL="%s" ;' % elems.group(2)
+        env += 'export GIT_COMMITTER_EMAIL="%s" ;' % elems.group(2)
     else:
         env += 'export GIT_AUTHOR_NAME="%s" ;' % user
-        env += 'export GIT_COMMITER_NAME="%s" ;' % user
+        env += 'export GIT_COMMITTER_NAME="%s" ;' % user
         env += 'export GIT_AUTHOR_EMAIL= ;'
-        env += 'export GIT_COMMITER_EMAIL= ;'
+        env += 'export GIT_COMMITTER_EMAIL= ;'
 
     env += 'export GIT_AUTHOR_DATE="%s" ;' % date
     env += 'export GIT_COMMITTER_DATE="%s" ;' % date
-- 
1.6.6.2.g9ad4f53

^ permalink raw reply related

* Re: git-svn: git svn dcommit doesn't handle network failure
From: Jonathan Nieder @ 2010-01-09  1:00 UTC (permalink / raw)
  To: Eric Wong
  Cc: Brice Goglin, 557477-forwarded, 557477, git, Alex Vandiver,
	Pedro Melo, Sam Vilain
In-Reply-To: <20091122112434.29914.59452.reportbug@localhost.localdomain>

Hi Eric,

'git svn dcommit' does not seem to have any error recovery built in,
so the user is put in a not-great situation if the network cable is
pulled in the middle, as in the report below.

See also <http://thread.gmane.org/gmane.comp.version-control.git/70458>

Ideally, it would be nice if users could pretend that each push of
a single commit is an atomic operation that rewrites the corresponding
local commit at the same time.  In other words, to clean up we might
unapply the unpushed patch, then run rebase_cmd().

What do you think?

Brice Goglin wrote:

> Package: git-svn
> Version: 1:1.6.5-1
> Severity: important
> 
> Hello,
> 
> If the network fails during dcommit, git svn gets totally screwed up.
> It will give you the prompt back with HEAD pointing to the last SVN-pushed
> commit, and the next one applied and not git-committed.
> 
> $ git svn dcommit
> Committing to svn+ssh://bgoglin@scm.gforge.inria.fr/svn/knem/branches/rma ...
> 	M	common/knem_io.h
> Committed r237
> 	M	common/knem_io.h
> r237 = f839e16b94052f8b71aa4b66124b37a8337182e0 (refs/remotes/rma)
> No changes between current HEAD and refs/remotes/rma
> Resetting to the latest refs/remotes/rma
> Unstaged changes after reset:
> M	common/knem_io.h
> M	driver/linux/knem_main.c
> ssh: connect to host scm.gforge.inria.fr port 22: Connection timed out
> Use of uninitialized value in concatenation (.) or string at /usr/lib/perl5/SVN/Core.pm line 584.
> Network connection closed unexpectedly:  at /usr/lib/git-core/git-svn line 557
> 
> 
> Now the only way I found to solve this mess is to git-reset --hard,
> git checkout the old HEAD again, and git svn dcommit the remaining stuff.
> 
> The big problem is that I don't see any easy way to find the old HEAD.
> I had to look back in my terminal history in case there's still the output
> of a git log showing the old HEAD commitid.
> 
> I don't know how dcommit works internally, but my feeling is that it
> should go to a new temporary branch during dcommit. This way, we would
> just have to checkout the non-temporary branch on failure. For now,
> I do it manually in case the SVN server fails again, but that's really
> annoying.
> 
> thanks,
> Brice

^ permalink raw reply

* Re: How to push changes from clone back to master
From: Tomas Carnecky @ 2010-01-09  1:00 UTC (permalink / raw)
  To: Simon C; +Cc: git
In-Reply-To: <1262986087967-4275010.post@n2.nabble.com>

On 1/8/10 10:28 PM, Simon C wrote:
 >
 >
 > I created master branch in git:
 >
 > mkdir git_master;
 > cd git_master
 > git init
 > git add .
 > git commit
 >
 > create a clone
 > git clone . ../git_clone1
 > create newfile
 > git add newfile
 > git commit .
 >
 > then try to propagate changes to master
 > git push
 >
 > it gave me some warning, and subsequent git push says everything is up to
 > date.
 > but I do not see the new file under git_master

If you asked that question in the official IRC channel, the answer would 
be 'faq non-bare', which would cause the bot to send you a link to this 
entry in the Git FAQ:
http://git.or.cz/gitwiki/GitFaq#Whywon.27tIseechangesintheremoterepoafter.22gitpush.22.3F

tom

^ permalink raw reply

* Re: [PATCH] ls-files: fix overeager pathspec optimization
From: Junio C Hamano @ 2010-01-09  0:54 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Jeff King, Michael J Gruber, Jon Schewe, spearce, git
In-Reply-To: <alpine.LFD.2.00.1001081619570.7821@localhost.localdomain>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> I have this memory that _used_ to have a per-filename flag in "git add" 
> that checked if that particular filename component was used or not. But 
> now it just looks at 'dir->ignored_nr' and 'dir->ignored[]'.

Yes, and the previous patch wasn't adding what is ignored to the array, so
here is a re-roll to fix that in addition to the fix to "should the loop
start from checking an empty path?" issue you noticed.

But I am starting to wonder if we might be better off restructuring
read_directory_recursive().  Currently it assumes that the path it was
given _must_ be of interest (i.e. not ignored) and runs excluded() on
subdirectories it finds to make that same decision before recursing into
them or skipping them.  It might make more sense if it first checked if
the path given by the caller should be ignored and act accordingly. If it
is to be ignored, perhaps it will simply return without doing anything
(normal case), or it will return but adds the path to ignored[]
(DIR_COLLECT_IGNORED case), or it will recurse into itself but tell it
that everything it finds is to be ignored.  I dunno...

-- >8 --
Subject: [PATCH (v3)] ls-files: fix overeager pathspec optimization

Given pathspecs that share a common prefix, ls-files optimized its call
into recursive directory reader by starting at the common prefix
directory.

If you have a directory "t" with an untracked file "t/junk" in it, but the
top-level .gitignore file told us to ignore "t/", this resulted in:

    $ git ls-files -o --exclude-standard
    $ git ls-files -o --exclude-standard t/
    t/junk
    $ git ls-files -o --exclude-standard t/junk
    t/junk
    $ cd t && git ls-files -o --exclude-standard
    junk

We could argue that you are overriding the ignore file by giving a
patchspec that matches or being in that directory, but it is somewhat
unexpected.  Worse yet, these behave differently:

    $ git ls-files -o --exclude-standard t/ .
    $ git ls-files -o --exclude-standard t/
    t/junk

This patch changes the optimization so that it notices when the common
prefix directory that it starts reading from is an ignored one.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 dir.c                              |   36 +++++++++++++++++++++++++++++++++
 t/t3001-ls-files-others-exclude.sh |   39 ++++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+), 0 deletions(-)

diff --git a/dir.c b/dir.c
index d0999ba..e8be909 100644
--- a/dir.c
+++ b/dir.c
@@ -778,6 +778,39 @@ static void free_simplify(struct path_simplify *simplify)
 	free(simplify);
 }
 
+static int has_leading_ignored_dir(struct dir_struct *dir,
+				   const char *path_, int len,
+				   const struct path_simplify *simplify)
+{
+	int dtype = DT_DIR;
+	char path[PATH_MAX], *cp = path;
+
+	memcpy(path, path_, len);
+	while (1) {
+		char *next = memchr(cp, '/', path + len - cp);
+		int exclude;
+
+		/*
+		 * NOTE! NOTE! NOTE!: we might want to actually lstat(2)
+		 * path[] to make sure it is a directory.
+		 */
+		if (next)
+			*next = '\0';
+		exclude = excluded(dir, path, &dtype);
+		if (next)
+			*next = '/';
+		if (exclude) {
+			if (dir->flags & DIR_COLLECT_IGNORED)
+				dir_add_ignored(dir, path, len);
+			return 1;
+		}
+		if (!next)
+			break;
+		cp = next + 1;
+	}
+	return 0;
+}
+
 int read_directory(struct dir_struct *dir, const char *path, int len, const char **pathspec)
 {
 	struct path_simplify *simplify;
@@ -786,6 +819,9 @@ int read_directory(struct dir_struct *dir, const char *path, int len, const char
 		return dir->nr;
 
 	simplify = create_simplify(pathspec);
+	if (has_leading_ignored_dir(dir, path, len, simplify))
+		return dir->nr;
+
 	read_directory_recursive(dir, path, len, 0, simplify);
 	free_simplify(simplify);
 	qsort(dir->entries, dir->nr, sizeof(struct dir_entry *), cmp_name);
diff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh
index c65bca8..9e71260 100755
--- a/t/t3001-ls-files-others-exclude.sh
+++ b/t/t3001-ls-files-others-exclude.sh
@@ -153,4 +153,43 @@ test_expect_success 'negated exclude matches can override previous ones' '
 	grep "^a.1" output
 '
 
+test_expect_success 'subdirectory ignore (setup)' '
+	mkdir -p top/l1/l2 &&
+	(
+		cd top &&
+		git init &&
+		echo /.gitignore >.gitignore &&
+		echo l1 >>.gitignore &&
+		echo l2 >l1/.gitignore &&
+		>l1/l2/l1
+	)
+'
+
+test_expect_success 'subdirectory ignore (toplevel)' '
+	(
+		cd top &&
+		git ls-files -o --exclude-standard
+	) >actual &&
+	>expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'subdirectory ignore (l1/l2)' '
+	(
+		cd top/l1/l2 &&
+		git ls-files -o --exclude-standard
+	) >actual &&
+	>expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'subdirectory ignore (l1)' '
+	(
+		cd top/l1 &&
+		git ls-files -o --exclude-standard
+	) >actual &&
+	>expect &&
+	test_cmp expect actual
+'
+
 test_done
-- 
1.6.6.209.g52296.dirty

^ permalink raw reply related

* Re: [msysGit] [PATCH/RFC 06/11] run-command: add kill_async() and  is_async_alive()
From: Erik Faye-Lund @ 2010-01-09  0:49 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: msysgit, git, dotzenlabs
In-Reply-To: <200912022027.23344.j6t@kdbg.org>

On Wed, Dec 2, 2009 at 8:27 PM, Johannes Sixt <j6t@kdbg.org> wrote:
> On Mittwoch, 2. Dezember 2009, Erik Faye-Lund wrote:
>> On Fri, Nov 27, 2009 at 8:59 PM, Johannes Sixt <j6t@kdbg.org> wrote:
>> > "relatively small chance of stuff blowing up"? The docs of
>> > TerminateThread: "... the kernel32 state for the thread's process could
>> > be inconsistent." That's scary if we are talking about a process that
>> > should run for days or weeks without interruption.
>>
>> I think there's a misunderstanding here. I thought your suggestion was
>> to simply call die(), which would take down the main process. After
>> reading this explanation, I think you're talking about giving an error
>> and rejecting the connection instead. Which makes more sense than to
>> risk crashing the main-process, indeed.
>
> Just rejecting a connection is certainly the simplest do to keep the daemon
> process alive. But the server can be DoS-ed from a single source IP.
>
> Currently git-daemon can only be DDoS-ed because there is a maximum number of
> connections, which are not closed if all of them originate from different
> IPs.
>

After some testing I've found that git-daemon can very much be DoS-ed
from a single IP in it's current form. This is for two reasons:
1) The clever xcalloc + memcmp trick has a fault; the port for each
connection is different, so there will never be a match. I have a
patch[1] for this that I plan to send out soon.
2) Even with this patch the effect of the DoS-protection is kind of
limited. This is because it's a child process of the fork()'d process
again that does all the heavy lifting, and kill(pid, SIGHUP) doesn't
kill child processes. So, the connection gets to continue the action
until upload-pack (or whatever the current command is) finish. This
might be quite lengthy.

As I said, I have a patch for 1), but I don't quite know how to fix
2). Perhaps this is a good use for process groups? I'm a Windows-guy;
my POSIX isn't exactly super-awesome...

I found these issues during my latest effort to port git-daemon to
Windows. I managed to get this to work fine on Windows, by
implementing a kill(x, SIGTERM) that terminated child-processes
(because I was under the impression that this was what happened... I
guess daemon.c lead me to believe that).

[1]: http://repo.or.cz/w/git/kusma.git/commit/b1d286d32f42c57b90a1db9b7b8d6775a5d1ad7b

-- 
Erik "kusma" Faye-Lund

^ permalink raw reply

* For real now: bug tracking and secretary tasks in git
From: Jan Krüger @ 2010-01-09  0:38 UTC (permalink / raw)
  To: Git ML

I thought about Cc'ing everyone who was involved in previous
discussions about this but that would have been a huge list so I
didn't. No more introductory stuff needed; onwards to the wonderfully
formatted proposal thingy!

I) SUMMARY OF EVERYTHING THAT EVER HAPPENED
-------------------------------------------

Mass consensus in previous discussions[1][2] goes a bit like this:

1. It would be desirable to have people who do the work of interfacing
   between bug reporters and developers. These same people could make
   sure reports didn't get lost. These people are the *secretaries*.
   They should be pretty reliable.

2. People who contribute to git shouldn't be forced to work with the
   tracker. Having a tracker that isn't actively maintained by dedicated
   secretaries is pretty much worthless anyway, so there's no need to
   pretend that forcing developers to use a tracker interface is any
   kind of improvement.

3. The "human element" is important. For example, automatic reminders
   are a lot less valuable than reminders from an actual person.

II) PROPOSAL
------------

Of course, since I am semi-formally proposing this, I'm also
volunteering to make it happen, BUT I think that no single person can
handle all the list traffic conscientiously enough to do a really good
job. This proposal can only work if more volunteers are found. If you
(and of course I'm speaking to YOU personally now) want to help out,
speak up now!

The proposal goes like this:

* Set up bug tracker (done; it's at http://gitbugs.jk.gs/).
* Optionally make it an official public bug tracker.
* To conform to (2) above, tasks are only ever assigned to secretaries.
  Whoever assigns a task to himself is responsible for finding someone
  to actually get the task done, and to keep that person on his toes.
  The bug tracker has features that make this easier (there is no
  actual field for "assigned to external entity 'dscho'" in the
  interface because there is no bug tracker software that doesn't suck,
  but a comment gets the job done, and you can send reminders to
  yourself).
* Tasks filed by the general public get pre-screened by secretaries;
  worthwhile tasks are (semi-manually, to conform with (3)) forwarded to
  this list. The task is updated with summaries of whatever gets
  discussed on the list whenever appropriate.
* Tasks get pruned mercilessly to remove anything that is irrelevant,
  e.g. comments that do not contribute anything to getting the task
  done.
* Things reported to the list get posted to the bug tracker by
  secretaries (unless, for example, patches have already been accepted
  by a maintainer), in order to be able to keep track of them more
  easily. The task contains links to list discussions related to it.
  To make it easier for a group of secretaries to collaborate, and for
  any interested party to see the progress of a discussion, whenever a
  secretary adds a task to the tracker, he replies to the list post
  that prompted him to do so, with a subject starting with
  "[TASK]" (ideally containing the task's summary line, too) and the URL
  of the task in the message body.

Advantages:

* Secretaries don't need to coordinate their activities much. As such,
  there can be dozens of secretaries without scalability issues, which
  would reduce the workload on each of them.
* People who report things don't have to involve themselves in a
  technical discussion that may be completely over their heads. For
  example, when Joe Randomuser reports that a certain command does weird
  things, he most likely won't want to hear anything about whether the
  current strategy for confabulating stochastic index entries in a
  distributed manner is error-free, nor does he benefit at all from
  getting all that technical stuff delivered to his mailbox.
* People who report things can have more confidence that their report
  doesn't get lost in The Noise(tm).
* Git developers don't have to deal with incomplete/nonsensical reports
  all if they are submitted to the tracker.
* Git developers can choose themselves how much they want to interact
  with the bug tracker.

Disadvantages:

* There is a certain level of redundancy in this approach. It's not
  clear to me whether that's a bad thing. I tend to think that it isn't.

III) THIS SECTION IS USELESS
----------------------------

Having section headings for just two sections looked stupid, so here is
another one.

If there are no general objections to the proposal, I will start using
the tracker for tracking less-than-all reports posted to this list.
Whether the tracker really takes off depends on everyone who reads
this... and I'm sure there are lots of great ideas that just didn't
occur to me that you guys can share here.

[1] http://thread.gmane.org/gmane.comp.version-control.git/108109
[2] http://thread.gmane.org/gmane.comp.version-control.git/110117

^ permalink raw reply

* Re: [PATCH] ls-files: fix overeager pathspec optimization
From: Linus Torvalds @ 2010-01-09  0:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Michael J Gruber, Jon Schewe, spearce, git
In-Reply-To: <7v3a2gnnv2.fsf@alter.siamese.dyndns.org>



On Fri, 8 Jan 2010, Junio C Hamano wrote:
> 
> Since "git add" uses the exact same codepath to find the untracked files
> that match the pathspecs, I expected the "fix" will make it complain about
> "t/gomi" being ignored.  Not so.

No. Since 't' is ignored, it won't even do a readdir() in there.

And since it didn't do a readdir() in there, it won't find any files in 
there to add to the ignored list.

And that's how the "git add" logic for "apparently ignored" files works: 
it looks at whether the list of ignored files is empty or not.

> Actually, it does start ignoring t/gomi (the index does not have t/gomi
> after the above sequence with the patch), but lack of the error message
> makes it a rather unfortunate regression---it works as specified in the
> sense that ignored paths are not added to the index unless --forced, but
> it does so without telling the user about it.

I have this memory that _used_ to have a per-filename flag in "git add" 
that checked if that particular filename component was used or not. But 
now it just looks at 'dir->ignored_nr' and 'dir->ignored[]'.

[ Digging back in history.. Yes: commit e96980ef ]

		Linus

^ permalink raw reply

* Re: [PATCH] ls-files: fix overeager pathspec optimization
From: Junio C Hamano @ 2010-01-09  0:06 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Junio C Hamano, Jeff King, Michael J Gruber, Jon Schewe, spearce,
	git
In-Reply-To: <alpine.LFD.2.00.1001081520240.7821@localhost.localdomain>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Fri, 8 Jan 2010, Junio C Hamano wrote:
>>
>> Given pathspecs that share a common prefix, ls-files optimized its call
>> into recursive directory reader by starting at the common prefix
>> directory.
>> 
>> If you have a directory "t" with an untracked file "t/junk" in it, but the
>> top-level .gitignore file told us to ignore "t/", this resulted in an
>> unexpected behaviour:
>
> Ok, I'm not sure how "unexpected" this is, since arguably you are 
> overriding the ignore file by _being_ in that directory (the same way 
> index contents override ignore files), but I could go either way on that.
>
> Your patch looks fine, although I think you did this in a very odd way.

Actually, there is some funny interaction with "git add" I ran out of time
to figure out what the right fix should be:

    $ git init new
    $ cd new
    $ mkdir t
    $ >kuzu
    $ >t/gomi
    $ echo t >.gitignore
    $ echo kuzu >>.gitignore
    $ git add kuzu
    The following paths are ignored...
    kuzu
    Use -f if you really want to add them.
    $ git add t/gomi

Since "git add" uses the exact same codepath to find the untracked files
that match the pathspecs, I expected the "fix" will make it complain about
"t/gomi" being ignored.  Not so.

Actually, it does start ignoring t/gomi (the index does not have t/gomi
after the above sequence with the patch), but lack of the error message
makes it a rather unfortunate regression---it works as specified in the
sense that ignored paths are not added to the index unless --forced, but
it does so without telling the user about it.

^ permalink raw reply

* Re: [PATCH] ls-files: fix overeager pathspec optimization
From: Junio C Hamano @ 2010-01-08 23:31 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Junio C Hamano, Jeff King, Michael J Gruber, Jon Schewe, spearce,
	git
In-Reply-To: <alpine.LFD.2.00.1001081520240.7821@localhost.localdomain>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Fri, 8 Jan 2010, Junio C Hamano wrote:
>>
>> Given pathspecs that share a common prefix, ls-files optimized its call
>> into recursive directory reader by starting at the common prefix
>> directory.
>> 
>> If you have a directory "t" with an untracked file "t/junk" in it, but the
>> top-level .gitignore file told us to ignore "t/", this resulted in an
>> unexpected behaviour:
>
> Ok, I'm not sure how "unexpected" this is, since arguably you are 
> overriding the ignore file by _being_ in that directory (the same way 
> index contents override ignore files), but I could go either way on that.
>
> Your patch looks fine, although I think you did this in a very odd way.
>
>> +	at = 0;
>> +	memcpy(path, path_, len);
>> +	while (1) {
>> +		char *cp;
>> +		path[at] = '\0';
>> +		/*
>> +		 * NOTE! NOTE! NOTE!: we might want to actually lstat(2)
>> +		 * path[] to make sure it is a directory.
>> +		 */
>> +		if (excluded(dir, path, &dtype))
>> +			return 1;
>
> The above starts by testing the empty string, and then after that test it 
> goes on to the next directory component. That is just _odd_.
>
> Wouldn't it be more natural to write the loop the other way around, ie 
> _first_ look up the next directory component, and _then_ do the exclude 
> processing for thoose components? 
>
> Or is there some subtle reason I'm missing for actually checking the empty 
> name?

No, just being paranoid in case somebody managed to .gitignore the
top-level of the working tree ;-)

^ permalink raw reply

* Re: [PATCH] ls-files: fix overeager pathspec optimization
From: Linus Torvalds @ 2010-01-08 23:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Michael J Gruber, Jon Schewe, spearce, git
In-Reply-To: <7veim0w68q.fsf_-_@alter.siamese.dyndns.org>



On Fri, 8 Jan 2010, Junio C Hamano wrote:
>
> Given pathspecs that share a common prefix, ls-files optimized its call
> into recursive directory reader by starting at the common prefix
> directory.
> 
> If you have a directory "t" with an untracked file "t/junk" in it, but the
> top-level .gitignore file told us to ignore "t/", this resulted in an
> unexpected behaviour:

Ok, I'm not sure how "unexpected" this is, since arguably you are 
overriding the ignore file by _being_ in that directory (the same way 
index contents override ignore files), but I could go either way on that.

Your patch looks fine, although I think you did this in a very odd way.

> +	at = 0;
> +	memcpy(path, path_, len);
> +	while (1) {
> +		char *cp;
> +		path[at] = '\0';
> +		/*
> +		 * NOTE! NOTE! NOTE!: we might want to actually lstat(2)
> +		 * path[] to make sure it is a directory.
> +		 */
> +		if (excluded(dir, path, &dtype))
> +			return 1;

The above starts by testing the empty string, and then after that test it 
goes on to the next directory component. That is just _odd_.

Wouldn't it be more natural to write the loop the other way around, ie 
_first_ look up the next directory component, and _then_ do the exclude 
processing for thoose components? 

Or is there some subtle reason I'm missing for actually checking the empty 
name?

				Linus

^ permalink raw reply

* Re: [PATCH 0/2] Support relative .git file in a submodule
From: Avery Pennarun @ 2010-01-08 23:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Brad King, git, Lars Hjemli
In-Reply-To: <7vocl4urc6.fsf@alter.siamese.dyndns.org>

On Fri, Jan 8, 2010 at 6:09 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Brad King <brad.king@kitware.com> writes:
>
>> ... if a submodule
>> has a .git file "symlink" with a relative path to the real submodule
>> repository then ...
>
> ... then I've always thought that is simply a misconfiguration (t0002
> seems to use full path for this exact reason).  Is there a reason why
> relative path should be used/usable here, other than "being able to is
> better than not being able to"???

If I have a bunch of git repos in ~/src, and I decide I'd rather
rename it all to ~/source, it seems like it would be nice for all my
links not to be broken.  This sort of thing can also happen if you
have NFS-mounted home directories on a farm of machines, and some of
them automount in /u/username and others use /home/username, for
example.  I think this is the same reason that common sysadmin advice
is to use relative symlinks instead of absolute links.

This problem seems especially true with submodules.  If the
submodule's repo is something like supermodule/.git/submodule.git, a
relative path would almost always be a appropriate, no?

Have fun,

Avery

^ 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