[Patch] git-svn: support dcommit --preserve-merges

2014-05-29 Thread Nate.
Need to update git-svn.perl to match Documentation/git-svn.txt so
--preserve-merges|p can be used in dcommit. Similar to
https://github.com/git/git/commit/b64e1f58158d1d1a8eafabbbf002a1a3c1d72929#diff-f9a64e34cbe6c3ee4f62698008a33773R571

Nate.


Documentation/git-svn.txt
 626 -m::
 627 --merge::
 628 -sstrategy::
 629 --strategy=strategy::
 630 -p::
 631 --preserve-merges::
 632 These are only used with the 'dcommit' and 'rebase' commands.


diff --git a/git-svn.perl b/git-svn.perl
index 0a32372..3f981f8 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -197,6 +197,7 @@ my %cmd = (
  'no-rebase' = \$_no_rebase,
  'mergeinfo=s' = \$_merge_info,
  'interactive|i' = \$_interactive,
+ 'preserve-merges|p' = \$_preserve_merges,
%cmt_opts, %fc_opts } ],
branch = [ \cmd_branch,
'Create a branch in the SVN repository',
--
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


[BUG REPORT] Git log pretty date

2014-05-29 Thread Rodrigo Fernandes
Hi,

I'm having some problems with the `git log` command.

Setup:
Happens in multiple versions, tested on 1.8.* and 2.*

The problem happens when I try to get a pretty log for a commit with a
wrong date.

Executing the command:
`git log --encoding=UTF-8 --date=raw --pretty=format:'%H,%P,%ad,%cd,%an,%s'`

I get an empty response on the date field, but since pretty has `%ad`
it should follow the --date and return the date even if wrong.

The problem happens in this repo:
`https://github.com/SamWM/jQuery-Plugins` on commit
`e9dddaf24c9de45d9b4efdf38eff7c30eb200f48`.

I posted it on SO too:
`http://stackoverflow.com/questions/23793188/find-date-for-not-dated-commit`

I think it is a bug probably on pretty since other raw formats show
the date as it should.

I tried to check the source code but have no idea where to start,
maybe if you point me on some direction I can take a look for my self.

Can you help?

Kind regards,
Rodrigo Fernandes
--
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 v3] Add an explicit GIT_DIR to the list of excludes

2014-05-29 Thread Jakub Narębski
On Thu, May 29, 2014 at 4:33 AM, Pasha Bolokhov
pasha.bolok...@gmail.com wrote:

 Agree, but partial here means... what? just a little doc?

I'm sorry, I tried to be cute and failed :$

This is not official name for documentation files that are meant for inclusion
and not as standalone manpages.  I think they are similar to 'partial templates'
http://guides.rubyonrails.org/layouts_and_rendering.html#using-partials
- usually just called partials - hence my use of this name.

Example: date-formats.txt included in git-commit(1), git-commit-tree(1)
and git-tag(1).

 On Wed, May 28, 2014 at 11:53 AM, Jakub Narębski jna...@gmail.com wrote:
 W dniu 2014-05-27 19:16, Pasha Bolokhov pisze:
[...]
  I suggest this. There appears to be a notion of standard
 excludes both in the code (dir.c) and in the man pages (git-grep,
 git-ls-files). However, it doesn't actually appear to be defined
 strictly speaking. So my suggestion is to define those standard
 excludes in one place (say gitignore(5)), and make other man pages
 (git-config, git-grip, git-ls-files) have references to that place
 instead of explaining every time in detail what is being excluded.


 Or define those standard excludes in standard-excludes.txt partial,
 and include said file from git-grep(1), git-ls-files(1), and perhaps
 gitignore(5).

-- 
Jakub Narębski
--
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 0/2] mingw: macro main(), const warnings

2014-05-29 Thread Stepan Kasal
Hello,

mingw.h defines a preprocessor macro main(), so that it can wrap the
original function and hoook into initialization.

The real main() function can have different types of its second
parameter (char**, const char**, char*[]).  It is not easy to match
the type and gcc issues a const warning.  My patch fixes that.

There were solutions for the same issue published ([1], [2]), but
none of them appeared in junio/pu.  This new solution should be more
future proof, as it modifies only compat/mingw.h; the *.c files can
have any of the types mentioned above.

I promise to take care of the integration into msysGit if this patch
gets accepted.  To make it easier, I'm submitting a patch that has
been part of msysGit for 3 years.

Karsten Blees (1):
  Win32: move main macro to a function

Stepan Kasal (1):
  mingw: avoid const warning

 compat/mingw.c | 15 +++
 compat/mingw.h | 17 ++---
 2 files changed, 21 insertions(+), 11 deletions(-)

-- 
1.9.2.msysgit.0.496.g23aa553

[1] a hack to fix the warning, by Pat Thoyts, in msysGit since
1.8.5.2.msysgit.0 (Dec 2013):
https://github.com/msysgit/git/commit/6949537a

[2] more elgant fix:
From: Marat Radchenko ma...@slonopotamus.org
Date: Tue, 29 Apr 2014 13:12:02 +0400
http://article.gmane.org/gmane.comp.version-control.git/247535
--
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 1/2] Win32: move main macro to a function

2014-05-29 Thread Stepan Kasal
From: Karsten Blees bl...@dcon.de
Date: Fri, 7 Jan 2011 19:47:23 +0100

The code in the MinGW main macro is getting more and more complex, move to
a separate initialization function for readabiliy and extensibility.

Signed-off-by: Karsten Blees bl...@dcon.de
Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
Signed-off-by: Stepan Kasal ka...@ucw.cz
---
 compat/mingw.c | 15 +++
 compat/mingw.h | 14 --
 2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index a0e13bc..c03bafa 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1847,3 +1847,18 @@ int mingw_offset_1st_component(const char *path)
 
return offset + is_dir_sep(path[offset]);
 }
+
+void mingw_startup()
+{
+   /* copy executable name to argv[0] */
+   __argv[0] = xstrdup(_pgmptr);
+
+   /* initialize critical section for waitpid pinfo_t list */
+   InitializeCriticalSection(pinfo_cs);
+
+   /* set up default file mode and file modes for stdin/out/err */
+   _fmode = _O_BINARY;
+   _setmode(_fileno(stdin), _O_BINARY);
+   _setmode(_fileno(stdout), _O_BINARY);
+   _setmode(_fileno(stderr), _O_BINARY);
+}
diff --git a/compat/mingw.h b/compat/mingw.h
index 3eaf822..15f0c9d 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -363,22 +363,16 @@ void free_environ(char **env);
 extern CRITICAL_SECTION pinfo_cs;
 
 /*
- * A replacement of main() that ensures that argv[0] has a path
- * and that default fmode and std(in|out|err) are in binary mode
+ * A replacement of main() that adds win32 specific initialization.
  */
 
+void mingw_startup();
 #define main(c,v) dummy_decl_mingw_main(); \
 static int mingw_main(c,v); \
 int main(int argc, char **argv) \
 { \
-   extern CRITICAL_SECTION pinfo_cs; \
-   _fmode = _O_BINARY; \
-   _setmode(_fileno(stdin), _O_BINARY); \
-   _setmode(_fileno(stdout), _O_BINARY); \
-   _setmode(_fileno(stderr), _O_BINARY); \
-   argv[0] = xstrdup(_pgmptr); \
-   InitializeCriticalSection(pinfo_cs); \
-   return mingw_main(argc, argv); \
+   mingw_startup(); \
+   return mingw_main(__argc, __argv); \
 } \
 static int mingw_main(c,v)
 
-- 
1.9.2.msysgit.0.496.g23aa553

--
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 2/2] mingw: avoid const warning

2014-05-29 Thread Stepan Kasal
Fix const warnings in http-fetch.c and remote-curl.c main() where is
argv declared as const.

The fix should work for all future declarations of main, no matter
whether the second parameter's type is char**, const char**, or
char *[].

Signed-off-by: Stepan Kasal ka...@ucw.cz
---
 compat/mingw.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/compat/mingw.h b/compat/mingw.h
index 15f0c9d..8745d19 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -369,10 +369,11 @@ extern CRITICAL_SECTION pinfo_cs;
 void mingw_startup();
 #define main(c,v) dummy_decl_mingw_main(); \
 static int mingw_main(c,v); \
-int main(int argc, char **argv) \
+int main(c, char **main_argv_not_used) \
 { \
+   typedef v, **argv_type; \
mingw_startup(); \
-   return mingw_main(__argc, __argv); \
+   return mingw_main(__argc, (argv_type)__argv); \
 } \
 static int mingw_main(c,v)
 
-- 
1.9.2.msysgit.0.496.g23aa553
--
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 REPORT] Git log pretty date

2014-05-29 Thread Duy Nguyen
On Thu, May 29, 2014 at 5:29 PM, Rodrigo Fernandes rtfrodr...@gmail.com wrote:
 I get an empty response on the date field, but since pretty has `%ad`
 it should follow the --date and return the date even if wrong.

 ...

 I tried to check the source code but have no idea where to start,
 maybe if you point me on some direction I can take a look for my self.

pretty.c, format_commit_one() - format_person_part() -
show_ident_date() - show_date(). The last one is in date.c
-- 
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: [BUG REPORT] Git log pretty date

2014-05-29 Thread Dennis Kaarsemaker
On do, 2014-05-29 at 11:29 +0100, Rodrigo Fernandes wrote:
 
 The problem happens when I try to get a pretty log for a commit with a
 wrong date.

The commit is:

===
$ git cat-file commit e9dddaf24c9de45d9b4efdf38eff7c30eb200f48
tree d63aeb159635cb231e191505a95a129a3b4a7b38
parent 9276202f1c0dcc360433df222c90f7874558f072
author SamWM s...@webmonkeysolutions.com 1288370243 --700
committer SamWM s...@webmonkeysolutions.com 1288370243 --700

Update version number, make text formatting and indentation consistent
with the rest of the code


Those dates are indeed wrong, I'm not surprised git refuses to parse
them.

-- 
Dennis Kaarsemaker
www.kaarsemaker.net

--
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: .gitmodules containing SSH clone URLs should fall back to HTTPS when SSH key is not valid/existent

2014-05-29 Thread Jens Lehmann
Am 29.05.2014 04:07, schrieb Jonathan Leonard:
 The title pretty much says it all.

But you do not give much information about your special use
case. I assume you have submodule repositories for which some
developers have a valid ssh key and others don't (maybe
because they should only have read access via https)?

If that is the case you might want to look into access control
tools like gitolite.

  Lack of this feature (or presence
 of this bug [depending on your perspective]) is a major PITA.

But why is https special? Why not fall back to the git
protocol? Or http? (And no: I'm not serious here ;-)

After the first failed clone of the submodule at via SSH the
developer could also just do a

   git config submodule.name.url https://host/repo

and override the URL from .gitmodules.

But maybe I misunderstood why you want to have a fall back?
--
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] tests: turn off git-daemon tests if FIFOs are not available

2014-05-29 Thread Stepan Kasal
Signed-off-by: Stepan Kasal ka...@ucw.cz
---

Hi,
  mingw does not have FIFOs, so it cannot run git-daemon tests.
Stepan

 t/lib-git-daemon.sh | 5 +
 1 file changed, 5 insertions(+)

diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh
index bc4b341..9b1271c 100644
--- a/t/lib-git-daemon.sh
+++ b/t/lib-git-daemon.sh
@@ -23,6 +23,11 @@ then
test_done
 fi
 
+if ! test_have_prereq PIPE
+then
+   test_skip_or_die $GIT_TEST_GIT_DAEMON file system does not support 
FIFOs
+fi
+
 LIB_GIT_DAEMON_PORT=${LIB_GIT_DAEMON_PORT-${this_test#t}}
 
 GIT_DAEMON_PID=
-- 
1.9.2.msysgit.0.493.g4becbf6.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


Re: [PATCH v4] Add an explicit GIT_DIR to the list of excludes

2014-05-29 Thread Duy Nguyen
On Thu, May 29, 2014 at 5:11 AM, Pasha Bolokhov
pasha.bolok...@gmail.com wrote:
 +   len = strlen(n_git); /* real_path() has stripped trailing slash */
 +   for (i = len - 1; i  0  !is_dir_sep(n_git[i]); i--) ;
 +   basename = n_git + i;
 +   if (is_dir_sep(*basename))
 +   basename++;
 +   if (strcmp(basename, .git)) {

 I think normalize_path_copy makes sure that dir sep is '/', so this
 code may be simplified to if (strcmp(n_git, .git)  (len == 4 ||
 strcmp(n_git + len - 5, /.git)))?

 Then if n_git is /ab  =  coredump. But I agree there is logic to
 this (if we check len = 4 first). However, we still need the
 basename.

Ah I missed this at add_exclude()

 So I've just shortened it a bit, what do you think: (notice
 the condition i = 0 btw)

 for (i = len - 1; i = 0  n_git[i] != '/'; i--) ;

There's basename() that does this for you. A compat version is
provided for Windows port so no portability worries.

 basename = n_git + i + 1;
 if (strcmp(basename, .git)) {


 +   const char *worktree = get_git_work_tree();
 +
 +   if (worktree == NULL ||
 +   dir_inside_of(n_git, get_git_work_tree()) = 0) {
 +   struct exclude_list *el = add_exclude_list(dir, 
 EXC_CMDL,
 +   GIT_DIR setup);
 +
 +   /* append a trailing slash to exclude directories */
 +   n_git[len] = '/';
 +   n_git[len + 1] = '\0';
 +   add_exclude(basename, , 0, el, 0);

Hmm.. I overlooked this bit before. So if  $GIT_DIR is /something/foo,
we set to ignore foo/. Because we know n_git must be part of
(normalized) get_git_work_tree() at this point, could we path n_git +
strlen(get_git_work_tree()) to add_exclude() instead of basename? Full
path makes sure we don't accidentally exclude too much.

The case when $GIT_DIR points to a _file_ seems uncovered.
setup_git_directory() will transform the file to the directory
internally and we never know the .git file's path (so we can't exclude
it). So people could accidentally add the .git file in, then remove it
from from work tree and suddenly the work tree becomes repo-less. It's
not as bad as .git _directory_ because we don't lose valuable data. I
don't know if you want to cover this too.
-- 
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] check_refname_component: Optimize

2014-05-29 Thread brian m. carlson
On Wed, May 28, 2014 at 03:57:35PM -0400, David Turner wrote:
 +ifdef NO_SSE
 + BASIC_CFLAGS += -DNO_SSE

This is not a great name.  This implies the system does not have SSE
(SSE 1), but in fact what you're concerned about is SSE 4.  This will be
confusing for users of older x86-64 processors, where SSE and SSE 2 are
architecturally guaranteed even though SSE 4 is not present.

 +else
 + BASIC_CFLAGS += -msse4
 +endif

You probably also want to autodetect when the system is not x86(-64)?
and turn this off.  I doubt PowerPC/SPARC/ARM users are going to want to
have to disable it manually.

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


signature.asc
Description: Digital signature


Re: [BUG REPORT] Git log pretty date

2014-05-29 Thread Rodrigo Fernandes
Dennis I think that could be an improvement.

Duy, can you point me where is the date print from normal `git log` or
`git show` so I can compare?


On Thu, May 29, 2014 at 12:07 PM, Dennis Kaarsemaker
den...@kaarsemaker.net wrote:
 On do, 2014-05-29 at 11:29 +0100, Rodrigo Fernandes wrote:

 The problem happens when I try to get a pretty log for a commit with a
 wrong date.

 The commit is:

 ===
 $ git cat-file commit e9dddaf24c9de45d9b4efdf38eff7c30eb200f48
 tree d63aeb159635cb231e191505a95a129a3b4a7b38
 parent 9276202f1c0dcc360433df222c90f7874558f072
 author SamWM s...@webmonkeysolutions.com 1288370243 --700
 committer SamWM s...@webmonkeysolutions.com 1288370243 --700

 Update version number, make text formatting and indentation consistent
 with the rest of the code
 

 Those dates are indeed wrong, I'm not surprised git refuses to parse
 them.

 --
 Dennis Kaarsemaker
 www.kaarsemaker.net

--
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 1/4] wrapper.c: introduce gentle xmallocz that does not die()

2014-05-29 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 git-compat-util.h |  1 +
 wrapper.c | 68 ++-
 2 files changed, 53 insertions(+), 16 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index f6d3a46..f23e4e4 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -524,6 +524,7 @@ extern try_to_free_t set_try_to_free_routine(try_to_free_t);
 extern char *xstrdup(const char *str);
 extern void *xmalloc(size_t size);
 extern void *xmallocz(size_t size);
+extern void *xmallocz_gentle(size_t size);
 extern void *xmemdupz(const void *data, size_t len);
 extern char *xstrndup(const char *str, size_t len);
 extern void *xrealloc(void *ptr, size_t size);
diff --git a/wrapper.c b/wrapper.c
index 0cc5636..7ab9a98 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -9,16 +9,23 @@ static void do_nothing(size_t size)
 
 static void (*try_to_free_routine)(size_t size) = do_nothing;
 
-static void memory_limit_check(size_t size)
+static int memory_limit_check(size_t size, int gentle)
 {
static int limit = -1;
if (limit == -1) {
const char *env = getenv(GIT_ALLOC_LIMIT);
limit = env ? atoi(env) * 1024 : 0;
}
-   if (limit  size  limit)
-   die(attempting to allocate %PRIuMAX over limit %d,
-   (intmax_t)size, limit);
+   if (limit  size  limit) {
+   if (gentle) {
+   error(attempting to allocate %PRIuMAX over limit %d,
+ (intmax_t)size, limit);
+   return -1;
+   } else
+   die(attempting to allocate %PRIuMAX over limit %d,
+   (intmax_t)size, limit);
+   }
+   return 0;
 }
 
 try_to_free_t set_try_to_free_routine(try_to_free_t routine)
@@ -42,11 +49,12 @@ char *xstrdup(const char *str)
return ret;
 }
 
-void *xmalloc(size_t size)
+static void *do_xmalloc(size_t size, int gentle)
 {
void *ret;
 
-   memory_limit_check(size);
+   if (memory_limit_check(size, gentle))
+   return NULL;
ret = malloc(size);
if (!ret  !size)
ret = malloc(1);
@@ -55,9 +63,16 @@ void *xmalloc(size_t size)
ret = malloc(size);
if (!ret  !size)
ret = malloc(1);
-   if (!ret)
-   die(Out of memory, malloc failed (tried to allocate 
%lu bytes),
-   (unsigned long)size);
+   if (!ret) {
+   if (!gentle)
+   die(Out of memory, malloc failed (tried to 
allocate %lu bytes),
+   (unsigned long)size);
+   else {
+   error(Out of memory, malloc failed (tried to 
allocate %lu bytes),
+ (unsigned long)size);
+   return NULL;
+   }
+   }
}
 #ifdef XMALLOC_POISON
