Re: [PATCH 1/3] Revert make error()'s constant return value more visible

2014-05-05 Thread Jeff King
On Mon, May 05, 2014 at 12:45:30AM -0500, Felipe Contreras wrote:

 Jeff King wrote:
  On Sun, May 04, 2014 at 01:12:53AM -0500, Felipe Contreras wrote:
  
   So it looks like gcc is smarter now, and in trying to fix a few warnings
   we generated hundreds more.
   
   This reverts commit e208f9cc7574f5980faba498d0aa30b4defeb34f.
  
  And now we've gone the other way, and re-enabled the initial warnings.
  Can we come up with a solution that helps both cases?
 
 What initial warnings? As I explained already I don't get any warnings
 with this patch series in gcc 4.9.0.

The few warnings in your statement quoted above.

You could try reading the commit message of the commit you are
reverting, which explains it, but the short answer is: try compiling
with -O3.

-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 1/3] Revert make error()'s constant return value more visible

2014-05-05 Thread Felipe Contreras
Jeff King wrote:
 On Mon, May 05, 2014 at 12:45:30AM -0500, Felipe Contreras wrote:
 
  Jeff King wrote:
   On Sun, May 04, 2014 at 01:12:53AM -0500, Felipe Contreras wrote:
   
So it looks like gcc is smarter now, and in trying to fix a few warnings
we generated hundreds more.

This reverts commit e208f9cc7574f5980faba498d0aa30b4defeb34f.
   
   And now we've gone the other way, and re-enabled the initial warnings.
   Can we come up with a solution that helps both cases?
  
  What initial warnings? As I explained already I don't get any warnings
  with this patch series in gcc 4.9.0.
 
 The few warnings in your statement quoted above.
 
 You could try reading the commit message of the commit you are
 reverting, which explains it, but the short answer is: try compiling
 with -O3.

Sigh. And I'm the one with the abrasive style of communication.

-- 
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 1/3] Revert make error()'s constant return value more visible

2014-05-05 Thread Jeff King
On Mon, May 05, 2014 at 01:14:43AM -0500, Felipe Contreras wrote:

 Jeff King wrote:
  On Mon, May 05, 2014 at 12:45:30AM -0500, Felipe Contreras wrote:
  
   Jeff King wrote:
On Sun, May 04, 2014 at 01:12:53AM -0500, Felipe Contreras wrote:

 So it looks like gcc is smarter now, and in trying to fix a few 
 warnings
 we generated hundreds more.
 
 This reverts commit e208f9cc7574f5980faba498d0aa30b4defeb34f.

And now we've gone the other way, and re-enabled the initial warnings.
Can we come up with a solution that helps both cases?
   
   What initial warnings? As I explained already I don't get any warnings
   with this patch series in gcc 4.9.0.
  
  The few warnings in your statement quoted above.
  
  You could try reading the commit message of the commit you are
  reverting, which explains it, but the short answer is: try compiling
  with -O3.
 
 Sigh. And I'm the one with the abrasive style of communication.

I apologize if that seemed abrasive. I am slightly annoyed that you
seemed to be reverting my commit without understanding (or dealing with)
the problem that the original fixed.

But I was _also_ trying to point you in the right direction by directing
you to -O3. Do you see the problem now?  And did you look at the
follow-up patch I sent?

-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 10/12] MINGW: compat/poll/poll.c: undef NOGDI

2014-05-05 Thread Stepan Kasal
Hello,

On Mon, May 05, 2014 at 12:55:52AM +0400, Marat Radchenko wrote:
 On Sun, May 04, 2014 at 08:52:44PM +0200, Stepan Kasal wrote:
  is really a work around: it would be in effect only for MinGW-W64,
  and the comment would explain that this is a hack to work around the
  bug.  
 
 Workarounds do not have to be ugly and full of #ifdef's.

I'm afraid they have to.  If you just select one of the reasonable
variants, without noting that the other ones would trigger a bug, you
are lying a trap for future contributors.

  If you manage to change the defs for poll.c without changing its
  content, no one could tell you to report to gnulib first.
 
 v1 does exactly this.

Yes, but it changes the define for other configurations as well
(MSVC, mingw 32bit).  I would suggest something along the change
below.

What do you think?

Stepan

diff --git a/config.mak.uname b/config.mak.uname
index 82b8dff..446dd41 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -508,7 +508,11 @@ ifneq (,$(findstring MINGW,$(uname_S)))
NO_POSIX_GOODIES = UnfortunatelyYes
DEFAULT_HELP_FORMAT = html
NO_D_INO_IN_DIRENT = YesPlease
-   COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -D_USE_32BIT_TIME_T -DNOGDI 
-Icompat -Icompat/win32
+   COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -D_USE_32BIT_TIME_T -Icompat 
-Icompat/win32
+ifneq ($(uname_M),x86_64)
+   # MinGW-W64  x.y headers do not provide MsgWaitForMultipleObjects with 
NOGDI
+   COMPAT_CFLAGS += -DNOGDI
+endif
COMPAT_CFLAGS += -DSTRIP_EXTENSION=\.exe\
COMPAT_OBJS += compat/mingw.o compat/winansi.o \
compat/win32/pthread.o compat/win32/syslog.o \
diff --git a/git-compat-util.h b/git-compat-util.h
index e6de32c..29a8afd 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -92,6 +92,9 @@
 #define WIN32_LEAN_AND_MEAN  /* stops windows.h including winsock.h */
 #include winsock2.h
 #include windows.h
+/* We cannot define NOGDI on MinGW-W64, so we unfortunately include
+   wingdi.h.  It then defines ERROR=0, undef it to avoid conflicts */
+#undef ERROR
 #define GIT_WINDOWS_NATIVE
 #endif
 
--
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] Revert make error()'s constant return value more visible

2014-05-05 Thread Felipe Contreras
Jeff King wrote:
 On Mon, May 05, 2014 at 01:14:43AM -0500, Felipe Contreras wrote:
  Jeff King wrote:

   You could try reading the commit message of the commit you are
   reverting, which explains it, but the short answer is: try compiling
   with -O3.
  
  Sigh. And I'm the one with the abrasive style of communication.
 
 I apologize if that seemed abrasive. I am slightly annoyed that you
 seemed to be reverting my commit without understanding (or dealing with)
 the problem that the original fixed.

The original problem happens only with -O3, I tried only with -O2 and
didn't see any problems.

If we have a) code that fixes a couple warnings with -O3 but introduces
hundreds with -O2, vs. b) code that has only a comple warnings with -O3,
I'd go for b) any day.

 But I was _also_ trying to point you in the right direction by directing
 you to -O3. Do you see the problem now?  And did you look at the
 follow-up patch I sent?

Yes, I see the problem now with -O3. And yes, I looked at the patch you
sent. I haven't tested it but I bet it would sove both problems.

Cheers.

-- 
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 10/12] MINGW: compat/poll/poll.c: undef NOGDI

2014-05-05 Thread Felipe Contreras
Stepan Kasal wrote:
 diff --git a/config.mak.uname b/config.mak.uname
 index 82b8dff..446dd41 100644
 --- a/config.mak.uname
 +++ b/config.mak.uname
 @@ -508,7 +508,11 @@ ifneq (,$(findstring MINGW,$(uname_S)))
   NO_POSIX_GOODIES = UnfortunatelyYes
   DEFAULT_HELP_FORMAT = html
   NO_D_INO_IN_DIRENT = YesPlease
 - COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -D_USE_32BIT_TIME_T -DNOGDI 
 -Icompat -Icompat/win32
 + COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -D_USE_32BIT_TIME_T -Icompat 
 -Icompat/win32
 +ifneq ($(uname_M),x86_64)
 + # MinGW-W64  x.y headers do not provide MsgWaitForMultipleObjects with 
 NOGDI

MinGW-w64 != x86_64; it provides a i686 compiler as well.

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


git fast-import: how to prevent incremental commit with no changes

2014-05-05 Thread Timo Teras
Hi,

I'm trying to script a setup that would periodically import a tarball
to git with fast-import. But things do not always change, so I'd like
fast-import to be able to not do the commit in case there is no change.

That is, I'm constructing the commit with deleteall + importing each
object by mark after that. Now, in case nothing changed, fast-import
will happily create an empty commit for me.

Would it be possible to add some flag that would make commit fail in
case nothing changed?

Any suggestions how to do this?

Thanks,
Timo
--
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/9] Define a structure for object IDs.

2014-05-05 Thread David Kastrup
Andreas Schwab sch...@linux-m68k.org writes:

 Johannes Sixt j...@kdbg.org writes:

 Isn't internal padding only allowed between members to achieve correct
 alignment of later members, and at the end only sufficient padding so
 that members are aligned correctly when the struct is part of an array?

 The standard allows arbitrary internal padding, it doesn't have to be
 minimal.

What the standard does guarantee is that a pointer to a struct can be
cast to a pointer to its first member and vice versa.  It does not as
far as I can see guarantee that a pointer to something of the same type
of its first member can be converted to a pointer to a struct even if
the struct only contains a member of such type.

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


Re: [PATCH 1/9] Define a structure for object IDs.

2014-05-05 Thread Andreas Schwab
David Kastrup d...@gnu.org writes:

 It does not as far as I can see guarantee that a pointer to something
 of the same type of its first member can be converted to a pointer to
 a struct even if the struct only contains a member of such type.

This sentence doesn't make any sense.  If you have an object of struct
type then any pointer to the first member of the object can only be a
pointer to the one and same object.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
And now for something completely different.
--
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/9] Define a structure for object IDs.

2014-05-05 Thread James Denholm
On 5 May 2014 19:23:06 GMT+10:00, Andreas Schwab sch...@linux-m68k.org wrote:
David Kastrup d...@gnu.org writes:

 It does not as far as I can see guarantee that a pointer to something
 of the same type of its first member can be converted to a pointer to
 a struct even if the struct only contains a member of such type.

This sentence doesn't make any sense.  If you have an object of struct
type then any pointer to the first member of the object can only be a
pointer to the one and same object.

I think what David means is that a pointer to a wrapper
can be derefed into its internal, sure, but an object of
that internal type can't necessarily pretend to be a
wrapper.

That said, obviously I'm not David, so I could be wrong.
That's what I got from his statement, though.

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: [PATCH 1/9] Define a structure for object IDs.

2014-05-05 Thread David Kastrup
Andreas Schwab sch...@linux-m68k.org writes:

 David Kastrup d...@gnu.org writes:

 It does not as far as I can see guarantee that a pointer to something
 of the same type of its first member can be converted to a pointer to
 a struct even if the struct only contains a member of such type.

 This sentence doesn't make any sense.

I disagree.

 If you have an object of struct type

Your premise is _not_ assumed in my statement.  My premise was a
pointer to something of the same type of [the struct's] first member.
That does quite explicitly _not_ state that an object of struct type is
in existence.

 then any pointer to the first member of the object can only be a
 pointer to the one and same object.

The case we are talking about is basically passing a pointer to some
actual bonafide toplevel unsigned char [20] object to a routine that
expects a pointer to a struct _only_ containing one such
unsigned char [20] element.

This is the situation we have to deal with if a caller has not been
converted to using such a struct, but the called function does.

More seriously, this is the situation we have to deal with when our SHA1
is actually embedded in some header or whatever else that is actually
available only inside of a larger byte buffer.

In that case, the standard does not permit us converting the address
where that SHA1 is into a pointer to struct.  It may well be that this
will fall under the let's ignore the standard and write for sensible
compilers/architectures dictum, but if it doesn't, it might be
necessary to first copy the data to a struct before passing it to
routines expecting a pointer to struct.

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


Re: GIT, libcurl and GSS-Negotiate

2014-05-05 Thread Ivo Bellin Salarin
Well, I'm on Windows.
using `git version 1.9.2.msysgit.0`.

You can find all the exchanges, recorded with wireshark, of the
following usecases:
* git vanilla (not working),
* VisualStudio2013 with libgit (working)
* curl (--ntlm, working)
* curl (--negotiate, not working)

They're available on
[github](https://github.com/nilleb/my-documents/tree/master/msysgit%23git%2C%20issue-171).



On Sat, Apr 26, 2014 at 7:47 PM, brian m. carlson
sand...@crustytoothpaste.net wrote:
 On Thu, Apr 24, 2014 at 07:17:36PM +0200, Ivo Bellin Salarin wrote:
 To shortly resume it, the problem is that:
 * when the authentication method (WWW-Authenticate) is Negotiate AND
 * when the server proposes a NTLMSSP_CHALLENGE in response of the
 client's NTLMSSP_NEGOTIATE,
 = libcurl yields an Authentication problem. Ignoring this.\n
 And the communication is closed.

 At this point, in a normal communication, the client should send a
 NTLMSSP_AUTH containing a Kerberos ticket.

 Having seen the libcurl source code, I think we're passing through the
 lines  from 776 to 780 of
 [http.c](https://github.com/bagder/curl/blob/2e57c7e0fcfb9214b2a9dfa8b3da258ded013b8a/lib/http.c).
 Some guy, on the github issue page, has suggested that this could be
 related to an update of libcurl, when git was at its 1.8.2 version.

 I'm not debugging libcurl, and I can't reproduce this problem @home.
 So, has somebody already experienced the same problem? Is there a
 solution?

 I'm personally using Git with GSS-Negotiate (and MIT Kerberos 5) and it
 does seem to work correctly for me.  For large pushes, your server (and
 any intermediate proxies) will need to support 100 Continue properly, as
 there's simply no other way to make it work.

 What version of curl are you using (and what distro if you didn't
 compile it yourself)?  Also, can you post output of an attempt to push
 with GIT_CURL_VERBOSE=1?

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



-- 
http://www.nilleb.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: [PATCH] Bump core.deltaBaseCacheLimit to 96m

2014-05-05 Thread Duy Nguyen
On Mon, May 5, 2014 at 12:13 AM, David Kastrup d...@gnu.org wrote:
 The default of 16m causes serious thrashing for large delta chains
 combined with large files.

 Here are some benchmarks (pu variant of git blame):

 time git blame -C src/xdisp.c /dev/null

...

 diff --git a/Documentation/config.txt b/Documentation/config.txt
 index 1932e9b..21a3c86 100644
 --- a/Documentation/config.txt
 +++ b/Documentation/config.txt
 @@ -489,7 +489,7 @@ core.deltaBaseCacheLimit::
 to avoid unpacking and decompressing frequently used base
 objects multiple times.
  +
 -Default is 16 MiB on all platforms.  This should be reasonable
 +Default is 96 MiB on all platforms.  This should be reasonable
  for all users/operating systems, except on the largest projects.
  You probably do not need to adjust this value.

So emacs.git falls exactly into the except on the largest projects
part. Would it make more sense to advise git devs to set this per repo
instead? The majority of (open source) repositories out there are
small if I'm not mistaken. Of those few big repos, we could have a
section listing all the tips and tricks to tune git. This is one of
them. Index v4 and sparse checkout are some other. In future, maybe
watchman support, split index and untracked cache as well.
-- 
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: [PATCH] Bump core.deltaBaseCacheLimit to 96m

2014-05-05 Thread Duy Nguyen
On Mon, May 5, 2014 at 5:26 PM, Duy Nguyen pclo...@gmail.com wrote:
 part. Would it make more sense to advise git devs to set this per repo

s/advise git devs/advise emacs devs/
-- 
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: [PATCH 1/9] Define a structure for object IDs.

2014-05-05 Thread Michael Haggerty
On 05/05/2014 11:50 AM, David Kastrup wrote:
 The case we are talking about is basically passing a pointer to some
 actual bonafide toplevel unsigned char [20] object to a routine that
 expects a pointer to a struct _only_ containing one such
 unsigned char [20] element.
 
 This is the situation we have to deal with if a caller has not been
 converted to using such a struct, but the called function does.

If the rewrite is done by first changing data structures and then
changing functions in caller - callee order then (1) the deltas can be
pretty small, and (2) such illegal casting should be unnecessary.

 More seriously, this is the situation we have to deal with when our SHA1
 is actually embedded in some header or whatever else that is actually
 available only inside of a larger byte buffer.
 
 In that case, the standard does not permit us converting the address
 where that SHA1 is into a pointer to struct.  It may well be that this
 will fall under the let's ignore the standard and write for sensible
 compilers/architectures dictum, but if it doesn't, it might be
 necessary to first copy the data to a struct before passing it to
 routines expecting a pointer to struct.

This sounds dangerous even for a sensible compiler.  For example, I
can imagine that a sensible compiler might make the assumption that a
sha1 field that it knows was obtained from oid-sha1 is word-aligned,
and generate optimized code based on that assumption, even though it
otherwise wouldn't have had trouble working with unaligned (unsigned
char *) pointers.

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: [PATCH/RFC] Makefile: do not depend on curl-config

2014-05-05 Thread Erik Faye-Lund
On Wed, Apr 30, 2014 at 9:46 PM, Sebastian Schuberth
sschube...@gmail.com wrote:
 On Wed, Apr 30, 2014 at 6:52 PM, Johannes Schindelin
 johannes.schinde...@gmx.de wrote:

 We can keep this patch in the msysGit repo for the 2.0 release.

 FWIW the plan is to switch to mingwGitDevEnv for the 2.0 release. It is
 not quite clear as of yet how patches will be managed with said
 environment.

 The environment is just that: The environment to build Git for
 Windows. This means that patches on top of Git for Windows could still
 be maintained in msysgit/git (or a fork thereof) on GitHub.

Thanks for the heads up. Even so, are you guys OK with me pushing this
patch to our downstream repo?
--
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/RFC] Makefile: do not depend on curl-config

2014-05-05 Thread Sebastian Schuberth
On Mon, May 5, 2014 at 12:53 PM, Erik Faye-Lund kusmab...@gmail.com wrote:

 FWIW the plan is to switch to mingwGitDevEnv for the 2.0 release. It is
 not quite clear as of yet how patches will be managed with said
 environment.

 The environment is just that: The environment to build Git for
 Windows. This means that patches on top of Git for Windows could still
 be maintained in msysgit/git (or a fork thereof) on GitHub.

 Thanks for the heads up. Even so, are you guys OK with me pushing this
 patch to our downstream repo?

Fine with me.

-- 
Sebastian Schuberth
--
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] Bump core.deltaBaseCacheLimit to 96m

2014-05-05 Thread Matthieu Moy
Duy Nguyen pclo...@gmail.com writes:

 On Mon, May 5, 2014 at 12:13 AM, David Kastrup d...@gnu.org wrote:
 The default of 16m causes serious thrashing for large delta chains
 combined with large files.

 Here are some benchmarks (pu variant of git blame):

 time git blame -C src/xdisp.c /dev/null

 ...

 diff --git a/Documentation/config.txt b/Documentation/config.txt
 index 1932e9b..21a3c86 100644
 --- a/Documentation/config.txt
 +++ b/Documentation/config.txt
 @@ -489,7 +489,7 @@ core.deltaBaseCacheLimit::
 to avoid unpacking and decompressing frequently used base
 objects multiple times.
  +
 -Default is 16 MiB on all platforms.  This should be reasonable
 +Default is 96 MiB on all platforms.  This should be reasonable
  for all users/operating systems, except on the largest projects.
  You probably do not need to adjust this value.

 So emacs.git falls exactly into the except on the largest projects
 part. Would it make more sense to advise git devs to set this per repo
 instead?

What's the impact of changing the default for small projects?

My guess is that changing from 16 to 96Mb is just following Moore's law.
Machines average RAM has increased a lot since the time 16Mb has been
chosen, and few people would actually notice the difference in RAM usage
nowadays.

If increasing the default does not harm small projects and benefits to
big projects, then we should obviously go this way.

(perhaps adding advices for people using Git on machines with low RAM)

-- 
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/9] Define a structure for object IDs.

2014-05-05 Thread Andreas Schwab
David Kastrup d...@gnu.org writes:

 Your premise is _not_ assumed in my statement.  My premise was a
 pointer to something of the same type of [the struct's] first member.
 That does quite explicitly _not_ state that an object of struct type is
 in existence.

So you are not taking about struct object_id, and it's irrelevant to
this thread.

This thread is about objects of type struct object_id, and their address
is always the same as the address of its first member.  Nothing else is
relevant.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
And now for something completely different.
--
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] Bump core.deltaBaseCacheLimit to 96m

2014-05-05 Thread David Kastrup
Duy Nguyen pclo...@gmail.com writes:

 On Mon, May 5, 2014 at 12:13 AM, David Kastrup d...@gnu.org wrote:
 The default of 16m causes serious thrashing for large delta chains
 combined with large files.

 Here are some benchmarks (pu variant of git blame):

 time git blame -C src/xdisp.c /dev/null

 ...

 diff --git a/Documentation/config.txt b/Documentation/config.txt
 index 1932e9b..21a3c86 100644
 --- a/Documentation/config.txt
 +++ b/Documentation/config.txt
 @@ -489,7 +489,7 @@ core.deltaBaseCacheLimit::
 to avoid unpacking and decompressing frequently used base
 objects multiple times.
  +
 -Default is 16 MiB on all platforms.  This should be reasonable
 +Default is 96 MiB on all platforms.  This should be reasonable
  for all users/operating systems, except on the largest projects.
  You probably do not need to adjust this value.

 So emacs.git falls exactly into the except on the largest projects
 part.

git gc --aggressive has been used/recommended for _all_ projects
regularly, leading to delta chains with a length of 250.  So this delta
chain size is not exceptional but will eventually occur in any archive
that has been created and maintained according to the recommendations of
Git's documentation (which recommends gc --aggressive every few hundreds
of revisions).  I was illustrating the effect on a file of size 1MB.
That's not an egregiously large file either.

96MB is the point of diminuishing returns for this case which is _6_
times larger than the current default and _small_ in comparison with the
memory installed on developer machines nowadays.  Similar slowdowns
occur with other examples.  Git will with the current defaults accept
files of 512Mb size into its compression scheme (and thus its core
memory) before punting.

The current delteBaseCacheLimit of 16Mb is rather ridiculous in
particular with the pre-2.0 settings for gc --aggressive and causes
serious performance degration.  It was actually ridiculous even 10 years
ago.

 Would it make more sense to advise git devs to set this per repo
 instead? The majority of (open source) repositories out there are
 small if I'm not mistaken. Of those few big repos, we could have a
 section listing all the tips and tricks to tune git. This is one of
 them. Index v4 and sparse checkout are some other. In future, maybe
 watchman support, split index and untracked cache as well.

Shrug.  The last version of the patch was refused because of wanting
more evidence.  I added the evidence.

And I have it on record in the mailing list and can point to it when
people ask me why Git is so slow for git blame in comparison to other
version control systems in spite of my purporting to having improved it.

I'm definitely not going to jump through any more hoops here.  I don't
see a point in this kind of spectacle.

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


Re: [PATCH 0/3] Use ref transactions for fetch

2014-05-05 Thread Michael Haggerty
On 04/22/2014 08:45 PM, Ronnie Sahlberg wrote:
 This change is based on the previous ref transaction patches.
 This is sent as a separate patch series since it implements a lot more
 non-trivial changes to the behaviour than the previous patches and thus can
 use more detailed review.
 
 Update fetch.c to use ref transactions when performing updates. Use a single
 ref transaction for all updates and only commit the transaction if all other
 checks and oeprations have been successful. This makes the ref updates during
 a fetch (mostly) atomic.

Is this always an improvement?  What kind of checks are there that might
fail?

It would be pretty annoying to spend a lot of time fetching a big pack,
only to have the fetch fail because one reference out of many couldn't
be updated.  This would force the user to download the entire pack
again, whereas if the successful reference updates had been allowed,
then probably most or all of the second download would have been avoidable.

On the other hand, if a reference was renamed on the remote side,
allowing a partial reference update could cause history to be discarded
locally if the old name's delete was accepted but the new name's
addition was rejected.  This wouldn't be the end of the world, because
the history is presumably still available remotely to fetch again, but
it's not ideal either.

I'm not sure myself what I would prefer, but I wanted to point out that
it is IMO not obvious that atomicity here is an improvement.

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: [PATCH 1/9] Define a structure for object IDs.

2014-05-05 Thread David Kastrup
Andreas Schwab sch...@linux-m68k.org writes:

 David Kastrup d...@gnu.org writes:

 Your premise is _not_ assumed in my statement.  My premise was a
 pointer to something of the same type of [the struct's] first member.
 That does quite explicitly _not_ state that an object of struct type is
 in existence.

 So you are not taking about struct object_id, and it's irrelevant to
 this thread.

 This thread is about objects of type struct object_id, and their address
 is always the same as the address of its first member.  Nothing else is
 relevant.

Have it your way.  I am too old for selective quotation games.

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


Re: [PATCH] Bump core.deltaBaseCacheLimit to 96m

2014-05-05 Thread Duy Nguyen
On Mon, May 5, 2014 at 6:03 PM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
 -Default is 16 MiB on all platforms.  This should be reasonable
 +Default is 96 MiB on all platforms.  This should be reasonable
  for all users/operating systems, except on the largest projects.
  You probably do not need to adjust this value.

 So emacs.git falls exactly into the except on the largest projects
 part. Would it make more sense to advise git devs to set this per repo
 instead?

 What's the impact of changing the default for small projects?

Good question. With git log --patch or something like that, we could
use up to the limit, which is now 96MB. On modern machines that's
probably nothing.

 My guess is that changing from 16 to 96Mb is just following Moore's law.
 Machines average RAM has increased a lot since the time 16Mb has been
 chosen, and few people would actually notice the difference in RAM usage
 nowadays.

 If increasing the default does not harm small projects and benefits to
 big projects, then we should obviously go this way.

I wrote without thinking it through. I agree with you.

 (perhaps adding advices for people using Git on machines with low RAM)
-- 
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: [msysGit] Re: [PATCH 10/12] MINGW: compat/poll/poll.c: undef NOGDI

2014-05-05 Thread Stepan Kasal
On Mon, May 05, 2014 at 02:32:11AM -0500, Felipe Contreras wrote:
 Stepan Kasal wrote:
  +ifneq ($(uname_M),x86_64)
  +   # MinGW-W64  x.y headers do not provide MsgWaitForMultipleObjects with 
  NOGDI
 
 MinGW-w64 != x86_64; it provides a i686 compiler as well.

thanks for correcting me.  The diff was just a quick sketch.
Every workaround should say what bug is working around, otherwise it
is destined to become a superstition in the future.

Stepan
--
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 v6 00/42] Use ref transactions for all ref updates

2014-05-05 Thread Michael Haggerty
On 05/01/2014 10:37 PM, Ronnie Sahlberg wrote:
 This patch series is based on next and expands on the transaction API. [...]

Meta-comment:

Ronnie,

It seems like successive versions of this patch series are growing not
only in maturity but also in breadth.  That makes it harder to review them.

I, for one, would prefer that a patch series cover a roughly fixed set
of changes [1], so that all of the patches in a version of the series
are at roughly the same level of maturity.  That way, the whole series
can progress from is this a good idea? to is the implementation
correct? to are all the details right? at roughly the same time, and
then Junio can merge the branch, locking in that bit of progress.  While
this is happening, other series can be making their way through other
stages of the pipeline.

When new patches are added to an old series, then they delay the merge
of the older patches, even if those are ripe.  Plus, it makes it harder
for reviewers to keep track of the maturity level of each patch and to
read off how the older patches have changed.  It makes the patch series
a moving target.

There's no need to re-split this patch series, but please take this wish
into account in the future.

Thanks,
Michael

[1] Of course, if a patch series has to grow to make the *existing*
changes correct, then that's perfectly OK.

-- 
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: [PATCH v6 08/42] refs.c: change ref_transaction_update() to do error checking and return status

2014-05-05 Thread Michael Haggerty
On 05/01/2014 10:37 PM, Ronnie Sahlberg wrote:
 Update ref_transaction_update() do some basic error checking and return
 true on error. Update all callers to check ref_transaction_update() for error.
 There are currently no conditions in _update that will return error but there
 will be in the future.

I would change s/true/non-zero/, because error return values are not
just boolean values; the error values sometimes encode the type of error
that occurred.

 
 Signed-off-by: Ronnie Sahlberg sahlb...@google.com
 ---
  builtin/update-ref.c | 10 ++
  refs.c   |  9 +++--
  refs.h   | 10 +-
  3 files changed, 18 insertions(+), 11 deletions(-)
 
 diff --git a/builtin/update-ref.c b/builtin/update-ref.c
 index 2bef2a0..59c4d6b 100644
 --- a/builtin/update-ref.c
 +++ b/builtin/update-ref.c
 @@ -197,8 +197,9 @@ static const char *parse_cmd_update(struct strbuf *input, 
 const char *next)
   if (*next != line_termination)
   die(update %s: extra input: %s, refname, next);
  
 - ref_transaction_update(transaction, refname, new_sha1, old_sha1,
 -update_flags, have_old);
 + if (ref_transaction_update(transaction, refname, new_sha1, old_sha1,
 +update_flags, have_old))
 + die(update %s: failed, refname);
  
   update_flags = 0;
   free(refname);
 @@ -286,8 +287,9 @@ static const char *parse_cmd_verify(struct strbuf *input, 
 const char *next)
   if (*next != line_termination)
   die(verify %s: extra input: %s, refname, next);
  
 - ref_transaction_update(transaction, refname, new_sha1, old_sha1,
 -update_flags, have_old);
 + if (ref_transaction_update(transaction, refname, new_sha1, old_sha1,
 +update_flags, have_old))
 + die(failed transaction update for %s, refname);
  
   update_flags = 0;
   free(refname);
 diff --git a/refs.c b/refs.c
 index 308e13e..1a903fb 100644
 --- a/refs.c
 +++ b/refs.c
 @@ -,19 +,24 @@ static struct ref_update *add_update(struct 
 ref_transaction *transaction,
   return update;
  }
  
 -void ref_transaction_update(struct ref_transaction *transaction,
 +int ref_transaction_update(struct ref_transaction *transaction,
   const char *refname,
   const unsigned char *new_sha1,
   const unsigned char *old_sha1,
   int flags, int have_old)
  {
 - struct ref_update *update = add_update(transaction, refname);
 + struct ref_update *update;
 +
 + if (have_old  !old_sha1)
 + die(have_old is true but old_sha1 is NULL);

This new check is orthogonal to the rest of the patch, isn't it?

  
 + update = add_update(transaction, refname);
   hashcpy(update-new_sha1, new_sha1);
   update-flags = flags;
   update-have_old = have_old;
   if (have_old)
   hashcpy(update-old_sha1, old_sha1);
 + return 0;
  }
  
  void ref_transaction_create(struct ref_transaction *transaction,
 diff --git a/refs.h b/refs.h
 index bc7715e..0364a3e 100644
 --- a/refs.h
 +++ b/refs.h
 @@ -237,11 +237,11 @@ void ref_transaction_rollback(struct ref_transaction 
 *transaction);
   * that the reference should have had before the update, or zeros if
   * it must not have existed beforehand.
   */

Please update the docstring above to explain the return value.

 -void ref_transaction_update(struct ref_transaction *transaction,
 - const char *refname,
 - const unsigned char *new_sha1,
 - const unsigned char *old_sha1,
 - int flags, int have_old);
 +int ref_transaction_update(struct ref_transaction *transaction,
 +const char *refname,
 +const unsigned char *new_sha1,
 +const unsigned char *old_sha1,
 +int flags, int have_old);
  
  /*
   * Add a reference creation to transaction.  new_sha1 is the value
 

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: [PATCH] git-p4: format-patch to diff-tree change breaks binary patches

2014-05-05 Thread Pete Wyckoff
tolga.cey...@gmail.com wrote on Fri, 02 May 2014 22:40 -0700:
 
 
 This is the error message git-apply emits in this case:
 
 error: cannot apply binary patch to 'filename' without full index line
 error: filename: patch does not apply
 
 Cheers,
 Tolga
 
 Any feedback is appreciated.

Sorry, travel delay.  This explanation is pretty
straight-forward, thanks.

Suggest you include it in the commit message along with the
other text you had, and resend to the list, cc me and junio.
Oh, and include an ack:

Acked-by: Pete Wyckoff p...@padd.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: [msysGit] Re: [PATCH/RFC] Makefile: do not depend on curl-config

2014-05-05 Thread Thomas Braun
Am 05.05.2014 12:53, schrieb Erik Faye-Lund:
 On Wed, Apr 30, 2014 at 9:46 PM, Sebastian Schuberth
 sschube...@gmail.com wrote:
 On Wed, Apr 30, 2014 at 6:52 PM, Johannes Schindelin
 johannes.schinde...@gmx.de wrote:

 We can keep this patch in the msysGit repo for the 2.0 release.

 FWIW the plan is to switch to mingwGitDevEnv for the 2.0 release. It is
 not quite clear as of yet how patches will be managed with said
 environment.

 The environment is just that: The environment to build Git for
 Windows. This means that patches on top of Git for Windows could still
 be maintained in msysgit/git (or a fork thereof) on GitHub.
 
 Thanks for the heads up. Even so, are you guys OK with me pushing this
 patch to our downstream repo?

Yes!


--
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 v6 00/42] Use ref transactions for all ref updates

2014-05-05 Thread Ronnie Sahlberg
On Mon, May 5, 2014 at 5:57 AM, Michael Haggerty mhag...@alum.mit.edu wrote:
 On 05/01/2014 10:37 PM, Ronnie Sahlberg wrote:
 This patch series is based on next and expands on the transaction API. [...]

 Meta-comment:

 Ronnie,

 It seems like successive versions of this patch series are growing not
 only in maturity but also in breadth.  That makes it harder to review them.

 I, for one, would prefer that a patch series cover a roughly fixed set
 of changes [1], so that all of the patches in a version of the series
 are at roughly the same level of maturity.  That way, the whole series
 can progress from is this a good idea? to is the implementation
 correct? to are all the details right? at roughly the same time, and
 then Junio can merge the branch, locking in that bit of progress.  While
 this is happening, other series can be making their way through other
 stages of the pipeline.

 When new patches are added to an old series, then they delay the merge
 of the older patches, even if those are ripe.  Plus, it makes it harder
 for reviewers to keep track of the maturity level of each patch and to
 read off how the older patches have changed.  It makes the patch series
 a moving target.

 There's no need to re-split this patch series, but please take this wish
 into account in the future.


Understood.
--
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-05 Thread Richard Hansen
On 2014-05-03 06:00, John Szakmeister wrote:
 FWIW, at my company, we took another approach.  We introduced a `git
 ffwd` command that fetches from all remotes, and fast-forwards all
 your local branches that are tracking a remote, and everyone on the
 team uses it all the time.  It should be said this team also likes to
 use Git bare-metal, because they like knowing how things work
 out-of-the-box.  But they all use the command because it's so
 convenient.

I also wrote a script to fast-forward all local branches to their
configured upstream refs.  I finally got around to uploading it
somewhere public:

   https://github.com/richardhansen/git-update-branch

I use it in my 'git up' alias:

   git config --global alias.up \
   '!git remote update -p; git update-branch -a'

If there's interest I can tweak the style to conform to
Documentation/CodingGuidelines and stick it in contrib/ or something.

-Richard
--
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: Watchman support for git

2014-05-05 Thread David Turner
On Fri, 2014-05-02 at 22:40 -0500, Felipe Contreras wrote:
 David Turner wrote:
  On Fri, 2014-05-02 at 18:20 -0500, Felipe Contreras wrote:
   dturner@ wrote:
Test repository 1: Linux

Linux is about 45k files in 3k directories.  The average length of a
filename is about 32 bytes.

Git status timing:
no watchman: 125ms
watchman: 90ms
   
   That's very interesting. Do you get similar improvements when doing
   something similar in Merurial (watchman vs . no watchman).
  
  I have not tried it.  My understanding is that this is why Facebook
  wrote Watchman and added support for it to Mercurial, so I would assume
  that the improvements are at least this good.
 
 Yeah, my bet is that they are actually much better (because Mercurial
 can't be so optimized as Git).

 I'm interested in this number because if watchman in Git is improving it
 by 30%, but in Mercurial it's improving it by 100% (made up number),
 therefore it makes sens that you might want it more if you are using hg,
 but not so much if you are using git.
 
 Also, if similar repositories with Mercurial+watchman are actually
 faster than Git+watchman, that means that there's room for improvement
 in your implementation. This is not a big issue at this point of the
 process, just something nice to know.

Converting git repos to hg seems to be incredibly slow.  (I have not yet
tried doing it with git-remote-hg).  But I did find a hg repository for
linux: https://bitbucket.org/orzel/linux-kernel-stable

On this repo, I get:
hg without watchman: 620ms
hg with watchman: 264ms
(compared to 125ms/90ms for git).

The number of syscalls is, perhaps also interesting:
no watchman / with watchman
hg  3   /  5421
git 59180   /  599 

(About 1/3 of hg's syscalls with watchman seem to be loading Python
stuff)

--
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: Watchman support for git

2014-05-05 Thread Felipe Contreras
David Turner wrote:
 On Fri, 2014-05-02 at 22:40 -0500, Felipe Contreras wrote:
  David Turner wrote:
   On Fri, 2014-05-02 at 18:20 -0500, Felipe Contreras wrote:
dturner@ wrote:
 Test repository 1: Linux
 
 Linux is about 45k files in 3k directories.  The average length of a
 filename is about 32 bytes.
 
 Git status timing:
 no watchman: 125ms
 watchman: 90ms

That's very interesting. Do you get similar improvements when doing
something similar in Merurial (watchman vs . no watchman).
   
   I have not tried it.  My understanding is that this is why Facebook
   wrote Watchman and added support for it to Mercurial, so I would assume
   that the improvements are at least this good.
  
  Yeah, my bet is that they are actually much better (because Mercurial
  can't be so optimized as Git).
 
  I'm interested in this number because if watchman in Git is improving it
  by 30%, but in Mercurial it's improving it by 100% (made up number),
  therefore it makes sens that you might want it more if you are using hg,
  but not so much if you are using git.
  
  Also, if similar repositories with Mercurial+watchman are actually
  faster than Git+watchman, that means that there's room for improvement
  in your implementation. This is not a big issue at this point of the
  process, just something nice to know.
 
 Converting git repos to hg seems to be incredibly slow.  (I have not yet
 tried doing it with git-remote-hg).

I haven't profiled the git-hg conversion, only the other way around,
but I don't see why it would be slow in git-remote-hg.

The only restriction is that octopus merges are not supported, so you
would probably need to find another repository.

 But I did find a hg repository for
 linux: https://bitbucket.org/orzel/linux-kernel-stable
 
 On this repo, I get:
 hg without watchman: 620ms
 hg with watchman: 264ms
 (compared to 125ms/90ms for git).
 
 The number of syscalls is, perhaps also interesting:
 no watchman / with watchman
 hg  3   /  5421
 git 59180   /  599 
 
 (About 1/3 of hg's syscalls with watchman seem to be loading Python
 stuff)

Exactly what I thought :) 28% improvement for Git, 76% improvement for
Mercurial. Still, the improvement would be welcome by big companies I
bet.

Cheers.

-- 
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: Pull is Mostly Evil

2014-05-05 Thread Felipe Contreras
Richard Hansen wrote:
 On 2014-05-03 06:00, John Szakmeister wrote:
  FWIW, at my company, we took another approach.  We introduced a `git
  ffwd` command that fetches from all remotes, and fast-forwards all
  your local branches that are tracking a remote, and everyone on the
  team uses it all the time.  It should be said this team also likes to
  use Git bare-metal, because they like knowing how things work
  out-of-the-box.  But they all use the command because it's so
  convenient.
 
 I also wrote a script to fast-forward all local branches to their
 configured upstream refs.  I finally got around to uploading it
 somewhere public:
 
https://github.com/richardhansen/git-update-branch
 
 I use it in my 'git up' alias:
 
git config --global alias.up \
'!git remote update -p; git update-branch -a'
 
 If there's interest I can tweak the style to conform to
 Documentation/CodingGuidelines and stick it in contrib/ or something.

I think this would fit perfectly in the proposed `git update` command as
an option: `git update --all`.

-- 
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 1/9] Define a structure for object IDs.

2014-05-05 Thread Felipe Contreras
Andreas Schwab wrote:
 This thread is about objects of type struct object_id, and their
 address is always the same as the address of its first member.
 Nothing else is relevant.

Indeed. I suggest you ingore that guy, he will only derail the
discussion.

-- 
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 v2] pager: remove 'S' from $LESS by default

2014-05-05 Thread Jonathan Nieder
Hi,

Matthieu Moy wrote:

 By default, Git used to set $LESS to -FRSX if $LESS was not set by the
 user. The FRX flags actually make sense for Git (F and X because Git
 sometimes pipes short output to less, and R because Git pipes colored
 output). The S flag (chop long lines), on the other hand, is not related
 to Git and is a matter of user preference. Git should not decide for the
 user to change LESS's default.

Thanks!  Sounds like a very good change.

(Nit: instead of because Git sometimes pipes short output to less,
it would be clearer to say something like when Git pipes short output
to less it is nice to exit and let the user type their next command.)

[...]
 The documentation in config.txt is made a bit longer to keep both an
 example setting the 'S' flag (needed to recover the old behavior) and an
 example showing how to unset a flag set by Git.

Interesting.  Looks good.

For what it's worth,
Reviewed-by: Jonathan Nieder jrnie...@gmail.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: What's cooking in git.git (Apr 2014, #09; Tue, 29)

2014-05-05 Thread John Keeping
On Tue, Apr 29, 2014 at 03:38:07PM -0700, Junio C Hamano wrote:
 * fc/remote-helpers-hg-bzr-graduation (2014-04-29) 11 commits
  - remote-hg: trivial cleanups
  - remote-hg: make sure we omit multiple heads
  - git-remote-hg: use internal clone's hgrc
  - t: remote-hg: split into setup test
  - remote-hg: properly detect missing contexts
  - remote-{hg,bzr}: store marks only on success
  - remote-hg: update to 'public' phase when pushing
  - remote-hg: fix parsing of custom committer
   (merged to 'next' on 2014-04-22 at fed170a)
  + remote-helpers: move tests out of contrib
  + remote-helpers: move out of contrib
  + remote-helpers: squelch python import exceptions
 
  Move remote-hg and remote-bzr out of contrib/.  There were some
  suggestions on the follow-up fix patches still not in 'next', which
  may result in a reroll.
 
  Will merge to 'next' and keep it there for the remainder of the
  cycle.

I'd like to register my opposition to moving git-remote-{bzr,hg} out of
contrib/.

I am not convinced that tools for interoperating with other VCSs need to
be part of core Git; as Junio has pointed out previously, while contrib/
was necessary for promoting associated tools when Git was young and had
not established mindshare, Git is now by far the most popular DVCS and
is rapidly catching up with centralized systems.  Associated tools can
therefore live on their own and do not need to be promoted as part of
Git itself (as git-imerge is doing successfully).

In the case of git-remote-hg specifically, the remote helper has to use
an interface that the Mercurial developers consider unstable [1]; the
version currently on 'pu' fails the test suite for me because I have
Mercurial 3.0:

AttributeError: 'mqrepo' object has no attribute 'getbundle'

I do not want to end up in a situation where an update to Git is blocked
by a distribution because git-remote-hg is not updated to support newer
versions of Mercurial sufficiently quickly; this previously happened in
Gentoo due to git-svn and meant that was stuck on 1.7.8 until 1.7.13 was
released [2].

Since the remote helper interface is stable and the remote helpers do
not use any of the Git internals, I consider the risks of including them
in core Git to outweigh the benefits of wider distribution.  In fact,
the remote helpers may benefit from having their own release cadences
so that they can respond to changes in related projects more quickly
than the normal Git release cycle.


[1] 
http://mercurial.selenic.com/wiki/MercurialApi#Why_you_shouldn.27t_use_Mercurial.27s_internal_API
[2] https://bugs.gentoo.org/show_bug.cgi?id=418431
--
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-05 Thread Felipe Contreras
John Keeping wrote:
 I am not convinced that tools for interoperating with other VCSs need to
 be part of core Git; as Junio has pointed out previously, while contrib/
 was necessary for promoting associated tools when Git was young and had
 not established mindshare, Git is now by far the most popular DVCS and
 is rapidly catching up with centralized systems.  Associated tools can
 therefore live on their own and do not need to be promoted as part of
 Git itself (as git-imerge is doing successfully).

Then let's remove git-p4.

 In the case of git-remote-hg specifically, the remote helper has to use
 an interface that the Mercurial developers consider unstable [1];

There is no other sensible way of doing them.

 the version currently on 'pu' fails the test suite for me because I
 have Mercurial 3.0:
 
   AttributeError: 'mqrepo' object has no attribute 'getbundle'

And because this patch has not been picked up[1].

 I do not want to end up in a situation where an update to Git is blocked
 by a distribution because git-remote-hg is not updated to support newer
 versions of Mercurial sufficiently quickly; this previously happened in
 Gentoo due to git-svn and meant that was stuck on 1.7.8 until 1.7.13 was
 released [2].

Travis-CI ensures that won't happen[2].

 Since the remote helper interface is stable and the remote helpers do
 not use any of the Git internals, I consider the risks of including them
 in core Git to outweigh the benefits of wider distribution.  In fact,
 the remote helpers may benefit from having their own release cadences
 so that they can respond to changes in related projects more quickly
 than the normal Git release cycle.

Maybe, but git-remote-hg has already benefitted a lot from the wider
exposure of being in 'contrib/', I'm sure it would benefit even more if
it's distributed by default.

Moreover, there's a ton of subpar tools out there[3], and I believe
giving the burden of choosing one to the user is detrimental to the Git
project. If we as a project say: this is the one we recommend, and has
the Git stamp, that helps the users tremendously.

Your point is valid though, but it's valid not just for
git-remote-hg/bzr.

So I think these are the two options:

  1) Include git-remote-hg/bzr to the core and distribute them by
 default (as is the current intention)

  2) Remove git-remote-hg/bzr entirely from the Git tree. And do the
 same for other tools: git-p4, git-svn, git-cvs*. Given the huge
 amount of people using Subversion, we might want to defer that one
 for later, but eventually do it.

I'd say for v2.0 we should go for 1), and 2) should be considered for
v3.0, perhaps.

[1] http://article.gmane.org/gmane.comp.version-control.git/248065
[2] https://travis-ci.org/felipec/git
[3] https://github.com/felipec/git/wiki/Comparison-of-git-remote-hg-alternatives

-- 
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-05 Thread John Keeping
On Mon, May 05, 2014 at 02:08:28PM -0500, Felipe Contreras wrote:
 John Keeping wrote:
  I am not convinced that tools for interoperating with other VCSs need to
  be part of core Git; as Junio has pointed out previously, while contrib/
  was necessary for promoting associated tools when Git was young and had
  not established mindshare, Git is now by far the most popular DVCS and
  is rapidly catching up with centralized systems.  Associated tools can
  therefore live on their own and do not need to be promoted as part of
  Git itself (as git-imerge is doing successfully).
 
 Then let's remove git-p4.

If git-p4 were not already in the core, I would be making precisely the
same argument regarding it (and the others you identify below).

  In the case of git-remote-hg specifically, the remote helper has to use
  an interface that the Mercurial developers consider unstable [1];
 
 There is no other sensible way of doing them.
 
  the version currently on 'pu' fails the test suite for me because I
  have Mercurial 3.0:
  
  AttributeError: 'mqrepo' object has no attribute 'getbundle'
 
 And because this patch has not been picked up[1].

And it is now probably too late for that to make Git 2.0, which means it
may be another 12 weeks before it makes it into a Git release.  Since
this is quite a minor change it may make it into a stable release, but
what would happen if the required changes were much more involved?

  I do not want to end up in a situation where an update to Git is blocked
  by a distribution because git-remote-hg is not updated to support newer
  versions of Mercurial sufficiently quickly; this previously happened in
  Gentoo due to git-svn and meant that was stuck on 1.7.8 until 1.7.13 was
  released [2].
 
 Travis-CI ensures that won't happen[2].

I don't see that building against Mercurial's default branch, so it will
not help with future releases.

  Since the remote helper interface is stable and the remote helpers do
  not use any of the Git internals, I consider the risks of including them
  in core Git to outweigh the benefits of wider distribution.  In fact,
  the remote helpers may benefit from having their own release cadences
  so that they can respond to changes in related projects more quickly
  than the normal Git release cycle.
 
 Maybe, but git-remote-hg has already benefitted a lot from the wider
 exposure of being in 'contrib/', I'm sure it would benefit even more if
 it's distributed by default.

Is that because it was included in contrib/ or just as a result of being
publicised on this list and elsewhere?  I don't think git-imerge is
suffering from being its own project and git-subtree appears to have
received very little attention despite being in contrib/.

 Moreover, there's a ton of subpar tools out there[3], and I believe
 giving the burden of choosing one to the user is detrimental to the Git
 project. If we as a project say: this is the one we recommend, and has
 the Git stamp, that helps the users tremendously.

But by choosing one now, we are stuck promoting that one even if a
better alternative comes along in the future.  We have seen that with
git-cvsimport and it's not dissimilar to the situation with git-pull.

 Your point is valid though, but it's valid not just for
 git-remote-hg/bzr.
 
 So I think these are the two options:
 
   1) Include git-remote-hg/bzr to the core and distribute them by
  default (as is the current intention)
 
   2) Remove git-remote-hg/bzr entirely from the Git tree. And do the
  same for other tools: git-p4, git-svn, git-cvs*. Given the huge
  amount of people using Subversion, we might want to defer that one
  for later, but eventually do it.

Don't forget git-archimport...

My personal vote would be for 2), splitting the bridges to other VCSs
into their own repositories but there would need to be some guarantee
that they would continue to be maintained.  I'm not sure it needs to
wait for a major Git release since most of the impact is on package
maintainers and not end users.

 I'd say for v2.0 we should go for 1), and 2) should be considered for
 v3.0, perhaps.

I don't think there is any advantage to adding new tools now if we only
intend to remove them in the future.

 [1] http://article.gmane.org/gmane.comp.version-control.git/248065
 [2] https://travis-ci.org/felipec/git
 [3] 
 https://github.com/felipec/git/wiki/Comparison-of-git-remote-hg-alternatives
--
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-05 Thread Matthieu Moy
- Original Message -
 Hi,
 
 Matthieu Moy wrote:
 
  By default, Git used to set $LESS to -FRSX if $LESS was not set by the
  user. The FRX flags actually make sense for Git (F and X because Git
  sometimes pipes short output to less, and R because Git pipes colored
  output). The S flag (chop long lines), on the other hand, is not related
  to Git and is a matter of user preference. Git should not decide for the
  user to change LESS's default.
 
 Thanks!  Sounds like a very good change.
 
 (Nit: instead of because Git sometimes pipes short output to less,
 it would be clearer to say something like when Git pipes short output
 to less it is nice to exit and let the user type their next command.)

It's actually a bit more than this: X to avoid initializing the terminal
and F for the exit behavior you describe.

But since the change is actually not about F and X, I prefered keeping
the text about them as short as possible, so I prefer my version actually.

-- 
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] Bump core.deltaBaseCacheLimit to 96m

2014-05-05 Thread Jeff King
On Mon, May 05, 2014 at 01:20:09PM +0200, David Kastrup wrote:

  Would it make more sense to advise git devs to set this per repo
  instead? The majority of (open source) repositories out there are
  small if I'm not mistaken. Of those few big repos, we could have a
  section listing all the tips and tricks to tune git. This is one of
  them. Index v4 and sparse checkout are some other. In future, maybe
  watchman support, split index and untracked cache as well.
 
 Shrug.  The last version of the patch was refused because of wanting
 more evidence.  I added the evidence.

FWIW, I was the one who asked for the evidence, and this patch looks
pretty straightforward and good. We may also want to revisit the data
structure for the delta cache, but that can come separately. My earlier
tests had not shown improvement with just bumping the cache size, but
these ones obviously do. So I think it's worth bumping the default.

-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


[PATCH] remote-helpers: add documentation for git-remote-hg/bzr

2014-05-05 Thread Felipe Contreras
Mostly copied from my personal github wiki.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 Documentation/git-remote-bzr.txt |  74 
 Documentation/git-remote-hg.txt  | 121 +++
 2 files changed, 195 insertions(+)
 create mode 100644 Documentation/git-remote-bzr.txt
 create mode 100644 Documentation/git-remote-hg.txt

diff --git a/Documentation/git-remote-bzr.txt b/Documentation/git-remote-bzr.txt
new file mode 100644
index 000..b211e7f
--- /dev/null
+++ b/Documentation/git-remote-bzr.txt
@@ -0,0 +1,74 @@
+git-remote-bzr(1)
+
+
+NAME
+
+git-remote-bzr - bidirectional bridge between Git and Bazaar
+
+
+SYNOPSIS
+
+[verse]
+'git clone' bzr::bzr repository
+
+
+DESCRIPTION
+---
+
+This tool allows you to transparently clone, fetch and push to and from Bazaar
+repositories as if they were Git ones.
+
+To use it you simply need to use the 'bzr::' prefix when specifying a remote 
URL
+(e.g. when cloning).
+
+
+EXAMPLE
+---
+--
+git clone bzr::lp:ubuntu/hello
+--
+
+
+NOTES
+-
+
+Remember to run `git gc --aggressive` after cloning a repository, specially if
+it's a big one. Otherwise lots of space will be wasted.
+
+The oldest version of Bazaar supported is 2.0. Older versions are not tested.
+
+Branches vs. repositories
+~
+
+Bazaar's UI can clone only branches, but a repository can contain multiple
+branches, and 'git-remote-bzr' can clone both.
+
+For example, to clone a branch:
+
+-
+git clone bzr::bzr://bzr.savannah.gnu.org/emacs/trunk emacs-trunk
+-
+
+And to clone the whole repository:
+
+-
+git clone bzr::bzr://bzr.savannah.gnu.org/emacs emacs
+-
+
+The second command would clone all the branches contained in the emacs
+repository, however, it's possible to specify only certain branches:
+
+-
+git config remote-bzr.branches 'trunk, xwindow'
+-
+
+Some remotes don't support listing the branches contained in the repository, in
+which cases you need to manually specify the branches, and while you can
+specify the configuration in the clone command, you might find this easier:
+
+-
+git init emacs
+git remote add origin bzr::bzr://bzr.savannah.gnu.org/emacs
+git config remote-bzr.branches 'trunk, xwindow'
+git fetch
+-
diff --git a/Documentation/git-remote-hg.txt b/Documentation/git-remote-hg.txt
new file mode 100644
index 000..0cceb76
--- /dev/null
+++ b/Documentation/git-remote-hg.txt
@@ -0,0 +1,121 @@
+git-remote-hg(1)
+
+
+NAME
+
+git-remote-hg - bidirectional bridge between Git and Mercurial
+
+
+SYNOPSIS
+
+[verse]
+'git clone' hg::hg repository
+
+
+DESCRIPTION
+---
+
+This tool allows you to transparently clone, fetch and push to and from 
Mercurial
+repositories as if they were Git ones.
+
+To use it you simply need to use the 'hg::' prefix when specifying a remote 
URL
+(e.g. when cloning).
+
+
+EXAMPLE
+---
+
+$ git clone hg::http://selenic.com/repo/hello
+
+
+
+CONFIGURATION
+-
+
+If you want to see Mercurial revisions as Git commit notes:
+
+--
+% git config core.notesRef refs/notes/hg
+--
+
+If you are not interested in Mercurial permanent and global branches (aka. 
commit labels):
+
+--
+% git config --global remote-hg.track-branches false
+--
+
+With this configuration, the 'branches/foo' refs won't appear.
+
+If you want the equivalent of `hg clone --insecure`:
+
+--
+% git config --global remote-hg.insecure true
+--
+
+If you want 'git-remote-hg' to be compatible with 'hg-git', and generate 
exactly the same commits:
+
+--
+% git config --global remote-hg.hg-git-compat true
+--
+
+NOTES
+-
+
+Remember to run `git gc --aggressive` after cloning a repository, specially if
+it's a big one. Otherwise lots of space will be wasted.
+
+The oldest version of Mercurial supported is 1.9. For the most part 1.8 works,
+but you might experience some issues.
+
+Pushing branches
+
+
+To push a Mercurial named branch, you need to use the branches/ prefix:
+
+--
+% git checkout branches/next
+# do stuff
+% git push origin branches/next
+--
+
+All the pushed commits will receive the next Mercurial named branch.
+
+*Note*: Make sure you don't have 

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

2014-05-05 Thread Felipe Contreras
John Keeping wrote:
 On Mon, May 05, 2014 at 02:08:28PM -0500, Felipe Contreras wrote:
  John Keeping wrote:
   I am not convinced that tools for interoperating with other VCSs need to
   be part of core Git; as Junio has pointed out previously, while contrib/
   was necessary for promoting associated tools when Git was young and had
   not established mindshare, Git is now by far the most popular DVCS and
   is rapidly catching up with centralized systems.  Associated tools can
   therefore live on their own and do not need to be promoted as part of
   Git itself (as git-imerge is doing successfully).
  
  Then let's remove git-p4.
 
 If git-p4 were not already in the core, I would be making precisely the
 same argument regarding it (and the others you identify below).

So basically you are arguing against any change.

   the version currently on 'pu' fails the test suite for me because I
   have Mercurial 3.0:
   
 AttributeError: 'mqrepo' object has no attribute 'getbundle'
  
  And because this patch has not been picked up[1].
 
 And it is now probably too late for that to make Git 2.0,

That's not for you to decide.

 which means it may be another 12 weeks before it makes it into a Git
 release.  Since this is quite a minor change it may make it into a
 stable release, but what would happen if the required changes were
 much more involved?

All the Mercurial API compatibility issues I have seen are trivial.

   I do not want to end up in a situation where an update to Git is blocked
   by a distribution because git-remote-hg is not updated to support newer
   versions of Mercurial sufficiently quickly; this previously happened in
   Gentoo due to git-svn and meant that was stuck on 1.7.8 until 1.7.13 was
   released [2].
  
  Travis-CI ensures that won't happen[2].
 
 I don't see that building against Mercurial's default branch, so it will
 not help with future releases.

I can easily add that.

   Since the remote helper interface is stable and the remote helpers do
   not use any of the Git internals, I consider the risks of including them
   in core Git to outweigh the benefits of wider distribution.  In fact,
   the remote helpers may benefit from having their own release cadences
   so that they can respond to changes in related projects more quickly
   than the normal Git release cycle.
  
  Maybe, but git-remote-hg has already benefitted a lot from the wider
  exposure of being in 'contrib/', I'm sure it would benefit even more if
  it's distributed by default.
 
 Is that because it was included in contrib/ or just as a result of being
 publicised on this list and elsewhere?

The former I'd bet.

 I don't think git-imerge is suffering from being its own project and
 git-subtree appears to have received very little attention despite
 being in contrib/.

Apples and oranges. There aren't scores of tools out there trying to do
what git-subtree does.

  Moreover, there's a ton of subpar tools out there[3], and I believe
  giving the burden of choosing one to the user is detrimental to the Git
  project. If we as a project say: this is the one we recommend, and has
  the Git stamp, that helps the users tremendously.
 
 But by choosing one now, we are stuck promoting that one even if a
 better alternative comes along in the future.

Are there better alternatives coming in the future?

  Your point is valid though, but it's valid not just for
  git-remote-hg/bzr.
  
  So I think these are the two options:
  
1) Include git-remote-hg/bzr to the core and distribute them by
   default (as is the current intention)
  
2) Remove git-remote-hg/bzr entirely from the Git tree. And do the
   same for other tools: git-p4, git-svn, git-cvs*. Given the huge
   amount of people using Subversion, we might want to defer that one
   for later, but eventually do it.
 
 Don't forget git-archimport...
 
 My personal vote would be for 2), splitting the bridges to other VCSs
 into their own repositories but there would need to be some guarantee
 that they would continue to be maintained.  I'm not sure it needs to
 wait for a major Git release since most of the impact is on package
 maintainers and not end users.

Sure, we might not need to wait for v3.0, but that's not the point. The
point is that we should be consistent, and that means going for 1) in
v2.0.

  I'd say for v2.0 we should go for 1), and 2) should be considered for
  v3.0, perhaps.
 
 I don't think there is any advantage to adding new tools now if we only
 intend to remove them in the future.

Do we intend to remove them in the future? That hasn't been decided.

-- 
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 1/3] Revert make error()'s constant return value more visible

2014-05-05 Thread Jeff King
On Mon, May 05, 2014 at 02:30:12AM -0500, Felipe Contreras wrote:

 If we have a) code that fixes a couple warnings with -O3 but introduces
 hundreds with -O2, vs. b) code that has only a comple warnings with -O3,
 I'd go for b) any day.

I agree. But my point was to ask whether we can fix both.

 Yes, I see the problem now with -O3. And yes, I looked at the patch you
 sent. I haven't tested it but I bet it would sove both problems.

Unfortunately, it does not work in all cases (and I obviously did
something wrong when testing it last night). I should have taken my own
advice and re-read the commit message for e208f9c more carefully, which
says:

If we can make the compiler aware that error() will always
return -1, it can do a better job of analysis. The simplest
method would be to inline the error() function. However,
this doesn't work, because gcc will not inline a variadc
function. We can work around this by defining a macro.[...]

I cannot think of any other way to make the compiler aware of the
constant value, but perhaps somebody else is more clever than I am.
Another alternative is to write out return error(...) as error(...);
return -1. In some ways that is more readable, though it is more
verbose (and would cause quite a bit of code churn).

So applying your patches may be the least-bad solution.

-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 v2 5/5] contrib/subtree/Makefile: clean rule cleanup

2014-05-05 Thread James Denholm
On 5 May 2014 15:09:39 GMT+10:00, Jeff King p...@peff.net wrote:
On Sat, May 03, 2014 at 10:49:35PM +1000, James Denholm wrote:

 diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile
 index f3834b5..4f96a24 100644
 --- a/contrib/subtree/Makefile
 +++ b/contrib/subtree/Makefile
 @@ -11,8 +11,9 @@ man1dir ?= $(mandir)/man1
  
  -include ../../GIT-VERSION-FILE
  
 -# this should be set to a 'standard' bsd-type install program
 -INSTALL ?= install
 +# These should be set to 'standard' bsd-type programs
 +INSTALL  ?= install
 +RM   ?= rm -f

I do not think BSD-ism matters for rm, as it works pretty much the
same everywhere. install, on the other hand, is a bit weirder between
systems. So you might want to leave that comment as-is.

True. I might just buff that out when sending the patch to Junio, unless
protocol dictates otherwise - a reroll for a single comment line seems
a bit excessive to me 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: [PATCH] t3910: show failure of core.precomposeunicode with decomposed filenames

2014-05-05 Thread Jeff King
On Sun, May 04, 2014 at 08:13:15AM +0200, Torsten Bögershausen wrote:

1. Tell everyone that NFD in the git repo is wrong, and
   they should make a new commit to normalize all their
   in-repo files to be precomposed.
   This is probably not the right thing to do, because it
   still doesn't fix checkouts of old history. And it
   spreads the problem to people on byte-preserving
   filesystems (like ext4), because now they have to start
   precomposing their filenames as they are adde to git.
  (typo:  
 ^added)
 I'm not sure if I follow. People running ext4 (or Linux in general,
 or Windows, or Unix) do not suffer from file system
 feature of Mac OS, which accepts precomposed/decomposed Unicode
 but returns decompomsed.

What I mean by spreads the problem is that git on Linux does not need
to care about utf8 at all. It treats filenames as a byte sequence. But
if we were to start enforcing filenames should be precomposed utf8,
then people adding files on Linux would want to enforce that, too.

People on Linux could ignore the issue as they do now, but they would
then create problems for OS X users if they add decomposed filenames.
IOW, if the OS X code assumes all repo filenames are precomposed, then
other systems become a possible vector for violating that assumption.

3. Convert index filenames to their precomposed form when
   we read the index from disk. This would be efficient,
   but we would have to be careful not to write the
   precomposed forms back out to disk.
 Question:
 How could we be careful?
 Mac OS writes always decomposed Unicode to disk.
 (And all other OS tend to use precomposed forms, mainly because the keyboard
 driver generates it.)

Sorry, I should have been more clear here. I meant do not write index
entries using the precomposed forms out to the on-disk index. Because
that would mean that git silently converts your filenames, and it would
look like you have changes to commit whenever you read in a tree with a
decomposed name.

Looking over the patch you sent earlier, I suspect that is part of its
problem (it stores the converted name in the index entry's name field).

 This is my understanding:
 Some possible fixes are:
 
   1. Accept that NFD in a Git repo which is shared between Mac OS
  and Linux or Windows is problematic.
  Whenever core.precomposeunicode = true, do the following:
  Let Git under Mac OS change all file names in the index
  into the precomposed form when a new commit is done.
  This is probably not a wrong thing to do.
 
  When the index file is read into memory, precompose the file names and 
 compare
  them with the precomposed form coming from precompose_utf8_readdir().
  This avoids decomposed file names to be reported as untracked by git 
 status.

This is the case I was specifically thinking of above (and I think what
your patch is doing).

   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.

-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 v2 5/5] contrib/subtree/Makefile: clean rule cleanup

2014-05-05 Thread Jeff King
On Tue, May 06, 2014 at 07:41:29AM +1000, James Denholm wrote:

 I do not think BSD-ism matters for rm, as it works pretty much the
 same everywhere. install, on the other hand, is a bit weirder between
 systems. So you might want to leave that comment as-is.
 
 True. I might just buff that out when sending the patch to Junio, unless
 protocol dictates otherwise - a reroll for a single comment line seems
 a bit excessive to me at the moment.

I don't think it is that big a deal either way.

It's fine to tweak when you send re-roll the final for Junio. Sometimes
for trivial fixups like this, Junio can just tweak it as he applies, but
I do not know if he is even paying attention to this thread, so you may
want to re-post anyway to get his attention.

Either way, feel free to add my:

  Reviewed-by: Jeff King p...@peff.net

-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: What's cooking in git.git (Apr 2014, #09; Tue, 29)

2014-05-05 Thread Felipe Contreras
Felipe Contreras wrote:
 John Keeping wrote:
  I don't see that building against Mercurial's default branch, so it
  will not help with future releases.
 
 I can easily add that.

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

-- 
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 v2 0/5] contrib/subtree/Makefile: Standardisation pass

2014-05-05 Thread James Denholm
On 5 May 2014 15:08:04 GMT+10:00, Jeff King p...@peff.net wrote:
On Sat, May 03, 2014 at 10:49:30PM +1000, James Denholm wrote:

 The main issues are that calls are made to git itself in the build
 process, and that a subtree-exclusive variable is used for specifying
 the exec path. Patches 1/5 through 3/5 resolve these.
 
 The cleanup fixes (4/5 and 5/5) are based on precedents set by
other
 makefiles across the project.

Thanks, these all look sane to me (I do not use subtree, but since it's
just about Makefiles, it was pretty easy to review).

Thanks for the review!

Given that subtree subtree doesn't really generate a lot of discussion,
would it be advisable to wrap this up (barring further discussion) and send
it off to Junio rather than waiting for further community consensus?

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] config: preserve config file permissions on edits

2014-05-05 Thread Eric Wong
Users may already store sensitive data such as imap.pass in
.git/config; making the file world-readable when git config
is called to edit means their password would be compromised
on a shared system.

Signed-off-by: Eric Wong normalper...@yhbt.net
---
 config.c   | 7 +++
 t/t1300-repo-config.sh | 6 ++
 2 files changed, 13 insertions(+)

diff --git a/config.c b/config.c
index a30cb5c..a0b6756 100644
--- a/config.c
+++ b/config.c
@@ -1636,6 +1636,13 @@ int git_config_set_multivar_in_file(const char 
*config_filename,
MAP_PRIVATE, in_fd, 0);
close(in_fd);
 
+   if (fchmod(fd, st.st_mode  0)  0) {
+   error(fchmod on %s failed: %s,
+   lock-filename, strerror(errno));
+   ret = CONFIG_NO_WRITE;
+   goto out_free;
+   }
+
if (store.seen == 0)
store.seen = 1;
 
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 58cd543..d87693e 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1158,4 +1158,10 @@ test_expect_failure 'adding a key into an empty section 
reuses header' '
test_cmp expect .git/config
 '
 
+test_expect_success POSIXPERM,PERL 'preserves existing permissions' '
+   chmod 0600 .git/config 
+   git config imap.pass Hunter2 
+   perl -e die q(badperm) if ((stat(q(.git/config)))[2]  0) != 0600
+'
+
 test_done
-- 
Eric Wong
--
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 5/5] contrib/subtree/Makefile: clean rule cleanup

2014-05-05 Thread James Denholm
On 6 May 2014 07:49:30 GMT+10:00, Jeff King p...@peff.net wrote:
On Tue, May 06, 2014 at 07:41:29AM +1000, James Denholm wrote:

 I do not think BSD-ism matters for rm, as it works pretty much the
 same everywhere. install, on the other hand, is a bit weirder
between
 systems. So you might want to leave that comment as-is.
 
 True. I might just buff that out when sending the patch to Junio,
unless
 protocol dictates otherwise - a reroll for a single comment line
seems
 a bit excessive to me at the moment.

I don't think it is that big a deal either way.

It's fine to tweak when you send re-roll the final for Junio. Sometimes
for trivial fixups like this, Junio can just tweak it as he applies,
but
I do not know if he is even paying attention to this thread, so you may
want to re-post anyway to get his attention.

Sure, sounds good and will do.

Either way, feel free to add my:

  Reviewed-by: Jeff King p...@peff.net

Awesome, thanks again.

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: [PATCH v2 0/5] contrib/subtree/Makefile: Standardisation pass

2014-05-05 Thread Jeff King
[fixed David's address in cc list]

On Tue, May 06, 2014 at 07:54:30AM +1000, James Denholm wrote:

 Given that subtree subtree doesn't really generate a lot of discussion,
 would it be advisable to wrap this up (barring further discussion) and send
 it off to Junio rather than waiting for further community consensus?

I do not know if lack of discussion is a good reason to consider
something in good shape; oftentimes it is a sign that nobody is
interested in the area. We usually rely on area maintainers to give an
OK to the patches if they are not something that the maintainer himself
has an interest in.

However, in this case, you did get review, and I think it is pretty easy
to see the patches are good even if one does not care about the
particular area. So I think they are fine to pass on and apply.

Note also that patches like this are a great place to get started, as
they help build trust in a contributor, who can later help out with
area maintenance.

-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


[PATCH] add a reflog_exists and delete_reflog abstraction

2014-05-05 Thread Ronnie Sahlberg
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.


Ronnie Sahlberg (1):
  refs.c: add new functions reflog_exists and delete_reflog

 builtin/checkout.c |  8 ++--
 builtin/reflog.c   |  2 +-
 refs.c | 20 ++--
 refs.h |  6 ++
 4 files changed, 23 insertions(+), 13 deletions(-)

-- 
2.0.0.rc1.351.gd2b7e18.dirty

--
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] refs.c: add new functions reflog_exists and delete_reflog

2014-05-05 Thread Ronnie Sahlberg
Add two new functions, reflog_exists and delete_reflog to hide the internal
reflog implementation (that they are files under .git/logs/...) from callers.
Update checkout.c to use these functions in update_refs_for_switch instead of
building pathnames and calling out to file access functions. Update reflog.c
to use these too check if the reflog exists. Now there are still many places
in reflog.c where we are still leaking the reflog storage implementation but
this at least reduces the number of such dependencies by one. Finally
change two places in refs.c itself to use the new function to check if a ref
exists or not isntead of build-path-and-stat(). Now, this is strictly not all
that important since these are in parts of refs that are implementing the
actual file storage backend but on the other hand it will not hurt either.

In config.c we also change to use the existing function ref_exists instead of
checking if the loose ref file exist. The previous code made the assumption
that the branch we switched from must exist as a loose ref and thus checking
the file would be sufficent. I think that assumption is always true in the
current code but it is still somewhat fragile since if git would change so that
the checkedout branch could exist as a packed ref without a corresponding
loose ref then this subtle 'does the lose ref not exist' check would suddenly
fail.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 builtin/checkout.c |  8 ++--
 builtin/reflog.c   |  2 +-
 refs.c | 20 ++--
 refs.h |  6 ++
 4 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index ff44921..f1dc56e 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -651,12 +651,8 @@ static void update_refs_for_switch(const struct 
checkout_opts *opts,
}
}
if (old-path  old-name) {
-   char log_file[PATH_MAX], ref_file[PATH_MAX];
-
-   git_snpath(log_file, sizeof(log_file), logs/%s, 
old-path);
-   git_snpath(ref_file, sizeof(ref_file), %s, old-path);
-   if (!file_exists(ref_file)  file_exists(log_file))
-   remove_path(log_file);
+   if (!ref_exists(old-path)  reflog_exists(old-path))
+   delete_reflog(old-path);
}
}
remove_branch_state();
diff --git a/builtin/reflog.c b/builtin/reflog.c
index c12a9784..0e7ea74 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -369,7 +369,7 @@ static int expire_reflog(const char *ref, const unsigned 
char *sha1, int unused,
if (!lock)
return error(cannot lock ref '%s', ref);
log_file = git_pathdup(logs/%s, ref);
-   if (!file_exists(log_file))
+   if (!ref_exists(ref))
goto finish;
if (!cmd-dry_run) {
newlog_path = git_pathdup(logs/%s.lock, ref);
diff --git a/refs.c b/refs.c
index e59bc18..7d12ac7 100644
--- a/refs.c
+++ b/refs.c
@@ -2013,7 +2013,6 @@ int dwim_log(const char *str, int len, unsigned char 
*sha1, char **log)
 
*log = NULL;
for (p = ref_rev_parse_rules; *p; p++) {
-   struct stat st;
unsigned char hash[20];
char path[PATH_MAX];
const char *ref, *it;
@@ -2022,12 +2021,9 @@ int dwim_log(const char *str, int len, unsigned char 
*sha1, char **log)
ref = resolve_ref_unsafe(path, hash, 1, NULL);
if (!ref)
continue;
-   if (!stat(git_path(logs/%s, path), st) 
-   S_ISREG(st.st_mode))
+   if (reflog_exists(path))
it = path;
-   else if (strcmp(ref, path) 
-!stat(git_path(logs/%s, ref), st) 
-S_ISREG(st.st_mode))
+   else if (strcmp(ref, path)  reflog_exists(ref))
it = ref;
else
continue;
@@ -3030,6 +3026,18 @@ int read_ref_at(const char *refname, unsigned long 
at_time, int cnt,
return 1;
 }
 
+int reflog_exists(const char *ref)
+{
+   struct stat st;
+
+   return !lstat(git_path(logs/%s, ref), st)  S_ISREG(st.st_mode);
+}
+
+int delete_reflog(const char *ref)
+{
+   return remove_path(git_path(logs/%s, ref));
+}
+
 static int show_one_reflog_ent(struct strbuf *sb, each_reflog_ent_fn fn, void 
*cb_data)
 {
unsigned char osha1[20], nsha1[20];
diff --git a/refs.h b/refs.h
index 71e39b9..5a93f27 100644
--- a/refs.h
+++ b/refs.h
@@ -159,6 +159,12 @@ extern int read_ref_at(const char *refname, unsigned long 
at_time, int cnt,
   unsigned char *sha1, char **msg,
   unsigned long *cutoff_time, int *cutoff_tz, int 
*cutoff_cnt);
 
+/** Check if a particular ref log exists */
+extern 

Re: [msysGit] Re: #178 parsing of pretty=format:%an %ad causes fatal: bad revision '%ad'

2014-05-05 Thread Junio C Hamano
Dave Bradley dbradl...@bell.net writes:

 Original #178 content
 G:\ws_test_env\GIT_TESTBED_TMP\fest-swing-1.xgit log --all
 --pretty=format:%an%ad -- pom.xml
 Mon Nov 23 03:09:17 2009 +
 Mon Nov 23 02:42:24 2009 +

 This added to my confusion as by right dq within dq should be
 formatted.

I do not see any dq within dq here.  Perhaps you need to count again
to see which one pairs with which one.

What I see are these three strings concatenated together:

--pretty=format: (which does not need any dq)
%an   %ad (the dq protects 3 SPs inside)
(an empty string inside the final dq pair)

So the single parameter that begins with --pretty given to git log
is exactly the same as what is inside the single dq pair in the
following:

--pretty=format:%an   %ad

A more conventional way to spell it may however be one of:

--pretty=format:%an   %ad
--pretty=format:%an   %ad

--
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 v6 08/42] refs.c: change ref_transaction_update() to do error checking and return status

2014-05-05 Thread Ronnie Sahlberg
Thanks!

On Mon, May 5, 2014 at 6:08 AM, Michael Haggerty mhag...@alum.mit.edu wrote:
 On 05/01/2014 10:37 PM, Ronnie Sahlberg wrote:
 Update ref_transaction_update() do some basic error checking and return
 true on error. Update all callers to check ref_transaction_update() for 
 error.
 There are currently no conditions in _update that will return error but there
 will be in the future.

 I would change s/true/non-zero/, because error return values are not
 just boolean values; the error values sometimes encode the type of error
 that occurred.

Done.



 Signed-off-by: Ronnie Sahlberg sahlb...@google.com
 ---
  builtin/update-ref.c | 10 ++
  refs.c   |  9 +++--
  refs.h   | 10 +-
  3 files changed, 18 insertions(+), 11 deletions(-)

 diff --git a/builtin/update-ref.c b/builtin/update-ref.c
 index 2bef2a0..59c4d6b 100644
 --- a/builtin/update-ref.c
 +++ b/builtin/update-ref.c
 @@ -197,8 +197,9 @@ static const char *parse_cmd_update(struct strbuf 
 *input, const char *next)
   if (*next != line_termination)
   die(update %s: extra input: %s, refname, next);

 - ref_transaction_update(transaction, refname, new_sha1, old_sha1,
 -update_flags, have_old);
 + if (ref_transaction_update(transaction, refname, new_sha1, old_sha1,
 +update_flags, have_old))
 + die(update %s: failed, refname);

   update_flags = 0;
   free(refname);
 @@ -286,8 +287,9 @@ static const char *parse_cmd_verify(struct strbuf 
 *input, const char *next)
   if (*next != line_termination)
   die(verify %s: extra input: %s, refname, next);

 - ref_transaction_update(transaction, refname, new_sha1, old_sha1,
 -update_flags, have_old);
 + if (ref_transaction_update(transaction, refname, new_sha1, old_sha1,
 +update_flags, have_old))
 + die(failed transaction update for %s, refname);

   update_flags = 0;
   free(refname);
 diff --git a/refs.c b/refs.c
 index 308e13e..1a903fb 100644
 --- a/refs.c
 +++ b/refs.c
 @@ -,19 +,24 @@ static struct ref_update *add_update(struct 
 ref_transaction *transaction,
   return update;
  }

 -void ref_transaction_update(struct ref_transaction *transaction,
 +int ref_transaction_update(struct ref_transaction *transaction,
   const char *refname,
   const unsigned char *new_sha1,
   const unsigned char *old_sha1,
   int flags, int have_old)
  {
 - struct ref_update *update = add_update(transaction, refname);
 + struct ref_update *update;
 +
 + if (have_old  !old_sha1)
 + die(have_old is true but old_sha1 is NULL);

 This new check is orthogonal to the rest of the patch, isn't it?

Yes.

I have updated the commit message to mention that we also check and
die(BUG:...) for this condition.




 + update = add_update(transaction, refname);
   hashcpy(update-new_sha1, new_sha1);
   update-flags = flags;
   update-have_old = have_old;
   if (have_old)
   hashcpy(update-old_sha1, old_sha1);
 + return 0;
  }

  void ref_transaction_create(struct ref_transaction *transaction,
 diff --git a/refs.h b/refs.h
 index bc7715e..0364a3e 100644
 --- a/refs.h
 +++ b/refs.h
 @@ -237,11 +237,11 @@ void ref_transaction_rollback(struct ref_transaction 
 *transaction);
   * that the reference should have had before the update, or zeros if
   * it must not have existed beforehand.
   */

 Please update the docstring above to explain the return value.

Done.


 -void ref_transaction_update(struct ref_transaction *transaction,
 - const char *refname,
 - const unsigned char *new_sha1,
 - const unsigned char *old_sha1,
 - int flags, int have_old);
 +int ref_transaction_update(struct ref_transaction *transaction,
 +const char *refname,
 +const unsigned char *new_sha1,
 +const unsigned char *old_sha1,
 +int flags, int have_old);

  /*
   * Add a reference creation to transaction.  new_sha1 is the value


 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: What's cooking in git.git (Apr 2014, #09; Tue, 29)

2014-05-05 Thread Junio C Hamano
John Keeping j...@keeping.me.uk writes:

 On Tue, Apr 29, 2014 at 03:38:07PM -0700, Junio C Hamano wrote:
 * fc/remote-helpers-hg-bzr-graduation (2014-04-29) 11 commits
  ...
  Move remote-hg and remote-bzr out of contrib/.  There were some
  suggestions on the follow-up fix patches still not in 'next', which
  may result in a reroll.
 
  Will merge to 'next' and keep it there for the remainder of the
  cycle.

 I'd like to register my opposition to moving git-remote-{bzr,hg} out of
 contrib/.
 ...
 In the case of git-remote-hg specifically, the remote helper has to use
 an interface that the Mercurial developers consider unstable [1];...
 I do not want to end up in a situation where an update to Git is blocked
 by a distribution because git-remote-hg is not updated to support newer
 versions of Mercurial sufficiently quickly; this previously happened in
 Gentoo due to git-svn and meant that was stuck on 1.7.8 until 1.7.13 was
 released [2].

The same argument would apply to git-svn, git-p4, and git-cvsimport,
I would think.

Among these, I am not sure if we can find willing maintainers who
can give enough love to them.  But unlike these other importers,
remote-hg and remote-bzr do have an active maintainer (and IIRC I
think I heard that Hg one even has an active competitor or two?) so
I am reasonably confident that these can live on their own merit
outside of my tree.  In the ideal world, I would think it may be
even beneficial to the end users of these helpers to unbundle them.

You raised a good point on the issue of external dependencies may
impact Git as a whole, even for those who are not interested in the
particular remote helpers at all.  I'll have to think about it.

The silly thing is that I totally forgot that we almost got
ourselves into a very similar situation on cvsimport when a series
wanted to make it cvsps3-only.  It is very possible nobody would
have picked up the entire new release, if we merged that change.

Having said all that, there is one caveat.

 Since the remote helper interface is stable and the remote helpers do
 not use any of the Git internals, I consider the risks of including them
 in core Git to outweigh the benefits of wider distribution.

You are correct to say that a remote helper has to talk with a
foreign system and it would not help to dictate the update schedule
of helpers to match the release cycle of Git itself.  At the same
time, however, the interface the remote helpers use to talk to Git
has not been as stable as you seem to think, I am afraid.  For
example, a recent remote-hg/bzr series needed some enhancements to
fast-import to achieve the feature parity with native transports by
adding a missing feature or two on the Git side.

So in reality, a helper has to talk with two sides, needs to adjust
to changes in the both sides, and both sides are changing.

Unbundling a helper from Git would place more burden on the helper's
maintainer, because the helper has to know enough about versions and
features of both sides (the foreign system and Git) to adjust its
behaviour, to stay compatible with wider versions of both foreign
systems and Git.  Unbundling, when done properly, may give more
ideal user experience to the end users, because such a helper would
allow them to pick up the latest (or stay on an older but known to
be stable) version of the helper and expect it to work with the
foreign system and Git they happen to have.

It however would be easier to maintain if the helper maintainer
knows a change to Git itself will be released at the same time as
the new version of the helper that takes advantage of the modified
Git.  The helper maintainer only has to worry about compatibility
with the foreign side if it is bundled with Git.

So it boils down to how much resource are there to make sure a
helper will stay compatible with wider versions of both sides? and
how far backwards are helper maintainers willing to bend to support
users better?.



--
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 v2] config: preserve config file permissions on edits

2014-05-05 Thread Eric Wong
Users may already store sensitive data such as imap.pass in
.git/config; making the file world-readable when git config
is called to edit means their password would be compromised
on a shared system.

[v2: updated for section renames, as noted by Junio]

Signed-off-by: Eric Wong normalper...@yhbt.net
---
 config.c   | 16 
 t/t1300-repo-config.sh | 10 ++
 2 files changed, 26 insertions(+)

diff --git a/config.c b/config.c
index a30cb5c..c227aa8 100644
--- a/config.c
+++ b/config.c
@@ -1636,6 +1636,13 @@ int git_config_set_multivar_in_file(const char 
*config_filename,
MAP_PRIVATE, in_fd, 0);
close(in_fd);
 
+   if (fchmod(fd, st.st_mode  0)  0) {
+   error(fchmod on %s failed: %s,
+   lock-filename, strerror(errno));
+   ret = CONFIG_NO_WRITE;
+   goto out_free;
+   }
+
if (store.seen == 0)
store.seen = 1;
 
@@ -1784,6 +1791,7 @@ int git_config_rename_section_in_file(const char 
*config_filename,
int out_fd;
char buf[1024];
FILE *config_file;
+   struct stat st;
 
if (new_name  !section_name_is_ok(new_name)) {
ret = error(invalid section name: %s, new_name);
@@ -1805,6 +1813,14 @@ int git_config_rename_section_in_file(const char 
*config_filename,
goto unlock_and_out;
}
 
+   fstat(fileno(config_file), st);
+
+   if (fchmod(out_fd, st.st_mode  0)  0) {
+   ret = error(fchmod on %s failed: %s,
+   lock-filename, strerror(errno));
+   goto out;
+   }
+
while (fgets(buf, sizeof(buf), config_file)) {
int i;
int length;
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 58cd543..3f80ff0 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1158,4 +1158,14 @@ test_expect_failure 'adding a key into an empty section 
reuses header' '
test_cmp expect .git/config
 '
 
+test_expect_success POSIXPERM,PERL 'preserves existing permissions' '
+   chmod 0600 .git/config 
+   git config imap.pass Hunter2 
+   perl -e \
+ die q(badset) if ((stat(q(.git/config)))[2]  0) != 0600 
+   git config --rename-section imap pop 
+   perl -e \
+ die q(badrename) if ((stat(q(.git/config)))[2]  0) != 0600
+'
+
 test_done
-- 
Eric Wong

--
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: Watchman support for git

2014-05-05 Thread Duy Nguyen
On Sat, May 3, 2014 at 7:52 AM, Duy Nguyen pclo...@gmail.com wrote:
 wt_status_collect_changes_index() depends on how damaged cache-tree is
 (in this case, totally scraped). watchman does not help this either.
 We need to try to heal cache-tree as much as possible to reduce the
 number.

On the topic of cache-tree, I notice that unpack_trees() will call
discard_index() in the end (or even discard_index() on a merge
failure). That destroys cache-tree. unpack_trees() is the core of
branch switching, or reset/merge, which I consider frequent
operations. Cache-tree destruction is bad for git diff --cached,
git commit and maybe more. This discard_index() code was added
recently in e28f764 (unpack-trees: plug a memory leak - 2013-08-13).
As a workaround, perhaps we only do so when the sequencer is running.
-- 
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: Watchman support for git

2014-05-05 Thread Duy Nguyen
On Tue, May 6, 2014 at 7:26 AM, Duy Nguyen pclo...@gmail.com wrote:
 This discard_index() code was added
 recently in e28f764 (unpack-trees: plug a memory leak - 2013-08-13).
 As a workaround, perhaps we only do so when the sequencer is running.

Hmm.. as o-result does not carry cache-tree anyway, the next
assignment after discard_index() will destroy cache-tree anyway. I
wrote and deleted the following sentence, but it looks like I should
rewrite again. So if cache-tree has always been destroyed before, be
careful when you try to keep cache-tree as cache-tree invalidation
code in unpack-trees.c may not be well tested.
-- 
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: What's cooking in git.git (Apr 2014, #09; Tue, 29)

2014-05-05 Thread Felipe Contreras
Junio C Hamano wrote:
 John Keeping j...@keeping.me.uk writes:

  In the case of git-remote-hg specifically, the remote helper has to use
  an interface that the Mercurial developers consider unstable [1];...
  I do not want to end up in a situation where an update to Git is blocked
  by a distribution because git-remote-hg is not updated to support newer
  versions of Mercurial sufficiently quickly; this previously happened in
  Gentoo due to git-svn and meant that was stuck on 1.7.8 until 1.7.13 was
  released [2].
 
 The same argument would apply to git-svn, git-p4, and git-cvsimport,
 I would think.
 
 Among these, I am not sure if we can find willing maintainers who
 can give enough love to them.  But unlike these other importers,
 remote-hg and remote-bzr do have an active maintainer (and IIRC I
 think I heard that Hg one even has an active competitor or two?)

Unfortunately there are no more real competitors to remote-hg. A far as
I can tell msysgit has dropped their remote helper, and gitifyhg is not
being actively maintatined and it's even pointing to our git-remote-hg
as probably the best alternative to use at the moment.

 so I am reasonably confident that these can live on their own merit
 outside of my tree.  In the ideal world, I would think it may be even
 beneficial to the end users of these helpers to unbundle them.

It might be benefitial in the future, but right now I'm willing to bet
there's many people that don't know git-remote-hg/bzr even exist. If Git
v2.0 distributes them by default, and they are mentioned in the release
notes:

 * Transparent support to pull and push to and from Mercurial and Bazaar
   repositories is now enabled by default.

Many more people will know about that, and in the future when we try to
unbundle them they can shout if for some reason it would be inconvenient
for them. At the moment I don't think we can say for sure.

Even if people don't use these bridges, I think just mentioning that
feature helps the project in general.

 You raised a good point on the issue of external dependencies may
 impact Git as a whole, even for those who are not interested in the
 particular remote helpers at all.  I'll have to think about it.

Yes, it's worth thinking about it because it's a real possibility.
However, real possibilities are many times not likely to happen, and I
think this is one of those cases.

As I've said, if history is any indication these issues won't happen. As
far as I can remember the only issues that have happened are backwards
compatibility issues, not present or future. And as I said I've setup
TravisCI builds to detect those, which is why we haven't had those
issues since then.

 So it boils down to how much resource are there to make sure a helper
 will stay compatible with wider versions of both sides? and how far
 backwards are helper maintainers willing to bend to support users
 better?.

This is not that big of an issue. For example, notice how the changes in
the transport-helper to enable say --force and --dry-run did not
require to align changes in remote-hg/bzr. That's because remote-hg/bzr
had already the code for these features, it was just not exercised until
the transport-helper was modified.

I think the current transport-helper infraestructure is already good
enough to detect the features and options of the remote helpers so
unbundling wouldn't be a major problem.

Having said that alignment issues do happen, and we have one of those in
Git v2.0, but I don't think they are a major concern (at least for
remote-hg/bzr).

-- 
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-05 Thread Felipe Contreras
Felipe Contreras wrote:
 Having said that alignment issues do happen, and we have one of those
 in Git v2.0, but I don't think they are a major concern (at least for
 remote-hg/bzr).

Actually I just noticed that the remote-helpers side is not in the
master branch.

I don't know what is your plan with fc/remote-helpers-hg-bzr-graduation,
but for v2.0 we really want the patch 'remote-{hg,bzr}: store marks only
on success'. Explaining precisely why would take a lot of effort, but
basically it's related to 3994e64 (transport-helper: fix sync issue on
crashes).

If you are worried about merging the whole branch, I could pick only the
important patches and reroll.

-- 
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: [BUG?] Patches created with 'diff.noprefix=true' don't 'git apply'.

2014-05-05 Thread Nathan Collins
On Wed, Apr 30, 2014 at 7:40 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 Hi,

 Nathan Collins wrote:

 Patches created with 'diff.noprefix=true' don't 'git apply' without
 specifying '-p0'.

 I'm not sure this is a bug -- the 'man git-apply' just says Reads the
 supplied diff output (i.e. a patch) and applies it to files -- but
 I would expect patches I create locally to apply cleanly locally.

 Sounds like a documentation bug, at least.  Any ideas for clearer
 wording?

Hmmm. Maybe a warning that the patch is expected to be in '-p1'
format, and that setting 'diff.noprefix=true' makes some commands
generate '-p0' patches? But I worry this would just confuse / distract
the people that don't have 'diff.noprefix=true' set, which I expect is
the majority of users.  Better I think would be for 'git apply' to be
smarter, as you suggest below.

   In
 real life the 'diff.noprefix=true' is in my ~/.gitconfig, so this was
 pretty confusing.

 I personally think setting diff.noprefix is not very sane (it also
 breaks patch -p1), and I suppose I should have been louder about
 that when it was introduced.

 Can you say more about the workflow you use that requires
 diff.noprefix?  Maybe we can make other changes to improve it, too.

I have 'diff.noprefix=true' set so I can copy and paste paths from the
'git diff' output easily.  I like to create small, logically
independent commits, usually comprising a subset of my current
changes. So, I do 'git diff' in one terminal, and then 'git add
path' or 'git add --patch path' in another terminal to build up a
commit (I suppose this is the work flow that 'git add --interactive'
is designed for ...), where I get 'path' from the diff by copying
and pasting. With 'diff.noprefix=true', I can copy with double left
click and paste with middle click; with 'diff.noprefix=false', to copy
I instead have to carefully highlight the non-prefix part of the path
in the diff, which is less convenient.

 At first glance I don't suspect making diff.noprefix imply -p0 for
 git am would be great, since that would generate the the opposite
 problem when applying patches from the outside world.  But maybe we
 need better autodetection and maybe noprefix is a good signal about
 when to use it.

Autodetecting the lack or presence of the 'a/' and 'b/' prefixes seems
like a great solution to me: externally user friendly and easy to
implement internally.

 Another complication is that unlike 'git diff', 'git apply' is
 plumbing that is meant to be useful and reliable for scripts.  And
 unlike most plumbing, there is no higher-level command with similar
 functionality for which we can experiment more freely with the UI.
 Adding a new command to fix that might be a good direction toward
 handling noprefix patches better.

Related to 'git apply' being a scriptable plumbing command: naively I
would expect there to be a scripting mode for Git commands which
ignored the local configuration entirely (e.g. ~/.gitconfig). I've
wanted this a few times and was surprised I could find no very sane
way to achieve it. In fact, here's the corresponding question I posted
on Stack Overflow while I was composing my original email (I wanted to
be sure that 'diff.noprefix=true' was the only relevant part of my
~/.gitconfig, so I wanted disable my ~/.gitconfig entirely):

http://stackoverflow.com/questions/23400449/how-to-make-git-temporarily-ignore-gitconfig

 [...]
 git show | git apply --reverse

 The following which only uses plumbing commands should work:

 git diff-tree -p HEAD^! |
 git apply --reverse

Nice! However, I don't now how to generalize this solution to other
(probably insane) use cases, e.g.

  git log -Sstring --patch | git apply --reverse

(Context: http://stackoverflow.com/a/23401018/470844).

 Thanks for some food for thought,
 Jonathan

Thanks for your reply. I didn't see it until today because a GMail
filter ate it :P

-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-05 Thread Jonathan Nieder
Nathan Collins wrote:
 On Wed, Apr 30, 2014 at 7:40 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 Nathan Collins wrote:

 git show | git apply --reverse

 The following which only uses plumbing commands should work:

 git diff-tree -p HEAD^! |
 git apply --reverse

 Nice! However, I don't now how to generalize this solution to other
 (probably insane) use cases, e.g.

   git log -Sstring --patch | git apply --reverse

This should do it:

git rev-list HEAD |
git diff-tree --no-commit-id -p -Sstring --stdin |
git apply --reverse

More generally, when scripting plumbing commands tend to do the right
thing.

Will think more about the documentation and other parts (or if someone
else sends a patch before I can, I won't mind).

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: Watchman support for git

2014-05-05 Thread David Turner
On Sun, 2014-05-04 at 07:15 +0700, Duy Nguyen wrote:
  I would like to merge the feature into master.  It works well for me,
  and some of my colleagues who have tried it out.
 
 Have you tried to turn watchman on by default, then run it with git
 test suite? That usually helps.

I have.  The tests work run fine under make, but prove sometimes freezes
due to an issue in libwatchman which I just fixed (and which I plan to
merge as soon as I can get a colleague to look the changes over).

  I can split the vmac patch into two, but one of them will remain quite
  large because it contains the code for VMAC and AES, which total a bit
  over 100k.  Since the list will probably reject that, I'll post a link
  to a repository containing the patches.
 
 With the read-cache deamon, I think hashing cost is less of an issue,
 so new hashing algorithm becomes less important. If you store the file
 cache in the deamon's memory only, there's no need to hash anything.
 But I guess you already tried this.

I agree that with the daemon, the cost is less of an issue, but I am not
100% sure it is a non-issue; consecutive commands that need to
read/write the index can still be slowed down.

  I'm not 100% sure how to split the watchman patch up.  I could add the
  fs_cache code and then separately add the watchman code that populates
  the cache.  Do you think there is a need to divide it up beyond this?
 
 I'll need to have closer look at your patches to give any suggestions.

I have uploaded a new version (which is about 5-10% faster and which
corrects some minor changes) to https://github.com/dturner-tw/git.git on
the watchman branch.  

 Although if you don't mind waiting a bit, I can try to put my
 untracked cache patches in good shape (hopefully in 2 weeks), then you
 can mostly avoid touching dir.c and reuse my work.

If the untracked cache patches are going to make it into master, then I
would of course be willing to rewrite on top of them.  But I would also
like to have a sense of whether there is any interest in watchman
support (outside of Twitter).

For what it's worth, the numbers today for index version 4 are for my
superscience repo are:
~380 (no watchman), ~260 (untracked-cache), ~175 (watchman).

That's because untracked-cache still has to stat every directory.

 I backed away from watchman support because I was worried about its
 overhead (of watchman itself, and git/watchman IPC because it's not
 designed specifically for git), which led me to try optimizing git as
 much as possible without watchman first, then see how/if watchman can
 help on top of that. I still think it's a good approach (maybe because
 it started to make me doubt if watchman could pull a big performance
 win on top to justify the changes to support it)

I think on large repositories (especially deeply-nested ones), with the
common case of a small number of changes, watchman will end up being a
big win.  Java tends towards deep nesting
(src/main/java/com/twitter/common/...), which is probably why my test
repo had the largest speedup (50%).  The IPC overhead might become bad
if there were a large number of changes, but so far this has not been an
issue for me in testing.  

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


Stilhaus Kitchens Complaints

2014-05-05 Thread kakonyet
Stilhaus Kitchens Complaints. fact is that there are no stilhaus kitchens
complaints.stilhaus
kitchens are the only kitchen company with no complaints




--
View this message in context: 
http://git.661346.n2.nabble.com/Stilhaus-Kitchens-Complaints-tp7609746.html
Sent from the git mailing list archive at Nabble.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: material for git training sessions/presentations

2014-05-05 Thread Jason St. John
On Mon, May 5, 2014 at 12:53 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 Scott Chacon wrote:
 The GitHub training team has all of their materials open sourced under
 a CC BY 3.0 license.  They're all written in Markdown and hosted on
 GitHub.  You can check them out here, including going through an
 online rendering of the materials:

 http://training.github.com/kit/

 Very nice!

 I'm skimming through the contents and I noticed you mention
 'color.ui = auto' a lot. There's no need for that, it has been the
 default since v1.8.4.

 Cheers.

 --
 Felipe Contreras

RHEL 6.5 ships Git version 1.7.1, so depending on what OS the audience
is using (e.g. RHEL 6.5 Workstation), that may still be relevant.

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