Is there any efficient way to track history of a piece of code?

2014-05-07 Thread Jianyu Zhan
Usually, a trivial change(like coding style fix) may bury a
original change of the code, and thus git blame is of less
help. And to address this situation, I have to do like this:

   git blame -s REF^  > temp

to dig into the history recursively by hand, to find out
the original change.

Here, REF is commit-id that git blame reports.

git log -L is a good alternative option, but sometimes it seems
too cubersome, as I care only one line of code.

Is there any current solution or suggestion?

Thanks,
Jianyu Zhan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] ignorecase: Fix git mv on insensitive filesystems

2014-05-07 Thread Johannes Sixt
Am 5/7/2014 19:46, schrieb Junio C Hamano:
> David Turner  writes:
> 
>> On Wed, 2014-05-07 at 08:17 +0200, Johannes Sixt wrote:
} else if (cache_name_pos(src, length) < 0)
bad = _("not under version control");
 -  else if (lstat(dst, &st) == 0) {
 +  else if (lstat(dst, &dst_st) == 0 &&
 +   (src_st.st_ino != dst_st.st_ino ||
 +(src_st.st_ino == 0 && strcasecmp(src, dst {
>>>
>>> Don't do that. st_ino is zero on Windows only because we do not spend time
>>> to fill in the field. Don't use it as an indicator for a case-insensitive
>>> file system; zero may be a valid inode number on other systems.
>>
>> I don't think it is a problem if zero is a valid inode.  The only thing
>> that happens when there is a zero inode, is that we have to compare
>> filenames.  The inode check is just an optimization to avoid doing a
>> bunch of strcasecmp on systems that don't have to.
> 
> Am I correct to rephrase you that the code assumes that any
> filesystem that cannot give unique inum to different files will use
> 0 as the placeholder inum, so if src/dst share the same non-zero
> inum, it is guaranteed that is not a placeholder and we know they
> are different files without comparing the two paths?

"If src and dst share the same non-zero inum, it is guaranteed that it is
not a placeholder and we know they are the same file" is more correct.

What if both inums are zero? Can this happen on any sane POSIX system? I
don't know, but my gut feeling is that inode zero is too special to be
allocated for files or directories.

In that case, it is safe to assume that the st_ino field is just a
placeholder when it is zero, and we have to compare the file name. Then we
can either assume that this happens only for our emulation layer on MinGW
(and the comparison can be case-insensitive) or choose the comparison mode
based on core.ignorecase. This patch does the former, but I think we
should do the latter.

-- Hannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: t5539 fails on ubuntu for v2.0.0-rc2

2014-05-07 Thread Fabio D'Alfonso

Hi,
this is the error in httpd error.log

 [Wed May 07 20:44:10 2014] [alert] getpwuid: couldn't determine user 
name from  uid 4294967295, you probably need to modify the User directive
 [Wed May 07 20:44:10 2014] [notice] Apache/2.2.17 (Ubuntu) configured 
--resuming normal operations
 [Wed May 07 20:44:10 2014] [alert] getpwuid: couldn't determine user 
name from  uid 4294967295, you probably need to modify the User directive
 [Wed May 07 20:44:10 2014] [alert] getpwuid: couldn't determine user 
name from  uid 4294967295, you probably need to modify the User directive
 [Wed May 07 20:44:10 2014] [alert] getpwuid: couldn't determine user 
name from  uid 4294967295, you probably need to modify the User directive
 [Wed May 07 20:44:10 2014] [alert] getpwuid: couldn't determine user 
name from  uid 4294967295, you probably need to modify the User directive
 [Wed May 07 20:44:11 2014] [alert] Child 12037 returned a Fatal 
error...Apache is exiting!


The 1.9.x versions compiled and tested here, before. It is a natty 
dektop 11.04.


Thanks

Fabio D'Alfonso
'Enabling Business Through IT'
cell.  +39.348.059.40.22 ***
web: www.fabiodalfonso.com 
email: fabio.dalfo...@fabiodalfonso.com
linkedin: 
www.linkedin.com/in/fabiodalfonso 
twitter: www.twitter.com/#!/fabio_dalfonso 



fax: +39.06.874.599.581
BlackBerry® Wireless Enabled Address.


 ** Hidden  numbers are automatically rejected by the phone*

On 5/8/2014 6:14 AM, Jeff King wrote:

On Wed, May 07, 2014 at 02:45:01PM -0700, Junio C Hamano wrote:


Fabio D'Alfonso  writes:


root@HDUBVM01:~/builds/git/t# ./t5539-fetch-http-shallow.sh
ok 1 - setup shallow clone
not ok 2 - clone http repository

[...]
Does not reproduce for me but I am on Ubuntu 12.04.2 LTS, so...

Nor me on Debian unstable.


Running it with the -v option might give you more hints, and
running it as

 $ sh -x ./t5539-fe* -v

might give you more to chew.

Yes, definitely that, but also, from:


httpd (pid 10653?) not running
# failed 2 among 3 test(s)

...it looks like the httpd server did not start. Look in httpd/error.log
of "trash directory.t5539-fetch-http-shallow" to see the apache log. It
may give some reason for failing to start.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html



--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


How to keep a project's canonical history correct.

2014-05-07 Thread Stephen P. Smith
During the mail thread about "Pull is mostly evil" a user asked how
the first parent could become reversed.

This howto explains how the first parent can get reversed when viewed
by the project and then explains a method to keep the history correct.

Signed-off-by: Stephen P. Smith 
---
 Documentation/Makefile |   1 +
 .../howto/keep-canonical-history-correct.txt   | 207 +
 2 files changed, 208 insertions(+)
 create mode 100644 Documentation/howto/keep-canonical-history-correct.txt

diff --git a/Documentation/Makefile b/Documentation/Makefile
index fc6b2cf..cea0e7a 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -59,6 +59,7 @@ SP_ARTICLES += howto/recover-corrupted-blob-object
 SP_ARTICLES += howto/recover-corrupted-object-harder
 SP_ARTICLES += howto/rebuild-from-update-hook
 SP_ARTICLES += howto/rebase-from-internal-branch
+SP_ARTICLES += howto/keep-canonical-history-correct
 SP_ARTICLES += howto/maintain-git
 API_DOCS = $(patsubst %.txt,%,$(filter-out technical/api-index-skel.txt 
technical/api-index.txt, $(wildcard technical/api-*.txt)))
 SP_ARTICLES += $(API_DOCS)
diff --git a/Documentation/howto/keep-canonical-history-correct.txt 
b/Documentation/howto/keep-canonical-history-correct.txt
new file mode 100644
index 000..dd310ea
--- /dev/null
+++ b/Documentation/howto/keep-canonical-history-correct.txt
@@ -0,0 +1,207 @@
+From: Junio C Hamano 
+Date: Wed, 07 May 2014 13:15:39 -0700
+Subject: Beginner question on "Pull is mostly evil"
+Abstract: This how-to explains a method for keeping a project's history 
correct when using git pull.
+Content-type: text/asciidoc
+
+Keep authoritative canonical history correct with git pull
+==
+
+Suppose that that central repository has this history:
+
+
+---o---o---A
+
+
+which ends at commit A (time flows from left to right and each node in
+the graph is a commit, lines between them indicating parent-child
+relationship).
+
+Then you clone it and work on your own commits, which leads you to
+have this in *your* repository:
+
+
+---o---o---A---B---C
+
+
+Imagine your coworker did the same and built on top of A in *his*
+repository this history in the meantime, and then pushed it to the
+central repository:
+
+
+---o---o---A---X---Y---Z
+
+
+Now, if you "git push" at this point, beause your history that leads
+to C lack X, Y and Z, it will fail.  You need to somehow make the
+tip of your history a descendant of Z.
+
+One suggested way to solve the problem is "fetch and then merge".
+If you fetch, your repository will have a history like this:
+
+
+---o---o---A---B---C
+\
+ X---Y---Z
+
+
+And then if you did merge after that, while still on *your* branch,
+i.e. C, you will create a merge M and make the history look like
+this:
+
+
+---o---o---A---B---C---M
+\ /
+ X---Y---Z
+
+
+M is a descendant of Z, so you can push to update the central
+repository.  Such a merge M does not lose any commit in both
+histories, so in that sense it may not be wrong, but when people
+would want to talk about "the authoritative canonical history that
+is shared among the project participants", i.e. "the trunk", the way
+they often use is to do:
+
+
+$ git log --first-parent
+
+
+For all other people who observed the central repository after your
+coworker pushed Z but before you pushed M, the commit on the trunk
+used to be "o-o-A-X-Y-Z".  But because you made M while you were on
+C, M's first parent is C, so by pushing M to advance the central
+repository, you made X-Y-Z a side branch, not on the trunk.
+
+You would rather want to have a history of this shape:
+
+
+---o---o---A---X---Y---Z---M'
+\ /
+ B---C
+
+
+so that in the first-parent chain, it is clear that the project
+first did X and then Y and then Z and merged a change that consists
+of two commits B and C that achieves a single goal.  You may have
+worked on fixing the bug #12345 with these two patches, and the
+merge M' with swapped parents can say in its log message "Merge
+'fix-bug-12345'".
+
+Note that I said "achieves a single goal" above, because this is
+important.  "swapping the merge order" only covers a special case
+where the project does not care too much about having unrelated
+things done on a single merge but cares a lot about first-parent
+chain.
+
+There are multiple schools of thought about the "trunk" management.
+
+ 1. Some projects want to keep a completely linear history without
+any merges.  Obviously, swapping the merge order would not help
+their taste.  You would need to flatten your history on top of
+the updated upstream to result in a history of

Re: [BUG?] Patches created with 'diff.noprefix=true' don't 'git apply'.

2014-05-07 Thread Nathan Collins
On Wed, May 7, 2014 at 9:38 PM, Nathan Collins  wrote:
> On Wed, May 7, 2014 at 4:39 PM, Nathan Collins  
> wrote:
>> On Wed, May 7, 2014 at 11:42 AM, Junio C Hamano  wrote:
>>> Nathan Collins  writes:
>>
 For (2), the solution may be to add a separate
 'diff.add-clickable-paths' option (probably there is a better name?
 'diff.add-copyable-paths'? ...),...
 ...
 Concretely, if 'diff.add-clickable-paths' is set, then instead of e.g.

   diff --git a/src/Data/Function/Decorator/Memoizer/Unsafe.hs
 b/src/Data/Function/Decorator/Memoizer
   index 3ef17da..a0586d3 100644
   --- a/src/Data/Function/Decorator/Memoizer/Unsafe.hs
   +++ b/src/Data/Function/Decorator/Memoizer/Unsafe.hs
>>>
>>> If you do something along that line, perhaps
>>>
>>> Index: src/Data/Function/Decorator/Memoizer/Unsafe.hs
>>> diff --git a/src/Data/Function/Decorator/Memoizer/Unsafe.hs ...
>>> index 3ef17da..a0586d3 100644
>>> --- a/src/Data/Function/Decorator/Memoizer/Unsafe.hs
>>> +++ b/src/Data/Function/Decorator/Memoizer/Unsafe.hs
>>>
>>> to imitate what "cvs diff" does may be more familar to people.
>>>
>>> What would you propose to make clickable in a renaming diff, though?
>>
>> Your 'Index' header looks good, and I would expect a renaming diff to
>> have something like
>>
>>   Index: foo -> bar
>>
>> as in 'git status', but I just realized that a "clickable paths"
>> option already exists in some sense! There is a '--patch-with-raw'
>> option (which is "short" for '--patch' and '--raw', hahaha) which
>> inserts clickable file names in the patch, above each diff.
>
> Or not: I stupidly only tested this with a single file modified: it
> turns out that all the clickable file names appear at the top of the
> patch, not as one file name above each corresponding diff as I
> claimed.

The following may be a non-option, since presumably many tools depend
on the current Git patch format.

The paths in the "extended header lines" in Git patches are clickable
by default, and respect the '--relative' option. So, adding a path to
the extended header lines that don't already have one would solve the
"clickable paths" problem.

E.g.

  index .. 

becomes

  index ..  

The 'man git-diff' description of extended header lines in the
"Generating Patches with -p" section:

  2. It is followed by one or more extended header lines:

 old mode 
 new mode 
 deleted file mode 
 new file mode 
 copy from 
 copy to 
 rename from 
 rename to 
 similarity index 
 dissimilarity index 
 index .. 

 File modes are printed as 6-digit octal numbers including the
file type and file
 permission bits.

 Path names in extended headers do not include the a/ and b/ prefixes.

Cheers,

-nathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [BUG?] Patches created with 'diff.noprefix=true' don't 'git apply'.

2014-05-07 Thread Nathan Collins
On Wed, May 7, 2014 at 4:39 PM, Nathan Collins  wrote:
> On Wed, May 7, 2014 at 11:42 AM, Junio C Hamano  wrote:
>> Nathan Collins  writes:
>
>>> For (2), the solution may be to add a separate
>>> 'diff.add-clickable-paths' option (probably there is a better name?
>>> 'diff.add-copyable-paths'? ...),...
>>> ...
>>> Concretely, if 'diff.add-clickable-paths' is set, then instead of e.g.
>>>
>>>   diff --git a/src/Data/Function/Decorator/Memoizer/Unsafe.hs
>>> b/src/Data/Function/Decorator/Memoizer
>>>   index 3ef17da..a0586d3 100644
>>>   --- a/src/Data/Function/Decorator/Memoizer/Unsafe.hs
>>>   +++ b/src/Data/Function/Decorator/Memoizer/Unsafe.hs
>>
>> If you do something along that line, perhaps
>>
>> Index: src/Data/Function/Decorator/Memoizer/Unsafe.hs
>> diff --git a/src/Data/Function/Decorator/Memoizer/Unsafe.hs ...
>> index 3ef17da..a0586d3 100644
>> --- a/src/Data/Function/Decorator/Memoizer/Unsafe.hs
>> +++ b/src/Data/Function/Decorator/Memoizer/Unsafe.hs
>>
>> to imitate what "cvs diff" does may be more familar to people.
>>
>> What would you propose to make clickable in a renaming diff, though?
>
> Your 'Index' header looks good, and I would expect a renaming diff to
> have something like
>
>   Index: foo -> bar
>
> as in 'git status', but I just realized that a "clickable paths"
> option already exists in some sense! There is a '--patch-with-raw'
> option (which is "short" for '--patch' and '--raw', hahaha) which
> inserts clickable file names in the patch, above each diff.

Or not: I stupidly only tested this with a single file modified: it
turns out that all the clickable file names appear at the top of the
patch, not as one file name above each corresponding diff as I
claimed.

D'oh,

-nathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: t5539 fails on ubuntu for v2.0.0-rc2

2014-05-07 Thread Jeff King
On Wed, May 07, 2014 at 02:45:01PM -0700, Junio C Hamano wrote:

> Fabio D'Alfonso  writes:
> 
> > root@HDUBVM01:~/builds/git/t# ./t5539-fetch-http-shallow.sh
> > ok 1 - setup shallow clone
> > not ok 2 - clone http repository
> [...]

> Does not reproduce for me but I am on Ubuntu 12.04.2 LTS, so...

Nor me on Debian unstable.

> Running it with the -v option might give you more hints, and
> running it as
> 
> $ sh -x ./t5539-fe* -v
> 
> might give you more to chew.

Yes, definitely that, but also, from:

> > httpd (pid 10653?) not running
> > # failed 2 among 3 test(s)

...it looks like the httpd server did not start. Look in httpd/error.log
of "trash directory.t5539-fetch-http-shallow" to see the apache log. It
may give some reason for failing to start.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] shell doc: remove stray "+" in example

2014-05-07 Thread Jeff King
On Wed, May 07, 2014 at 04:44:01PM -0700, Jonathan Nieder wrote:

> The git-shell(1) manpage says
> 
>   EXAMPLE
>  To disable interactive logins, displaying a greeting
>   instead:
> 
>   +
> 
>  $ chsh -s /usr/bin/git-shell
>  $ mkdir $HOME/git-shell-commands
> [...]
> 
> The stray "+" has been there ever since the example was added in
> v1.8.3-rc0~210^2 (shell: new no-interactive-login command to print a
> custom message, 2013-03-09).  The "+" sign between paragraphs is
> needed in asciidoc to attach extra paragraphs to a list item but here
> it is not needed and ends up rendered as a literal "+".  Remove it.
> 
> A quick search with "grep -e '+' /usr/share/doc/git/html/*.html"
> doesn't find any other instances of this problem.
> 
> Signed-off-by: Jonathan Nieder 

Looks good to me. I suspect it was copied from another spot where the
examples _were_ in a list (e.g., git-add's EXAMPLES section). Either
way, your fix is the right thing to do.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] remote-hg: more improvements

2014-05-07 Thread Felipe Contreras
Junio C Hamano wrote:
> Junio C Hamano  writes:
> 
> > Felipe Contreras  writes:
> >
> >> Here's a bunch of tests more, and a fixes for Mercurial v3.0.
> >
> > I think the discussion with John Keeping hints that we shouldn't be
> > rushing fc/remote-helpers-hg-bzr-graduation and this does not appear
> > to assume the presense of that series, which is good in order to
> > make these fixes jump over them.
> 
> When merged to a place somewhere early between the next and the pu
> branches (aka "the jch branch", which is the version I usually use
> myself), this seemed to break t5810, so I did today's integration
> cycle one more time by excluding this topic and then instead queuing
> it near the tip of the pu branch (read: today's 'pu' does not pass
> the test suite for me).
> 
> There may be some other changes that this series depends on that I
> may have missed that caused this breakage.  Can you take a look?

I'm such a bad maintainer and I don't take constructive criticism well
why would you expect me to take a look?

As usual, I did more than take a look and I went ahead to manually test
with multiple versions of Mercurial. The problem appears with hg < 2.6.

Here's the fix:

--- a/git-remote-hg.py
+++ b/git-remote-hg.py
@@ -905,7 +905,7 @@ def write_tag(repo, tag, node, msg, author):
 try:
 fctx = tip.filectx(f)
 data = fctx.data()
-except error.ManifestLookupError:
+except error.LookupError:
 data = ""
 content = data + "%s %s\n" % (node, tag)
 return context.memfilectx(f, content, False, False, None)

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] remote-hg: more improvements

2014-05-07 Thread James Denholm
Felipe, I would ask, suggest, beg, implore you to calm down. It's
generally not a good plan to alienate the maintainer of a project,
regardless of the correctness or incorrectness of one's arguments, but I
fear that's only what you will achieve at the moment.

--
Regards,
James Denholm.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Beginner question on "Pull is mostly evil"

2014-05-07 Thread Stephen & Linda Smith
I'll create a patch.

On Wednesday, May 07, 2014 01:51:04 PM Junio C Hamano wrote:
> Jim Garrison  writes:
> 
> >> -Original Message-
> >> From: Junio C Hamano
> >> Sent: Wednesday, May 07, 2014 1:16 PM
> >> Subject: Re: Beginner question on "Pull is mostly evil"
> >> 
> >> No.  This is most often true for people who use a single repository as a
> >> place for everybody to meet, in the same way as SVN.
> > [snip lots of excellent detail]
> >> HTH.
> >
> > Wow.  That helps tremendously, and should be incorporated somewhere in the
> > Git documentation.  Thank you for your immensely detailed response.
> 
> We used to collect useful list postings in Documentation/howto/;
> perhaps somebody wants to do the minimum copyediting of the message
> and send a patch?
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] ignorecase: Fix git mv on insensitive filesystems

2014-05-07 Thread brian m. carlson
On Tue, May 06, 2014 at 03:59:04PM -0700, dtur...@twopensource.com wrote:
> @@ -74,7 +74,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
>   };
>   const char **source, **destination, **dest_path, **submodule_gitfile;
>   enum update_mode { BOTH = 0, WORKING_DIRECTORY, INDEX } *modes;
> - struct stat st;
> + struct stat src_st,dst_st;

Extremely minor nit: we generally put a space after the comma.

-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH 0/4] remote-hg: more improvements

2014-05-07 Thread Felipe Contreras
Junio C Hamano wrote:
> Felipe Contreras  writes:
> 
> > And you are still conveniently avoiding the question:
> >
> > Based on what reasoning?
> 
> Go re-read what was already said in the thread.

I already read it, and I already responded.

> I still think remote-hg and remote-bzr can and will flourish on their
> own merit,

Oh, you *think*. Well, what if you are wrong?

Or is that never a possibility? You are always right. Right?

> Having said that, I've been thinking (not because of this thread,
> but because I like imerge better and better these days) that there
> should be a much better way to have a list of recommended third-party
> plug-ins that enrich the Git ecosystem.

If and when such a mechanism exists, sure, it makes sense to move
functionality like git-p4 and git-remote-hg out of the core and contrib
areas.

But in the meantime what is ready for the core should be in the core.

> > Normally I would explain the details of why this is the case, and send
> > the crash regresion fix for v2.0 with a clear explanation,...
> 
> Without such an explanation in the log message, how would you expect
> anybody to guess correctly?

I don't. I told you it wasn't a mistake. If that's not enough for you,
that's *your* problem.

*If* git-remote-hg was to be part of the core, then sure, I would care
that you didn't understand why the patch is correct, and I would resend
immediately what a clear explanation.

But since it's only part of the contrib area which has such abundant
crap without documentation or tests. I do not care.

> Seriously, if you do not care about my first reaction, why do you
> even want to live in my tree?

As I already explained; I don't care about your reaction *because* you
don't want these tools to live in your tree.

> > The fact that I'm the maintainer and I say it'ss good should be good
> > enough, and if the current version in "master" renders unusable the
> > existing Mercurial clones, hey, it's only in contrib, right?
> 
> One potential merit I would see for keeping them in my tree is that
> your change will see second opinions from others involved in the
> project (including me), without giving a total rein based on the
> sub-maintainership alone.  All the changes from sub-area maintainers
> are vetted by at least two sets of eyeballs that way.
> 
> But after having to deal with you and seeing that you do not take
> constructive criticism well,

Oh, please. Up to the point where you decided unilaterally to move them
out of the core (they are alread in), all the constructive criticism to
git-remote-hg has been addressed properly.

I have spent an absurdely large amount of time working on git-remote-hg,
and the transport-helper to make sure everything works right. I even started
git-remote-bzr just to prove that the Python git_remote_helpers
framework was not needed, and eventually I made it work better than any
of the alternatives. I had to fight tooth-and-nail to prove that the
msysgit guys were wrong and my patch to handle UNINTERESTING refs
properly was right. Not to mention all the tests, the compatibility with
hg-git, and with gittifyhg, just to prove that my approach was superior
than the alternatives.

I addressed every issue reported constructively, every bug report was
fixed, every patch reviewed and usually improved by me. I made sure
users of older versions wouldn't be affected negatively when the marks
file was upgraded, and I even setup automatic tests for different
versions Bazaar and Mercurial that run every time I push to my
repository.

It is *way* beyond the quality of any other tool in 'contrib/' and even
some tools in the core, like 'git-request-pull' (which has known bugs),
and probably even 'git-pt'.

Even you agreed it would be beneficial to move them out of contrib; it
would benefit *everyone*. And there was no reason not to.

And then some random guy comes with a few bad arguments, and you change
your mind.

That's f*cking double standards. Pure and simple.

If git-remote-hg belongs out-of-tree, so does git-svn and git-p4. If
git-remote-hg belongs in the contrib area, so does git-svn, and git-p4.

After all this insane amout of work you are acting as if git-remote-hg
wasn't ready to move to the core, because I didn't explain *one* commit
properly to you (which happened after this bullshit).

If these helpers are not going to move forward why would I care? Give me
why one good reason why I should give a flying f*ck about the state of
remote-helpers in *your* tree after this (and BTW as things stand now,
it's not good).

It was *your* users who urged me to send my patches upstream.

> I doubt such a possibile merit will ever materialize in the area where
> you alone work on.

And there it is. Ad hominem rationale.

> Letting you do whatever you want in your own tree may benefit the
> users of remote-hg/remote-bzr better as the (bitter) second best
> option.

If and when there is a mechanism promoting out-of-tree tools, that
might be the case.

In the meant

Re: [PATCH] contrib/subtree bugfix: Crash if FETCH_HEAD is tag

2014-05-07 Thread James Denholm
After a closer look, it seems the initial patch wasn't correctly sent
to the list. Please disregard, I'm re-sending the patch entirely.

Regards,
James Denholm.

On 8 May 2014 07:53, James Denholm  wrote:
> On 4 May 2014 16:33:32 GMT+10:00, James Denholm  wrote:
>>cmd_add_commit() is passed FETCH_HEAD by cmd_add_repository, which is
>>then rev-parsed into an object ID. However, if the user is fetching a
>>tag rather than a branch HEAD, such as by executing:
>>
>>$ git subtree add -P oldGit https://github.com/git/git.git tags/v1.8.0
>>
>>The object ID is a tag and is never peeled, and the git commit-tree
>>call
>>(line 561) slaps us in the face because it doesn't handle tag IDs.
>>
>>Because peeling a committish ID doesn't do anything if it's already a
>>commit, fix by peeling[1] the object ID before assigning it to $rev, as
>>per the patch.
>>
>>[*1*]: Via peel_committish(), from git:git-sh-setup.sh
>>
>>Reported-by: Kevin Cagle 
>>Diagnosed-by: Junio C Hamano 
>>Signed-off-by: James Denholm 
>>---
>>NB: This bug doesn't surface when using --squash, as $rev is reassigned
>>to the squash commit via new_squash_commit before git commit-tree sees
>>it (though for simplicity, new_squash_commit now also sees the peeled
>>ID).
>>
>>Also doesn't surface when using "git subtree merge", as git merge can
>>handle tag objects.
>>
>>On a side note, if merging a tag without --squash, git merge recognises
>>that it's a tag and adds a note to the merge commit body. It may be
>>worth mimicking this when using "subtree merge --squash" or
>>"subtree add".
>>
>> contrib/subtree/git-subtree.sh | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>>diff --git a/contrib/subtree/git-subtree.sh
>>b/contrib/subtree/git-subtree.sh
>>index dc59a91..9453dae 100755
>>--- a/contrib/subtree/git-subtree.sh
>>+++ b/contrib/subtree/git-subtree.sh
>>@@ -538,7 +538,7 @@ cmd_add_commit()
>> {
>>   revs=$(git rev-parse $default --revs-only "$@") || exit $?
>>   set -- $revs
>>-  rev="$1"
>>+  rev=$(peel_committish "$1")
>>
>>   debug "Adding $dir as '$rev'..."
>>   git read-tree --prefix="$dir" $rev || exit $?
>
> I know that subtree isn't exactly the most popular or exciting part of
> the project at the moment, but given that this is adding a subtree based
> on an annotated tag is a reasonably sensible operation and (to me) the
> fix seems reasonably trivial, could I get some eyes on this?
>
> Regards,
> James Denholm.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] contrib/subtree bugfix: Can't `add` annotated tag

2014-05-07 Thread James Denholm
cmd_add_commit() is passed FETCH_HEAD by cmd_add_repository, which is
then rev-parsed into an object ID. However, if the user is fetching a
tag rather than a branch HEAD, such as by executing:

$ git subtree add -P oldGit https://github.com/git/git.git tags/v1.8.0

The object ID is a tag and is never peeled, and the git commit-tree call
(line 561) slaps us in the face because it doesn't handle tag IDs.

Because peeling a committish ID doesn't do anything if it's already a
commit, fix by peeling[1] the object ID before assigning it to $rev, as
per the patch.

[*1*]: Via peel_committish(), from git:git-sh-setup.sh

Reported-by: Kevin Cagle 
Diagnosed-by: Junio C Hamano 
Signed-off-by: James Denholm 
---
NB: This bug doesn't surface when using --squash, as $rev is reassigned
to the squash commit via new_squash_commit before git commit-tree sees
it (though for simplicity, new_squash_commit now also sees the peeled
ID).

Also doesn't surface when using "git subtree merge", as git merge can
handle tag objects.

On a side note, if merging a tag without --squash, git merge recognises
that it's a tag and adds a note to the merge commit body. It may be
worth mimicking this when using "subtree merge --squash" or
"subtree add".

 contrib/subtree/git-subtree.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index dc59a91..9453dae 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -538,7 +538,7 @@ cmd_add_commit()
 {
revs=$(git rev-parse $default --revs-only "$@") || exit $?
set -- $revs
-   rev="$1"
+   rev=$(peel_committish "$1")

debug "Adding $dir as '$rev'..."
git read-tree --prefix="$dir" $rev || exit $?
-- 
1.9.2

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Apr 2014, #09; Tue, 29)

2014-05-07 Thread Felipe Contreras
Greg Troxel wrote:
> 
> Felipe Contreras  writes:
> 
> > Greg Troxel wrote:
> >> In a packaging system, dependencies are much more troublesome.
> >> Dependencies have to be declared, and the build limited to use only
> >> those declared dependencies, in order to get repeatable builds and
> >> binary packages that can be used on other systems.  Dependencies that
> >> really are required are fine.  But optional dependencies are a
> >> problem, because e.g. one doesn't want to require the presence of qt
> >> to build something (that isn't already enormous).   So if git needs
> >> mercurial and subversion installed, plus perhaps 5 other things for
> >> less popular remote helpers, that starts to be a real burden.
> >
> > It doesn't *need* them to build. The Mercurial/Bazaar dependencies are
> > optional, both at build-time and at run-time. Most distributions would
> > want to test the functionality they are distributing, and for testing
> > they do need these dependencies.
> 
> My point is that a packaging of git needs to either decide to forego
> these optional parts, or to include them, in the default case.

That is currently the case. They would be included by default, but not
usable unless the *optional* dependencies are installed.

> So I'm merely trying to suggest that it's better to have a core
> package with a restrained set of dependencies, and then a way to build
> the other things independently (perhaps assuming the core is
> built/installed), each with their own dependencies.

I'm all in favor of 'make install' installing only the core of Git, and
different targets for all the other parts.

However, if you take a look at any distribution's packaing of Git you
would see why that wouldn't be desirable (they are full of hacks and
fixes). If the build system is eventually fixed so one package can do
'make install', another 'make install-p4', another 'make install-hg' and
so on, that would be great. But we are pretty far from that.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] remote-hg: more improvements

2014-05-07 Thread Junio C Hamano
Junio C Hamano  writes:

> Felipe Contreras  writes:
>
>> Here's a bunch of tests more, and a fixes for Mercurial v3.0.
>
> I think the discussion with John Keeping hints that we shouldn't be
> rushing fc/remote-helpers-hg-bzr-graduation and this does not appear
> to assume the presense of that series, which is good in order to
> make these fixes jump over them.

When merged to a place somewhere early between the next and the pu
branches (aka "the jch branch", which is the version I usually use
myself), this seemed to break t5810, so I did today's integration
cycle one more time by excluding this topic and then instead queuing
it near the tip of the pu branch (read: today's 'pu' does not pass
the test suite for me).

There may be some other changes that this series depends on that I
may have missed that caused this breakage.  Can you take a look?

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] remote-hg: more improvements

2014-05-07 Thread Junio C Hamano
Felipe Contreras  writes:

> And you are still conveniently avoiding the question:
>
> Based on what reasoning?

Go re-read what was already said in the thread.  I still think
remote-hg and remote-bzr can and will flourish on their own merit,
and unbundling will be better for the end users for the reasons
stated there already.

Having said that, I've been thinking (not because of this thread,
but because I like imerge better and better these days) that there
should be a much better way to have a list of recommended third-party
plug-ins that enrich the Git ecosystem.  We have 

  https://git.wiki.kernel.org/index.php/InterfacesFrontendsAndTools

(Jakub cc'ed as he often nudges those who announce their tools to
add an entry there) but honestly, git-scm.com is the first site the
end users who do not hack on Git would visit, and we probably should
have a catalog there (Scott and Peff cc'ed for that site).  One way
to help it may be to add a Documentation/gitthirdparty-tools.txt in
my tree that would automatically be pulled into git-scm.com site as
a part of the manual.  A richer ecosystem with tools outside my tree
would not materialize unless there is such a mechanism to advertise
the existence of them, and having to include a copy of each and
every third-party tools in my tree and keeping them relatively fresh
is not going to scale in the long run (Michael and Matthieu Cc'ed as
I saw exchanges on multimail in a near-by thread).

Such a file would probably cluster "plugins" into different
categories (post-receive hooks, remote-helpers, mergetools, etc.)
and limit the entry descriptions to a single paragraph of several
lines tops, with URL referring to the third-party maintainer's site
(e.g. a repository at GitHub).

>> > Of course it wasn't a mistake.
>> 
>> I doubt about the "Of course" part.  The first reaction after seeing
>> that the new "changegroup" is used only inside check_version(3,0)
>> and nowhere else was to wonder if that import is necessary (or even
>> safe) for the pre-v3.0 versions.
>
> I don't care about your first reaction. If that was only present in
> newer versions, how do you think it would pass the testing on older
> versions?
>
> https://travis-ci.org/felipec/git
>
> Normally I would explain the details of why this is the case, and send
> the crash regresion fix for v2.0 with a clear explanation,...

Without such an explanation in the log message, how would you expect
anybody to guess correctly?

Seriously, if you do not care about my first reaction, why do you
even want to live in my tree?

> The fact that I'm the maintainer and I say it'ss good should be good
> enough, and if the current version in "master" renders unusable the
> existing Mercurial clones, hey, it's only in contrib, right?

One potential merit I would see for keeping them in my tree is that
your change will see second opinions from others involved in the
project (including me), without giving a total rein based on the
sub-maintainership alone.  All the changes from sub-area maintainers
are vetted by at least two sets of eyeballs that way.

But after having to deal with you and seeing that you do not take
constructive criticism well, I doubt such a possibile merit will
ever materialize in the area where you alone work on.  Letting you
do whatever you want in your own tree may benefit the users of
remote-hg/remote-bzr better as the (bitter) second best option.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] shell doc: remove stray "+" in example

2014-05-07 Thread Jonathan Nieder
The git-shell(1) manpage says

EXAMPLE
   To disable interactive logins, displaying a greeting
instead:

+

   $ chsh -s /usr/bin/git-shell
   $ mkdir $HOME/git-shell-commands
[...]

The stray "+" has been there ever since the example was added in
v1.8.3-rc0~210^2 (shell: new no-interactive-login command to print a
custom message, 2013-03-09).  The "+" sign between paragraphs is
needed in asciidoc to attach extra paragraphs to a list item but here
it is not needed and ends up rendered as a literal "+".  Remove it.

A quick search with "grep -e '+' /usr/share/doc/git/html/*.html"
doesn't find any other instances of this problem.

Signed-off-by: Jonathan Nieder 
---
 Documentation/git-shell.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-shell.txt b/Documentation/git-shell.txt
index c35051b..e4bdd22 100644
--- a/Documentation/git-shell.txt
+++ b/Documentation/git-shell.txt
@@ -66,7 +66,7 @@ EXAMPLE
 ---
 
 To disable interactive logins, displaying a greeting instead:
-+
+
 
 $ chsh -s /usr/bin/git-shell
 $ mkdir $HOME/git-shell-commands
-- 
1.9.1.423.g4596e3a

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [BUG?] Patches created with 'diff.noprefix=true' don't 'git apply'.

2014-05-07 Thread Nathan Collins
On Wed, May 7, 2014 at 11:42 AM, Junio C Hamano  wrote:
> Nathan Collins  writes:

>> For (2), the solution may be to add a separate
>> 'diff.add-clickable-paths' option (probably there is a better name?
>> 'diff.add-copyable-paths'? ...),...
>> ...
>> Concretely, if 'diff.add-clickable-paths' is set, then instead of e.g.
>>
>>   diff --git a/src/Data/Function/Decorator/Memoizer/Unsafe.hs
>> b/src/Data/Function/Decorator/Memoizer
>>   index 3ef17da..a0586d3 100644
>>   --- a/src/Data/Function/Decorator/Memoizer/Unsafe.hs
>>   +++ b/src/Data/Function/Decorator/Memoizer/Unsafe.hs
>
> If you do something along that line, perhaps
>
> Index: src/Data/Function/Decorator/Memoizer/Unsafe.hs
> diff --git a/src/Data/Function/Decorator/Memoizer/Unsafe.hs ...
> index 3ef17da..a0586d3 100644
> --- a/src/Data/Function/Decorator/Memoizer/Unsafe.hs
> +++ b/src/Data/Function/Decorator/Memoizer/Unsafe.hs
>
> to imitate what "cvs diff" does may be more familar to people.
>
> What would you propose to make clickable in a renaming diff, though?

Your 'Index' header looks good, and I would expect a renaming diff to
have something like

  Index: foo -> bar

as in 'git status', but I just realized that a "clickable paths"
option already exists in some sense! There is a '--patch-with-raw'
option (which is "short" for '--patch' and '--raw', hahaha) which
inserts clickable file names in the patch, above each diff.  Moreover,
it respects the '--relative' option, so you can get relative or
absolute (relative repo root) clickable paths. It handles renaming by
inserting the old and new paths separated by space.

So then, having a way to make '--patch-with-raw' the default for all
non-plumbing patch-producing commands would solve the clickable paths
problem.

In a summary, a possible complete solution:

1. improve Git apply error message: mention '-p$n' and '-p1' default,
   and report if path in patch makes sense for some non-default '-p'
   value.

2. improve 'diff.noprefix' documentation: tell user that this option
   is for producing '-p0' patches, and that using it to produce
   clickable paths is insane and may cause problems with generated
   patches.  Suggest the user use '--patch-with-raw', and possibly
   '--relative', instead, or refer to (3).

3. add a Git config for making '--patch-with-raw' and optionally
   '--relative' the default for non-plumbing patch-producing commands.

Cheers,

-nathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Apr 2014, #09; Tue, 29)

2014-05-07 Thread Greg Troxel

Felipe Contreras  writes:

> Greg Troxel wrote:
>> In a packaging system, dependencies are much more troublesome.
>> Dependencies have to be declared, and the build limited to use only
>> those declared dependencies, in order to get repeatable builds and
>> binary packages that can be used on other systems.  Dependencies that
>> really are required are fine.  But optional dependencies are a
>> problem, because e.g. one doesn't want to require the presence of qt
>> to build something (that isn't already enormous).   So if git needs
>> mercurial and subversion installed, plus perhaps 5 other things for
>> less popular remote helpers, that starts to be a real burden.
>
> It doesn't *need* them to build. The Mercurial/Bazaar dependencies are
> optional, both at build-time and at run-time. Most distributions would
> want to test the functionality they are distributing, and for testing
> they do need these dependencies.

My point is that a packaging of git needs to either decide to forego
these optional parts, or to include them, in the default case.  One
choice means that anyone who builds the package from source has to have
the dependencies, and the other means that users of the built package(s)
can't use the features.  I realize that in Linux it's perhaps typical to
not worry about burdening builders because actually building is very
rare, but that's only reasonable because of having only one OS and
perhaps three CPUs; with dozens each, building from source becomes more
frequent.  So I'm merely trying to suggest that it's better to have a
core package with a restrained set of dependencies, and then a way to
build the other things independently (perhaps assuming the core is
built/installed), each with their own dependencies.

It turns out in pkgsrc that git-svn is a meta-package (with no files)
that depends on git-base (no man pages, no gitk) and p5-subversion.
hg-git appears to be a separate source distribution, depending on a
python implementation of the git formats.  So perhaps the situation is
currently ok.  I was just trying to point out the issue to avoid
regressions in the packaging situation.




pgptdIoL4IFDe.pgp
Description: PGP signature


[GIT PULL] l10n update on Swedish

2014-05-07 Thread Jiang Xin
Hi Junio,

The following changes since commit b4f86a4ce85e4e370a67455de6586a02f158a789:

  Git 2.0-rc2 (2014-05-02 13:15:52 -0700)

are available in the git repository at:

  git://github.com/git-l10n/git-po master

for you to fetch changes up to 80dad719fb4bd8090b70da0e6f61ac22a6ff8754:

  l10n: Fix a couple of typos in the Swedish translation (2014-05-07
07:06:37 +0100)


Peter Krefting (1):
  l10n: Fix a couple of typos in the Swedish translation

 po/sv.po | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

--
Jiang Xin
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Pull is Mostly Evil

2014-05-07 Thread Max Kirillov
Hi.

I might be late to this discussion, but here either
something I don't understand or something is missed.

On Sat, May 03, 2014 at 03:56:51AM -0400, Richard Hansen wrote:
> In my experience 'git pull' is mostly (only?) used for the following
> three tasks:
> 
>  1. update a local branch to incorporate the latest upstream changes
> 
> In this case, the local branch (master) is a
> derivative of the upstream branch (origin/master).
> The user wants all of the commits in the remote branch
> to be in the local branch.  And the user would like
> the local changes, if any, to descend from the tip of
> the remote branch.
> 
> For this case, 'git pull --ff-only' followed by 'git
> rebase -p' works well, as does 'git pull
> --rebase=preserve' if the user is comfortable rebasing
> without reviewing the incoming commits first.  A plain
> 'git pull' or 'git pull --ff' is suboptimal due to the
> awkward backwards-parents merge commit.

This is actually not a finally defined use case. What kind
of "local changes" user can have ahead of the remote? As
far I understand, there are 3 cases:

 1a. Changes that are going to be merged back to the master,
 but not yet ready to be there.

This is essentially the same as case 2, but it does not name
the development branch explicitely. Switching parents for
this case is not desirable.

 1b. Some truly local changes which never goes anywhere.

For this case the parent order does not matter.

 1c. The local changes prepared for integration, but instead
 of filing a pull request of otherwise publishing the
 branch for integrator, the leaf developer does the
 integrator's job and merges it back to master and then
 publishing the master.

As far as I understand, this is the only case when somebody
would want the parents to be switched. And this does not
seem to be a good practice, because it's prone to push races
and requires letting everyone to push to master. So maybe
git should not encourage people to do so.

And the name "update", proposed here, does not seem to be
correct. Because what happens is not updating, but merging
feature to master and closing it.

>  2. update a published feature branch with the latest
> changes from its parent branch

>  3. integrate a more-or-less complete feature/fix back
> into the line of development it forked off of

-- 
Max
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Standardize python shebangs

2014-05-07 Thread James Denholm
On 8 May 2014 06:57:13 GMT+10:00, Matthieu Moy  
wrote:
>Felipe Contreras  writes:
>
>> Matthieu Moy wrote:
>>> Felipe Contreras  writes:
>>> > If you want to use python2, then use '/usr/bin/env python2'.
>>> 
>>> Err, yes, this is what the code does before your patch.
>>
>> Not for all the instances.
>
>Well, I guess not all scripts require python2, hence not all scripts
>declare that they depend on python2.

Does it make sense for git to have a strict subset of python scripts as
version agnostic, though? Given that some scripts depend on python2
already, and hence we know that the user has python2, and these scripts
run perfectly well on python2, why not mandate that the agnostic subset
be run on python2?

Regards,
James Denholm.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GUILT 25/28] "guilt push" now fails when there are no more patches to push.

2014-05-07 Thread Jeff Sipek
On Fri, Mar 21, 2014 at 08:32:03AM +0100, Per Cederqvist wrote:
> This makes it easier to script operations on the entire queue, for
> example run the test suite on each patch in the queue:
> 
> guilt pop -a;while guilt push; do make test||break; done
> 
> This brings "guilt push" in line with the push operation in Mercurial
> Queues (hg qpush), which fails when there are no patches to apply.
> 
> Updated the test suite.
> 
> "guilt push -a" still does not fail.  (It successfully manages to
> ensure that all patches are pushed, even if it did not have to do
> anything to make it so.)
> 
> Signed-off-by: Per Cederqvist 
> ---
>  guilt-push   | 14 -
>  regression/t-020.out | 89 
> 
>  regression/t-020.sh  | 13 +++-
>  3 files changed, 108 insertions(+), 8 deletions(-)
> 
> diff --git a/guilt-push b/guilt-push
> index 67687e7..350 100755
> --- a/guilt-push
> +++ b/guilt-push
> @@ -55,6 +55,7 @@ fi
>  
>  patch="$1"
>  [ ! -z "$all" ] && patch="-a"
> +[ -z "$patch" ] && { patch=1; num=t; }

I don't think there's any other place in the repo that does this.  Instead
you see a lot of if-then-fi.  To keep it consistent, I'd suggest:

if [ -z "$patch" ] ; then
patch=1
num=t
fi

Ah, this took me a while to figure out.  The above turns:

$ guilt push

into 

$ guilt push -n 1

I'd throw in a comment.

(Note to self, this file is a huge mess and could use a bit of cleanup.)

