Git development
 help / color / mirror / Atom feed
* 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

* [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: 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

* 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: [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: [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: 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] 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

* [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

* [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] 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

* 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

* 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: 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: [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 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: 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: Junio C Hamano @ 2006-05-18 16:50 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0605180929450.10823@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

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

I like this much better than the old (pre-builtin) behaviour.

> 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).

Perhaps, but unresolve splits an entry that is available from
the heads being merged, so it would use unverified filename to
try finding the ents from trees, but get_tree_entry() would not
find one, so I think we are safe already.  Nevertheless, I think
your change makes things more strict and safer.

I doubt this would break people's scripts.  If they were relying
on the old behaviour, that means they threw real paths and
garbage at update-index and relied on it to sift them apart,
which indicates they were buggy to begin with anyway.

^ permalink raw reply

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



On Thu, 18 May 2006, Junio C Hamano wrote:
> 
> Perhaps, but unresolve splits an entry that is available from
> the heads being merged, so it would use unverified filename to
> try finding the ents from trees, but get_tree_entry() would not
> find one, so I think we are safe already.

Yes, I agree, that looks safe.

> I doubt this would break people's scripts.  If they were relying
> on the old behaviour, that means they threw real paths and
> garbage at update-index and relied on it to sift them apart,
> which indicates they were buggy to begin with anyway.

Agreed. I don't think the semantic change can really break anything, and 
if anything will probably help expose any really strange/broken stuff.

		Linus

^ permalink raw reply

* git status and empty directories
From: Matthias Lederhofer @ 2006-05-18 17:57 UTC (permalink / raw)
  To: git

Today I've been working on some source with many empty directories.
git-status shows all empty directories under "Untracked files: (use
"git add" to add to commit)" but adding is impossible. And if it would
be possible to add the directory, because a new file exists in the
directory, it still shows up in the same place, indistinguishable from
the empty directories.

Things I could think of to solve this:
- hide empty directories if the user does not ask explicitly for them
  with a command line option
- vice versa: add a command line option to hide empty directories
(I'd prefer the first one)

Another category in git-status for empty directories which cannot be
added could be interesting too because the comment 'use "git add" to
add to commit' is quite misleading.

Perhaps there should also be another option to show files in untracked
directories.

Any comments? If a patch would be accepted I'll take a look at
writing one. Adding the correct options to git-ls-files seems quite
trivial.

^ permalink raw reply

* Re: git status and empty directories
From: Linus Torvalds @ 2006-05-18 18:12 UTC (permalink / raw)
  To: Matthias Lederhofer; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <E1Fgmkh-0000ur-Hn@moooo.ath.cx>



On Thu, 18 May 2006, Matthias Lederhofer wrote:
>
> - hide empty directories if the user does not ask explicitly for them
>   with a command line option

Like this?

Makes sense to me, although I didn't add the option to disable it.

		Linus

---
diff --git a/git-commit.sh b/git-commit.sh
index 6ef1a9d..ed00b5c 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -136,11 +136,11 @@ #'
 
 	if test -f "$GIT_DIR/info/exclude"
 	then
-	    git-ls-files -z --others --directory \
+	    git-ls-files -z --others --directory --no-empty-directory \
 		--exclude-from="$GIT_DIR/info/exclude" \
 		--exclude-per-directory=.gitignore
 	else
-	    git-ls-files -z --others --directory \
+	    git-ls-files -z --others --directory --no-empty-directory \
 		--exclude-per-directory=.gitignore
 	fi |
 	perl -e '$/ = "\0";

^ permalink raw reply related

* Re: git status and empty directories
From: Matthias Lederhofer @ 2006-05-18 18:29 UTC (permalink / raw)
  To: Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0605181112040.10823@g5.osdl.org>

> Like this?
This is exactly the patch I'm using at the moment and I don't really
need an option to unhide empty directories. Maybe someone else
complains for some reason but git clean -d -n shows empty directories
too.

An option to show untracked files instead of untracked directories
like
> git-ls-files -z --others --exclude-per-directory=.gitignore
would be interesting to see what would be added instead of doing ls
manually.

^ permalink raw reply

* Re: Shipping man pages?
From: Junio C Hamano @ 2006-05-18 18:33 UTC (permalink / raw)
  To: Mark Rosenstand; +Cc: git, Tilman Sauerbeck
In-Reply-To: <1147945298.1320.35.camel@mjollnir>

Mark Rosenstand <mark@borkware.net> writes:

> On Thu, 2006-05-18 at 01:06 -0700, Junio C Hamano wrote:
>
>> 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.

That's a consequence of _your_ choice to build the documentation
files from the source, when I give you preformatted files in
html/man branches and/or prepackaged binary distributions.  Even
plain "make all" nor "make install" do not build them.

IOW, not my problem.

We accomplish things by saying "I did this, it solves my
problem, and it would help others -- so I share", not by
demanding others to do things for you by saying "If you do this,
it would solve my problem.  Now go do it".  That's how open
source works.

>> 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

I do not see why that is political.  Do you need a politician to
tell you what is source and what isn't?

> (it's trivial to add the docs as a prerequisite for the dist target.)

Being trivial does not change things a whit, because I do not do
things I consider useless only because they are trivial.

You have to first convince me that it is useful to others, and
one way to do so is by showing that you care deeply enough about
it -- doing the work yourself (instead of demanding _me_ to do
something I do not believe is a good idea yet) is a good way to
do so.  That would tell me that it is a real problem to you.
When that happens, I might start considering the possibility
that a solution to that problem may be useful to other people.

And it actually makes things actively worse to whine without
doing the work yourself when the necessary change is trivial.
You are saying that you cannot be bothered to do that yourself
even though the change is trivial, which implies you _can_ live
without formatted pages just fine.  The conclusion is that not
having the formatted pages is not such a big deal to you (after
all, asciidoc toolchain might be a bear to install, but the
documents formatted in it are very easy to read in the source
form).

Now, with a patch, Tilman showed us he cares deeply enough, so
I'll take a look at it.  Thanks, Tilman.

^ permalink raw reply

* Re: Shipping man pages?
From: Linus Torvalds @ 2006-05-18 18:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Mark Rosenstand, git, Tilman Sauerbeck
In-Reply-To: <7vodxvkws8.fsf@assigned-by-dhcp.cox.net>



On Thu, 18 May 2006, Junio C Hamano wrote:
> 
> That's a consequence of _your_ choice to build the documentation
> files from the source, when I give you preformatted files in
> html/man branches and/or prepackaged binary distributions.  Even
> plain "make all" nor "make install" do not build them.

Btw, in case others didn't notice, the easiest way to install the 
pre-packaged branch is a truly disgusting hack:

	cd git
	git tar-tree man | (cd /usr/share/man ; tar xvf -)

or similar (yeah, you need to be root to do the unpack, of course, and 
you may need to change the /usr/share/man to whatever is appropriate for 
your distribution).

No need to actually even check out the 'man' branch.

		Linus

^ permalink raw reply

* Feature wish: Cloning without history
From: Sven Ekman @ 2006-05-18 19:21 UTC (permalink / raw)
  To: git

Hello,

Would it be possible to add an option to git-clone to
skip the complete history? The result should be a
repository which contains the current head only (or
maybe a specified tag) and has that commit id added to
.git/info/grafts. For the fetch process, this would
certainly have to imply the --no-tags flag.

>From a user's point of view I'd imagine something like
this:

git-clone --no-history=v2.6.16 \
    git://git.kernel.org/.../linux-2.6.git

The background: I'm regularly building kernels for a
handful of machines, and while I am happy to use the
blessings of git to get updates from the -stable
releases, I see no point in wasting space for a copy
of the complete kernel history on every single
machine. In practice this works pretty good, once I
have manually created such a castrated repository.

Sven

^ 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