Git development
 help / color / mirror / Atom feed
* Re: Do "git add" as a builtin
From: Linus Torvalds @ 2006-05-18 16:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0605180917060.10823@g5.osdl.org>



On Thu, 18 May 2006, Linus Torvalds wrote:
> > 
> > But if people are ok with changing it from a "print a warning and ignore" 
> > into an _error_, we could just move it into "add_cache_entry()".
> 
> This is the incremental patch to do that instead, if you prefer it.

Thinking some more about it, I think I personally much prefer this.

Especially as a quick look-through seems to say that there's actually a 
path through git-update-index too that allows a unverified filename to get 
into the index (the new "--unresolve" thing also misses the need to verify 
the path).

Making it a fatal error makes it more obvious that the user did something 
fundamentally wrong. And the safety in doing it in add_cache_entry() just 
makes be happier, particularly in light of the above problem with 
--unresolve.

That still leaves the question of whether we should also drop all the 
"verify_path()" calls in update-index.c, and make it fatal there too. I 
think we probably should.

(At that point we could turn verify_path() back into a static function, 
this time local entirely to read-cache.c, rather than update-index.c)

			Linus

^ permalink raw reply

* Re: Do "git add" as a builtin
From: Linus Torvalds @ 2006-05-18 16:25 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0605180807060.10823@g5.osdl.org>



On Thu, 18 May 2006, Linus Torvalds wrote:
> 
> But if people are ok with changing it from a "print a warning and ignore" 
> into an _error_, we could just move it into "add_cache_entry()".

This is the incremental patch to do that instead, if you prefer it.

Putting it into add_cache_entry() definitely has advantages, in that it 
should make it much harder for this bug to happen again - all users will 
now be verified.

With this one, it's now a fatal error to try to add a pathname that cannot 
be added, ie

	[torvalds@g5 git]$ git add .git/config 
	fatal: unable to add .git/config to index

and

	[torvalds@g5 git]$ git add foo/../bar 
	fatal: unable to add foo/../bar to index

instead of the old "Ignoring path xyz" warning that would end up silently 
succeeding on any other paths.

		Linus