> @@ -78,11 +79,6 @@ elif [ ! -z "$num" ]; then
>   # clamp to minimum
>   [ $tidx -lt $eidx ] && eidx=$tidx
>  
> -elif [ -z "$patch" ]; then
> - # we are supposed to push only the next patch onto the stack
> -
> - eidx=`wc -l < "$applied"`
> - eidx=`expr $eidx + 1`
>  else
>   # we're supposed to push only up to a patch, make sure the patch is
>   # in the series
> @@ -109,7 +105,11 @@ if [ "$sidx" -gt "$eidx" ]; then
>   else
>   disp "File series fully applied, ends at patch `get_series | 
> tail -n 1`"
>   fi
> - exit 0
> + if [ -n "$all" ]; then
> + exit 0
> + else
> + exit 1
> + fi

This changes the output on stdout.  E.g.,

$ guilt pu
$ guilt pu -n 1
File series fully applied, ends at patch crashdump

With this patch, both will print the message. Right?

>  fi
>  
>  get_series | sed -n -e "${sidx},${eidx}p" | while read p

Jeff.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] contrib/subtree bugfix: Crash if FETCH_HEAD is tag

2014-05-07 Thread James Denholm
On 4 May 2014 16:33:32 GMT+10:00, James Denholm  wrote:
>cmd_add_commit() is passed FETCH_HEAD by cmd_add_repository, which is
>then rev-parsed into an object ID. However, if the user is fetching a
>tag rather than a branch HEAD, such as by executing:
>
>$ git subtree add -P oldGit https://github.com/git/git.git tags/v1.8.0
>
>The object ID is a tag and is never peeled, and the git commit-tree
>call
>(line 561) slaps us in the face because it doesn't handle tag IDs.
>
>Because peeling a committish ID doesn't do anything if it's already a
>commit, fix by peeling[1] the object ID before assigning it to $rev, as
>per the patch.
>
>[*1*]: Via peel_committish(), from git:git-sh-setup.sh
>
>Reported-by: Kevin Cagle 
>Diagnosed-by: Junio C Hamano 
>Signed-off-by: James Denholm 
>---
>NB: This bug doesn't surface when using --squash, as $rev is reassigned
>to the squash commit via new_squash_commit before git commit-tree sees
>it (though for simplicity, new_squash_commit now also sees the peeled
>ID).
>
>Also doesn't surface when using "git subtree merge", as git merge can
>handle tag objects.
>
>On a side note, if merging a tag without --squash, git merge recognises
>that it's a tag and adds a note to the merge commit body. It may be
>worth mimicking this when using "subtree merge --squash" or
>"subtree add".
>
> contrib/subtree/git-subtree.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/contrib/subtree/git-subtree.sh
>b/contrib/subtree/git-subtree.sh
>index dc59a91..9453dae 100755
>--- a/contrib/subtree/git-subtree.sh
>+++ b/contrib/subtree/git-subtree.sh
>@@ -538,7 +538,7 @@ cmd_add_commit()
> {
>   revs=$(git rev-parse $default --revs-only "$@") || exit $?
>   set -- $revs
>-  rev="$1"
>+  rev=$(peel_committish "$1")
>   
>   debug "Adding $dir as '$rev'..."
>   git read-tree --prefix="$dir" $rev || exit $?

I know that subtree isn't exactly the most popular or exciting part of
the project at the moment, but given that this is adding a subtree based
on an annotated tag is a reasonably sensible operation and (to me) the
fix seems reasonably trivial, could I get some eyes on this?

Regards,
James Denholm.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Apr 2014, #09; Tue, 29)

2014-05-07 Thread Felipe Contreras
John Keeping wrote:
> On Wed, May 07, 2014 at 03:26:15PM -0500, Felipe Contreras wrote:
> > Junio C Hamano wrote:
> > > Your git-integrate might turn into something I could augment my
> > > workflow with with some additions.
> > > 
> > >  - specifying a merge strategy per branch being merged;
> > 
> > git-reintegrate[1] supports this.
> > 
> > >  - support evil merges or picking a fix-up commit;
> > 
> > git-reintegrate supports this.
> > 
> > >  - leaving an empty commit only to leave comment in the history.
> > 
> > Done[2].
> > 
> > 
> > > and until that happens, I'll keep using the Reintegrate script found
> > > in my 'todo' branch.
> > 
> > My git-reintegrate supports everything John's git-integrate and in
> > addition it supports generating the commands from an existing branch,
> > like your Reintegrate. IOW; it's superior.
> 
> And yet the documentation is unchanged from the version you copied in
> from git-integration.

Not much has changed since v0.1 since that version already worked
perfectly. But I'll update it.

> Personally I would much rather use a project which takes time to
> document all of the features rather than relying on reading the code
> to figure out the options.

And I would rather use a project that concentrates on having the
features users need.

> More features does not make a project superior.

No, better features do.

Either way. Documentation updated.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: t5539 fails on ubuntu for v2.0.0-rc2

2014-05-07 Thread Junio C Hamano
Fabio D'Alfonso  writes:

> Hi,
> I am getting this in Ubuntu, something wrong with my env?
> Thanks
>
> root@HDUBVM01:~/builds/git/t# ./t5539-fetch-http-shallow.sh
> ok 1 - setup shallow clone
> not ok 2 - clone http repository
> #
> #git clone --bare --no-local shallow
> "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
> #git clone $HTTPD_URL/smart/repo.git clone &&
> #(
> #cd clone &&
> #git fsck &&
> #git log --format=%s origin/master >actual &&
> #cat  #7
> #6
> #5
> #4
> #3
> #EOF
> #test_cmp expect actual
> #)

Does not reproduce for me but I am on Ubuntu 12.04.2 LTS, so...

Running it with the -v option might give you more hints, and
running it as

$ sh -x ./t5539-fe* -v

might give you more to chew.

I do not think it is wise to run tests as root, though ;-)


> #
> not ok 3 - no shallow lines after receiving ACK ready
> #
> #(
> #cd shallow &&
> #for i in $(test_seq 15)
> #do
> #git checkout --orphan unrelated$i &&
> #test_commit unrelated$i &&
> #git push -q "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" \
> #refs/heads/unrelated$i:refs/heads/unrelated$i &&
> #git push -q ../clone/.git \
> #refs/heads/unrelated$i:refs/heads/unrelated$i ||
> #exit 1
> #done &&
> #git checkout master &&
> #test_commit new &&
> #git push  "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" master
> #) &&
> #(
> #cd clone &&
> #git checkout --orphan newnew &&
> #test_commit new-too &&
> #GIT_TRACE_PACKET="$TRASH_DIRECTORY/trace" git fetch
> --depth=2 &&
> #grep "fetch-pack< ACK .* ready" ../trace &&
> #! grep "fetch-pack> done" ../trace
> #)
> #
> httpd (pid 10653?) not running
> # failed 2 among 3 test(s)
> 1..3
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] builtin/tag.c: Add tag name to user message

2014-05-07 Thread Junio C Hamano
Thorsten Glaser  writes:

> Display the tag name about to be added to the user during interactive
> editing.
>
> Signed-off-by: Thorsten Glaser 
> Signed-off-by: Richard Hartmann 
> ---

Sounds sensible from a first glance.  Will queue; thanks.

>  builtin/tag.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/tag.c b/builtin/tag.c
> index 6c7c6bd..8a7265b 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> @@ -278,11 +278,11 @@ static int do_sign(struct strbuf *buffer)
>  }
>  
>  static const char tag_template[] =
> - N_("\nWrite a tag message\n"
> + N_("\nWrite a message for tag:\n  %s\n"
>   "Lines starting with '%c' will be ignored.\n");
>  
>  static const char tag_template_nocleanup[] =
> - N_("\nWrite a tag message\n"
> + N_("\nWrite a message for tag:\n  %s\n"
>   "Lines starting with '%c' will be kept; you may remove them"
>   " yourself if you want to.\n");
>  
> @@ -378,9 +378,9 @@ static void create_tag(const unsigned char *object, const 
> char *tag,
>   struct strbuf buf = STRBUF_INIT;
>   strbuf_addch(&buf, '\n');
>   if (opt->cleanup_mode == CLEANUP_ALL)
> - strbuf_commented_addf(&buf, _(tag_template), 
> comment_line_char);
> + strbuf_commented_addf(&buf, _(tag_template), 
> tag, comment_line_char);
>   else
> - strbuf_commented_addf(&buf, 
> _(tag_template_nocleanup), comment_line_char);
> + strbuf_commented_addf(&buf, 
> _(tag_template_nocleanup), tag, comment_line_char);
>   write_or_die(fd, buf.buf, buf.len);
>   strbuf_release(&buf);
>   }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GUILT 24/28] disp no longer processes backslashes.

2014-05-07 Thread Jeff Sipek
On Fri, Mar 21, 2014 at 08:32:02AM +0100, Per Cederqvist wrote:
> Only one invocation of "disp" or "_disp" actually needed backslash
> processing.  In quite a few instances, it was wrong to do backslash
> processing, as the message contained data derived from the user.
> 
> Created the new function "disp_e" that should be used when backslash
> processing is required, and changed "disp" and "disp_" to use printf

s/disp_/_disp/

otherwise LGTM

> code %s instead of "%b".
> 
> Signed-off-by: Per Cederqvist 
> ---
>  guilt | 17 +
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/guilt b/guilt
> index ca922aa..36cfd1e 100755
> --- a/guilt
> +++ b/guilt
> @@ -36,15 +36,24 @@ usage()
>   exit 1
>  }
>  
> -# echo -n is a bashism, use printf instead
> +# Print arguments, but no trailing newline.
> +# (echo -n is a bashism, use printf instead)
>  _disp()
>  {
> - printf "%b" "$*"
> + printf "%s" "$*"
>  }
>  
> -# echo -e is a bashism, use printf instead
> +# Print arguments.
> +# (echo -E is a bashism, use printf instead)
>  disp()
>  {
> + printf "%s\n" "$*"
> +}
> +
> +# Print arguments, processing backslash sequences.
> +# (echo -e is a bashism, use printf instead)
> +disp_e()
> +{
>   printf "%b\n" "$*"
>  }
>  
> @@ -117,7 +126,7 @@ else
>  
>   disp ""
>   disp "Example:"
> - disp "\tguilt push"
> + disp_e "\tguilt push"
>  
>   # now, let's exit
>   exit 1
> -- 
> 1.8.3.1
> 

-- 
We have joy, we have fun, we have Linux on a Sun...
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GUILT 27/28] Minor testsuite fix.

2014-05-07 Thread Jeff Sipek
Signed-off-by: Josef 'Jeff' Sipek 

On Fri, Mar 21, 2014 at 08:32:05AM +0100, Per Cederqvist wrote:
> Fix remove_topic() in t-061.sh so that it doesn't print a git hash.
> 
> Signed-off-by: Per Cederqvist 
> ---
>  regression/t-061.out | 1 -
>  regression/t-061.sh  | 2 +-
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/regression/t-061.out b/regression/t-061.out
> index ef0f335..60ad56d 100644
> --- a/regression/t-061.out
> +++ b/regression/t-061.out
> @@ -381,7 +381,6 @@ ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba commit   
> refs/patches/master/mode
>  ffb7faa126a6d91bcdd44a494f76b96dd860b8b9 commit  
> refs/patches/master/remove
>  % guilt pop -a
>  No patches applied.
> -ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba
>  % git checkout guilt/master
>  Switched to branch "guilt/master"
>  % guilt pop -a
> diff --git a/regression/t-061.sh b/regression/t-061.sh
> index a9a4fea..b573885 100755
> --- a/regression/t-061.sh
> +++ b/regression/t-061.sh
> @@ -15,7 +15,7 @@ old_style_branch() {
>  
>  remove_topic() {
>   cmd guilt pop -a
> - if git rev-parse --verify --quiet guilt/master
> + if git rev-parse --verify --quiet guilt/master >/dev/null
>   then
>   cmd git checkout guilt/master
>   else
> -- 
> 1.8.3.1
> 

-- 
Fact: 12.5% of all statistics are generated randomly.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GUILT 07/28] Added test cases for "guilt fold".

2014-05-07 Thread Jeff Sipek
On Wed, May 07, 2014 at 10:59:56PM +0200, Per Cederqvist wrote:
> On Tue, May 6, 2014 at 9:40 PM, Jeff Sipek  wrote:
> > On Fri, Mar 21, 2014 at 08:31:45AM +0100, Per Cederqvist wrote:
> >> Test that we can combine any combination of patches with empty and
> >> non-empty messages, both with and without guilt.diffstat.  (All
> >> patches are empty.)
> >
> > I feel like we should have *some* content there - most of the time, I care
> > more about the diffs getting folded than the commit message :)
> 
> I added these tests for a reason: the reproduce a bug in guilt that I found.
> 
> I'm afraid that having some content might hide the bug I found. (I'm also
> equally afraid that it might uncover other bugs in guilt, which would delay
> integration of this patch series. So adding more test cases with content
> is a good thing to do, but maybe not in this patch series.)

Fair enough.  I use guilt-fold all the time and it hasn't lost any of my
diffs, so I'm happy to defer this until some point in the future.

...
> > for using_diffstat in true false ; do
> > for patcha in empty nonempty ; do
> > for patchb in empty nonempty ; do
> > echo "%% $patcha + $patchb 
> > (diffstat=$using_diffstat)"
> > ${patcha}_patch $patcha
> > ${patchb}_patch $patchb
> > cmd guilt pop $patchb
> > cmd guilt fold $patchb
> > fixup_time_info $patcha
> > cmd list_files
> > cleanup $patcha
> > cmd list_files
> > done
> > done
> > done
> >
> > Aha!  That's better, IMO.
> 
> I'll try that and post a version 2 of the series. It might take a few
> days, though.

No problem.  I'm still the slower one of the two of us. :/

Jeff.

-- 
Keyboard not found!
Press F1 to enter Setup
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Standardize python shebangs

2014-05-07 Thread Matthieu Moy
Felipe Contreras  writes:

> Matthieu Moy wrote:
>> Felipe Contreras  writes:
>> > If you want to use python2, then use '/usr/bin/env python2'.
>> 
>> Err, yes, this is what the code does before your patch.
>
> Not for all the instances.

Well, I guess not all scripts require python2, hence not all scripts
declare that they depend on python2.

And anyway, I don't see how changing python2 to python in git-multimail
solves any problem.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GUILT 07/28] Added test cases for "guilt fold".

2014-05-07 Thread Per Cederqvist
On Tue, May 6, 2014 at 9:40 PM, Jeff Sipek  wrote:
> On Fri, Mar 21, 2014 at 08:31:45AM +0100, Per Cederqvist wrote:
>> Test that we can combine any combination of patches with empty and
>> non-empty messages, both with and without guilt.diffstat.  (All
>> patches are empty.)
>
> I feel like we should have *some* content there - most of the time, I care
> more about the diffs getting folded than the commit message :)

I added these tests for a reason: the reproduce a bug in guilt that I found.

I'm afraid that having some content might hide the bug I found. (I'm also
equally afraid that it might uncover other bugs in guilt, which would delay
integration of this patch series. So adding more test cases with content
is a good thing to do, but maybe not in this patch series.)

>> Signed-off-by: Per Cederqvist 
>> ---
>>  regression/t-035.out | 659 
>> +++
>>  regression/t-035.sh  |  88 +++
>>  2 files changed, 747 insertions(+)
>>  create mode 100644 regression/t-035.out
>>  create mode 100755 regression/t-035.sh
>>
>> diff --git a/regression/t-035.out b/regression/t-035.out
>> new file mode 100644
>> index 000..04af146
>> --- /dev/null
>> +++ b/regression/t-035.out
>> @@ -0,0 +1,659 @@
>> +% setup_repo
>> +% git config guilt.diffstat true
>> +%% empty + empty (diffstat=true)
>> +% guilt new empty-1
>> +% guilt pop
>> +All patches popped.
>> +% guilt push
>> +Applying patch..empty-1
>> +Patch applied.
>> +% guilt new empty-2
>> +% guilt pop
>> +Now at empty-1.
>> +% guilt push
>> +Applying patch..empty-2
>> +Patch applied.
>> +% guilt pop
>> +Now at empty-1.
>> +% guilt fold empty-2
>> +% guilt pop
>> +All patches popped.
>> +% guilt push
>> +Applying patch..empty-1
>> +Patch applied.
>> +% list_files
>> +d .git/patches
>> +d .git/patches/master
>> +d .git/refs/patches
>> +d .git/refs/patches/master
>> +f 22930c6d1f1938f298a4fca51c57e4b47171db21  .git/patches/master/mode
>> +f 413390f3906f16f30b054a4fb86c1e014b964504  .git/patches/master/remove
>> +f 4ea806e306f0228a8ef41f186035e7b04097f1f2  .git/patches/master/status
>> +f 7d261b8caad0f161c21daf5de65eeb521ff8c067  .git/patches/master/empty-1
>> +f 9c18cc7abe6b87f18503714a80a677b4094eb457  .git/patches/master/add
>> +f bc9ab2e0f5db99d483961e956e814d963f0309f8  .git/patches/master/modify
>> +f d28d87b88c1e24d637e390dc3603cfa7c1715711  .git/patches/master/series
>> +f da39a3ee5e6b4b0d3255bfef95601890afd80709  .git/patches/master/empty-1~
>> +f da39a3ee5e6b4b0d3255bfef95601890afd80709  .git/patches/master/empty-2~
>> +r bde3d337af70f36836ad606c800d194006f883b3  .git/refs/patches/master/empty-1
>> +% git log -p
>
> Strictly speaking, git log isn't necessary since list_files prints the
> hashes of each of the files as well as the refs for all the applied patches.
> If anything mismatches, the hashes will catch it.  I'm ok with keeping the
> git log here as long as people can't mess up the formatting with git
> config/etc.

Having the patches included was very helpful while I developed the
test case, but your concern about messing up the formatting is valid.
I'll remove them.

> ...
>> diff --git a/regression/t-035.sh b/regression/t-035.sh
>> new file mode 100755
>> index 000..aed3ef2
>> --- /dev/null
>> +++ b/regression/t-035.sh
>> @@ -0,0 +1,88 @@
>> +#!/bin/bash
>> +#
>> +# Test the fold code
>> +#
>> +
>> +source "$REG_DIR/scaffold"
>> +
>> +cmd setup_repo
>> +
>> +function fixup_time_info
>> +{
>> + cmd guilt pop
>> + touch -a -m -t "$TOUCH_DATE" ".git/patches/master/$1"
>> + cmd guilt push
>> +}
>> +
>> +function test_fold
>> +{
>> +using_diffstat=$1
>> +
>> +cmd git config guilt.diffstat $using_diffstat
>> +
>> +# Empty message + empty message = empty message.
>> +echo "%% empty + empty (diffstat=$using_diffstat)"
>> +cmd guilt new empty-1
>> +fixup_time_info empty-1
>> +cmd guilt new empty-2
>> +fixup_time_info empty-2
>> +cmd guilt pop
>> +cmd guilt fold empty-2
>> +fixup_time_info empty-1
>> +cmd list_files
>> +cmd git log -p
>> +cmd guilt pop
>> +cmd guilt delete -f empty-1
>> +cmd list_files
>> +
>> +# Empty message + non-empty message
>> +echo "%% empty + non-empty (diffstat=$using_diffstat)"
>> +cmd guilt new empty
>> +fixup_time_info empty
>> +cmd echo test > a
>
> I see these redirected echos... what are they for?

The intent was to create files with some content. I was under
the impression that the "guilt new" on the next line would
include the file, since I use the -f option. Now I see that
that is not the case, and I will remove these lines.

>> +cmd guilt new -f -s -m "A commit message." non-empty
>> +fixup_time_info non-empty
>> +cmd guilt pop
>> +cmd guilt fold non-empty
>> +fixup_time_info empty
>> +cmd list_files
>> +cmd git log -p
>> +cmd guilt pop
>> +cmd guilt delete -f empty
>> +cmd list_files
>
> Maybe make two helper functions.. one to make a p

Re: [PATCH 1/3] After chdir to run grep, return to old directory

2014-05-07 Thread David Turner
On Wed, 2014-05-07 at 10:42 -0700, Junio C Hamano wrote:
> David Turner  writes:
> 
> > This causes my test to pass and generally seems correct to me.
> 
> Yes, this approach is very sensible, and I'll queue.
> 
> But watchman support _should_ be prepared for a program that does
> not do this.  Developing your support in on a codebase with this
> patch may be sweeping a bug in your code under the rug.

I agree that good defensive coding practice would be to not depend on
the cwd.  That's just what everything in environment.c presently does.
I don't want to change the rest of the get_*_file functions, because I
don't know what their callers expect of them.  Here is my patch to do
change just the fs_cache bit:

https://github.com/dturner-tw/git/commit/3fe93aeaee9719ee171a253c49af5126a057c513.patch

(I went ahead and made it part of 
the watchman branch on https://github.com/dturner-tw/git.git )

I wanted to just do the fixup where the path is used, but I couldn't see
a function for that.  If there's no function, that indicates to me that
it's probably a bad idea.  But maybe I'm just missing it.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] merge-recursive.c: Fix case-changing merge.

2014-05-07 Thread Junio C Hamano
Jonathan Nieder  writes:

>> Please make it a habit to use "test -f" when you expect "the path
>> exists as a file", not merely "something exists there I do not care
>> if it is a file or a directory", for which "test -e" is perfectly
>> appropriate.
>
> Or, better in tests,
>
>   test_path_is_file testcase
>
> which prints an error instead of just silently failing when
> the path is not a file.

Thanks, will tweak.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


t5539 fails on ubuntu for v2.0.0-rc2

2014-05-07 Thread Fabio D'Alfonso

Hi,
I am getting this in Ubuntu, something wrong with my env?
Thanks

root@HDUBVM01:~/builds/git/t# ./t5539-fetch-http-shallow.sh
ok 1 - setup shallow clone
not ok 2 - clone http repository
#
#git clone --bare --no-local shallow 
"$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&

#git clone $HTTPD_URL/smart/repo.git clone &&
#(
#cd clone &&
#git fsck &&
#git log --format=%s origin/master >actual &&
#cat  done" ../trace
#)
#
httpd (pid 10653?) not running
# failed 2 among 3 test(s)
1..3

--

Fabio D'Alfonso
'Enabling Business Through IT'
cell.  +39.348.059.40.22 ***
web: www.fabiodalfonso.com 
email: fabio.dalfo...@fabiodalfonso.com
linkedin: 
www.linkedin.com/in/fabiodalfonso 
twitter: www.twitter.com/#!/fabio_dalfonso 



fax: +39.06.874.599.581
BlackBerry® Wireless Enabled Address.


 ** Hidden  numbers are automatically rejected by the phone*

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Problem: staging of an alternative repository

2014-05-07 Thread Pasha Bolokhov
Hi,

I've looked more attentively, here are my observations and the
resulting suggestions:

- Suggest only to check *string-wise* the given "path" against
$GIT_DIR. Both or one of them may be relative paths (but comparison
best be performed when converted to absolute paths). That's the only
solution which will give predictable results, but does not handle
symlinks: if you have symlinks (so that both differing paths actually
point to the same location) the comparison will return failure. Only
if you have an exact string-wise match do we ignore the "path"

- With this way of comparison, only the root ".git_new" will be
ignored. Submodules will likely contain the usual ".git". Since the
code normally ignores ".git" anyway, and I do not intend to change
that, I don't see how submodule detection can be affected

- The problem of resolving symlinks is in a very general case
insolvable (e.g. imagine one of the symlinks points to another
filesystem which may be up or down depending on the day of week - it's
easy to plot a scenario where symlinks will resolve (or even fail to
resolve) differently at different runs)

- Even if it was solvable, the current implementation of handling
".git" certainly does not check any symlinks:
$  mv -i   .git   .metadata
$  ln -s .metadata .git
  Then certainly "git add -A" will grab all ".metadata" and store into itself

  Please let me know what you think. Again, I can try to carefully do
this suggestion

Pavel


On Thu, May 1, 2014 at 11:20 PM, Duy Nguyen  wrote:
> On Thu, May 1, 2014 at 4:35 AM, Jonathan Nieder  wrote:
>>> Now I know, the '--git-dir' option may usually be meant to use
>>> when the repository is somewhere outside of the work tree, and such a
>>> problem would not arise. And even if it is inside, sure enough, you
>>> can add this '.git_new' to the ignores or excludes. But is this really
>>> what you expect?
>>
>> I think it's more that it never came up.  Excluding the current
>> $GIT_DIR from what "git add" can add (on top of the current rule of
>> excluding all instances of ".git") seems like a sensible change,
>> assuming it can be done without hurting the code too much. ;-)
>
> I think it came up before. Changes could be very messy (but I did not
> check carefully) because right now we just compare $(basename $path)
> with ".git", one path component, simple and easy. Checking against
> $GIT_DIR means all path components. You also have to deal with
> relative and absolute paths and symlinks in some path components. You
> may also need to think if submodule detection code (checking ".git"
> again) is impacted. On top of that, read_directory() code is already
> messy (or at least scary to me) with all kinds of shortcuts we have
> added over the years. A simpler solution may be ignoring all
> directories whose last component is  "$GIT_DIR_NAME" (e.g.
> GIT_DIR_NAME=.git_new).
> --
> Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Beginner question on "Pull is mostly evil"

2014-05-07 Thread Junio C Hamano
Jim Garrison  writes:

>> -Original Message-
>> From: Junio C Hamano
>> Sent: Wednesday, May 07, 2014 1:16 PM
>> Subject: Re: Beginner question on "Pull is mostly evil"
>> 
>> No.  This is most often true for people who use a single repository as a
>> place for everybody to meet, in the same way as SVN.
> [snip lots of excellent detail]
>> HTH.
>
> Wow.  That helps tremendously, and should be incorporated somewhere in the
> Git documentation.  Thank you for your immensely detailed response.

We used to collect useful list postings in Documentation/howto/;
perhaps somebody wants to do the minimum copyediting of the message
and send a patch?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Standardize python shebangs

2014-05-07 Thread Felipe Contreras
Matthieu Moy wrote:
> Felipe Contreras  writes:
> > If you want to use python2, then use '/usr/bin/env python2'.
> 
> Err, yes, this is what the code does before your patch.

Not for all the instances.

> > If you are going to follow practices different than git.git, then
> > multimail shouldn't live in 'contrib/'.
> 
> git-multimail is not the only part of git.git that has a separate
> maintainer. Same goes for git-gui and gitk.

They are not part of contrib.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] remote-hg: more improvements

2014-05-07 Thread Felipe Contreras
Junio C Hamano wrote:
> Felipe Contreras  writes:
> > Really? Based on what reasoning? I have proven his reasoning to be
> > basically wrong.
> 
> Perhaps s/proven/convinced myself only/; you didn't prove it to me
> and I doubt you proved it to John.

And you are still conveniently avoiding the question:

Based on what reasoning?

> > Of course it wasn't a mistake.
> 
> I doubt about the "Of course" part.  The first reaction after seeing
> that the new "changegroup" is used only inside check_version(3,0)
> and nowhere else was to wonder if that import is necessary (or even
> safe) for the pre-v3.0 versions.

I don't care about your first reaction. If that was only present in
newer versions, how do you think it would pass the testing on older
versions?

https://travis-ci.org/felipec/git

Normally I would explain the details of why this is the case, and send
the crash regresion fix for v2.0 with a clear explanation, but since you
are adamant in threating git-remote-hg/bzr as just another crappy
contrib script that doesn't even have tests like diff-highlight or
hg-to-git. Why would I care?

The fact that I'm the maintainer and I say it'ss good should be good
enough, and if the current version in "master" renders unusable the
existing Mercurial clones, hey, it's only in contrib, right?

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] add a reflog_exists and delete_reflog abstraction

2014-05-07 Thread Ronnie Sahlberg
On Tue, May 6, 2014 at 8:23 AM, Michael Haggerty  wrote:
> On 05/06/2014 12:57 AM, Ronnie Sahlberg wrote:
>> This is a single patch that adds two new functions to try to hide the reflog
>> implementation details from the callers in checkout.c and reflog.c.
>> It adds new functions to test if a reflog exists and to delete it, thus
>> allowing checkout.c to perform this if-test-then-delete operation without
>> having to know the internal implementation of reflogs (i.e. that they are 
>> files
>> that live unde r.git/logs)
>>
>> It also changes checkout.c to use ref_exists when checking for whether a ref
>> exists or not instead of checking if the loose ref file exists or not.
>> Using ref_exists instead both hides the reflog implementation details from
>> checkout.c as well as making the code more robust against future changes.
>> It currently has a hard assumption that the loose ref file must exist at this
>> stage or else it would end up deleting the reflog which is true today, as far
>> as I can tell, but would break if git would change such that we could have
>> a branch checked out without having a loose ref file for that branch.
>
> For single patches, people usually don't send a separate cover letter.
> Any comments that you want to make that are not suitable for the commit
> message can go between the "---" line and the diffstat of the patch email.
>
> Regarding this change, I think before too long we will also need an API
> to turn reflogs on for a reference.  We might want to add a flag to ref
> transaction entries that cause the reflog to be created if it doesn't
> already exist.  Reflogs can currently be created for a reference via the
> config setting "core.logAllRefUpdates" or (for branches) by "git branch
> --create-reflog" (maybe there are others).

Yes. I agree.

I think we need a flag to ref_transaction_create and ref_transaction_update to
create a new reflog.

Then I also think we need to add a new transaction command,
ref_transaction_reflog() that can be used to either append or overwrite the
reflog.  Builtin/reflog.c will need an API to create/overwrite the reflog
with new text.

And finally we also need an api to create symrefs.
ref_transaction_symref() or something.


But this will wait until my current patch series is merged.

>
> Several tests in the test suite currently create reflog files by hand.
> Thus we might also need a way to create reflogs at the command line by
> the time we implement pluggable reference backends.

I can add this once we have an api.

>
> Michael
>
> --
> Michael Haggerty
> mhag...@alum.mit.edu
> http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GUILT 06/28] Fix and simplify the do_get_patch function.

2014-05-07 Thread Per Cederqvist
n Tue, May 6, 2014 at 9:08 PM, Jeff Sipek  wrote:
> On Sun, Mar 23, 2014 at 10:03:48PM +0100, Per Cederqvist wrote:
>> On Sun, Mar 23, 2014 at 6:09 PM, Jeff Sipek  wrote:
>>
>> > On Fri, Mar 21, 2014 at 08:31:44AM +0100, Per Cederqvist wrote:
>> >> When extracting the patch, we only want the actual patches.  We don't
>> >> want the "---" delimiter.  Simplify the extraction by simply deleting
>> >> everything before the first "diff " line.  (Use sed instead of awk to
>> >> simplify the code.)
>> >>
>> >> Without this patch, "guilt fold" and "guilt push" sometimes fails if
>> >> guilt.diffstat is true.
>
> Hrm, I just did a test and guilt-push seems to work with a patch such as:
>
> aoeuaoeu
> this is
> ---
> not a patch!
> ---
>  foo |1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/foo b/foo
> index e69de29..203a557 100644
> --- a/foo
> +++ b/foo
> @@ -0,0 +1 @@
> +aoue
>
> What sort of thing breaks fold/push?

This sequence breaks:

  mkdir guiltdemo
  cd guiltdemo
  git init
  git commit --allow-empty -mstart
  guilt init
  git config guilt.diffstat true
  guilt new empty-1
  guilt new empty-2
  guilt pop
  guilt fold empty-2
  guilt pop
  guilt push

That is, create two empty patches, fold them together, pop them, and try to push
the combined (still empty) patch.

The last command fails with:

  Applying patch..empty-1
  fatal: unrecognized input
  To force apply this patch, use 'guilt push -f'

At this point, the patch series consists of a single patch, empty-1,
which contains
the five characters "-", "-", "-", newline, newline.

The 06 patch (Added test cases for "guilt fold".) contains a test case
for this issue.
Which opens a style issue. When you fix a bug, should you:

 - commit the bug fix first and the test case later, so that "git bisect" always
   finds commits where "make test" works, or
 - commit the test case first, and the bug fix later, so that it is more obvious
   that you are actually fixing a pre-existing bug, or
 - commit the test case and the bug fix in the same commit?

In this series I have committed bug fixes first and test cases later, but
I'm not convinced that is the right thing to do.

> ...
>> >> diff --git a/guilt b/guilt
>> >> index 8701481..c59cd0f 100755
>> >> --- a/guilt
>> >> +++ b/guilt
>> >> @@ -332,12 +332,7 @@ do_make_header()
>> >>  # usage: do_get_patch patchfile
>> >>  do_get_patch()
>> >>  {
>> >> - awk '
>> >> -BEGIN{}
>> >> -/^(diff |---$|--- )/ {patch = 1}
>> >> -patch == 1 {print $0}
>> >> -END{}
>> >> -' < "$1"
>> >> + sed -n '/^diff /,$p' < "$1"
>> >
>> > So, the thought behind this mess was to allow minimal patches to work.  The
>> > 'diff' line is *not* required by patch(1)!
>>
>> I see. I can see that this is sometimes useful, even though I think
>> it is more important that guilt actually works with the patches itself
>> creates.
>
> Fair enough.  I'm convinced that we should assume that all patches start
> with 'diff '.
>
> ...
>> I had to solve a similar problem when I wrote my utility for diffing two
>> diff files (https://git.lysator.liu.se/pdiffdiff). Based on the experience
>> I got doing that, I propose this new do_get_patch function:
> ...
>>
>> The idea is to collect lines that *might* start the patch in
>> patchheader. Once we detect the start of the patch (via a
>> line that matches "--- " or any of the mode change lines)
>> we print the patcheader and the current line. If we get a
>> line that neither looks like a patchheader nor starts a patch,
>> we discard the patcheader. This is the case of a commit
>> message with a line that starts with "diff ".
>>
>> The function above solves the issue with lines that
>> start with "diff " in the commit message.  On the other
>> hand, it introduces the same issue for lines that start
>> with for instance "old mode ".
>
> Hrm.  I'm open to revisiting the get-patch/get-header functions to address
> the ambiguity issues in the future.

Postponing that for the future seems like a good plan. I think
there are three possible ways to deal with the problem.

 - Store the commit message, diffstat, and diffs in separate files.
   I don't like this option, as it would make it a lot less convenient
   to work with the patch files.

 - Add a quoting mechanism, so that you need to write ">diff" or
   ">---" instead of just "diff" or "---" if you want to include those
   characters at the start of the line in a commit message.

 - Ignore the problem. This is what guilt has done in the past,
   and I see no reason why it needs to change *in this patch series*.

>> Actually, a smaller change that probably fixes the
>> issue with diffstat is to replace
>>
>> /^(diff |---$|--- )/ {patch = 1}
>>
>> witih
>>
>> /^(diff |--- )/ {patch = 1}
>>
>> as the problem with the original implementation is that
>> the "---\n" line that starts the diffstat should not start
>> the patch.
>
> (Thinking out loud...) I suppose there are three portions to a patch file...
>
> (1) the description
> (2) o

Re: What's cooking in git.git (Apr 2014, #09; Tue, 29)

2014-05-07 Thread John Keeping
On Wed, May 07, 2014 at 03:26:15PM -0500, Felipe Contreras wrote:
> Junio C Hamano wrote:
> > Your git-integrate might turn into something I could augment my
> > workflow with with some additions.
> > 
> >  - specifying a merge strategy per branch being merged;
> 
> git-reintegrate[1] supports this.
> 
> >  - support evil merges or picking a fix-up commit;
> 
> git-reintegrate supports this.
> 
> >  - leaving an empty commit only to leave comment in the history.
> 
> Done[2].
> 
> 
> > and until that happens, I'll keep using the Reintegrate script found
> > in my 'todo' branch.
> 
> My git-reintegrate supports everything John's git-integrate and in
> addition it supports generating the commands from an existing branch,
> like your Reintegrate. IOW; it's superior.

And yet the documentation is unchanged from the version you copied in
from git-integration.  Personally I would much rather use a project
which takes time to document all of the features rather than relying on
reading the code to figure out the options.

More features does not make a project superior.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] pager: remove 'S' from $LESS by default

2014-05-07 Thread Junio C Hamano
Matthieu Moy  writes:

> Perhaps it deserves a mention in the doc, e.g. squashing this on top of
> my patch:

Thanks, will do.

>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index b7f92ac..ebd1676 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -570,7 +570,9 @@ command to `LESS=FRX less -S`. The environment does not 
> set the
>  long lines. Similarly, setting `core.pager` to `less -+F` will
>  deactivate the `F` option specified by the environment from the
>  command-line, deactivating the "quit if one screen" behavior of
> -`less`.
> +`less`.  One can specifically activate some flags for particular
> +commands: for example, setting `pager.blame` to `less -S` enables
> +line truncation only for `git blame`.
>  +
>  Likewise, when the `LV` environment variable is unset, Git sets it
>  to `-c`.  You can override this setting by exporting `LV` with
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Windows: Always normalize paths to Windows-style

2014-05-07 Thread Junio C Hamano
Heiko Voigt  writes:

> On Mon, Apr 28, 2014 at 04:29:31PM +0200, Stepan Kasal wrote:
>> this is another patch that lives in msysGit for a long time.
>> Originally, it had two parts:
>> (Cf https://github.com/msysgit/git/commit/64a8a03 )
>> 
>> 1) adding alias pwd='pwd -W' to git-sh-setup.sh
>>This one went upstream, though as a shell function.
>> 
>> 2) revert of commit 4dce7d9b by Johannes Sixt 
>> This mingw-specific commit was created less than 3 weeks before
>> it was reverted.  And it stayed reverted for two years.
>> 
>> Could you please either accept this patch, or revert 4dce7d9b ?
>> (Both alternatives are exactly the same.)
>
> Sorry for the late reply.  To me reverting (or omitting at the next
> rebasing merge) this patch sound fine, as it seems to be superseeded by
> the upstream change.
>
> As I can see thats already done on master, so it seems to be all good.

Are you guys talking about be39048a (git-sh-setup.sh: Add an pwd()
function for MinGW, 2012-04-17) which has been in since v1.7.11?

The change introduced by 4dce7d9b (submodules: fix ambiguous
absolute paths under Windows, 2012-03-04) still exists, but your
"reverting this patch sound fine" confuses me.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Apr 2014, #09; Tue, 29)

2014-05-07 Thread Felipe Contreras
Junio C Hamano wrote:
> That "when I manually" part is what I meant by "we give a good way for
> these third-party tools" above, and "make it really easy to install
> these third-party tools" in the remaining part of the message you are
> responding to.

We need two things:

  1) Provied a pkg-config, as all sane shared components do
  2) Split the testing framework so third-parties don't have to rely on
 yet another third-parth (shareness)

> Your git-integrate might turn into something I could augment my
> workflow with with some additions.
> 
>  - specifying a merge strategy per branch being merged;

git-reintegrate[1] supports this.

>  - support evil merges or picking a fix-up commit;

git-reintegrate supports this.

>  - leaving an empty commit only to leave comment in the history.

Done[2].


> and until that happens, I'll keep using the Reintegrate script found
> in my 'todo' branch.

My git-reintegrate supports everything John's git-integrate and in
addition it supports generating the commands from an existing branch,
like your Reintegrate. IOW; it's superior.

[1] https://github.com/felipec/git-reintegrate
[2] 
https://github.com/felipec/git-reintegrate/commit/332412470c6e084f10ac2f8dc11e86ab4680974a

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] Silence a bunch of format-zero-length warnings

2014-05-07 Thread Junio C Hamano
Heiko Voigt  writes:

> On Wed, May 07, 2014 at 11:19:09AM -0700, Junio C Hamano wrote:
>> Jeff King  writes:
>> ...
>> > Yeah, this started last summer when we added __attribute__((format)) to
>> > the status_printf_ln calls, and I posted essentially the same patch.  We
>> > kind of waffled between "eh, just set -Wno-format-zero-length" and doing
>> > something, and ended up at the former. I'd be fine with doing it this
>> > way; we're not likely to add a lot of new callsites that would make it a
>> > hassle to keep up with.
>> 
>> OK, so I'll take it as your Ack ;-)
>
> What happened to this patch? These warnings are still annoying me on my
> Ubuntu 14.04.

As Peff summarized, an earlier patch was dropped with "It is easy
for builders with such compilers to squelch the warning."  We are
reversing the course, with an updated log message like this ;-)

   The user have long been told to pass -Wno-format-zero-length, but a
   patch that avoids warning altogether is not too noisy, so let's do
   so.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: Beginner question on "Pull is mostly evil"

2014-05-07 Thread Jim Garrison
> -Original Message-
> From: Junio C Hamano
> Sent: Wednesday, May 07, 2014 1:16 PM
> Subject: Re: Beginner question on "Pull is mostly evil"
> 
> No.  This is most often true for people who use a single repository as a
> place for everybody to meet, in the same way as SVN.
[snip lots of excellent detail]
> HTH.

Wow.  That helps tremendously, and should be incorporated somewhere in the
Git documentation.  Thank you for your immensely detailed response.

Apologies about not breaking lines... I'll remember that in future.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] remote-hg: more improvements

2014-05-07 Thread Junio C Hamano
Felipe Contreras  writes:

> Junio C Hamano wrote:
>> Felipe Contreras  writes:
>> 
>> > Here's a bunch of tests more, and a fixes for Mercurial v3.0.
>> 
>> I think the discussion with John Keeping hints that we shouldn't be
>> rushing fc/remote-helpers-hg-bzr-graduation
>
> Really? Based on what reasoning? I have proven his reasoning to be
> basically wrong.

Perhaps s/proven/convinced myself only/; you didn't prove it to me
and I doubt you proved it to John.

>> For example, I see
>> 
>>  from mercurial import changegroup
>> if check_version(3, 0):
>>  cg = changegroup.getbundle(...)
>>  else:
>>  cg = repo.getbundle(...)
>> 
>> and offhand it was unclear if the unconditional import was a
>> mistake.
>
> Of course it wasn't a mistake.

I doubt about the "Of course" part.  The first reaction after seeing
that the new "changegroup" is used only inside check_version(3,0)
and nowhere else was to wonder if that import is necessary (or even
safe) for the pre-v3.0 versions.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Standardize python shebangs

2014-05-07 Thread Matthieu Moy
Felipe Contreras  writes:

> Matthieu Moy wrote:
>> Felipe Contreras  writes:
>> 
>> > It's better if all our scripts use the same '/usr/bin/env python'.
>> 
>> Why?
>> 
>> Using python2 for git_multimail.py is a deliberate decision:
>
> If you want to use python2, then use '/usr/bin/env python2'.

Err, yes, this is what the code does before your patch.

>> "The git-multimail project itself is currently hosted on GitHub:
>> 
>> https://github.com/mhagger/git-multimail
>> 
>> We use the GitHub issue tracker to keep track of bugs and feature
>> requests, and GitHub pull requests to exchange patches (though, if you
>> prefer, you can send patches via the Git mailing list with cc to me).
>> Please sign off your patches as per the Git project practice."
>> )
>
> If you are going to follow practices different than git.git, then
> multimail shouldn't live in 'contrib/'.

git-multimail is not the only part of git.git that has a separate
maintainer. Same goes for git-gui and gitk.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 0/2] add a reflog_exists and delete_reflog abstraction

2014-05-07 Thread Junio C Hamano
Michael Haggerty  writes:

> +1 Looks good to me.  Thanks!

Will queue with your Ack; thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Beginner question on "Pull is mostly evil"

2014-05-07 Thread Junio C Hamano
Jim Garrison  writes:

> During my initial self-education I came across the maxim "don't
> pull, fetch+merge instead" and have been doing that.  I think I
> followed most of the "pull is (mostly) evil" discussion but one
> facet still puzzles me: the idea that pull will do a merge "in the
> wrong direction" sometimes.

[administrivia: wrap your lines to reasonable length like ~70 cols,
please]

> Do I understand correctly that this occurs only in the presence of
> multiple remotes?

No.  This is most often true for people who use a single repository
as a place for everybody to meet, in the same way as SVN.

Suppose that that central repository has this history:

---o---o---A

which ends at commit A (time flows from left to right and each node
in the graph is a commit, lines between them indicating parent-child
relationship).

Then you clone it and work on your own commits, which leads you to
have this in *your* repository:

---o---o---A---B---C

Imagine your coworker did the same and built on top of A in *his*
repository this history in the meantime, and then pushed it to the
central repository:

---o---o---A---X---Y---Z

Now, if you "git push" at this point, beause your history that leads
to C lack X, Y and Z, it will fail.  You need to somehow make the
tip of your history a descendant of Z.

One way that "mostly evil" thread discusses is to "pull", which is
"fetch and then merge" (note that I am saying "don't pull, instead
fetch and merge" is not an advice to solve "pull is mostly evil"
issue at all).  If you fetch, your repository will have a history
like this:

---o---o---A---B---C
\
 X---Y---Z

And then if you did merge after that, while still on *your* branch,
i.e. C, you will create a merge M and make the history look like
this:

---o---o---A---B---C---M
\ /
 X---Y---Z

M is a descendant of Z, so you can push to update the central
repository.  Such a merge M does not lose any commit in both
histories, so in that sense it may not be wrong, but when people
would want to talk about "the authoritative canonical history that
is shared among the project participants", i.e. "the trunk", the way
they often use is to do:

$ git log --first-parent

For all other people who observed the central repository after your
coworker pushed Z but before you pushed M, the commit on the trunk
used to be "o-o-A-X-Y-Z".  But because you made M while you were on
C, M's first parent is C, so by pushing M to advance the central
repository, you made X-Y-Z a side branch, not on the trunk.

You would rather want to have a history of this shape:

---o---o---A---X---Y---Z---M'
\ / 
 B---C

so that in the first-parent chain, it is clear that the project
first did X and then Y and then Z and merged a change that consists
of two commits B and C that achieves a single goal.  You may have
worked on fixing the bug #12345 with these two patches, and the
merge M' with swapped parents can say in its log message "Merge
'fix-bug-12345'".

Note that I said "achieves a single goal" above, because this is
important.  "swapping the merge order" only covers a special case
where the project does not care too much about having unrelated
things done on a single merge but cares a lot about first-parent
chain.

There are multiple schools of thought about the "trunk" management.

 1. Some projects want to keep a completely linear history without
any merges.  Obviously, swapping the merge order would not help
their taste.  You would need to flatten your history on top of
the updated upstream to result in a history of this shape
instead:

---o---o---A---X---Y---Z---B---C

with "git pull --rebase" or something.

 2. Some projects tolerate merges in their history, but do not worry
too much about the first-parent order, and allows fast-forward
merges.  To them, swapping the merge order does not hurt, but
it is unnecessary.

 3. Some projects want each commit on the "trunk" to do one single
thing.  The output of "git log --first-parent" in such a project
would show either a merge of a side branch that completes a
single theme, or a single commit that completes a single theme
by itself.  If your two commits B and C (or they may even be two
groups of commits) were solving two independent issues, then the
merge M' we made in the earlier example by swapping the merge
order is still not up to the project standard.  It merges two
unrelated efforts B and C at the same time.

For projects in the last category (git itself is one of them),
individual developers would want to prepare a history more like
this:

 C0--C1--C2 topic-c
/
---o---o---Amaster
\
 B0--B1--B2 topic-b

That is, keeping separate topics on separate branches, perhaps like
so:

$ git clone $U

Re: Re: [PATCH 3/3] Silence a bunch of format-zero-length warnings

2014-05-07 Thread Heiko Voigt
On Wed, May 07, 2014 at 11:19:09AM -0700, Junio C Hamano wrote:
> Jeff King  writes:
> 
> > On Sun, May 04, 2014 at 07:01:22PM +, brian m. carlson wrote:
> >
> >> On Sun, May 04, 2014 at 01:12:55AM -0500, Felipe Contreras wrote:
> >> > This is in gcc 4.9.0:
> >> > 
> >> >   wt-status.c: In function ‘wt_status_print_unmerged_header’:
> >> >   wt-status.c:191:2: warning: zero-length gnu_printf format string 
> >> > [-Wformat-zero-length]
> >> > status_printf_ln(s, c, "");
> >> > ^
> >> > 
> >> > We could pass -Wno-format-zero-length, but it seems compiler-specific
> >> > flags are frowned upon, so let's just avoid the warning altogether.
> >> 
> >> I believe these warnings existed before GCC 4.9 as well, but I'm not
> >> opposed to the change.
> >
> > Yeah, this started last summer when we added __attribute__((format)) to
> > the status_printf_ln calls, and I posted essentially the same patch.  We
> > kind of waffled between "eh, just set -Wno-format-zero-length" and doing
> > something, and ended up at the former. I'd be fine with doing it this
> > way; we're not likely to add a lot of new callsites that would make it a
> > hassle to keep up with.
> 
> OK, so I'll take it as your Ack ;-)

What happened to this patch? These warnings are still annoying me on my
Ubuntu 14.04.

Cheers Heiko
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Apr 2014, #09; Tue, 29)

2014-05-07 Thread Felipe Contreras
Greg Troxel wrote:
> In a packaging system, dependencies are much more troublesome.
> Dependencies have to be declared, and the build limited to use only
> those declared dependencies, in order to get repeatable builds and
> binary packages that can be used on other systems.  Dependencies that
> really are required are fine.  But optional dependencies are a
> problem, because e.g. one doesn't want to require the presence of qt
> to build something (that isn't already enormous).   So if git needs
> mercurial and subversion installed, plus perhaps 5 other things for
> less popular remote helpers, that starts to be a real burden.

It doesn't *need* them to build. The Mercurial/Bazaar dependencies are
optional, both at build-time and at run-time. Most distributions would
want to test the functionality they are distributing, and for testing
they do need these dependencies.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Apr 2014, #09; Tue, 29)

2014-05-07 Thread Felipe Contreras
John Keeping wrote:
> Having thought about it a bit more after reading Felipe's reply, it
> would be nice if there were some way for third-party tools to install
> HTML documentation without relying on `git --html-path` but I cannot
> see an obvious way to do that as there isn't a standard $HTML_PATH to
> match $MAN_PATH and $PATH.

Using `git --html-path` for that is wrong.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] completion: move out of contrib

2014-05-07 Thread Felipe Contreras
Junio C Hamano wrote:
> Felipe Contreras  writes:
> 
> > As an example of all the hacks needed by a real distribution package,
> > here's the stuff ArchLinux packagers have to do:
> >
> >   # bash completion
> >   mkdir -p "$pkgdir"/usr/share/bash-completion/completions/
> >   install -m644 ./contrib/completion/git-completion.bash 
> > "$pkgdir"/usr/share/bash-completion/completions/git
> >...
> >
> > And here's what debian packagers have to do:
> >
> >   # bash completion
> >   install -d -m0755 '$(GIT)'/etc/bash_completion.d
> >   install -m0644 contrib/completion/git-completion.bash \
> > '$(GIT)'/etc/bash_completion.d/git
> >...
> >

This is what the latest debian package does:

# bash completion
install -d -m0755 '$(GIT)'/usr/share/bash-completion/completions
install -m0644 contrib/completion/git-completion.bash \
  '$(GIT)'/usr/share/bash-completion/completions/git
ln -s git '$(GIT)'/usr/share/bash-completion/completions/gitk

> > If our build system was sane, they wouldn't need so many hacks.
> 
> I do not see how the above two examples lead to that conclusion.
> How would it help to blindly install to $(sharedir),

It is not blind, it is the location bash-completion uses *by default*,
and it's what most (all?) distributions use.

> or suggestion to use pkg-info when major distros do not even use one?

Which major distros do not ship with the pkg-config? It is part of
bash-completion (as it should be part of every decent shared softare
component), they all ship it.

Do you want me to go on a hunt and list all the distrubionts that ship
both?

  /usr/share/bash-completion/completions/git
  /usr/share/pkgconfig/bash-completion.pc

How many distributions would it take for you to accept the facts?

> I would understand if the saneness you seek were for distros to
> agree on where things should go, or at least agree on how to find
> out where things should go.

They all gree.

> I do not think we are there yet.

You are wrong.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Apr 2014, #09; Tue, 29)

2014-05-07 Thread John Keeping
On Wed, May 07, 2014 at 11:56:18AM -0700, Junio C Hamano wrote:
> John Keeping  writes:
> 
> > On Tue, May 06, 2014 at 05:01:59PM -0700, Junio C Hamano wrote:
> > ...
> >> Another thing to keep in mind is that we need to ensure that we give
> >> a good way for these third-party tools to integrate well with the
> >> core Git tools to form a single toolchest for the users.  I would
> >> love to be able to do
> >> 
> >> $ (cd git.git && make install)
> >> $ (cd git-imerge.git && make install)
> >> 
> >> and then say "git imerge", "git --help imerge", etc.  The same for
> >> the remote helpers that we may be splitting out of my tree into
> >> their own stand-alone projects.
> >
> > This can already work given suitable installation.  With
> > git-integration[1] I can type `git help integration` and it shows me the
> > man page in the same way that `git help commit` does.  When I manually
> > linked the HTML file to the right place `git help -w integration` worked
> > as well.
> 
> That "when I manually" part is what I meant by "we give a good way
> for these third-party tools" above, and "make it really easy to
> install these third-party tools" in the remaining part of the
> message you are responding to.
> 
> > I think this is enough...

Having thought about it a bit more after reading Felipe's reply, it
would be nice if there were some way for third-party tools to install
HTML documentation without relying on `git --html-path` but I cannot see
an obvious way to do that as there isn't a standard $HTML_PATH to match
$MAN_PATH and $PATH.

I've never tried `git help --info` until this thread, but I think we
could make some trivial improvements to that in order to support .info
documentation for third-party tools.

> The reason why I CC'ed Michael was primarily because I thought you
> were not one of those third-party tools maintainers (and secondarily
> I am a fairly big fan of imerge), but it is good to hear your
> opinion as another third-party provider.  Your git-integrate might
> turn into something I could augment my workflow with with some
> additions.  What is missing (I only read the full manual page at
> http://johnkeeping.github.io/git-integration/git-integration.html)
> to support my workflow seems to be:
> 
>  - specifying a merge strategy per branch being merged;

This is already supported by the "merge" instruction:

If any options are given after the ref (and on the same line)
then these are passed to git merge. This may be useful for
specifying an alternative merge strategy for a branch.

>  - support evil merges or picking a fix-up commit;

I have an implementation of this on a branch, but have never merged it
because it's not something I need to do often and it is very hard to
support for git-integration's "status" output.

One of my primary use cases for git-integration involves pulling
together branches owned by others (either in the same repository or by
having fetched from their repositories); in this case it is interesting
to see if/how a branch has changed since the last time the integration
branch was built.  This also handles changes to the instruction sheet
without an immediate rebuild.

I have not found a good way of figuring out whether a fixup commit has
been applied and squashed into a merge) so I have let the branch sit
there awaiting a perfect solution (which I doubt exists).  It may be
that the status of a fixup is unimportant, so it could just be marked as
unknown; I am mostly convinced that marking it as unknown is going to be
better than an heuristic that is right most of the time.

>  - leaving an empty commit only to leave comment in the history.

This would be easy to add.

> and until that happens, I'll keep using the Reintegrate script found
> in my 'todo' branch.

When I originally wrote git-integration I purposefully did not target
your workflow because I (perhaps wrongly) assumed that the interaction
between the different integration branches would mean that Git was
better served sticking to the custom Reintegrate script.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] t3910: show failure of core.precomposeunicode with decomposed filenames

2014-05-07 Thread Torsten Bögershausen
On 2014-05-05 23.46, Jeff King wrote:
[]
>>   2. Do all index filename comparisons under Mac OS X using a UTF-8 aware
>>  comparison function regardless if core.precomposeunicode is set.
>>  This would probably have bad performance, and somewhat
>>  defeats the point of converting the filenames at the
>>  readdir level in the first place.
> 
> Right, I'm concerned about performance here, but I wonder if we can
> reuse the name-hash solutions from ignorecase.
Some short question, (sorry being short)
What do you think about this:

diff --git a/compat/precompose_utf8.c b/compat/precompose_utf8.c
index 95fe849..c807f38 100644
--- a/compat/precompose_utf8.c
+++ b/compat/precompose_utf8.c
@@ -57,6 +57,22 @@ void probe_utf8_pathname_composition(char *path, int len)
 }
 
 
+char *inverscompose_str_len(const char *in, size_t insz, int *outsz)
+{
+   char *prec_str = NULL;
+
+   if (has_non_ascii(in, insz, NULL)) {
+   int old_errno = errno;
+   prec_str = reencode_string_len(in, (int)insz,
+  precomposed_unicode ? 
path_encoding : repo_encoding,
+  precomposed_unicode ? 
repo_encoding : path_encoding,
+  outsz);
+   errno = old_errno;
+   }
+   return prec_str;
+}
+
+
 void precompose_argv(int argc, const char **argv)
 {
int i = 0;
diff --git a/compat/precompose_utf8.h b/compat/precompose_utf8.h
index 3b73585..b9ac099 100644
--- a/compat/precompose_utf8.h
+++ b/compat/precompose_utf8.h
@@ -26,6 +26,7 @@ typedef struct {
struct dirent_prec_psx *dirent_nfc;
 } PREC_DIR;
 
+char *inverscompose_str_len(const char *in, size_t insz, int *outsz);
 void precompose_argv(int argc, const char **argv);
 void probe_utf8_pathname_composition(char *, int);
 
diff --git a/git-compat-util.h b/git-compat-util.h
index f6d3a46..5ae5f57 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -177,7 +177,7 @@ typedef unsigned long uintptr_t;
 #ifdef PRECOMPOSE_UNICODE
 #include "compat/precompose_utf8.h"
 #else
-#define precompose_str(in,i_nfd2nfc)
+#define inverscompose_str_len(s,i,o) NULL
 #define precompose_argv(c,v)
 #define probe_utf8_pathname_composition(a,b)
 #endif
diff --git a/name-hash.c b/name-hash.c
index 97444d0..3c4a4ee 100644
--- a/name-hash.c
+++ b/name-hash.c
@@ -210,7 +210,7 @@ struct cache_entry *index_dir_exists(struct index_state 
*istate, const char *nam
return NULL;
 }
 
-struct cache_entry *index_file_exists(struct index_state *istate, const char 
*name, int namelen, int icase)
+static struct cache_entry *index_file_exists_int(struct index_state *istate, 
const char *name, int namelen, int icase)
 {
struct cache_entry *ce;
struct hashmap_entry key;
@@ -227,6 +227,25 @@ struct cache_entry *index_file_exists(struct index_state 
*istate, const char *na
return NULL;
 }
 
+
+struct cache_entry *index_file_exists(struct index_state *istate, const char 
*name, int namelen, int icase)
+{
+   struct cache_entry *ce;
+   ce = index_file_exists_int(istate, name, namelen, icase);
+   if (ce)
+   return ce;
+   else {
+   int prec_len;
+   char *prec_name = inverscompose_str_len(name, namelen, 
&prec_len);
+   if (prec_name) {
+   ce = index_file_exists_int(istate, prec_name, prec_len, 
icase);
+   free(prec_name);
+   }
+   }
+   return ce;
+}
+
+
 void free_name_hash(struct index_state *istate)
 {
if (!istate->name_hash_initialized)
diff --git a/t/t3910-mac-os-precompose.sh b/t/t3910-mac-os-precompose.sh
index e4ba601..4414658 100755
--- a/t/t3910-mac-os-precompose.sh
+++ b/t/t3910-mac-os-precompose.sh
@@ -140,6 +140,22 @@ test_expect_success "Add long precomposed filename" '
git add * &&
git commit -m "Long filename"
 '
+
+test_expect_success 'handle existing decomposed filenames' '
+   git checkout master &&
+   git reset --hard &&
+   git checkout -b exist_decomposed &&
+   echo content >"verbatim.$Adiarnfd" &&
+   git -c core.precomposeunicode=false add "verbatim.$Adiarnfd" &&
+   git commit -m "existing decomposed file" &&
+   echo \"verbatim.A\\314\\210\" >expect &&
+   git ls-files >actual &&
+   test_cmp expect actual &&
+   >expect &&
+   git status | grep verbatim || : >actual &&
+   test_cmp expect actual
+'
+
 # Test if the global core.precomposeunicode stops autosensing
 # Must be the last test case
 test_expect_success "respect git config --global core.precomposeunicode" '
diff --git a/utf8.c b/utf8.c
index 77c28d4..fb8a99a 100644
--- a/utf8.c
+++ b/utf8.c
@@ -519,7 +519,7 @@ char *reencode_string_iconv(const char *in, size_t insz, 
iconv_t conv, int *outs
char *out, *outpos;
iconv_ibp cp;
 
-   outsz = insz;
+   outsz = 3*insz; /* for decompose */

Re: [PATCH] Standardize python shebangs

2014-05-07 Thread Felipe Contreras
Matthieu Moy wrote:
> Felipe Contreras  writes:
> 
> > It's better if all our scripts use the same '/usr/bin/env python'.
> 
> Why?
> 
> Using python2 for git_multimail.py is a deliberate decision:

If you want to use python2, then use '/usr/bin/env python2'.

> "The git-multimail project itself is currently hosted on GitHub:
> 
> https://github.com/mhagger/git-multimail
> 
> We use the GitHub issue tracker to keep track of bugs and feature
> requests, and GitHub pull requests to exchange patches (though, if you
> prefer, you can send patches via the Git mailing list with cc to me).
> Please sign off your patches as per the Git project practice."
> )

If you are going to follow practices different than git.git, then
multimail shouldn't live in 'contrib/'. Junio is already thretening to
remove stuff out of contrib even when development behaves as the Git
core does, so it makes more sense for stuff that doesn't.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] remote-hg: more improvements

2014-05-07 Thread Felipe Contreras
Junio C Hamano wrote:
> Felipe Contreras  writes:
> 
> > Here's a bunch of tests more, and a fixes for Mercurial v3.0.
> 
> I think the discussion with John Keeping hints that we shouldn't be
> rushing fc/remote-helpers-hg-bzr-graduation

Really? Based on what reasoning? I have proven his reasoning to be
basically wrong.

> I'll queue this separately on a topic based on the tip of
> yesterday's master for now.
> 
> One thing I couldn't read from the proposed log messages was if this
> is meant to work with and tested with both v3.0 and pre-v3.0 Hg, or
> this is to request others who run pre-v3.0 Hg to test these changes.
> 
> For example, I see
> 
>   from mercurial import changegroup
> if check_version(3, 0):
>   cg = changegroup.getbundle(...)
>   else:
>   cg = repo.getbundle(...)
> 
> and offhand it was unclear if the unconditional import was a
> mistake.

Of course it wasn't a mistake.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (May 2014, #01; Tue, 6)

2014-05-07 Thread Junio C Hamano
Felipe Contreras  writes:

> Junio C Hamano wrote:
>> Felipe Contreras  writes:
>> > Plus this one which has been completely ignored:
>> >
>> >completion: move out of contrib
>> 
>> It is not about "ignored".  It is about running out of time before
>> concluding the day's integration.
>
> A comment doesn't require integration.

It requires time and attention that are limited commodities.
Moreover, I won't read or respond to pieces of e-mail in the order
they come in.

The topics listed in the What's cooking report is fixed at the end
of the day's integration cycle, as they are essentially output of
"log --first-parent master..pu | grep Merge".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Apr 2014, #09; Tue, 29)

2014-05-07 Thread Junio C Hamano
John Keeping  writes:

> On Tue, May 06, 2014 at 05:01:59PM -0700, Junio C Hamano wrote:
> ...
>> Another thing to keep in mind is that we need to ensure that we give
>> a good way for these third-party tools to integrate well with the
>> core Git tools to form a single toolchest for the users.  I would
>> love to be able to do
>> 
>> $ (cd git.git && make install)
>> $ (cd git-imerge.git && make install)
>> 
>> and then say "git imerge", "git --help imerge", etc.  The same for
>> the remote helpers that we may be splitting out of my tree into
>> their own stand-alone projects.
>
> This can already work given suitable installation.  With
> git-integration[1] I can type `git help integration` and it shows me the
> man page in the same way that `git help commit` does.  When I manually
> linked the HTML file to the right place `git help -w integration` worked
> as well.

That "when I manually" part is what I meant by "we give a good way
for these third-party tools" above, and "make it really easy to
install these third-party tools" in the remaining part of the
message you are responding to.

> I think this is enough...

Thanks.

The reason why I CC'ed Michael was primarily because I thought you
were not one of those third-party tools maintainers (and secondarily
I am a fairly big fan of imerge), but it is good to hear your
opinion as another third-party provider.  Your git-integrate might
turn into something I could augment my workflow with with some
additions.  What is missing (I only read the full manual page at
http://johnkeeping.github.io/git-integration/git-integration.html)
to support my workflow seems to be:

 - specifying a merge strategy per branch being merged;
 - support evil merges or picking a fix-up commit;
 - leaving an empty commit only to leave comment in the history.

and until that happens, I'll keep using the Reintegrate script found
in my 'todo' branch.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Windows: Always normalize paths to Windows-style

2014-05-07 Thread Heiko Voigt
Hi,

On Mon, Apr 28, 2014 at 04:29:31PM +0200, Stepan Kasal wrote:
> this is another patch that lives in msysGit for a long time.
> Originally, it had two parts:
> (Cf https://github.com/msysgit/git/commit/64a8a03 )
> 
> 1) adding alias pwd='pwd -W' to git-sh-setup.sh
>This one went upstream, though as a shell function.
> 
> 2) revert of commit 4dce7d9b by Johannes Sixt 
> This mingw-specific commit was created less than 3 weeks before
> it was reverted.  And it stayed reverted for two years.
> 
> Could you please either accept this patch, or revert 4dce7d9b ?
> (Both alternatives are exactly the same.)

Sorry for the late reply.  To me reverting (or omitting at the next
rebasing merge) this patch sound fine, as it seems to be superseeded by
the upstream change.

As I can see thats already done on master, so it seems to be all good.

Cheers Heiko
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [BUG?] Patches created with 'diff.noprefix=true' don't 'git apply'.

2014-05-07 Thread Junio C Hamano
Nathan Collins  writes:

> More concretely, what I had in mind was that if 'diff.noprefix=true'
> is set in the user's config, and the patch is in '-p0' format, then
> Git could suggest to the user that the 'diff.noprefix' setting *might*
> be causing them to generate '-p0' patches. If the user had in fact not
> generated the patch themselves, then they could safely ignore the
> suggestion.

Issuing a suggestion that can be ignored too often sounds like
crying wolf to train users to ignore all other more useful
suggestions, so I would advise to be very cautious before doing so.
If you cannot come up with a way to make the heuristics very sure
and foolproof, I do not think it would help more than it hurt.

One possibility I considered briefly before discarding it was to see
if the input is coming from a pipe *and* these configuration is set.

> But this may just be an overcomplicated solution to my and others'
> misuse of the 'diff.noprefix' option; see below.
>
>> So that is not workable.

Yes.

>> However, Before issuing this error message
>>
>>>   git -c diff.noprefix=true diff HEAD~ | git apply --reverse
>>>   error: Data/Function/Decorator/Memoizer/Unsafe.hs: No such file or 
>>> directory
>>
>> we _could_ check that there is Data/ directory in the target tree
>> the patch is being applied and suggest to:
>
> To clarify, the actual path was
>
>   src/Data/Function/Decorator/Memoizer/Unsafe.hs

Yeah, and I meant to suggest checking that path is plausible.  It
would not help when adding a new file, but we can issue the original
error message and that is perfectly readable, so catching only paths
that are modified or deleted and suggesting -p$n is still making
things better.

> 1. 'git apply' doesn't give a very helpful error message when the
> patch does not apply due to not being in '-p1' format.

That "Data/Function/..." path in the error message is helpful
enough, once you learn that -p$n strips the leading directory and
that -p1 is the default.  For new people, however, -p1 being the
default might be unexpected, but the learning curve is not that
steep, I would think.  So this is a one-time thing.

> 2. the 'diff.noprefix=true' option is used for two unrelated things in
> practice. One of them is related to diffing -- namely, making Git
> generate '-p0' patches -- and the other is unrelated to diffing --
> namely, users want file names that can be easily copied with
> double-click.

I do not share this observation, as I never double-click.  I let my
shell to complete instead ;-)

> For (2), the solution may be to add a separate
> 'diff.add-clickable-paths' option (probably there is a better name?
> 'diff.add-copyable-paths'? ...),...
> ...
> Concretely, if 'diff.add-clickable-paths' is set, then instead of e.g.
>
>   diff --git a/src/Data/Function/Decorator/Memoizer/Unsafe.hs
> b/src/Data/Function/Decorator/Memoizer
>   index 3ef17da..a0586d3 100644
>   --- a/src/Data/Function/Decorator/Memoizer/Unsafe.hs
>   +++ b/src/Data/Function/Decorator/Memoizer/Unsafe.hs

If you do something along that line, perhaps

Index: src/Data/Function/Decorator/Memoizer/Unsafe.hs
diff --git a/src/Data/Function/Decorator/Memoizer/Unsafe.hs ...
index 3ef17da..a0586d3 100644
--- a/src/Data/Function/Decorator/Memoizer/Unsafe.hs
+++ b/src/Data/Function/Decorator/Memoizer/Unsafe.hs

to imitate what "cvs diff" does may be more familar to people.

What would you propose to make clickable in a renaming diff, though?


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] merge-recursive.c: Fix case-changing merge.

2014-05-07 Thread Jonathan Nieder
Junio C Hamano wrote:
> dtur...@twopensource.com writes:

>> +test -e testcase
>
> Please make it a habit to use "test -f" when you expect "the path
> exists as a file", not merely "something exists there I do not care
> if it is a file or a directory", for which "test -e" is perfectly
> appropriate.

Or, better in tests,

test_path_is_file testcase

which prints an error instead of just silently failing when
the path is not a file.

Thanks,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] Silence a bunch of format-zero-length warnings

2014-05-07 Thread Junio C Hamano
Jeff King  writes:

> On Sun, May 04, 2014 at 07:01:22PM +, brian m. carlson wrote:
>
>> On Sun, May 04, 2014 at 01:12:55AM -0500, Felipe Contreras wrote:
>> > This is in gcc 4.9.0:
>> > 
>> >   wt-status.c: In function ‘wt_status_print_unmerged_header’:
>> >   wt-status.c:191:2: warning: zero-length gnu_printf format string 
>> > [-Wformat-zero-length]
>> > status_printf_ln(s, c, "");
>> > ^
>> > 
>> > We could pass -Wno-format-zero-length, but it seems compiler-specific
>> > flags are frowned upon, so let's just avoid the warning altogether.
>> 
>> I believe these warnings existed before GCC 4.9 as well, but I'm not
>> opposed to the change.
>
> Yeah, this started last summer when we added __attribute__((format)) to
> the status_printf_ln calls, and I posted essentially the same patch.  We
> kind of waffled between "eh, just set -Wno-format-zero-length" and doing
> something, and ended up at the former. I'd be fine with doing it this
> way; we're not likely to add a lot of new callsites that would make it a
> hassle to keep up with.

OK, so I'll take it as your Ack ;-)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] remote-hg: more improvements

2014-05-07 Thread Junio C Hamano
Felipe Contreras  writes:

> Here's a bunch of tests more, and a fixes for Mercurial v3.0.

I think the discussion with John Keeping hints that we shouldn't be
rushing fc/remote-helpers-hg-bzr-graduation and this does not appear
to assume the presense of that series, which is good in order to
make these fixes jump over them.

I'll queue this separately on a topic based on the tip of
yesterday's master for now.

One thing I couldn't read from the proposed log messages was if this
is meant to work with and tested with both v3.0 and pre-v3.0 Hg, or
this is to request others who run pre-v3.0 Hg to test these changes.

For example, I see

from mercurial import changegroup
if check_version(3, 0):
cg = changegroup.getbundle(...)
else:
cg = repo.getbundle(...)

and offhand it was unclear if the unconditional import was a
mistake.

Thanks.

>
>
> Felipe Contreras (4):
>   remote-hg: add more tests
>   t: remote-hg: add file operation tests
>   t: remote-hg: trivial cleanups and fixes
>   remote-hg: add support for hg v3.0
>
>  contrib/remote-helpers/git-remote-hg |   6 +-
>  contrib/remote-helpers/test-hg.sh| 240 
> ++-
>  2 files changed, 238 insertions(+), 8 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Standardize python shebangs

2014-05-07 Thread Matthieu Moy
Felipe Contreras  writes:

> It's better if all our scripts use the same '/usr/bin/env python'.

Why?

Using python2 for git_multimail.py is a deliberate decision:

https://github.com/mhagger/git-multimail/pull/2

(also, contrib/hooks/multimail/README says:

"The git-multimail project itself is currently hosted on GitHub:

https://github.com/mhagger/git-multimail

We use the GitHub issue tracker to keep track of bugs and feature
requests, and GitHub pull requests to exchange patches (though, if you
prefer, you can send patches via the Git mailing list with cc to me).
Please sign off your patches as per the Git project practice."
)

> Signed-off-by: Felipe Contreras 
> ---
>  contrib/hooks/multimail/README  | 6 +++---
>  contrib/hooks/multimail/git_multimail.py| 2 +-
>  contrib/hooks/multimail/migrate-mailhook-config | 2 +-
>  contrib/hooks/multimail/post-receive| 2 +-
>  contrib/svn-fe/svnrdump_sim.py  | 2 +-
>  5 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/contrib/hooks/multimail/README b/contrib/hooks/multimail/README
> index 477d65f..cf0bcb8 100644
> --- a/contrib/hooks/multimail/README
> +++ b/contrib/hooks/multimail/README
> @@ -76,10 +76,10 @@ Requirements
>The example scripts invoke Python using the following shebang line
>(following PEP 394 [1]):
>  
> -  #! /usr/bin/env python2
> +  #! /usr/bin/env python
>  
> -  If your system's Python2 interpreter is not in your PATH or is not
> -  called "python2", you can change the lines accordingly.  Or you can
> +  If your system's Python interpreter is not in your PATH or is not
> +  called "python", you can change the lines accordingly.  Or you can
>invoke the Python interpreter explicitly, for example via a tiny
>shell script like
>  
> diff --git a/contrib/hooks/multimail/git_multimail.py 
> b/contrib/hooks/multimail/git_multimail.py
> index 8b58ed6..f6dcdc6 100755
> --- a/contrib/hooks/multimail/git_multimail.py
> +++ b/contrib/hooks/multimail/git_multimail.py
> @@ -1,4 +1,4 @@
> -#! /usr/bin/env python2
> +#! /usr/bin/env python
>  
>  # Copyright (c) 2012-2014 Michael Haggerty and others
>  # Derived from contrib/hooks/post-receive-email, which is
> diff --git a/contrib/hooks/multimail/migrate-mailhook-config 
> b/contrib/hooks/multimail/migrate-mailhook-config
> index 04eeaac..fba0b90 100755
> --- a/contrib/hooks/multimail/migrate-mailhook-config
> +++ b/contrib/hooks/multimail/migrate-mailhook-config
> @@ -1,4 +1,4 @@
> -#! /usr/bin/env python2
> +#! /usr/bin/env python
>  
>  """Migrate a post-receive-email configuration to be usable with 
> git_multimail.py.
>  
> diff --git a/contrib/hooks/multimail/post-receive 
> b/contrib/hooks/multimail/post-receive
> index 4d46828..4f2cf9d 100755
> --- a/contrib/hooks/multimail/post-receive
> +++ b/contrib/hooks/multimail/post-receive
> @@ -1,4 +1,4 @@
> -#! /usr/bin/env python2
> +#! /usr/bin/env python
>  
>  """Example post-receive hook based on git-multimail.
>  
> diff --git a/contrib/svn-fe/svnrdump_sim.py b/contrib/svn-fe/svnrdump_sim.py
> index 4e78a1c..bf20819 100755
> --- a/contrib/svn-fe/svnrdump_sim.py
> +++ b/contrib/svn-fe/svnrdump_sim.py
> @@ -1,4 +1,4 @@
> -#!/usr/bin/python
> +#!/usr/bin/env python
>  """
>  Simulates svnrdump by replaying an existing dump from a file, taking care
>  of the specified revision range.



-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] merge-recursive.c: Fix case-changing merge.

2014-05-07 Thread Junio C Hamano
dtur...@twopensource.com writes:

> +if ! test_have_prereq CASE_INSENSITIVE_FS
> +then
> + skip_all='skipping case insensitive tests - case sensitive file system'
> + test_done
> +fi
> +
> +test_expect_success 'merge with case-changing rename' '
> + test $(git config core.ignorecase) = true &&

This check seems a bit strange.  You already know you are on a case
insensitive filesystem, so I would understand that if you assume it
is set, or if you make sure it is set (if you are really paranoid).

But I'll let it pass, as it is not wrong per-se.  Just looked strange.

> + > TestCase &&

Please have no SP between redirection operator and its target.

> + test -e testcase

Please make it a habit to use "test -f" when you expect "the path
exists as a file", not merely "something exists there I do not care
if it is a file or a directory", for which "test -e" is perfectly
appropriate.

I'll fix up locally before queuing; no need to resend.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] ignorecase: Fix git mv on insensitive filesystems

2014-05-07 Thread David Turner
On Wed, 2014-05-07 at 10:46 -0700, Junio C Hamano wrote:
> David Turner  writes:
> 
> > On Wed, 2014-05-07 at 08:17 +0200, Johannes Sixt wrote:
> >> >  } else if (cache_name_pos(src, length) < 0)
> >> >  bad = _("not under version control");
> >> > -else if (lstat(dst, &st) == 0) {
> >> > +else if (lstat(dst, &dst_st) == 0 &&
> >> > + (src_st.st_ino != dst_st.st_ino ||
> >> > +  (src_st.st_ino == 0 && strcasecmp(src, 
> >> > dst {
> >> 
> >> Don't do that. st_ino is zero on Windows only because we do not spend time
> >> to fill in the field. Don't use it as an indicator for a case-insensitive
> >> file system; zero may be a valid inode number on other systems.
> >
> > I don't think it is a problem if zero is a valid inode.  The only thing
> > that happens when there is a zero inode, is that we have to compare
> > filenames.  The inode check is just an optimization to avoid doing a
> > bunch of strcasecmp on systems that don't have to.
> 
> Am I correct to rephrase you that the code assumes that any
> filesystem that cannot give unique inum to different files will use
> 0 as the placeholder inum, so if src/dst share the same non-zero
> inum, it is guaranteed that is not a placeholder and we know they
> are different files without comparing the two paths?

Yes, this is indeed a fair rephrasing.  In fact, the entire zero-check
should not be necessary, as POSIX requires that the st_ino field has a
"meaningful" value.  So in the case that this ever runs into a problem,
we ought to wrap the lstat call with a compatibility layer anyway. 

But maybe there is an OS I'm not thinking of which fills in st_ino with
something else?

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] pager: remove 'S' from $LESS by default

2014-05-07 Thread Matthieu Moy
Junio C Hamano  writes:

> While I fully agree with the above conclusion, I just noticed that I
> will be irritated enough to eventually set pager.blame myself, after
> running a short "git blame -L1311,+7 git-p4.py", which is one of the
> standard first steps for me to start reading patches submit on the
> list.

Perhaps it deserves a mention in the doc, e.g. squashing this on top of
my patch:

diff --git a/Documentation/config.txt b/Documentation/config.txt
index b7f92ac..ebd1676 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -570,7 +570,9 @@ command to `LESS=FRX less -S`. The environment does not set 
the
 long lines. Similarly, setting `core.pager` to `less -+F` will
 deactivate the `F` option specified by the environment from the
 command-line, deactivating the "quit if one screen" behavior of
-`less`.
+`less`.  One can specifically activate some flags for particular
+commands: for example, setting `pager.blame` to `less -S` enables
+line truncation only for `git blame`.
 +
 Likewise, when the `LV` environment variable is unset, Git sets it
 to `-c`.  You can override this setting by exporting `LV` with

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] ignorecase: Fix git mv on insensitive filesystems

2014-05-07 Thread Junio C Hamano
David Turner  writes:

> On Wed, 2014-05-07 at 08:17 +0200, Johannes Sixt wrote:
>> >} else if (cache_name_pos(src, length) < 0)
>> >bad = _("not under version control");
>> > -  else if (lstat(dst, &st) == 0) {
>> > +  else if (lstat(dst, &dst_st) == 0 &&
>> > +   (src_st.st_ino != dst_st.st_ino ||
>> > +(src_st.st_ino == 0 && strcasecmp(src, dst {
>> 
>> Don't do that. st_ino is zero on Windows only because we do not spend time
>> to fill in the field. Don't use it as an indicator for a case-insensitive
>> file system; zero may be a valid inode number on other systems.
>
> I don't think it is a problem if zero is a valid inode.  The only thing
> that happens when there is a zero inode, is that we have to compare
> filenames.  The inode check is just an optimization to avoid doing a
> bunch of strcasecmp on systems that don't have to.

Am I correct to rephrase you that the code assumes that any
filesystem that cannot give unique inum to different files will use
0 as the placeholder inum, so if src/dst share the same non-zero
inum, it is guaranteed that is not a placeholder and we know they
are different files without comparing the two paths?

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/3] After chdir to run grep, return to old directory

2014-05-07 Thread Junio C Hamano
David Turner  writes:

> This causes my test to pass and generally seems correct to me.

Yes, this approach is very sensible, and I'll queue.

But watchman support _should_ be prepared for a program that does
not do this.  Developing your support in on a codebase with this
patch may be sweeping a bug in your code under the rug.

Thanks both for working on your respective changes ;-).

>
> On Tue, 2014-05-06 at 23:00 -0400, Jeff King wrote:
> ...
>> That being said, this really seems like something that the run-command
>> interface should be doing, since it can handle the chdir in the forked
>> child. And indeed, it seems to support that.
>> 
>> Maybe:
>> 
>> -- >8 --
>> Subject: grep: use run-command's "dir" option for --open-files-in-pager
>> 
>> Git generally changes directory to the repository root on
>> startup.  When running "grep --open-files-in-pager" from a
>> subdirectory, we chdir back to the original directory before
>> running the pager, so that we can feed the relative
>> pathnames to the pager.
>> 
>> We currently do this chdir manually, but we can ask
>> run_command to do it for us. This is fewer lines of code,
>> and as a bonus, the chdir is limited to the child process,
>> which avoids any unexpected surprises for code running after
>> the pager (there isn't any currently, but this is
>> future-proofing).
>> 
>> Signed-off-by: Jeff King 
>> ---
>>  builtin/grep.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>> 
>> diff --git a/builtin/grep.c b/builtin/grep.c
>> index 69ac2d8..43af5b7 100644
>> --- a/builtin/grep.c
>> +++ b/builtin/grep.c
>> @@ -361,9 +361,7 @@ static void run_pager(struct grep_opt *opt, const char 
>> *prefix)
>>  argv[i] = path_list->items[i].string;
>>  argv[path_list->nr] = NULL;
>>  
>> -if (prefix && chdir(prefix))
>> -die(_("Failed to chdir: %s"), prefix);
>> -status = run_command_v_opt(argv, RUN_USING_SHELL);
>> +status = run_command_v_opt_cd_env(argv, RUN_USING_SHELL, prefix, NULL);
>>  if (status)
>>  exit(status);
>>  free(argv);
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] completion: move out of contrib

2014-05-07 Thread Junio C Hamano
Felipe Contreras  writes:

> As an example of all the hacks needed by a real distribution package,
> here's the stuff ArchLinux packagers have to do:
>
>   # bash completion
>   mkdir -p "$pkgdir"/usr/share/bash-completion/completions/
>   install -m644 ./contrib/completion/git-completion.bash 
> "$pkgdir"/usr/share/bash-completion/completions/git
>...
>
> And here's what debian packagers have to do:
>
>   # bash completion
>   install -d -m0755 '$(GIT)'/etc/bash_completion.d
>   install -m0644 contrib/completion/git-completion.bash \
> '$(GIT)'/etc/bash_completion.d/git
>...
>
> If our build system was sane, they wouldn't need so many hacks.

I do not see how the above two examples lead to that conclusion.
How would it help to blindly install to $(sharedir), or suggestion
to use pkg-info when major distros do not even use one?

I would understand if the saneness you seek were for distros to
agree on where things should go, or at least agree on how to find
out where things should go.

The situation for completion scripts unfortunately has to be even
worse than others (like gitweb) in that distros, in order to serve
users (not upstream projects) better, want to package various pieces
of completion scripts from different projects into a location their
bash-completion package wants to see.  I applaud their effort and I
wish that projects they pull various completion scriptlets from can
do more to help them, but I do not think a single makefile target to
unconditionally install to $(sharedir) is it, and honestly speaking,
the distro completion packagers could do more to help us projects to
help them by at least agreeing on how to find out where they want to
see our completion scripts installed.

I do not think we are there yet.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] git-p4: format-patch to diff-tree change breaks binary patches

2014-05-07 Thread Junio C Hamano
Tolga Ceylan  writes:

> When applying binary patches a full index is required. format-patch
> already handles this, but diff-tree needs '--full-index' argument
> to always output full index. When git-p4 runs git-apply to test
> the patch, git-apply rejects the patch due to abbreviated blob
> object names. This is the error message git-apply emits in this
> case:
>
> error: cannot apply binary patch to '' without full index line
> error: : patch does not apply
>
> Signed-off-by: Tolga Ceylan 
> Acked-by: Pete Wyckoff 
> ---

Because the original breakage was already in 1.9, not a regression
between 1.9 and master, as the matter of principle our default is to
defer until 2.0 final to avoid risking unintended additional
breakages elsewhere.  But this fix is an obviously correct and
trivial single liner that were eyeballed by more than one person,
and that affects only three calls to os.system(), and its
correctness can be seen even without knowing p4 at all by somebody
like me ;-)

So let's queue it for 2.0.

Thanks.

>  git-p4.py |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index cdfa2df..4ee6739 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -1311,7 +1311,7 @@ class P4Submit(Command, P4UserMap):
>  else:
>  die("unknown modifier %s for %s" % (modifier, path))
>  
> -diffcmd = "git diff-tree -p \"%s\"" % (id)
> +diffcmd = "git diff-tree --full-index -p \"%s\"" % (id)
>  patchcmd = diffcmd + " | git apply "
>  tryPatchCmd = patchcmd + "--check -"
>  applyPatchCmd = patchcmd + "--check --apply -"
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] pager: remove 'S' from $LESS by default

2014-05-07 Thread Junio C Hamano
Jeff King  writes:

> On Tue, May 06, 2014 at 08:49:22PM +0200, Matthieu Moy wrote:
> ...
>> The idea of having a separate default value for pager.blame (or set
>> $LESS differently for blame) crossed my mind, but I actually don't like
>> it, as it would make it harder for a user to fine-tune his configuration
>> manually (one would have to cancel all the corner-cases that Git would
>> set by default).
>
> Agreed. We already get some confusion from users with "git has set $LESS
> for me".  Changing it to "git set up $LESS depending on which command is
> running" seems like it would cause more of the same.

While I fully agree with the above conclusion, I just noticed that I
will be irritated enough to eventually set pager.blame myself, after
running a short "git blame -L1311,+7 git-p4.py", which is one of the
standard first steps for me to start reading patches submit on the
list.

Even with line wrapping, the output fits on a single page, so "F"
(aka "--quit-if-one-screen") kicks in, before giving me a chance to
scroll horizontally around.

Not objecting to the conclusion of the discussion with a concrete
counterproposal.  Just hoping somebody clever enough might come up
with a good trick to make things easier to use and explain, which I
however suspect may be an incompatible pair of goals.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Beginner question on "Pull is mostly evil"

2014-05-07 Thread Jeff King
On Wed, May 07, 2014 at 03:40:28PM +, Jim Garrison wrote:

> During my initial self-education I came across the maxim "don't pull,
> fetch+merge instead" and have been doing that.  I think I followed
> most of the "pull is (mostly) evil" discussion but one facet still
> puzzles me: the idea that pull will do a merge "in the wrong
> direction" sometimes.
> 
> Do I understand correctly that this occurs only in the presence of
> multiple remotes?

No, it does not have to do with multiple remotes. It is about "X merged
into Y" versus "Y merged into X". The ordering of parents in a merge
doesn't matter for the merge result, but git must choose some order, and
it always uses your current HEAD first, and then the commit you are
merging second (and so on, in an octopus merge).

As a result, you can use "git log --first-parent" to follow the line of
development that always got merged into. In a strict topic-branch
workflow like git.git, this will show you just what happened on master:
a linear sequence of merges of topic branches, with occasional
direct-to-master commits like version bumps.

For an integrator who is pulling from other people, "git pull bob topic"
from "master" does the right thing: master is the first parent, and
topic is the second parent.

For somebody with a centralized repo who follows the "push was a
non-fastforward, so pull then push" advice, the merge between their work
and master will be "backwards". The merge commit will have upstream's
work (i.e., "master") merged into their topic. Following --first-parent
will walk down their work instead of the merge commits on master.

Does that explain it?

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] ignorecase: Fix git mv on insensitive filesystems

2014-05-07 Thread David Turner
On Wed, 2014-05-07 at 08:17 +0200, Johannes Sixt wrote:
> > } else if (cache_name_pos(src, length) < 0)
> > bad = _("not under version control");
> > -   else if (lstat(dst, &st) == 0) {
> > +   else if (lstat(dst, &dst_st) == 0 &&
> > +(src_st.st_ino != dst_st.st_ino ||
> > + (src_st.st_ino == 0 && strcasecmp(src, dst {
> 
> Don't do that. st_ino is zero on Windows only because we do not spend time
> to fill in the field. Don't use it as an indicator for a case-insensitive
> file system; zero may be a valid inode number on other systems.

I don't think it is a problem if zero is a valid inode.  The only thing
that happens when there is a zero inode, is that we have to compare
filenames.  The inode check is just an optimization to avoid doing a
bunch of strcasecmp on systems that don't have to.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Beginner question on "Pull is mostly evil"

2014-05-07 Thread David Kastrup
Jim Garrison  writes:

> During my initial self-education I came across the maxim "don't pull,
> fetch+merge instead" and have been doing that.  I think I followed
> most of the "pull is (mostly) evil" discussion but one facet still
> puzzles me: the idea that pull will do a merge "in the wrong
> direction" sometimes.
>
> Do I understand correctly that this occurs only in the presence of
> multiple remotes?
> Can someone provide a simple example of a situation where pull would
> do the "wrong" thing?

That's basically unavoidable.  Two opposing directions are actually part
of the same workflow usually handled by "git pull":

"Codeveloper X sends a pull request to Y who maintains the mainline.
Y executes git pull to merge X' sidebranch into the mainline."

"Codeveloper X executes git pull in order to merge the mainline from Y
back into his private sidebranch."

-- 
David Kastrup
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Beginner question on "Pull is mostly evil"

2014-05-07 Thread Jim Garrison
During my initial self-education I came across the maxim "don't pull, 
fetch+merge instead" and have been doing that.  I think I followed most of the 
"pull is (mostly) evil" discussion but one facet still puzzles me: the idea 
that pull will do a merge "in the wrong direction" sometimes.

Do I understand correctly that this occurs only in the presence of multiple 
remotes?  
Can someone provide a simple example of a situation where pull would do the 
"wrong" thing?

Thanks

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: material for git training sessions/presentations

2014-05-07 Thread Sitaram Chamarty

On 05/05/2014 09:48 AM, Chris Packham wrote:

Hi,

I know there are a few people on this list that do git training in
various forms. At $dayjob I've been asked to run a few training
sessions in house. The initial audience is SW developers so they are
fairly clued up on VCS concepts and most have some experience
(although some not positive) with git. Eventually this may also
include some QA folks who are writing/maintaining test suites who
might be less clued up on VCSes in general.

I know if I googled for git tutorials I'll find a bunch and I can
probably write a few myself but does anyone have any advice from
training sessions they've run about how best to present the subject
matter. Particularly to a fairly savy audience who may have developed
some bad habits. My plan was to try and have a few PCs/laptops handy
and try to make it a little interactive.

Also if anyone has any presentations I could use under a CC-BY-SA (or
other liberal license) as a basis for any material I produce that
would save me starting from scratch.


I've written and used the following; the first one is a bit more popular
(or at least has been mentioned several times on #git)

1. git concepts simplified: http://gitolite.com/gcs.html
2. a presentation on git: http://gitolite.com/git.html

You can use them straight off the web.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 20/20] t7063: tests for untracked cache

2014-05-07 Thread Nguyễn Thái Ngọc Duy
---
 .gitignore |   1 +
 Makefile   |   1 +
 t/t7063-status-untracked-cache.sh (new +x) | 352 +
 test-dump-untracked-cache.c (new)  |  61 +
 4 files changed, 415 insertions(+)
 create mode 100755 t/t7063-status-untracked-cache.sh
 create mode 100644 test-dump-untracked-cache.c

diff --git a/.gitignore b/.gitignore
index dc600f9..7f3bfdb 100644
--- a/.gitignore
+++ b/.gitignore
@@ -180,6 +180,7 @@
 /test-date
 /test-delta
 /test-dump-cache-tree
+/test-dump-untracked-cache
 /test-scrap-cache-tree
 /test-genrandom
 /test-hashmap
diff --git a/Makefile b/Makefile
index a53f3a8..d8a0482 100644
--- a/Makefile
+++ b/Makefile
@@ -553,6 +553,7 @@ TEST_PROGRAMS_NEED_X += test-ctype
 TEST_PROGRAMS_NEED_X += test-date
 TEST_PROGRAMS_NEED_X += test-delta
 TEST_PROGRAMS_NEED_X += test-dump-cache-tree
+TEST_PROGRAMS_NEED_X += test-dump-untracked-cache
 TEST_PROGRAMS_NEED_X += test-genrandom
 TEST_PROGRAMS_NEED_X += test-hashmap
 TEST_PROGRAMS_NEED_X += test-index-version
diff --git a/t/t7063-status-untracked-cache.sh 
b/t/t7063-status-untracked-cache.sh
new file mode 100755
index 000..bb5124b
--- /dev/null
+++ b/t/t7063-status-untracked-cache.sh
@@ -0,0 +1,352 @@
+#!/bin/sh
+
+test_description='test untracked cache'
+
+. ./test-lib.sh
+
+avoid_racy() {
+   sleep 1
+}
+
+git update-index --untracked-cache
+# It's fine if git update-index returns an error code other than one,
+# it'll be caught in the first test.
+if test $? -eq 1; then
+   skip_all='This system does not support untracked cache'
+   test_done
+fi
+
+test_expect_success 'setup' '
+   git init worktree &&
+   cd worktree &&
+   mkdir done dtwo dthree &&
+   touch one two three done/one dtwo/two dthree/three &&
+   git add one two done/one &&
+   : >.git/info/exclude &&
+   git update-index --untracked-cache
+'
+
+test_expect_success 'untracked cache is empty' '
+   test-dump-untracked-cache >../actual &&
+   cat >../expect <../status.expect <../dump.expect <../trace &&
+   GIT_TRACE_UNTRACKED="$TRASH_DIRECTORY/trace" \
+   git status --porcelain >../actual &&
+   test_cmp ../status.expect ../actual &&
+   cat >../trace.expect <../actual &&
+   test_cmp ../dump.expect ../actual
+'
+
+test_expect_success 'status second time (fully populated cache)' '
+   avoid_racy &&
+   : >../trace &&
+   GIT_TRACE_UNTRACKED="$TRASH_DIRECTORY/trace" \
+   git status --porcelain >../actual &&
+   test_cmp ../status.expect ../actual &&
+   cat >../trace.expect <../actual &&
+   test_cmp ../dump.expect ../actual
+'
+
+test_expect_success 'modify in root directory, one dir invalidation' '
+   avoid_racy &&
+   : >four &&
+   : >../trace &&
+   GIT_TRACE_UNTRACKED="$TRASH_DIRECTORY/trace" \
+   git status --porcelain >../actual &&
+   cat >../status.expect <../trace.expect <../actual &&
+   cat >../expect <.gitignore &&
+   : >../trace &&
+   GIT_TRACE_UNTRACKED="$TRASH_DIRECTORY/trace" \
+   git status --porcelain >../actual &&
+   cat >../status.expect <../trace.expect <../actual &&
+   cat >../expect <>.git/info/exclude &&
+   : >../trace &&
+   GIT_TRACE_UNTRACKED="$TRASH_DIRECTORY/trace" \
+   git status --porcelain >../actual &&
+   cat >../status.expect <../trace.expect <../actual &&
+   cat >../expect <../actual &&
+   cat >../expect <../trace &&
+   GIT_TRACE_UNTRACKED="$TRASH_DIRECTORY/trace" \
+   git status --porcelain >../actual &&
+   cat >../status.expect <../trace.expect <../actual &&
+   cat >../expect <../actual &&
+   cat >../expect <../trace &&
+   GIT_TRACE_UNTRACKED="$TRASH_DIRECTORY/trace" \
+   git status --porcelain >../actual &&
+   cat >../status.expect <../trace.expect <../actual &&
+   cat >../expect name);
+}
+
+static void dump(struct untracked_cache_dir *ucd, struct strbuf *base)
+{
+   int i, len;
+   qsort(ucd->untracked, ucd->untracked_nr, sizeof(*ucd->untracked),
+ compare_untracked);
+   qsort(ucd->dirs, ucd->dirs_nr, sizeof(*ucd->dirs),
+ compare_dir);
+   len = base->len;
+   strbuf_addf(base, "%s/", ucd->name);
+   printf("%s %s", base->buf,
+  sha1_to_hex(ucd->exclude_sha1));
+   if (ucd->recurse)
+   fputs(" recurse", stdout);
+   if (ucd->check_only)
+   fputs(" check_only", stdout);
+   if (ucd->valid)
+   fputs(" valid", stdout);
+   printf("\n");
+   for (i = 0; i < ucd->untracked_nr; i++)
+   printf("%s\n", ucd->untracked[i]);
+   for (i = 0; i < ucd->dirs_nr; i++)
+   dump(ucd->dirs[i], base);
+   strbuf_setlen(base, len);
+}
+
+int main(int ac, char **av)
+{
+   struct untracked_cache *uc;
+   struct strbuf base = STRBUF_INIT;
+   if (read_cache() < 0)
+   

[PATCH 18/20] update-index: manually enable or disable untracked cache

2014-05-07 Thread Nguyễn Thái Ngọc Duy
Some numbers below. In short, the saving on wt_status_collect_untracked()
is about 80%. There are some overhead on read/write_cache, but it seems
lower than 50ms, while the .._untracked() saving is in the 500ms range
(except linux-2.6, about 150ms). "git status" time saving ranges from
33% to 42%.

On gentoo-x86.git (100k files, 23k dirs, quite balance
tree, 8.5MB index v4, cache-tree fully populated), before turning
untracked cache on (the most important line is wt_status_collect:625)

   184.650 gitmodules_config:201 if (read_cache() < 0) die("index
 0.004 cmd_status:1299 read_cache_preload(&s.pathspec)
   226.231 cmd_status:1300 refresh_index(&the_index, REFRESH_QUIET
 3.096 cmd_status:1304 update_index_if_able(&the_index, &index_lock)
 6.788  wt_status_collect:619 wt_status_collect_changes_worktree(s)
 6.780  wt_status_collect:624 wt_status_collect_changes_index(s)
   772.866  wt_status_collect:625 wt_status_collect_untracked(s)
   786.686 cmd_status:1308 wt_status_collect(&s)

real0m1.211s
user0m0.566s
sys 0m0.638s

and after (saving 42% total time):

   220.888 gitmodules_config:201 if (read_cache() < 0) die("index
36.368 cmd_status:1299 read_cache_preload(&s.pathspec)
   223.936 cmd_status:1300 refresh_index(&the_index, REFRESH_QUIET
 2.935 cmd_status:1304 update_index_if_able(&the_index, &index_lock)
 7.031  wt_status_collect:619 wt_status_collect_changes_worktree(s)
 7.156  wt_status_collect:624 wt_status_collect_changes_index(s)
   148.022  wt_status_collect:625 wt_status_collect_untracked(s)
   162.443 cmd_status:1308 wt_status_collect(&s)
36.943 cmd_status:1348 if (the_index.untracked) { struct

real0m0.696s
user0m0.406s
sys 0m0.288s

On webkit.git (182k files, 6k dirs, 14M index v4), before:

   283.182 gitmodules_config:201 if (read_cache() < 0) die("index
 0.004 cmd_status:1299 read_cache_preload(&s.pathspec)
   515.836 cmd_status:1300 refresh_index(&the_index, REFRESH_QUIET
 5.428 cmd_status:1304 update_index_if_able(&the_index, &index_lock)
11.162  wt_status_collect:619 wt_status_collect_changes_worktree(s)
11.068  wt_status_collect:624 wt_status_collect_changes_index(s)
   887.247  wt_status_collect:625 wt_status_collect_untracked(s)
   909.722 cmd_status:1308 wt_status_collect(&s)

real0m1.729s
user0m0.785s
sys 0m0.941s

and after (saving 38% total time):

   290.994 gitmodules_config:201 if (read_cache() < 0) die("index
10.132 cmd_status:1299 read_cache_preload(&s.pathspec)
   516.656 cmd_status:1300 refresh_index(&the_index, REFRESH_QUIET
 5.159 cmd_status:1304 update_index_if_able(&the_index, &index_lock)
12.605  wt_status_collect:619 wt_status_collect_changes_worktree(s)
11.262  wt_status_collect:624 wt_status_collect_changes_index(s)
   186.032  wt_status_collect:625 wt_status_collect_untracked(s)
   210.134 cmd_status:1308 wt_status_collect(&s)
12.332 cmd_status:1348 if (the_index.untracked) { struct index_state

real0m1.058s
user0m0.525s
sys 0m0.532s

And linux-2.6 (45k files, 3k dirs, 3.2MB index v4), before:

68.668 gitmodules_config:201 if (read_cache() < 0) die("index
 0.004 cmd_status:1299 read_cache_preload(&s.pathspec)
   114.270 cmd_status:1300 refresh_index(&the_index, REFRESH_QUIET
 1.180 cmd_status:1304 update_index_if_able(&the_index, &index_lock)
 4.027  wt_status_collect:619 wt_status_collect_changes_worktree(s)
 4.265  wt_status_collect:624 wt_status_collect_changes_index(s)
   191.285  wt_status_collect:625 wt_status_collect_untracked(s)
   199.825 cmd_status:1308 wt_status_collect(&s)

real0m0.392s
user0m0.177s
sys 0m0.215s

and after (saving 33%):

71.756 gitmodules_config:201 if (read_cache() < 0) die("index
 5.201 cmd_status:1299 read_cache_preload(&s.pathspec)
   111.064 cmd_status:1300 refresh_index(&the_index, REFRESH_QUIET
 1.171 cmd_status:1304 update_index_if_able(&the_index, &index_lock)
 3.054  wt_status_collect:619 wt_status_collect_changes_worktree(s)
 4.945  wt_status_collect:624 wt_status_collect_changes_index(s)
27.203  wt_status_collect:625 wt_status_collect_untracked(s)
35.475 cmd_status:1308 wt_status_collect(&s)
25.759 cmd_status:1348 if (the_index.untracked) { struct index_state

real0m0.259s
user0m0.106s
sys 0m0.132s
---
 builtin/update-index.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index ba54e19..003e28e 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -734,6 +734,7 @@ static int reupdate_callback(struct parse_opt_ctx_t *ctx,
 int cmd_update_index(int argc, const char **argv, const char *prefix)
 {
int newfd, entries, has_errors = 0, line_termination = '\n';
+   int untracked_cache = -1;
int read_from_stdin = 0;
int prefix_length = prefix ? strlen(prefix) : 0;
int preferred_index_format = 0;
@@ -822,6 +823,8 @@ int c

[PATCH 19/20] update-index: test the system before enabling untracked cache

2014-05-07 Thread Nguyễn Thái Ngọc Duy
---
 builtin/update-index.c | 144 +
 1 file changed, 144 insertions(+)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index 003e28e..f18076b 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -46,6 +46,145 @@ static void report(const char *fmt, ...)
va_end(vp);
 }
 
+static void remove_test_directory(void)
+{
+   struct strbuf sb = STRBUF_INIT;
+   strbuf_addstr(&sb, git_path("dir-mtime-test"));
+   remove_dir_recursively(&sb, 0);
+   strbuf_release(&sb);
+}
+
+static void xmkdir(const char *path)
+{
+   if (mkdir(path, 0700))
+   die_errno(_("failed to create directory %s"), path);
+}
+
+static int xstat(const char *path, struct stat *st)
+{
+   if (stat(path, st))
+   die_errno(_("failed to stat %s"), path);
+   return 0;
+}
+
+static int create_file(const char *path)
+{
+   int fd = open(path, O_CREAT | O_RDWR, 0644);
+   if (fd < 0)
+   die_errno(_("failed to create file %s"), path);
+   return fd;
+}
+
+static void xunlink(const char *path)
+{
+   if (unlink(path))
+   die_errno(_("failed to delete file %s"), path);
+}
+
+static void xrmdir(const char *path)
+{
+   if (rmdir(path))
+   die_errno(_("failed to delete directory %s"), path);
+}
+
+static void avoid_racy(void)
+{
+   /*
+* not use if we could usleep(10) if USE_NSEC is defined. The
+* field nsec could be there, but the OS could choose to
+* ignore it?
+*/
+   sleep(1);
+}
+
+static int test_if_untracked_cache_is_supported(void)
+{
+   struct stat st;
+   struct stat_data base;
+   int fd;
+
+   fprintf(stderr, _("Testing "));
+   xmkdir(git_path("dir-mtime-test"));
+   atexit(remove_test_directory);
+   xstat(git_path("dir-mtime-test"), &st);
+   fill_stat_data(&base, &st);
+   fputc('.', stderr);
+
+   avoid_racy();
+   fd = create_file(git_path("dir-mtime-test/newfile"));
+   xstat(git_path("dir-mtime-test"), &st);
+   if (!match_stat_data(&base, &st)) {
+   fputc('\n', stderr);
+   fprintf_ln(stderr,_("directory stat info does not "
+   "change after adding a new file"));
+   return 0;
+   }
+   fill_stat_data(&base, &st);
+   fputc('.', stderr);
+
+   avoid_racy();
+   xmkdir(git_path("dir-mtime-test/new-dir"));
+   xstat(git_path("dir-mtime-test"), &st);
+   if (!match_stat_data(&base, &st)) {
+   fputc('\n', stderr);
+   fprintf_ln(stderr, _("directory stat info does not change "
+"after adding a new directory"));
+   return 0;
+   }
+   fill_stat_data(&base, &st);
+   fputc('.', stderr);
+
+   avoid_racy();
+   write_or_die(fd, "data", 4);
+   close(fd);
+   xstat(git_path("dir-mtime-test"), &st);
+   if (match_stat_data(&base, &st)) {
+   fputc('\n', stderr);
+   fprintf_ln(stderr, _("directory stat info changes "
+"after updating a file"));
+   return 0;
+   }
+   fputc('.', stderr);
+
+   avoid_racy();
+   close(create_file(git_path("dir-mtime-test/new-dir/new")));
+   xstat(git_path("dir-mtime-test"), &st);
+   if (match_stat_data(&base, &st)) {
+   fputc('\n', stderr);
+   fprintf_ln(stderr, _("directory stat info changes after "
+"adding a file inside subdirectory"));
+   return 0;
+   }
+   fputc('.', stderr);
+
+   avoid_racy();
+   xunlink(git_path("dir-mtime-test/newfile"));
+   xstat(git_path("dir-mtime-test"), &st);
+   if (!match_stat_data(&base, &st)) {
+   fputc('\n', stderr);
+   fprintf_ln(stderr, _("directory stat info does not "
+"change after deleting a file"));
+   return 0;
+   }
+   fill_stat_data(&base, &st);
+   fputc('.', stderr);
+
+   avoid_racy();
+   xunlink(git_path("dir-mtime-test/new-dir/new"));
+   xrmdir(git_path("dir-mtime-test/new-dir"));
+   xstat(git_path("dir-mtime-test"), &st);
+   if (!match_stat_data(&base, &st)) {
+   fputc('\n', stderr);
+   fprintf_ln(stderr, _("directory stat info does not "
+"change after deleting a directory"));
+   return 0;
+   }
+
+   xrmdir(git_path("dir-mtime-test"));
+   fprintf_ln(stderr, _(" OK"));
+   return 1;
+}
+
 static int mark_ce_flags(const char *path, int flag, int mark)
 {
int namelen = strlen(path);
@@ -823,6 +962,8 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
resolve_undo_clear_callback},
OPT_INTEGER(0, "index-version"

[PATCH 15/20] read-cache.c: split racy stat test to a separate function

2014-05-07 Thread Nguyễn Thái Ngọc Duy
---
 read-cache.c | 24 +++-
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index 66c2279..72adcd6 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -258,20 +258,26 @@ static int ce_match_stat_basic(const struct cache_entry 
*ce, struct stat *st)
return changed;
 }
 
-static int is_racy_timestamp(const struct index_state *istate,
-const struct cache_entry *ce)
+static int is_racy_stat(const struct index_state *istate,
+   const struct stat_data *sd)
 {
-   return (!S_ISGITLINK(ce->ce_mode) &&
-   istate->timestamp.sec &&
+   return (istate->timestamp.sec &&
 #ifdef USE_NSEC
 /* nanosecond timestamped files can also be racy! */
-   (istate->timestamp.sec < ce->ce_stat_data.sd_mtime.sec ||
-(istate->timestamp.sec == ce->ce_stat_data.sd_mtime.sec &&
- istate->timestamp.nsec <= ce->ce_stat_data.sd_mtime.nsec))
+   (istate->timestamp.sec < sd->sd_mtime.sec ||
+(istate->timestamp.sec == sd->sd_mtime.sec &&
+ istate->timestamp.nsec <= sd->sd_mtime.nsec))
 #else
-   istate->timestamp.sec <= ce->ce_stat_data.sd_mtime.sec
+   istate->timestamp.sec <= sd->sd_mtime.sec
 #endif
-);
+   );
+}
+
+static int is_racy_timestamp(const struct index_state *istate,
+const struct cache_entry *ce)
+{
+   return (!S_ISGITLINK(ce->ce_mode) &&
+   is_racy_stat(istate, &ce->ce_stat_data));
 }
 
 int ie_match_stat(const struct index_state *istate,
-- 
1.9.1.346.ga2b5940

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 17/20] status: support untracked cache

2014-05-07 Thread Nguyễn Thái Ngọc Duy
---
 builtin/commit.c | 8 
 wt-status.c  | 6 ++
 2 files changed, 14 insertions(+)

diff --git a/builtin/commit.c b/builtin/commit.c
index 9cfef6c..1e45ff0 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1327,6 +1327,14 @@ int cmd_status(int argc, const char **argv, const char 
*prefix)
wt_status_print(&s);
break;
}
+
+   if (active_cache_changed) {
+   fd = hold_locked_index(&index_lock, 0);
+   if (0 <= fd &&
+   (write_cache(fd, active_cache, active_nr) ||
+commit_locked_index(&index_lock)))
+   die("Unable to write new index file");
+   }
return 0;
 }
 
diff --git a/wt-status.c b/wt-status.c
index ec7344e..0355129 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -578,9 +578,15 @@ static void wt_status_collect_untracked(struct wt_status 
*s)
DIR_SHOW_OTHER_DIRECTORIES | DIR_HIDE_EMPTY_DIRECTORIES;
if (s->show_ignored_files)
dir.flags |= DIR_SHOW_IGNORED_TOO;
+   dir.untracked = the_index.untracked;
setup_standard_excludes(&dir);
 
fill_directory(&dir, &s->pathspec);
+   if (dir.untracked &&
+   (dir.untracked->dir_opened ||
+dir.untracked->gitignore_invalidated ||
+dir.untracked->dir_invalidated))
+   active_cache_changed = 1;
 
for (i = 0; i < dir.nr; i++) {
struct dir_entry *ent = dir.entries[i];
-- 
1.9.1.346.ga2b5940

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 14/20] untracked cache: print untracked statistics with $GIT_TRACE_UNTRACKED

2014-05-07 Thread Nguyễn Thái Ngọc Duy
This could be used to verify correct behavior in tests
---
 dir.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/dir.c b/dir.c
index 18fe44c..58303ca 100644
--- a/dir.c
+++ b/dir.c
@@ -14,6 +14,8 @@
 #include "pathspec.h"
 #include "varint.h"
 
+#define TRACE_KEY "GIT_TRACE_UNTRACKED"
+
 struct path_simplify {
int len;
const char *path;
@@ -1929,6 +1931,16 @@ int read_directory(struct dir_struct *dir, const char 
*path, int len, const stru
free_simplify(simplify);
qsort(dir->entries, dir->nr, sizeof(struct dir_entry *), cmp_name);
qsort(dir->ignored, dir->ignored_nr, sizeof(struct dir_entry *), 
cmp_name);
+   if (dir->untracked) {
+   trace_printf_key(TRACE_KEY, "node creation: %u\n",
+dir->untracked->dir_created);
+   trace_printf_key(TRACE_KEY, "gitignore invalidation: %u\n",
+dir->untracked->gitignore_invalidated);
+   trace_printf_key(TRACE_KEY, "directory invalidation: %u\n",
+dir->untracked->dir_invalidated);
+   trace_printf_key(TRACE_KEY, "opendir: %u\n",
+dir->untracked->dir_opened);
+   }
return dir->nr;
 }
 
-- 
1.9.1.346.ga2b5940

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 13/20] untracked cache: invalidate at index addition or removal

2014-05-07 Thread Nguyễn Thái Ngọc Duy
Ideally we should replace untracked_cache_invalidate_path() with
untracked_cache_remove_from_index() and untracked_cache_add_to_index(),
and the two last functions will update untracked cache right away
instead of invalidating it and wait for read_directory() next time to
deal with it. But that may need some more work in unpack-trees.c. So
stay simple as the first step.
---
 dir.c  | 31 +++
 dir.h  |  4 
 read-cache.c   |  4 
 unpack-trees.c |  7 +--
 4 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/dir.c b/dir.c
index 3c61b42..18fe44c 100644
--- a/dir.c
+++ b/dir.c
@@ -2363,3 +2363,34 @@ struct untracked_cache *read_untracked_extension(const 
void *data, unsigned long
free(uc);
return NULL;
 }
+
+void untracked_cache_invalidate_path(struct index_state *istate,
+const char *path)
+{
+   const char *sep;
+   struct untracked_cache_dir *d;
+   if (!istate->untracked || !istate->untracked->root)
+   return;
+   sep = strrchr(path, '/');
+   if (sep)
+   d = lookup_untracked(istate->untracked,
+istate->untracked->root,
+path, sep - path);
+   else
+   d = istate->untracked->root;
+   istate->untracked->dir_invalidated++;
+   d->valid = 0;
+   d->untracked_nr = 0;
+}
+
+void untracked_cache_remove_from_index(struct index_state *istate,
+  const char *path)
+{
+   untracked_cache_invalidate_path(istate, path);
+}
+
+void untracked_cache_add_to_index(struct index_state *istate,
+ const char *path)
+{
+   untracked_cache_invalidate_path(istate, path);
+}
diff --git a/dir.h b/dir.h
index 42a09ff..d56c43a 100644
--- a/dir.h
+++ b/dir.h
@@ -295,6 +295,10 @@ static inline int dir_path_match(const struct dir_entry 
*ent,
  has_trailing_dir);
 }
 
+void untracked_cache_invalidate_path(struct index_state *, const char *);
+void untracked_cache_remove_from_index(struct index_state *, const char *);
+void untracked_cache_add_to_index(struct index_state *, const char *);
+
 struct untracked_cache *read_untracked_extension(const void *data, unsigned 
long sz);
 void write_untracked_extension(struct strbuf *out, struct untracked_cache 
*untracked);
 #endif
diff --git a/read-cache.c b/read-cache.c
index c350b7b..66c2279 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -66,6 +66,7 @@ void rename_index_entry_at(struct index_state *istate, int 
nr, const char *new_n
memcpy(new->name, new_name, namelen + 1);
 
cache_tree_invalidate_path(istate->cache_tree, old->name);
+   untracked_cache_remove_from_index(istate, old->name);
remove_index_entry_at(istate, nr);
add_index_entry(istate, new, 
ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);
 }
@@ -520,6 +521,7 @@ int remove_file_from_index(struct index_state *istate, 
const char *path)
if (pos < 0)
pos = -pos-1;
cache_tree_invalidate_path(istate->cache_tree, path);
+   untracked_cache_remove_from_index(istate, path);
while (pos < istate->cache_nr && !strcmp(istate->cache[pos]->name, 
path))
remove_index_entry_at(istate, pos);
return 0;
@@ -948,6 +950,8 @@ static int add_index_entry_with_check(struct index_state 
*istate, struct cache_e
}
pos = -pos-1;
 
+   untracked_cache_add_to_index(istate, ce->name);
+
/*
 * Inserting a merged entry ("stage 0") into the index
 * will always replace all non-merged entries..
diff --git a/unpack-trees.c b/unpack-trees.c
index 97fc995..35ef298 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -8,6 +8,7 @@
 #include "progress.h"
 #include "refs.h"
 #include "attr.h"
+#include "dir.h"
 
 /*
  * Error messages expected by scripts out of plumbing commands such as
@@ -1258,8 +1259,10 @@ static int verify_uptodate_sparse(const struct 
cache_entry *ce,
 static void invalidate_ce_path(const struct cache_entry *ce,
   struct unpack_trees_options *o)
 {
-   if (ce)
-   cache_tree_invalidate_path(o->src_index->cache_tree, ce->name);
+   if (!ce)
+   return;
+   cache_tree_invalidate_path(o->src_index->cache_tree, ce->name);
+   untracked_cache_invalidate_path(o->src_index, ce->name);
 }
 
 /*
-- 
1.9.1.346.ga2b5940

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 16/20] untracked cache: avoid racy timestamps

2014-05-07 Thread Nguyễn Thái Ngọc Duy
When a directory is updated within the same second that its timestamp
is last saved, we cannot realize the directory has been updated by
checking timestamps. Assume the worst (something is update). See
29e4d36 (Racy GIT - 2005-12-20) for more information.
---
 cache.h  | 2 ++
 dir.c| 6 --
 read-cache.c | 8 
 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/cache.h b/cache.h
index 06fcb6b..98c22c4 100644
--- a/cache.h
+++ b/cache.h
@@ -525,6 +525,8 @@ extern void fill_stat_data(struct stat_data *sd, struct 
stat *st);
  * INODE_CHANGED, and DATA_CHANGED.
  */
 extern int match_stat_data(const struct stat_data *sd, struct stat *st);
+extern int match_stat_data_racy(const struct index_state *istate,
+   const struct stat_data *sd, struct stat *st);
 
 extern void fill_stat_cache_info(struct cache_entry *ce, struct stat *st);
 
diff --git a/dir.c b/dir.c
index 58303ca..24ccd22 100644
--- a/dir.c
+++ b/dir.c
@@ -677,7 +677,9 @@ static int add_excludes(const char *fname,
!ce_stage(active_cache[pos]) &&
ce_uptodate(active_cache[pos]))
hashcpy(sha1, active_cache[pos]->sha1);
-   else if (ref_stat && !match_stat_data(ref_stat, &st)) {
+   else if (ref_stat &&
+!match_stat_data_racy(&the_index,
+  ref_stat, &st)) {
if (ref_sha1 != sha1) /* support ref_sha1 == 
sha1 */
hashcpy(sha1, ref_sha1);
} else
@@ -1543,7 +1545,7 @@ static int valid_cached_dir(struct dir_struct *dir,
return 0;
}
if (!untracked->valid ||
-   match_stat_data(&untracked->stat_data, &st)) {
+   match_stat_data_racy(&the_index, &untracked->stat_data, &st)) {
if (untracked->valid)
invalidate_directory(dir->untracked, untracked);
fill_stat_data(&untracked->stat_data, &st);
diff --git a/read-cache.c b/read-cache.c
index 72adcd6..823db9b 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -280,6 +280,14 @@ static int is_racy_timestamp(const struct index_state 
*istate,
is_racy_stat(istate, &ce->ce_stat_data));
 }
 
+int match_stat_data_racy(const struct index_state *istate,
+const struct stat_data *sd, struct stat *st)
+{
+   if (is_racy_stat(istate, sd))
+   return MTIME_CHANGED;
+   return match_stat_data(sd, st);
+}
+
 int ie_match_stat(const struct index_state *istate,
  const struct cache_entry *ce, struct stat *st,
  unsigned int options)
-- 
1.9.1.346.ga2b5940

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 12/20] untracked cache: load from UNTR index extension

2014-05-07 Thread Nguyễn Thái Ngọc Duy
FIXME: load check_only
---
 dir.c| 107 +++
 dir.h|   1 +
 read-cache.c |   3 ++
 3 files changed, 111 insertions(+)

diff --git a/dir.c b/dir.c
index b7d394a..3c61b42 100644
--- a/dir.c
+++ b/dir.c
@@ -2256,3 +2256,110 @@ void write_untracked_extension(struct strbuf *out, 
struct untracked_cache *untra
if (untracked->root)
write_one_dir(out, untracked->root);
 }
+
+static void stat_data_from_disk(struct stat_data *to, const struct stat_data 
*from)
+{
+   to->sd_ctime.sec  = get_be32(&from->sd_ctime.sec);
+   to->sd_ctime.nsec = get_be32(&from->sd_ctime.nsec);
+   to->sd_mtime.sec  = get_be32(&from->sd_mtime.sec);
+   to->sd_mtime.nsec = get_be32(&from->sd_mtime.nsec);
+   to->sd_dev= get_be32(&from->sd_dev);
+   to->sd_ino= get_be32(&from->sd_ino);
+   to->sd_uid= get_be32(&from->sd_uid);
+   to->sd_gid= get_be32(&from->sd_gid);
+   to->sd_size   = get_be32(&from->sd_size);
+}
+
+static int read_one_dir(struct untracked_cache_dir **untracked_,
+   const unsigned char *data_, unsigned long sz)
+{
+#define NEXT(x) \
+   next = data + (x); \
+   if (next > data_ + sz) \
+   return -1;
+
+   struct untracked_cache_dir ud, *untracked;
+   const unsigned char *next, *data = data_;
+   unsigned int value;
+   int i, len;
+
+   memset(&ud, 0, sizeof(ud));
+   ud.recurse = 1;
+
+   NEXT(sizeof(struct stat_data));
+   stat_data_from_disk(&ud.stat_data, (struct stat_data *)data);
+   data = next;
+
+   NEXT(20);
+   hashcpy(ud.exclude_sha1, data);
+   data = next;
+
+   next = data;
+   value = decode_varint(&next);
+   if (next > data_ + sz)
+   return -1;
+   ud.untracked_alloc = ud.untracked_nr = value >> 2;
+   if (ud.untracked_nr)
+   ud.untracked = xmalloc(sizeof(*ud.untracked) * ud.untracked_nr);
+   if (value & 1)
+   ud.valid = 1;
+   if (value & 2)
+   ud.check_only = 1;
+   data = next;
+
+   next = data;
+   ud.dirs_alloc = ud.dirs_nr = decode_varint(&next);
+   if (next > data_ + sz)
+   return -1;
+   ud.dirs = xmalloc(sizeof(*ud.dirs) * ud.dirs_nr);
+   data = next;
+
+   len = strlen((const char *)data);
+   NEXT(len + 1);
+   *untracked_ = untracked = xmalloc(sizeof(*untracked) + len);
+   memcpy(untracked, &ud, sizeof(ud));
+   memcpy(untracked->name, data, len + 1);
+   data = next;
+
+   for (i = 0; i < untracked->untracked_nr; i++) {
+   len = strlen((const char *)data);
+   NEXT(len + 1);
+   untracked->untracked[i] = xstrdup((const char*)data);
+   data = next;
+   }
+
+   for (i = 0; i < untracked->dirs_nr; i++) {
+   len = read_one_dir(untracked->dirs + i, data, sz - (data - 
data_));
+   if (len < 0)
+   return -1;
+   data += len;
+   }
+   return data - data_;
+}
+
+struct untracked_cache *read_untracked_extension(const void *data, unsigned 
long sz)
+{
+   const struct ondisk_untracked_cache *ouc = data;
+   struct untracked_cache *uc;
+   int len;
+
+   if (sz < sizeof(*ouc))
+   return NULL;
+
+   uc = xcalloc(1, sizeof(*uc));
+   stat_data_from_disk(&uc->info_exclude_stat, &ouc->info_exclude_stat);
+   stat_data_from_disk(&uc->excludes_file_stat, &ouc->excludes_file_stat);
+   hashcpy(uc->info_exclude_sha1, ouc->info_exclude_sha1);
+   hashcpy(uc->excludes_file_sha1, ouc->excludes_file_sha1);
+   uc->dir_flags = get_be32(&ouc->dir_flags);
+   uc->exclude_per_dir = xstrdup(ouc->exclude_per_dir);
+   len = sizeof(*ouc) + strlen(ouc->exclude_per_dir);
+   if (sz == len)
+   return uc;
+   if (sz > len &&
+   read_one_dir(&uc->root, (const unsigned char *)data + len,
+sz - len) == sz - len)
+   return uc;
+   free(uc);
+   return NULL;
+}
diff --git a/dir.h b/dir.h
index e520d58..42a09ff 100644
--- a/dir.h
+++ b/dir.h
@@ -295,5 +295,6 @@ static inline int dir_path_match(const struct dir_entry 
*ent,
  has_trailing_dir);
 }
 
+struct untracked_cache *read_untracked_extension(const void *data, unsigned 
long sz);
 void write_untracked_extension(struct strbuf *out, struct untracked_cache 
*untracked);
 #endif
diff --git a/read-cache.c b/read-cache.c
index a619666..c350b7b 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1332,6 +1332,9 @@ static int read_index_extension(struct index_state 
*istate,
case CACHE_EXT_RESOLVE_UNDO:
istate->resolve_undo = resolve_undo_read(data, sz);
break;
+   case CACHE_EXT_UNTRACKED:
+   istate->untracked = read_untracked_extension(data, sz);
+

[PATCH 09/20] untracked cache: mark what dirs should be recursed/saved

2014-05-07 Thread Nguyễn Thái Ngọc Duy
Suppose untracked cache stores that in directory A we need to recurse
in A/B and A/C. Then A/B is removed. When read_directory() is executed
again, of course we detect that we only need to recurse in A/C when in
A, not A/B any more.

We need a way though to let the write phase know not to write A/B
down. Which is the purpose of this bit. We can't simply destroy A/B
when A is invalidated, because at that moment we don't know if A/B is
deleted or not.
---
 dir.c | 15 ++-
 dir.h |  1 +
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/dir.c b/dir.c
index 205f323..63fa960 100644
--- a/dir.c
+++ b/dir.c
@@ -591,6 +591,7 @@ static void do_invalidate_gitignore(struct 
untracked_cache_dir *dir)
int i;
dir->valid = 0;
dir->untracked_nr = 0;
+   /* dir->recurse = 0; ? */
for (i = 0; i < dir->dirs_nr; i++)
do_invalidate_gitignore(dir->dirs[i]);
 }
@@ -605,9 +606,12 @@ static void invalidate_gitignore(struct untracked_cache 
*uc,
 static void invalidate_directory(struct untracked_cache *uc,
 struct untracked_cache_dir *dir)
 {
+   int i;
uc->dir_invalidated++;
dir->valid = 0;
dir->untracked_nr = 0;
+   for (i = 0; i < dir->dirs_nr; i++)
+   dir->dirs[i]->recurse = 0;
 }
 
 static int add_excludes(const char *fname,
@@ -1581,6 +1585,10 @@ int read_cached_dir(struct cached_dir *cdir)
}
while (cdir->nr_dirs < cdir->untracked->dirs_nr) {
struct untracked_cache_dir *d = 
cdir->untracked->dirs[cdir->nr_dirs];
+   if (!d->recurse) {
+   cdir->nr_dirs++;
+   continue;
+   }
cdir->ucd = d;
cdir->nr_dirs++;
return 0;
@@ -1602,8 +1610,10 @@ static void close_cached_dir(struct cached_dir *cdir)
 * We have gone through this directory and found no untracked
 * entries. Set untracked_nr to zero to make it valid.
 */
-   if (cdir->untracked && !cdir->untracked->valid)
+   if (cdir->untracked) {
cdir->untracked->valid = 1;
+   cdir->untracked->recurse = 1;
+   }
 }
 
 /*
@@ -1849,6 +1859,9 @@ static struct untracked_cache_dir 
*validate_untracked_cache(struct dir_struct *d
hashcpy(dir->untracked->excludes_file_sha1,
dir->excludes_file_sha1);
}
+
+   /* Make sure this directory is not dropped out at saving phase */
+   root->recurse = 1;
return root;
 }
 
diff --git a/dir.h b/dir.h
index 8955945..5dde37b 100644
--- a/dir.h
+++ b/dir.h
@@ -108,6 +108,7 @@ struct untracked_cache_dir {
/* null SHA-1 means this directory does not have .gitignore */
unsigned char exclude_sha1[20];
struct stat_data stat_data;
+   unsigned int recurse : 1;
unsigned int check_only : 1;
unsigned int valid : 1;
unsigned int untracked_nr : 29;
-- 
1.9.1.346.ga2b5940

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 10/20] untracked cache: don't open non-existent .gitignore

2014-05-07 Thread Nguyễn Thái Ngọc Duy
This cuts down a signficant number of open(.gitignore) because most
directories usually don't have .gitignore files.
---
 dir.c | 26 +-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/dir.c b/dir.c
index 63fa960..b5bfda8 100644
--- a/dir.c
+++ b/dir.c
@@ -1021,7 +1021,21 @@ static void prep_exclude(struct dir_struct *dir, const 
char *base, int baselen)
 
/* Try to read per-directory file */
hashclr(sha1);
-   if (dir->exclude_per_dir) {
+   if (dir->exclude_per_dir &&
+   /*
+* If we know that no files have been added in
+* this directory (i.e. valid_cached_dir() has
+* been executed and set untracked->valid) ..
+*/
+   (!untracked || !untracked->valid ||
+/*
+ * .. and .gitignore does not exist before
+ * (i.e. null exclude_sha1 and skip_worktree is
+ * not set). Then we can skip loading .gitignore,
+ * which would result in ENOENT anyway.
+ * skip_worktree is taken care in read_directory()
+ */
+!is_null_sha1(untracked->exclude_sha1))) {
/*
 * dir->basebuf gets reused by the traversal, but we
 * need fname to remain unchanged to ensure the src
@@ -1788,6 +1802,7 @@ static struct untracked_cache_dir 
*validate_untracked_cache(struct dir_struct *d
  const struct pathspec 
*pathspec)
 {
struct untracked_cache_dir *root;
+   int i;
 
if (!dir->untracked)
return NULL;
@@ -1839,6 +1854,15 @@ static struct untracked_cache_dir 
*validate_untracked_cache(struct dir_struct *d
if (dir->exclude_list_group[EXC_CMDL].nr)
return NULL;
 
+   /*
+* An optimization in prep_exclude() does not play well with
+* CE_SKIP_WORKTREE. It's a rare case anyway, if a single
+* entry has that bit set, disable the whole untracked cache.
+*/
+   for (i = 0; i < active_nr; i++)
+   if (ce_skip_worktree(active_cache[i]))
+   return NULL;
+
if (!dir->untracked->root) {
const int len = sizeof(*dir->untracked->root);
dir->untracked->root = xmalloc(len);
-- 
1.9.1.346.ga2b5940

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 05/20] untracked cache: record .gitignore information and dir hierarchy

2014-05-07 Thread Nguyễn Thái Ngọc Duy
The idea is if we can capture all input and output of
read_directory_recursive() and verify at a later time that all the
input is the same, then read_directory_recursive() should produce the
same output, so we can bypass read_directory_recursive() and reuse the
cached output for the directory in question (the bypass code needs to
verify subdirectories separately)

The list of input of read_directory_recursive() is in the big comment
block in dir.h. This cache focuses on only untracked files as the
output from r_d_r(), not ignored files because the number of tracked
files is usually small, so small cache overhead, while the number of
ignored files could go really high (e.g. *.o files mixing with source
code).

This patch captures .gitignore information, check_only bit and the
list of directories that read_directory_recursive() examines.

Two hash_sha1_file() are required for $GIT_DIR/info/exclude and
core.excludesfile unless their stat data matches. hash_sha1_file() is
only needed when .gitignore files in the worktree are modified,
otherwise their SHA-1 in index is used.

We could store stat data for .gitignore files so we don't have to
rehash them if their content is different from index, but I think
.gitignore files are rarely modified, so not worth extra cache data
(and hashing penalty read-cache.c:verify_hdr(), as we will be storing
this as an index extension).

This means if you change .gitignore at root directory, you better add
it to the index soon or you lose all the benefit of untracked cache (a
change at root .gitignore invalidates everything) and pay the cache
overhead for nothing.
---
 dir.c | 167 +-
 dir.h |  64 +
 2 files changed, 210 insertions(+), 21 deletions(-)

diff --git a/dir.c b/dir.c
index e2edeca..34a10b2 100644
--- a/dir.c
+++ b/dir.c
@@ -32,7 +32,7 @@ enum path_treatment {
 };
 
 static enum path_treatment read_directory_recursive(struct dir_struct *dir,
-   const char *path, int len,
+   const char *path, int len, struct untracked_cache_dir *untracked,
int check_only, const struct path_simplify *simplify);
 static int get_dtype(struct dirent *de, const char *path, int len);
 
@@ -528,6 +528,46 @@ static void trim_trailing_spaces(char *buf)
buf[last_space] = '\0';
 }
 
+static struct untracked_cache_dir *lookup_untracked(struct untracked_cache *uc,
+   struct untracked_cache_dir 
*dir,
+   const char *name, int len)
+{
+   int first, last;
+   struct untracked_cache_dir *d;
+   if (!dir)
+   return NULL;
+   if (len && name[len - 1] == '/')
+   len--;
+   first = 0;
+   last = dir->dirs_nr;
+   while (last > first) {
+   int cmp, next = (last + first) >> 1;
+   d = dir->dirs[next];
+   cmp = strncmp(name, d->name, len);
+   if (!cmp && strlen(d->name) > len)
+   cmp = -1;
+   if (!cmp)
+   return d;
+   if (cmp < 0) {
+   last = next;
+   continue;
+   }
+   first = next+1;
+   }
+
+   uc->dir_created++;
+   d = xmalloc(sizeof(*d) + len);
+   memset(d, 0, sizeof(*d) + len);
+   memcpy(d->name, name, len);
+
+   ALLOC_GROW(dir->dirs, dir->dirs_nr + 1, dir->dirs_alloc);
+   memmove(dir->dirs + first + 1, dir->dirs + first,
+   (dir->dirs_nr - first) * sizeof(*dir->dirs));
+   dir->dirs_nr++;
+   dir->dirs[first] = d;
+   return d;
+}
+
 static int add_excludes(const char *fname,
const char *base,
int baselen,
@@ -639,14 +679,22 @@ struct exclude_list *add_exclude_list(struct dir_struct 
*dir,
 /*
  * Used to set up core.excludesfile and .git/info/exclude lists.
  */
-void add_excludes_from_file(struct dir_struct *dir, const char *fname)
+void add_excludes_from_file_1(struct dir_struct *dir, const char *fname,
+ unsigned char *sha1,
+ struct stat_data *ref_stat,
+ const unsigned char *ref_sha1)
 {
struct exclude_list *el;
el = add_exclude_list(dir, EXC_FILE, fname);
-   if (add_excludes_from_file_to_list(fname, "", 0, el, 0) < 0)
+   if (add_excludes(fname, "", 0, el, 0, sha1, ref_stat, ref_sha1) < 0)
die("cannot use %s as an exclude file", fname);
 }
 
+void add_excludes_from_file(struct dir_struct *dir, const char *fname)
+{
+   add_excludes_from_file_1(dir, fname, NULL, NULL, NULL);
+}
+
 int match_basename(const char *basename, int basenamelen,
   const char *pattern, int prefix, int patternlen,
   int flags)
@@ -821,6 +869,7 @@ static void prep_exclude(struct dir_struct *dir, const char 
*

[PATCH 06/20] untracked cache: initial untracked cache validation

2014-05-07 Thread Nguyễn Thái Ngọc Duy
---
 dir.c | 115 --
 dir.h |   3 ++
 2 files changed, 115 insertions(+), 3 deletions(-)

diff --git a/dir.c b/dir.c
index 34a10b2..a198aa8 100644
--- a/dir.c
+++ b/dir.c
@@ -568,6 +568,22 @@ static struct untracked_cache_dir *lookup_untracked(struct 
untracked_cache *uc,
return d;
 }
 
+static void do_invalidate_gitignore(struct untracked_cache_dir *dir)
+{
+   int i;
+   dir->valid = 0;
+   dir->untracked_nr = 0;
+   for (i = 0; i < dir->dirs_nr; i++)
+   do_invalidate_gitignore(dir->dirs[i]);
+}
+
+static void invalidate_gitignore(struct untracked_cache *uc,
+struct untracked_cache_dir *dir)
+{
+   uc->gitignore_invalidated++;
+   do_invalidate_gitignore(dir);
+}
+
 static int add_excludes(const char *fname,
const char *base,
int baselen,
@@ -685,6 +701,13 @@ void add_excludes_from_file_1(struct dir_struct *dir, 
const char *fname,
  const unsigned char *ref_sha1)
 {
struct exclude_list *el;
+   /*
+* catch setup_standard_excludes() that's called before
+* dir->untracked is assigned. That function behaves
+* differently when dir->untracked is non-NULL.
+*/
+   if (!dir->untracked)
+   dir->unmanaged_exclude_files++;
el = add_exclude_list(dir, EXC_FILE, fname);
if (add_excludes(fname, "", 0, el, 0, sha1, ref_stat, ref_sha1) < 0)
die("cannot use %s as an exclude file", fname);
@@ -692,6 +715,7 @@ void add_excludes_from_file_1(struct dir_struct *dir, const 
char *fname,
 
 void add_excludes_from_file(struct dir_struct *dir, const char *fname)
 {
+   dir->unmanaged_exclude_files++; /* see validate_untracked_cache() */
add_excludes_from_file_1(dir, fname, NULL, NULL, NULL);
 }
 
@@ -1570,9 +1594,89 @@ static int treat_leading_path(struct dir_struct *dir,
return rc;
 }
 
+static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct 
*dir,
+ int base_len,
+ const struct pathspec 
*pathspec)
+{
+   struct untracked_cache_dir *root;
+
+   if (!dir->untracked)
+   return NULL;
+
+   /*
+* We only support $GIT_DIR/info/exclude and core.excludesfile
+* as the global ignore rule files. Any other additions
+* (e.g. from command line) invalidate the cache. This
+* condition also catches running setup_standard_excludes()
+* before setting dir->untracked!
+*/
+   if (dir->unmanaged_exclude_files)
+   return NULL;
+
+   /*
+* Optimize for the main use case only: whole-tree git
+* status. More work involved in treat_leading_path() if we
+* use cache on just a subset of the worktree. pathspec
+* support could make the matter even worse.
+*/
+   if (base_len || (pathspec && pathspec->nr))
+   return NULL;
+
+   /* Different set of flags may produce different results */
+   if (dir->flags != dir->untracked->dir_flags ||
+   /*
+* See treat_directory(), case index_nonexistent. Without
+* this flag, we may need to also cache .git file content
+* for the resolve_gitlink_ref() call, which we don't.
+*/
+   !(dir->flags & DIR_SHOW_OTHER_DIRECTORIES) ||
+   /* We don't support collecting ignore files */
+   (dir->flags & (DIR_SHOW_IGNORED | DIR_SHOW_IGNORED_TOO |
+  DIR_COLLECT_IGNORED)))
+   return NULL;
+
+   /*
+* If we use .gitignore in the cache and now you change it to
+* .gitexclude, everything will go wrong.
+*/
+   if (dir->exclude_per_dir != dir->untracked->exclude_per_dir &&
+   strcmp(dir->exclude_per_dir, dir->untracked->exclude_per_dir))
+   return NULL;
+
+   /*
+* EXC_CMDL is not considered in the cache. If people set it,
+* skip the cache.
+*/
+   if (dir->exclude_list_group[EXC_CMDL].nr)
+   return NULL;
+
+   if (!dir->untracked->root) {
+   const int len = sizeof(*dir->untracked->root);
+   dir->untracked->root = xmalloc(len);
+   memset(dir->untracked->root, 0, len);
+   }
+
+   /* Validate $GIT_DIR/info/exclude and core.excludesfile */
+   root = dir->untracked->root;
+   if (hashcmp(dir->info_exclude_sha1,
+   dir->untracked->info_exclude_sha1)) {
+   invalidate_gitignore(dir->untracked, root);
+   hashcpy(dir->untracked->info_exclude_sha1,
+   dir->info_exclude_sha1);
+   }
+   if (hashcmp(dir->excludes_file_sha1,
+   dir->untracked->excludes_file_sha1)) {
+

[PATCH 07/20] untracked cache: invalidate dirs recursively if .gitignore changes

2014-05-07 Thread Nguyễn Thái Ngọc Duy
It's easy to see that if an existing .gitignore changes, its SHA-1
would be different and invalidate_gitignore() is called.

If .gitignore is removed, add_excludes() will treat it like an empty
.gitignore, which again should invalidate the cached directory data.

if .gitignore is added, lookup_untracked() already fills initial
.gitignore SHA-1 as "empty file", so again invalidate_gitignore() is
called.
---
 dir.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/dir.c b/dir.c
index a198aa8..6370f6e 100644
--- a/dir.c
+++ b/dir.c
@@ -1007,7 +1007,26 @@ static void prep_exclude(struct dir_struct *dir, const 
char *base, int baselen)
add_excludes(el->src, el->src, stk->baselen, el, 1,
 untracked ? sha1 : NULL, NULL, NULL);
}
+   /*
+* NEEDSWORK: when untracked cache is enabled,
+* prep_exclude() will first be called in
+* valid_cached_dir() then maybe many times more in
+* last_exclude_matching(). When the cache is used,
+* last_exclude_matching() will not be called and
+* reading .gitignore content will be a waste.
+*
+* So when it's called by valid_cached_dir() and we
+* can get .gitignore SHA-1 from the index
+* (i.e. .gitignore is not modified on work tree), we
+* could delay reading the .gitignore content until we
+* absolutely need it in last_exclude_matching(). Be
+* careful about ignore rule order, though, if you do
+* that.
+*/
if (untracked) {
+   if (hashcmp(sha1, untracked->exclude_sha1))
+   invalidate_gitignore(dir->untracked,
+untracked);
hashcpy(untracked->exclude_sha1, sha1);
}
dir->exclude_stack = stk;
-- 
1.9.1.346.ga2b5940

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 08/20] untracked cache: record/validate dir mtime and reuse cached output

2014-05-07 Thread Nguyễn Thái Ngọc Duy
The main readdir loop in read_directory_recursive() is replaced with a
new one that checks if cached results of a directory is still valid.

If a file is added or removed from the index, the containing directory
is invalidated (but not its subdirs). If directory's mtime is changed,
the same happens. If a .gitignore is updated, the containing directory
and all subdirs are invalidated recursively. If dir_struct#flags or
other conditions change, the cache is ignored.

If a directory is invalidated, we opendir/readdir/closedir and run the
exclude machinery on that directory listing as usual. If untracked
cache is also enabled, we'll update the cache along the way. If a
directory is validated, we simply pull the untracked listing out from
the cache. The cache also records the list of direct subdirs that we
have to recurse in. Fully excluded directories are seen as "untracked
files".

In the best case when no dirs are invalidated, read_directory()
becomes a series of stat(dir), open(.gitignore), fstat, read, close
and optionally hash_sha1_file. For comparison, standard
read_directory() is a sequence of opendir, readdir, open(gitignore),
fstat, read, close, (expensive) last_exclude_matching and closedir.

We already try not to open(gitignore) if we know it does not exist, so
open/fstat/read/close sequence does not apply to every directory. The
sequence could be reduced further, as noted in prep_exclude(). So in
theory, the entire best-case read_directory sequence could be reduced
to a series of stat() and nothing else.

This is not a silver bullet approach. When you compile a C file, for
example, the old .o file is removed and a new one with the same name
created, effectively invalidating the containing directory's cache
(but not its subdirectories). If your build process touches every
directory, this cache adds extra overhead for nothing, so it's a good
idea to separate generated files from tracked files.. Editors may use
the same strategy for saving files. And of course you're out of luck
running your repo on an unsupported filesytem and/or operating system.
---
 dir.c | 180 ++
 dir.h |   2 +
 2 files changed, 172 insertions(+), 10 deletions(-)

diff --git a/dir.c b/dir.c
index 6370f6e..205f323 100644
--- a/dir.c
+++ b/dir.c
@@ -31,6 +31,24 @@ enum path_treatment {
path_untracked
 };
 
+/*
+ * Support data structure for our opendir/readdir/closedir wrappers
+ */
+struct cached_dir {
+   DIR *fdir;
+   struct untracked_cache_dir *untracked;
+   int nr_files;
+   int nr_dirs;
+
+   /*
+* return data from read_cached_dir(). name and state are only
+* valid if de is NULL
+*/
+   struct dirent *de;
+   const char *file;
+   struct untracked_cache_dir *ucd;
+};
+
 static enum path_treatment read_directory_recursive(struct dir_struct *dir,
const char *path, int len, struct untracked_cache_dir *untracked,
int check_only, const struct path_simplify *simplify);
@@ -584,6 +602,14 @@ static void invalidate_gitignore(struct untracked_cache 
*uc,
do_invalidate_gitignore(dir);
 }
 
+static void invalidate_directory(struct untracked_cache *uc,
+struct untracked_cache_dir *dir)
+{
+   uc->dir_invalidated++;
+   dir->valid = 0;
+   dir->untracked_nr = 0;
+}
+
 static int add_excludes(const char *fname,
const char *base,
int baselen,
@@ -1415,15 +1441,54 @@ static enum path_treatment treat_one_path(struct 
dir_struct *dir,
}
 }
 
+static enum path_treatment treat_path_fast(struct dir_struct *dir,
+  struct untracked_cache_dir 
*untracked,
+  struct cached_dir *cdir,
+  struct strbuf *path,
+  int baselen,
+  const struct path_simplify *simplify)
+{
+   if (!cdir->ucd) {
+   strbuf_setlen(path, baselen);
+   strbuf_addstr(path, cdir->file);
+   return path_untracked;
+   }
+   strbuf_setlen(path, baselen);
+   strbuf_addstr(path, cdir->ucd->name);
+   /* treat_one_path() does this before it calls treat_directory() */
+   if (path->buf[path->len - 1] != '/')
+   strbuf_addch(path, '/');
+   if (cdir->ucd->check_only)
+   /*
+* check_only is set as a result of treat_directory() getting
+* to its bottom. Verify again the same set of directories
+* with check_only set.
+*/
+   return read_directory_recursive(dir, path->buf, path->len,
+
+   cdir->ucd, 1, simplify);
+   /*
+* We get path_recurse in the first run when
+* directory_exists_in_index() returns index_nonexis

[PATCH 11/20] untracked cache: save to an index extension

2014-05-07 Thread Nguyễn Thái Ngọc Duy
FIXME: save check_only
---
 cache.h  |  3 ++
 dir.c| 91 
 dir.h|  1 +
 read-cache.c | 12 
 4 files changed, 107 insertions(+)

diff --git a/cache.h b/cache.h
index 107ac61..06fcb6b 100644
--- a/cache.h
+++ b/cache.h
@@ -268,6 +268,8 @@ static inline unsigned int canon_mode(unsigned int mode)
 
 #define cache_entry_size(len) (offsetof(struct cache_entry,name) + (len) + 1)
 
+
+struct untracked_cache;
 struct index_state {
struct cache_entry **cache;
unsigned int version;
@@ -279,6 +281,7 @@ struct index_state {
 initialized : 1;
struct hashmap name_hash;
struct hashmap dir_hash;
+   struct untracked_cache *untracked;
 };
 
 extern struct index_state the_index;
diff --git a/dir.c b/dir.c
index b5bfda8..b7d394a 100644
--- a/dir.c
+++ b/dir.c
@@ -12,6 +12,7 @@
 #include "refs.h"
 #include "wildmatch.h"
 #include "pathspec.h"
+#include "varint.h"
 
 struct path_simplify {
int len;
@@ -2165,3 +2166,93 @@ void clear_directory(struct dir_struct *dir)
}
strbuf_release(&dir->basebuf);
 }
+
+struct ondisk_untracked_cache {
+   struct stat_data info_exclude_stat;
+   struct stat_data excludes_file_stat;
+   uint32_t dir_flags;
+   unsigned char info_exclude_sha1[20];
+   unsigned char excludes_file_sha1[20];
+   char exclude_per_dir[1];
+};
+
+static void stat_data_to_disk(struct stat_data *to, const struct stat_data 
*from)
+{
+   to->sd_ctime.sec  = htonl(from->sd_ctime.sec);
+   to->sd_ctime.nsec = htonl(from->sd_ctime.nsec);
+   to->sd_mtime.sec  = htonl(from->sd_mtime.sec);
+   to->sd_mtime.nsec = htonl(from->sd_mtime.nsec);
+   to->sd_dev= htonl(from->sd_dev);
+   to->sd_ino= htonl(from->sd_ino);
+   to->sd_uid= htonl(from->sd_uid);
+   to->sd_gid= htonl(from->sd_gid);
+   to->sd_size   = htonl(from->sd_size);
+}
+
+static void write_one_dir(struct strbuf *out, struct untracked_cache_dir 
*untracked)
+{
+   struct stat_data stat_data;
+   unsigned char intbuf[16];
+   unsigned int intlen, value;
+   int i;
+
+   stat_data_to_disk(&stat_data, &untracked->stat_data);
+   strbuf_add(out, &stat_data, sizeof(stat_data));
+   strbuf_add(out, untracked->exclude_sha1, 20);
+
+   /*
+* untracked_nr should be reset whenever valid is clear, but
+* for safety..
+*/
+   if (!untracked->valid) {
+   untracked->untracked_nr = 0;
+   untracked->check_only = 0;
+   }
+
+   /*
+* encode_varint does not deal with signed integers. Use the
+* lowest bit to store the sign.
+*/
+   value = untracked->untracked_nr << 2;
+   if (untracked->valid)
+   value |= 1;
+   if (untracked->check_only)
+   value |= 2;
+   intlen = encode_varint(value, intbuf);
+   strbuf_add(out, intbuf, intlen);
+
+   /* skip non-recurse directories */
+   for (i = 0, value = 0; i < untracked->dirs_nr; i++)
+   if (untracked->dirs[i]->recurse)
+   value++;
+   intlen = encode_varint(value, intbuf);
+   strbuf_add(out, intbuf, intlen);
+
+   strbuf_add(out, untracked->name, strlen(untracked->name) + 1);
+
+   for (i = 0; i < untracked->untracked_nr; i++)
+   strbuf_add(out, untracked->untracked[i],
+  strlen(untracked->untracked[i]) + 1);
+
+   for (i = 0; i < untracked->dirs_nr; i++)
+   if (untracked->dirs[i]->recurse)
+   write_one_dir(out, untracked->dirs[i]);
+}
+
+void write_untracked_extension(struct strbuf *out, struct untracked_cache 
*untracked)
+{
+   struct ondisk_untracked_cache *ouc;
+   int len = 0;
+   if (untracked->exclude_per_dir)
+   len = strlen(untracked->exclude_per_dir);
+   ouc = xmalloc(sizeof(*ouc) + len);
+   stat_data_to_disk(&ouc->info_exclude_stat, 
&untracked->info_exclude_stat);
+   stat_data_to_disk(&ouc->excludes_file_stat, 
&untracked->excludes_file_stat);
+   hashcpy(ouc->info_exclude_sha1, untracked->info_exclude_sha1);
+   hashcpy(ouc->excludes_file_sha1, untracked->excludes_file_sha1);
+   ouc->dir_flags = htonl(untracked->dir_flags);
+   memcpy(ouc->exclude_per_dir, untracked->exclude_per_dir, len + 1);
+   strbuf_add(out, ouc, sizeof(*ouc) + len);
+   if (untracked->root)
+   write_one_dir(out, untracked->root);
+}
diff --git a/dir.h b/dir.h
index 5dde37b..e520d58 100644
--- a/dir.h
+++ b/dir.h
@@ -295,4 +295,5 @@ static inline int dir_path_match(const struct dir_entry 
*ent,
  has_trailing_dir);
 }
 
+void write_untracked_extension(struct strbuf *out, struct untracked_cache 
*untracked);
 #endif
diff --git a/read-cache.c b/read-cache.c
index ba13353..a619666 100644
--- a/read-cache.c
+

[PATCH 03/20] prep_exclude: remove the artificial PATH_MAX limit

2014-05-07 Thread Nguyễn Thái Ngọc Duy
This also fixes the problem of silently ignoring .gitignore if the
full path exceeds PATH_MAX. Now add_excludes_from_file() will report
if it gets ENAMETOOLONG.
---
 dir.c | 47 ---
 dir.h |  2 +-
 2 files changed, 29 insertions(+), 20 deletions(-)

diff --git a/dir.c b/dir.c
index 7a83f70..c081754 100644
--- a/dir.c
+++ b/dir.c
@@ -795,12 +795,12 @@ static void prep_exclude(struct dir_struct *dir, const 
char *base, int baselen)
 */
while ((stk = dir->exclude_stack) != NULL) {
if (stk->baselen <= baselen &&
-   !strncmp(dir->basebuf, base, stk->baselen))
+   !strncmp(dir->basebuf.buf, base, stk->baselen))
break;
el = &group->el[dir->exclude_stack->exclude_ix];
dir->exclude_stack = stk->prev;
dir->exclude = NULL;
-   free((char *)el->src); /* see strdup() below */
+   free((char *)el->src); /* see strbuf_detach() below */
clear_exclude_list(el);
free(stk);
group->nr--;
@@ -810,8 +810,17 @@ static void prep_exclude(struct dir_struct *dir, const 
char *base, int baselen)
if (dir->exclude)
return;
 
+   /*
+* Lazy initialization. All call sites currently just
+* memset(dir, 0, sizeof(*dir)) before use. Changing all of
+* them seems lots of work for little benefit.
+*/
+   if (!dir->basebuf.alloc)
+   strbuf_init(&dir->basebuf, PATH_MAX);
+
/* Read from the parent directories and push them down. */
current = stk ? stk->baselen : -1;
+   strbuf_setlen(&dir->basebuf, current < 0 ? 0 : current);
while (current < baselen) {
struct exclude_stack *stk = xcalloc(1, sizeof(*stk));
const char *cp;
@@ -829,48 +838,47 @@ static void prep_exclude(struct dir_struct *dir, const 
char *base, int baselen)
stk->baselen = cp - base;
stk->exclude_ix = group->nr;
el = add_exclude_list(dir, EXC_DIRS, NULL);
-   memcpy(dir->basebuf + current, base + current,
-  stk->baselen - current);
+   strbuf_add(&dir->basebuf, base + current, stk->baselen - 
current);
+   assert(stk->baselen == dir->basebuf.len);
 
/* Abort if the directory is excluded */
if (stk->baselen) {
int dt = DT_DIR;
-   dir->basebuf[stk->baselen - 1] = 0;
+   dir->basebuf.buf[stk->baselen - 1] = 0;
dir->exclude = last_exclude_matching_from_lists(dir,
-   dir->basebuf, stk->baselen - 1,
-   dir->basebuf + current, &dt);
-   dir->basebuf[stk->baselen - 1] = '/';
+   dir->basebuf.buf, stk->baselen - 1,
+   dir->basebuf.buf + current, &dt);
+   dir->basebuf.buf[stk->baselen - 1] = '/';
if (dir->exclude &&
dir->exclude->flags & EXC_FLAG_NEGATIVE)
dir->exclude = NULL;
if (dir->exclude) {
-   dir->basebuf[stk->baselen] = 0;
dir->exclude_stack = stk;
return;
}
}
 
-   /* Try to read per-directory file unless path is too long */
-   if (dir->exclude_per_dir &&
-   stk->baselen + strlen(dir->exclude_per_dir) < PATH_MAX) {
-   strcpy(dir->basebuf + stk->baselen,
-   dir->exclude_per_dir);
+   /* Try to read per-directory file */
+   if (dir->exclude_per_dir) {
/*
 * dir->basebuf gets reused by the traversal, but we
 * need fname to remain unchanged to ensure the src
 * member of each struct exclude correctly
 * back-references its source file.  Other invocations
 * of add_exclude_list provide stable strings, so we
-* strdup() and free() here in the caller.
+* strbuf_detach() and free() here in the caller.
 */
-   el->src = strdup(dir->basebuf);
-   add_excludes_from_file_to_list(dir->basebuf,
-   dir->basebuf, stk->baselen, el, 1);
+   struct strbuf sb = STRBUF_INIT;
+   strbuf_addbuf(&sb, &dir->basebuf);
+   strbuf_addstr(&sb, dir->exclude_per_dir);
+   el->src = strbuf_detach(&sb, NULL);
+  

[PATCH 00/20] Untracked cache to speed up "git status"

2014-05-07 Thread Nguyễn Thái Ngọc Duy
First of all, thanks for pointing to many more big repos. I'll look at
them tomorrow. End-of-day report (or ranting :D) time.

The series now looks good enough for public eyes. I haven't run the
test suite with untracked cache on by default so confidence level is
not so high. Although I suspect racy timestamp issue will practically
disable the cache anyway.

The idea is as before, exploit directory mtime to cache untracked
files. MSDN tells me NTFS on Windows exposes the "good" dir mtime
behavior, which means this series could speed up Git on Windows (I
think Karsten fscache only deals with slow lstat, untracked files..).

It would be nice if Windows people could try and confirm this. This
could be a good point for untracked cache vs watchman (no windows
support, last time I checked). Usage is very simple, "git update-index
--untracked-cache" and you're ready. Do --no-untracked-cache to revert
back.

The peformance numbers on webkit look good. If we focus on
read_directory time only. Normally it takes 890ms. The first run with
untracked cache goes up to 922ms (filling up the cache, not counting
index write time). The next run goes down to 184ms (best case). The
gain is about 80%. lstat costs on directories only about 20ms out of
that 184ms, so I still need to see if I can lower that number further
down.

"git status" performance gain is less impressive of course. Only about
38% with refresh time now becomes the top offender. With
core.preloadindex on, the gain increases to 50%. There's still room
for improvement to maybe make it to 65% by reducing read time, I think.

But again we may not stay in the best case forever. The more dirs are
damaged, the slower it gets. At the end of the spectrum, all dirs are
damanged, we gain nothing but overhead. This is actually when watchman
shines, although projects that do that may need some improvements.

Another bad point for untracked cache is, the extension data is
so specifiec to core git algorithm that it probably cannot be reused
by other implementations. Again, watchman shines here.

Last note, this series conflicts with split-index due to the
write_cache API change, so not a candidate for 'pu' yet. The series
could also be fetched from

https://github.com/pclouds/git/commits/untracked-cache

except the last few timing/experimental patches.

Nguyễn Thái Ngọc Duy (20):
  dir.c: coding style fix
  dir.h: move struct exclude declaration to top level
  prep_exclude: remove the artificial PATH_MAX limit
  dir.c: optionally compute sha-1 of a .gitignore file
  untracked cache: record .gitignore information and dir hierarchy
  untracked cache: initial untracked cache validation
  untracked cache: invalidate dirs recursively if .gitignore changes
  untracked cache: record/validate dir mtime and reuse cached output
  untracked cache: mark what dirs should be recursed/saved
  untracked cache: don't open non-existent .gitignore
  untracked cache: save to an index extension
  untracked cache: load from UNTR index extension
  untracked cache: invalidate at index addition or removal
  untracked cache: print untracked statistics with $GIT_TRACE_UNTRACKED
  read-cache.c: split racy stat test to a separate function
  untracked cache: avoid racy timestamps
  status: support untracked cache
  update-index: manually enable or disable untracked cache
  update-index: test the system before enabling untracked cache
  t7063: tests for untracked cache

 .gitignore |   1 +
 Makefile   |   1 +
 builtin/commit.c   |   8 +
 builtin/update-index.c | 161 ++
 cache.h|   5 +
 dir.c  | 853 +++--
 dir.h  | 120 +++-
 read-cache.c   |  51 +-
 t/t7063-status-untracked-cache.sh (new +x) | 352 
 test-dump-untracked-cache.c (new)  |  61 +++
 unpack-trees.c |   7 +-
 wt-status.c|   6 +
 12 files changed, 1537 insertions(+), 89 deletions(-)
 create mode 100755 t/t7063-status-untracked-cache.sh
 create mode 100644 test-dump-untracked-cache.c

-- 
1.9.1.346.ga2b5940

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


  1   2   >