memset(ret, 0xA5, size);
@@ -65,16 +80,37 @@ void *xmalloc(size_t size)
return ret;
 }
 
-void *xmallocz(size_t size)
+void *xmalloc(size_t size)
+{
+   return do_xmalloc(size, 0);
+}
+
+static void *do_xmallocz(size_t size, int gentle)
 {
void *ret;
-   if (unsigned_add_overflows(size, 1))
-   die(Data too large to fit into virtual memory space.);
-   ret = xmalloc(size + 1);
-   ((char*)ret)[size] = 0;
+   if (unsigned_add_overflows(size, 1)) {
+   if (gentle) {
+   error(Data too large to fit into virtual memory 
space.);
+   return NULL;
+   } else
+   die(Data too large to fit into virtual memory space.);
+   }
+   ret = do_xmalloc(size + 1, gentle);
+   if (ret)
+   ((char*)ret)[size] = 0;
return ret;
 }
 
+void *xmallocz(size_t size)
+{
+   return do_xmallocz(size, 0);
+}
+
+void *xmallocz_gentle(size_t size)
+{
+   return do_xmallocz(size, 1);
+}
+
 /*
  * xmemdupz() allocates (len + 1) bytes of memory, duplicates len bytes of
  * data to the allocated memory, zero terminates the allocated memory,
@@ -96,7 +132,7 @@ void *xrealloc(void *ptr, size_t size)
 {
void *ret;
 
-   memory_limit_check(size);
+   memory_limit_check(size, 0);
ret = realloc(ptr, size);
if (!ret  !size)
ret = realloc(ptr, 1);
@@ -115,7 +151,7 @@ void *xcalloc(size_t nmemb, size_t size)
 {
void *ret;
 
-   memory_limit_check(size * nmemb);
+   memory_limit_check(size * nmemb, 0);
ret = calloc(nmemb, size);
if (!ret  (!nmemb || !size))
ret = calloc(1, 1);
-- 
1.9.1.346.ga2b5940

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

[PATCH 4/4] diff: mark any file larger than core.bigfilethreshold binary

2014-05-29 Thread Nguyễn Thái Ngọc Duy
Too large files may lead to failure to allocate memory. If it happens
here, it could impact quite a few commands that involve
diff. Moreover, too large files are inefficient to compare anyway (and
most likely non-text), so mark them binary and skip looking at their
content.

Noticed-by: Dale R. Worley wor...@alum.mit.edu
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 diff.c   | 26 ++
 diffcore.h   |  1 +
 t/t1050-large.sh |  4 
 3 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/diff.c b/diff.c
index 54281cb..0a2f865 100644
--- a/diff.c
+++ b/diff.c
@@ -2185,8 +2185,8 @@ int diff_filespec_is_binary(struct diff_filespec *one)
one-is_binary = one-driver-binary;
else {
if (!one-data  DIFF_FILE_VALID(one))
-   diff_populate_filespec(one, 0);
-   if (one-data)
+   diff_populate_filespec(one, 
DIFF_POPULATE_IS_BINARY);
+   if (one-is_binary == -1  one-data)
one-is_binary = buffer_is_binary(one-data,
one-size);
if (one-is_binary == -1)
@@ -2721,6 +2721,11 @@ int diff_populate_filespec(struct diff_filespec *s, 
unsigned int flags)
}
if (size_only)
return 0;
+   if ((flags  DIFF_POPULATE_IS_BINARY) 
+   s-size  big_file_threshold  s-is_binary == -1) {
+   s-is_binary = 1;
+   return 0;
+   }
fd = open(s-path, O_RDONLY);
if (fd  0)
goto err_empty;
@@ -2742,16 +2747,21 @@ int diff_populate_filespec(struct diff_filespec *s, 
unsigned int flags)
}
else {
enum object_type type;
-   if (size_only) {
+   if (size_only || (flags  DIFF_POPULATE_IS_BINARY)) {
type = sha1_object_info(s-sha1, s-size);
if (type  0)
die(unable to read %s, sha1_to_hex(s-sha1));
-   } else {
-   s-data = read_sha1_file(s-sha1, type, s-size);
-   if (!s-data)
-   die(unable to read %s, sha1_to_hex(s-sha1));
-   s-should_free = 1;
+   if (size_only)
+   return 0;
+   if (s-size  big_file_threshold  s-is_binary == -1) 
{
+   s-is_binary = 1;
+   return 0;
+   }
}
+   s-data = read_sha1_file(s-sha1, type, s-size);
+   if (!s-data)
+   die(unable to read %s, sha1_to_hex(s-sha1));
+   s-should_free = 1;
}
return 0;
 }
diff --git a/diffcore.h b/diffcore.h
index a186d7c..e7760d9 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -56,6 +56,7 @@ extern void fill_filespec(struct diff_filespec *, const 
unsigned char *,
  int, unsigned short);
 
 #define DIFF_POPULATE_SIZE_ONLY 1
+#define DIFF_POPULATE_IS_BINARY 2
 extern int diff_populate_filespec(struct diff_filespec *, unsigned int);
 extern void diff_free_filespec_data(struct diff_filespec *);
 extern void diff_free_filespec_blob(struct diff_filespec *);
diff --git a/t/t1050-large.sh b/t/t1050-large.sh
index 333909b..4d922e2 100755
--- a/t/t1050-large.sh
+++ b/t/t1050-large.sh
@@ -112,6 +112,10 @@ test_expect_success 'diff --raw' '
git diff --raw HEAD^
 '
 
+test_expect_success 'diff --stat' '
+   git diff --stat HEAD^ HEAD
+'
+
 test_expect_success 'hash-object' '
git hash-object large1
 '
-- 
1.9.1.346.ga2b5940

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


[PATCH 3/4] diff.c: allow to pass more flags to diff_populate_filespec

2014-05-29 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 diff.c| 13 +++--
 diffcore-rename.c |  6 --
 diffcore.h|  3 ++-
 3 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/diff.c b/diff.c
index f72769a..54281cb 100644
--- a/diff.c
+++ b/diff.c
@@ -373,7 +373,7 @@ static unsigned long diff_filespec_size(struct 
diff_filespec *one)
 {
if (!DIFF_FILE_VALID(one))
return 0;
-   diff_populate_filespec(one, 1);
+   diff_populate_filespec(one, DIFF_POPULATE_SIZE_ONLY);
return one-size;
 }
 
@@ -1907,11 +1907,11 @@ static void show_dirstat(struct diff_options *options)
diff_free_filespec_data(p-one);
diff_free_filespec_data(p-two);
} else if (DIFF_FILE_VALID(p-one)) {
-   diff_populate_filespec(p-one, 1);
+   diff_populate_filespec(p-one, DIFF_POPULATE_SIZE_ONLY);
copied = added = 0;
diff_free_filespec_data(p-one);
} else if (DIFF_FILE_VALID(p-two)) {
-   diff_populate_filespec(p-two, 1);
+   diff_populate_filespec(p-two, DIFF_POPULATE_SIZE_ONLY);
copied = 0;
added = p-two-size;
diff_free_filespec_data(p-two);
@@ -2664,8 +2664,9 @@ static int diff_populate_gitlink(struct diff_filespec *s, 
int size_only)
  * grab the data for the blob (or file) for our own in-core comparison.
  * diff_filespec has data and size fields for this purpose.
  */
-int diff_populate_filespec(struct diff_filespec *s, int size_only)
+int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)
 {
+   int size_only = flags  DIFF_POPULATE_SIZE_ONLY;
int err = 0;
/*
 * demote FAIL to WARN to allow inspecting the situation
@@ -4695,8 +4696,8 @@ static int diff_filespec_check_stat_unmatch(struct 
diff_filepair *p)
!DIFF_FILE_VALID(p-two) ||
(p-one-sha1_valid  p-two-sha1_valid) ||
(p-one-mode != p-two-mode) ||
-   diff_populate_filespec(p-one, 1) ||
-   diff_populate_filespec(p-two, 1) ||
+   diff_populate_filespec(p-one, DIFF_POPULATE_SIZE_ONLY) ||
+   diff_populate_filespec(p-two, DIFF_POPULATE_SIZE_ONLY) ||
(p-one-size != p-two-size) ||
!diff_filespec_is_identical(p-one, p-two)) /* (2) */
p-skip_stat_unmatch_result = 1;
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 749a35d..8437917 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -147,9 +147,11 @@ static int estimate_similarity(struct diff_filespec *src,
 * is a possible size - we really should have a flag to
 * say whether the size is valid or not!)
 */
-   if (!src-cnt_data  diff_populate_filespec(src, 1))
+   if (!src-cnt_data 
+   diff_populate_filespec(src, DIFF_POPULATE_SIZE_ONLY))
return 0;
-   if (!dst-cnt_data  diff_populate_filespec(dst, 1))
+   if (!dst-cnt_data 
+   diff_populate_filespec(dst, DIFF_POPULATE_SIZE_ONLY))
return 0;
 
max_size = ((src-size  dst-size) ? src-size : dst-size);
diff --git a/diffcore.h b/diffcore.h
index c876dac..a186d7c 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -55,7 +55,8 @@ extern void free_filespec(struct diff_filespec *);
 extern void fill_filespec(struct diff_filespec *, const unsigned char *,
  int, unsigned short);
 
-extern int diff_populate_filespec(struct diff_filespec *, int);
+#define DIFF_POPULATE_SIZE_ONLY 1
+extern int diff_populate_filespec(struct diff_filespec *, unsigned int);
 extern void diff_free_filespec_data(struct diff_filespec *);
 extern void diff_free_filespec_blob(struct diff_filespec *);
 extern int diff_filespec_is_binary(struct diff_filespec *);
-- 
1.9.1.346.ga2b5940

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


Re: [BUG REPORT] Git log pretty date

2014-05-29 Thread Duy Nguyen
On Thu, May 29, 2014 at 7:24 PM, Rodrigo Fernandes rtfrodr...@gmail.com wrote:
 Duy, can you point me where is the date print from normal `git log` or
 `git show` so I can compare?

It's the same function, show_date() in date.c.
-- 
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] tests: turn off git-daemon tests if FIFOs are not available

2014-05-29 Thread Jeff King
On Thu, May 29, 2014 at 01:36:14PM +0200, Stepan Kasal wrote:

 Signed-off-by: Stepan Kasal ka...@ucw.cz
 ---
 
 Hi,
   mingw does not have FIFOs, so it cannot run git-daemon tests.

Thanks. I took a peek at the mkfifo call here. It is used to make sure
the daemon has started before we run the tests which depend on it. I
suspect we could do something with a regular pipe, but the code would be
rather tricky.  Simply skipping the daemon tests is a reasonable
compromise until somebody feels like trying to make it work without a
fifo.

-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: git-multimail: migration: Config is not iterable

2014-05-29 Thread Elijah Newren
On Thu, May 29, 2014 at 7:22 AM, Azat Khuzhin a3at.m...@gmail.com wrote:
 Hi there,

 Using the latest version of git-multimail there is an issue with
 migration:

 $ ~azat/git-multimail/git-multimail/migrate-mailhook-config --overwrite
 Traceback (most recent call last):
...
   File /home/azat/git-multimail/git-multimail/migrate-mailhook-config, line 
 66, in _check_old_config_exists
 if name in old:
 TypeError: argument of type 'Config' is not iterable

Thanks for the report.  I'm not the strongest on python data model and
its special methods but I believe I've got a fix:
https://github.com/mhagger/git-multimail/pull/58
--
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 v12 00/44] Use ref transactions for all ref updates

2014-05-29 Thread Ronnie Sahlberg
This patch series can also be found at
https://github.com/rsahlberg/git/tree/ref-transactions


Ronnie please review these remaining patches in this series.


 Sahlberg (44):
  refs.c: constify the sha arguments for
ref_transaction_create|delete|update
  refs.c: allow passing NULL to ref_transaction_free
  refs.c: add a strbuf argument to ref_transaction_commit for error
logging
  refs.c: add an err argument to repack_without_refs
  refs.c: make ref_update_reject_duplicates take a strbuf argument for
errors
  refs.c: add an err argument to delete_ref_loose
  refs.c: make update_ref_write update a strbuf on failure
  update-ref.c: log transaction error from the update_ref
  refs.c: remove the onerr argument to ref_transaction_commit
  refs.c: change ref_transaction_update() to do error checking and
return status
  refs.c: change ref_transaction_create to do error checking and return
status
  refs.c: ref_transaction_delete to check for error and return status
  tag.c: use ref transactions when doing updates
  replace.c: use the ref transaction functions for updates
  commit.c: use ref transactions for updates
  sequencer.c: use ref transactions for all ref updates
  fast-import.c: change update_branch to use ref transactions
  branch.c: use ref transaction for all ref updates
  refs.c: change update_ref to use a transaction
  refs.c: free the transaction before returning when number of updates
is 0
  refs.c: ref_transaction_commit should not free the transaction
  fetch.c: clear errno before calling functions that might set it
  fetch.c: change s_update_ref to use a ref transaction
  fetch.c: use a single ref transaction for all ref updates
  receive-pack.c: use a reference transaction for updating the refs
  fast-import.c: use a ref transaction when dumping tags
  walker.c: use ref transaction for ref updates
  refs.c: make write_ref_sha1 static
  refs.c: make lock_ref_sha1 static
  refs.c: add transaction.status and track OPEN/CLOSED/ERROR
  refs.c: remove the update_ref_lock function
  refs.c: remove the update_ref_write function
  refs.c: remove lock_ref_sha1
  refs.c: make prune_ref use a transaction to delete the ref
  refs.c: make delete_ref use a transaction
  refs.c: pass the ref log message to _create/delete/update instead of
_commit
  refs.c: pass NULL as *flags to read_ref_full
  refs.c: pack all refs before we start to rename a ref
  refs.c: move the check for valid refname to lock_ref_sha1_basic
  refs.c: call lock_ref_sha1_basic directly from commit
  refs.c: add a new flag for transaction delete for refs we know are
packed only
  refs.c: pass a skip list to name_conflict_fn
  refs.c: make rename_ref use a transaction
  refs.c: remove forward declaration of write_ref_sha1

 branch.c   |  30 ++--
 builtin/commit.c   |  24 ++-
 builtin/fetch.c|  29 +--
 builtin/receive-pack.c |  21 +--
 builtin/replace.c  |  15 +-
 builtin/tag.c  |  15 +-
 builtin/update-ref.c   |  32 ++--
 cache.h|   2 +
 fast-import.c  |  42 +++--
 lockfile.c |  21 ++-
 refs.c | 468 +
 refs.h |  54 +++---
 sequencer.c|  24 ++-
 t/t3200-branch.sh  |   2 +-
 walker.c   |  51 +++---
 15 files changed, 491 insertions(+), 339 deletions(-)

-- 
2.0.0.rc3.510.g20c254b

--
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 v12 25/41] fast-import.c: use a ref transaction when dumping tags

2014-05-29 Thread Ronnie Sahlberg
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 fast-import.c | 29 +++--
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index 4a7b196..587ef4a 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1735,15 +1735,32 @@ static void dump_tags(void)
 {
static const char *msg = fast-import;
struct tag *t;
-   struct ref_lock *lock;
-   char ref_name[PATH_MAX];
+   struct strbuf ref_name = STRBUF_INIT;
+   struct strbuf err = STRBUF_INIT;
+   struct ref_transaction *transaction;
 
+   transaction = ref_transaction_begin(err);
+   if (!transaction) {
+   failure |= error(%s, err.buf);
+   goto cleanup;
+   }
for (t = first_tag; t; t = t-next_tag) {
-   sprintf(ref_name, tags/%s, t-name);
-   lock = lock_ref_sha1(ref_name, NULL);
-   if (!lock || write_ref_sha1(lock, t-sha1, msg)  0)
-   failure |= error(Unable to update %s, ref_name);
+   strbuf_reset(ref_name);
+   strbuf_addf(ref_name, refs/tags/%s, t-name);
+
+   if (ref_transaction_update(transaction, ref_name.buf, t-sha1,
+  NULL, 0, 0, err)) {
+   failure |= error(%s, err.buf);
+   goto cleanup;
+   }
}
+   if (ref_transaction_commit(transaction, msg, err))
+   failure |= error(%s, err.buf);
+
+ cleanup:
+   ref_transaction_free(transaction);
+   strbuf_release(ref_name);
+   strbuf_release(err);
 }
 
 static void dump_marks_helper(FILE *f,
-- 
2.0.0.rc3.474.g3833130

--
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 v12 40/41] fetch.c: change s_update_ref to use a ref transaction

2014-05-29 Thread Ronnie Sahlberg
Change s_update_ref to use a ref transaction for the ref update.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 builtin/fetch.c | 21 +
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index faa1233..b13e8f9 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -375,22 +375,27 @@ static int s_update_ref(const char *action,
 {
char msg[1024];
char *rla = getenv(GIT_REFLOG_ACTION);
-   static struct ref_lock *lock;
+   struct ref_transaction *transaction;
+   struct strbuf err = STRBUF_INIT;
 
if (dry_run)
return 0;
if (!rla)
rla = default_rla.buf;
snprintf(msg, sizeof(msg), %s: %s, rla, action);
-   lock = lock_any_ref_for_update(ref-name,
-  check_old ? ref-old_sha1 : NULL,
-  0, NULL);
-   if (!lock)
-   return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
- STORE_REF_ERROR_OTHER;
-   if (write_ref_sha1(lock, ref-new_sha1, msg)  0)
+
+   transaction = ref_transaction_begin(err);
+   if (!transaction ||
+   ref_transaction_update(transaction, ref-name, ref-new_sha1,
+  ref-old_sha1, 0, check_old, msg, err) ||
+   ref_transaction_commit(transaction, err)) {
+   ref_transaction_free(transaction);
+   error(%s, err.buf);
+   strbuf_release(err);
return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
  STORE_REF_ERROR_OTHER;
+   }
+   ref_transaction_free(transaction);
return 0;
 }
 
-- 
2.0.0.rc3.474.g3833130

--
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 v12 06/41] refs.c: add an err argument to repack_without_refs

2014-05-29 Thread Ronnie Sahlberg
Update repack_without_refs to take an err argument and update it if there
is a failure. Pass the err variable from ref_transaction_commit to this
function so that callers can print a meaningful error message if _commit
fails due to a problem in repack_without_refs.

Add a new function unable_to_lock_message that takes a strbuf argument and
fills in the reason for the failure.

In commit_packed_refs, make sure that we propagate any errno that
commit_lock_file might have set back to our caller.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 cache.h|  2 ++
 lockfile.c | 21 -
 refs.c | 25 +++--
 3 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/cache.h b/cache.h
index 8c6cdc2..5858da8 100644
--- a/cache.h
+++ b/cache.h
@@ -559,6 +559,8 @@ struct lock_file {
 #define LOCK_DIE_ON_ERROR 1
 #define LOCK_NODEREF 2
 extern int unable_to_lock_error(const char *path, int err);
+extern void unable_to_lock_message(const char *path, int err,
+  struct strbuf *buf);
 extern NORETURN void unable_to_lock_index_die(const char *path, int err);
 extern int hold_lock_file_for_update(struct lock_file *, const char *path, 
int);
 extern int hold_lock_file_for_append(struct lock_file *, const char *path, 
int);
diff --git a/lockfile.c b/lockfile.c
index 8fbcb6a..92e0397 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -157,33 +157,36 @@ static int lock_file(struct lock_file *lk, const char 
*path, int flags)
return lk-fd;
 }
 
-static char *unable_to_lock_message(const char *path, int err)
+void unable_to_lock_message(const char *path, int err, struct strbuf *buf)
 {
-   struct strbuf buf = STRBUF_INIT;
 
if (err == EEXIST) {
-   strbuf_addf(buf, Unable to create '%s.lock': %s.\n\n
+   strbuf_addf(buf, Unable to create '%s.lock': %s.\n\n
If no other git process is currently running, this 
probably means a\n
git process crashed in this repository earlier. Make sure 
no other git\n
process is running and remove the file manually to 
continue.,
absolute_path(path), strerror(err));
} else
-   strbuf_addf(buf, Unable to create '%s.lock': %s,
+   strbuf_addf(buf, Unable to create '%s.lock': %s,
absolute_path(path), strerror(err));
-   return strbuf_detach(buf, NULL);
 }
 
 int unable_to_lock_error(const char *path, int err)
 {
-   char *msg = unable_to_lock_message(path, err);
-   error(%s, msg);
-   free(msg);
+   struct strbuf buf = STRBUF_INIT;
+
+   unable_to_lock_message(path, err, buf);
+   error(%s, buf.buf);
+   strbuf_release(buf);
return -1;
 }
 
 NORETURN void unable_to_lock_index_die(const char *path, int err)
 {
-   die(%s, unable_to_lock_message(path, err));
+   struct strbuf buf = STRBUF_INIT;
+
+   unable_to_lock_message(path, err, buf);
+   die(%s, buf.buf);
 }
 
 int hold_lock_file_for_update(struct lock_file *lk, const char *path, int 
flags)
diff --git a/refs.c b/refs.c
index 25c3a93..6e28841 100644
--- a/refs.c
+++ b/refs.c
@@ -2208,6 +2208,7 @@ int commit_packed_refs(void)
struct packed_ref_cache *packed_ref_cache =
get_packed_ref_cache(ref_cache);
int error = 0;
+   int save_errno = 0;
 
if (!packed_ref_cache-lock)
die(internal error: packed-refs not locked);
@@ -2217,10 +2218,13 @@ int commit_packed_refs(void)
do_for_each_entry_in_dir(get_packed_ref_dir(packed_ref_cache),
 0, write_packed_entry_fn,
 packed_ref_cache-lock-fd);
-   if (commit_lock_file(packed_ref_cache-lock))
+   if (commit_lock_file(packed_ref_cache-lock)) {
+   save_errno = errno;
error = -1;
+   }
packed_ref_cache-lock = NULL;
release_packed_ref_cache(packed_ref_cache);
+   errno = save_errno;
return error;
 }
 
@@ -2427,12 +2431,12 @@ static int curate_packed_ref_fn(struct ref_entry 
*entry, void *cb_data)
return 0;
 }
 
-static int repack_without_refs(const char **refnames, int n)
+static int repack_without_refs(const char **refnames, int n, struct strbuf 
*err)
 {
struct ref_dir *packed;
struct string_list refs_to_delete = STRING_LIST_INIT_DUP;
struct string_list_item *ref_to_delete;
-   int i, removed = 0;
+   int i, ret, removed = 0;
 
/* Look for a packed ref */
for (i = 0; i  n; i++)
@@ -2444,6 +2448,11 @@ static int repack_without_refs(const char **refnames, 
int n)
return 0; /* no refname exists in packed refs */
 
if (lock_packed_refs(0)) {
+   if (err) {
+   unable_to_lock_message(git_path(packed-refs), errno,
+  err);
+   

[PATCH v12 08/41] refs.c: add an err argument to delete_ref_loose

2014-05-29 Thread Ronnie Sahlberg
Add an err argument to delete_loose_ref so that we can pass a descriptive
error string back to the caller. Pass the err argument from transaction
commit to this function so that transaction users will have a nice error
string if the transaction failed due to delete_loose_ref.

Add a new function unlink_or_err that we can call from delete_ref_loose. This
function is similar to unlink_or_warn except that we can pass it an err
argument. If err is non-NULL the function will populate err instead of
printing a warning().

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs.c | 35 +--
 1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/refs.c b/refs.c
index 9d079ec..d768a5e 100644
--- a/refs.c
+++ b/refs.c
@@ -2491,16 +2491,38 @@ static int repack_without_ref(const char *refname)
return repack_without_refs(refname, 1, NULL);
 }
 
-static int delete_ref_loose(struct ref_lock *lock, int flag)
+static int add_err_if_unremovable(const char *op, const char *file,
+ struct strbuf *e, int rc)
+{
+   int err = errno;
+   if (rc  0   errno != ENOENT) {
+   strbuf_addf(e, unable to %s %s: %s,
+   op, file, strerror(errno));
+   errno = err;
+   return -1;
+   }
+   return 0;
+}
+
+static int unlink_or_err(const char *file, struct strbuf *err)
+{
+   if (err)
+   return add_err_if_unremovable(unlink, file, err,
+ unlink(file));
+   else
+   return unlink_or_warn(file);
+}
+
+static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf 
*err)
 {
if (!(flag  REF_ISPACKED) || flag  REF_ISSYMREF) {
/* loose */
-   int err, i = strlen(lock-lk-filename) - 5; /* .lock */
+   int res, i = strlen(lock-lk-filename) - 5; /* .lock */
 
lock-lk-filename[i] = 0;
-   err = unlink_or_warn(lock-lk-filename);
+   res = unlink_or_err(lock-lk-filename, err);
lock-lk-filename[i] = '.';
-   if (err  errno != ENOENT)
+   if (res  errno != ENOENT)
return 1;
}
return 0;
@@ -2514,7 +2536,7 @@ int delete_ref(const char *refname, const unsigned char 
*sha1, int delopt)
lock = lock_ref_sha1_basic(refname, sha1, delopt, flag);
if (!lock)
return 1;
-   ret |= delete_ref_loose(lock, flag);
+   ret |= delete_ref_loose(lock, flag, NULL);
 
/* removing the loose one could have resurrected an earlier
 * packed one.  Also, if it was not loose we need to repack
@@ -3494,7 +3516,8 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
 
if (update-lock) {
delnames[delnum++] = update-lock-ref_name;
-   ret |= delete_ref_loose(update-lock, update-type);
+   ret |= delete_ref_loose(update-lock, update-type,
+   err);
}
}
 
-- 
2.0.0.rc3.474.g3833130

--
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 v12 33/41] refs.c: pass the ref log message to _create/delete/update instead of _commit

2014-05-29 Thread Ronnie Sahlberg
Change the reference transactions so that we pass the reflog message
through to the create/delete/update function instead of the commit message.
This allows for individual messages for each change in a multi ref
transaction.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 branch.c   |  4 ++--
 builtin/commit.c   |  4 ++--
 builtin/fetch.c|  3 +--
 builtin/receive-pack.c |  7 ---
 builtin/replace.c  |  4 ++--
 builtin/tag.c  |  4 ++--
 builtin/update-ref.c   | 13 +++--
 fast-import.c  |  8 
 refs.c | 34 +-
 refs.h |  8 
 sequencer.c|  4 ++--
 walker.c   |  5 ++---
 12 files changed, 53 insertions(+), 45 deletions(-)

diff --git a/branch.c b/branch.c
index c1eae00..e0439af 100644
--- a/branch.c
+++ b/branch.c
@@ -301,8 +301,8 @@ void create_branch(const char *head,
transaction = ref_transaction_begin(err);
if (!transaction ||
ref_transaction_update(transaction, ref.buf, sha1,
-  null_sha1, 0, !forcing, err) ||
-   ref_transaction_commit(transaction, msg, err))
+  null_sha1, 0, !forcing, msg, err) ||
+   ref_transaction_commit(transaction, err))
die(%s, err.buf);
ref_transaction_free(transaction);
}
diff --git a/builtin/commit.c b/builtin/commit.c
index 7db194f..608296c 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1721,8 +1721,8 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
ref_transaction_update(transaction, HEAD, sha1,
   current_head ?
   current_head-object.sha1 : NULL,
-  0, !!current_head, err) ||
-   ref_transaction_commit(transaction, sb.buf, err)) {
+  0, !!current_head, sb.buf, err) ||
+   ref_transaction_commit(transaction, err)) {
rollback_index_files();
die(%s, err.buf);
}
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 55f457c..faa1233 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -673,10 +673,9 @@ static int store_updated_refs(const char *raw_url, const 
char *remote_name,
}
}
}
-
if (rc  STORE_REF_ERROR_DF_CONFLICT)
error(_(some local refs could not be updated; try running\n
-  'git remote prune %s' to remove any old, conflicting 
+ 'git remote prune %s' to remove any old, conflicting 
  branches), remote_name);
 
  abort:
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 13f4a63..5653fa2 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -586,10 +586,11 @@ static const char *update(struct command *cmd, struct 
shallow_info *si)
transaction = ref_transaction_begin(err);
if (!transaction ||
ref_transaction_update(transaction, namespaced_name,
-  new_sha1, old_sha1, 0, 1, err) ||
-   ref_transaction_commit(transaction, push, err)) {
-
+  new_sha1, old_sha1, 0, 1, push,
+  err) ||
+   ref_transaction_commit(transaction, err)) {
const char *str;
+
string_list_append(error_strings, err.buf);
str = error_strings.items[error_strings.nr - 1].string;
strbuf_release(err);
diff --git a/builtin/replace.c b/builtin/replace.c
index f24d814..09bfd40 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -160,8 +160,8 @@ static int replace_object_sha1(const char *object_ref,
transaction = ref_transaction_begin(err);
if (!transaction ||
ref_transaction_update(transaction, ref, repl, prev,
-  0, !is_null_sha1(prev), err) ||
-   ref_transaction_commit(transaction, NULL, err))
+  0, !is_null_sha1(prev), NULL, err) ||
+   ref_transaction_commit(transaction, err))
die(%s, err.buf);
 
ref_transaction_free(transaction);
diff --git a/builtin/tag.c b/builtin/tag.c
index c9bfc9a..74af63e 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -705,8 +705,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
transaction = ref_transaction_begin(err);
if (!transaction ||
ref_transaction_update(transaction, ref.buf, object, prev,
-  0, !is_null_sha1(prev), err) ||
-   ref_transaction_commit(transaction, NULL, err))
+  

[PATCH v12 31/41] refs.c: make prune_ref use a transaction to delete the ref

2014-05-29 Thread Ronnie Sahlberg
Change prune_ref to delete the ref using a ref transaction. To do this we also
need to add a new flag REF_ISPRUNING that will tell the transaction that we
do not want to delete this ref from the packed refs. This flag is private to
refs.c and not exposed to external callers.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs.c | 28 +---
 refs.h | 11 ++-
 2 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/refs.c b/refs.c
index 7cea694..60593d7 100644
--- a/refs.c
+++ b/refs.c
@@ -30,6 +30,12 @@ static inline int bad_ref_char(int ch)
 }
 
 /*
+ * Used as a flag to ref_transaction_delete when a loose ref is being
+ * pruned.
+ */
+#define REF_ISPRUNING  0x0100
+
+/*
  * Try to read one refname component from the front of refname.  Return
  * the length of the component found, or -1 if the component is not
  * legal.
@@ -2328,17 +2334,24 @@ static void try_remove_empty_parents(char *name)
 /* make sure nobody touched the ref, and unlink */
 static void prune_ref(struct ref_to_prune *r)
 {
-   struct ref_lock *lock;
+   struct ref_transaction *transaction;
+   struct strbuf err = STRBUF_INIT;
 
if (check_refname_format(r-name + 5, 0))
return;
 
-   lock = lock_ref_sha1_basic(r-name, r-sha1, 0, NULL);
-   if (lock) {
-   unlink_or_warn(git_path(%s, r-name));
-   unlock_ref(lock);
-   try_remove_empty_parents(r-name);
+   transaction = ref_transaction_begin(err);
+   if (!transaction ||
+   ref_transaction_delete(transaction, r-name, r-sha1,
+  REF_ISPRUNING, 1, err) ||
+   ref_transaction_commit(transaction, NULL, err)) {
+   ref_transaction_free(transaction);
+   error(%s, err.buf);
+   strbuf_release(err);
+   return;
}
+   ref_transaction_free(transaction);
+   try_remove_empty_parents(r-name);
 }
 
 static void prune_refs(struct ref_to_prune *r)
@@ -3536,9 +3549,10 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
struct ref_update *update = updates[i];
 
if (update-lock) {
-   delnames[delnum++] = update-lock-ref_name;
ret |= delete_ref_loose(update-lock, update-type,
err);
+   if (!(update-flags  REF_ISPRUNING))
+   delnames[delnum++] = update-lock-ref_name;
}
}
 
diff --git a/refs.h b/refs.h
index c38ee09..dee7c8f 100644
--- a/refs.h
+++ b/refs.h
@@ -171,8 +171,17 @@ extern int ref_exists(const char *);
  */
 extern int peel_ref(const char *refname, unsigned char *sha1);
 
-/** Locks any ref (for 'HEAD' type refs). */
+/*
+ * Flags controlling lock_any_ref_for_update(), ref_transaction_update(),
+ * ref_transaction_create(), etc.
+ * REF_NODEREF: act on the ref directly, instead of dereferencing
+ *  symbolic references.
+ *
+ * Flags = 0x100 are reserved for internal use.
+ */
 #define REF_NODEREF0x01
+
+/** Locks any ref (for 'HEAD' type refs). */
 extern struct ref_lock *lock_any_ref_for_update(const char *refname,
const unsigned char *old_sha1,
int flags, int *type_p);
-- 
2.0.0.rc3.474.g3833130

--
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 v12 16/41] refs.c: add transaction.status and track OPEN/CLOSED/ERROR

2014-05-29 Thread Ronnie Sahlberg
Track the status of a transaction in a new status field. Check the field for
sanity, i.e. that status must be OPEN when _commit/_create/_delete or
_update is called or else die(BUG:...)

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs.c | 36 +++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/refs.c b/refs.c
index b5186cd..00d8fc9 100644
--- a/refs.c
+++ b/refs.c
@@ -3331,6 +3331,30 @@ struct ref_update {
 };
 
 /*
+ * Transaction states.
+ * OPEN:   The transaction is in a valid state and can accept new updates.
+ * An OPEN transaction can be committed.
+ * CLOSED: If an open transaction is successfully committed the state will
+ * change to CLOSED. No further changes can be made to a CLOSED
+ * transaction.
+ * CLOSED means that all updates have been successfully committed and
+ * the only thing that remains is to free the completed transaction.
+ * ERROR:  The transaction has failed and is no longer committable.
+ * Eventhough the transaction can no longer be committed it is still
+ * possible to add additional updates to the transaction. The reason
+ * for this is to allow a caller to continue trying more updates
+ * so that a caller can report a list of ALL updates that would fail
+ * instead of just the first update that fails.
+ * An ERRORed transaction can not be committed and must be rolled
+ * back using transaction_free.
+ */
+enum ref_transaction_state {
+   REF_TRANSACTION_OPEN   = 0,
+   REF_TRANSACTION_CLOSED = 1,
+   REF_TRANSACTION_ERROR  = 2,
+};
+
+/*
  * Data structure for holding a reference transaction, which can
  * consist of checks and updates to multiple references, carried out
  * as atomically as possible.  This structure is opaque to callers.
@@ -3339,6 +3363,8 @@ struct ref_transaction {
struct ref_update **updates;
size_t alloc;
size_t nr;
+   enum ref_transaction_state state;
+   int status;
 };
 
 struct ref_transaction *ref_transaction_begin(struct strbuf *err)
@@ -3476,8 +3502,13 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
int n = transaction-nr;
struct ref_update **updates = transaction-updates;
 
-   if (!n)
+   if (transaction-state != REF_TRANSACTION_OPEN)
+   return transaction-status;
+
+   if (!n) {
+   transaction-state = REF_TRANSACTION_CLOSED;
return 0;
+   }
 
/* Allocate work space */
delnames = xmalloc(sizeof(*delnames) * n);
@@ -3540,6 +3571,9 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
clear_loose_ref_cache(ref_cache);
 
 cleanup:
+   transaction-state = ret ? REF_TRANSACTION_ERROR
+   : REF_TRANSACTION_CLOSED;
+
for (i = 0; i  n; i++)
if (updates[i]-lock)
unlock_ref(updates[i]-lock);
-- 
2.0.0.rc3.474.g3833130

--
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 v12 36/41] refs.c: move the check for valid refname to lock_ref_sha1_basic

2014-05-29 Thread Ronnie Sahlberg
Move the check for check_refname_format from lock_any_ref_for_update
to lock_ref_sha1_basic. At some later stage we will get rid of
lock_any_ref_for_update completely.

This leaves lock_any_ref_for_updates as a no-op wrapper which could be removed.
But this wrapper is also called from an external caller and we will soon
make changes to the signature to lock_ref_sha1_basic that we do not want to
expose to that caller.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/refs.c b/refs.c
index 5680028..df00993 100644
--- a/refs.c
+++ b/refs.c
@@ -2043,6 +2043,9 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
*refname,
int missing = 0;
int attempts_remaining = 3;
 
+   if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL))
+   return NULL;
+
lock = xcalloc(1, sizeof(struct ref_lock));
lock-lock_fd = -1;
 
@@ -2134,8 +2137,6 @@ struct ref_lock *lock_any_ref_for_update(const char 
*refname,
 const unsigned char *old_sha1,
 int flags, int *type_p)
 {
-   if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL))
-   return NULL;
return lock_ref_sha1_basic(refname, old_sha1, flags, type_p);
 }
 
-- 
2.0.0.rc3.474.g3833130

--
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 v12 38/41] refs.c: pass a skip list to name_conflict_fn

2014-05-29 Thread Ronnie Sahlberg
Allow passing a list of refs to skip checking to name_conflict_fn.
There are some conditions where we want to allow a temporary conflict and skip
checking those refs. For example if we have a transaction that
1, guarantees that m is a packed refs and there is no loose ref for m
2, the transaction will delete m from the packed ref
3, the transaction will create conflicting m/m

For this case we want to be able to lock and create m/m since we know that the
conflict is only transient. I.e. the conflict will be automatically resolved
by the transaction when it deletes m.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs.c | 36 ++--
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/refs.c b/refs.c
index edf73b7..ec8d642 100644
--- a/refs.c
+++ b/refs.c
@@ -793,11 +793,17 @@ struct name_conflict_cb {
const char *refname;
const char *oldrefname;
const char *conflicting_refname;
+   const char **skip;
+   int skipnum;
 };
 
 static int name_conflict_fn(struct ref_entry *entry, void *cb_data)
 {
struct name_conflict_cb *data = (struct name_conflict_cb *)cb_data;
+   int i;
+   for (i = 0; i  data-skipnum; i++)
+   if (!strcmp(entry-name, data-skip[i]))
+   return 0;
if (data-oldrefname  !strcmp(data-oldrefname, entry-name))
return 0;
if (names_conflict(data-refname, entry-name)) {
@@ -812,15 +818,19 @@ static int name_conflict_fn(struct ref_entry *entry, void 
*cb_data)
  * conflicting with the name of an existing reference in dir.  If
  * oldrefname is non-NULL, ignore potential conflicts with oldrefname
  * (e.g., because oldrefname is scheduled for deletion in the same
- * operation).
+ * operation). skip contains a list of refs we want to skip checking for
+ * conflicts with.
  */
 static int is_refname_available(const char *refname, const char *oldrefname,
-   struct ref_dir *dir)
+   struct ref_dir *dir,
+   const char **skip, int skipnum)
 {
struct name_conflict_cb data;
data.refname = refname;
data.oldrefname = oldrefname;
data.conflicting_refname = NULL;
+   data.skip = skip;
+   data.skipnum = skipnum;
 
sort_ref_dir(dir);
if (do_for_each_entry_in_dir(dir, 0, name_conflict_fn, data)) {
@@ -2032,7 +2042,8 @@ int dwim_log(const char *str, int len, unsigned char 
*sha1, char **log)
 
 static struct ref_lock *lock_ref_sha1_basic(const char *refname,
const unsigned char *old_sha1,
-   int flags, int *type_p)
+   int flags, int *type_p,
+   const char **skip, int skipnum)
 {
char *ref_file;
const char *orig_refname = refname;
@@ -2079,7 +2090,9 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
*refname,
 * name is a proper prefix of our refname.
 */
if (missing 
-!is_refname_available(refname, NULL, get_packed_refs(ref_cache))) 
{
+!is_refname_available(refname, NULL,
+  get_packed_refs(ref_cache),
+  skip, skipnum)) {
last_errno = ENOTDIR;
goto error_return;
}
@@ -2137,7 +2150,7 @@ struct ref_lock *lock_any_ref_for_update(const char 
*refname,
 const unsigned char *old_sha1,
 int flags, int *type_p)
 {
-   return lock_ref_sha1_basic(refname, old_sha1, flags, type_p);
+   return lock_ref_sha1_basic(refname, old_sha1, flags, type_p, NULL, 0);
 }
 
 /*
@@ -2624,10 +2637,12 @@ int rename_ref(const char *oldrefname, const char 
*newrefname, const char *logms
if (!symref)
return error(refname %s not found, oldrefname);
 
-   if (!is_refname_available(newrefname, oldrefname, 
get_packed_refs(ref_cache)))
+   if (!is_refname_available(newrefname, oldrefname,
+ get_packed_refs(ref_cache), NULL, 0))
return 1;
 
-   if (!is_refname_available(newrefname, oldrefname, 
get_loose_refs(ref_cache)))
+   if (!is_refname_available(newrefname, oldrefname,
+ get_loose_refs(ref_cache), NULL, 0))
return 1;
 
if (log  rename(git_path(logs/%s, oldrefname), 
git_path(TMP_RENAMED_LOG)))
@@ -2660,7 +2675,7 @@ int rename_ref(const char *oldrefname, const char 
*newrefname, const char *logms
 
logmoved = log;
 
-   lock = lock_ref_sha1_basic(newrefname, NULL, 0, NULL);
+   lock = lock_ref_sha1_basic(newrefname, NULL, 0, NULL, NULL, 0);
if (!lock) {
error(unable to lock %s for update, newrefname);
goto rollback;
@@ 

[PATCH v12 41/41] refs.c: make write_ref_sha1 static

2014-05-29 Thread Ronnie Sahlberg
No external users call write_ref_sha1 any more so lets declare it static.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs.c | 6 +-
 refs.h | 3 ---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/refs.c b/refs.c
index 6414afc..aefa5bc 100644
--- a/refs.c
+++ b/refs.c
@@ -2618,6 +2618,9 @@ static int rename_tmp_log(const char *newrefname)
return 0;
 }
 
+static int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1,
+ const char *logmsg);
+
 int rename_ref(const char *oldrefname, const char *newrefname, const char 
*logmsg)
 {
unsigned char sha1[20], orig_sha1[20];
@@ -2848,7 +2851,8 @@ static int is_branch(const char *refname)
return !strcmp(refname, HEAD) || starts_with(refname, refs/heads/);
 }
 
-int write_ref_sha1(struct ref_lock *lock,
+/** Writes sha1 into the ref specified by the lock. **/
+static int write_ref_sha1(struct ref_lock *lock,
const unsigned char *sha1, const char *logmsg)
 {
static char term = '\n';
diff --git a/refs.h b/refs.h
index 88732a1..88020c5 100644
--- a/refs.h
+++ b/refs.h
@@ -195,9 +195,6 @@ extern int commit_ref(struct ref_lock *lock);
 /** Release any lock taken but not written. **/
 extern void unlock_ref(struct ref_lock *lock);
 
-/** Writes sha1 into the ref specified by the lock. **/
-extern int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1, 
const char *msg);
-
 /** Setup reflog before using. **/
 int log_ref_setup(const char *refname, char *logfile, int bufsize);
 
-- 
2.0.0.rc3.474.g3833130

--
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 v12 24/41] receive-pack.c: use a reference transaction for updating the refs

2014-05-29 Thread Ronnie Sahlberg
Wrap all the ref updates inside a transaction.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 builtin/receive-pack.c | 31 ++-
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index c323081..13f4a63 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -46,6 +46,7 @@ static void *head_name_to_free;
 static int sent_capabilities;
 static int shallow_update;
 static const char *alt_shallow_file;
+static struct string_list error_strings = STRING_LIST_INIT_DUP;
 
 static enum deny_action parse_deny_action(const char *var, const char *value)
 {
@@ -475,7 +476,6 @@ static const char *update(struct command *cmd, struct 
shallow_info *si)
const char *namespaced_name;
unsigned char *old_sha1 = cmd-old_sha1;
unsigned char *new_sha1 = cmd-new_sha1;
-   struct ref_lock *lock;
 
/* only refs/... are allowed */
if (!starts_with(name, refs/) || check_refname_format(name + 5, 0)) {
@@ -576,19 +576,31 @@ static const char *update(struct command *cmd, struct 
shallow_info *si)
return NULL; /* good */
}
else {
+   struct strbuf err = STRBUF_INIT;
+   struct ref_transaction *transaction;
+
if (shallow_update  si-shallow_ref[cmd-index] 
update_shallow_ref(cmd, si))
return shallow error;
 
-   lock = lock_any_ref_for_update(namespaced_name, old_sha1,
-  0, NULL);
-   if (!lock) {
-   rp_error(failed to lock %s, name);
-   return failed to lock;
-   }
-   if (write_ref_sha1(lock, new_sha1, push)) {
-   return failed to write; /* error() already called */
+   transaction = ref_transaction_begin(err);
+   if (!transaction ||
+   ref_transaction_update(transaction, namespaced_name,
+  new_sha1, old_sha1, 0, 1, err) ||
+   ref_transaction_commit(transaction, push, err)) {
+
+   const char *str;
+   string_list_append(error_strings, err.buf);
+   str = error_strings.items[error_strings.nr - 1].string;
+   strbuf_release(err);
+
+   ref_transaction_free(transaction);
+   rp_error(%s, str);
+   return str;
}
+
+   ref_transaction_free(transaction);
+   strbuf_release(err);
return NULL; /* good */
}
 }
@@ -1215,5 +1227,6 @@ int cmd_receive_pack(int argc, const char **argv, const 
char *prefix)
packet_flush(1);
sha1_array_clear(shallow);
sha1_array_clear(ref);
+   string_list_clear(error_strings, 0);
return 0;
 }
-- 
2.0.0.rc3.474.g3833130

--
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 v12 39/41] refs.c: propagate any errno==ENOTDIR from _commit back to the callers

2014-05-29 Thread Ronnie Sahlberg
In _commit, ENOTDIR can happen in the call to lock_ref_sha1_basic, either when
we lstat the new refname and it returns ENOTDIR or if the name checking
function reports that the same type of conflict happened. In both cases it
means that we can not create the new ref due to a name conflict.

For these cases, save the errno value and abort and make sure that the caller
can see errno==ENOTDIR.

Also start defining specific return codes for _commit, assign -1 as a generic
error and -2 as the error that refers to a name conflict. Callers can (and
should) use that return value inspecting errno directly.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs.c | 23 ---
 refs.h |  4 
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/refs.c b/refs.c
index ec8d642..6414afc 100644
--- a/refs.c
+++ b/refs.c
@@ -3504,7 +3504,7 @@ static int ref_update_reject_duplicates(struct ref_update 
**updates, int n,
 int ref_transaction_commit(struct ref_transaction *transaction,
   struct strbuf *err)
 {
-   int ret = 0, delnum = 0, i;
+   int ret = 0, delnum = 0, i, save_errno = 0;
const char **delnames;
int n = transaction-nr;
struct ref_update **updates = transaction-updates;
@@ -3522,9 +3522,10 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
 
/* Copy, sort, and reject duplicate refs */
qsort(updates, n, sizeof(*updates), ref_update_compare);
-   ret = ref_update_reject_duplicates(updates, n, err);
-   if (ret)
+   if (ref_update_reject_duplicates(updates, n, err)) {
+   ret = -1;
goto cleanup;
+   }
 
/* Acquire all locks while verifying old values */
for (i = 0; i  n; i++) {
@@ -3538,10 +3539,12 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
   update-type,
   delnames, delnum);
if (!update-lock) {
+   if (errno == ENOTDIR)
+   save_errno = errno;
if (err)
strbuf_addf(err, Cannot lock the ref '%s'.,
update-refname);
-   ret = 1;
+   ret = -1;
goto cleanup;
}
}
@@ -3559,6 +3562,7 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
 
if (err)
strbuf_addf(err, str, update-refname);
+   ret = -1;
goto cleanup;
}
}
@@ -3569,14 +3573,16 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
struct ref_update *update = updates[i];
 
if (update-lock) {
-   ret |= delete_ref_loose(update-lock, update-type,
-   err);
+   if (delete_ref_loose(update-lock, update-type, err))
+   ret = -1;
+
if (!(update-flags  REF_ISPRUNING))
delnames[delnum++] = update-lock-ref_name;
}
}
 
-   ret |= repack_without_refs(delnames, delnum, err);
+   if (repack_without_refs(delnames, delnum, err))
+   ret = -1;
for (i = 0; i  delnum; i++)
unlink_or_warn(git_path(logs/%s, delnames[i]));
clear_loose_ref_cache(ref_cache);
@@ -3589,6 +3595,9 @@ cleanup:
if (updates[i]-lock)
unlock_ref(updates[i]-lock);
free(delnames);
+   errno = save_errno;
+   if (save_errno == ENOTDIR)
+   ret = -2;
return ret;
 }
 
diff --git a/refs.h b/refs.h
index 5c6c20f..88732a1 100644
--- a/refs.h
+++ b/refs.h
@@ -323,6 +323,10 @@ int ref_transaction_delete(struct ref_transaction 
*transaction,
  * Commit all of the changes that have been queued in transaction, as
  * atomically as possible.  Return a nonzero value if there is a
  * problem.
+ * If the transaction is already in failed state this function will return
+ * an error.
+ * Function returns 0 on success, -1 for generic failures and -2 if the
+ * failure was due to a name collision (ENOTDIR).
  */
 int ref_transaction_commit(struct ref_transaction *transaction,
   struct strbuf *err);
-- 
2.0.0.rc3.474.g3833130

--
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 v12 26/41] walker.c: use ref transaction for ref updates

2014-05-29 Thread Ronnie Sahlberg
Switch to using ref transactions in walker_fetch(). As part of the refactoring
to use ref transactions we also fix a potential memory leak where in the
original code if write_ref_sha1() would fail we would end up returning from
the function without free()ing the msg string.

Note that this function is only called when fetching from a remote HTTP
repository onto the local (most of the time single-user) repository which
likely means that the type of collissions that the previous locking would
protect against and cause the fetch to fail for to be even more rare.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 walker.c | 59 +++
 1 file changed, 35 insertions(+), 24 deletions(-)

diff --git a/walker.c b/walker.c
index 1dd86b8..60d9f9e 100644
--- a/walker.c
+++ b/walker.c
@@ -251,39 +251,36 @@ void walker_targets_free(int targets, char **target, 
const char **write_ref)
 int walker_fetch(struct walker *walker, int targets, char **target,
 const char **write_ref, const char *write_ref_log_details)
 {
-   struct ref_lock **lock = xcalloc(targets, sizeof(struct ref_lock *));
+   struct strbuf ref_name = STRBUF_INIT;
+   struct strbuf err = STRBUF_INIT;
+   struct ref_transaction *transaction = NULL;
unsigned char *sha1 = xmalloc(targets * 20);
-   char *msg;
-   int ret;
+   char *msg = NULL;
int i;
 
save_commit_buffer = 0;
 
-   for (i = 0; i  targets; i++) {
-   if (!write_ref || !write_ref[i])
-   continue;
-
-   lock[i] = lock_ref_sha1(write_ref[i], NULL);
-   if (!lock[i]) {
-   error(Can't lock ref %s, write_ref[i]);
-   goto unlock_and_fail;
+   if (write_ref) {
+   transaction = ref_transaction_begin(err);
+   if (!transaction) {
+   error(%s, err.buf);
+   goto rollback_and_fail;
}
}
-
if (!walker-get_recover)
for_each_ref(mark_complete, NULL);
 
for (i = 0; i  targets; i++) {
if (interpret_target(walker, target[i], sha1[20 * i])) {
error(Could not interpret response from server '%s' as 
something to pull, target[i]);
-   goto unlock_and_fail;
+   goto rollback_and_fail;
}
if (process(walker, lookup_unknown_object(sha1[20 * i])))
-   goto unlock_and_fail;
+   goto rollback_and_fail;
}
 
if (loop(walker))
-   goto unlock_and_fail;
+   goto rollback_and_fail;
 
if (write_ref_log_details) {
msg = xmalloc(strlen(write_ref_log_details) + 12);
@@ -294,19 +291,33 @@ int walker_fetch(struct walker *walker, int targets, char 
**target,
for (i = 0; i  targets; i++) {
if (!write_ref || !write_ref[i])
continue;
-   ret = write_ref_sha1(lock[i], sha1[20 * i], msg ? msg : fetch 
(unknown));
-   lock[i] = NULL;
-   if (ret)
-   goto unlock_and_fail;
+   strbuf_reset(ref_name);
+   strbuf_addf(ref_name, refs/%s, write_ref[i]);
+   if (ref_transaction_update(transaction, ref_name.buf,
+  sha1[20 * i], NULL, 0, 0,
+  err)) {
+   error(%s, err.buf);
+   goto rollback_and_fail;
+   }
+   }
+   if (write_ref) {
+   if (ref_transaction_commit(transaction,
+  msg ? msg : fetch (unknown),
+  err)) {
+   error(%s, err.buf);
+   goto rollback_and_fail;
+   }
+   ref_transaction_free(transaction);
}
-   free(msg);
 
+   free(msg);
return 0;
 
-unlock_and_fail:
-   for (i = 0; i  targets; i++)
-   if (lock[i])
-   unlock_ref(lock[i]);
+rollback_and_fail:
+   ref_transaction_free(transaction);
+   free(msg);
+   strbuf_release(err);
+   strbuf_release(ref_name);
 
return -1;
 }
-- 
2.0.0.rc3.474.g3833130

--
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 v12 32/41] refs.c: make delete_ref use a transaction

2014-05-29 Thread Ronnie Sahlberg
Change delete_ref to use a ref transaction for the deletion. At the same time
since we no longer have any callers of repack_without_ref we can now delete
this function.

Change delete_ref to return 0 on success and -1 on failure instead of the
previous 0 on success either 1 or -1 on failure.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs.c | 36 ++--
 1 file changed, 14 insertions(+), 22 deletions(-)

diff --git a/refs.c b/refs.c
index 60593d7..e2c2249 100644
--- a/refs.c
+++ b/refs.c
@@ -2494,11 +2494,6 @@ static int repack_without_refs(const char **refnames, 
int n, struct strbuf *err)
return ret;
 }
 
-static int repack_without_ref(const char *refname)
-{
-   return repack_without_refs(refname, 1, NULL);
-}
-
 static int add_err_if_unremovable(const char *op, const char *file,
  struct strbuf *e, int rc)
 {
@@ -2538,24 +2533,21 @@ static int delete_ref_loose(struct ref_lock *lock, int 
flag, struct strbuf *err)
 
 int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
 {
-   struct ref_lock *lock;
-   int ret = 0, flag = 0;
-
-   lock = lock_ref_sha1_basic(refname, sha1, delopt, flag);
-   if (!lock)
-   return 1;
-   ret |= delete_ref_loose(lock, flag, NULL);
-
-   /* removing the loose one could have resurrected an earlier
-* packed one.  Also, if it was not loose we need to repack
-* without it.
-*/
-   ret |= repack_without_ref(lock-ref_name);
+   struct ref_transaction *transaction;
+   struct strbuf err = STRBUF_INIT;
 
-   unlink_or_warn(git_path(logs/%s, lock-ref_name));
-   clear_loose_ref_cache(ref_cache);
-   unlock_ref(lock);
-   return ret;
+   transaction = ref_transaction_begin(err);
+   if (!transaction ||
+   ref_transaction_delete(transaction, refname, sha1, delopt,
+  sha1  !is_null_sha1(sha1), err) ||
+   ref_transaction_commit(transaction, NULL, err)) {
+   error(%s, err.buf);
+   ref_transaction_free(transaction);
+   strbuf_release(err);
+   return -1;
+   }
+   ref_transaction_free(transaction);
+   return 0;
 }
 
 /*
-- 
2.0.0.rc3.474.g3833130

--
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 v11 25/41] fast-import.c: use a ref transaction when dumping tags

2014-05-29 Thread Ronnie Sahlberg
On Wed, May 28, 2014 at 4:39 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 Ronnie Sahlberg wrote:

 I rely on the fact that if the transaction has failed then it is safe
 to call ref_transaction_commit since it is guaranteed to return an
 error too.

 Yes, I am saying that behavior for ref_transaction_commit is weird.

 Usually when ref_transaction_commit is called I can do

 struct strbuf err = STRBUF_INIT;
 if (ref_transaction_commit(..., err))
 die(%s, err.buf);

 and I know that since ref_transaction_commit has returned a nonzero
 result, err.buf is populated with a sensible message that will
 describe what went wrong.

 That's true even if there's a bug elsewhere in code I didn't write
 (e.g., someone forgot to check the return value from
 ref_transaction_update).

 But the guarantee you are describing removes that property.  It
 creates a case where ref_transaction_commit can return nonzero without
 updating err.  So I get the following message:

 fatal:

 I don't think that's a good outcome.

In this case fatal: can not happen.
This is no more subtle than most of the git core.


I have changed this function to explicitly abort on _update failing
but I think this is making the api too restrictive.



 Sure, if I am well acquainted with the API, I can make sure to use the
 same strbuf for all transaction API calls.  But that would result in
 strange behavior, too: if multiple _update calls fail, then I get
 concatenated messages.

 Okay, I can make sure to do at most one failing _update, before
 calling _commit and printing the error.  But at that point, what is
 the advantage over normal exception handling, where the error gets
 handled at the _update call site?

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


Re: [PATCH v12 00/44] Use ref transactions for all ref updates

2014-05-29 Thread Ronnie Sahlberg
These patches are the remaining patches that need review in this series.
Please review them.

On Thu, May 29, 2014 at 9:07 AM, Ronnie Sahlberg sahlb...@google.com wrote:
 This patch series can also be found at
 https://github.com/rsahlberg/git/tree/ref-transactions


 Ronnie please review these remaining patches in this series.


  Sahlberg (44):
   refs.c: constify the sha arguments for
 ref_transaction_create|delete|update
   refs.c: allow passing NULL to ref_transaction_free
   refs.c: add a strbuf argument to ref_transaction_commit for error
 logging
   refs.c: add an err argument to repack_without_refs
   refs.c: make ref_update_reject_duplicates take a strbuf argument for
 errors
   refs.c: add an err argument to delete_ref_loose
   refs.c: make update_ref_write update a strbuf on failure
   update-ref.c: log transaction error from the update_ref
   refs.c: remove the onerr argument to ref_transaction_commit
   refs.c: change ref_transaction_update() to do error checking and
 return status
   refs.c: change ref_transaction_create to do error checking and return
 status
   refs.c: ref_transaction_delete to check for error and return status
   tag.c: use ref transactions when doing updates
   replace.c: use the ref transaction functions for updates
   commit.c: use ref transactions for updates
   sequencer.c: use ref transactions for all ref updates
   fast-import.c: change update_branch to use ref transactions
   branch.c: use ref transaction for all ref updates
   refs.c: change update_ref to use a transaction
   refs.c: free the transaction before returning when number of updates
 is 0
   refs.c: ref_transaction_commit should not free the transaction
   fetch.c: clear errno before calling functions that might set it
   fetch.c: change s_update_ref to use a ref transaction
   fetch.c: use a single ref transaction for all ref updates
   receive-pack.c: use a reference transaction for updating the refs
   fast-import.c: use a ref transaction when dumping tags
   walker.c: use ref transaction for ref updates
   refs.c: make write_ref_sha1 static
   refs.c: make lock_ref_sha1 static
   refs.c: add transaction.status and track OPEN/CLOSED/ERROR
   refs.c: remove the update_ref_lock function
   refs.c: remove the update_ref_write function
   refs.c: remove lock_ref_sha1
   refs.c: make prune_ref use a transaction to delete the ref
   refs.c: make delete_ref use a transaction
   refs.c: pass the ref log message to _create/delete/update instead of
 _commit
   refs.c: pass NULL as *flags to read_ref_full
   refs.c: pack all refs before we start to rename a ref
   refs.c: move the check for valid refname to lock_ref_sha1_basic
   refs.c: call lock_ref_sha1_basic directly from commit
   refs.c: add a new flag for transaction delete for refs we know are
 packed only
   refs.c: pass a skip list to name_conflict_fn
   refs.c: make rename_ref use a transaction
   refs.c: remove forward declaration of write_ref_sha1

  branch.c   |  30 ++--
  builtin/commit.c   |  24 ++-
  builtin/fetch.c|  29 +--
  builtin/receive-pack.c |  21 +--
  builtin/replace.c  |  15 +-
  builtin/tag.c  |  15 +-
  builtin/update-ref.c   |  32 ++--
  cache.h|   2 +
  fast-import.c  |  42 +++--
  lockfile.c |  21 ++-
  refs.c | 468 
 +
  refs.h |  54 +++---
  sequencer.c|  24 ++-
  t/t3200-branch.sh  |   2 +-
  walker.c   |  51 +++---
  15 files changed, 491 insertions(+), 339 deletions(-)

 --
 2.0.0.rc3.510.g20c254b

--
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 REPORT] Git log pretty date

2014-05-29 Thread Jeff King
On Thu, May 29, 2014 at 05:50:56PM +0700, Duy Nguyen wrote:

 On Thu, May 29, 2014 at 5:29 PM, Rodrigo Fernandes rtfrodr...@gmail.com 
 wrote:
  I get an empty response on the date field, but since pretty has `%ad`
  it should follow the --date and return the date even if wrong.
 
  ...
 
  I tried to check the source code but have no idea where to start,
  maybe if you point me on some direction I can take a look for my self.
 
 pretty.c, format_commit_one() - format_person_part() -
 show_ident_date() - show_date(). The last one is in date.c

show_ident_date() recently learned to always print Jan 1 1970 as a
sentinel for dates that cannot be parsed (or for --date=raw, 0 +).
But I do not think we hit that code path at all here.

It looks like split_ident_line notices the bogus date and returns NULL
for s.date_begin. Then format_person_part skips the placeholder, since
it knows we have nothing worth showing.

I think we probably want to do something like:

diff --git a/pretty.c b/pretty.c
index 3c43db5..7214a2c 100644
--- a/pretty.c
+++ b/pretty.c
@@ -732,14 +732,6 @@ static size_t format_person_part(struct strbuf *sb, char 
part,
return placeholder_len;
}
 
-   if (!s.date_begin)
-   goto skip;
-
-   if (part == 't') {  /* date, UNIX timestamp */
-   strbuf_add(sb, s.date_begin, s.date_end - s.date_begin);
-   return placeholder_len;
-   }
-
switch (part) {
case 'd':   /* date */
strbuf_addstr(sb, show_ident_date(s, dmode));
@@ -753,6 +745,9 @@ static size_t format_person_part(struct strbuf *sb, char 
part,
case 'i':   /* date, ISO 8601 */
strbuf_addstr(sb, show_ident_date(s, DATE_ISO8601));
return placeholder_len;
+   case 't':   /* date, UNIX timestamp */
+   strbuf_addstr(sb, show_ident_date(s, DATE_RAW));
+   return placeholder_len;
}
 
 skip:

to at least make --format date output consistent with the rest of git
(and to make %at consistent with %ad and --date=raw). That still
doesn't address Rodrigo's concern, though (we would print 0 +).

For that, we would want on top:

  1. Teach split_ident_line to return _something_ for his malformed
 case.

  2. Teach show_ident_line to treat DATE_RAW to truly output raw
 content, even if it is malformed.

I am not sure whether those are a good idea or not. The current code
provides some level of sanitization, so that a parser of log output will
not get malformed cruft. And DATE_RAW may mean show me what is in the
raw commit, even if it is broken. Or it may just mean show me the date
in unix timestamp format, because that is easy to parse.

I'd argue that if somebody really wants the former, they should be using
--pretty=raw, which will show the raw commit headers, without any
parsing or fixup at all.

-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] check_refname_component: Optimize

2014-05-29 Thread Junio C Hamano
Duy Nguyen pclo...@gmail.com writes:

 On Thu, May 29, 2014 at 6:49 AM, David Turner dtur...@twopensource.com 
 wrote:
 I assume that most of the time spent in check_refname_component() is
 while reading the packed-refs file, right?

 Yes.

 I wonder if we can get away without SSE code by saving stat info of
 the packed-refs version that we have verified. When we read pack-refs,
 if stat info matches, skip check_refname_component(). Assuming that
 pack-refs does not change often, of course.

Can you elaborate a bit more?

Regardless, I think I would prefer to see this patch done as at
least a two step series, one that does only the look-up table thing,
and then the other with arch-specific tweaks as a follow-up on top.
--
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 v11 25/41] fast-import.c: use a ref transaction when dumping tags

2014-05-29 Thread Jonathan Nieder
Ronnie Sahlberg wrote:
 On Wed, May 28, 2014 at 4:39 PM, Jonathan Nieder jrnie...@gmail.com wrote:

 Usually when ref_transaction_commit is called I can do

 struct strbuf err = STRBUF_INIT;
 if (ref_transaction_commit(..., err))
 die(%s, err.buf);

 and I know that since ref_transaction_commit has returned a nonzero
 result, err.buf is populated with a sensible message that will
 describe what went wrong.
[...]
 But the guarantee you are describing removes that property.  It
 creates a case where ref_transaction_commit can return nonzero without
 updating err.  So I get the following message:

 fatal:

 I don't think that's a good outcome.

 In this case fatal: can not happen.
 This is no more subtle than most of the git core.

 I have changed this function to explicitly abort on _update failing
 but I think this is making the api too restrictive.

I don't want to push you toward making a change you think is wrong.  I
certainly don't own the codebase, and there are lots of other people
(e.g., Michael, Junio, Jeff) to get advice from.  So I guess I should
try to address this.

I'm not quite sure what you mean by too restrictive.

 a. Having API constraints that aren't enforced by the function makes
using the API too fussy.

I agree with that.  That was something I liked about keeping track
of the OPEN/CLOSED state of a transaction, which would let
functions like _commit die() if someone is misusing the API so the
problem gets detected early.

 b. Having to check the return value from _update() is too fussy.
 
It certainly seems *possible* to have an API that doesn't require
checking the return value, while still avoiding the usability
problem I described in the quoted message above.  For example:

 * _update() returns void and has no strbuf parameter
 * error handling happens by checking the error from _commit()

That would score well on the scale described at
http://ozlabs.org/~rusty/index.cgi/tech/2008-03-30.html

An API where checking the return value is optional would be
doable, too.  For example:

 * _update() returns int and has a strbuf parameter
 * if the strbuf parameter is NULL, the caller is expected to
   wait for _commit() to check for errors, and a relevant
   message will be passed back then
 * if the strbuf parameter is non-NULL, then calling _commit()
   after an error is an API violation

I don't understand the comment about no more subtle than most of git.
Are you talking about the errno action at a distance you found in some
functions?  I thought we agreed that those were mistakes that accrue
when people aim for a quick fix without thinking about maintainability
and something git should have less of.

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


Re: [PATCH] sideband.c: Get rid of ANSI sequences for non-terminal shell

2014-05-29 Thread Jeff King
On Wed, May 28, 2014 at 03:12:15AM +, Naumov, Michael (North Sydney) wrote:

 Some git tools such as GitExtensions for Windows use environment
 variable TERM=msys which causes the weird ANSI sequence shown for the
 messages returned from server-side hooks
 We add those ANSI sequences to help format sideband data on the user's
 terminal. However, GitExtensions is not using a terminal, and the ANSI
 sequences just confuses it. We can recognize this use by checking
 isatty().
 See https://github.com/gitextensions/gitextensions/issues/1313 for
 more details

Please wrap your commit messages (and emails) at something reasonable to
fit on an 80-char terminal with indentation (I usually use 60, but
anything = 72 is probably fine).

Other than that, I think the patch looks fine.

 P.S. I gave up trying to send this letter from gmail, it eats my tab character
 P.S 2. Sorry, my tab character has been eaten again!

This version looks OK to me (and applies via git-am).

-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 v12 06/41] refs.c: add an err argument to repack_without_refs

2014-05-29 Thread Jonathan Nieder
Hi,

Ronnie Sahlberg wrote:

 Update repack_without_refs to take an err argument and update it if there
 is a failure. Pass the err variable from ref_transaction_commit to this
 function so that callers can print a meaningful error message if _commit
 fails due to a problem in repack_without_refs.

 Add a new function unable_to_lock_message that takes a strbuf argument and
 fills in the reason for the failure.

 In commit_packed_refs, make sure that we propagate any errno that
 commit_lock_file might have set back to our caller.

 Signed-off-by: Ronnie Sahlberg sahlb...@google.com
 ---
  cache.h|  2 ++
  lockfile.c | 21 -
  refs.c | 25 +++--
  3 files changed, 33 insertions(+), 15 deletions(-)

I don't want to sound like a broken record, but this still has the
same issues I described before at
http://thread.gmane.org/gmane.comp.version-control.git/250197/focus=250309.

The commit message or documentation or notes after the three dashes
above could explain what I missed when making my suggestions.
Otherwise I get no reality check as a reviewer, other reviewers get
less insight into what's happening in the patch, people in the future
looking into the patch don't understand its design as well, etc.

As a general rule, that is a good practice anyway --- even when a
reviewer was confused, what they got confused about can be an
indication of where to make the code or design documentation (commit
message) more clear, and when reposting a patch it can be a good
opportunity to explain how the patch evolved.

What would be wrong with the line of API documentation and the TODO
comment for a known bug I suggested?  If they are a bad idea, can you
explain that so I can learn from it?  Or if they were confusing, would
you like a patch that explains what I mean?

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: .gitmodules containing SSH clone URLs should fall back to HTTPS when SSH key is not valid/existent

2014-05-29 Thread Junio C Hamano
Jens Lehmann jens.lehm...@web.de writes:

 Am 29.05.2014 04:07, schrieb Jonathan Leonard:
 The title pretty much says it all.

 But you do not give much information about your special use
 case.

Perhaps git grep insteadOf Documentation/ is all that is needed?
--
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: [ANNOUNCE] Git v2.0.0

2014-05-29 Thread Jeff King
On Wed, May 28, 2014 at 06:17:25PM -0500, Felipe Contreras wrote:

 This is the last mail I sent to you, because you ignore them anyway, and
 remove them from the mailing list.
 [...]
 [2], a mail you conveniently removed from the tracked record.
 [...]
 You also conveniently removed this mail from the archives.

I see you already noticed the changes in v2.0, but I wanted to address
these points, because I consider silent censorship to be a serious
accusation.

I do not think Junio or anyone else has the technical ability to remove
messages from the archive. There is not one archive, but rather several
that get messages straight from vger.kernel.org and keep their own
database (e.g., gmane, marc, spinics, nabble). E.g., here is the
deleted message on nabble:

  
http://git.661346.n2.nabble.com/PATCH-remote-helpers-point-at-their-upstream-repositories-td7610799i20.html#a7611324

Here it is on gmane:

  http://permalink.gmane.org/gmane.comp.version-control.git/249741

However, I had to pull that link from the NNTP interface. If you look at
the non-threaded web interface for gmane here:

  http://blog.gmane.org/gmane.comp.version-control.git/day=20140520

and here:

  http://blog.gmane.org/gmane.comp.version-control.git/day=20140521

you will see that there is a huge gap in the list coverage from about
midnight on the 20th until the 24th (but these messages are available
via nntp). So this seems much more like a gmane bug than anything else.

I've reported the bug to gmane.discuss (no link yet, as I'm waiting for
the message to go through, but it is not a high traffic group, so it
should be easy to find the thread once it is there).

-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: [BUG REPORT] Git log pretty date

2014-05-29 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 ...
 to at least make --format date output consistent with the rest of git
 (and to make %at consistent with %ad and --date=raw). That still
 doesn't address Rodrigo's concern, though (we would print 0 +).

 For that, we would want on top:

   1. Teach split_ident_line to return _something_ for his malformed
  case.

   2. Teach show_ident_line to treat DATE_RAW to truly output raw
  content, even if it is malformed.

 I am not sure whether those are a good idea or not. The current code
 provides some level of sanitization, so that a parser of log output will
 not get malformed cruft. And DATE_RAW may mean show me what is in the
 raw commit, even if it is broken. Or it may just mean show me the date
 in unix timestamp format, because that is easy to parse.

 I'd argue that if somebody really wants the former, they should be using
 --pretty=raw, which will show the raw commit headers, without any
 parsing or fixup at all.

I actually am not very much interested in deciding what to show for
a broken timestamp.  An empty string is just as good as any random
cruft.  I agree with you that it would be nice to have one escape
hatch to let the users see what garbage is recorded, if only for
diagnostic purposes, and DATE_RAW may be one good way to do so (but
I'd rather recommend cat-file commit for real diagnostics).

I would be more interested to see whatever broken tool that created
such a commit gets fixed.  Do we know where it came from?
--
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/10] t9904: new __git_ps1 tests for Zsh

2014-05-29 Thread Thomas Rast
Richard Hansen rhan...@bbn.com writes:

 These are the same tests as in t9903, but run in zsh instead of bash.

 Signed-off-by: Richard Hansen rhan...@bbn.com
 ---
  t/lib-zsh.sh  | 30 ++
  t/t9904-zsh-prompt.sh | 10 ++
  2 files changed, 40 insertions(+)
  create mode 100644 t/lib-zsh.sh
  create mode 100755 t/t9904-zsh-prompt.sh

This doesn't appear to work in valgrind mode:

$ ./t9904-zsh-prompt.sh --valgrind
error: Test script did not set test_description.

t9903 however works.  I'm not sure how much of a difference it makes,
but: I use bash as my shell and as /bin/sh, but I do have zsh installed.

Can you look into it?

-- 
Thomas Rast
t...@thomasrast.ch
--
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 chokes on large file

2014-05-29 Thread Dale R. Worley
 From: David Lang da...@lang.hm

 well, as others noted, the problem is actually caused by doing the diffs, and 
 that is something that is a very common thing to do with source code.

To some degree, my attitude comes from When I Was A Boy, when you got
16k for both your bytecode and your data, so you never kept more than
one line of a file in memory unless you had to.

Regardless of that, the problem with git fsck is not due to doing
diffs, and git commit by default does diffs even if you don't ask
for them, so the observed problems cannot be subsumed under the label
you asked for a diff of a file that can't be diffed.

 And I would assume that files of several MB would be able to fit in
 memory (again, this was assumed to be for development, and compilers
 take a lot of ram to run, so having enough ram to hold any
 individual source file while the compiler is _not_ using ram doesn't
 seem likely to be a problem)

At least the first versions of GCC only kept one function (and the
global symbol table) in memory at once, so you could compile a source
file that was larger than the available memory.

In any case, if Git is no longer limited to handling source files, it
needs to be updated so it can handle large files.

Dale
--
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: [ANNOUNCE] Git v2.0.0

2014-05-29 Thread David Kastrup
Jeff King p...@peff.net writes:

 On Wed, May 28, 2014 at 06:17:25PM -0500, Felipe Contreras wrote:

 This is the last mail I sent to you, because you ignore them anyway, and
 remove them from the mailing list.
 [...]
 [2], a mail you conveniently removed from the tracked record.
 [...]
 You also conveniently removed this mail from the archives.

 I see you already noticed the changes in v2.0, but I wanted to address
 these points, because I consider silent censorship to be a serious
 accusation.

 I do not think Junio or anyone else has the technical ability to remove
 messages from the archive.

You can post self-destructing messages by adding X-no-archive: yes if I
am not mistaken.  But that only concerns stuff you post yourself.

 There is not one archive, but rather several that get messages
 straight from vger.kernel.org and keep their own database (e.g.,
 gmane, marc, spinics, nabble).

Frankly, I find it weird that vger.kernel.org does not have an archive
of its own.  But yes, that makes it unlikely that silent censorship is
much of a thing.  A list moderator may put a particular sender on silent
moderation, where postings will only appear once he has acknowledged
them.  However, that is a manual and burdensome process and requires an
up-front decision to do so.  Once a message made it to the list, the
various archives will pick it up.

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


Reminder Notice

2014-05-29 Thread Jennifer Heldy Gerald



- Original Message -
From: Jennifer H Gerald

This Email is to notify you that your Email address have Won you an Award
Sum of € 1.500,000.00 (1.5 Million Euros) in an E-mail balloting program
Held in Europe.

Please contact the claim Officer with your winning info Ref Num(NL
030126)Batch Num(EU-0840113NL)Ticket Num(04,558,130) Name; Mr Nelson
Paul,Email: agentnp...@aol.co.uk

Sincerely yours,
Jennifer Heldy Gerald,
Public Relation Officer.

--
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: [ANNOUNCE] Git v2.0.0

2014-05-29 Thread Jeff King
On Thu, May 29, 2014 at 09:23:06PM +0200, David Kastrup wrote:

  I do not think Junio or anyone else has the technical ability to remove
  messages from the archive.
 
 You can post self-destructing messages by adding X-no-archive: yes if I
 am not mistaken.  But that only concerns stuff you post yourself.

Right, I was thinking specifically of deleting somebody else's message.
The other obvious choke point is to convince vger to silently drop
messages from somebody, but:

  1. That is different than deleting already-posted messages.

  2. vger is maintained by kernel.org folks, none of whom is a
 regular on the git list. So it would involve some collusion with
 those admins.

That is pretty clearly not what happened here (the messages did go out
to the list).

 Frankly, I find it weird that vger.kernel.org does not have an archive
 of its own.

I don't think there is much need, as gmane is pretty featureful (and if
you disagree, there are other competitors, or you can set up your own).
I assume the current situation grew organically (somebody wanted an
archive, so they set it up, and then vger saw no need to compete).

-Peff

PS My gmane bug report posted here:

 http://article.gmane.org/gmane.discuss/16175
--
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 REPORT] Git log pretty date

2014-05-29 Thread Jeff King
On Thu, May 29, 2014 at 11:57:15AM -0700, Junio C Hamano wrote:

 Jeff King p...@peff.net writes:
 
  ...
  to at least make --format date output consistent with the rest of git
  (and to make %at consistent with %ad and --date=raw). That still
  doesn't address Rodrigo's concern, though (we would print 0 +).
 [...]
 
 I actually am not very much interested in deciding what to show for
 a broken timestamp.  An empty string is just as good as any random
 cruft.

I was thinking specifically of the first part I quoted above. We are not
consistent between various methods of parsing/printing the date. That
may fall into the if were doing it from scratch... category, though;
it's possible that people currently using --format=%ad prefer and
expect the empty string to denote a bogus value. I'm OK with leaving it.

 I agree with you that it would be nice to have one escape
 hatch to let the users see what garbage is recorded, if only for
 diagnostic purposes, and DATE_RAW may be one good way to do so (but
 I'd rather recommend cat-file commit for real diagnostics).

Yeah, in case I wasn't clear, I don't actually like DATE_RAW as a way to
do that. I'd prefer --pretty=raw or cat-file commit, which already
work.

 I would be more interested to see whatever broken tool that created
 such a commit gets fixed.  Do we know where it came from?

I don't think it has been said yet in the thread. Rodrigo?

-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: [BUG REPORT] Git log pretty date

2014-05-29 Thread Rodrigo Fernandes
Jeff,
I have no idea what was the tool. The repo is not mine. I found the
problem when I was doing some tests and the commit parsing was failing
on that repo.

Cumprimentos,
Rodrigo Fernandes


On Thu, May 29, 2014 at 8:49 PM, Jeff King p...@peff.net wrote:
 On Thu, May 29, 2014 at 11:57:15AM -0700, Junio C Hamano wrote:

 Jeff King p...@peff.net writes:

  ...
  to at least make --format date output consistent with the rest of git
  (and to make %at consistent with %ad and --date=raw). That still
  doesn't address Rodrigo's concern, though (we would print 0 +).
 [...]

 I actually am not very much interested in deciding what to show for
 a broken timestamp.  An empty string is just as good as any random
 cruft.

 I was thinking specifically of the first part I quoted above. We are not
 consistent between various methods of parsing/printing the date. That
 may fall into the if were doing it from scratch... category, though;
 it's possible that people currently using --format=%ad prefer and
 expect the empty string to denote a bogus value. I'm OK with leaving it.

 I agree with you that it would be nice to have one escape
 hatch to let the users see what garbage is recorded, if only for
 diagnostic purposes, and DATE_RAW may be one good way to do so (but
 I'd rather recommend cat-file commit for real diagnostics).

 Yeah, in case I wasn't clear, I don't actually like DATE_RAW as a way to
 do that. I'd prefer --pretty=raw or cat-file commit, which already
 work.

 I would be more interested to see whatever broken tool that created
 such a commit gets fixed.  Do we know where it came from?

 I don't think it has been said yet in the thread. Rodrigo?

 -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] Improve function dir.c:trim_trailing_spaces()

2014-05-29 Thread Jeff King
On Wed, May 28, 2014 at 04:45:57PM -0700, Pasha Bolokhov wrote:

 Move backwards from the end of the string (more efficient for
 lines which do not have trailing spaces or have just a couple).

The original code reads the string from left to right. In theory, that
means we could get away with not calling strlen() at all, over a
right-to-left that must start with a call to strlen().

That being said, I think we already have the length in the calling
function, so you could probably avoid the strlen() altogether, which
makes a right-to-left function more efficient.

However, I doubt it makes that much of a difference in practice, so
unless it's measurable, I would certainly go with the version that is
more readable (and correct, of course).

 Slightly more rare occurrences of 'text  \' with a backslash
 in between spaces are handled correctly.

Can you add a test for this?

Also, if you are refactoring this function, I think it makes sense to
check that:

  foo\\ 

and

  foo\\\ 

match foo\ and foo\ , respectively (I think they do with your patch,
but it is a tricky case that is not immediately obvious).

-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] Improve function dir.c:trim_trailing_spaces()

2014-05-29 Thread Pasha Bolokhov
On Thu, May 29, 2014 at 1:13 PM, Jeff King p...@peff.net wrote:
 On Wed, May 28, 2014 at 04:45:57PM -0700, Pasha Bolokhov wrote:

 Move backwards from the end of the string (more efficient for
 lines which do not have trailing spaces or have just a couple).

 The original code reads the string from left to right. In theory, that
 means we could get away with not calling strlen() at all, over a
 right-to-left that must start with a call to strlen().

 That being said, I think we already have the length in the calling
 function, so you could probably avoid the strlen() altogether, which
 makes a right-to-left function more efficient.

 However, I doubt it makes that much of a difference in practice, so
 unless it's measurable, I would certainly go with the version that is
 more readable (and correct, of course).

Sorry, just to recap, you would go with the existing version
(which needs correction), or with the one that is being suggested? (I
agree I can format the style a tiny bit better in the latter one)

 Tests should not be a big problem, although it's kind of clumsy
to test an internal function which does not really give any output
(you can only measure the outcome). Just again to stress, I have
tested both implementation extensively and the suggested new
implementation gives the correct answers for all your examples below
and all others. If I show this with explicit t/ tests, will it
suffice then?

Basically what I suggest is

-- either: improve the existing function such that it does correctly
that text  \case, and also does not use 'strlen' since it anyway
moves left to right

-- or: use the new suggested implementation (and just brush the
formatting a bit), and perhaps borrow 'len' from the calling routine

And add tests in any case. What is the preference?



 Slightly more rare occurrences of 'text  \' with a backslash
 in between spaces are handled correctly.

 Can you add a test for this?

 Also, if you are refactoring this function, I think it makes sense to
 check that:

   foo\\ 

 and

   foo\\\ 

 match foo\ and foo\ , respectively (I think they do with your patch,
 but it is a tricky case that is not immediately obvious).

 -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 11/10] fixup! t9904: new __git_ps1 tests for Zsh

2014-05-29 Thread Richard Hansen
Signed-off-by: Richard Hansen rhan...@bbn.com
---

On 2014-05-29 15:02, Thomas Rast wrote:
 Richard Hansen rhan...@bbn.com writes:

 These are the same tests as in t9903, but run in zsh instead of bash.

 Signed-off-by: Richard Hansen rhan...@bbn.com
 ---
  t/lib-zsh.sh  | 30 ++
  t/t9904-zsh-prompt.sh | 10 ++
  2 files changed, 40 insertions(+)
  create mode 100644 t/lib-zsh.sh
  create mode 100755 t/t9904-zsh-prompt.sh

 This doesn't appear to work in valgrind mode:

 $ ./t9904-zsh-prompt.sh --valgrind
 error: Test script did not set test_description.

 t9903 however works.  I'm not sure how much of a difference it makes,
 but: I use bash as my shell and as /bin/sh, but I do have zsh installed.

 Can you look into it?

*sigh* By default, Zsh munges $0 whenever a function is called or a
file is sourced, with no (immediately obvious) way to get the original
value of $0.  This fixup causes that feature to be temporarily turned
off so that test-lib.sh does the right thing when it execs $0.

Thank you for finding this bug!

-Richard


 t/lib-zsh.sh | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/t/lib-zsh.sh b/t/lib-zsh.sh
index fa6fcd9..ab4bef2 100644
--- a/t/lib-zsh.sh
+++ b/t/lib-zsh.sh
@@ -2,17 +2,23 @@
 # run under Zsh; primarily intended for tests of the git-prompt.sh
 # script.
 
-if test -n $ZSH_VERSION  test -z $POSIXLY_CORRECT; then
+if test -n $ZSH_VERSION  test -z $POSIXLY_CORRECT  [[ ! -o 
FUNCTION_ARGZERO ]]; then
true
 elif command -v zsh /dev/null 21; then
unset POSIXLY_CORRECT
-   exec zsh $0 $@
+   # Run Zsh with the FUNCTION_ARGZERO option disabled so that
+   # test-lib.sh sees the test script pathname when it examines
+   # $0 instead of ./lib-zsh.sh.  (This works around a Zsh bug;
+   # 'emulate sh -c' should temporarily restore $0 to the POSIX
+   # specification for $0, but it doesn't.)
+   exec zsh +o FUNCTION_ARGZERO $0 $@
 else
echo '1..0 #SKIP skipping Zsh-specific tests; zsh not available'
exit 0
 fi
 
-# ensure that we are in full-on Zsh mode
+# ensure that we are in full-on Zsh mode.  note: this re-enables the
+# FUNCTION_ARGZERO option
 emulate -R zsh || exit 1
 
 shellname=Zsh
@@ -27,4 +33,7 @@ set_ps1_format_vars () {
c_clear='%%f'
 }
 
+# note: although the FUNCTION_ARGZERO option is currently enabled, sh
+# emulation mode temporarily turns it off ($0 is left alone when
+# sourcing test-lib.sh)
 emulate sh -c '. ./test-lib.sh'
-- 
2.0.0

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


[RFC PATCH] git log: support auto decorations

2014-05-29 Thread Linus Torvalds

From: Linus Torvalds torva...@linux-foundation.org
Date: Thu, 29 May 2014 15:19:40 -0700
Subject: [RFC PATCH] git log: support auto decorations

This works kind of like --color=auto - add decorations for interactive
use, but do not change defaults when scripting or when piping the output
to anything but a terminal.

You can use either

[log]
 decorate=auto

in the git config files, or the --decorate=auto command line option to
choose this behavior.

Signed-off-by: Linus Torvalds torva...@linux-foundation.org
---

I actually like seeing decorations by default, but I do *not* think our 
current log.decorate options make sense, since they will change any 
random use of git log to have decorations. I much prefer the 
ui.color=auto behavior that we have for coloration. This is a trivial 
patch that tries to approximate that.

It's marked with RFC because

 (a) that isatty(1) || pager_in_use() test is kind of hacky, maybe we 
 would be better off sharing something with the auto-coloration?

 (b) I also think it would be nice to have the equivalent for 
 --show-signature, but there we don't have any preexisting config 
 file option.

 (c) maybe somebody would like a way to combine auto and full, 
 although personally that doesn't seem to strike me as all that useful 
 (would you really want to see the full refname when not scripting it)

but the patch is certainly simple and seems to work. Comments?

 builtin/log.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/builtin/log.c b/builtin/log.c
index 39e883635279..df6396c9c3d9 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -63,6 +63,8 @@ static int parse_decoration_style(const char *var, const char 
*value)
return DECORATE_FULL_REFS;
else if (!strcmp(value, short))
return DECORATE_SHORT_REFS;
+   else if (!strcmp(value, auto))
+   return (isatty(1) || pager_in_use()) ? DECORATE_SHORT_REFS : 0;
return -1;
 }
 
-- 
2.0.0.1.g5beb60c

--
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 1/5] fetch doc: update introductory part for clarity

2014-05-29 Thread Junio C Hamano
 - Branches is a more common way to say heads in these days.

 - Remote-tracking branches are used a lot more these days and it is
   worth mentioning that it is one of the primary side effects of
   the command to update them.

 - Avoid X. That means Y.  If Y is easier to understand to
   readers, just say that upfront.

 - Use of explicit refspec to fetch tags does not have much to do
   with turning auto following on or off.  It is a way to fetch
   tags that otherwise would not be fetched by auto-following.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 Documentation/git-fetch.txt | 29 -
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
index 5809aa4..d5f5b54 100644
--- a/Documentation/git-fetch.txt
+++ b/Documentation/git-fetch.txt
@@ -17,20 +17,23 @@ SYNOPSIS
 
 DESCRIPTION
 ---
-Fetches named heads or tags from one or more other repositories,
-along with the objects necessary to complete them.
-
-The ref names and their object names of fetched refs are stored
-in `.git/FETCH_HEAD`.  This information is left for a later merge
-operation done by 'git merge'.
-
-By default, tags are auto-followed.  This means that when fetching
-from a remote, any tags on the remote that point to objects that exist
-in the local repository are fetched.  The effect is to fetch tags that
+Fetch branches and/or tags (collectively, refs) from one or more
+other repositories, along with the objects necessary to complete the
+histories of them.
+
+The names of refs that are fetched, together with the object names
+they point at, are written to `.git/FETCH_HEAD`.  This information
+is used by a later merge operation done by 'git merge'.  In addition,
+the remote-tracking branches may be updated (see description on
+refspec below for details).
+
+By default, any tag that points into the histories being fetched is
+also fetched; the effect is to fetch tags that
 point at branches that you are interested in.  This default behavior
-can be changed by using the --tags or --no-tags options, by
-configuring remote.name.tagopt, or by using a refspec that fetches
-tags explicitly.
+can be changed by using the --tags or --no-tags options or by
+configuring remote.name.tagopt.  By using a refspec that fetches tags
+explicitly, you can fetch tags that do not point into branches you
+are interested in as well.
 
 'git fetch' can fetch from either a single named repository,
 or from several repositories at once if group is given and
-- 
2.0.0-479-g59ac8f9

--
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 2/5] fetch doc: update note on '+' in front of the refspec

2014-05-29 Thread Junio C Hamano
While it is not *wrong* per-se to say that pulling a rewound/rebased
branch will lead to an unnecessary merge conflict, that is not what
the leading + sign to allow non-fast-forward update of remote-tracking
branch is at all.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 Documentation/pull-fetch-param.txt | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/Documentation/pull-fetch-param.txt 
b/Documentation/pull-fetch-param.txt
index 18cffc2..2a7e2b7 100644
--- a/Documentation/pull-fetch-param.txt
+++ b/Documentation/pull-fetch-param.txt
@@ -24,15 +24,15 @@ is updated even if it does not result in a fast-forward
 update.
 +
 [NOTE]
-If the remote branch from which you want to pull is
-modified in non-linear ways such as being rewound and
-rebased frequently, then a pull will attempt a merge with
-an older version of itself, likely conflict, and fail.
-It is under these conditions that you would want to use
-the `+` sign to indicate non-fast-forward updates will
-be needed.  There is currently no easy way to determine
-or declare that a branch will be made available in a
-repository with this behavior; the pulling user simply
+When the remote branch you want to fetch is known to
+be rewound and rebased regularly, it is expected that
+the tip of it will not be descendant of the commit that
+used to be at its tip the last time you fetched it and
+stored in your remote-tracking branch.  You would want
+to use the `+` sign to indicate non-fast-forward updates
+will be needed for such branches.  There is no way to
+determine or declare that a branch will be made available
+in a repository with this behavior; the pulling user simply
 must know this is the expected usage pattern for a branch.
 +
 [NOTE]
-- 
2.0.0-479-g59ac8f9

--
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 4/5] fetch doc: on pulling multiple refspecs

2014-05-29 Thread Junio C Hamano
Replace desription of old-style Pull: lines in remotes/
configuration with modern remote.*.fetch variables.

As this note applies only to git pull, enable it only
in git-pull manual page.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 Documentation/pull-fetch-param.txt | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/Documentation/pull-fetch-param.txt 
b/Documentation/pull-fetch-param.txt
index e266c2d..ea4c5a6 100644
--- a/Documentation/pull-fetch-param.txt
+++ b/Documentation/pull-fetch-param.txt
@@ -34,22 +34,26 @@ will be needed for such branches.  There is no way to
 determine or declare that a branch will be made available
 in a repository with this behavior; the pulling user simply
 must know this is the expected usage pattern for a branch.
+ifdef::git-pull[]
 +
 [NOTE]
 There is a difference between listing multiple refspec
 directly on 'git pull' command line and having multiple
-`Pull:` refspec lines for a repository and running
+`remote.repository.fetch` entries in your configuration
+for a repository and running
 'git pull' command without any explicit refspec parameters.
 refspec listed explicitly on the command line are always
 merged into the current branch after fetching.  In other words,
 if you list more than one remote refs, you would be making
-an Octopus.  While 'git pull' run without any explicit refspec
-parameter takes default refspecs from `Pull:` lines, it
+an Octopus merge. On the other hand, 'git pull' that is run
+without any explicit refspec parameter takes default
+refspecs from `remote.repository.fetch` configuration, it
 merges only the first refspec found into the current branch,
-after fetching all the remote refs.  This is because making an
+after fetching all the remote refs specified.  This is because making an
 Octopus from remote refs is rarely done, while keeping track
 of multiple remote heads in one-go by fetching more than one
 is often useful.
+endif::git-pull[]
 +
 Some short-cut notations are also supported.
 +
-- 
2.0.0-479-g59ac8f9

--
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 5/5] fetch doc: update refspec format description

2014-05-29 Thread Junio C Hamano
The text made it sound as if the leading plus is the only thing that
is optional, and forgot that lhs is the same as lhs:, i.e. fetch
it and do not store anywhere.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 Documentation/pull-fetch-param.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/pull-fetch-param.txt 
b/Documentation/pull-fetch-param.txt
index ea4c5a6..27cfd5c 100644
--- a/Documentation/pull-fetch-param.txt
+++ b/Documentation/pull-fetch-param.txt
@@ -15,6 +15,7 @@ endif::git-pull[]
The format of a refspec parameter is an optional plus
`+`, followed by the source ref src, followed
by a colon `:`, followed by the destination ref dst.
+   The colon can be omitted when dst is empty.
 +
 The remote ref that matches src
 is fetched, and if dst is not empty string, the local
-- 
2.0.0-479-g59ac8f9

--
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 0/5] Documentation updates for 'git fetch'

2014-05-29 Thread Junio C Hamano
Noticed that this page has antiquated description, which may still
be correct, that can use some modernization.

There are a few more larger updates coming, but these are small
enough to be reviewed separately and quickly, so I am sending them
out early.

Junio C Hamano (5):
  fetch doc: update introductory part for clarity
  fetch doc: update note on '+' in front of the refspec
  fetch doc: remove notes on outdated mixed layout
  fetch doc: on pulling multiple refspecs
  fetch doc: update refspec format description

 Documentation/git-fetch.txt| 29 ++---
 Documentation/pull-fetch-param.txt | 44 --
 2 files changed, 34 insertions(+), 39 deletions(-)

-- 
2.0.0-479-g59ac8f9

--
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 3/5] fetch doc: remove notes on outdated mixed layout

2014-05-29 Thread Junio C Hamano
In old days before Git 1.5, it was customery for git fetch to use
the same local branch namespace to keep track of the remote-tracking
branches, and it was necessary to tell users not to check them out
and commit on them.  Since everybody uses the separate remote layout
these days, there is no need to warn against the practice to check
out the right-hand side of refspec and build on it---the RHS is
typically not even a local branch.

Incidentally, this also kills one mention of Pull: line of
$GIT_DIR/remotes/* configuration, which is a lot less familiar to
new people than the more modern remote.*.fetch configuration
variable.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 Documentation/pull-fetch-param.txt | 13 -
 1 file changed, 13 deletions(-)

diff --git a/Documentation/pull-fetch-param.txt 
b/Documentation/pull-fetch-param.txt
index 2a7e2b7..e266c2d 100644
--- a/Documentation/pull-fetch-param.txt
+++ b/Documentation/pull-fetch-param.txt
@@ -36,19 +36,6 @@ in a repository with this behavior; the pulling user simply
 must know this is the expected usage pattern for a branch.
 +
 [NOTE]
-You never do your own development on branches that appear
-on the right hand side of a refspec colon on `Pull:` lines;
-they are to be updated by 'git fetch'.  If you intend to do
-development derived from a remote branch `B`, have a `Pull:`
-line to track it (i.e. `Pull: B:remote-B`), and have a separate
-branch `my-B` to do your development on top of it.  The latter
-is created by `git branch my-B remote-B` (or its equivalent `git
-checkout -b my-B remote-B`).  Run `git fetch` to keep track of
-the progress of the remote side, and when you see something new
-on the remote branch, merge it into your development branch with
-`git pull . remote-B`, while you are on `my-B` branch.
-+
-[NOTE]
 There is a difference between listing multiple refspec
 directly on 'git pull' command line and having multiple
 `Pull:` refspec lines for a repository and running
-- 
2.0.0-479-g59ac8f9

--
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: .gitmodules containing SSH clone URLs should fall back to HTTPS when SSH key is not valid/existent

2014-05-29 Thread Jonathan Leonard
 But you do not give much information about your special use
 case. I assume you have submodule repositories for which some
 developers have a valid ssh key and others don't (maybe
 because they should only have read access via https)?


Precisely. Specifically this is for a collection (17 or more) of
GitHub-hosted projects which are maintained by only a couple of people
(who have the ability to directly push via git:// or ssh://).
Everyone else (including deployments and ordinary users) who clones
the repo should be able to just grab the code via HTTPS and have it
work.

 If that is the case you might want to look into access control
 tools like gitolite.


We are using GitHub.

  Lack of this feature (or presence
 of this bug [depending on your perspective]) is a major PITA.

 But why is https special? Why not fall back to the git
 protocol? Or http? (And no: I'm not serious here ;-)


HTTPS isn't special except in that it is the least privileged
transport type (and thus should be the last resort). Whether to
fallback to git:// from ssh:// or vice versa is inconsequential to
this request.

 After the first failed clone of the submodule at via SSH the
 developer could also just do a

git config submodule.name.url https://host/repo

 and override the URL from .gitmodules.


Yes, this would work. But it would be a painful manual step which we
would not want to force on ordinary users (and would not want to
experience ourselves either).

It should be noted that this is only really a problem as the other
options GitHub gives us are also equally (or more) painful:
a) - a unique deploy key per machine and project. (which at current
would be 17 * 3 keys all manually maintained via clicking through a
GUI [unless we wanted to automate via GitHub API (which is also a
non-trivial amount of work)]).
- or -
b) - a fake 'team' with read-only access with a single fake GitHub
account as member thereof.

I imagine this feature would be convenient for non-GitHub scenarios as
well though.

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


Re: [PATCH] check_refname_component: Optimize

2014-05-29 Thread Duy Nguyen
On Thu, May 29, 2014 at 11:36 PM, Junio C Hamano gits...@pobox.com wrote:
 Duy Nguyen pclo...@gmail.com writes:

 On Thu, May 29, 2014 at 6:49 AM, David Turner dtur...@twopensource.com 
 wrote:
 I assume that most of the time spent in check_refname_component() is
 while reading the packed-refs file, right?

 Yes.

 I wonder if we can get away without SSE code by saving stat info of
 the packed-refs version that we have verified. When we read pack-refs,
 if stat info matches, skip check_refname_component(). Assuming that
 pack-refs does not change often, of course.

 Can you elaborate a bit more?

The first time we read packed_refs, check_refname_format() is called
in read_packed_refs()-create_ref_entry() as usual. If we find no
problem, we store packed_refs stat() info in maybe packed_refs.stat.
Next time we read packed_refs, if packed_refs.stat is there and
indicates that packed_refs has not changed, we can make
create_ref_entry() ignore check_refname_format() completely. That's
even cheaper than SSE-enhanced check_refname_component() and easier to
do. If packed_refs is updated, we do the whole check_refname_format
dance again.
-- 
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


git 2.0.0 PROFILE=BUILD check-phase problems with ./t5561-http-backend.sh; GIT_TEST_HTTPD=false problems with t5537-fetch-shallow.sh

2014-05-29 Thread Nix
I observe test failures with git 2.0.0 which are attributable to the
change to run network tests by default. I'm lumping them both together
into one report because I'm lazy and I've blown too much time on this
already.

I've got Apache 2.2.24 on this box, and t5551-http-fetch-smart.sh fails
in a peculiar fashion during the PROFILE=BUILD stage:

--- exp 2014-05-29 22:42:50.221599297 +
+++ act 2014-05-29 22:42:50.231598452 +
@@ -12,10 +12,10 @@
 GET  /smart/repo.git/objects/info/http-alternates HTTP/1.1 200 -
 GET  /smart/repo.git/objects/41/57d6f47fc8b7cb455fbf6f18d6f47fed49a6a5 
HTTP/1.1 200
 GET  
/smart/repo.git/objects/pack/pack-2cb3186872e8768852fcb6b22cdf2182e12f7490.pack 
HTTP/1.1 200
-GET  
/smart/repo.git/objects/pack/pack-2cb3186872e8768852fcb6b22cdf2182e12f7490.idx 
HTTP/1.1 200

 ###  no git-daemon-export-ok
 ###
+GET  
/smart/repo.git/objects/pack/pack-2cb3186872e8768852fcb6b22cdf2182e12f7490.idx 
HTTP/1.1 200
 GET  /smart_noexport/repo.git/HEAD HTTP/1.1 404 -
 GET  /smart_noexport/repo.git/info/refs HTTP/1.1 404 -
 GET  /smart_noexport/repo.git/objects/info/packs HTTP/1.1 404 -
@@ -34,10 +34,10 @@
 GET  /smart_noexport/repo.git/objects/info/http-alternates HTTP/1.1 200 -
 GET  
/smart_noexport/repo.git/objects/41/57d6f47fc8b7cb455fbf6f18d6f47fed49a6a5 
HTTP/1.1 200
 GET  
/smart_noexport/repo.git/objects/pack/pack-2cb3186872e8768852fcb6b22cdf2182e12f7490.pack
 HTTP/1.1 200
-GET  
/smart_noexport/repo.git/objects/pack/pack-2cb3186872e8768852fcb6b22cdf2182e12f7490.idx
 HTTP/1.1 200

 ###  getanyfile true
 ###
+GET  
/smart_noexport/repo.git/objects/pack/pack-2cb3186872e8768852fcb6b22cdf2182e12f7490.idx
 HTTP/1.1 200
 GET  /smart/repo.git/HEAD HTTP/1.1 200
 GET  /smart/repo.git/info/refs HTTP/1.1 200
 GET  /smart/repo.git/objects/info/packs HTTP/1.1 200
@@ -45,10 +45,10 @@
 GET  /smart/repo.git/objects/info/http-alternates HTTP/1.1 200 -
 GET  /smart/repo.git/objects/41/57d6f47fc8b7cb455fbf6f18d6f47fed49a6a5 
HTTP/1.1 200
 GET  
/smart/repo.git/objects/pack/pack-2cb3186872e8768852fcb6b22cdf2182e12f7490.pack 
HTTP/1.1 200
-GET  
/smart/repo.git/objects/pack/pack-2cb3186872e8768852fcb6b22cdf2182e12f7490.idx 
HTTP/1.1 200

 ###  getanyfile false
 ###
+GET  
/smart/repo.git/objects/pack/pack-2cb3186872e8768852fcb6b22cdf2182e12f7490.idx 
HTTP/1.1 200
 GET  /smart/repo.git/HEAD HTTP/1.1 403 -
 GET  /smart/repo.git/info/refs HTTP/1.1 403 -
 GET  /smart/repo.git/objects/info/packs HTTP/1.1 403 -
not ok 14 - server request log matches test results
#
#   sed -e 
#   s/^.* \//
#   s/\//
#   s/ [1-9][0-9]*\$//
#   s/^GET /GET  /
#act $HTTPD_ROOT_PATH/access.log 
#   test_cmp exp act
#

# failed 1 among 14 test(s)

It appears that the Apache daemon is writing to the log slowly enough
that its log lines only get there after the testsuite has written its
separator, so a bunch of log lines appear to be attached to the wrong
test, and the comparison fails.

Curiously, I can't make this happen in a conventional 'make check', even
though the only relevant components would seem to be bash and httpd: if
anything, you'd expect the gcovvery to slow down git and thus make it
*more* likely that any race between httpd log syncing and testsuite
framework output to the same logfile would be hit...

Attempting to work around this by building with GIT_TEST_HTTPD=false
doesn't work either:

*** t5537-fetch-shallow.sh ***
ok 1 - setup
ok 2 - setup shallow clone
ok 3 - clone from shallow clone
ok 4 - fetch from shallow clone
ok 5 - fetch --depth from shallow clone
ok 6 - fetch --unshallow from shallow clone
ok 7 - fetch something upstream has but hidden by clients shallow boundaries
ok 8 - fetch that requires changes in .git/shallow is filtered
ok 9 - fetch --update-shallow
error: Can't use skip_all after running some tests
Makefile:43: recipe for target 't5537-fetch-shallow.sh' failed
make[3]: *** [t5537-fetch-shallow.sh] Error 1

since this is trying to run the httpd halfway through the test, which
will never work if it's skipping it. Moving the httpd sourcing to the
top of the test isn't going to work either, because that would skip
*everything*, when we want to skip only the httpd bits. Maybe splitting
the httpd bits into a separate test is best here? I'm not sure.

-- 
NULL  (void)
--
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] check_refname_component: Optimize

2014-05-29 Thread Duy Nguyen
On Fri, May 30, 2014 at 6:41 AM, Jeff King p...@peff.net wrote:
 On Fri, May 30, 2014 at 06:24:30AM +0700, Duy Nguyen wrote:

  I wonder if we can get away without SSE code by saving stat info of
  the packed-refs version that we have verified. When we read pack-refs,
  if stat info matches, skip check_refname_component(). Assuming that
  pack-refs does not change often, of course.
 
  Can you elaborate a bit more?

 The first time we read packed_refs, check_refname_format() is called
 in read_packed_refs()-create_ref_entry() as usual. If we find no
 problem, we store packed_refs stat() info in maybe packed_refs.stat.
 Next time we read packed_refs, if packed_refs.stat is there and
 indicates that packed_refs has not changed, we can make
 create_ref_entry() ignore check_refname_format() completely.

 I'm confused. Why would we re-open packed-refs at all if the stat
 information hasn't changed?

No, not in the same process. In the next run.


 read_packed_refs is only called from get_packed_ref_cache, and we only
 do so if !refs-packed. And refs-packed is only NULL if we haven't read
 the file yet, or it is stat-dirty.

 If that is working as intended, then we should generally only open and
 read packed-refs once per invocation of git.

 -Peff



-- 
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: .gitmodules containing SSH clone URLs should fall back to HTTPS when SSH key is not valid/existent

2014-05-29 Thread Jeff King
On Thu, May 29, 2014 at 04:12:38PM -0700, Jonathan Leonard wrote:

 We are using GitHub.
 [...]
  But why is https special? Why not fall back to the git
  protocol? Or http? (And no: I'm not serious here ;-)
 
 
 HTTPS isn't special except in that it is the least privileged
 transport type (and thus should be the last resort). Whether to
 fallback to git:// from ssh:// or vice versa is inconsequential to
 this request.

That's not quite true. git:// is the least privileged transport, as it
always anonymous and read-only (there ways to allow insecure pushes over
it, but GitHub does not enable them). Https is actually the most
flexible protocol, in that the same URL works of the box for both
logged-in and anonymous users (the latter assuming the repo is publicly
available). The server only prompts for credentials if necessary.

For that reason, it's a good choice for things like submodule URLs baked
into .gitmodules files.

The reasons not to are:

  1. It isn't _quite_ as efficient or robust as the regular git
 protocol, though in practice it's generally not a big deal.

  2. Pushers may prefer to authenticate with ssh keys (e.g., because
 they run ssh agent). I hope with modern credential helpers that
 logging in via http should not be a pain, either, though.

  After the first failed clone of the submodule at via SSH the
  developer could also just do a
 
 git config submodule.name.url https://host/repo
 
  and override the URL from .gitmodules.

 Yes, this would work. But it would be a painful manual step which we
 would not want to force on ordinary users (and would not want to
 experience ourselves either).

Using insteadOf of in your global ~/.gitconfig would make this a
one-liner per-user. So one option would be to reverse things. Put
https URLs into the .gitmodules file, so most people have to do
nothing, and then developers who really want to do git-over-ssh can do a
one-time:

  git config --global url@github.com:.insteadOf https://github.com/

-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] check_refname_component: Optimize

2014-05-29 Thread Jeff King
On Fri, May 30, 2014 at 06:43:18AM +0700, Duy Nguyen wrote:

  The first time we read packed_refs, check_refname_format() is called
  in read_packed_refs()-create_ref_entry() as usual. If we find no
  problem, we store packed_refs stat() info in maybe packed_refs.stat.
  Next time we read packed_refs, if packed_refs.stat is there and
  indicates that packed_refs has not changed, we can make
  create_ref_entry() ignore check_refname_format() completely.
 
  I'm confused. Why would we re-open packed-refs at all if the stat
  information hasn't changed?
 
 No, not in the same process. In the next run.

Ah, I thought packed_refs.stat was a struct member, but I guess you
mean it as a filename.

But then we're just trusting that the trust me flag on disk is
correct. Why not just trust that packed-refs is correct in the first
place?

IOW, consider this progression of changes:

  1. Check refname format when we read packed-refs (the current
 behavior).

  2. Keep a separate file packed-refs.stat with stat information. If
 the packed-refs file matches that stat information, do not bother
 checking refname formats.

  3. Put a flag in packed-refs that says trust me, I'm valid. Check
 the refnames when it is generated.

  4. Realize that we already check the refnames when we write it out.
 Don't bother writing trust me, I'm valid; readers can assume that
 it is.

What is the scenario that option (2) protects against that options (3)
and (4) do not?

I could guess something like the writer has a different idea of what a
valid refname is than we do. But that applies as well to (2), but just
as the reader who wrote packed-refs.stat has a different idea than we
do.

As a side note, while it is nice that we might make check_refname_format
faster, I think if you _really_ want to make repos with a lot of refs
faster, it would make more sense to introduce an on-disk format that
does not need linear parsing (e.g., something we could mmap and binary
search, or even something dbm-ish that could be updated without
rewriting the whole file (deletions, for example, must rewrite the
whole file, giving quadratic performance when deleting all refs one by
one).

-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


Bugreport: git push disobeys -c remote.xxx.url=...

2014-05-29 Thread fuz
Hello!

I've tried to changing the URL of a remote temporarily because of network
issues. I tried something like this:

git -c remote.foo.url=http://gitserver.example/repo.git push foo bar

Tracing shows that git push does not use the provided URL for the remote foo
and instead uses the URL configured in the repository configuration as if the
-c option was not present at all. This looks like a bug to me.

My git identifies as version 1.8.1.2.

Yours sincerely,
Robert Clausecker

-- 
()  ascii ribbon campaign - for an 8-bit clean world 
/\  - against html email  - against proprietary attachments
--
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 2.0.0 PROFILE=BUILD check-phase problems with ./t5561-http-backend.sh; GIT_TEST_HTTPD=false problems with t5537-fetch-shallow.sh

2014-05-29 Thread Jeff King
On Thu, May 29, 2014 at 11:44:37PM +0100, Nix wrote:

 I observe test failures with git 2.0.0 which are attributable to the
 change to run network tests by default. I'm lumping them both together
 into one report because I'm lazy and I've blown too much time on this
 already.

Weird. I also see a strange failure on t5310 when building with
PROFILE=BUILD. We get a segfault when reading jgit-produced bitmaps.
Tracking it down, we're getting inexplicably bogus data from an mmap'd
file (!). Compiling without PROFILE=BUILD, the test passes fine (even
with valgrind).

If I instrument it like this:

diff --git a/ewah/ewah_io.c b/ewah/ewah_io.c
index f7f700e..8cafacf 100644
--- a/ewah/ewah_io.c
+++ b/ewah/ewah_io.c
@@ -119,6 +119,7 @@ int ewah_read_mmap(struct ewah_bitmap *self, void *map, 
size_t len)
ptr += sizeof(uint32_t);
 
self-buffer_size = self-alloc_size = get_be32(ptr);
+   warning(got buffer_size of %lu from %lu, self-buffer_size, 
*(uint32_t *)ptr);
ptr += sizeof(uint32_t);
 
self-buffer = ewah_realloc(self-buffer,

a regular test-run reads:

  warning: got buffer_size of 2 from 33554432
  warning: got buffer_size of 2 from 33554432
  warning: got buffer_size of 3 from 50331648
  warning: got buffer_size of 1 from 16777216
  warning: got buffer_size of 2 from 33554432

and a PROFILE=BUILD one reads:

  warning: got buffer_size of 2 from 33554432
  warning: got buffer_size of 2 from 33554432
  warning: got buffer_size of 3 from 50331648
  warning: got buffer_size of 1 from 16777216
  warning: got buffer_size of 131072 from 512

I'm willing to believe we're doing something weird that violates the C
standard there, but I really can't see it.

Anyway, that's a side note to your problem...

 It appears that the Apache daemon is writing to the log slowly enough
 that its log lines only get there after the testsuite has written its
 separator, so a bunch of log lines appear to be attached to the wrong
 test, and the comparison fails.

Yeah, that looks to me like what is happening, too. If I put a 'sleep
1' into log_div, it passes. I would think apache would write the log
before serving the file, but perhaps not. And like you, I would expect
gcov to make things slower, not faster. Could there be something in the
environment that

I'm not sure what the best fix is. We could check the logfiles after
each test instead of at the end, but that will just end up with the same
race: we may check them before apache has written them.

 Attempting to work around this by building with GIT_TEST_HTTPD=false
 doesn't work either:
 
 *** t5537-fetch-shallow.sh ***
 ok 1 - setup
 ok 2 - setup shallow clone
 ok 3 - clone from shallow clone
 ok 4 - fetch from shallow clone
 ok 5 - fetch --depth from shallow clone
 ok 6 - fetch --unshallow from shallow clone
 ok 7 - fetch something upstream has but hidden by clients shallow boundaries
 ok 8 - fetch that requires changes in .git/shallow is filtered
 ok 9 - fetch --update-shallow
 error: Can't use skip_all after running some tests
 Makefile:43: recipe for target 't5537-fetch-shallow.sh' failed
 make[3]: *** [t5537-fetch-shallow.sh] Error 1

Hrm. This already came up, and we dealt with it in 0232852 (t5537: move
http tests out to t5539, 2014-02-13). Which is in v2.0.0.

But somehow the code is back in v2.0.0 Presumably this is the result of a
mis-merge. I'll send a patch.

-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] t5537: re-drop http tests

2014-05-29 Thread Jeff King
These were originally removed by 0232852 (t5537: move
http tests out to t5539, 2014-02-13). However, they were
accidentally re-added in 1ddb4d7 (Merge branch
'nd/upload-pack-shallow', 2014-03-21).

This looks like an error in manual conflict resolution.
Here's what happened:

  1. v1.9.0 shipped with the http tests in t5537.

  2. We realized that this caused problems, and built
 0232852 on top to move the tests to their own file.
 This fix made it into v1.9.1.

  3. We later had another fix in nd/upload-pack-shallow that
 also touched t5537. It was built directly on v1.9.0.

When we merged nd/upload-pack-shallow to master, we got a
conflict; it was built on a version with the http tests, but
we had since removed them. The correct resolution was to
drop the http tests and keep the new ones, but instead we
kept everything.

Signed-off-by: Jeff King p...@peff.net
---
This is a regression in running make test in v2.0.0 for people who do
not have apache installed, so it would be nice to hit maint for 2.0.1.

I had a very hard time finding the merge to blame, because the
combined-diff code is convinced this isn't an interesting hunk (because
we kept no content from the other side, as there was none to keep!). I
eventually tracked it down with git log --first-parent -m t/t5537*.

And a final side note. If you retry the merge by:

  m=1ddb4d7
  git checkout $m^1
  git merge $m^2

you can see the resulting conflict is quite tricky to understand. You
can tell from the blank ours side that _something_ was removed
(otherwise there would not have been a conflict), but there is no way to
tell what, or that the same something is part of the theirs side.
Viewing it with diff3 conflict-style makes it much more obvious.

 t/t5537-fetch-shallow.sh | 28 
 1 file changed, 28 deletions(-)

diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
index be951a4..a980574 100755
--- a/t/t5537-fetch-shallow.sh
+++ b/t/t5537-fetch-shallow.sh
@@ -173,33 +173,6 @@ EOF
)
 '
 
-if test -n $NO_CURL -o -z $GIT_TEST_HTTPD; then
-   say 'skipping remaining tests, git built without http support'
-   test_done
-fi
-
-LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5537'}
-. $TEST_DIRECTORY/lib-httpd.sh
-start_httpd
-
-test_expect_success 'clone http repository' '
-   git clone --bare --no-local shallow 
$HTTPD_DOCUMENT_ROOT_PATH/repo.git 
-   git clone $HTTPD_URL/smart/repo.git clone 
-   (
-   cd clone 
-   git fsck 
-   git log --format=%s origin/master actual 
-   cat EOF expect 
-7
-6
-5
-4
-3
-EOF
-   test_cmp expect actual
-   )
-'
-
 test_expect_success POSIXPERM,SANITY 'shallow fetch from a read-only repo' '
cp -R .git read-only.git 
find read-only.git -print | xargs chmod -w 
@@ -213,5 +186,4 @@ EOF
test_cmp expect actual
 '
 
-stop_httpd
 test_done
-- 
2.0.0.rc1.436.g03cb729


--
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: Bugreport: git push disobeys -c remote.xxx.url=...

2014-05-29 Thread Jeff King
On Fri, May 30, 2014 at 02:03:56AM +0200, f...@fuz.su wrote:

 I've tried to changing the URL of a remote temporarily because of network
 issues. I tried something like this:
 
 git -c remote.foo.url=http://gitserver.example/repo.git push foo bar
 
 Tracing shows that git push does not use the provided URL for the remote foo
 and instead uses the URL configured in the repository configuration as if the
 -c option was not present at all. This looks like a bug to me.

You are correct that this won't work, but the reason is a bit
complicated.

The remote.*.url config field is actually a multivar, meaning that you
can specify it multiple times. In that case, git push will push to
each configured URL in order in which they appear in the config.

In the command above you are not overwriting remote.foo.url, but rather
adding an extra value to it. So we first try to push to the remote
defined in your actual config, and then to the one on the command-line.
You can see this in action like:

  $ git init --bare /tmp/foo
  $ git init --bare /tmp/bar
  $ git remote add foo /tmp/foo
  $ git -c remote.foo.url=/tmp/bar push foo HEAD

You should see pushes to both /tmp/foo and /tmp/bar.

Config multivars like this are rather hard to work with, as there is no
way to say reset the multivar to empty, _then_ add this new value. So
there isn't a simple solution using remote.*.url from the command-line
like this.

Another way of doing what you want is to use url.*.insteadOf, like:

   git -c url./tmp/bar.insteadof=/tmp/foo push foo HEAD

-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: [RFC PATCH] git log: support auto decorations

2014-05-29 Thread Jeff King
On Thu, May 29, 2014 at 03:31:58PM -0700, Linus Torvalds wrote:

 From: Linus Torvalds torva...@linux-foundation.org
 Date: Thu, 29 May 2014 15:19:40 -0700
 Subject: [RFC PATCH] git log: support auto decorations

I will spare you the usual lecture on having these lines in the message
body. ;)

 I actually like seeing decorations by default, but I do *not* think our 
 current log.decorate options make sense, since they will change any 
 random use of git log to have decorations. I much prefer the 
 ui.color=auto behavior that we have for coloration. This is a trivial 
 patch that tries to approximate that.

Yeah, I think this makes a lot of sense. I do use log.decorate=true, and
it is usually not a big deal. However, I think I have run into
annoyances once or twice when piping it. I'd probably use
log.decorate=auto if we had it.

 It's marked with RFC because
 
  (a) that isatty(1) || pager_in_use() test is kind of hacky, maybe we 
  would be better off sharing something with the auto-coloration?

The magic for this is in color.c, want_color() and check_auto_color().

The color code checks pager_use_color when the pager is in use, but I
do not think that makes any sense here.  It also checks that $TERM is
not dumb, but that also does not make sense here.

So I think your check is fine. It would be nice to share with the color
code, but I doubt it will end up any more readable, because of
conditionally dealing with those two differences.

  (b) I also think it would be nice to have the equivalent for 
  --show-signature, but there we don't have any preexisting config 
  file option.

Potentially yes, though there is a real performance impact for log
--show-signature if you actually have a lot of signatures. Even on
linux.git, a full git log is 15s with --show-signature, and 5s
without. Maybe that is acceptable for interactive use (and certainly it
is not a reason to make it an _option_, if somebody wants to turn it
on).

  (c) maybe somebody would like a way to combine auto and full, 
  although personally that doesn't seem to strike me as all that useful 
  (would you really want to see the full refname when not scripting it)

Yeah, full/short is really orthogonal to true/false/auto. If we were
starting from scratch, I think putting full/short into
log.decorateStyle would make more sense, but it is probably not worth
changing now. I agree that full auto is probably not something useful,
and we can live without it.

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


Re: [PATCH] check_refname_component: Optimize

2014-05-29 Thread Duy Nguyen
On Fri, May 30, 2014 at 7:07 AM, Jeff King p...@peff.net wrote:
 But then we're just trusting that the trust me flag on disk is
 correct. Why not just trust that packed-refs is correct in the first
 place?

 IOW, consider this progression of changes:

   1. Check refname format when we read packed-refs (the current
  behavior).

   2. Keep a separate file packed-refs.stat with stat information. If
  the packed-refs file matches that stat information, do not bother
  checking refname formats.

   3. Put a flag in packed-refs that says trust me, I'm valid. Check
  the refnames when it is generated.

   4. Realize that we already check the refnames when we write it out.
  Don't bother writing trust me, I'm valid; readers can assume that
  it is.

 What is the scenario that option (2) protects against that options (3)
 and (4) do not?

 I could guess something like the writer has a different idea of what a
 valid refname is than we do. But that applies as well to (2), but just
 as the reader who wrote packed-refs.stat has a different idea than we
 do.

The reader and the writer have to agree on the same valid definition
or it wouldn't work. I don't suppose this packed-refs.stat idea would
spread out to other implementations than C git, so we're still good.
If we could write a flag in packed-refs saying trust me and other
implementations will strip it when they update packed-refs, then we're
good too.


 As a side note, while it is nice that we might make check_refname_format
 faster, I think if you _really_ want to make repos with a lot of refs
 faster, it would make more sense to introduce an on-disk format that
 does not need linear parsing (e.g., something we could mmap and binary
 search, or even something dbm-ish that could be updated without
 rewriting the whole file (deletions, for example, must rewrite the
 whole file, giving quadratic performance when deleting all refs one by
 one).

Yeah, I bring up the idea because I think Mike's multiple ref backends
is the way to go (assuming that it won't take as long as pack v4
development). If we assume we'll go with that, then we can keep the
workaround to minimum.
-- 
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: .gitmodules containing SSH clone URLs should fall back to HTTPS when SSH key is not valid/existent

2014-05-29 Thread Chris Packham
On Fri, May 30, 2014 at 11:12 AM, Jonathan Leonard johana...@gmail.com wrote:
 But you do not give much information about your special use
 case. I assume you have submodule repositories for which some
 developers have a valid ssh key and others don't (maybe
 because they should only have read access via https)?


 Precisely. Specifically this is for a collection (17 or more) of
 GitHub-hosted projects which are maintained by only a couple of people
 (who have the ability to directly push via git:// or ssh://).
 Everyone else (including deployments and ordinary users) who clones
 the repo should be able to just grab the code via HTTPS and have it
 work.

 If that is the case you might want to look into access control
 tools like gitolite.


 We are using GitHub.

  Lack of this feature (or presence
 of this bug [depending on your perspective]) is a major PITA.

 But why is https special? Why not fall back to the git
 protocol? Or http? (And no: I'm not serious here ;-)


 HTTPS isn't special except in that it is the least privileged
 transport type (and thus should be the last resort). Whether to
 fallback to git:// from ssh:// or vice versa is inconsequential to
 this request.


The problem is that a ssh:// url can't necessarily be transformed into
a correct https:// or git:// with a simple sed 's/ssh/https/' chances
are other parts of the URL will need changing. Which quickly becomes
non-trivial.

One solution that we use at work is to use relative paths (e.g.
../code/mod1.git) in .gitmodules (assuming the submodules are all part
of the same project). That way if you clone the superproject over
https:// all the submodules use that too. This works well for us using
local mirrors across multiple sites _and_ different protocols.

Another option would be to have a policy of storing the most
permissive transport in .gitmodules which makes it easy for users and
puts the special config requirements on the maintainers.

Both of these are obviously solutions you need to convince the
maintainer(s) of the superproject to implement.

Perhaps what git could do is allow multiple urls for a submodule and
at git submodule init time try them in order until the fetch is
successful. Or provide a mechanism to map transports, arguably this is
the url.foo.insteadOf mechanisim that has already been suggested.
--
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: [RFC PATCH] git log: support auto decorations

2014-05-29 Thread Linus Torvalds
On Thu, May 29, 2014 at 6:58 PM, Jeff King p...@peff.net wrote:

 I will spare you the usual lecture on having these lines in the message
 body. ;)

We do it for the kernel because they often get lost otherwise.
Particularly the date/author.

git doesn't tend to have the same kind of deep email forwarding
models, and nobody uses quilt to develop git (I hope!) but it's a
habit I like to encourage for the kernel, so I use it in general..

 Yeah, I think this makes a lot of sense. I do use log.decorate=true, and
 it is usually not a big deal. However, I think I have run into
 annoyances once or twice when piping it. I'd probably use
 log.decorate=auto if we had it.

I have various scripts to generate releases etc, using git log and
piping it to random other stuff. I don't _think_ they care, but quite
frankly, I'd rather not even have to think about it.

Also, I actually find the un-colorized version of the decorations to
be distracting. With color (not that I really like the default color
scheme, but I'm too lazy to set it up to anything else), the
colorization ends up making the decorations much easier to visually
separate, so I like them there.

 It's marked with RFC because

  (a) that isatty(1) || pager_in_use() test is kind of hacky, maybe we
  would be better off sharing something with the auto-coloration?

 The magic for this is in color.c, want_color() and check_auto_color().

Oh, I know. That's where I stole it from. But the colorization
actually has very different rules, and looks at TERM etc. So I only
stole the actual non-color-specific parts of it, but I was wondering
whether it might make sense to unify them.

  (b) I also think it would be nice to have the equivalent for
  --show-signature, but there we don't have any preexisting config
  file option.

 Potentially yes, though there is a real performance impact for log
 --show-signature if you actually have a lot of signatures. Even on
 linux.git, a full git log is 15s with --show-signature, and 5s
 without. Maybe that is acceptable for interactive use (and certainly it
 is not a reason to make it an _option_, if somebody wants to turn it
 on).

Yes. This is an example of where the whole tty is fundamentally
different from scripting happens. It really is about the whole user
is looking at it vs scripting. There is no way in hell that
--show-signature should be on by default for anybody sane.

That said, part of it is just that show-signature is so suboptimal
performance-wise, re-parsing the commit buffer for each commit when
show_signature is set. That's just crazy, we've already parsed the
commit text, we already *could* know if it has a signature or not, and
skip it if it doesn't. That would require one of the flag bits in the
object, though, or something, so it's probably not worth doing.

Sure, performance might matter if you end up searching for something
in the pager after you've done git log, but personally I think I'd
never even notice..

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