---
diff --git a/builtin-add.c b/builtin-add.c
index 0346bb5..ac9ed2d 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -172,11 +172,6 @@ static int add_file_to_index(const char 
 	if (lstat(path, &st))
 		die("%s: unable to stat (%s)", path, strerror(errno));
 
-	if (!verify_path(path)) {
-		fprintf(stderr, "Ignoring path %s\n", path);
-		return -1;
-	}
-		
 	if (!S_ISREG(st.st_mode) && !S_ISLNK(st.st_mode))
 		die("%s: can only add regular files or symbolic links", path);
 
diff --git a/read-cache.c b/read-cache.c
index 6b323dd..9a272f8 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -551,6 +551,8 @@ int add_cache_entry(struct cache_entry *
 
 	if (!ok_to_add)
 		return -1;
+	if (!verify_path(ce->name))
+		return -1;
 
 	if (!skip_df_check &&
 	    check_file_directory_conflict(ce, pos, ok_to_replace)) {

^ permalink raw reply related

* Re: [PATCH] Handle branch names with slashes
From: Karl Hasselström @ 2006-05-18 16:00 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: Junio C Hamano, git
In-Reply-To: <b0943d9e0605180511v5cf9256dx84825866b1691f51@mail.gmail.com>

On 2006-05-18 13:11:52 +0100, Catalin Marinas wrote:

> On 18/05/06, Karl Hasselström <kha@treskal.com> wrote:
>
> > +    if len(dirs) != 0:
> > +        # We have branch names with / in them.
> > +        branch_chars = r'[^@]'
> > +        patch_id_mark = r'//'
> > +    else:
> > +        # No / in branch names.
> > +        branch_chars = r'[^@%/]'
>
> I removed % from the above regexp.

Ah, I missed one. Perhaps I should act surprised . . . :-)

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

^ permalink raw reply

* Re: Do "git add" as a builtin
From: Linus Torvalds @ 2006-05-18 15:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwtcj4tp6.fsf@assigned-by-dhcp.cox.net>



On Thu, 18 May 2006, Junio C Hamano wrote:
> 
> Ouch, things are worse than I thought...
> 
> 	$ mkdir foo
>         $ date >bar
>         $ git-add foo/../bar
> 	$ git ls-files
>         foo/../bar
> 
> Huh?

"git-update-index" does a "verify_path()" which I missed, so the new 
builtin add doesn't do it.

We could do it either in the "add_cache_entry()" function (which would be 
safest, because then by _definition_ you couldn't make this mistake 
again), or we could do it in the caller.

This patch does it in the caller, just to match the old "git add" (which 
would just say "Ignoring path ..." and not do anything).

But if people are ok with changing it from a "print a warning and ignore" 
into an _error_, we could just move it into "add_cache_entry()".

The real _meat_ of this patch is really just that first hunk to 
"builtin-add.c" - all the rest is just making that "verify_path()" 
function available in the git library.

		Linus
---
diff --git a/builtin-add.c b/builtin-add.c
index 7083820..0346bb5 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -172,6 +172,11 @@ static int add_file_to_index(const char 
 	if (lstat(path, &st))
 		die("%s: unable to stat (%s)", path, strerror(errno));
 
+	if (!verify_path(path)) {
+		fprintf(stderr, "Ignoring path %s\n", path);
+		return -1;
+	}
+		
 	if (!S_ISREG(st.st_mode) && !S_ISLNK(st.st_mode))
 		die("%s: can only add regular files or symbolic links", path);
 
diff --git a/cache.h b/cache.h
index b1300cd..f9b7049 100644
--- a/cache.h
+++ b/cache.h
@@ -143,6 +143,7 @@ #define alloc_nr(x) (((x)+16)*3/2)
 /* Initialize and use the cache information */
 extern int read_cache(void);
 extern int write_cache(int newfd, struct cache_entry **cache, int entries);
+extern int verify_path(const char *path);
 extern int cache_name_pos(const char *name, int namelen);
 #define ADD_CACHE_OK_TO_ADD 1		/* Ok to add */
 #define ADD_CACHE_OK_TO_REPLACE 2	/* Ok to replace file/directory */
diff --git a/read-cache.c b/read-cache.c
index ed0da38..6b323dd 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -347,6 +347,70 @@ int ce_path_match(const struct cache_ent
 }
 
 /*
+ * We fundamentally don't like some paths: we don't want
+ * dot or dot-dot anywhere, and for obvious reasons don't
+ * want to recurse into ".git" either.
+ *
+ * Also, we don't want double slashes or slashes at the
+ * end that can make pathnames ambiguous.
+ */
+static int verify_dotfile(const char *rest)
+{
+	/*
+	 * The first character was '.', but that
+	 * has already been discarded, we now test
+	 * the rest.
+	 */
+	switch (*rest) {
+	/* "." is not allowed */
+	case '\0': case '/':
+		return 0;
+
+	/*
+	 * ".git" followed by  NUL or slash is bad. This
+	 * shares the path end test with the ".." case.
+	 */
+	case 'g':
+		if (rest[1] != 'i')
+			break;
+		if (rest[2] != 't')
+			break;
+		rest += 2;
+	/* fallthrough */
+	case '.':
+		if (rest[1] == '\0' || rest[1] == '/')
+			return 0;
+	}
+	return 1;
+}
+
+int verify_path(const char *path)
+{
+	char c;
+
+	goto inside;
+	for (;;) {
+		if (!c)
+			return 1;
+		if (c == '/') {
+inside:
+			c = *path++;
+			switch (c) {
+			default:
+				continue;
+			case '/': case '\0':
+				break;
+			case '.':
+				if (verify_dotfile(path))
+					continue;
+			}
+			return 0;
+		}
+		c = *path++;
+	}
+}
+
+/*
  * Do we have another file that has the beginning components being a
  * proper superset of the name we're trying to add?
  */
diff --git a/update-index.c b/update-index.c
index f6b09a4..69b9a71 100644
--- a/update-index.c
+++ b/update-index.c
@@ -8,6 +8,7 @@ #include "strbuf.h"
 #include "quote.h"
 #include "cache-tree.h"
 #include "tree-walk.h"
+#include "dir.h"
 
 /*
  * Default to not allowing changes to the list of files. The
@@ -245,70 +246,6 @@ static int refresh_cache(int really)
 	return has_errors;
 }
 
-/*
- * We fundamentally don't like some paths: we don't want
- * dot or dot-dot anywhere, and for obvious reasons don't
- * want to recurse into ".git" either.
- *
- * Also, we don't want double slashes or slashes at the
- * end that can make pathnames ambiguous.
- */
-static int verify_dotfile(const char *rest)
-{
-	/*
-	 * The first character was '.', but that
-	 * has already been discarded, we now test
-	 * the rest.
-	 */
-	switch (*rest) {
-	/* "." is not allowed */
-	case '\0': case '/':
-		return 0;
-
-	/*
-	 * ".git" followed by  NUL or slash is bad. This
-	 * shares the path end test with the ".." case.
-	 */
-	case 'g':
-		if (rest[1] != 'i')
-			break;
-		if (rest[2] != 't')
-			break;
-		rest += 2;
-	/* fallthrough */
-	case '.':
-		if (rest[1] == '\0' || rest[1] == '/')
-			return 0;
-	}
-	return 1;
-}
-
-static int verify_path(const char *path)
-{
-	char c;
-
-	goto inside;
-	for (;;) {
-		if (!c)
-			return 1;
-		if (c == '/') {
-inside:
-			c = *path++;
-			switch (c) {
-			default:
-				continue;
-			case '/': case '\0':
-				break;
-			case '.':
-				if (verify_dotfile(path))
-					continue;
-			}
-			return 0;
-		}
-		c = *path++;
-	}
-}
-
 static int add_cacheinfo(unsigned int mode, const unsigned char *sha1,
 			 const char *path, int stage)
 {

^ permalink raw reply related

* Re: Do "git add" as a builtin
From: Linus Torvalds @ 2006-05-18 15:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v64k3698l.fsf@assigned-by-dhcp.cox.net>



On Thu, 18 May 2006, Junio C Hamano wrote:
> 
> Well, not good as-is.  This makes it barf on this sequence:
> 
> 	$ rm -f junk
>         $ cd junk
>         $ git init-db
>         $ date >frotz
>         $ mkdir nitfol
>         $ date >nitfol/rezrov
> 	$ git add .		;# OK up to this point - added everything.
> 
> 	$ git add .		;# This is bogus because...
>         fatal: pathspec '' did not match any files

Ahh. The empty pathspec is special.

It's special for a totally stupid reason: it's not a valid pathname, but 
we obviously _turn_ it into a valid pathname when we read the directory 
tree (ie from a filesystem standpoint, it means ".").

So then, when we do the "lstat()", we really _should_ have done the same.

If course, since the lstat() is there just to test for existence, and 
since "." always exists, it's easier to just pass an empty match entirely 
than to turn it into "." and then do an lstat that we know is going to 
succeed.

So the patch would be something trivial like this..

		Linus

---
diff --git a/builtin-add.c b/builtin-add.c
index 7083820..ac9ed2d 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -124,7 +124,7 @@ static void prune_directory(struct dir_s
 
 		/* Existing file? We must have ignored it */
 		match = pathspec[i];
-		if (!lstat(match, &st))
+		if (!*match || !lstat(match, &st))
 			continue;
 		die("pathspec '%s' did not match any files", match);
 	}

^ permalink raw reply related

* Re: [Patch] git-cvsimport: tiny fix
From: Elrond @ 2006-05-18 13:29 UTC (permalink / raw)
  To: Martin Langhoff, Junio C Hamano; +Cc: git
In-Reply-To: <46a038f90605172153mac96f40id9a50d2f29c75915@mail.gmail.com>

On Thu, May 18, 2006 at 04:53:56PM +1200, Martin Langhoff wrote:
> On 5/18/06, Junio C Hamano <junkio@cox.net> wrote:
> >Could somebody who actually works with CVS import Ack this?
> >Pretty please?
> 
> Sounds sane. It would be interesting to hear about what repo (and
> server) this was seen against. Elrond, can you tell us more about
> this?

This is a private local repository.
git-cvsimport starts a local "cvs server" for this.

I tried to create a minimal repo, that would trigger the
same behaviour, but didn't succeed. My current guessing is,
that this happened in cooperation with "cvsps" caching.
(cvsps is a tool used by git-cvsimport).

I will move the cvsps cache for my problematic repo out of
the way and try a git-cvsimport from scratch to verify that
the above mentioned issue is related to cvsps caching.
But I still think, that handling all cvs pserver replies
should be done anyway in git-cvsimport (when it relies on
this mode of operation).


    Elrond

^ permalink raw reply

* [PATCH] SubmittingPatches: The download location of External Editor has moved
From: Lukas Sandström @ 2006-05-18 12:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List


Signed-off-by: Lukas Sandström <lukass@etek.chalmers.se>

---
I'm using the new version right now.

 Documentation/SubmittingPatches |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

d95d1ac9e4cd408cca8da421a4313b149002e1f9
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 318b04f..8601949 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -266,8 +266,8 @@ This recipe appears to work with the cur
 The following Thunderbird extensions are needed:
 	AboutConfig 0.5
 		http://aboutconfig.mozdev.org/
-	External Editor 0.5.4
-		http://extensionroom.mozdev.org/more-info/exteditor
+	External Editor 0.7.2
+		http://globs.org/articles.php?lng=en&pg=8
 
 1) Prepare the patch as a text file using your method of choice.
 
-- 
1.3.2.g3c45-dirty

^ permalink raw reply related

* [PATCH] Remove the non-builtin git-check-ref-format
From: Lukas Sandström @ 2006-05-18 12:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Lukas Sandström, Git Mailing List
In-Reply-To: <446C657B.7020100@etek.chalmers.se>


Signed-off-by: Lukas Sandström <lukass@etek.chalmers.se>

---

 Makefile           |    2 +-
 check-ref-format.c |   17 -----------------
 2 files changed, 1 insertions(+), 18 deletions(-)
 delete mode 100644 check-ref-format.c

d459dd88b8b541bbfa03bb3bebfac152d3d1b9e5
diff --git a/Makefile b/Makefile
index b737cb8..ee62ca2 100644
--- a/Makefile
+++ b/Makefile
@@ -164,7 +164,7 @@ PROGRAMS = \
 	git-ssh-upload$X git-tar-tree$X git-unpack-file$X \
 	git-unpack-objects$X git-update-index$X git-update-server-info$X \
 	git-upload-pack$X git-verify-pack$X git-write-tree$X \
-	git-update-ref$X git-symbolic-ref$X git-check-ref-format$X \
+	git-update-ref$X git-symbolic-ref$X \
 	git-name-rev$X git-pack-redundant$X git-repo-config$X git-var$X \
 	git-describe$X git-merge-tree$X git-blame$X git-imap-send$X
 
diff --git a/check-ref-format.c b/check-ref-format.c
deleted file mode 100644
index a0adb3d..0000000
--- a/check-ref-format.c
+++ /dev/null
@@ -1,17 +0,0 @@
-/*
- * GIT - The information manager from hell
- */
-
-#include "cache.h"
-#include "refs.h"
-
-#include <stdio.h>
-
-int main(int ac, char **av)
-{
-	if (ac != 2)
-		usage("git-check-ref-format refname");
-	if (check_ref_format(av[1]))
-		exit(1);
-	return 0;
-}
-- 
1.3.2.g3c45-dirty

^ permalink raw reply related

* [PATCH] Make git-check-format-ref a builtin.
From: Lukas Sandström @ 2006-05-18 12:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List


Signed-off-by: Lukas Sandström <lukass@etek.chalmers.se>

---
It was already written in C, but now it's 219k less in my ~/bin dir.

 Makefile                   |    4 ++--
 builtin-check-ref-format.c |   13 +++++++++++++
 builtin.h                  |    2 ++
 git.c                      |    1 +
 4 files changed, 18 insertions(+), 2 deletions(-)
 create mode 100644 builtin-check-ref-format.c

2e18872b12d1635a6814053f3cfc3c9e433db428
diff --git a/Makefile b/Makefile
index 3a28580..b737cb8 100644
--- a/Makefile
+++ b/Makefile
@@ -170,7 +170,7 @@ PROGRAMS = \
 
 BUILT_INS = git-log$X git-whatchanged$X git-show$X \
 	git-count-objects$X git-diff$X git-push$X \
-	git-grep$X git-add$X
+	git-grep$X git-add$X git-check-ref-format$X
 
 # what 'all' will build and 'install' will install, in gitexecdir
 ALL_PROGRAMS = $(PROGRAMS) $(SIMPLE_PROGRAMS) $(SCRIPTS)
@@ -218,7 +218,7 @@ LIB_OBJS = \
 
 BUILTIN_OBJS = \
 	builtin-log.o builtin-help.o builtin-count.o builtin-diff.o builtin-push.o \
-	builtin-grep.o builtin-add.o
+	builtin-grep.o builtin-add.o builtin-check-ref-format.o
 
 GITLIBS = $(LIB_FILE) $(XDIFF_LIB)
 LIBS = $(GITLIBS) -lz
diff --git a/builtin-check-ref-format.c b/builtin-check-ref-format.c
new file mode 100644
index 0000000..ac29383
--- /dev/null
+++ b/builtin-check-ref-format.c
@@ -0,0 +1,13 @@
+/*
+ * GIT - The information manager from hell
+ */
+
+#include "cache.h"
+#include "refs.h"
+
+int cmd_check_ref_format(int argc, const char **argv, char **envp)
+{
+	if (argc != 2)
+		usage("git check-ref-format refname");
+	return check_ref_format(argv[1]) == 0 ? 0 : 1;
+}
diff --git a/builtin.h b/builtin.h
index ccd0e31..a26f2c2 100644
--- a/builtin.h
+++ b/builtin.h
@@ -27,4 +27,6 @@ extern int cmd_push(int argc, const char
 extern int cmd_grep(int argc, const char **argv, char **envp);
 extern int cmd_add(int argc, const char **argv, char **envp);
 
+extern int cmd_check_ref_format(int argc, const char **argv, char **envp);
+
 #endif
diff --git a/git.c b/git.c
index 6a470cf..62baf25 100644
--- a/git.c
+++ b/git.c
@@ -52,6 +52,7 @@ static void handle_internal_command(int 
 		{ "diff", cmd_diff },
 		{ "grep", cmd_grep },
 		{ "add", cmd_add },
+		{ "check-ref-format", cmd_check_ref_format }
 	};
 	int i;
 
-- 
1.3.2.g3c45-dirty

^ permalink raw reply related

* Re: [PATCH] Handle branch names with slashes
From: Catalin Marinas @ 2006-05-18 12:11 UTC (permalink / raw)
  To: Karl Hasselström; +Cc: Junio C Hamano, git
In-Reply-To: <20060518065040.GA10497@backpacker.hemma.treskal.com>

On 18/05/06, Karl Hasselström <kha@treskal.com> wrote:
> Teach stgit to handle branch names with slashes in them; that is,
> branches living in a subdirectory of .git/refs/heads.
[...]
> Catalin, I hope you're paying attention when trying to pick the
> correct three patches out of the salvos I've sent you. :-)

Hopefully, I applied them correctly. I'll update the public repository
tonight and you can check whether they are OK or not.

> --- a/stgit/commands/common.py
> +++ b/stgit/commands/common.py
[...]
> +    if len(dirs) != 0:
> +        # We have branch names with / in them.
> +        branch_chars = r'[^@]'
> +        patch_id_mark = r'//'
> +    else:
> +        # No / in branch names.
> +        branch_chars = r'[^@%/]'

I removed % from the above regexp.

Thanks for the patches. Great work.

-- 
Catalin

^ permalink raw reply

* Re: Shipping man pages?
From: Tilman Sauerbeck @ 2006-05-18 10:57 UTC (permalink / raw)
  To: git
In-Reply-To: <7vac9f69la.fsf@assigned-by-dhcp.cox.net>


[-- Attachment #1.1: Type: text/plain, Size: 1049 bytes --]

Junio C Hamano [2006-05-18 01:06]:
> Tilman Sauerbeck <tilman@code-monkey.de> writes:
> 
> [snip]
> 
> Why does this have to come up so often, and everybody who asks
> for them never supplies the patch to do so?

If it comes up that often it would indicate that this is actually a
concern to many people o_O

Also, I prefer to ask whether a patch would even be accepted so I don't
waste 3 hours of my life trying to figure out how to set up asciidoc and
docbook.

> > Or maybe offer them in a separate tarball?
> 
> Things that are buildable from the source do not belong in the
> source tarball.  If somebody wants to do this as a patch, I can
> be talked into accepting it, but the build procedure should
> build a separate tarball (or two; one for man and another for
> woman^Whtml).

I attached a patch.

Regards,
Tilman

-- 
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?

[-- Attachment #1.2: git-doc-dist.patch --]
[-- Type: text/plain, Size: 1555 bytes --]

Created a dist target for Documentation/Makefile that tars up the man pages and html files.

Signed-off-by: Tilman Sauerbeck <tilman@code-monkey.de>


---

 Documentation/Makefile |   16 +++++++++++++++-
 1 files changed, 15 insertions(+), 1 deletions(-)

f8b6b70c89364418724899ab5ca28aaaf3eee7dc
diff --git a/Documentation/Makefile b/Documentation/Makefile
index c1af22c..271d9e4 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -31,6 +31,7 @@ man7=$(mandir)/man7
 # DESTDIR=
 
 INSTALL?=install
+TAR?=tar
 
 #
 # Please note that there is a minor bug in asciidoc.
@@ -55,6 +56,18 @@ install: man
 	$(INSTALL) $(DOC_MAN1) $(DESTDIR)/$(man1)
 	$(INSTALL) $(DOC_MAN7) $(DESTDIR)/$(man7)
 
+-include ../GIT-VERSION-FILE
+
+dist: html man
+	@mkdir -p git-doc-{html,man}-$(GIT_VERSION)
+	@cp $(DOC_HTML) git-doc-html-$(GIT_VERSION)
+	@cp $(DOC_MAN1) $(DOC_MAN7) git-doc-man-$(GIT_VERSION)
+	
+	@for d in html man; do \
+		$(TAR) cf git-doc-$$d-$(GIT_VERSION).tar git-doc-$$d-$(GIT_VERSION) && \
+		rm -rf git-doc-$$d-$(GIT_VERSION) && \
+		gzip -f -9 git-doc-$$d-$(GIT_VERSION).tar \
+	; done
 
 #
 # Determine "include::" file references in asciidoc files.
@@ -73,7 +86,8 @@ README: ../README
 
 
 clean:
-	rm -f *.xml *.html *.1 *.7 howto-index.txt howto/*.html doc.dep README
+	rm -f *.xml *.html *.1 *.7 howto-index.txt howto/*.html doc.dep README \
+	      git-doc-{html,man}-$(GIT_VERSION).tar.gz
 
 %.html : %.txt
 	asciidoc -b xhtml11 -d manpage -f asciidoc.conf $<
-- 
1.3.3


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

^ permalink raw reply related

* Re: [PATCH] Implement git-quiltimport (take 2)
From: Eric W. Biederman @ 2006-05-18 10:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy7x09qet.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> writes:

> ebiederm@xmission.com (Eric W. Biederman) writes:
>
>> Junio C Hamano <junkio@cox.net> writes:
>>
>>> What's the expected workflow for you to work on a 1300 patch
>>> series you get from Andrew in the next installment to deal with
>>> 88 unattributed patches?  Answer the question 88 times and make
>>> sure you get the answers right every time?  Or abort and
>>> hand-edit them to help mailinfo to notice the correct
>>> attribution and re-run?
>>
>> For the internal consumption case it isn't a big deal.  I
>> can specify --author with something bogus and it works. 
>
> Yes.
>
>>> I know I am guilty of suggesting "going interactive", but I have
>>> a feeling that having an optional file that maps patch-name to
>>> author might be easier to work with.  If the old patches are
>>> recycled in the updated -mm set, you probably can reuse the
>>> mapping for them, adding entries for newly introduced "unnamed"
>>> patches as needed.
>>
>> Short of getting the script where it has a sane restart in the
>> middle mode going interactive and asking questions makes a lot
>> of sense.  Especially with smaller trees.
>
> Yes perhaps on smaller trees, but that does not mean much.  For
> smaller trees and/or smaller patch series almost anything would
> do.

Yes, a smaller patch series, that is what I meant.
Most quilt trees that I know about are in needed small.

Andrews is the only one I know of that has gets as far as sucking in
other quilt trees.

> How about doing something like this, so that the user can record
> the fixup information, especially with --dry-run patch?  Then
> the next round from the updated -mm tree the user would not have
> to retype them again ("then..fi" part should be indented in the
> final version, but I did not want indentation changes to
> distract you):

This might be a sane work flow.  My imagination actually had
the user making a copy of the quilt tree and editing it by
hand.  My --dry-run doesn't ask the question it just throws
errors so --dry-run isn't quite the right name.

So I guess with something like --dry-run there isn't a restart
problem.  The question is if we don't edit the patches themselves
where do we put your author-fixup tag?  .dotest? 

Eric

^ permalink raw reply

* Re: [PATCH] Provide a way to flush git-diff-tree's output
From: Paul Mackerras @ 2006-05-18  9:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmzdf6bj5.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano writes:

> Sounds low impact and sane.
> 
> I suspect the usual caveat on bidirectional pipe deadlock
> applies to the caller.  Does gitk do that?  The current code

Gitk will use non-blocking mode on the pipes to/from the git-diff-tree
process, so there isn't a possibility of deadlock that I can see.

> seems to feed a pre-generated list with "open | cmd <<"
> construct to the command, so perhaps you are planning to change
> that?

That's for the "Find" function.  I'm in the process of adding the code
to let users enter a list of paths and have gitk highlight the commits
affecting those paths.  That will involve a separate invocation of
git-diff-tree.  To make it responsive, I'm only going to ask
git-diff-tree about the commits that are visible on the screen - but I
need git-diff-tree to give me an answer quickly, i.e. in less time
than a human can perceive.

Thanks,
Paul.

^ permalink raw reply

* Re: Shipping man pages?
From: Mark Rosenstand @ 2006-05-18  9:41 UTC (permalink / raw)
  To: git
In-Reply-To: <7vac9f69la.fsf@assigned-by-dhcp.cox.net>

On Thu, 2006-05-18 at 01:06 -0700, Junio C Hamano wrote:
> Tilman Sauerbeck <tilman@code-monkey.de> writes:
> 
> > atm, the git release tarballs don't contain man pages.
> 
> I ship *source* tarball.

Which is great for generating binaries and other things that are likely
to be incompatible across systems.

> I also happen to do RPM for people who do not want to build from
> the source (btw, I do that from pure inertia). In addition,
> preformatted manual pages and html docs are available from man
> and html branches of the git.git repository.
> 
> If you are building from the source, please build from the
> source.  Everything you need is right there.

But asciidoc is a royal PITA to package or install - it doesn't even
provide a Makefile: http://www.methods.co.nz/asciidoc/userguide.html#X38

Additionally it carries the whole docbook dependency chain with it.

> If you don't build from the source, please use whatever binary
> distribution available out there.  RPM happens to be available
> from kernel.org.  If you are on Debian/Ubuntu/Gentoo/others,
> please ask your distribution packager to include the manpages
> and html docs, if they don't already.

Even the packagers are likely to hate the unneccessary asciidoc
dependency. As a result some of the small distributions that don't have
the manpower to support 1000+ packages choose to ship git without the
man pages, which is a shame, IMO.

> Why does this have to come up so often, and everybody who asks
> for them never supplies the patch to do so?

Because it seems like a political decision rather than a technical one
(it's trivial to add the docs as a prerequisite for the dist target.)

> > Or maybe offer them in a separate tarball?
> 
> Things that are buildable from the source do not belong in the
> source tarball.  If somebody wants to do this as a patch, I can
> be talked into accepting it, but the build procedure should
> build a separate tarball (or two; one for man and another for
> woman^Whtml).

That would be great! I'd love to submit a patch, but I wouldn't be able
to test it, because I'd need asciidoc.

^ permalink raw reply

* Re: cvsimport weird
From: Bertrand Jacquin @ 2006-05-18  9:33 UTC (permalink / raw)
  To: Pavel Roskin; +Cc: Martin Langhoff, Git Mailing List
In-Reply-To: <1147931094.32050.51.camel@dv>

On 5/18/06, Pavel Roskin <proski@gnu.org> wrote:
> On Wed, 2006-05-17 at 23:59 -0400, Pavel Roskin wrote:
> > I'm quite sure that it's a bug in cvsps.  It displays such things on
> > x86_64, but works properly on 32-bit PowerPC.
>
> Address resolution is broken in cvsps on 64-bit machines.  This patch to
> cvsps is needed:

Ah Thanks, I'm running it on a x86_64.

-- 
Beber
#e.fr@freenode

^ permalink raw reply

* [WARNING] Please be careful when using "git add" from "next" branch
From: Junio C Hamano @ 2006-05-18  8:52 UTC (permalink / raw)
  To: git; +Cc: linux-kernel
In-Reply-To: <7vwtcj4tp6.fsf@assigned-by-dhcp.cox.net>

There is still a small breakage in the built-in "git add" on the
"next" branch that ends up creating nonsense tree objects.

        $ mkdir foo
        $ date >bar
        $ git-add foo/../bar
        $ git ls-files
        foo/../bar
        $ git ls-tree -r -t $(git-write-tree)
        040000 tree ef5562cd3a9bf66d41a8d4f42f159e8c694ce7e3	foo
        040000 tree 6e1612248e8da43fc5f91592e559da1ad5f9a852	foo/..
        100644 blob 53ab446c3f4e42ce9bb728a0ccb283a101be4979	foo/../bar

If you do not do funky things like foo/../bar, I do not think
you have to worry, but scripted use might break.  It used to
warn and ignore such bogus input, but now it happily accepts it
and produces bogus index which results in bogus trees.

"git update-index --add" is not affected by this breakage.

^ permalink raw reply

* Re: Do "git add" as a builtin
From: Junio C Hamano @ 2006-05-18  8:34 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <7v64k3698l.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> writes:

> Linus Torvalds <torvalds@osdl.org> writes:
>
>> On Wed, 17 May 2006, Junio C Hamano wrote:
>>> 
>>> By "not seeing the point", do you mean you do not agree with
>>> what bba319b5 and 45e48120 tried to do to help users?
>>
>> Naah, I just didn't see why, and didn't bother to go exploring.
>>
>> How about this patch on top of the previous one?
>
> Well, not good as-is.  This makes it barf on this sequence:
>...

Ouch, things are worse than I thought...

	$ mkdir foo
        $ date >bar
        $ git-add foo/../bar
	$ git ls-files
        foo/../bar

Huh?

^ permalink raw reply

* Re: Fwd: [OT] Re: Git via a proxy server?
From: Jan-Benedict Glaw @ 2006-05-18  8:31 UTC (permalink / raw)
  To: Sam Song; +Cc: Petr Vandrovec, git
In-Reply-To: <20060518034428.42456.qmail@web32002.mail.mud.yahoo.com>

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

On Wed, 2006-05-17 20:44:28 -0700, Sam Song <samlinuxkernel@yahoo.com> wrote:
> Petr Vandrovec <petr@vmware.com> wrote:
> > Jan-Benedict Glaw <jbglaw@lug-owl.de> wrote:
> > > Well, install some package to have `socket'
> > > available? Debian calls
> > > the packet `socket', too, so I guess Fedora may
> > > have something similar.
> > 
> > Surprisingly they do not...  You should be able to
> > replace 'socket' with 
> > 'netcat' - and I believe that netcat/nc package is
> > available for Fedora.  For 
> > this purpose they have same command line & behavior.
> 
> Ummm, I am trying on that. nc is avaiable for Fedora.
> But what could be the replacement for CONNECT in
> Fedora? :-)

Erm, you haven't understood what you're doing there, have you?

With the GIT_PROXY_COMMAND helper, you're expected to create a clean
tunnel which in turn git can use to transfer its data.

You've only got some limited internet connectivity via a HTTP proxy
available, so you need to use this. This means:

  * The proxy administrator needs to allos outgoing connections for
    the CONNECT method with git's TCP port.
  * You need to have some minimalistic program to initially speak HTTP
    with the proxy and later on just stream the raw git protocol
    through the link.
  * You may or may not need to strip anything that came into the git
    stream by accident because you tunnled it through a HTTP proxy. A
    reply message from the proxy server is an example for this.

So this little script (using "CONNECT" and netcat or socket) does the
first part: it talks in the language HTTP with the proxy server. It
may be enough to just use CONNECT, but you may need to speak some more
lines, eg. for proxy authorization.

The first `cat' in there is just for pushing the git protocol though the
HTTP proxy connection later on (hopefully after the proxy was made to
accept the the CONNECT request.)  Once the proxy accepted it, it'll
send you a HTTP/200 message (or something like that) and an empty
line. This is what the two reads are for; the next `cat' simply again
transfers all the rest (the git protocol).

To draw the line, there's not _one_ solution to HTTP proxy tunneling,
there are many, and you'll need to design one that fits your network.
It should be quite simple, given that you've got nice tools like
`strace' and `tcpdump', which will help you to understand how the
proxy reacts and so on.

MfG, JBG

-- 
Jan-Benedict Glaw       jbglaw@lug-owl.de    . +49-172-7608481             _ O _
"Eine Freie Meinung in  einem Freien Kopf    | Gegen Zensur | Gegen Krieg  _ _ O
 für einen Freien Staat voll Freier Bürger"  | im Internet! |   im Irak!   O O O
ret = do_actions((curr | FREE_SPEECH) & ~(NEW_COPYRIGHT_LAW | DRM | TCPA));

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

^ permalink raw reply

* Re: Do "git add" as a builtin
From: Junio C Hamano @ 2006-05-18  8:13 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0605171321020.10823@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> On Wed, 17 May 2006, Junio C Hamano wrote:
>> 
>> By "not seeing the point", do you mean you do not agree with
>> what bba319b5 and 45e48120 tried to do to help users?
>
> Naah, I just didn't see why, and didn't bother to go exploring.
>
> How about this patch on top of the previous one?

Well, not good as-is.  This makes it barf on this sequence:

	$ rm -f junk
        $ cd junk
        $ git init-db
        $ date >frotz
        $ mkdir nitfol
        $ date >nitfol/rezrov
	$ git add .		;# OK up to this point - added everything.

	$ git add .		;# This is bogus because...
        fatal: pathspec '' did not match any files
	$ git add nitfol	;# ...this does not barf.

I admit I did not spot it when I read the code, but this part
gets an empty string for 'match' when pathspec is '.'.

> +		/* Existing file? We must have ignored it */
> +		match = pathspec[i];
> +		if (!lstat(match, &st))
> +			continue;
> +		die("pathspec '%s' did not match any files", match);

That's why '.' barfs but nitfol doesn't.

^ permalink raw reply

* Re: Shipping man pages?
From: Fernando J. Pereda @ 2006-05-18  8:10 UTC (permalink / raw)
  To: Tilman Sauerbeck; +Cc: git
In-Reply-To: <20060518074630.GA2994@code-monkey.de>

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

On Thu, May 18, 2006 at 09:46:32AM +0200, Tilman Sauerbeck wrote:
> Hi,
> atm, the git release tarballs don't contain man pages. They can be
> generated from the asciidoc source files, which makes the build depend
> on python and asciidoc.
> 
> That's *very* inconvenient; would it be possible to include the man
> pages in the release tarball?
> 
> Or maybe offer them in a separate tarball?

Hi Tilman,

Actually Junio has 'html' and 'man' branches in his git.git repository
so you just have to use git tar-tree on them.

I do this for the Gentoo packages, you can grab a tarball from any of
our mirrors, the files are called git-{html,man}-VERSION.tar.bz2

- ferdy

-- 
Fernando J. Pereda Garcimartín
Gentoo Developer (Alpha,net-mail,mutt,git)
20BB BDC3 761A 4781 E6ED  ED0B 0A48 5B0C 60BD 28D4

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

^ permalink raw reply

* Re: Shipping man pages?
From: Junio C Hamano @ 2006-05-18  8:06 UTC (permalink / raw)
  To: Tilman Sauerbeck; +Cc: git
In-Reply-To: <20060518074630.GA2994@code-monkey.de>

Tilman Sauerbeck <tilman@code-monkey.de> writes:

> atm, the git release tarballs don't contain man pages.

I ship *source* tarball.

I also happen to do RPM for people who do not want to build from
the source (btw, I do that from pure inertia). In addition,
preformatted manual pages and html docs are available from man
and html branches of the git.git repository.

If you are building from the source, please build from the
source.  Everything you need is right there.

If you don't build from the source, please use whatever binary
distribution available out there.  RPM happens to be available
from kernel.org.  If you are on Debian/Ubuntu/Gentoo/others,
please ask your distribution packager to include the manpages
and html docs, if they don't already.

Why does this have to come up so often, and everybody who asks
for them never supplies the patch to do so?

> Or maybe offer them in a separate tarball?

Things that are buildable from the source do not belong in the
source tarball.  If somebody wants to do this as a patch, I can
be talked into accepting it, but the build procedure should
build a separate tarball (or two; one for man and another for
woman^Whtml).

^ permalink raw reply

* Shipping man pages?
From: Tilman Sauerbeck @ 2006-05-18  7:46 UTC (permalink / raw)
  To: git

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

Hi,
atm, the git release tarballs don't contain man pages. They can be
generated from the asciidoc source files, which makes the build depend
on python and asciidoc.

That's *very* inconvenient; would it be possible to include the man
pages in the release tarball?

Or maybe offer them in a separate tarball?

Thanks,
Tilman

-- 
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?

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

^ permalink raw reply

* Re: [PATCH] Provide a way to flush git-diff-tree's output
From: Junio C Hamano @ 2006-05-18  7:24 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: git
In-Reply-To: <17516.6955.282732.460675@cargo.ozlabs.ibm.com>

Paul Mackerras <paulus@samba.org> writes:

> The --stdin flag to git-diff-tree is suitable for this, but the main
> difficulty is that the output of git-diff-tree gets buffered and
> doesn't get sent until the buffer is full.
>
> This provides a way to get git-diff-tree to flush its output buffers.
> If a blank line is supplied on git-diff-tree's standard input, it will
> flush its output buffers and then accept further input.

Sounds low impact and sane.

I suspect the usual caveat on bidirectional pipe deadlock
applies to the caller.  Does gitk do that?  The current code
seems to feed a pre-generated list with "open | cmd <<"
construct to the command, so perhaps you are planning to change
that?

^ permalink raw reply

* [PATCH] Provide a way to flush git-diff-tree's output
From: Paul Mackerras @ 2006-05-18  6:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Gitk wants to use git-diff-tree as a filter to tell it which ids from
a given list affect a set of files or directories.  We don't want to
fork and exec a new git-diff-tree process for each batch of ids, since
there could be a lot of relatively small batches.  For example, a
batch could contain as many ids as fit in gitk's headline display
window, i.e. 20 or so, and we would be processing a new batch every
time the user scrolls that window.

The --stdin flag to git-diff-tree is suitable for this, but the main
difficulty is that the output of git-diff-tree gets buffered and
doesn't get sent until the buffer is full.

This provides a way to get git-diff-tree to flush its output buffers.
If a blank line is supplied on git-diff-tree's standard input, it will
flush its output buffers and then accept further input.

Signed-off-by: Paul Mackerras <paulus@samba.org>
---
diff --git a/diff-tree.c b/diff-tree.c
index 7207867..69bb74b 100644
--- a/diff-tree.c
+++ b/diff-tree.c
@@ -138,7 +138,10 @@ int main(int argc, const char **argv)
 		opt->diffopt.setup |= (DIFF_SETUP_USE_SIZE_CACHE |
 				       DIFF_SETUP_USE_CACHE);
 	while (fgets(line, sizeof(line), stdin))
-		diff_tree_stdin(line);
+		if (line[0] == '\n')
+			fflush(stdout);
+		else
+			diff_tree_stdin(line);
 
 	return 0;
 }

^ permalink raw reply related

* [PATCH] Handle branch names with slashes
From: Karl Hasselström @ 2006-05-18  6:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Catalin Marinas
In-Reply-To: <20060518064214.GA10390@backpacker.hemma.treskal.com>

Teach stgit to handle branch names with slashes in them; that is,
branches living in a subdirectory of .git/refs/heads.

I had to change the patch@branch/top command-line syntax to
patch@branch//top, in order to get sane parsing. The /top variant is
still available for repositories that have no slashy branches; it is
disabled as soon as there exists at least one subdirectory of
refs/heads. Preferably, this compatibility hack can be killed some
time in the future.

Signed-off-by: Karl Hasselström <kha@treskal.com>


---

Oh yeah, remember to change % to // in the commit comment as well . . .

Catalin, I hope you're paying attention when trying to pick the
correct three patches out of the salvos I've sent you. :-)

 stgit/commands/branch.py |    5 ++
 stgit/commands/common.py |  108 +++++++++++++++++++++++++++-------------------
 stgit/commands/diff.py   |   16 ++++---
 stgit/commands/files.py  |    4 +-
 stgit/commands/id.py     |    2 -
 stgit/commands/mail.py   |    8 ++-
 stgit/git.py             |   42 +++++++++---------
 stgit/stack.py           |   21 ++++++---
 stgit/utils.py           |   88 +++++++++++++++++++++++++++++++++++--
 9 files changed, 202 insertions(+), 92 deletions(-)

4ce56cd9e2d39f3a98b6dd010d11beb6037f8ff3
diff --git a/stgit/commands/branch.py b/stgit/commands/branch.py
index 2218bbb..d348409 100644
--- a/stgit/commands/branch.py
+++ b/stgit/commands/branch.py
@@ -172,7 +172,10 @@ def func(parser, options, args):
         if len(args) != 0:
             parser.error('incorrect number of arguments')
 
-        branches = os.listdir(os.path.join(basedir.get(), 'refs', 'heads'))
+        branches = []
+        basepath = os.path.join(basedir.get(), 'refs', 'heads')
+        for path, files, dirs in walk_tree(basepath):
+            branches += [os.path.join(path, f) for f in files]
         branches.sort()
 
         if branches:
diff --git a/stgit/commands/common.py b/stgit/commands/common.py
index c6ca514..93344aa 100644
--- a/stgit/commands/common.py
+++ b/stgit/commands/common.py
@@ -18,7 +18,7 @@ along with this program; if not, write t
 Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 """
 
-import sys, os, re
+import sys, os, os.path, re
 from optparse import OptionParser, make_option
 
 from stgit.utils import *
@@ -34,54 +34,74 @@ class CmdException(Exception):
 
 
 # Utility functions
+class RevParseException(Exception):
+    """Revision spec parse error."""
+    pass
+
+def parse_rev(rev):
+    """Parse a revision specification into its
+    patchname@branchname//patch_id parts. If no branch name has a slash
+    in it, also accept / instead of //."""
+    files, dirs = list_files_and_dirs(os.path.join(basedir.get(),
+                                                   'refs', 'heads'))
+    if len(dirs) != 0:
+        # We have branch names with / in them.
+        branch_chars = r'[^@]'
+        patch_id_mark = r'//'
+    else:
+        # No / in branch names.
+        branch_chars = r'[^@%/]'
+        patch_id_mark = r'(/|//)'
+    patch_re = r'(?P<patch>[^@/]+)'
+    branch_re = r'@(?P<branch>%s+)' % branch_chars
+    patch_id_re = r'%s(?P<patch_id>[a-z.]*)' % patch_id_mark
+
+    # Try //patch_id.
+    m = re.match(r'^%s$' % patch_id_re, rev)
+    if m:
+        return None, None, m.group('patch_id')
+
+    # Try path[@branch]//patch_id.
+    m = re.match(r'^%s(%s)?%s$' % (patch_re, branch_re, patch_id_re), rev)
+    if m:
+        return m.group('patch'), m.group('branch'), m.group('patch_id')
+
+    # Try patch[@branch].
+    m = re.match(r'^%s(%s)?$' % (patch_re, branch_re), rev)
+    if m:
+        return m.group('patch'), m.group('branch'), None
+
+    # No, we can't parse that.
+    raise RevParseException
+
 def git_id(rev):
     """Return the GIT id
     """
     if not rev:
         return None
-    
-    rev_list = rev.split('/')
-    if len(rev_list) == 2:
-        patch_id = rev_list[1]
-        if not patch_id:
-            patch_id = 'top'
-    elif len(rev_list) == 1:
-        patch_id = 'top'
-    else:
-        patch_id = None
-
-    patch_branch = rev_list[0].split('@')
-    if len(patch_branch) == 1:
-        series = crt_series
-    elif len(patch_branch) == 2:
-        series = stack.Series(patch_branch[1])
-    else:
-        raise CmdException, 'Unknown id: %s' % rev
-
-    patch_name = patch_branch[0]
-    if not patch_name:
-        patch_name = series.get_current()
-        if not patch_name:
-            raise CmdException, 'No patches applied'
-
-    # patch
-    if patch_name in series.get_applied() \
-           or patch_name in series.get_unapplied():
-        if patch_id == 'top':
-            return series.get_patch(patch_name).get_top()
-        elif patch_id == 'bottom':
-            return series.get_patch(patch_name).get_bottom()
-        # Note we can return None here.
-        elif patch_id == 'top.old':
-            return series.get_patch(patch_name).get_old_top()
-        elif patch_id == 'bottom.old':
-            return series.get_patch(patch_name).get_old_bottom()
-
-    # base
-    if patch_name == 'base' and len(rev_list) == 1:
-        return read_string(series.get_base_file())
-
-    # anything else failed
+    try:
+        patch, branch, patch_id = parse_rev(rev)
+        if branch == None:
+            series = crt_series
+        else:
+            series = stack.Series(branch)
+        if patch == None:
+            patch = series.get_current()
+            if not patch:
+                raise CmdException, 'No patches applied'
+        if patch in series.get_applied() or patch in series.get_unapplied():
+            if patch_id in ['top', '', None]:
+                return series.get_patch(patch).get_top()
+            elif patch_id == 'bottom':
+                return series.get_patch(patch).get_bottom()
+            elif patch_id == 'top.old':
+                return series.get_patch(patch).get_old_top()
+            elif patch_id == 'bottom.old':
+                return series.get_patch(patch).get_old_bottom()
+        if patch == 'base' and patch_id == None:
+            return read_string(series.get_base_file())
+    except RevParseException:
+        pass
     return git.rev_parse(rev + '^{commit}')
 
 def check_local_changes():
diff --git a/stgit/commands/diff.py b/stgit/commands/diff.py
index 7dc6c5d..d765784 100644
--- a/stgit/commands/diff.py
+++ b/stgit/commands/diff.py
@@ -33,12 +33,12 @@ or a tree-ish object and another tree-is
 be given to restrict the diff output. The tree-ish object can be a
 standard git commit, tag or tree. In addition to these, the command
 also supports 'base', representing the bottom of the current stack,
-and '[patch]/[bottom | top]' for the patch boundaries (defaulting to
+and '[patch][//[bottom | top]]' for the patch boundaries (defaulting to
 the current one):
 
-rev = '([patch]/[bottom | top]) | <tree-ish> | base'
+rev = '([patch][//[bottom | top]]) | <tree-ish> | base'
 
-If neither bottom or top are given but a '/' is present, the command
+If neither bottom nor top are given but a '//' is present, the command
 shows the specified patch (defaulting to the current one)."""
 
 options = [make_option('-r', metavar = 'rev1[:[rev2]]', dest = 'revs',
@@ -55,10 +55,14 @@ def func(parser, options, args):
         rev_list = options.revs.split(':')
         rev_list_len = len(rev_list)
         if rev_list_len == 1:
-            if rev_list[0][-1] == '/':
+            rev = rev_list[0]
+            if rev[-1] == '/':
                 # the whole patch
-                rev1 = rev_list[0] + 'bottom'
-                rev2 = rev_list[0] + 'top'
+                rev = rev[:-1]
+                if rev[-1] == '/':
+                    rev = rev[:-1]
+                rev1 = rev + '//bottom'
+                rev2 = rev + '//top'
             else:
                 rev1 = rev_list[0]
                 rev2 = None
diff --git a/stgit/commands/files.py b/stgit/commands/files.py
index 0694d83..b33bd2a 100644
--- a/stgit/commands/files.py
+++ b/stgit/commands/files.py
@@ -53,8 +53,8 @@ def func(parser, options, args):
     else:
         parser.error('incorrect number of arguments')
 
-    rev1 = git_id('%s/bottom' % patch)
-    rev2 = git_id('%s/top' % patch)
+    rev1 = git_id('%s//bottom' % patch)
+    rev2 = git_id('%s//top' % patch)
 
     if options.stat:
         print git.diffstat(rev1 = rev1, rev2 = rev2)
diff --git a/stgit/commands/id.py b/stgit/commands/id.py
index 1cf6ea6..284589a 100644
--- a/stgit/commands/id.py
+++ b/stgit/commands/id.py
@@ -28,7 +28,7 @@ usage = """%prog [options] [id]
 
 Print the hash value of a GIT id (defaulting to HEAD). In addition to
 the standard GIT id's like heads and tags, this command also accepts
-'base[@<branch>]' and '[<patch>[@<branch>]][/(bottom | top)]'. If no
+'base[@<branch>]' and '[<patch>[@<branch>]][//[bottom | top]]'. If no
 'top' or 'bottom' are passed and <patch> is a valid patch name, 'top'
 will be used by default."""
 
diff --git a/stgit/commands/mail.py b/stgit/commands/mail.py
index 5e01ea1..3928b81 100644
--- a/stgit/commands/mail.py
+++ b/stgit/commands/mail.py
@@ -324,10 +324,10 @@ def __build_message(tmpl, patch, patch_n
                  'shortdescr':   short_descr,
                  'longdescr':    long_descr,
                  'endofheaders': headers_end,
-                 'diff':         git.diff(rev1 = git_id('%s/bottom' % patch),
-                                          rev2 = git_id('%s/top' % patch)),
-                 'diffstat':     git.diffstat(rev1 = git_id('%s/bottom'%patch),
-                                              rev2 = git_id('%s/top' % patch)),
+                 'diff':         git.diff(rev1 = git_id('%s//bottom' % patch),
+                                          rev2 = git_id('%s//top' % patch)),
+                 'diffstat':     git.diffstat(rev1 = git_id('%s//bottom'%patch),
+                                              rev2 = git_id('%s//top' % patch)),
                  'date':         email.Utils.formatdate(localtime = True),
                  'version':      version_str,
                  'patchnr':      patch_nr_str,
diff --git a/stgit/git.py b/stgit/git.py
index 2884f36..716609c 100644
--- a/stgit/git.py
+++ b/stgit/git.py
@@ -225,7 +225,8 @@ def get_head():
 def get_head_file():
     """Returns the name of the file pointed to by the HEAD link
     """
-    return os.path.basename(_output_one_line('git-symbolic-ref HEAD'))
+    return strip_prefix('refs/heads/',
+                        _output_one_line('git-symbolic-ref HEAD'))
 
 def set_head_file(ref):
     """Resets HEAD to point to a new ref
@@ -233,7 +234,8 @@ def set_head_file(ref):
     # head cache flushing is needed since we might have a different value
     # in the new head
     __clear_head_cache()
-    if __run('git-symbolic-ref HEAD', [ref]) != 0:
+    if __run('git-symbolic-ref HEAD',
+             [os.path.join('refs', 'heads', ref)]) != 0:
         raise GitException, 'Could not set head to "%s"' % ref
 
 def __set_head(val):
@@ -272,6 +274,7 @@ def rev_parse(git_id):
 def branch_exists(branch):
     """Existence check for the named branch
     """
+    branch = os.path.join('refs', 'heads', branch)
     for line in _output_lines('git-rev-parse --symbolic --all 2>&1'):
         if line.strip() == branch:
             return True
@@ -282,12 +285,11 @@ def branch_exists(branch):
 def create_branch(new_branch, tree_id = None):
     """Create a new branch in the git repository
     """
-    new_head = os.path.join('refs', 'heads', new_branch)
-    if branch_exists(new_head):
+    if branch_exists(new_branch):
         raise GitException, 'Branch "%s" already exists' % new_branch
 
     current_head = get_head()
-    set_head_file(new_head)
+    set_head_file(new_branch)
     __set_head(current_head)
 
     # a checkout isn't needed if new branch points to the current head
@@ -297,22 +299,22 @@ def create_branch(new_branch, tree_id = 
     if os.path.isfile(os.path.join(basedir.get(), 'MERGE_HEAD')):
         os.remove(os.path.join(basedir.get(), 'MERGE_HEAD'))
 
-def switch_branch(name):
+def switch_branch(new_branch):
     """Switch to a git branch
     """
     global __head
 
-    new_head = os.path.join('refs', 'heads', name)
-    if not branch_exists(new_head):
-        raise GitException, 'Branch "%s" does not exist' % name
+    if not branch_exists(new_branch):
+        raise GitException, 'Branch "%s" does not exist' % new_branch
 
-    tree_id = rev_parse(new_head + '^{commit}')
+    tree_id = rev_parse(os.path.join('refs', 'heads', new_branch)
+                        + '^{commit}')
     if tree_id != get_head():
         refresh_index()
         if __run('git-read-tree -u -m', [get_head(), tree_id]) != 0:
             raise GitException, 'git-read-tree failed (local changes maybe?)'
         __head = tree_id
-    set_head_file(new_head)
+    set_head_file(new_branch)
 
     if os.path.isfile(os.path.join(basedir.get(), 'MERGE_HEAD')):
         os.remove(os.path.join(basedir.get(), 'MERGE_HEAD'))
@@ -320,25 +322,23 @@ def switch_branch(name):
 def delete_branch(name):
     """Delete a git branch
     """
-    branch_head = os.path.join('refs', 'heads', name)
-    if not branch_exists(branch_head):
+    if not branch_exists(name):
         raise GitException, 'Branch "%s" does not exist' % name
-    os.remove(os.path.join(basedir.get(), branch_head))
+    remove_file_and_dirs(os.path.join(basedir.get(), 'refs', 'heads'),
+                         name)
 
 def rename_branch(from_name, to_name):
     """Rename a git branch
     """
-    from_head = os.path.join('refs', 'heads', from_name)
-    if not branch_exists(from_head):
+    if not branch_exists(from_name):
         raise GitException, 'Branch "%s" does not exist' % from_name
-    to_head = os.path.join('refs', 'heads', to_name)
-    if branch_exists(to_head):
+    if branch_exists(to_name):
         raise GitException, 'Branch "%s" already exists' % to_name
 
     if get_head_file() == from_name:
-        set_head_file(to_head)
-    os.rename(os.path.join(basedir.get(), from_head), \
-              os.path.join(basedir.get(), to_head))
+        set_head_file(to_name)
+    rename(os.path.join(basedir.get(), 'refs', 'heads'),
+           from_name, to_name)
 
 def add(names):
     """Add the files or recursively add the directory contents
diff --git a/stgit/stack.py b/stgit/stack.py
index f83161b..49b50e7 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -443,8 +443,7 @@ class Series:
 
         os.makedirs(self.__patch_dir)
 
-        if not os.path.isdir(bases_dir):
-            os.makedirs(bases_dir)
+        create_dirs(bases_dir)
 
         create_empty_file(self.__applied_file)
         create_empty_file(self.__unapplied_file)
@@ -502,11 +501,14 @@ class Series:
         git.rename_branch(self.__name, to_name)
 
         if os.path.isdir(self.__series_dir):
-            os.rename(self.__series_dir, to_stack.__series_dir)
+            rename(os.path.join(self.__base_dir, 'patches'),
+                   self.__name, to_stack.__name)
         if os.path.exists(self.__base_file):
-            os.rename(self.__base_file, to_stack.__base_file)
+            rename(os.path.join(self.__base_dir, 'refs', 'bases'),
+                   self.__name, to_stack.__name)
         if os.path.exists(self.__refs_dir):
-            os.rename(self.__refs_dir, to_stack.__refs_dir)
+            rename(os.path.join(self.__base_dir, 'refs', 'patches'),
+                   self.__name, to_stack.__name)
 
         self.__init__(to_name)
 
@@ -560,16 +562,19 @@ class Series:
             else:
                 print 'Patch directory %s is not empty.' % self.__name
             if not os.listdir(self.__series_dir):
-                os.rmdir(self.__series_dir)
+                remove_dirs(os.path.join(self.__base_dir, 'patches'),
+                            self.__name)
             else:
                 print 'Series directory %s is not empty.' % self.__name
             if not os.listdir(self.__refs_dir):
-                os.rmdir(self.__refs_dir)
+                remove_dirs(os.path.join(self.__base_dir, 'refs', 'patches'),
+                            self.__name)
             else:
                 print 'Refs directory %s is not empty.' % self.__refs_dir
 
         if os.path.exists(self.__base_file):
-            os.remove(self.__base_file)
+            remove_file_and_dirs(
+                os.path.join(self.__base_dir, 'refs', 'bases'), self.__name)
 
     def refresh_patch(self, files = None, message = None, edit = False,
                       show_patch = False,
diff --git a/stgit/utils.py b/stgit/utils.py
index 5749b3b..68b8f58 100644
--- a/stgit/utils.py
+++ b/stgit/utils.py
@@ -1,6 +1,8 @@
 """Common utility functions
 """
 
+import errno, os, os.path
+
 __copyright__ = """
 Copyright (C) 2005, Catalin Marinas <catalin.marinas@gmail.com>
 
@@ -18,6 +20,12 @@ along with this program; if not, write t
 Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 """
 
+def mkdir_file(filename, mode):
+    """Opens filename with the given mode, creating the directory it's
+    in if it doesn't already exist."""
+    create_dirs(os.path.dirname(filename))
+    return file(filename, mode)
+
 def read_string(filename, multiline = False):
     """Reads the first line from a file
     """
@@ -32,7 +40,7 @@ def read_string(filename, multiline = Fa
 def write_string(filename, line, multiline = False):
     """Writes 'line' to file and truncates it
     """
-    f = file(filename, 'w+')
+    f = mkdir_file(filename, 'w+')
     if multiline:
         f.write(line)
     else:
@@ -42,7 +50,7 @@ def write_string(filename, line, multili
 def append_strings(filename, lines):
     """Appends 'lines' sequence to file
     """
-    f = file(filename, 'a+')
+    f = mkdir_file(filename, 'a+')
     for line in lines:
         print >> f, line
     f.close()
@@ -50,14 +58,14 @@ def append_strings(filename, lines):
 def append_string(filename, line):
     """Appends 'line' to file
     """
-    f = file(filename, 'a+')
+    f = mkdir_file(filename, 'a+')
     print >> f, line
     f.close()
 
 def insert_string(filename, line):
     """Inserts 'line' at the beginning of the file
     """
-    f = file(filename, 'r+')
+    f = mkdir_file(filename, 'r+')
     lines = f.readlines()
     f.seek(0); f.truncate()
     print >> f, line
@@ -67,4 +75,74 @@ def insert_string(filename, line):
 def create_empty_file(name):
     """Creates an empty file
     """
-    file(name, 'w+').close()
+    mkdir_file(name, 'w+').close()
+
+def list_files_and_dirs(path):
+    """Return the sets of filenames and directory names in a
+    directory."""
+    files, dirs = [], []
+    for fd in os.listdir(path):
+        full_fd = os.path.join(path, fd)
+        if os.path.isfile(full_fd):
+            files.append(fd)
+        elif os.path.isdir(full_fd):
+            dirs.append(fd)
+    return files, dirs
+
+def walk_tree(basedir):
+    """Starting in the given directory, iterate through all its
+    subdirectories. For each subdirectory, yield the name of the
+    subdirectory (relative to the base directory), the list of
+    filenames in the subdirectory, and the list of directory names in
+    the subdirectory."""
+    subdirs = ['']
+    while subdirs:
+        subdir = subdirs.pop()
+        files, dirs = list_files_and_dirs(os.path.join(basedir, subdir))
+        for d in dirs:
+            subdirs.append(os.path.join(subdir, d))
+        yield subdir, files, dirs
+
+def strip_prefix(prefix, string):
+    """Return string, without the prefix. Blow up if string doesn't
+    start with prefix."""
+    assert string.startswith(prefix)
+    return string[len(prefix):]
+
+def remove_dirs(basedir, dirs):
+    """Starting at join(basedir, dirs), remove the directory if empty,
+    and try the same with its parent, until we find a nonempty
+    directory or reach basedir."""
+    path = dirs
+    while path:
+        try:
+            os.rmdir(os.path.join(basedir, path))
+        except OSError:
+            return # can't remove nonempty directory
+        path = os.path.dirname(path)
+
+def remove_file_and_dirs(basedir, file):
+    """Remove join(basedir, file), and then remove the directory it
+    was in if empty, and try the same with its parent, until we find a
+    nonempty directory or reach basedir."""
+    os.remove(os.path.join(basedir, file))
+    remove_dirs(basedir, os.path.dirname(file))
+
+def create_dirs(directory):
+    """Create the given directory, if the path doesn't already exist."""
+    if directory:
+        create_dirs(os.path.dirname(directory))
+        try:
+            os.mkdir(directory)
+        except OSError, e:
+            if e.errno != errno.EEXIST:
+                raise e
+
+def rename(basedir, file1, file2):
+    """Rename join(basedir, file1) to join(basedir, file2), not
+    leaving any empty directories behind and creating any directories
+    necessary."""
+    full_file2 = os.path.join(basedir, file2)
+    create_dirs(os.path.dirname(full_file2))
+    os.rename(os.path.join(basedir, file1), full_file2)
+    remove_dirs(basedir, os.path.dirname(file1))
-- 
1.3.2.g639c


-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox