Git development
 help / color / mirror / Atom feed
* Re: [msysGit] Using VC build git
From: Frank Li @ 2009-08-11  1:26 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, msysGit
In-Reply-To: <alpine.DEB.1.00.0908101609170.8324@intel-tinevez-2-302>

Thank you take care my patch.
I can fix all problems.

This patch is base on v1.6.4 release.  My working branch is vc_build
at git://repo.or.cz/tgit.git.

That is actually prototype to approve VC can build git.
The code style is not big problem. I will fix it.

VC build will reuse many msysgit works because msysgit really do many
work at windows porting.

I think the below is most important problem.

1.  Where are vcbuild directory put, is it okay under contrib ?
2.  How to handle external library, such as zlib? Can use submodule?


2009/8/10, Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> Hi,
>
> On Mon, 10 Aug 2009, Johannes Schindelin wrote:
>
>> Please, we have _high_ standards in git.git, and I really do not want to
>> have to take anything to Junio that does not fulfill that standard.
>
> To elaborate:  if I see something like this in the --stat:
>
>  contrib/vcbuild/include/zlib.h        | 1357
> +++++++++++++++++++++++++++++++++
>  contrib/vcbuild/lib/zlib.lib          |  Bin 0 -> 104148 bytes
>
> ... I know already that there is no way this can make it into git.git.
> There just is not.
>
> Also, if the first commit says nothing else than "Rebase to v1.6.4", it
> is pretty obvious to me that I will not sign off on that (and I just guess
> that is the very reason you did not sign off on that, either).
>
> Further, putting anything into contrib/ that really belongs into contrib/
> is not cutting it, either.
>
> And I am pretty astonished that mingw.[ch] is touched, as VC is definitely
> not MinGW32.
>
> Changing 1000+ lines of libgit.vcproj in almost every commit is also
> something I really do not look upon favorably.
>
> Finally, if _no single_ commit message says _anything_ about the reasons
> why you had to change code outside of vcbuild/, I am only puzzled.
>
> Now, I want to give you a pretty clear idea what has to be done if this is
> going into 4msysgit.git, ever, because you obviously spent a lot of time
> on it, and other people want it, too:
>
> - changing "open" to "_open" in mingw.c is a no-no-no.  If you need to use
>   "_open" in VC, then define "open" in the compile flags for mingw.c, but
>   leave code that is not written for VC alone.
>
> - introducing trailing whitespace is usually a sign of not caring enough
>   about clean and neat code.  So just don't do it.
>
> - making link() fail on MinGW32 just to be able to compile it with VC is
>   outright rude against all people who use a free and open compiler
>   instead of a closed one.
>
> - changing an "_snprintf" to "_vsnprintf" in vcbuild/porting.c without
>   anything else is a clear and loud sign that the code before was broken,
>   and that you fix a faulty patch in a later patch.  This is not how we do
>   things in git.git.  We fix the proper patch before the patch series is
>   accepted into mainline.
>
> - violating the coding style -- even if it is in your VC-specific part --
>   is not an option.  You need to fix the coding style.
>
> - violating the coding style in files that are not VC-specific is not an
>   option at all.  You really need to fix it.
>
> - changing the default editor from "vi" to "notepad2" will break almost
>   every existing Git user's setup.  That is just inexcusable.
>
> Note: these comments are _just for the last_ of your 5 patches.
>
> Just a brief comment on the 4th patch, because I really do not want to
> spend more time on this round of patches: spelling the opendir() function
> as "open dir" function in the commit message is misleading, to say the
> least, and moving code that was added in a previous patch in the same
> patch series just shows that it was a mistake to begin with.  Besides,
> don't move anything into mingw.c if MinGW32 does not need it.
>
> Hth,
> Dscho
>
>
>

^ permalink raw reply

* Re: [PATCH] git-instaweb: detect difference between mod_cgi and mod_cgid for apache2
From: Mark A Rada @ 2009-08-11  1:23 UTC (permalink / raw)
  To: Mark A Rada; +Cc: git
In-Reply-To: <8366E0BD-3A64-4DDD-B12C-2FD48C357FFB@mailservices.uwaterloo.ca>

If you haven't already, ignore this patch, I'm pretty sure it won't  
work for cases where mod_cgid
is built in. I think it should be including the ScriptSock directive  
when built in as well, but I don't
have a setup to test that on, and the apache documentation isn't very  
specific.

I'll try again later, sorry for the spam.


--
Mark A Rada (ferrous26)
marada@uwaterloo.ca


On 10-Aug-09, at 1:19 AM, Mark A Rada wrote:

> I'm not sure if this should have been two patches or just one.
>
> First I changed the check of whether mod_cgi is built in or not from  
> matching against
> 'mod_cgi.c' to 'mod_cgi' because I made the assumption that mod_cgid  
> would be called
> 'mod_cgid.c' (I haven't actually checked this, but it sounds  
> reasonable). Then, the
> second change handles the two cases when no cgi module is built in,  
> which I  did try
> (for mod_cgid that is).
>
> Also, should the socket needed for mod_cgid be put in $GIT_DIR or  
> somewhere else?
>
>
> tl;dr I only actually tried 1/4 cases, everything else looks  
> reasonable
>
>
> --
> Mark A Rada (ferrous26)
> marada@uwaterloo.ca
>
>
> --->8---
> Some people have mod_cgid in place of mod_cgi, this will check which  
> one
> the user has available to them first, and then act accordingly.
>
> Signed-off-by: Mark Rada <marada@uwaterloo.ca>
> ---
> git-instaweb.sh |   10 ++++++++--
> 1 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/git-instaweb.sh b/git-instaweb.sh
> index 5f5cac7..fdac0d0 100755
> --- a/git-instaweb.sh
> +++ b/git-instaweb.sh
> @@ -297,8 +297,14 @@ EOF
> 		# plain-old CGI
> 		resolve_full_httpd
> 		list_mods=$(echo "$full_httpd" | sed "s/-f$/-l/")
> -		$list_mods | grep 'mod_cgi\.c' >/dev/null 2>&1 || \
> -		echo "LoadModule cgi_module $module_path/mod_cgi.so" >> "$conf"
> +		$list_mods | grep 'mod_cgi' >/dev/null 2>&1 || \
> +		if test -f "$module_path/mod_cgi.so"
> +		then
> +			echo "LoadModule cgi_module $module_path/mod_cgi.so" >> "$conf"
> +		else
> +			echo "LoadModule cgi_module $module_path/mod_cgid.so" >> "$conf"
> +			echo "ScriptSock $GIT_DIR/gitweb.sock" >> "$conf"
> +		fi
> 		cat >> "$conf" <<EOF
> AddHandler cgi-script .cgi
> <Location /gitweb.cgi>
> -- 
> 1.6.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] fix potential infinite loop given large unsigned integer
From: Jeff Epler @ 2009-08-11  0:55 UTC (permalink / raw)
  To: Erik Faye-Lund; +Cc: Junio C Hamano, Ryan Flynn, git
In-Reply-To: <40aa078e0908090525h7b4d6efeh658e2edcfbe16c7e@mail.gmail.com>

On Sun, Aug 09, 2009 at 02:25:40PM +0200, Erik Faye-Lund wrote:
> log10() appears to be C99, but can be emulated on earlier C-versions by doing
> #define log10(x) (log(x) / log(10.0))

I don't think you'll like the results of this very much.
    #include <math.h>
    #include <stdio.h>

    int main(void) {
        double n=1;
        int i, j;
        for(i=0; i<10; i++, n*=10) {
            j = (int)(log(n)/log(10));
            if(i != j) printf("%d %d\n", i, (int)j);
        }
        return 0;
    }

(on my system, 3 of the 10 tested cases give the wrong answer due to
rounding)

For a tour of some of the difficulties of implementing log10,
    http://www.cs.berkeley.edu/~wkahan/LOG10HAF.TXT

Jeff

^ permalink raw reply

* [PATCH] Fix typos in git-remote.txt and git-symbolic-ref.txt
From: Štěpán Němec @ 2009-08-11  0:52 UTC (permalink / raw)
  To: git; +Cc: trast

Signed-off-by: Štěpán Němec <stepnem@gmail.com>
---
I apologize for the previous mail.
Thank you for clarification and guidance.

I did look at some of the other documentation files (and now also grepped
the whole directory for `--' and `\--' occurences), but wasn't able to see
any rule or consistence in the usage. Whether it is an indication of
real inconsistence or just another example of my ignorance I do not know
(not now, at least).

The stray backslashes were fixed by setting ASCIIDOC8, thanks again.

Štěpán Němec


 Documentation/git-remote.txt       |    4 ++--
 Documentation/git-symbolic-ref.txt |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt
index 9e2b4ea..82a3d29 100644
--- a/Documentation/git-remote.txt
+++ b/Documentation/git-remote.txt
@@ -114,14 +114,14 @@ These stale branches have already been removed from the remote repository
 referenced by <name>, but are still locally available in
 "remotes/<name>".
 +
-With `--dry-run` option, report what branches will be pruned, but do no
+With `--dry-run` option, report what branches will be pruned, but do not
 actually prune them.

 'update'::

 Fetch updates for a named set of remotes in the repository as defined by
 remotes.<group>.  If a named group is not specified on the command line,
-the configuration parameter remotes.default will get used; if
+the configuration parameter remotes.default will be used; if
 remotes.default is not defined, all remotes which do not have the
 configuration parameter remote.<name>.skipDefaultUpdate set to true will
 be updated.  (See linkgit:git-config[1]).
diff --git a/Documentation/git-symbolic-ref.txt b/Documentation/git-symbolic-ref.txt
index 210fde0..6392538 100644
--- a/Documentation/git-symbolic-ref.txt
+++ b/Documentation/git-symbolic-ref.txt
@@ -14,9 +14,9 @@ DESCRIPTION
 Given one argument, reads which branch head the given symbolic
 ref refers to and outputs its path, relative to the `.git/`
 directory.  Typically you would give `HEAD` as the <name>
-argument to see on which branch your working tree is on.
+argument to see which branch your working tree is on.

-Give two arguments, create or update a symbolic ref <name> to
+Given two arguments, creates or updates a symbolic ref <name> to
 point at the given branch <ref>.

 A symbolic ref is a regular file that stores a string that
--
1.6.3.3

^ permalink raw reply related

* block-sha1: improve code on large-register-set machines
From: Linus Torvalds @ 2009-08-10 23:52 UTC (permalink / raw)
  To: Git Mailing List, Junio C Hamano


For x86 performance (especially in 32-bit mode) I added that hack to write 
the SHA1 internal temporary hash using a volatile pointer, in order to get 
gcc to not try to cache the array contents. Because gcc will do all the 
wrong things, and then spill things in insane random ways.

But on architectures like PPC, where you have 32 registers, it's actually 
perfectly reasonable to put the whole temporary array[] into the register 
set, and gcc can do so.

So make the 'volatile unsigned int *' cast be dependent on a 
SMALL_REGISTER_SET preprocessor symbol, and enable it (currently) on just 
x86 and x86-64.  With that, the routine is fairly reasonable even when 
compared to the hand-scheduled PPC version. Ben Herrenschmidt reports on 
a G5:

 * Paulus asm version:       about 3.67s
 * Yours with no change:     about 5.74s
 * Yours without "volatile": about 3.78s

so with this the C version is within about 3% of the asm one.

And add a lot of commentary on what the heck is going on.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---

I also asked David Miller to test the non-volatile version on Sparc, but I 
suspect it will have the same pattern. ia64 likewise (but I have not asked 
anybody to test).

Of the other architectures, ARM probably would wants SMALL_REGISTER_SET, 
but I suspect the problem there is the htonl() (on little-endian), and 
possibly the unaligned loads - at least on older ARM. The latter is 
something gcc could be taught about, though (the SHA_SRC macro would just 
need to use a pointer that goes through a packed struct member or 
something).

 block-sha1/sha1.c |   25 ++++++++++++++++++++++++-
 1 files changed, 24 insertions(+), 1 deletions(-)

diff --git a/block-sha1/sha1.c b/block-sha1/sha1.c
index 36da763..9bc8b8a 100644
--- a/block-sha1/sha1.c
+++ b/block-sha1/sha1.c
@@ -82,6 +82,7 @@ void blk_SHA1_Final(unsigned char hashout[20], blk_SHA_CTX *ctx)
 #define SHA_ASM(op, x, n) ({ unsigned int __res; asm(op " %1,%0":"=r" (__res):"i" (n), "0" (x)); __res; })
 #define SHA_ROL(x,n)	SHA_ASM("rol", x, n)
 #define SHA_ROR(x,n)	SHA_ASM("ror", x, n)
+#define SMALL_REGISTER_SET
 
 #else
 
@@ -93,7 +94,29 @@ void blk_SHA1_Final(unsigned char hashout[20], blk_SHA_CTX *ctx)
 
 /* This "rolls" over the 512-bit array */
 #define W(x) (array[(x)&15])
-#define setW(x, val) (*(volatile unsigned int *)&W(x) = (val))
+
+/*
+ * If you have 32 registers or more, the compiler can (and should)
+ * try to change the array[] accesses into registers. However, on
+ * machines with less than ~25 registers, that won't really work,
+ * and at least gcc will make an unholy mess of it.
+ *
+ * So to avoid that mess which just slows things down, we force
+ * the stores to memory to actually happen (we might be better off
+ * with a 'W(t)=(val);asm("":"+m" (W(t))' there instead, as
+ * suggested by Artur Skawina - that will also make gcc unable to
+ * try to do the silly "optimize away loads" part because it won't
+ * see what the value will be).
+ *
+ * Ben Herrenschmidt reports that on PPC, the C version comes close
+ * to the optimized asm with this (ie on PPC you don't want that
+ * 'volatile', since there are lots of registers).
+ */
+#ifdef SMALL_REGISTER_SET
+  #define setW(x, val) (*(volatile unsigned int *)&W(x) = (val))
+#else
+  #define setW(x, val) (W(x) = (val))
+#endif
 
 /*
  * Where do we get the source from? The first 16 iterations get it from

^ permalink raw reply related

* Re: bug with .git file and aliases
From: Johannes Schindelin @ 2009-08-10 23:05 UTC (permalink / raw)
  To: Geoffrey Irving; +Cc: Jeff King, Junio C Hamano, git, Lars Hjemli
In-Reply-To: <7f9d599f0908101322i46384247m303e28955f88bbb@mail.gmail.com>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1933 bytes --]

Hi,

On Mon, 10 Aug 2009, Geoffrey Irving wrote:

> On Mon, Jul 20, 2009 at 11:21 AM, Jeff King<peff@peff.net> wrote:
> > On Mon, Jul 20, 2009 at 09:54:12AM -0400, Geoffrey Irving wrote:
> >
> >> git 1.6.3.3 has a bug related to .git file support and aliases.
> >> Specifically, if you make an alias for status and call it from a
> >> subdirectory, git status chdirs into the true .git dir but then
> >> chdir's back to the wrong place in order to run the lstats for status.
> >>  The result is that git status thinks all files have disappeared.
> >
> > Yeah, this is a known problem. The problem is that the 'git' wrapper
> > sets up the environment only partially when running aliases, and then
> > the resulting command ends up confused about where the worktree is. I
> > really don't remember the specifics, but you can probably find some
> > discussion in the list archives.  Fixing it, IIRC, required some
> > refactoring of the setup code (which I had hoped to get to at some
> > point, but I am way behind on my git todo list).
> 
> The attached patch fixes the bug for me.  I'll leave it to others to
> determine whether this is a good way to fix the problem.

Note that you made it particularly hard to comment on your patch by not 
granting us the wish stated in Documentation/SubmittingPatches, namely to 
inline your patch.

I'll just forego inlining it myself, as I am way past my bed-time and 
cannot be bothered.

However, I think that it is necessary to comment on your patch.

There is a few style issues, such as declaring offset outside of the 
block that is the only user, and there is the issue that you go out of 
your way to append a slash if you're resetting the work tree, but not when 
not resetting it.

But the bigger issue is that you now broke overriding the work tree via 
the command line.

The proper fix, of course, is to avoid calling the function with the wrong 
path to begin with.

Ciao,
Dscho

^ permalink raw reply

* [EGIT] Push to GitHub caused corruption
From: John Bito @ 2009-08-10 21:46 UTC (permalink / raw)
  To: git

Using the 'release' build of EGit (0.4.9.200906240051) I pushed a
commit to GitHub.  After that, using git to pull, I get 'bad tree
object' resulting in 'remote: aborting due to possible repository
corruption on the remote side'.  I had a similar problem back in April
(using integration builds of 0.4.0).  I'm willing to investigate if
there's interest in finding the root of the problem.

^ permalink raw reply

* Re: [EGIT PATCH] Provide a more JavaBeans-style 'getName' accessor for the id Signed-off-by: Alex Blewitt <alex.blewitt@gmail.com>
From: Shawn O. Pearce @ 2009-08-10 20:59 UTC (permalink / raw)
  To: Alex Blewitt; +Cc: robin.rosenberg@dewire.com, git@vger.kernel.org
In-Reply-To: <E861EAC5-150C-4CF8-AD0F-EBF15CEAE114@gmail.com>

Alex Blewitt <alex.blewitt@gmail.com> wrote:
> That patch was originally mailed on the 11th May. Has it taken until now 
> to notice the problem, or was the other method added in the last month or 
> so? If I'm to blame, I apologise but didn't note any compile time issues 
> at the time.

Arrgh, you are right, I lost this patch in my inbox, and in the
interm we applied new features to RevTag which added getName there.

> On 10 Aug 2009, at 16:52, "Shawn O. Pearce" <spearce@spearce.org> wrote:
>> Obviously you didn't compile test this in JGit:

Never mind.

>> ./org/spearce/jgit/revwalk/RevTag.java:206: getName() in
>> org.spearce.jgit.revwalk.RevTag cannot override getName() in
>> org.spearce.jgit.lib.AnyObjectId; overridden method is final

I can't apply this patch because getName() on RevTag is already
defined with a different meaning.  :-(

-- 
Shawn.

^ permalink raw reply

* Re: bug with .git file and aliases
From: Geoffrey Irving @ 2009-08-10 20:22 UTC (permalink / raw)
  To: Jeff King, Junio C Hamano; +Cc: git, Lars Hjemli
In-Reply-To: <20090720152117.GB5347@coredump.intra.peff.net>

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

On Mon, Jul 20, 2009 at 11:21 AM, Jeff King<peff@peff.net> wrote:
> On Mon, Jul 20, 2009 at 09:54:12AM -0400, Geoffrey Irving wrote:
>
>> git 1.6.3.3 has a bug related to .git file support and aliases.
>> Specifically, if you make an alias for status and call it from a
>> subdirectory, git status chdirs into the true .git dir but then
>> chdir's back to the wrong place in order to run the lstats for status.
>>  The result is that git status thinks all files have disappeared.
>
> Yeah, this is a known problem. The problem is that the 'git' wrapper
> sets up the environment only partially when running aliases, and then
> the resulting command ends up confused about where the worktree is. I
> really don't remember the specifics, but you can probably find some
> discussion in the list archives.  Fixing it, IIRC, required some
> refactoring of the setup code (which I had hoped to get to at some
> point, but I am way behind on my git todo list).

The attached patch fixes the bug for me.  I'll leave it to others to
determine whether this is a good way to fix the problem.

Thanks,
Geoffrey

[-- Attachment #2: 0001-setup.c-fix-work-tree-setup-for-.git-files-and-alias.patch --]
[-- Type: text/x-patch, Size: 1797 bytes --]

From ec47aa09e5bc8d9a8c07cca9f8ef17a9898819c1 Mon Sep 17 00:00:00 2001
From: Geoffrey Irving <irving@naml.us>
Date: Mon, 10 Aug 2009 15:59:21 -0400
Subject: [PATCH] setup.c: fix work tree setup for .git-files and aliases

When .git-files and aliases are used together, the setup machinery
gets confused and ends up with the wrong work_tree.  Specifically,
git_work_tree_cfg is set to the correct value first, but set_work_tree
resets git_work_tree_cfg to the current directory, which (at least in
this case) is incorrect.

set_work_tree now detects this case by checking to see if
git_work_tree_cfg is already set.  If so, it leaves git_work_tree_cfg
unchanged and instead uses the current directory to compute and return
the correct prefix (where we are relative to the work tree).

Signed-off-by: Geoffrey Irving <irving@naml.us>
---
 setup.c |   15 +++++++++++++--
 1 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/setup.c b/setup.c
index e3781b6..97f7eb1 100644
--- a/setup.c
+++ b/setup.c
@@ -198,13 +198,24 @@ int is_inside_work_tree(void)
 static const char *set_work_tree(const char *dir)
 {
 	char buffer[PATH_MAX + 1];
+	size_t offset;
 
 	if (!getcwd(buffer, sizeof(buffer)))
 		die ("Could not get the current working directory");
-	git_work_tree_cfg = xstrdup(buffer);
 	inside_work_tree = 1;
 
-	return NULL;
+	if (!git_work_tree_cfg) {
+		git_work_tree_cfg = xstrdup(buffer);
+		return NULL;
+	} else {
+		offset = strlen(git_work_tree_cfg);
+		if (memcmp(git_work_tree_cfg, buffer, offset)
+			|| (buffer[offset] && buffer[offset] != '/'))
+			die ("fatal: not inside work tree (should not happen)");
+		if (!buffer[offset] || !buffer[offset+1])
+			return NULL;
+		return xstrdup(strcat(buffer + offset + 1, "/"));
+	}
 }
 
 void setup_work_tree(void)
-- 
1.6.3.3


^ permalink raw reply related

* Re: [PATCH 5/8] Add a config option for remotes to specify a foreign vcs
From: Daniel Barkalow @ 2009-08-10 19:30 UTC (permalink / raw)
  To: Johan Herland; +Cc: git, Junio C Hamano, Brian Gernhardt, Johannes Schindelin
In-Reply-To: <200908101032.12835.johan@herland.net>

On Mon, 10 Aug 2009, Johan Herland wrote:

> On Monday 10 August 2009, Daniel Barkalow wrote:
> > On Sun, 9 Aug 2009, Junio C Hamano wrote:
> > > Daniel Barkalow <barkalow@iabervon.org> writes:
> > > > If this is set, the url is not required, and the transport always
> > > > uses a helper named "git-remote-<value>".
> > > >
> > > > It is a separate configuration option in order to allow a sensible
> > > > configuration for foreign systems which either have no meaningful
> > > > urls for repositories or which require urls that do not specify the
> > > > system used by the repository at that location. However, this only
> > > > affects how the name of the helper is determined, not anything about
> > > > the interaction with the helper, and the contruction is such that, if
> > > > the foreign scm does happen to use a co-named url method, a url with
> > > > that method may be used directly.
> > >
> > > Personally, I do not like this.
> > >
> > > Why isn't it enough to define the canonical remote name git takes as
> > > "<name of the helper>:<whatever string the helper understands>"?
> >
> > [...]
> >
> > The only way I've been able to come up with to support this at all
> > usefully is to have a bunch of helper-specific options that specify what
> > the helper needs to know about the locations you consider to be part of
> > the project and an option that tells git that this remote uses the p4
> > helper. I'm not sure what makes sense for other helpers, but the case I
> > actually use needs something like what's in this patch.
> 
> I'm somewhat agnostic on this issue. At the moment, I follow the P4 cues,
> and use a config like this:
> 
>     [remote "foo"]
>         vcs = cvs
>         cvsRoot = ":pserver:user@cvs.server.example.com/var/cvs/cvsroot"
>         cvsModule = "bar"
>         ...
> 
> But I could just as well use a config like this instead:
> 
>     [remote "foo"]
>         url = "cvs::pserver:user@cvs.server.example.com/var/cvs/cvsroot#bar"
>         ...
> 
> Either is fine with me, although I suspect users might find the
> current/first alternative easier to parse.

I suspect there will be some users who want (1) and some who want (2), and 
both ought to work.

> > I think it makes sense for svn access to support just having a url
> > option like "svn://something (svn native protocol)", or
> > "svn+ssh://something (svn protocol over ssh)" or "svn+https://something
> > (https access to a svn repo)", or some other similar syntax, but this is
> > a poor fit for p4.
> >
> > In order to support this, there just needs to be a call to check whether
> > "remote-<something>" is an available git command (without running it or
> > giving an error), and the helper code should be used if it is. This is
> > actually required so that people with workstations whose domain is
> > .kernel.org and who have cloned "master:/home/linus/something.git" don't
> > start getting "remote-master isn't a git command" errors (that is,
> > misinterpreting ssh-format location hostnames as helper names. Johan,
> > perhaps you could write that for your CVS helper?
> 
> Sorry, not following you here. Write exactly what?
> 
> - The code in the transport layer for checking if "remote-<something>"
>   is an available git command?

That's what I was thinking.

> - The code in my CVS helper for handling the ssh-format misinterpretation,
>   i.e. the case where someone has a git/ssh server called "cvs"? If so,
>   how should this be handled?

I'd been thinking that people's servers wouldn't be named "cvs", but they 
might be named things that aren't clearly not plausible names for helpers. 
So I don't think there should be a need to deal with misdirected names 
going to actual helpers, just names for helpers that don't exist.

> > I think that, ideally, helpers for foreign systems would be portable
> > across multiple native systems. The svn helper could be a program
> > "svn-remote-access-helper", and anything that speaks fast-import (e.g.,
> > bzr or hg) would be able to use it. When installing it for git, you'd
> > symlink it to git-remote-svn; if you decided to install
> > "svn-remote-access-helper-ng", you'd change the symlink.
> 
> In that case, helpers must keep their metadata in a repo-independent
> format. Currently that is outside the scope of my CVS helper, since I'm
> leveraging git-notes for most of the CVS helper's metadata.

Yeah, that's one of several tricky issues. I don't think there's enough 
experience yet to design something to support portable helpers, but I 
think it's worth thinking about.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: Help/Advice needed on diff bug in xutils.c
From: Thell Fowler @ 2009-08-10 18:54 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0908052239180.8306@pacific.mpi-cbg.de>

Johannes Schindelin (Johannes.Schindelin@gmx.de) wrote on Aug 5, 2009:

> On Tue, 4 Aug 2009, Thell Fowler wrote:
>
>> There is a bug in git diff (ignoring whitespace) that does not take into
>> account a trailing space at the end of a line at the end of a file when
>> no new line follows.

First, please forgive my hubris at thinking I had _found_ a bug when my 
ignorance of the issue was so obvious.  I am definitely humbled after 
reading every post in the archive having to do with whitespace and diff.

>>
>> Here is the example of the bug:
>> mkdir test_ws_eof
>> cd test_ws_eof
>> git init
>> echo -n "Test" > test.txt
>> git add .
>> git commit -m'test'
>> git symbolic-ref HEAD refs/heads/with_space
>> rm .git/index
>> git clean -f
>> echo -n "Test ">test.txt
>> git add .
>> git commit -m'test'
>> # Ignoring all whitespace there shouldn't be a diff.
>> git diff -w master -- test.txt
>> # Ignoring space at eol there shouldn't be a diff
>> git diff --ignore-space-at-eol master -- test.txt
>> # Ignoring with -b might have a case for a diff showing.
>> git diff -b master -- test.txt
>
> If you turn that into a patch to, say, t/t4015-diff-whitespace.sh (adding
> a test_expect_failure for a known bug), it is much easier to convince
> developers to work on the issue.
>

Thank you.  In progress.

I am curious if t4015 is planned to be to be rewritten to follow what 
Junio had outlined and Giuseppe implemented for 
t4107-apply-ignore-whitespace.sh to make the spaces more obvious to the reader:

One other question on the making of the test in regards to the 
following quote from:
http://article.gmane.org/gmane.comp.version-control.git/124765
where Junio C Hamano wrote:

>>>>	sed -e 's/Z/ /g' >patch3.patch <<\EOF
>>>>        ...
>>>>        +Z 	print_int(func(i));Z
>>>>        EOF
>>>>
>>>> to make invisible SP stand out more for the benefit of people reading 
>>>> the test script (I know you did not have leading SP before HT in 
>>>> yours, but the above illustrates the visibility issues).  For other 
>>>> tests with test vector patches, visibility of whitespace is not much 
>>>> an issue, but this script is _all about_ whitespace, so anything that 
>>>> clarifies what is going on better would help.

The test being implemented was t4107-apply-ignore-whitespace.sh.

Are there any plans to have t4015-diff-whitespace.sh tests rewritten in 
the same fashion?

> First, your coding style is different from the surrounding code.  I think
> it goes without saying that this should be fixed.
>
> Second, you do not need the parentheses at all (and therefore they should
> go).
>
> Third, libxdiff does not assume to be fed NUL delimited strings.
>

You're absolutely right, I'll be more aware in the future.

> Fourth, that condition "ptr + 1 < top" is already doing what you tried to
> accomplish here.
>
> So I guess that you need to do add "ptr + 1 < top" checks
> instead.
>

I'll give it another go.

Thank you for the advice Dscho.

-- 
Thell

^ permalink raw reply

* Re: [Feature Request] git export
From: Tim Visher @ 2009-08-10 18:31 UTC (permalink / raw)
  To: thomas; +Cc: git
In-Reply-To: <200908101841.00522.thomas@koch.ro>

On Mon, Aug 10, 2009 at 12:41 PM, Thomas Koch<thomas@koch.ro> wrote:
> thanks for your explanation. Still I don't get how to do it with git checkout-
> index. The man page says "Just read the desired tree into the index", but I
> don't know what is meant by this.

No problem.  That phrase gave me some trouble the first time I read it
too.  All it means is 'do a `git checkout` of the branch that you want
to export'.  `git checkout-index` will then do an export of the
contents the branch that you currently have checked out.

Junio and Dscho's comments are possibly salient as well.

-- 

In Christ,

Timmy V.

http://burningones.com/
http://five.sentenc.es/ - Spend less time on e-mail

^ permalink raw reply

* Re: [PATCH] fix potential infinite loop given large unsigned integer
From: Tony Finch @ 2009-08-10 18:19 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Ryan Flynn, git
In-Reply-To: <7v3a81a13z.fsf@alter.siamese.dyndns.org>

On Sun, 9 Aug 2009, Junio C Hamano wrote:

> +	if (numbered) {
> +		static char num_buf[64];
>  		rev.total = total + start_number - 1;
> +		sprintf(num_buf, "%d", rev.total);
> +		rev.num_width = strlen(num_buf);
> +	}

why not

	if (numbered) {
		rev.total = total + start_number - 1;
		rev.num_width = snprintf(NULL, 0, "%d", rev.total);
	}

?

Tony.
-- 
f.anthony.n.finch  <dot@dotat.at>  http://dotat.at/
GERMAN BIGHT HUMBER: SOUTHWEST 5 TO 7. MODERATE OR ROUGH. SQUALLY SHOWERS.
MODERATE OR GOOD.

^ permalink raw reply

* Re: [EGIT PATCH] Provide a more JavaBeans-style 'getName' accessor for the id Signed-off-by: Alex Blewitt <alex.blewitt@gmail.com>
From: Alex Blewitt @ 2009-08-10 18:19 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: robin.rosenberg@dewire.com, git@vger.kernel.org
In-Reply-To: <20090810155207.GW1033@spearce.org>

That patch was originally mailed on the 11th May. Has it taken until  
now to notice the problem, or was the other method added in the last  
month or so? If I'm to blame, I apologise but didn't note any compile  
time issues at the time.

Alex

Sent from my (new) iPhone

On 10 Aug 2009, at 16:52, "Shawn O. Pearce" <spearce@spearce.org> wrote:

> Alex Blewitt <alex.blewitt@gmail.com> wrote:
>> ---
>> .../src/org/spearce/jgit/lib/AnyObjectId.java      |    6 ++++++
>> 1 files changed, 6 insertions(+), 0 deletions(-)
>>
>> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ 
>> AnyObjectId.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ 
>> AnyObjectId.java
>> index acb3cb5..015042a 100644
>> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/AnyObjectId.java
>> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/AnyObjectId.java
>> @@ -448,6 +448,12 @@ public final String name() {
>>    }
>>
>>    /**
>> +     * @return string form of the SHA-1, in lower case hexadecimal.
>> +     */
>> +    public final String getName() {
>> +        return name();
>> +    }
>
> Obviously you didn't compile test this in JGit:
>
> ./org/spearce/jgit/revwalk/RevTag.java:206: getName() in
> org.spearce.jgit.revwalk.RevTag cannot override getName() in
> org.spearce.jgit.lib.AnyObjectId; overridden method is final
>
> :-(
>
> -- 
> Shawn.

^ permalink raw reply

* Re: [EGIT PATCH] Provide a more JavaBeans-style 'getName' accessor for the id Signed-off-by: Alex Blewitt <alex.blewitt@gmail.com>
From: Alex Blewitt @ 2009-08-10 18:11 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: robin.rosenberg@dewire.com, git@vger.kernel.org
In-Reply-To: <20090810155207.GW1033@spearce.org>

That was ages ago that I submitted that patch. I'm pretty sure that it  
compiled at the time for the version of JGit that I had back then.

Sent from my (new) iPhone

On 10 Aug 2009, at 16:52, "Shawn O. Pearce" <spearce@spearce.org> wrote:

> Alex Blewitt <alex.blewitt@gmail.com> wrote:
>> ---
>> .../src/org/spearce/jgit/lib/AnyObjectId.java      |    6 ++++++
>> 1 files changed, 6 insertions(+), 0 deletions(-)
>>
>> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ 
>> AnyObjectId.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ 
>> AnyObjectId.java
>> index acb3cb5..015042a 100644
>> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/AnyObjectId.java
>> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/AnyObjectId.java
>> @@ -448,6 +448,12 @@ public final String name() {
>>    }
>>
>>    /**
>> +     * @return string form of the SHA-1, in lower case hexadecimal.
>> +     */
>> +    public final String getName() {
>> +        return name();
>> +    }
>
> Obviously you didn't compile test this in JGit:
>
> ./org/spearce/jgit/revwalk/RevTag.java:206: getName() in
> org.spearce.jgit.revwalk.RevTag cannot override getName() in
> org.spearce.jgit.lib.AnyObjectId; overridden method is final
>
> :-(
>
> -- 
> Shawn.

^ permalink raw reply

* Re: [PATCH] Limited git-gui to displaying 5000 new files
From: Dan Zwell @ 2009-08-10 15:15 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Alex Riesen, Git Mailing List
In-Reply-To: <20090810170842.GX1033@spearce.org>

Shawn O. Pearce wrote:
> Alex Riesen <raa.lkml@gmail.com> wrote:
>> On Mon, Aug 10, 2009 at 17:38, Shawn O. Pearce<spearce@spearce.org> wrote:
>>>> + ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? set warning "Displaying only $display_limit of "
>>>> + ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? append warning "[llength $to_display] files."
>>>> + ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? info_popup [mc $warning]
>>> This needs to be in the translated strings.
>> Will do, as soon as this hits git://repo.or.cz/git-gui.git.
>> Or should I have looked at the internationalization repo
>> git://repo.or.cz/git-gui/git-gui-i18n.git?
> 
> No, I meant that the patch needs to use [mc] or whatever it is
> to enable the string to be localized.  After its marked with the
> necessary code change, I can apply the patch, regenerate the .pot,
> and let translators update their .po when they have time.
> 

I will change it to use [mc] (and make the other changes that Shawn 
mentioned) and resubmit the patch.

-Dan

^ permalink raw reply

* Re: [PATCH] Limited git-gui to displaying 5000 new files
From: Shawn O. Pearce @ 2009-08-10 17:08 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Dan Zwell, Git Mailing List
In-Reply-To: <81b0412b0908101006n66672cabi865a980d978c2865@mail.gmail.com>

Alex Riesen <raa.lkml@gmail.com> wrote:
> On Mon, Aug 10, 2009 at 17:38, Shawn O. Pearce<spearce@spearce.org> wrote:
> >> + ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? set warning "Displaying only $display_limit of "
> >> + ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? append warning "[llength $to_display] files."
> >> + ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? info_popup [mc $warning]
> >
> > This needs to be in the translated strings.
> 
> Will do, as soon as this hits git://repo.or.cz/git-gui.git.
> Or should I have looked at the internationalization repo
> git://repo.or.cz/git-gui/git-gui-i18n.git?

No, I meant that the patch needs to use [mc] or whatever it is
to enable the string to be localized.  After its marked with the
necessary code change, I can apply the patch, regenerate the .pot,
and let translators update their .po when they have time.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] Limited git-gui to displaying 5000 new files
From: Alex Riesen @ 2009-08-10 17:06 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Dan Zwell, Git Mailing List
In-Reply-To: <20090810153859.GT1033@spearce.org>

On Mon, Aug 10, 2009 at 17:38, Shawn O. Pearce<spearce@spearce.org> wrote:
>> +                     set warning "Displaying only $display_limit of "
>> +                     append warning "[llength $to_display] files."
>> +                     info_popup [mc $warning]
>
> This needs to be in the translated strings.
>

Will do, as soon as this hits git://repo.or.cz/git-gui.git.
Or should I have looked at the internationalization repo
git://repo.or.cz/git-gui/git-gui-i18n.git?

^ permalink raw reply

* Re: [Feature Request] git export
From: Johannes Schindelin @ 2009-08-10 16:56 UTC (permalink / raw)
  To: Thomas Koch; +Cc: git
In-Reply-To: <200908101822.59940.thomas@koch.ro>

Hi,

On Mon, 10 Aug 2009, Thomas Koch wrote:

> could you please provide a simple solution to save a tree object into an 
> arbitrary location?
>
> I found some hints, that it would be possible by missusing either git 
> checkout-index or git-archive, but I think that it shouldn't require 
> that much GIT FU to do such a simple thing.

You can check out Documentation/install-doc-quick.sh and find that 
something like

	GIT_INDEX_FILE=.git/tmp
	export GIT_INDEX_FILE
	git read-tree $TREE
	git checkout-index -a -f --prefix=/the/path
	unset GIT_INDEX_FILE

should work.

Hth,
Dscho

^ permalink raw reply

* Re: [Feature Request] git export
From: Junio C Hamano @ 2009-08-10 16:54 UTC (permalink / raw)
  To: thomas; +Cc: git
In-Reply-To: <200908101822.59940.thomas@koch.ro>

Thomas Koch <thomas@koch.ro> writes:

> could you please provide a simple solution to save a tree object into an 
> arbitrary location?
> I found some hints, that it would be possible by missusing either git 
> checkout-index or git-archive, but I think that it shouldn't require that much 

If you are talking about a tree object, then git-archive is what you want.
It was invented almost for this exact purpose, but unlike "export" that
would only copy to a directory, you can get a tarball out of it when you
do not want to expand the result into a directory; it is more versatile
form.  The manual page for the command has an example (look for
"/var/tmp") so I won't repeat the cut-and-paste recipe here.

If you are not talking about a tree object but is talking about the
contents recorded in your index, checkout-index with the --prefix option
may be what you want.  The manual page for the command has an example
(look for "--prefix=git-export-dir/") so I won't repeat it here.

^ permalink raw reply

* Re: [PATCH] fix potential infinite loop given large unsigned integer
From: Johannes Schindelin @ 2009-08-10 16:53 UTC (permalink / raw)
  To: Ryan Flynn; +Cc: Erik Faye-Lund, Christian Couder, Junio C Hamano, git
In-Reply-To: <a3f15ee60908100917k2a2d6de7ndd5e4ddd1d926cc3@mail.gmail.com>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 845 bytes --]

Hi,

[please cull the quoted text to what you are actually replying to.  
Thanks]

On Mon, 10 Aug 2009, Ryan Flynn wrote:

> On Mon, Aug 10, 2009 at 8:24 AM, Johannes
> Schindelin<Johannes.Schindelin@gmx.de> wrote:
> >
> > static unsigned int digits_of_number(unsigned int number) {
> >     unsigned int result;
> >     for (result = 1; number; number /= 10, result++)
> >         ; /* do nothing */
> >     return result;
> > }
> 
> whoops, actually yours: digits_of_number(1) -> 2

static unsigned int digits(unsigned int number)
{
        unsigned int result;
        for (result = 1; (number /= 10); result++)
                ; /* do nothing */
        return result;
}

I'm sorry, I forgot the "something like this" in my mail.

This version is actually tested.

It has non-optimal runtime, but then, it does not really matter.

Ciao,
Dscho

^ permalink raw reply

* Re: [RFC PATCH v2 4/4] read-tree: add --no-sparse to turn off sparse hook
From: Johannes Schindelin @ 2009-08-10 16:46 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1249917562-5931-5-git-send-email-pclouds@gmail.com>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1238 bytes --]

Hi,

On Mon, 10 Aug 2009, Nguyễn Thái Ngọc Duy wrote:

> diff --git a/unpack-trees.c b/unpack-trees.c
> index f407bf5..d087112 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -530,8 +530,10 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
>  	if (o->trivial_merges_only && o->nontrivial_merge)
>  		return unpack_failed(o, "Merge requires file-level merging");
>  
> -	if (run_sparse_hook(o))
> -		return unpack_failed(o, NULL);
> +	if (!o->no_sparse_hook) {
> +		if (run_sparse_hook(o))
> +			return unpack_failed(o, NULL);
> +	}
>  

IMHO this would read nicelier as

	if (!o->no_sparse_hook && run_sparse_hook(o))
		return unpack_failed(o, NULL);

> diff --git a/unpack-trees.h b/unpack-trees.h
> index ad21823..81eb2ef 100644
> --- a/unpack-trees.h
> +++ b/unpack-trees.h
> @@ -30,6 +30,7 @@ struct unpack_trees_options {
>  		     skip_unmerged,
>  		     initial_checkout,
>  		     diff_index_cached,
> +		     no_sparse_hook,
>  		     gently;

Hmm.  I understand that the assumption is that memset(&opts, 0, 
sizeof(opts)); should give you a sensible default, but I cannot avoid 
noticing that "no_sparse_hook = 0" is a double negation, something to be 
avoided...

Thanks,
Dscho

^ permalink raw reply

* Re: [RFC PATCH v2 3/4] unpack_trees(): add support for sparse checkout
From: Johannes Schindelin @ 2009-08-10 16:41 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1249917562-5931-4-git-send-email-pclouds@gmail.com>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1057 bytes --]

Hi,

On Mon, 10 Aug 2009, Nguyễn Thái Ngọc Duy wrote:

> This patch teaches unpack_trees() to checkout/remove entries on working 
> directories appropriately when sparse checkout area is changed. Hook 
> "sparse" is needed to help determine which entry will be checked out, 
> which will not be.
> 
> When the hook is run, it is prepared with a pseudo index. The hook then 
> can use "git update-index --[no-]assume-unchanged" to manipulate the 
> index. It should not do anything else on the index. Assume unchanged 
> information from the index will be used to shape working directory.

If I understand correctly, the purpose of the hook is to allow the user to 
mark something as unwanted preemptively, right?

If that is the sole reason for the hook, would it not be better to add 
support for a file .git/info/sparse which has the same syntax as 
.git/info/exclude, and which is used to determine if an 
added/modified/deleted file is supposed to be in the "sparse" area or not?

Something like

	*
	!/Documentation/

comes to mind.

Thanks,
Dscho

^ permalink raw reply

* Re: [Feature Request] git export
From: Thomas Koch @ 2009-08-10 16:41 UTC (permalink / raw)
  To: Tim Visher; +Cc: git
In-Reply-To: <c115fd3c0908100927v4d9e4a87we4502e91d3789138@mail.gmail.com>

Hi Tim,

thanks for your explanation. Still I don't get how to do it with git checkout-
index. The man page says "Just read the desired tree into the index", but I 
don't know what is meant by this.

> Hey Thomas,
>
> On Mon, Aug 10, 2009 at 12:22 PM, Thomas Koch<thomas@koch.ro> wrote:
> > could you please provide a simple solution to save a tree object into an
> > arbitrary location?
>
> This has been requested a few times and I have no idea what the
> development status of it is.  Most people seem to recommend just
> making an alias around git checkout-index.
>
> > I found some hints, that it would be possible by missusing either git
> > checkout-index or git-archive, but I think that it shouldn't require that
> > much GIT FU to do such a simple thing.
>
> It's not really a misuse, as the [man page
> itself](http://www.kernel.org/pub/software/scm/git/docs/git-checkout-index.
>html) recommends using checkout-index as a way to export an entire tree.
> Until someone decides that it's worth making the porcelain command (like
> you perhaps) then I think you can just stick with checkout-index knowing
> you're not abusing anything.
Thomas Koch, http://www.koch.ro

^ permalink raw reply

* Re: [RFC PATCH v2 2/4] gitignore: read from index if .gitignore is assume-unchanged
From: Johannes Schindelin @ 2009-08-10 16:33 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1249917562-5931-3-git-send-email-pclouds@gmail.com>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2887 bytes --]

Hi,

On Mon, 10 Aug 2009, Nguyễn Thái Ngọc Duy wrote:

> diff --git a/dir.c b/dir.c
> index e05b850..e55344f 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -200,11 +200,36 @@ void add_exclude(const char *string, const char *base,
>  	which->excludes[which->nr++] = x;
>  }
>  
> +static void *read_index_data(const char *path, size_t *size)

How about calling it "read_assume_unchanged_from_index()" instead?  I 
suggest this because it does not read the index from the data if the path 
is not marked assume unchanged...

> @@ -212,27 +237,31 @@ static int add_excludes_from_file_1(const char *fname,
>
> [...]
>  
>  	if (buf_p)
>  		*buf_p = buf;
> -	buf[size++] = '\n';
>  	entry = buf;
> -	for (i = 0; i < size; i++) {
> -		if (buf[i] == '\n') {
> +	for (i = 0; i <= size; i++) {
> +		if (i == size || buf[i] == '\n') {
>  			if (entry != buf + i && entry[0] != '#') {
>  				buf[i - (i && buf[i-1] == '\r')] = 0;
>  				add_exclude(entry, base, baselen, which);

Should this change not rather be a separate one?

> @@ -241,17 +270,12 @@ static int add_excludes_from_file_1(const char *fname,
>  		}
>  	}
>  	return 0;
> -
> - err:
> -	if (0 <= fd)
> -		close(fd);
> -	return -1;
>  }
>  
>  void add_excludes_from_file(struct dir_struct *dir, const char *fname)
>  {
>  	if (add_excludes_from_file_1(fname, "", 0, NULL,
> -				     &dir->exclude_list[EXC_FILE]) < 0)
> +				     &dir->exclude_list[EXC_FILE], 0) < 0)

Could you mention in the commit message why this function does not want to 
check the index (I _guess_ it is because this code path only tries to read 
.git/info/exclude, but it would be better to be sure).

>  		die("cannot use %s as an exclude file", fname);
>  }
>  
> @@ -302,7 +326,7 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)
>  		strcpy(dir->basebuf + stk->baselen, dir->exclude_per_dir);
>  		add_excludes_from_file_1(dir->basebuf,
>  					 dir->basebuf, stk->baselen,
> -					 &stk->filebuf, el);
> +					 &stk->filebuf, el, 1);
>  		dir->exclude_stack = stk;
>  		current = stk->baselen;
>  	}
> diff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh
> index c65bca8..88b69bc 100755
> --- a/t/t3001-ls-files-others-exclude.sh
> +++ b/t/t3001-ls-files-others-exclude.sh
> @@ -85,6 +85,26 @@ test_expect_success \
>         >output &&
>       test_cmp expect output'
>  
> +test_expect_success 'setup sparse gitignore' '
> +	git add .gitignore one/.gitignore one/two/.gitignore &&
> +	git update-index --assume-unchanged .gitignore one/.gitignore one/two/.gitignore &&
> +	rm .gitignore one/.gitignore one/two/.gitignore
> +'

You're probably less sloppy than me; I'd have defined a variable like 
this:

	ignores=".gitignore one/.gitignore one/two/.gitignore"

and used it for the three calls, just to make sure that I do not fsck 
anything up there due to fat fingers.

Thanks,
Dscho

^ 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