[PATCH 0/2] wincred makefile changes

[Stepan Kasal]

Hello again,
  I revisited a patch that is present in msysgit since Oct 2012.
Dropping the cosmetic changes, there are two separate parts:

Pat Thoyts (2):
  wincred: add install target
  wincred: avoid overwriting configured variables

wincred: add install target
   Git for Windows does contain git-credential-wincred
wincred: avoid overwriting configured variables
   Generally, overriding these variables this deep is evil.
   (We could use ?= if really necessary.)

Erik, does this iteration look better?

Stepan

 contrib/credential/wincred/Makefile | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

-- 
1.9.2.msysgit.0.161.g83227c1

--
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] wincred: add install target

[Stepan Kasal]

From: Pat Thoyts pattho...@users.sourceforge.net
Date: Wed, 24 Oct 2012 00:15:29 +0100

Signed-off-by: Pat Thoyts pattho...@users.sourceforge.net
Signed-off-by: Stepan Kasal ka...@ucw.cz
---
 contrib/credential/wincred/Makefile | 8 
 1 file changed, 8 insertions(+)

diff --git a/contrib/credential/wincred/Makefile 
b/contrib/credential/wincred/Makefile
index bad45ca..39fa5e0 100644
--- a/contrib/credential/wincred/Makefile
+++ b/contrib/credential/wincred/Makefile
@@ -7,8 +7,16 @@ CFLAGS = -O2 -Wall
 -include ../../../config.mak.autogen
 -include ../../../config.mak
 
+prefix ?= /usr/local
+libexecdir ?= $(prefix)/libexec/git-core
+
+INSTALL ?= install
+
 git-credential-wincred.exe : git-credential-wincred.c
$(LINK.c) $^ $(LOADLIBES) $(LDLIBS) -o $@
 
+install: git-credential-wincred.exe
+   $(INSTALL) -m 755 $^ $(libexecdir)
+
 clean:
$(RM) git-credential-wincred.exe
-- 
1.9.2.msysgit.0.161.g83227c1

--
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] wincred: avoid overwriting configured variables

[Stepan Kasal]

From: Pat Thoyts pattho...@users.sourceforge.net
Date: Wed, 24 Oct 2012 00:15:29 +0100

Signed-off-by: Pat Thoyts pattho...@users.sourceforge.net
Signed-off-by: Stepan Kasal ka...@ucw.cz
---
 contrib/credential/wincred/Makefile | 4 
 1 file changed, 4 deletions(-)

diff --git a/contrib/credential/wincred/Makefile 
b/contrib/credential/wincred/Makefile
index 39fa5e0..e64cd9a 100644
--- a/contrib/credential/wincred/Makefile
+++ b/contrib/credential/wincred/Makefile
@@ -1,9 +1,5 @@
 all: git-credential-wincred.exe
 
-CC = gcc
-RM = rm -f
-CFLAGS = -O2 -Wall
-
 -include ../../../config.mak.autogen
 -include ../../../config.mak
 
-- 
1.9.2.msysgit.0.161.g83227c1

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

Bash prompt namespace issue

[szeder]

Hi,

Commit 59d3924fb (prompt: fix missing file errors in zsh) added the  
helper function eread() to git-prompt.sh.  That function should be in  
the git namespace, i.e. prefixed with __git_, otherwise it might  
collide with a function of the same name in the user's environment.


It's already in master and I don't have the means to send a patch  
fixing this ATM, sorry.


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

Re: [PATCH 2/2] wincred: avoid overwriting configured variables

[Erik Faye-Lund]

On Tue, May 13, 2014 at 8:01 AM, Stepan Kasal ka...@ucw.cz wrote:
 From: Pat Thoyts pattho...@users.sourceforge.net
 Date: Wed, 24 Oct 2012 00:15:29 +0100

 Signed-off-by: Pat Thoyts pattho...@users.sourceforge.net
 Signed-off-by: Stepan Kasal ka...@ucw.cz
 ---
  contrib/credential/wincred/Makefile | 4 
  1 file changed, 4 deletions(-)

 diff --git a/contrib/credential/wincred/Makefile 
 b/contrib/credential/wincred/Makefile
 index 39fa5e0..e64cd9a 100644
 --- a/contrib/credential/wincred/Makefile
 +++ b/contrib/credential/wincred/Makefile
 @@ -1,9 +1,5 @@
  all: git-credential-wincred.exe

 -CC = gcc
 -RM = rm -f
 -CFLAGS = -O2 -Wall
 -

Would it be better to set these if not already set, i.e:

-CC = gcc
-RM = rm -f
-CFLAGS = -O2 -Wall
+CC ?= gcc
+RM ?= rm -f
+CFLAGS ?= -O2 -Wall

instead?
--
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] wincred: avoid overwriting configured variables

[Stepan Kasal]

From: Pat Thoyts pattho...@users.sourceforge.net
Date: Wed, 24 Oct 2012 00:15:29 +0100

Signed-off-by: Pat Thoyts pattho...@users.sourceforge.net
Signed-off-by: Stepan Kasal ka...@ucw.cz
---

Hi kusma,

On Tue, May 13, 2014 at 08:34:36AM +0200, Erik Faye-Lund wrote:
 Would it be better to set these if not already set, i.e:
 
 -CC = gcc
 -RM = rm -f
 -CFLAGS = -O2 -Wall
 +CC ?= gcc
 +RM ?= rm -f
 +CFLAGS ?= -O2 -Wall
 
 instead?

... and moving it after the two includes, so that it does not
override the values set there?

Can you ack this?

 contrib/credential/wincred/Makefile | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/contrib/credential/wincred/Makefile 
b/contrib/credential/wincred/Makefile
index 39fa5e0..6e992c0 100644
--- a/contrib/credential/wincred/Makefile
+++ b/contrib/credential/wincred/Makefile
@@ -1,12 +1,12 @@
 all: git-credential-wincred.exe
 
-CC = gcc
-RM = rm -f
-CFLAGS = -O2 -Wall
-
 -include ../../../config.mak.autogen
 -include ../../../config.mak
 
+CC ?= gcc
+RM ?= rm -f
+CFLAGS ?= -O2 -Wall
+
 prefix ?= /usr/local
 libexecdir ?= $(prefix)/libexec/git-core
 
-- 
1.9.2.msysgit.0.161.g83227c1

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

Returning error message from custom smart http server

[Ákos, Tajti]

Dear List,

we implemented our own git smart http server to be able to check 
permissions and other thing before pushes. It works fine, however, the 
error messages we generate on the server side are not displayed by the 
command line client. On the server we generate error messages like this:


response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
response.getWriter().write(msg);

On the command line we get this:

Total 0 (delta 0), reused 0 (delta 0)
POST git-receive-pack (290 bytes)
efrror: RPC failed; result=22, HTTP code = 401
atal: The remote end hung up unexpectedly
fatal: The remote end hung up unexpectedly

The server message is completely missing. Is there a solution for this?

Thanks,
Ákos Tajti


---
A levél vírus, és rosszindulatú kód mentes, mert az avast! Antivirus védelme 
ellenőrizte azt.
http://www.avast.com

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

Re: Error using git-remote-hg

[Charles Brossollet]

Le 12 mai 2014 à 21:37, Felipe Contreras felipe.contre...@gmail.com a écrit :

 Torsten Bögershausen wrote:
 I'm using git 1.9.3 on Mac OS X 10.9.2, with hg 3.0 installed with brew.
 
 It used to work before, on this same repository, since then git and hg were 
 both upgraded.
 In short: The remote helper of Git 1.9.3 is not compatible with hg 3.0
 You can eiher downgrade hg, or rebuild Git and cherry-pick this commit:
 
 No need to rebuild Git.
 
 You can also use the latest version:
 https://github.com/felipec/git-remote-hg

Thanks Felipe and Torsten, just using the HEAD version of git-remote-hg solved 
the problem.

Best regards,
Charles--
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] git format-patch --signature string | file

[Jeremiah Mahler]

Improved format-patch --signature option so that it can
read from a file as well as from a string.

  # from a string
  $ git format-patch --signature from a string origin

  # or from a file
  $ git format-patch --signature ~/.signature origin

Now signatures with newlines or other special characters
can be easily included.

Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
 builtin/log.c | 26 --
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 39e8836..5988f8f 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1147,6 +1147,27 @@ static int from_callback(const struct option *opt, const 
char *arg, int unset)
return 0;
 }
 
+static int signature_callback(const struct option *opt, const char *arg,
+   int unset)
+{
+   const char **signature = opt-value;
+   static char buf[1024];
+   size_t sz;
+   FILE *fp;
+
+   fp = fopen(arg, r);
+   if (fp) {
+   sz = sizeof(buf);
+   sz = fread(buf, 1, sz - 1, fp);
+   buf[sz] = '\0';
+   *signature = buf;
+   fclose(fp);
+   } else {
+   *signature = arg;
+   }
+   return 0;
+}
+
 int cmd_format_patch(int argc, const char **argv, const char *prefix)
 {
struct commit *commit;
@@ -1228,8 +1249,9 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
{ OPTION_CALLBACK, 0, thread, thread, N_(style),
N_(enable message threading, styles: shallow, 
deep),
PARSE_OPT_OPTARG, thread_callback },
-   OPT_STRING(0, signature, signature, N_(signature),
-   N_(add a signature)),
+   { OPTION_CALLBACK, 0, signature, signature, 
N_(signature-file),
+   N_(add a signature from a string or a file),
+   PARSE_OPT_NONEG, signature_callback },
OPT__QUIET(quiet, N_(don't print the patch filenames)),
OPT_END()
};
-- 
Jeremiah Mahler
jmmah...@gmail.com
http://github.com/jmahler


--
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] git-gui regression in 2.0rcX within submodule

[Chris Packham]

Hi,

On 13/05/14 11:45, Yann Dirson wrote:
 In 2.0rc2, git-gui is unable to work inside submodules, where 1.9.2
 did not show such a problem:
 
 
 yann@home:~$ cd /tmp/
 yann@home:tmp$ mkdir foo
 yann@home:tmp$ cd foo/
 yann@home:foo$ git init
 Initialized empty Git repository in /tmp/foo/.git/
 yann@home:foo (master)$ git submodule add 
 git://git.debian.org/git/collab-maint/tulip.git debian
 Cloning into 'debian'...
 remote: Counting objects: 317, done.
 remote: Compressing objects: 100% (199/199), done.
 remote: Total 317 (delta 184), reused 166 (delta 95)
 Receiving objects: 100% (317/317), 73.81 KiB | 0 bytes/s, done.
 Resolving deltas: 100% (184/184), done.
 Checking connectivity... done.
 yann@home:foo (master)$ git status 
 On branch master
 
 Initial commit
 
 Changes to be committed:
   (use git rm --cached file... to unstage)
 
 new file:   .gitmodules
 new file:   debian
 
 yann@home:foo (master)$ (cd debian/  git gui)
 [errors out after showing the following error dialog]
 
 | No working directory ../../../debian:
 | 
 | couldn't change working directory
 | to ../../../debian: no such file or
 | directory
 

I've already reported the same issue[1] and have posted a possible
solution[2] although I haven't seen any feedback from Pat or anyone else.

 
 strace shows the failing chdir call is from git-gui itself, after
 getcwd() told him that it is in the dir that is indeed the workdir
 already.
 

--

[1] - http://article.gmane.org/gmane.comp.version-control.git/247511
[2] - http://article.gmane.org/gmane.comp.version-control.git/247564

--
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 - Git reset --quiet not quiet

[Erik Faye-Lund]

On Mon, May 12, 2014 at 9:16 PM, Thomas-Louis Laforest
tllafor...@arbault.ca wrote:
 Good afternoon,

 When running this command on Git for Windows (version 1.9.2-preview20140411)
 git reset --quiet --hard with one file having read/write lock git ask this 
 question :
 Unlink of file '' failed. Should I try again? (y/n)

 I will have expected the command --quiet to remove the question and 
 auto-answer no.
 This broke an automated script we use.

Thanks for reporting this.

The problem here is really a nasty case of layering: the question
comes from a place deep inside the OS compatibility layer, which
doesn't know about the --quiet flag.

However, do note that if fixed, the command would still fail under
these conditions. But it won't hang forever, as it does now.

Mainline Git-folks: The problem here is essentially unlink returning
EBUSY (although that's not *exactly* what happes - but it's close
enough, implementation details in mingw_unlink), which most of the git
codebase assume won't happen. On Windows, this happens *all* the time,
usually due to antivirus-software scanning a newly written file. We
currently retry a few times with some waiting in mingw_unlink, and
then finally prompts the user. But this gives the problem described
above, as mingw_unlink has no clue about --quiet.

I guess this could be solved in a few ways.
1) Let mingw_unlink() know about the quiet-flag. This probably
involves moving the quiet-flag from each tool into libgit.a.
2) Make the quiet-flags close stdout instead of suppressing every output.
3) Make the higher level call-sites of Git EBUSY-aware. This probably
involves making an interactive convenience-wrapper of unlink, that
accepts a quiet flag - similar to what mingw_unlink does.

Option 1) seems quite error-prone to me - it's difficult to make sure
all code-paths actually set this flag, so there's a good chance of
regressions. Option 2) also sounds a bit risky, as we lose stdout
forever, with no escape-hatch. So to me option 3) seems preferable
although it might translate into a bit of churn. Thoughts?
--
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 - Git reset --quiet not quiet

[Johannes Sixt]

Am 5/13/2014 11:09, schrieb Erik Faye-Lund:
 On Mon, May 12, 2014 at 9:16 PM, Thomas-Louis Laforest
 tllafor...@arbault.ca wrote:
 When running this command on Git for Windows (version 1.9.2-preview20140411)
 git reset --quiet --hard with one file having read/write lock git ask this 
 question :
 Unlink of file '' failed. Should I try again? (y/n)

 I will have expected the command --quiet to remove the question and 
 auto-answer no.
 This broke an automated script we use.
...
 I guess this could be solved in a few ways.
 1) Let mingw_unlink() know about the quiet-flag. This probably
 involves moving the quiet-flag from each tool into libgit.a.
 2) Make the quiet-flags close stdout instead of suppressing every output.
 3) Make the higher level call-sites of Git EBUSY-aware. This probably
 involves making an interactive convenience-wrapper of unlink, that
 accepts a quiet flag - similar to what mingw_unlink does.

Is any of this really needed? We have this in ask_yes_no_if_possible():

if (!isatty(_fileno(stdin)) || !isatty(_fileno(stderr)))
return 0;

i.e., we answer no automatically without asking if at least one of stdin
and stderr are not a terminal. Perhaps the OP's problem is that they do
not redirect these channels to files or something in their automated
scripts? In particular, it should be sufficient to redirect stdin from
/dev/null (a.k.a. nul on Windows).

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

Re: Bug - Git reset --quiet not quiet

[Erik Faye-Lund]

On Tue, May 13, 2014 at 11:38 AM, Johannes Sixt j.s...@viscovery.net wrote:
 Am 5/13/2014 11:09, schrieb Erik Faye-Lund:
 On Mon, May 12, 2014 at 9:16 PM, Thomas-Louis Laforest
 tllafor...@arbault.ca wrote:
 When running this command on Git for Windows (version 1.9.2-preview20140411)
 git reset --quiet --hard with one file having read/write lock git ask this 
 question :
 Unlink of file '' failed. Should I try again? (y/n)

 I will have expected the command --quiet to remove the question and 
 auto-answer no.
 This broke an automated script we use.
 ...
 I guess this could be solved in a few ways.
 1) Let mingw_unlink() know about the quiet-flag. This probably
 involves moving the quiet-flag from each tool into libgit.a.
 2) Make the quiet-flags close stdout instead of suppressing every output.
 3) Make the higher level call-sites of Git EBUSY-aware. This probably
 involves making an interactive convenience-wrapper of unlink, that
 accepts a quiet flag - similar to what mingw_unlink does.

 Is any of this really needed? We have this in ask_yes_no_if_possible():

 if (!isatty(_fileno(stdin)) || !isatty(_fileno(stderr)))
 return 0;

 i.e., we answer no automatically without asking if at least one of stdin
 and stderr are not a terminal. Perhaps the OP's problem is that they do
 not redirect these channels to files or something in their automated
 scripts? In particular, it should be sufficient to redirect stdin from
 /dev/null (a.k.a. nul on Windows).

Well, sure. But if sounds like surprising behavior to me. And I also
suspect doing this unconditionally on all platforms will fix strange
corner-cases on other systems, when using Windows file-systems. AFAIK,
the whole cannot delete an open file-thing is an NTFS-detail (and
possibly also FAT, which is quite commonly used on non-Windows).
--
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/8] read-cache: allow to keep mmap'd memory after reading

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 cache.h  |  3 +++
 read-cache.c | 13 -
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/cache.h b/cache.h
index c6b7770..6549e02 100644
--- a/cache.h
+++ b/cache.h
@@ -290,10 +290,13 @@ struct index_state {
struct split_index *split_index;
struct cache_time timestamp;
unsigned name_hash_initialized : 1,
+keep_mmap : 1,
 initialized : 1;
struct hashmap name_hash;
struct hashmap dir_hash;
unsigned char sha1[20];
+   void *mmap;
+   size_t mmap_size;
 };
 
 extern struct index_state the_index;
diff --git a/read-cache.c b/read-cache.c
index 342fe52..a5031f3 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1495,6 +1495,10 @@ int do_read_index(struct index_state *istate, const char 
*path, int must_exist)
mmap = xmmap(NULL, mmap_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 
0);
if (mmap == MAP_FAILED)
die_errno(unable to map index file);
+   if (istate-keep_mmap) {
+   istate-mmap = mmap;
+   istate-mmap_size = mmap_size;
+   }
close(fd);
 
hdr = mmap;
@@ -1547,10 +1551,12 @@ int do_read_index(struct index_state *istate, const 
char *path, int must_exist)
src_offset += 8;
src_offset += extsize;
}
-   munmap(mmap, mmap_size);
+   if (!istate-keep_mmap)
+   munmap(mmap, mmap_size);
return istate-cache_nr;
 
 unmap:
+   istate-mmap = NULL;
munmap(mmap, mmap_size);
die(index file corrupt);
 }
@@ -1576,6 +1582,7 @@ int read_index_from(struct index_state *istate, const 
char *path)
discard_index(split_index-base);
else
split_index-base = xcalloc(1, sizeof(*split_index-base));
+   split_index-base-keep_mmap = istate-keep_mmap;
ret = do_read_index(split_index-base,
git_path(sharedindex.%s,
 sha1_to_hex(split_index-base_sha1)), 1);
@@ -1618,6 +1625,10 @@ int discard_index(struct index_state *istate)
free(istate-cache);
istate-cache = NULL;
istate-cache_alloc = 0;
+   if (istate-keep_mmap  istate-mmap) {
+   munmap(istate-mmap, istate-mmap_size);
+   istate-mmap = NULL;
+   }
discard_split_index(istate);
return 0;
 }
-- 
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 0/8] Speed up cache loading time

[Nguyễn Thái Ngọc Duy]

On Fri, May 9, 2014 at 5:27 PM, Duy Nguyen pclo...@gmail.com wrote:
 The below patch implements such a daemon to cache the index. It takes
 91ms and 377ms to load a 25MB index with and without the daemon. I use
 share memory instead of pipe, but the format is still on disk not
 in memory for simplicity. I think we're good even without in memory
 format.

Here is a better version (on top of split-index). I duplicated webkit
index 8 times to get its size to 199MB (version 2), close to what
Facebook tried last time [1]. read_cache() on index v2, v4, with the
daemon caching v2 and v4 respectively is 2994.861ms (199MB index
file), 2245.113ms (118MB) and 663.399ms and 880.935ms. The best number
is 4.5 times better the worst.

That is clocked at 800 MHz. A repository at this size deserves a
better CPU. At 2.5 GHz we spend 183.228ms on loading the index. A
reasonable number to me. If we scale other parts of git-status as well
as this, we should be able to make git status within 1 or 2 seconds.

The tested index does not have fully populated cache-tree so real
world numbers could be a bit higher.

[1] http://thread.gmane.org/gmane.comp.version-control.git/189776/focus=190156

Nguyễn Thái Ngọc Duy (8):
  read-cache: allow to keep mmap'd memory after reading
  unix-socket: stub impl. for platforms with no unix socket support
  daemonize: set a flag before exiting the main process
  Add read-cache--daemon for caching index and related stuff
  read-cache: try index data from shared memory
  read-cache--daemon: do not read index from shared memory
  read-cache: skip verifying trailing SHA-1 on cached index
  read-cache: inform the daemon that the index has been updated

 .gitignore |   1 +
 Documentation/config.txt   |   4 +
 Documentation/git-read-cache--daemon.txt (new) |  27 
 Makefile   |   8 +
 builtin/gc.c   |   2 +-
 cache.h|   7 +-
 config.c   |  12 ++
 config.mak.uname   |   1 +
 daemon.c   |   2 +-
 environment.c  |   1 +
 read-cache--daemon.c (new) | 208 +
 read-cache.c   | 116 +-
 setup.c|   4 +-
 submodule.c|   1 +
 unix-socket.h  |  18 +++
 wrapper.c  |  14 ++
 16 files changed, 414 insertions(+), 12 deletions(-)
 create mode 100644 Documentation/git-read-cache--daemon.txt
 create mode 100644 read-cache--daemon.c

-- 
1.9.1.346.ga2b5940

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

[PATCH 2/3] Add read-cache--daemon

[Nguyễn Thái Ngọc Duy]

---
 .gitignore |   1 +
 Makefile   |   6 ++
 config.mak.uname   |   1 +
 git-compat-util.h  |   8 +++
 read-cache--daemon.c (new) | 167 +
 5 files changed, 183 insertions(+)
 create mode 100644 read-cache--daemon.c

diff --git a/.gitignore b/.gitignore
index 70992a4..07e0cb6 100644
--- a/.gitignore
+++ b/.gitignore
@@ -110,6 +110,7 @@
 /git-pull
 /git-push
 /git-quiltimport
+/git-read-cache--daemon
 /git-read-tree
 /git-rebase
 /git-rebase--am
diff --git a/Makefile b/Makefile
index 028749b..98d22de 100644
--- a/Makefile
+++ b/Makefile
@@ -1502,6 +1502,12 @@ ifdef HAVE_DEV_TTY
BASIC_CFLAGS += -DHAVE_DEV_TTY
 endif
 
+ifdef HAVE_SHM
+   BASIC_CFLAGS += -DHAVE_SHM
+   EXTLIBS += -lrt
+   PROGRAM_OBJS += read-cache--daemon.o
+endif
+
 ifdef DIR_HAS_BSD_GROUP_SEMANTICS
COMPAT_CFLAGS += -DDIR_HAS_BSD_GROUP_SEMANTICS
 endif
diff --git a/config.mak.uname b/config.mak.uname
index 23a8803..b6a37e5 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -33,6 +33,7 @@ ifeq ($(uname_S),Linux)
HAVE_PATHS_H = YesPlease
LIBC_CONTAINS_LIBINTL = YesPlease
HAVE_DEV_TTY = YesPlease
+   HAVE_SHM = YesPlease
 endif
 ifeq ($(uname_S),GNU/kFreeBSD)
NO_STRLCPY = YesPlease
diff --git a/git-compat-util.h b/git-compat-util.h
index f6d3a46..b2116ab 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -723,4 +723,12 @@ struct tm *git_gmtime_r(const time_t *, struct tm *);
 #define gmtime_r git_gmtime_r
 #endif
 
+#ifndef HAVE_SHM
+static inline int shm_open(const char *path, int flags, int mode)
+{
+   errno = ENOSYS;
+   return -1;
+}
+#endif
+
 #endif
diff --git a/read-cache--daemon.c b/read-cache--daemon.c
new file mode 100644
index 000..52b4067
--- /dev/null
+++ b/read-cache--daemon.c
@@ -0,0 +1,167 @@
+#include cache.h
+#include sigchain.h
+#include unix-socket.h
+#include split-index.h
+#include pkt-line.h
+
+static char *socket_path;
+static struct strbuf shm_index = STRBUF_INIT;
+static struct strbuf shm_sharedindex = STRBUF_INIT;
+
+static void cleanup_socket(void)
+{
+   if (socket_path)
+   unlink(socket_path);
+   if (shm_index.len)
+   shm_unlink(shm_index.buf);
+   if (shm_sharedindex.len)
+   shm_unlink(shm_sharedindex.buf);
+}
+
+static void cleanup_socket_on_signal(int sig)
+{
+   cleanup_socket();
+   sigchain_pop(sig);
+   raise(sig);
+}
+
+static void share_index(struct index_state *istate, struct strbuf *shm_path)
+{
+   struct strbuf sb = STRBUF_INIT;
+   void *map;
+   int fd;
+
+   strbuf_addf(sb, /git-index-%s, sha1_to_hex(istate-sha1));
+   if (shm_path-len  strcmp(sb.buf, shm_path-buf)) {
+   shm_unlink(shm_path-buf);
+   strbuf_reset(shm_path);
+   }
+   fd = shm_open(sb.buf, O_RDWR | O_CREAT | O_TRUNC, 0700);
+   if (fd  0)
+   return;
+   /*
+* We lock the shm in preparation by set its size larger
+* than expected. The reader is supposed to check the size and
+* ignore if shm size is different than the actual file size
+*/
+   if (ftruncate(fd, istate-mmap_size + 1)) {
+   close(fd);
+   shm_unlink(shm_path-buf);
+   return;
+   }
+   strbuf_addbuf(shm_path, sb);
+   map = xmmap(NULL, istate-mmap_size, PROT_READ | PROT_WRITE,
+   MAP_SHARED, fd, 0);
+   if (map == MAP_FAILED) {
+   close(fd);
+   shm_unlink(shm_path-buf);
+   return;
+   }
+   memcpy(map, istate-mmap, istate-mmap_size);
+   munmap(map, istate-mmap_size);
+   /* Now unlock it */
+   if (ftruncate(fd, istate-mmap_size)) {
+   close(fd);
+   shm_unlink(shm_path-buf);
+   return;
+   }
+   close(fd);
+}
+
+static void refresh()
+{
+   the_index.keep_mmap = 1;
+   if (read_cache()  0)
+   die(could not read index);
+   share_index(the_index, shm_index);
+   if (the_index.split_index 
+   the_index.split_index-base)
+   share_index(the_index.split_index-base, shm_sharedindex);
+   discard_index(the_index);
+}
+
+static unsigned long next;
+static int serve_cache_loop(int fd)
+{
+   struct pollfd pfd;
+   unsigned long now = time(NULL);
+
+   if (now  next)
+   return 0;
+
+   pfd.fd = fd;
+   pfd.events = POLLIN;
+   if (poll(pfd, 1, 1000 * (next - now))  0) {
+   if (errno != EINTR)
+   die_errno(poll failed);
+   return 1;
+   }
+
+   if (pfd.revents  POLLIN) {
+   int client = accept(fd, NULL, NULL);
+   if (client  0) {
+   warning(accept failed: %s, strerror(errno));
+   return 1;
+   }
+   refresh();
+ 

[PATCH 3/8] daemonize: set a flag before exiting the main process

[Nguyễn Thái Ngọc Duy]

This allows signal handlers and atexit functions to realize this
situation and not clean up.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 builtin/gc.c | 2 +-
 cache.h  | 2 +-
 daemon.c | 2 +-
 setup.c  | 4 +++-
 4 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 85f5c2b..50275af 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -325,7 +325,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 * failure to daemonize is ok, we'll continue
 * in foreground
 */
-   daemonize();
+   daemonize(NULL);
} else
add_repack_all_option();
 
diff --git a/cache.h b/cache.h
index 6549e02..d0ff11c 100644
--- a/cache.h
+++ b/cache.h
@@ -450,7 +450,7 @@ extern int set_git_dir_init(const char *git_dir, const char 
*real_git_dir, int);
 extern int init_db(const char *template_dir, unsigned int flags);
 
 extern void sanitize_stdfds(void);
-extern int daemonize(void);
+extern int daemonize(int *);
 
 #define alloc_nr(x) (((x)+16)*3/2)
 
diff --git a/daemon.c b/daemon.c
index eba1255..2650504 100644
--- a/daemon.c
+++ b/daemon.c
@@ -1311,7 +1311,7 @@ int main(int argc, char **argv)
return execute();
 
if (detach) {
-   if (daemonize())
+   if (daemonize(NULL))
die(--detach not supported on this platform);
} else
sanitize_stdfds();
diff --git a/setup.c b/setup.c
index 613e3b3..e8e129a 100644
--- a/setup.c
+++ b/setup.c
@@ -842,7 +842,7 @@ void sanitize_stdfds(void)
close(fd);
 }
 
-int daemonize(void)
+int daemonize(int *daemonized)
 {
 #ifdef NO_POSIX_GOODIES
errno = ENOSYS;
@@ -854,6 +854,8 @@ int daemonize(void)
case -1:
die_errno(fork failed);
default:
+   if (daemonized)
+   *daemonized = 1;
exit(0);
}
if (setsid() == -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  http://vger.kernel.org/majordomo-info.html

[PATCH 2/8] unix-socket: stub impl. for platforms with no unix socket support

[Nguyễn Thái Ngọc Duy]

With this we can make unix_stream_* calls without #ifdef.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Makefile  |  2 ++
 unix-socket.h | 18 ++
 2 files changed, 20 insertions(+)

diff --git a/Makefile b/Makefile
index 028749b..d0a2b4b 100644
--- a/Makefile
+++ b/Makefile
@@ -1417,6 +1417,8 @@ ifndef NO_UNIX_SOCKETS
LIB_H += unix-socket.h
PROGRAM_OBJS += credential-cache.o
PROGRAM_OBJS += credential-cache--daemon.o
+else
+   BASIC_CFLAGS += -DNO_UNIX_SOCKETS
 endif
 
 ifdef NO_ICONV
diff --git a/unix-socket.h b/unix-socket.h
index e271aee..f1cba70 100644
--- a/unix-socket.h
+++ b/unix-socket.h
@@ -1,7 +1,25 @@
 #ifndef UNIX_SOCKET_H
 #define UNIX_SOCKET_H
 
+#ifndef NO_UNIX_SOCKETS
+
 int unix_stream_connect(const char *path);
 int unix_stream_listen(const char *path);
 
+#else
+
+static inline int unix_stream_connect(const char *path)
+{
+   errno = ENOSYS;
+   return -1;
+}
+
+static inline int unix_stream_listen(const char *path)
+{
+   errno = ENOSYS;
+   return -1;
+}
+
+#endif
+
 #endif /* UNIX_SOCKET_H */
-- 
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/3] read-cache: try index data from shared memory

[Nguyễn Thái Ngọc Duy]

---
 read-cache.c | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/read-cache.c b/read-cache.c
index 9e742c7..3100a59 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1462,6 +1462,35 @@ static struct cache_entry *create_from_disk(struct 
ondisk_cache_entry *ondisk,
return ce;
 }
 
+static void *try_shm(void *mmap, size_t mmap_size)
+{
+   struct strbuf sb = STRBUF_INIT;
+   void *new_mmap;
+   struct stat st;
+   int fd;
+
+   if (mmap_size = 20)
+   return mmap;
+
+   strbuf_addf(sb, /git-index-%s,
+   sha1_to_hex((unsigned char *)mmap + mmap_size - 20));
+   fd = shm_open(sb.buf, O_RDONLY, 0777);
+   strbuf_release(sb);
+   if (fd  0)
+   return mmap;
+   if (fstat(fd, st) || st.st_size != mmap_size) {
+   close(fd);
+   return mmap;
+   }
+   new_mmap = xmmap(NULL, mmap_size, PROT_READ, MAP_PRIVATE, fd, 0);
+   close(fd);
+   if (new_mmap == MAP_FAILED)
+   return mmap;
+   munmap(mmap, mmap_size);
+   return new_mmap;
+}
+
+
 /* remember to discard_cache() before reading a different cache! */
 int do_read_index(struct index_state *istate, const char *path, int must_exist)
 {
@@ -1501,6 +1530,7 @@ int do_read_index(struct index_state *istate, const char 
*path, int must_exist)
}
close(fd);
 
+   mmap = try_shm(mmap, mmap_size);
hdr = mmap;
if (verify_hdr(hdr, mmap_size)  0)
goto unmap;
-- 
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 6/8] read-cache--daemon: do not read index from shared memory

[Nguyễn Thái Ngọc Duy]

It does not hurt doing that. But it does not help anybody either.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 read-cache--daemon.c | 1 +
 read-cache.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/read-cache--daemon.c b/read-cache--daemon.c
index 4531978..bd6d84f 100644
--- a/read-cache--daemon.c
+++ b/read-cache--daemon.c
@@ -145,6 +145,7 @@ static void serve_cache(const char *socket_path, int detach)
if (fd  0)
die_errno(unable to bind to '%s', socket_path);
 
+   use_read_cache_daemon = -1;
refresh();
if (detach  daemonize(daemonized))
die_errno(unable to detach);
diff --git a/read-cache.c b/read-cache.c
index 0e46523..4041485 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1470,7 +1470,7 @@ static void *try_shm(void *mmap, size_t *mmap_size)
struct stat st;
int fd;
 
-   if (old_size = 20 || !use_read_cache_daemon)
+   if (old_size = 20 || use_read_cache_daemon = 0)
return mmap;
 
strbuf_addf(sb, /git-index-%s.lock,
-- 
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 5/8] read-cache: try index data from shared memory

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/config.txt |  4 
 cache.h  |  1 +
 config.c | 12 
 environment.c|  1 +
 read-cache.c | 43 +++
 submodule.c  |  1 +
 6 files changed, 62 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index d8b6cc9..ccbe00b 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -617,6 +617,10 @@ relatively high IO latencies.  With this set to 'true', 
Git will do the
 index comparison to the filesystem data in parallel, allowing
 overlapping IO's.
 
+core.useReadCacheDaemon::
+   Use `git read-cache--daemon` to speed up index reading. See
+   linkgit:git-read-cache--daemon for more information.
+
 core.createObject::
You can set this to 'link', in which case a hardlink followed by
a delete of the source are used to make sure that object creation
diff --git a/cache.h b/cache.h
index d0ff11c..fb29c7e 100644
--- a/cache.h
+++ b/cache.h
@@ -603,6 +603,7 @@ extern size_t packed_git_limit;
 extern size_t delta_base_cache_limit;
 extern unsigned long big_file_threshold;
 extern unsigned long pack_size_limit_cfg;
+extern int use_read_cache_daemon;
 
 /*
  * Do replace refs need to be checked this run?  This variable is
diff --git a/config.c b/config.c
index a30cb5c..5c832ad 100644
--- a/config.c
+++ b/config.c
@@ -874,6 +874,18 @@ static int git_default_core_config(const char *var, const 
char *value)
return 0;
}
 
+#ifdef HAVE_SHM
+   /*
+* Currently git-read-cache--daemon is only built when
+* HAVE_SHM is set. Ignore user settings if HAVE_SHM is not
+* defined.
+*/
+   if (!strcmp(var, core.usereadcachedaemon)) {
+   use_read_cache_daemon = git_config_bool(var, value);
+   return 0;
+   }
+#endif
+
/* Add other config variables here and to Documentation/config.txt. */
return 0;
 }
diff --git a/environment.c b/environment.c
index 5c4815d..b76a414 100644
--- a/environment.c
+++ b/environment.c
@@ -63,6 +63,7 @@ int merge_log_config = -1;
 int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
 struct startup_info *startup_info;
 unsigned long pack_size_limit_cfg;
+int use_read_cache_daemon;
 
 /*
  * The character that begins a commented line in user-editable file
diff --git a/read-cache.c b/read-cache.c
index a5031f3..0e46523 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1462,6 +1462,48 @@ static struct cache_entry *create_from_disk(struct 
ondisk_cache_entry *ondisk,
return ce;
 }
 
+static void *try_shm(void *mmap, size_t *mmap_size)
+{
+   struct strbuf sb = STRBUF_INIT;
+   size_t old_size = *mmap_size;
+   void *new_mmap;
+   struct stat st;
+   int fd;
+
+   if (old_size = 20 || !use_read_cache_daemon)
+   return mmap;
+
+   strbuf_addf(sb, /git-index-%s.lock,
+   sha1_to_hex((unsigned char *)mmap + old_size - 20));
+   fd = shm_open(sb.buf, O_RDONLY, 0777);
+   if (fd = 0) {
+   close(fd);
+   return mmap;
+   }
+   strbuf_setlen(sb, sb.len - 5); /* no .lock */
+   fd = shm_open(sb.buf, O_RDONLY, 0777);
+   strbuf_release(sb);
+   if (fd  0)
+   /*
+* git-read-cache--daemon is probably not started yet. For
+* simplicity, only start it at the next index update, which
+* should happen often.
+*/
+   return mmap;
+   if (fstat(fd, st)) {
+   close(fd);
+   return mmap;
+   }
+   new_mmap = xmmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
+   close(fd);
+   if (new_mmap == MAP_FAILED)
+   return mmap;
+   munmap(mmap, old_size);
+   *mmap_size = st.st_size;
+   return new_mmap;
+}
+
+
 /* remember to discard_cache() before reading a different cache! */
 int do_read_index(struct index_state *istate, const char *path, int must_exist)
 {
@@ -1501,6 +1543,7 @@ int do_read_index(struct index_state *istate, const char 
*path, int must_exist)
}
close(fd);
 
+   mmap = try_shm(mmap, mmap_size);
hdr = mmap;
if (verify_hdr(hdr, mmap_size)  0)
goto unmap;
diff --git a/submodule.c b/submodule.c
index b80ecac..9872928 100644
--- a/submodule.c
+++ b/submodule.c
@@ -195,6 +195,7 @@ void gitmodules_config(void)
int pos;
strbuf_addstr(gitmodules_path, work_tree);
strbuf_addstr(gitmodules_path, /.gitmodules);
+   git_config(git_default_config, NULL);
if (read_cache()  0)
die(index file corrupt);
pos = cache_name_pos(.gitmodules, 11);
-- 
1.9.1.346.ga2b5940

--
To unsubscribe from this list: send the line 

[PATCH 4/8] Add read-cache--daemon for caching index and related stuff

[Nguyễn Thái Ngọc Duy]

The name of the shared memory folows the template /git-index-SHA1
where SHA1 is the trailing SHA-1 of the index file. If such a shared
memory exists, it contains the same index content as on disk. The
content is already validated by the daemon. Note that it does not
necessarily use the same format as the on-disk version. The content
could be in a format that can be parsed much faster, or even reused
without parsing).

While preparing the shm object, the daemon would keep the shm object
/git-index-SHA1.lock. After git-index-SHA1 is ready, the
.lock object is removed. A shared object must not be updated
afterwards. So if .lock does not exist, it's safe to assume that the
associated shm object is ready.

Other info could also by cached if it's tied to the index. For
example, name hash could be stored in /git-namehash-SHA1..

After Git writes a new index down, it may want to ask the daemon to
preload the new index so next time Git runs the index is already
validated and in memory. It does so by send a command to a UNIX socket
in $GIT_DIR/daemon/index.

Windows can use its named shared memory instead of POSIX shared memory
and probably named pipe in place of UNIX socket.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 .gitignore |   1 +
 Documentation/git-read-cache--daemon.txt (new) |  27 
 Makefile   |   6 +
 config.mak.uname   |   1 +
 read-cache--daemon.c (new) | 207 +
 wrapper.c  |  14 ++
 6 files changed, 256 insertions(+)
 create mode 100644 Documentation/git-read-cache--daemon.txt
 create mode 100644 read-cache--daemon.c

diff --git a/.gitignore b/.gitignore
index 70992a4..07e0cb6 100644
--- a/.gitignore
+++ b/.gitignore
@@ -110,6 +110,7 @@
 /git-pull
 /git-push
 /git-quiltimport
+/git-read-cache--daemon
 /git-read-tree
 /git-rebase
 /git-rebase--am
diff --git a/Documentation/git-read-cache--daemon.txt 
b/Documentation/git-read-cache--daemon.txt
new file mode 100644
index 000..1b05be4
--- /dev/null
+++ b/Documentation/git-read-cache--daemon.txt
@@ -0,0 +1,27 @@
+git-read-cache--daemon(1)
+=
+
+NAME
+
+git-daemon - A simple cache server for speeding up index file access
+
+SYNOPSIS
+
+[verse]
+'git daemon' [--detach]
+
+DESCRIPTION
+---
+Keep the index file in memory for faster access. This daemon is per
+repository. Note that core.useReadCacheDaemon must be set for Git to
+contact the daemon. This daemon is only available on POSIX system with
+shared memory support (e.g. Linux)
+
+OPTIONS
+---
+--detach::
+   Detach from the shell.
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Makefile b/Makefile
index d0a2b4b..a44ab0b 100644
--- a/Makefile
+++ b/Makefile
@@ -1504,6 +1504,12 @@ ifdef HAVE_DEV_TTY
BASIC_CFLAGS += -DHAVE_DEV_TTY
 endif
 
+ifdef HAVE_SHM
+   BASIC_CFLAGS += -DHAVE_SHM
+   EXTLIBS += -lrt
+   PROGRAM_OBJS += read-cache--daemon.o
+endif
+
 ifdef DIR_HAS_BSD_GROUP_SEMANTICS
COMPAT_CFLAGS += -DDIR_HAS_BSD_GROUP_SEMANTICS
 endif
diff --git a/config.mak.uname b/config.mak.uname
index 23a8803..b6a37e5 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -33,6 +33,7 @@ ifeq ($(uname_S),Linux)
HAVE_PATHS_H = YesPlease
LIBC_CONTAINS_LIBINTL = YesPlease
HAVE_DEV_TTY = YesPlease
+   HAVE_SHM = YesPlease
 endif
 ifeq ($(uname_S),GNU/kFreeBSD)
NO_STRLCPY = YesPlease
diff --git a/read-cache--daemon.c b/read-cache--daemon.c
new file mode 100644
index 000..4531978
--- /dev/null
+++ b/read-cache--daemon.c
@@ -0,0 +1,207 @@
+#include cache.h
+#include sigchain.h
+#include unix-socket.h
+#include split-index.h
+#include pkt-line.h
+
+static char *socket_path;
+static struct strbuf shm_index = STRBUF_INIT;
+static struct strbuf shm_sharedindex = STRBUF_INIT;
+static struct strbuf shm_lock = STRBUF_INIT;
+static int lock_fd = -1;
+static int daemonized;
+
+static void cleanup_socket(void)
+{
+   if (daemonized)
+   return;
+   if (socket_path)
+   unlink(socket_path);
+   if (shm_index.len)
+   shm_unlink(shm_index.buf);
+   if (shm_sharedindex.len)
+   shm_unlink(shm_sharedindex.buf);
+   if (lock_fd != -1)
+   close(lock_fd);
+   if (shm_lock.len)
+   shm_unlink(shm_lock.buf);
+}
+
+static void cleanup_socket_on_signal(int sig)
+{
+   cleanup_socket();
+   sigchain_pop(sig);
+   raise(sig);
+}
+
+static int do_share_index(struct index_state *istate, struct strbuf *shm_path)
+{
+   struct strbuf sb = STRBUF_INIT;
+   void *map;
+   int fd;
+
+   strbuf_addf(sb, /git-index-%s, sha1_to_hex(istate-sha1));
+   fd = shm_open(sb.buf, O_RDWR | O_CREAT | O_EXCL, 0700);
+   if (fd  0)
+   return -1;
+   if (shm_path-len) {
+   

[PATCH 7/8] read-cache: skip verifying trailing SHA-1 on cached index

[Nguyễn Thái Ngọc Duy]

The daemon is responsible for verifying the index before putting it in
the shared memory. No need to redo it again.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 read-cache.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index 4041485..e98521f 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1332,6 +1332,8 @@ static int verify_hdr(struct cache_header *hdr, unsigned 
long size)
hdr_version = ntohl(hdr-hdr_version);
if (hdr_version  INDEX_FORMAT_LB || INDEX_FORMAT_UB  hdr_version)
return error(bad index version %d, hdr_version);
+   if (!size)
+   return 0;
git_SHA1_Init(c);
git_SHA1_Update(c, hdr, size - 20);
git_SHA1_Final(sha1, c);
@@ -1511,7 +1513,7 @@ int do_read_index(struct index_state *istate, const char 
*path, int must_exist)
struct stat st;
unsigned long src_offset;
struct cache_header *hdr;
-   void *mmap;
+   void *mmap, *old_mmap;
size_t mmap_size;
struct strbuf previous_name_buf = STRBUF_INIT, *previous_name;
 
@@ -1543,9 +1545,10 @@ int do_read_index(struct index_state *istate, const char 
*path, int must_exist)
}
close(fd);
 
+   old_mmap = mmap;
mmap = try_shm(mmap, mmap_size);
hdr = mmap;
-   if (verify_hdr(hdr, mmap_size)  0)
+   if (verify_hdr(hdr, old_mmap != mmap ? 0 : mmap_size)  0)
goto unmap;
 
hashcpy(istate-sha1, (const unsigned char *)hdr + mmap_size - 20);
-- 
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 8/8] read-cache: inform the daemon that the index has been updated

[Nguyễn Thái Ngọc Duy]

The daemon would immediately load the new index in memory in
background. Next time Git needs to read the index again, everything is
ready.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 cache.h  |  1 +
 read-cache.c | 53 -
 2 files changed, 49 insertions(+), 5 deletions(-)

diff --git a/cache.h b/cache.h
index fb29c7e..3115b86 100644
--- a/cache.h
+++ b/cache.h
@@ -483,6 +483,7 @@ extern int is_index_unborn(struct index_state *);
 extern int read_index_unmerged(struct index_state *);
 #define COMMIT_LOCK(1  0)
 #define CLOSE_LOCK (1  1)
+#define REFRESH_DAEMON (1  2)
 extern int write_locked_index(struct index_state *, struct lock_file *lock, 
unsigned flags);
 extern int discard_index(struct index_state *);
 extern int unmerged_index(const struct index_state *);
diff --git a/read-cache.c b/read-cache.c
index e98521f..d5c9247 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -16,6 +16,9 @@
 #include varint.h
 #include split-index.h
 #include sigchain.h
+#include unix-socket.h
+#include pkt-line.h
+#include run-command.h
 
 static struct cache_entry *refresh_cache_entry(struct cache_entry *ce,
   unsigned int options);
@@ -2030,6 +2033,32 @@ void set_alternate_index_output(const char *name)
alternate_index_output = name;
 }
 
+static void refresh_daemon(struct index_state *istate)
+{
+   int fd;
+   fd = unix_stream_connect(git_path(daemon/index));
+   if (fd  0) {
+   struct child_process cp;
+   const char *av[] = {read-cache--daemon, --detach, NULL };
+   memset(cp, 0, sizeof(cp));
+   cp.argv = av;
+   cp.git_cmd = 1;
+   cp.no_stdin = 1;
+   if (run_command(cp))
+   warning(_(failed to start read-cache--daemon: %s),
+   strerror(errno));
+   return;
+   }
+   /*
+* packet_write() could die() but unless this is from
+* update_index_if_able(), we're about to exit anyway,
+* probably ok to die (for now). Blocking mode is another
+* problem to deal with later.
+*/
+   packet_write(fd, refresh);
+   close(fd);
+}
+
 static int commit_locked_index(struct lock_file *lk)
 {
if (alternate_index_output) {
@@ -2052,9 +2081,22 @@ static int do_write_locked_index(struct index_state 
*istate, struct lock_file *l
return ret;
assert((flags  (COMMIT_LOCK | CLOSE_LOCK)) !=
   (COMMIT_LOCK | CLOSE_LOCK));
-   if (flags  COMMIT_LOCK)
-   return commit_locked_index(lock);
-   else if (flags  CLOSE_LOCK)
+   if (flags  COMMIT_LOCK) {
+   int ret;
+   int len = strlen(lock-filename) - 5; /* .lock */
+   if (!use_read_cache_daemon || len  6 ||
+   /*
+* do not wake the daemon when we update a temporary
+* index. This is not a perfect test for this, but good
+* enough.
+*/
+   strncmp(lock-filename + len - 6, /index, 6))
+   flags = ~REFRESH_DAEMON;
+   ret = commit_locked_index(lock);
+   if (!ret  use_read_cache_daemon)
+   refresh_daemon(istate);
+   return ret;
+   } else if (flags  CLOSE_LOCK)
return close_lock_file(lock);
else
return ret;
@@ -2066,7 +2108,7 @@ static int write_split_index(struct index_state *istate,
 {
int ret;
prepare_to_write_split_index(istate);
-   ret = do_write_locked_index(istate, lock, flags);
+   ret = do_write_locked_index(istate, lock, flags | REFRESH_DAEMON);
finish_writing_split_index(istate);
return ret;
 }
@@ -2133,7 +2175,8 @@ int write_locked_index(struct index_state *istate, struct 
lock_file *lock,
(istate-cache_changed  ~EXTMASK)) {
if (si)
hashclr(si-base_sha1);
-   return do_write_locked_index(istate, lock, flags);
+   return do_write_locked_index(istate, lock,
+flags | REFRESH_DAEMON);
}
 
if (getenv(GIT_TEST_SPLIT_INDEX)) {
-- 
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 9/8] even faster loading time with index version 254

[Nguyễn Thái Ngọc Duy]

This dirty (and likely buggy) patch shows a direction of lowering load
time even more. Basically the shared memory now contains a clean
memory dump that a git process could use with little preparation
(which also means it's tied to C Git, other implementations can't use
this)

Memory is actually shared, git won't malloc and copy over, so even if
the v254 is 235 MB (larger than v2 199MB), we use less memory.

With this patch, we can get as low as 256.442ms (compared to 663ms in
0/8) at 800 MHz, or 91ms at 2.5 GHz. Index load time should be a
solved problem.

But I'm not going to polish this patch and try to get it merged. I'd
rather see a real world repository of this size first to justify
messing up read-cache.c even more.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 cache.h  |   2 +
 read-cache--daemon.c |  31 +--
 read-cache.c | 154 ++-
 split-index.c|   3 +
 4 files changed, 149 insertions(+), 41 deletions(-)

diff --git a/cache.h b/cache.h
index c246dee..7f0ef1e 100644
--- a/cache.h
+++ b/cache.h
@@ -297,6 +297,8 @@ struct index_state {
unsigned char sha1[20];
void *mmap;
size_t mmap_size;
+   int mmap_fd;
+   void *(*allocate_254)(struct index_state *, size_t);
 };
 
 extern struct index_state the_index;
diff --git a/read-cache--daemon.c b/read-cache--daemon.c
index bd6d84f..a44bd09 100644
--- a/read-cache--daemon.c
+++ b/read-cache--daemon.c
@@ -34,10 +34,19 @@ static void cleanup_socket_on_signal(int sig)
raise(sig);
 }
 
+static void *allocate_254(struct index_state *istate, unsigned long size)
+{
+   ftruncate(istate-mmap_fd, size);
+   istate-mmap_size = size;
+   istate-mmap = xmmap(NULL, istate-mmap_size, PROT_READ | PROT_WRITE,
+MAP_SHARED, istate-mmap_fd, 0);
+   return istate-mmap != MAP_FAILED ? istate-mmap : NULL;
+}
+
+extern int do_write_index(struct index_state *istate, int newfd, int 
strip_extensions);
 static int do_share_index(struct index_state *istate, struct strbuf *shm_path)
 {
struct strbuf sb = STRBUF_INIT;
-   void *map;
int fd;
 
strbuf_addf(sb, /git-index-%s, sha1_to_hex(istate-sha1));
@@ -48,21 +57,16 @@ static int do_share_index(struct index_state *istate, 
struct strbuf *shm_path)
shm_unlink(shm_path-buf);
strbuf_reset(shm_path);
}
-   if (ftruncate(fd, istate-mmap_size)) {
-   close(fd);
-   shm_unlink(shm_path-buf);
-   return -1;
-   }
+   istate-version = 254;
+   istate-allocate_254 = allocate_254;
+   istate-mmap_fd = fd;
+   do_write_index(istate, -1, 0);
strbuf_addbuf(shm_path, sb);
-   map = xmmap(NULL, istate-mmap_size, PROT_READ | PROT_WRITE,
-   MAP_SHARED, fd, 0);
-   if (map == MAP_FAILED) {
+   if (istate-mmap == MAP_FAILED) {
close(fd);
shm_unlink(shm_path-buf);
return -1;
}
-   memcpy(map, istate-mmap, istate-mmap_size);
-   munmap(map, istate-mmap_size);
fchmod(fd, 0400);
close(fd);
return 0;
@@ -88,13 +92,9 @@ static void share_index(struct index_state *istate, struct 
strbuf *shm_path)
 
 static void refresh()
 {
-   the_index.keep_mmap = 1;
if (read_cache()  0)
die(could not read index);
share_index(the_index, shm_index);
-   if (the_index.split_index 
-   the_index.split_index-base)
-   share_index(the_index.split_index-base, shm_sharedindex);
discard_index(the_index);
 }
 
@@ -145,7 +145,6 @@ static void serve_cache(const char *socket_path, int detach)
if (fd  0)
die_errno(unable to bind to '%s', socket_path);
 
-   use_read_cache_daemon = -1;
refresh();
if (detach  daemonize(daemonized))
die_errno(unable to detach);
diff --git a/read-cache.c b/read-cache.c
index d5c9247..4db1c30 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -61,7 +61,8 @@ static void replace_index_entry(struct index_state *istate, 
int nr, struct cache
 
replace_index_entry_in_base(istate, old, ce);
remove_name_hash(istate, old);
-   free(old);
+   if (old-index != 0x) /* special mark by v254 entry writing 
code */
+   free(old);
set_index_entry(istate, nr, ce);
ce-ce_flags |= CE_UPDATE_IN_BASE;
istate-cache_changed |= CE_ENTRY_CHANGED;
@@ -1333,9 +1334,11 @@ static int verify_hdr(struct cache_header *hdr, unsigned 
long size)
if (hdr-hdr_signature != htonl(CACHE_SIGNATURE))
return error(bad signature);
hdr_version = ntohl(hdr-hdr_version);
-   if (hdr_version  INDEX_FORMAT_LB || INDEX_FORMAT_UB  hdr_version)
+   if (!size  hdr_version == 254)
+   fprintf(stderr, yeah\n);  /* go on */
+   

Re: [PATCH 2/3] Add read-cache--daemon

[Erik Faye-Lund]

On Tue, May 13, 2014 at 1:15 PM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
 diff --git a/Makefile b/Makefile
 index 028749b..98d22de 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -1502,6 +1502,12 @@ ifdef HAVE_DEV_TTY
 BASIC_CFLAGS += -DHAVE_DEV_TTY
  endif

 +ifdef HAVE_SHM
 +   BASIC_CFLAGS += -DHAVE_SHM
 +   EXTLIBS += -lrt
 +   PROGRAM_OBJS += read-cache--daemon.o
 +endif
 +

I think read-cache--daemon will fail in case of NO_UNIX_SOCKETS.

But, read-cache--daemon.c only gets compiled if we have shm_open...

 diff --git a/git-compat-util.h b/git-compat-util.h
 index f6d3a46..b2116ab 100644
 --- a/git-compat-util.h
 +++ b/git-compat-util.h
 @@ -723,4 +723,12 @@ struct tm *git_gmtime_r(const time_t *, struct tm *);
  #define gmtime_r git_gmtime_r
  #endif

 +#ifndef HAVE_SHM
 +static inline int shm_open(const char *path, int flags, int mode)
 +{
 +   errno = ENOSYS;
 +   return -1;
 +}
 +#endif


...yet, you introduce a compatibility-shim...

 diff --git a/read-cache--daemon.c b/read-cache--daemon.c
 new file mode 100644
 index 000..52b4067
 --- /dev/null
 +++ b/read-cache--daemon.c
 @@ -0,0 +1,167 @@
 +#include cache.h
 +#include sigchain.h
 +#include unix-socket.h
 +#include split-index.h
 +#include pkt-line.h
 +
 +static char *socket_path;
 +static struct strbuf shm_index = STRBUF_INIT;
 +static struct strbuf shm_sharedindex = STRBUF_INIT;
 +
 +static void cleanup_socket(void)
 +{
 +   if (socket_path)
 +   unlink(socket_path);
 +   if (shm_index.len)
 +   shm_unlink(shm_index.buf);
 +   if (shm_sharedindex.len)
 +   shm_unlink(shm_sharedindex.buf);
 +}
 +
 +static void cleanup_socket_on_signal(int sig)
 +{
 +   cleanup_socket();
 +   sigchain_pop(sig);
 +   raise(sig);
 +}
 +
 +static void share_index(struct index_state *istate, struct strbuf *shm_path)
 +{
 +   struct strbuf sb = STRBUF_INIT;
 +   void *map;
 +   int fd;
 +
 +   strbuf_addf(sb, /git-index-%s, sha1_to_hex(istate-sha1));
 +   if (shm_path-len  strcmp(sb.buf, shm_path-buf)) {
 +   shm_unlink(shm_path-buf);
 +   strbuf_reset(shm_path);
 +   }
 +   fd = shm_open(sb.buf, O_RDWR | O_CREAT | O_TRUNC, 0700);
 +   if (fd  0)
 +   return;

...that only gets called from read-cache--daemon.c, which already only
gets compiled if we have open?
--
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 4/8] Add read-cache--daemon for caching index and related stuff

[Erik Faye-Lund]

On Tue, May 13, 2014 at 1:15 PM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
 diff --git a/Documentation/git-read-cache--daemon.txt 
 b/Documentation/git-read-cache--daemon.txt
 new file mode 100644
 index 000..1b05be4
 --- /dev/null
 +++ b/Documentation/git-read-cache--daemon.txt
 @@ -0,0 +1,27 @@
 +git-read-cache--daemon(1)
 +=
 +
 +NAME
 +
 +git-daemon - A simple cache server for speeding up index file access
 +
 +SYNOPSIS
 +
 +[verse]
 +'git daemon' [--detach]
 +

Huh, git daemon can't be right...

 diff --git a/Makefile b/Makefile
 index d0a2b4b..a44ab0b 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -1504,6 +1504,12 @@ ifdef HAVE_DEV_TTY
 BASIC_CFLAGS += -DHAVE_DEV_TTY
  endif

 +ifdef HAVE_SHM
 +   BASIC_CFLAGS += -DHAVE_SHM
 +   EXTLIBS += -lrt
 +   PROGRAM_OBJS += read-cache--daemon.o
 +endif

I think this also needs to be protected against NO_UNIX_SOCKETS
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/8] unix-socket: stub impl. for platforms with no unix socket support

[Erik Faye-Lund]

On Tue, May 13, 2014 at 1:15 PM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
 With this we can make unix_stream_* calls without #ifdef.

 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---
  Makefile  |  2 ++
  unix-socket.h | 18 ++
  2 files changed, 20 insertions(+)

 diff --git a/Makefile b/Makefile
 index 028749b..d0a2b4b 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -1417,6 +1417,8 @@ ifndef NO_UNIX_SOCKETS
 LIB_H += unix-socket.h
 PROGRAM_OBJS += credential-cache.o
 PROGRAM_OBJS += credential-cache--daemon.o
 +else
 +   BASIC_CFLAGS += -DNO_UNIX_SOCKETS
  endif

  ifdef NO_ICONV
 diff --git a/unix-socket.h b/unix-socket.h
 index e271aee..f1cba70 100644
 --- a/unix-socket.h
 +++ b/unix-socket.h
 @@ -1,7 +1,25 @@
  #ifndef UNIX_SOCKET_H
  #define UNIX_SOCKET_H

 +#ifndef NO_UNIX_SOCKETS
 +
  int unix_stream_connect(const char *path);
  int unix_stream_listen(const char *path);

 +#else
 +
 +static inline int unix_stream_connect(const char *path)
 +{
 +   errno = ENOSYS;
 +   return -1;
 +}
 +
 +static inline int unix_stream_listen(const char *path)
 +{
 +   errno = ENOSYS;
 +   return -1;
 +}
 +
 +#endif
 +
  #endif /* UNIX_SOCKET_H */

OK, so I missed this before my other two comments. But still... in
what way does errno=ENOSYS make this *work*? Won't we end up compiling
lots of non-functional tools on Windows in this case?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] Add read-cache--daemon

[Duy Nguyen]

On Tue, May 13, 2014 at 6:52 PM, Erik Faye-Lund kusmab...@gmail.com wrote:
 On Tue, May 13, 2014 at 1:15 PM, Nguyễn Thái Ngọc Duy pclo...@gmail.com 
 wrote:
 diff --git a/Makefile b/Makefile
 index 028749b..98d22de 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -1502,6 +1502,12 @@ ifdef HAVE_DEV_TTY
 BASIC_CFLAGS += -DHAVE_DEV_TTY
  endif

 +ifdef HAVE_SHM
 +   BASIC_CFLAGS += -DHAVE_SHM
 +   EXTLIBS += -lrt
 +   PROGRAM_OBJS += read-cache--daemon.o
 +endif
 +

 I think read-cache--daemon will fail in case of NO_UNIX_SOCKETS.

 But, read-cache--daemon.c only gets compiled if we have shm_open...

Portability is something to be sorted out.Ideally we should not build
this unless we have both unix socket and shared memory support. On
Windows, I'm not sure how much code can be shared, or it'll be a
completely different program. In that case maybe this program should
be read-cache--daemon-posix (at least the .c file name, the binary may
be still git-read-cache--daemon) or something and the Windows version
read-cache--daemon-windows..
-- 
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 2/8] unix-socket: stub impl. for platforms with no unix socket support

[Erik Faye-Lund]

On Tue, May 13, 2014 at 1:59 PM, Erik Faye-Lund kusmab...@gmail.com wrote:
 On Tue, May 13, 2014 at 1:15 PM, Nguyễn Thái Ngọc Duy pclo...@gmail.com 
 wrote:
 With this we can make unix_stream_* calls without #ifdef.

 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---
  Makefile  |  2 ++
  unix-socket.h | 18 ++
  2 files changed, 20 insertions(+)

 diff --git a/Makefile b/Makefile
 index 028749b..d0a2b4b 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -1417,6 +1417,8 @@ ifndef NO_UNIX_SOCKETS
 LIB_H += unix-socket.h
 PROGRAM_OBJS += credential-cache.o
 PROGRAM_OBJS += credential-cache--daemon.o
 +else
 +   BASIC_CFLAGS += -DNO_UNIX_SOCKETS
  endif

  ifdef NO_ICONV
 diff --git a/unix-socket.h b/unix-socket.h
 index e271aee..f1cba70 100644
 --- a/unix-socket.h
 +++ b/unix-socket.h
 @@ -1,7 +1,25 @@
  #ifndef UNIX_SOCKET_H
  #define UNIX_SOCKET_H

 +#ifndef NO_UNIX_SOCKETS
 +
  int unix_stream_connect(const char *path);
  int unix_stream_listen(const char *path);

 +#else
 +
 +static inline int unix_stream_connect(const char *path)
 +{
 +   errno = ENOSYS;
 +   return -1;
 +}
 +
 +static inline int unix_stream_listen(const char *path)
 +{
 +   errno = ENOSYS;
 +   return -1;
 +}
 +
 +#endif
 +
  #endif /* UNIX_SOCKET_H */

 OK, so I missed this before my other two comments. But still... in
 what way does errno=ENOSYS make this *work*? Won't we end up compiling
 lots of non-functional tools on Windows in this case?

ENOSYS makes git-credential-cache.c just die with the message unable
to start cache daemon, and git-credential--daemon.c die with unable
to bind to socket_path.
--
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 5/8] read-cache: try index data from shared memory

[Erik Faye-Lund]

On Tue, May 13, 2014 at 1:15 PM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---
  Documentation/config.txt |  4 
  cache.h  |  1 +
  config.c | 12 
  environment.c|  1 +
  read-cache.c | 43 +++
  submodule.c  |  1 +
  6 files changed, 62 insertions(+)

 diff --git a/Documentation/config.txt b/Documentation/config.txt
 index d8b6cc9..ccbe00b 100644
 --- a/Documentation/config.txt
 +++ b/Documentation/config.txt
 @@ -617,6 +617,10 @@ relatively high IO latencies.  With this set to 'true', 
 Git will do the
  index comparison to the filesystem data in parallel, allowing
  overlapping IO's.

 +core.useReadCacheDaemon::
 +   Use `git read-cache--daemon` to speed up index reading. See
 +   linkgit:git-read-cache--daemon for more information.
 +
  core.createObject::
 You can set this to 'link', in which case a hardlink followed by
 a delete of the source are used to make sure that object creation
 diff --git a/cache.h b/cache.h
 index d0ff11c..fb29c7e 100644
 --- a/cache.h
 +++ b/cache.h
 @@ -603,6 +603,7 @@ extern size_t packed_git_limit;
  extern size_t delta_base_cache_limit;
  extern unsigned long big_file_threshold;
  extern unsigned long pack_size_limit_cfg;
 +extern int use_read_cache_daemon;

  /*
   * Do replace refs need to be checked this run?  This variable is
 diff --git a/config.c b/config.c
 index a30cb5c..5c832ad 100644
 --- a/config.c
 +++ b/config.c
 @@ -874,6 +874,18 @@ static int git_default_core_config(const char *var, 
 const char *value)
 return 0;
 }

 +#ifdef HAVE_SHM
 +   /*
 +* Currently git-read-cache--daemon is only built when
 +* HAVE_SHM is set. Ignore user settings if HAVE_SHM is not
 +* defined.
 +*/
 +   if (!strcmp(var, core.usereadcachedaemon)) {
 +   use_read_cache_daemon = git_config_bool(var, value);
 +   return 0;
 +   }
 +#endif
 +
 /* Add other config variables here and to Documentation/config.txt. */
 return 0;
  }
 diff --git a/environment.c b/environment.c
 index 5c4815d..b76a414 100644
 --- a/environment.c
 +++ b/environment.c
 @@ -63,6 +63,7 @@ int merge_log_config = -1;
  int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
  struct startup_info *startup_info;
  unsigned long pack_size_limit_cfg;
 +int use_read_cache_daemon;

  /*
   * The character that begins a commented line in user-editable file
 diff --git a/read-cache.c b/read-cache.c
 index a5031f3..0e46523 100644
 --- a/read-cache.c
 +++ b/read-cache.c
 @@ -1462,6 +1462,48 @@ static struct cache_entry *create_from_disk(struct 
 ondisk_cache_entry *ondisk,
 return ce;
  }

 +static void *try_shm(void *mmap, size_t *mmap_size)
 +{
 +   struct strbuf sb = STRBUF_INIT;
 +   size_t old_size = *mmap_size;
 +   void *new_mmap;
 +   struct stat st;
 +   int fd;
 +
 +   if (old_size = 20 || !use_read_cache_daemon)
 +   return mmap;
 +
 +   strbuf_addf(sb, /git-index-%s.lock,
 +   sha1_to_hex((unsigned char *)mmap + old_size - 20));
 +   fd = shm_open(sb.buf, O_RDONLY, 0777);

OK, so *here* you do an unguarded use of shm_open, but the code never
gets executed because of the HAVE_SHM guard in
git_default_core_config. Perhaps not introduce the compatibility shim
until this patch, then?
--
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 8/8] read-cache: inform the daemon that the index has been updated

[Erik Faye-Lund]

On Tue, May 13, 2014 at 1:15 PM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
 The daemon would immediately load the new index in memory in
 background. Next time Git needs to read the index again, everything is
 ready.

 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---
  cache.h  |  1 +
  read-cache.c | 53 -
  2 files changed, 49 insertions(+), 5 deletions(-)

 diff --git a/cache.h b/cache.h
 index fb29c7e..3115b86 100644
 --- a/cache.h
 +++ b/cache.h
 @@ -483,6 +483,7 @@ extern int is_index_unborn(struct index_state *);
  extern int read_index_unmerged(struct index_state *);
  #define COMMIT_LOCK(1  0)
  #define CLOSE_LOCK (1  1)
 +#define REFRESH_DAEMON (1  2)
  extern int write_locked_index(struct index_state *, struct lock_file *lock, 
 unsigned flags);
  extern int discard_index(struct index_state *);
  extern int unmerged_index(const struct index_state *);
 diff --git a/read-cache.c b/read-cache.c
 index e98521f..d5c9247 100644
 --- a/read-cache.c
 +++ b/read-cache.c
 @@ -16,6 +16,9 @@
  #include varint.h
  #include split-index.h
  #include sigchain.h
 +#include unix-socket.h
 +#include pkt-line.h
 +#include run-command.h

  static struct cache_entry *refresh_cache_entry(struct cache_entry *ce,
unsigned int options);
 @@ -2030,6 +2033,32 @@ void set_alternate_index_output(const char *name)
 alternate_index_output = name;
  }

 +static void refresh_daemon(struct index_state *istate)
 +{
 +   int fd;
 +   fd = unix_stream_connect(git_path(daemon/index));
 +   if (fd  0) {
 +   struct child_process cp;
 +   const char *av[] = {read-cache--daemon, --detach, NULL };
 +   memset(cp, 0, sizeof(cp));
 +   cp.argv = av;
 +   cp.git_cmd = 1;
 +   cp.no_stdin = 1;
 +   if (run_command(cp))
 +   warning(_(failed to start read-cache--daemon: %s),
 +   strerror(errno));
 +   return;
 +   }
 +   /*
 +* packet_write() could die() but unless this is from
 +* update_index_if_able(), we're about to exit anyway,
 +* probably ok to die (for now). Blocking mode is another
 +* problem to deal with later.
 +*/
 +   packet_write(fd, refresh);
 +   close(fd);
 +}
 +

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

Re: [PATCH 2/3] Add read-cache--daemon

[Duy Nguyen]

What do you think is a good replacement for unix socket on Windows?
It's only used to refresh the cache in the daemon, no sensitive data
sent over, so security is not a problem. I'm thinking maybe just
TCP/IP server, but that's going to be a system-wide daemon.. Perhaps
the windows daemon could just monitor $GIT_DIR/index and refresh it?
-- 
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 2/2] wincred: avoid overwriting configured variables

[Felipe Contreras]

On Tue, May 13, 2014 at 1:34 AM, Erik Faye-Lund kusmab...@gmail.com wrote:
 On Tue, May 13, 2014 at 8:01 AM, Stepan Kasal ka...@ucw.cz wrote:
 From: Pat Thoyts pattho...@users.sourceforge.net
 Date: Wed, 24 Oct 2012 00:15:29 +0100

 Signed-off-by: Pat Thoyts pattho...@users.sourceforge.net
 Signed-off-by: Stepan Kasal ka...@ucw.cz
 ---
  contrib/credential/wincred/Makefile | 4 
  1 file changed, 4 deletions(-)

 diff --git a/contrib/credential/wincred/Makefile 
 b/contrib/credential/wincred/Makefile
 index 39fa5e0..e64cd9a 100644
 --- a/contrib/credential/wincred/Makefile
 +++ b/contrib/credential/wincred/Makefile
 @@ -1,9 +1,5 @@
  all: git-credential-wincred.exe

 -CC = gcc
 -RM = rm -f
 -CFLAGS = -O2 -Wall
 -

 Would it be better to set these if not already set, i.e:

 -CC = gcc
 -RM = rm -f
 -CFLAGS = -O2 -Wall
 +CC ?= gcc
 +RM ?= rm -f
 +CFLAGS ?= -O2 -Wall

 instead?

Set by whom? If it's by the environment you should be running 'make -e'.

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

[PATCH] contrib: completion: fix 'eread()' namespace

[Felipe Contreras]

Otherwise it might collide with a function of the same name in the
user's environment.

Suggested-by: SZEDER Gábor sze...@ira.uka.de
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 contrib/completion/git-prompt.sh | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 96b8087..853425d 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -270,7 +270,7 @@ __git_ps1_colorize_gitstring ()
r=$c_clear$r
 }
 
-eread ()
+__git_eread ()
 {
f=$1
shift
@@ -339,9 +339,9 @@ __git_ps1 ()
local step=
local total=
if [ -d $g/rebase-merge ]; then
-   eread $g/rebase-merge/head-name b
-   eread $g/rebase-merge/msgnum step
-   eread $g/rebase-merge/end total
+   __git_eread $g/rebase-merge/head-name b
+   __git_eread $g/rebase-merge/msgnum step
+   __git_eread $g/rebase-merge/end total
if [ -f $g/rebase-merge/interactive ]; then
r=|REBASE-i
else
@@ -349,10 +349,10 @@ __git_ps1 ()
fi
else
if [ -d $g/rebase-apply ]; then
-   eread $g/rebase-apply/next step
-   eread $g/rebase-apply/last total
+   __git_eread $g/rebase-apply/next step
+   __git_eread $g/rebase-apply/last total
if [ -f $g/rebase-apply/rebasing ]; then
-   eread $g/rebase-apply/head-name b
+   __git_eread $g/rebase-apply/head-name b
r=|REBASE
elif [ -f $g/rebase-apply/applying ]; then
r=|AM
@@ -376,7 +376,7 @@ __git_ps1 ()
b=$(git symbolic-ref HEAD 2/dev/null)
else
local head=
-   if ! eread $g/HEAD head; then
+   if ! __git_eread $g/HEAD head; then
if [ $pcmode = yes ]; then
PS1=$ps1pc_start$ps1pc_end
fi
-- 
1.9.3+fc1

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

Re: [PATCH 2/3] Add read-cache--daemon

[Erik Faye-Lund]

On Tue, May 13, 2014 at 3:01 PM, Duy Nguyen pclo...@gmail.com wrote:
 What do you think is a good replacement for unix socket on Windows?
 It's only used to refresh the cache in the daemon, no sensitive data
 sent over, so security is not a problem. I'm thinking maybe just
 TCP/IP server, but that's going to be a system-wide daemon.. Perhaps
 the windows daemon could just monitor $GIT_DIR/index and refresh it?

Windows has support for Named Pipes, which seems like the right kind
of communication channel. However, the programming model differs quite
a bit from unix-sockets:

http://msdn.microsoft.com/en-us/library/windows/desktop/aa365594%28v=vs.85%29.aspx
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] Add read-cache--daemon

[Duy Nguyen]

On Tue, May 13, 2014 at 8:37 PM, Erik Faye-Lund kusmab...@gmail.com wrote:
 On Tue, May 13, 2014 at 3:01 PM, Duy Nguyen pclo...@gmail.com wrote:
 What do you think is a good replacement for unix socket on Windows?
 It's only used to refresh the cache in the daemon, no sensitive data
 sent over, so security is not a problem. I'm thinking maybe just
 TCP/IP server, but that's going to be a system-wide daemon.. Perhaps
 the windows daemon could just monitor $GIT_DIR/index and refresh it?

 Windows has support for Named Pipes, which seems like the right kind
 of communication channel. However, the programming model differs quite
 a bit from unix-sockets:

 http://msdn.microsoft.com/en-us/library/windows/desktop/aa365594%28v=vs.85%29.aspx

Yeah that was my first option, but if code cannot be shared to
differences then we probably should go another way. The old
FindWindow/PostMessage still works with modern Windows, right? Maybe
we could create a window with a name derived from the daemon's pid and
save the name in the index, then PostMessage can signal the daemon. On
the UNIX front, we store pid and send SIGUSR1 instead..The good thing
here is the Git side will be very simple (PostMessage vs kill).
-- 
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 2/2] wincred: avoid overwriting configured variables

[Stepan Kasal]

Hello,

On Tue, May 13, 2014 at 08:18:26AM -0500, Felipe Contreras wrote:
 Set by whom? If it's by the environment you should be running 'make -e'.

... or on command line (through recursive make, prehaps).
But these both take precedence over makefile assignments.

Another issue is the interaction with included makefile fragments.
Actually, both
CC = gcc
-include ../../../config.mak
and
-include ../../../config.mak
CC ?= gcc
are very close.  (They would differ if config.mak contained ?=.)

I was confused by the former, but perhaps it's only me.

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

Re: [PATCH 2/3] Add read-cache--daemon

[Erik Faye-Lund]

On Tue, May 13, 2014 at 3:49 PM, Duy Nguyen pclo...@gmail.com wrote:
 On Tue, May 13, 2014 at 8:37 PM, Erik Faye-Lund kusmab...@gmail.com wrote:
 On Tue, May 13, 2014 at 3:01 PM, Duy Nguyen pclo...@gmail.com wrote:
 What do you think is a good replacement for unix socket on Windows?
 It's only used to refresh the cache in the daemon, no sensitive data
 sent over, so security is not a problem. I'm thinking maybe just
 TCP/IP server, but that's going to be a system-wide daemon.. Perhaps
 the windows daemon could just monitor $GIT_DIR/index and refresh it?

 Windows has support for Named Pipes, which seems like the right kind
 of communication channel. However, the programming model differs quite
 a bit from unix-sockets:

 http://msdn.microsoft.com/en-us/library/windows/desktop/aa365594%28v=vs.85%29.aspx

 Yeah that was my first option, but if code cannot be shared to
 differences then we probably should go another way. The old
 FindWindow/PostMessage still works with modern Windows, right? Maybe
 we could create a window with a name derived from the daemon's pid and
 save the name in the index, then PostMessage can signal the daemon. On
 the UNIX front, we store pid and send SIGUSR1 instead..The good thing
 here is the Git side will be very simple (PostMessage vs kill).

Hmmm I'm a bit worried about having to load in USER32.DLL just to
read the cache that way. But it seems we already do that, thanks to
compat/poll/poll.c (it depends on DispatchMessage,
MsgWaitForMultipleObjects, PeekMessage and TranslateMessage, all from
that DLL).

Preferably, we should delay-load USER32.DLL in compat/poll/poll.c, but
if we start needing it for the reading the index, it'll be loaded by
the vast majority of processes anyway.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] Add read-cache--daemon

[Duy Nguyen]

On Tue, May 13, 2014 at 9:06 PM, Erik Faye-Lund kusmab...@gmail.com wrote:
 On Tue, May 13, 2014 at 3:49 PM, Duy Nguyen pclo...@gmail.com wrote:
 On Tue, May 13, 2014 at 8:37 PM, Erik Faye-Lund kusmab...@gmail.com wrote:
 On Tue, May 13, 2014 at 3:01 PM, Duy Nguyen pclo...@gmail.com wrote:
 What do you think is a good replacement for unix socket on Windows?
 It's only used to refresh the cache in the daemon, no sensitive data
 sent over, so security is not a problem. I'm thinking maybe just
 TCP/IP server, but that's going to be a system-wide daemon.. Perhaps
 the windows daemon could just monitor $GIT_DIR/index and refresh it?

 Windows has support for Named Pipes, which seems like the right kind
 of communication channel. However, the programming model differs quite
 a bit from unix-sockets:

 http://msdn.microsoft.com/en-us/library/windows/desktop/aa365594%28v=vs.85%29.aspx

 Yeah that was my first option, but if code cannot be shared to
 differences then we probably should go another way. The old
 FindWindow/PostMessage still works with modern Windows, right? Maybe
 we could create a window with a name derived from the daemon's pid and
 save the name in the index, then PostMessage can signal the daemon. On
 the UNIX front, we store pid and send SIGUSR1 instead..The good thing
 here is the Git side will be very simple (PostMessage vs kill).

 Hmmm I'm a bit worried about having to load in USER32.DLL just to
 read the cache that way. But it seems we already do that, thanks to
 compat/poll/poll.c (it depends on DispatchMessage,
 MsgWaitForMultipleObjects, PeekMessage and TranslateMessage, all from
 that DLL).

 Preferably, we should delay-load USER32.DLL in compat/poll/poll.c, but
 if we start needing it for the reading the index, it'll be loaded by
 the vast majority of processes anyway.

Thanks for the info. I'll see if we can still stick to named pipes. If
we have to load user32.dll, hopefully the gain will outweigh load time
for that dell.
-- 
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 2/3] Add read-cache--daemon

[Erik Faye-Lund]

On Tue, May 13, 2014 at 4:10 PM, Duy Nguyen pclo...@gmail.com wrote:
 On Tue, May 13, 2014 at 9:06 PM, Erik Faye-Lund kusmab...@gmail.com wrote:
 On Tue, May 13, 2014 at 3:49 PM, Duy Nguyen pclo...@gmail.com wrote:
 On Tue, May 13, 2014 at 8:37 PM, Erik Faye-Lund kusmab...@gmail.com wrote:
 On Tue, May 13, 2014 at 3:01 PM, Duy Nguyen pclo...@gmail.com wrote:
 What do you think is a good replacement for unix socket on Windows?
 It's only used to refresh the cache in the daemon, no sensitive data
 sent over, so security is not a problem. I'm thinking maybe just
 TCP/IP server, but that's going to be a system-wide daemon.. Perhaps
 the windows daemon could just monitor $GIT_DIR/index and refresh it?

 Windows has support for Named Pipes, which seems like the right kind
 of communication channel. However, the programming model differs quite
 a bit from unix-sockets:

 http://msdn.microsoft.com/en-us/library/windows/desktop/aa365594%28v=vs.85%29.aspx

 Yeah that was my first option, but if code cannot be shared to
 differences then we probably should go another way. The old
 FindWindow/PostMessage still works with modern Windows, right? Maybe
 we could create a window with a name derived from the daemon's pid and
 save the name in the index, then PostMessage can signal the daemon. On
 the UNIX front, we store pid and send SIGUSR1 instead..The good thing
 here is the Git side will be very simple (PostMessage vs kill).

 Hmmm I'm a bit worried about having to load in USER32.DLL just to
 read the cache that way. But it seems we already do that, thanks to
 compat/poll/poll.c (it depends on DispatchMessage,
 MsgWaitForMultipleObjects, PeekMessage and TranslateMessage, all from
 that DLL).

 Preferably, we should delay-load USER32.DLL in compat/poll/poll.c, but
 if we start needing it for the reading the index, it'll be loaded by
 the vast majority of processes anyway.

 Thanks for the info. I'll see if we can still stick to named pipes. If
 we have to load user32.dll, hopefully the gain will outweigh load time
 for that dell.

I just timed it here on my system, and omitting USER32.DLL didn't gain
anything for git --version, so I suspect I was worrying too soon.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/8] Speed up cache loading time

[Stefan Beller]

 That is clocked at 800 MHz. A repository at this size deserves a
 better CPU. At 2.5 GHz we spend 183.228ms on loading the index. A
 reasonable number to me. If we scale other parts of git-status as well
 as this, we should be able to make git status within 1 or 2 seconds.


Which harddrive do you use? Traditional or SSDs?
Does have harddrive loading time significant impact here? (just a
guess/question)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/8] Speed up cache loading time

[Duy Nguyen]

On Tue, May 13, 2014 at 9:24 PM, Stefan Beller stefanbel...@gmail.com wrote:
 That is clocked at 800 MHz. A repository at this size deserves a
 better CPU. At 2.5 GHz we spend 183.228ms on loading the index. A
 reasonable number to me. If we scale other parts of git-status as well
 as this, we should be able to make git status within 1 or 2 seconds.


 Which harddrive do you use? Traditional or SSDs?

Traditional

 Does have harddrive loading time significant impact here? (just a
 guess/question)

In the hot cache case, I assume the index stays in OS cache anyway so
hard drive should not impact much (the other parts of git-status like
index refresh or untracked file listing is a different story and some
may fall out of cache). My laptop has 4G ram, with my repeated tests,
I guess the index (even 200mb) stayed in the cache (but did not really
verify it).
-- 
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: Error using git-remote-hg

[Torsten Bögershausen]

(Please use reply-all and don't snip the important stuff)
On 2014-05-13 09.54, Charles Brossollet wrote:
 Le 12 mai 2014 à 21:37, Felipe Contreras felipe.contre...@gmail.com a écrit 
 :
 
 Torsten Bögershausen wrote:
 I'm using git 1.9.3 on Mac OS X 10.9.2, with hg 3.0 installed with brew.

 It used to work before, on this same repository, since then git and hg 
 were both upgraded.
 In short: The remote helper of Git 1.9.3 is not compatible with hg 3.0
 You can eiher downgrade hg, or rebuild Git and cherry-pick this commit:


 No need to rebuild Git.

 You can also use the latest version:
 https://github.com/felipec/git-remote-hg
 
 Thanks Felipe and Torsten, just using the HEAD version of git-remote-hg 
 solved the problem.
 
 Best regards,
 Charles--

I did some testing with Git 2.0-rc3 + 58aee0864adeeb5363f.
The remote-helper tests for hg-git worked OK
with both hg version 2.9 and 3.0 under both Mac OS and Linux.

Should we consider 58aee086 to be included in Git 2.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

Re: [PATCH] git format-patch --signature string | file

[Jonathan Nieder]

Hi,

Jeremiah Mahler wrote:

   # from a string
   $ git format-patch --signature from a string origin

   # or from a file
   $ git format-patch --signature ~/.signature origin

Interesting.  But... what if I want my patch to end with

-- 
/home/jrnieder/.signature

?  It seems safer to introduce a separate --signature-file option.

[...]
  builtin/log.c | 26 --
  1 file changed, 24 insertions(+), 2 deletions(-)

Tests?

Thanks and hope that helps,
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: Error using git-remote-hg

[Junio C Hamano]

Torsten Bögershausen tbo...@web.de writes:

 I did some testing with Git 2.0-rc3 + 58aee0864adeeb5363f.
 The remote-helper tests for hg-git worked OK
 with both hg version 2.9 and 3.0 under both Mac OS and Linux.

 Should we consider 58aee086 to be included in Git 2.0 ?

It is way too late for Git 2.0, unless we are willing to pretend
that it is before 2.0-rc1, and cut one rc release tarball or two and
delay the final release by a few weeks.  And I tested it with 2.9
and 3.0 is not the kind of test that is sufficient before the
release (e.g. my desktop seems to have 2.0.2-1ubuntu1); it is the
kind of test we want to see before the patch is sent to the list,
which I know Felipe did.

A clarification for new people might be in order, as there seem to
be some confusions in some recent threads.

Code freeze in preparation for the next release is *not* the time to
test new code.  It is time to catch regressions by asking wider
audiences who do not normally follow Git development (i.e. those who
are not the ones that follow 'master' and rebuild/install it once or
twice a week for their daily use).

These people (and I am one of them for other projects whose products
I use myself and whose development I do not follow) may be curious
what will be in the upcoming release, but what they care more about
is that the upcoming release will *not* break their established
usage.  If the version of Git they currently run allows them to do
something, and if the upcoming release stops them from doing that
thing they care, that will prevent them from using the new version
of Git.

That is what we want to catch by announcing release candidate
tarballs; in exchange for giving an early access to those who do not
always live on the edge by having a clone of git.git and following
its development, we want them to catch regressions and report to us,
so that we can fix (and the fix often is to revert when deep in
the release candidate cycles).

On the other hand, if their current Git did not allow them to do
something, and if the new version still does not, it is unfortunate
but it is the same flaw they have been living with.

My understanding is that versions of remote-hg shipped with all
versions of Git did not work with Hg 3.0, so 58aee08 is not a
regression fix at all.  Is working with Hg 3.0 at such a high
priority that it makes it worth to slip the whole release schedule
by a few weeks?  I do not think so.

Another thing to consider is that fewer and fewer people test later
release candidates, so issuing a 2.0-rc4 tarball that wasn't
scheduled and giving it a soak time of two weeks will not get as
wide a testing as we would get for 2.0-rc0/rc1.

So the answer to your question is an unambiguous no.

It is a different matter if we want a change to allow us to operate
with both older and newer version of mercurial in a release of Git
in near future.  The answer is a resounding yes, even if the
solution may not be the exact 58aee0864 commit [*2*].  I think an
update should eventurally go to the maintenance tracks of even older
releases, perhaps maint-1.8.5 and upwards, after we merge it to
'master' in preparation for the feature release that comes after Git
2.0, which probably will be called 2.1 but do not quote me on that.

Regarding the commit in question, I asked Felipe a question and
never got a straight answer.

Why do we import changegroup unconditionally, even though it
is only used in the new codepath meant only for version 3.0 or
higher, not inside the if block that decides if we need that
module?

I had a few more questions in mind that I didn't have a chance to
ask, and the commit log message did not explain.

Is the reason why this fix is needed is because repo stopped
being a way to ask .getbundle() and in the new world order
changegroup is the new way to ask .getbundle()?

If the answer is yes, then asking are we with 3.0 or
later---if so ask changegroup for .getbundle()?, which is this
patch does, may not be the right condition to trigger the new
codepath.  Does repo know how to do .getbundle()?  If not,
import changegroup and ask that module to perform .getbundle()
instead may be a way to base the decision on a more directly
relevant condition.

Answers to these questions, if they came, were meant to convince
myself that the patch 58aee0864 is the best solution to the problem,
but because I only got Of course it is not a mistake [*1*], seeing
it did not lead to a productive discussion, I gave up.  So I haven't
even managed to convince myself that that commit is the best
solution to the problem.


[References]

*1* http://thread.gmane.org/gmane.comp.version-control.git/248063/focus=248348

*2*
commit 58aee0864adeeb5363f2a06728596f9c9315811f
Author: Felipe Contreras felipe.contre...@gmail.com
Date:   Sat May 3 21:16:54 2014 -0500

remote-hg: add support for hg v3.0

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
Signed-off-by: Junio C 

Re: [PATCH v1 1/2] Remove 'git archimport'

[Martin Langhoff]

On Thu, May 8, 2014 at 9:33 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 No updates since 2010, and no tests.

NAK.

IMHO, this is quite unfriendly.

Is this removal based on your opinion, or Junio's position (or
consensus from maintainers from the list)? If there is a clear
consensus or direction for old code such as this, please let me know
(but copy martin.langh...@gmail.com, not just my very old address!).

 Plus, foreign SCM tools should live out-of-tree anyway.

Says who? Is there consensus on this?

It's generally the privilege of the maintainer -- in this case Junio
or perhaps Linus -- to take harsh stances like this.

Junio, what's your position?



m
-- 
 mar...@laptop.org
 - ask interesting questions
 - don't get distracted with shiny stuff  - working code first
 - http://wiki.laptop.org/go/User:Martinlanghoff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 01/17] contrib: remove outdated README

[Martin Langhoff]

On Fri, May 9, 2014 at 5:54 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 You are once more twisting the sequence of events.

Found this gem looking for background to the proposed removal to code of mine.

Felipe, if you are wanting to have a war of words with Junio, go have
it, with him. I don't know (nor care) who's right, I'll just buy
popcorn.

If you are going to bother every maintainer under contrib over a
problem they have nothing to do with, you'll make an enemy of the
whole group. I think you're doing fantastic on that track.

Right now, you're acting as a remarkable troll.




m
-- 
 martin.langh...@gmail.com
 -  ask interesting questions
 - don't get distracted with shiny stuff  - working code first
 ~ http://docs.moodle.org/en/User:Martin_Langhoff
--
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 v1 1/2] Remove 'git archimport'

[Felipe Contreras]

Martin Langhoff wrote:
 On Thu, May 8, 2014 at 9:33 PM, Felipe Contreras
 felipe.contre...@gmail.com wrote:
  No updates since 2010, and no tests.
 
 NAK.
 
 IMHO, this is quite unfriendly.
 
 Is this removal based on your opinion, or Junio's position (or
 consensus from maintainers from the list)? If there is a clear
 consensus or direction for old code such as this, please let me know
 (but copy martin.langh...@gmail.com, not just my very old address!).

This tool doesn't even work anyway. Why do we want to distribute code
that doesn't work?

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

Re: [PATCH v1 1/2] Remove 'git archimport'

[Martin Langhoff]

On Fri, May 9, 2014 at 1:44 PM, Junio C Hamano gits...@pobox.com wrote:
 Eric Wong normalper...@yhbt.net writes:

 Felipe Contreras felipe.contre...@gmail.com wrote:
 No updates since 2010, and no tests.

 Who benefits from this removal?  Is this causing a maintenance
 burden for Junio?

 No.  See http://thread.gmane.org/gmane.comp.version-control.git/248587

Thanks for this link. Took me a while to find -- git ML is quite busy
:-) -- to be honest it might be good if you make it a separate post,
rather than having to find buried in the removal threads that
everything's ok, safe to ignore this very thread you're reading;
specially for the casual readers.

Can we ban Felipe from the ML? If he's been a positive contributor in
the past, perhaps it can be a temporary ban.

Right now he is far from a positive member of the community.

About code I wrote... I'm still around, and care if folks find
significant bugs. Don't read the list very actively. If maintenance
standards change, I'll make an effort to meet them.



m
-- 
 martin.langh...@gmail.com
 -  ask interesting questions
 - don't get distracted with shiny stuff  - working code first
 ~ http://docs.moodle.org/en/User:Martin_Langhoff
--
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 v1 1/2] Remove 'git archimport'

[Martin Langhoff]

On Tue, May 13, 2014 at 2:05 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 This tool doesn't even work anyway.

It doesn't? Bug report / more info please?

cheers,


m
-- 
 martin.langh...@gmail.com
 -  ask interesting questions
 - don't get distracted with shiny stuff  - working code first
 ~ http://docs.moodle.org/en/User:Martin_Langhoff
--
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 v1 1/2] Remove 'git archimport'

[Junio C Hamano]

Martin Langhoff mar...@laptop.org writes:

 On Thu, May 8, 2014 at 9:33 PM, Felipe Contreras
 felipe.contre...@gmail.com wrote:
 No updates since 2010, and no tests.

 NAK.

 IMHO, this is quite unfriendly.

 Is this removal based on your opinion, or Junio's position (or
 consensus from maintainers from the list)? If there is a clear
 consensus or direction for old code such as this, please let me know
 (but copy martin.langh...@gmail.com, not just my very old address!).

 Plus, foreign SCM tools should live out-of-tree anyway.

 Says who? Is there consensus on this?

 It's generally the privilege of the maintainer -- in this case Junio
 or perhaps Linus -- to take harsh stances like this.

 Junio, what's your position?

We may think longer when somebody proposes to add a new thing that
may better live outside our tree (including the contrib/ area) than
we used to, simply because Git is more mature these days and the
ecosystem is there to support successful third-party tools, but
removal of existing subcommands needs to weigh the impact of such a
removal to existing users. No recent updates does not say anything
with respect to that---we cannot tell between The tool is perfect
to fill needs of the users and Even though the users are reporting
issues, the area maintainer is not being responsive by non activity
alone, and we know there weren't many unresponded issues in the
recent past.

There is no longer any project that still hosts anything worth
salvaging in tla, if such a claim can be substantiated, might be a
valid reason to propose a removal, but I do not think this is such a
proposal.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 01/17] contrib: remove outdated README

[Junio C Hamano]

Martin Langhoff martin.langh...@gmail.com writes:

 On Fri, May 9, 2014 at 5:54 PM, Felipe Contreras
 felipe.contre...@gmail.com wrote:
 You are once more twisting the sequence of events.

 Found this gem looking for background to the proposed removal to code of mine.

 Felipe, if you are wanting to have a war of words with Junio, go have
 it, with him.

Please don't feed the troll.  I was happy to be done with it.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Error using git-remote-hg

[Felipe Contreras]

Junio C Hamano wrote:

 It is time to catch regressions by asking wider audiences who do not
 normally follow Git development (i.e. those who are not the ones that
 follow 'master' and rebuild/install it once or twice a week for their
 daily use).

And you have one of those regressions in Git v2.0.

 My understanding is that versions of remote-hg shipped with all
 versions of Git did not work with Hg 3.0, so 58aee08 is not a
 regression fix at all.  Is working with Hg 3.0 at such a high priority
 that it makes it worth to slip the whole release schedule by a few
 weeks?  I do not think so.

It is in the contrib/ area, you don't need to slip the schedule for
something that is not part of the core.

Moreover, it *CANNOT POSSIBLY INTRODUCE REGRESSIONS*. I bet my
reputation on that.

Some time ago I asked you to trust my judgement and introduce changes
late into a release, and I told you there wouldn't be any problems, and
to trust me. If any problems arise I wouldn't be asking for something
like that again.

Well, I was right, there were no problems. And I'm right this time too,
there would be no issues.

But you don't care about reality and facts. You don't care about the
intent behind the release-candidates rule, you would rather follow the
rule blindly.

 Regarding the commit in question, I asked Felipe a question and
 never got a straight answer.

Nor will you get one, because thanks to your stupid decision for which
you still haven't provided a rationale, the git-remote-hg and
git-remote-hg are no longer maintained in git.git.

If you hadn't made such a move, you would have your answer, the fix
would be properly explained, the regression fixed, and all your users
would be happy that git-remote-hg and git-remote-bzr get distributed by
default.

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

Re: [PATCH v1 1/2] Remove 'git archimport'

[Felipe Contreras]

Martin Langhoff wrote:
 On Tue, May 13, 2014 at 2:05 PM, Felipe Contreras
 felipe.contre...@gmail.com wrote:
  This tool doesn't even work anyway.
 
 It doesn't? Bug report / more info please?

Show me that it does. The documentation is lacking, and there's no tests
at all.

So it's hard to say is anybody supposed to use this and verify that it
works, but I ran this from Jeff:

  tla register-archive http://www.atai.org/archarchives/a...@atai.org--public/
  tla my-default-archive a...@atai.org--public
  mkdir repo
  cd repo
  git archimport a...@atai.org--public

And the command threw hundreds of errors and the result seemed to miss
tons of commits.

Does it work?

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

[PATCH] Documentation: mention config sources for @{upstream}

[W. Trevor King]

The earlier documentation made vague references to is set to build
on.  Flesh that out with references to the config settings, so folks
can use git-config(1) to get more detail on what @{upstream} means.
For example, @{upstream} does not care about remote.pushdefault or
branch.name.pushremote.

Signed-off-by: W. Trevor King wk...@tremily.us
---
 Documentation/revisions.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 5a286d0..0796118 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -94,7 +94,9 @@ some output processing may assume ref names in UTF-8.
 'branchname@\{upstream\}', e.g. 'master@\{upstream\}', '@\{u\}'::
   The suffix '@\{upstream\}' to a branchname (short form 'branchname@\{u\}')
   refers to the branch that the branch specified by branchname is set to build 
on
-  top of.  A missing branchname defaults to the current one.
+  top of (configured with `branch.name.remote` and
+  `branch.name.merge`).  A missing branchname defaults to the
+  current one.
 
 'rev{caret}', e.g. 'HEAD{caret}, v1.5.1{caret}0'::
   A suffix '{caret}' to a revision parameter means the first parent of
-- 
1.9.1.353.gc66d89d

--
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: Error using git-remote-hg

[William Giokas]

On Tue, May 13, 2014 at 10:30:26AM -0700, Junio C Hamano wrote:
 Torsten Bögershausen tbo...@web.de writes:
 
  I did some testing with Git 2.0-rc3 + 58aee0864adeeb5363f.
  The remote-helper tests for hg-git worked OK
  with both hg version 2.9 and 3.0 under both Mac OS and Linux.
 
  Should we consider 58aee086 to be included in Git 2.0 ?
 
 So the answer to your question is an unambiguous no.
 
 It is a different matter if we want a change to allow us to operate
 with both older and newer version of mercurial in a release of Git
 in near future.  The answer is a resounding yes, even if the
 solution may not be the exact 58aee0864 commit [*2*].  I think an
 update should eventurally go to the maintenance tracks of even older
 releases, perhaps maint-1.8.5 and upwards, after we merge it to
 'master' in preparation for the feature release that comes after Git
 2.0, which probably will be called 2.1 but do not quote me on that.
 
 Regarding the commit in question, I asked Felipe a question and
 never got a straight answer.
 
 Why do we import changegroup unconditionally, even though it
 is only used in the new codepath meant only for version 3.0 or
 higher, not inside the if block that decides if we need that
 module?

It should not be, as it is not used outside of hg 3.0. In fact, what
would be even better would be to test whether changegroup.getbundle is
available, and then set a function like `getbundl()` to run either
`changegroup.getbundl()` with the correct args, or `repo.getbundle()` as
a fallback.

changegroup is prefectly /okay/ to import unconditionally, though as you
say it will never be used.

We can also be even more explicit with what we import by doing something
like::

  try:
  from mercurial.changegroup import getbundle

  except ImportError:
  def getbundle(__empty__, **kwargs):
  return repo.getbundle(**kwargs)

and then just calling::

  cg = getbundle(repo, 'push', heads=list(p_revs), common=common)

The `__empty__` arg is there because repo.getbundle uses a different
number of arguments than the changegroup.getbundle function. It's a much
cleaner solution than assuming that that will stay in changegroup, and
not get moved back to repo in the future. Seems to be what you described
in the second bit, though. If you would like I can submit a patch once
I've finished running the tests on it, and possibly after having Felipe
run it through his tests to be sure thta the 2.[7-9] series works as
well, and maybe you would want to test it on other versions of
mercurial that you are running alongside git.

Just my 2 cents.

 
 I had a few more questions in mind that I didn't have a chance to
 ask, and the commit log message did not explain.
 
 Is the reason why this fix is needed is because repo stopped
 being a way to ask .getbundle() and in the new world order
 changegroup is the new way to ask .getbundle()?
 
 If the answer is yes, then asking are we with 3.0 or
 later---if so ask changegroup for .getbundle()?, which is this
 patch does, may not be the right condition to trigger the new
 codepath.  Does repo know how to do .getbundle()?  If not,
 import changegroup and ask that module to perform .getbundle()
 instead may be a way to base the decision on a more directly
 relevant condition.
 
 Answers to these questions, if they came, were meant to convince
 myself that the patch 58aee0864 is the best solution to the problem,
 but because I only got Of course it is not a mistake [*1*], seeing
 it did not lead to a productive discussion, I gave up.  So I haven't
 even managed to convince myself that that commit is the best
 solution to the problem.

I was really sad to see that, and didn't have time to really look at it
because of work and other projects, but I hope this presents a better
solution than the current patch.

Thanks,

-- 
William Giokas | KaiSforza | http://kaictl.net/
GnuPG Key: 0x73CD09CF
Fingerprint: F73F 50EF BBE2 9846 8306  E6B8 6902 06D8 73CD 09CF


pgpC7ZLGG4hd9.pgp
Description: PGP signature

Re: Bash prompt namespace issue

[Junio C Hamano]

sze...@ira.uka.de writes:

 Commit 59d3924fb (prompt: fix missing file errors in zsh) added the  
 helper function eread() to git-prompt.sh.  That function should be in  
 the git namespace, i.e. prefixed with __git_, otherwise it might  
 collide with a function of the same name in the user's environment.

 It's already in master and I don't have the means to send a patch  
 fixing this ATM, sorry.

Thanks.  I think the patch Felipe posted to rename it to __git_eread
is a reasonable regression fix, so I'll queue it on top of the
problematic commit 59d3924f (prompt: fix missing file errors in zsh,
2014-04-11) and merge the result to 'master'.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 01/17] contrib: remove outdated README

[Felipe Contreras]

Junio C Hamano wrote:
 Martin Langhoff martin.langh...@gmail.com writes:
 
  On Fri, May 9, 2014 at 5:54 PM, Felipe Contreras
  felipe.contre...@gmail.com wrote:
  You are once more twisting the sequence of events.
 
  Found this gem looking for background to the proposed removal to code of 
  mine.
 
  Felipe, if you are wanting to have a war of words with Junio, go have
  it, with him.
 
 Please don't feed the troll.  I was happy to be done with it.

I was going to let this comment go (as I let the endless stream of
ad hominem attacks go), but this just one ridiculous.

I've contributed 400 patches, changed 12700 lines, sent 4200 mails on
the list. Then I'm not happy with a decision you made, and I ask you
*one* question to clarify your rationale, and I'm still waiting for an
answer.

I think after this insane amount of work I'm entitled to an answer for
this *one* question.

Instead you passive aggressively label me as a troll?

This is really disquieting.

Junio, do you honestly think I am a troll? Have at least the decency of
telling it to me.

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

[PATCH] remote-hg: getbundle changed in mercurial 3.0

[William Giokas]

In mercurial 3.0, getbundle was moved to the changegroup module, and
gained a new argument. Due to this we cannot simply start using
getbundle(...) imported from either one unconditionally, as that would
cause errors in mercurial 3.0 without changing the syntax, and errors in
mercurial 3.0 if we do change it.

The try:except block at the beginning of git-remote-hg.py tries first to
import mercurial.changegroup.getbundle, and if that fails we set the
function 'getbundle' to work correctly with mercurial.repo.getbundle by
removing the first argument.

Signed-off-by: William Giokas 1007...@gmail.com
---

I have tested this briefly with mercurial 3.0, but have not yet really run
through its paces. The tests that are included in next do pass with
mercurial 3.0.

 git-remote-hg.py | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/git-remote-hg.py b/git-remote-hg.py
index 34cda02..3dc9e11 100755
--- a/git-remote-hg.py
+++ b/git-remote-hg.py
@@ -14,6 +14,13 @@
 
 from mercurial import hg, ui, bookmarks, context, encoding, node, error, 
extensions, discovery, util
 
+try:
+from mercurial.changegroup import getbundle
+
+except ImportError:
+def getbundle(__empty__, **kwargs):
+return repo.getbundle(**kwargs)
+
 import re
 import sys
 import os
@@ -985,7 +992,7 @@ def push_unsafe(repo, remote, parsed_refs, p_revs):
 if not checkheads(repo, remote, p_revs):
 return None
 
-cg = repo.getbundle('push', heads=list(p_revs), common=common)
+cg = getbundle(repo, 'push', heads=list(p_revs), common=common)
 
 unbundle = remote.capable('unbundle')
 if unbundle:
-- 
2.0.0.rc3

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

Re: [PATCH v2 2/2] t: git-show: adapt tests to fixed 'git show'

[Junio C Hamano]

Max Kirillov m...@max630.net writes:

 'git show' used to print extra newline after merge commit, and it was
 recorded so into the test reference data. Now when the behavior is
 fixed, the tests should be updated.

 Signed-off-by: Max Kirillov m...@max630.net
 ---

Hmph.  Having these as two separate commits would mean that 1/2
alone will break the test, hurting bisectability a little bit.  The
necessary adjustments in this patch is small enough that we may be
better off squashing them into one.

  t/t1507-rev-parse-upstream.sh |  2 +-
  t/t7600-merge.sh  | 11 +--
  2 files changed, 6 insertions(+), 7 deletions(-)

 diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
 index 2a19e79..672280b 100755
 --- a/t/t1507-rev-parse-upstream.sh
 +++ b/t/t1507-rev-parse-upstream.sh
 @@ -100,7 +100,7 @@ test_expect_success 'merge my-side@{u} records the 
 correct name' '
   git branch -D new ;# can fail but is ok
   git branch -t new my-side@{u} 
   git merge -s ours new@{u} 
 - git show -s --pretty=format:%s actual 
 + git show -s --pretty=tformat:%s actual 
   echo Merge remote-tracking branch ${sq}origin/side${sq} expect 
   test_cmp expect actual
  )

This seems to me that it is updating how the output is produced, not
updating the expected outputs from commands with arguments we have
been testing.  I have been expecting to see changes to the pieces of
the code that prepare the expected output, so that we can compare
old expectations, which would have been expecting something strange,
with new expectations from the updated code, which would show that
the new behaviour is a welcome change because it would produce more
sensible output.

Having said all that, for this particular test piece, I agree with
the patch that using --pretty=tformat:%s is a lot more sensible and
using --pretty=format:%s and expecting its output to be terminated
with the newline was an unrealistic test.  After all, tformat is
the version with line terminator semantics, as opposed to item
separator semantics offered by --pretty=format:, and the test
merely was depending on the bug to expect a complete line output
(i.e. echo without -n), which you fixed.  The new test makes a
lot more sense and is relevant to the real world use, and I would
have preferred to see it explained as such in the log message.

 diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
 index 10aa028..b164621 100755
 --- a/t/t7600-merge.sh
 +++ b/t/t7600-merge.sh
 @@ -57,11 +57,10 @@ create_merge_msgs () {
   git log --no-merges ^HEAD c2 c3
   } squash.1-5-9 
   : msg.nologff 
 - echo msg.nolognoff 
 + : msg.nolognoff 
   {
   echo * tag 'c3': 
 - echo   commit 3 
 - echo
 + echo   commit 3
   } msg.log
  }

These are updating the expectation.  We used to expect an
unnecessary trailing blank line, and now we do not, which clearly
shows that the previous fix is a welcome change.

 @@ -71,7 +70,7 @@ verify_merge () {
   git diff --exit-code 
   if test -n $3
   then
 - git show -s --pretty=format:%s HEAD msg.act 
 + git show -s --pretty=tformat:%s HEAD msg.act 
   test_cmp $3 msg.act
   fi
  }

It is hard to judge what is fed as $3 here without context, but
this is in line with the --pretty=tformat aka --format is the
normal thing to use we saw in 1507.  The same for the other hunk.

 @@ -620,10 +619,10 @@ test_expect_success 'merge early part of c2' '
   git tag c6 
   git branch -f c5-branch c5 
   git merge c5-branch~1 
 - git show -s --pretty=format:%s HEAD actual.branch 
 + git show -s --pretty=tformat:%s HEAD actual.branch 
   git reset --keep HEAD^ 
   git merge c5~1 
 - git show -s --pretty=format:%s HEAD actual.tag 
 + git show -s --pretty=tformat:%s HEAD actual.tag 
   test_cmp expected.branch actual.branch 
   test_cmp expected.tag actual.tag
  '

How about squashing these two into a single patch, and at the end of
the log message for 1/2, add this to explain the changes to the
test:

Also existing tests are updated to demonstrate the new
behaviour.  Earlier, the tests that used git show -s
--pretty=format:%s, even though --pretty=format:%s calls for
item separator semantics and does not ask for the terminating
newline after the last item, expected the output to end with
such a newline.  They were relying on the buggy behaviour.  Use
of --format=%s, which is equivalent to --pretty=tformat:%s
that asks for a terminating newline after each item, is a more
realistic way to use the command.

The update to verify_merge in t7600 adjusts the the merge
message that used to expect an extra blank line in the output,
which has been eliminated with this fix.

or something like that?


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to 

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

[Junio C Hamano]

James Denholm nod.h...@gmail.com writes:

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

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

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

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

 [*1*]: Via peel_committish(), from git:git-sh-setup.sh, pre-existing
 dependency of git-subtree.

 Reported-by: Kevin Cagle kca...@micron.com
 Helped-by: Junio C Hamano gits...@pobox.com
 Signed-off-by: James Denholm nod.h...@gmail.com
 ---
 I felt that defining revp would be a little more self-documenting than
 using $rev^0.

That is a good decision, but as long as we are attempting to peel,
don't we want to stop the damage when it does not peel to a commit?

I'll tentatively queue this.  Thanks.

-- 8 --
From: James Denholm nod.h...@gmail.com
Date: Tue, 13 May 2014 14:08:58 +1000
Subject: [PATCH] contrib/subtree: allow adding an annotated tag

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

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

the object name refers to a tag and is never peeled, and the git
commit-tree call (line 561) slaps us in the face because it doesn't
peel tags to commits.

Because peeling a committish doesn't do anything if it's already a
commit, fix by peeling the object name before assigning it to $rev,
using peel_committish() from git:git-sh-setup.sh, a pre-existing
dependency of git-subtree.

Reported-by: Kevin Cagle kca...@micron.com
Helped-by: Junio C Hamano gits...@pobox.com
Signed-off-by: James Denholm nod.h...@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
 contrib/subtree/git-subtree.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index db925ca..fa1a583 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -558,8 +558,9 @@ cmd_add_commit()
commit=$(add_squashed_msg $rev $dir |
 git commit-tree $tree $headp -p $rev) || exit $?
else
+   revp=$(peel_committish $rev) 
commit=$(add_msg $dir $headrev $rev |
-git commit-tree $tree $headp -p $rev) || exit $?
+git commit-tree $tree $headp -p $revp) || exit $?
fi
git reset $commit || exit $?

-- 
2.0.0-rc3-404-gb0be553

--
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: Error using git-remote-hg

[Felipe Contreras]

William Giokas wrote:
 On Tue, May 13, 2014 at 10:30:26AM -0700, Junio C Hamano wrote:

  Why do we import changegroup unconditionally, even though it
  is only used in the new codepath meant only for version 3.0 or
  higher, not inside the if block that decides if we need that
  module? 

 changegroup is prefectly /okay/ to import unconditionally, though as you
 say it will never be used.

As you say, it's perfectly OK.

 We can also be even more explicit with what we import by doing something
 like::
 
   try:
   from mercurial.changegroup import getbundle
 
   except ImportError:
   def getbundle(__empty__, **kwargs):
   return repo.getbundle(**kwargs)

We could try that, but that would assume we want to maintain getbundle()
for the long run, and I personally don't want to do that. I would rather
contact the Mercurial developers about ways in which the push() method
can be improved so we don't need to have our own version. Hopefully
after it's improved we wouldn't have to call getbundle().

Moreover, eventually there will be a Mercurial 4.0, even 5.0, and at
some point we would want to remove the hacks for older versions. When we
do so we would want the import to remain unconditionally, and remove the
'check_version(3, 0)' which is already helping to explain what the code
is for without the need of comments.

 I was really sad to see that, and didn't have time to really look at it
 because of work and other projects, but I hope this presents a better
 solution than the current patch.

Either way Junio doesn't maintain this code, I do. And it's not
maintained in git.git, git's maintained out-of-tree (thanks to Junio's
decisions).

So please post your suggestions and patches to git...@googlegroups.com,
and use the latest code at https://github.com/felipec/git-remote-hg.

Cheers.

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

Re: Error using git-remote-hg

[William Giokas]

On Tue, May 13, 2014 at 02:09:55PM -0500, Felipe Contreras wrote:
 William Giokas wrote:
  On Tue, May 13, 2014 at 10:30:26AM -0700, Junio C Hamano wrote:
 
   Why do we import changegroup unconditionally, even though it
   is only used in the new codepath meant only for version 3.0 or
   higher, not inside the if block that decides if we need that
   module? 
 
  changegroup is prefectly /okay/ to import unconditionally, though as you
  say it will never be used.
 
 As you say, it's perfectly OK.

But wrong. Yes, it works, but it's not how it should be done when we
have a code review such as this. It should simply not be done and makes
no sense to do with an 'if check ver; else' kind of thing later in the
application.

 
  We can also be even more explicit with what we import by doing something
  like::
  
try:
from mercurial.changegroup import getbundle
  
except ImportError:
def getbundle(__empty__, **kwargs):
return repo.getbundle(**kwargs)
 
 We could try that, but that would assume we want to maintain getbundle()
 for the long run, and I personally don't want to do that. I would rather
 contact the Mercurial developers about ways in which the push() method
 can be improved so we don't need to have our own version. Hopefully
 after it's improved we wouldn't have to call getbundle().

Assuming that mercurial 3.0 will not change, then this should never
need to change. Changes in 'getbundle' upstream would require changes
either way.

 Moreover, eventually there will be a Mercurial 4.0, even 5.0, and at
 some point we would want to remove the hacks for older versions. When we
 do so we would want the import to remain unconditionally, and remove the
 'check_version(3, 0)' which is already helping to explain what the code
 is for without the need of comments.

The same exact thing can be done with this. In fact, it would probably
allow us to have better future-proofing with regards to new versions of
mercurial, there would just be different try:except statements at the
beginning.

 
  I was really sad to see that, and didn't have time to really look at it
  because of work and other projects, but I hope this presents a better
  solution than the current patch.
 
 Either way Junio doesn't maintain this code, I do. And it's not
 maintained in git.git, git's maintained out-of-tree (thanks to Junio's
 decisions).

I still see it in git.git, and I will contribute this upstream for as
long as it is in the tree. If you want to use the patch that I sent to
this list, feel free.

 So please post your suggestions and patches to git...@googlegroups.com,
 and use the latest code at https://github.com/felipec/git-remote-hg.

Thanks,

-- 
William Giokas | KaiSforza | http://kaictl.net/
GnuPG Key: 0x73CD09CF
Fingerprint: F73F 50EF BBE2 9846 8306  E6B8 6902 06D8 73CD 09CF


pgp_mLEPyhosF.pgp
Description: PGP signature

Re: [PATCH v2 2/2] t: git-show: adapt tests to fixed 'git show'

[Max Kirillov]

On Tue, May 13, 2014 at 12:26:42PM -0700, Junio C Hamano wrote:
 Hmph.  Having these as two separate commits would mean that 1/2
 alone will break the test, hurting bisectability a little bit.  The
 necessary adjustments in this patch is small enough that we may be
 better off squashing them into one.

Ok, will squash them.

  t/t1507-rev-parse-upstream.sh |  2 +-
  t/t7600-merge.sh  | 11 +--
  2 files changed, 6 insertions(+), 7 deletions(-)

 diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
 index 2a19e79..672280b 100755
 --- a/t/t1507-rev-parse-upstream.sh
 +++ b/t/t1507-rev-parse-upstream.sh
 @@ -100,7 +100,7 @@ test_expect_success 'merge my-side@{u} records the 
 correct name' '
  git branch -D new ;# can fail but is ok
  git branch -t new my-side@{u} 
  git merge -s ours new@{u} 
 -git show -s --pretty=format:%s actual 
 +git show -s --pretty=tformat:%s actual 
  echo Merge remote-tracking branch ${sq}origin/side${sq} expect 
  test_cmp expect actual
  )
 
 This seems to me that it is updating how the output is produced, not
 updating the expected outputs from commands with arguments we have
 been testing.  I have been expecting to see changes to the pieces of
 the code that prepare the expected output, so that we can compare
 old expectations, which would have been expecting something strange,
 with new expectations from the updated code, which would show that
 the new behaviour is a welcome change because it would produce more
 sensible output.
 
 Having said all that, for this particular test piece, I agree with
 the patch that using --pretty=tformat:%s is a lot more sensible and
 using --pretty=format:%s and expecting its output to be terminated
 with the newline was an unrealistic test.  After all, tformat is
 the version with line terminator semantics, as opposed to item
 separator semantics offered by --pretty=format:, and the test
 merely was depending on the bug to expect a complete line output
 (i.e. echo without -n), which you fixed.  The new test makes a
 lot more sense and is relevant to the real world use, and I would
 have preferred to see it explained as such in the log message.

In analogous cases with non-merge commits I have found the
form --format=..., in t3505-cherry-pick-empty.sh for
example, so I have decided that merges should also use it. The
form --pretty=tformat:... seems more explicit to me, so I
have chosen this one.

Will add the message as you have suggested.

 diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
 index 10aa028..b164621 100755
 --- a/t/t7600-merge.sh
 +++ b/t/t7600-merge.sh
 @@ -57,11 +57,10 @@ create_merge_msgs () {
  git log --no-merges ^HEAD c2 c3
  } squash.1-5-9 
  : msg.nologff 
 -echo msg.nolognoff 
 +: msg.nolognoff 
  {
  echo * tag 'c3': 
 -echo   commit 3 
 -echo
 +echo   commit 3
  } msg.log
  }
 
 These are updating the expectation.  We used to expect an
 unnecessary trailing blank line, and now we do not, which clearly
 shows that the previous fix is a welcome change.
 
 @@ -71,7 +70,7 @@ verify_merge () {
  git diff --exit-code 
  if test -n $3
  then
 -git show -s --pretty=format:%s HEAD msg.act 
 +git show -s --pretty=tformat:%s HEAD msg.act 
  test_cmp $3 msg.act
  fi
  }
 
 It is hard to judge what is fed as $3 here without context, but
 this is in line with the --pretty=tformat aka --format is the
 normal thing to use we saw in 1507.  The same for the other hunk.
 
 @@ -620,10 +619,10 @@ test_expect_success 'merge early part of c2' '
  git tag c6 
  git branch -f c5-branch c5 
  git merge c5-branch~1 
 -git show -s --pretty=format:%s HEAD actual.branch 
 +git show -s --pretty=tformat:%s HEAD actual.branch 
  git reset --keep HEAD^ 
  git merge c5~1 
 -git show -s --pretty=format:%s HEAD actual.tag 
 +git show -s --pretty=tformat:%s HEAD actual.tag 
  test_cmp expected.branch actual.branch 
  test_cmp expected.tag actual.tag
  '
 
 How about squashing these two into a single patch, and at the end of
 the log message for 1/2, add this to explain the changes to the
 test:
 
 Also existing tests are updated to demonstrate the new
 behaviour.  Earlier, the tests that used git show -s
 --pretty=format:%s, even though --pretty=format:%s calls for
 item separator semantics and does not ask for the terminating
 newline after the last item, expected the output to end with
 such a newline.  They were relying on the buggy behaviour.  Use
 of --format=%s, which is equivalent to --pretty=tformat:%s
 that asks for a terminating newline after each item, is a more
 realistic way to use the command.
 
 The update to verify_merge in t7600 adjusts the the merge
 message that used to expect an extra blank line in the output,
 which has been eliminated with this 

Re: [PATCH v2 1/2] git-show: fix 'git show -s' to not add extra terminator after merge commit

[Max Kirillov]

On Tue, May 13, 2014 at 07:57:08AM +0200, Johannes Sixt wrote:
 Am 5/13/2014 1:10, schrieb Max Kirillov:
 --- a/t/t7007-show.sh
 +++ b/t/t7007-show.sh
 @@ -25,6 +25,7 @@ test_expect_success 'set up a bit of history' '
  git checkout -b side HEAD^^ 
  test_commit side2 
  test_commit side3
 +test_merge merge main3
  '
 
 Broken -chain.

Thank you, will fix it.

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

[PATCH v2] remote-hg: getbundle changed in mercurial 3.0

[William Giokas]

In mercurial 3.0, getbundle was moved to the changegroup module, and
gained a new argument. Due to this we cannot simply start using
getbundle(...) imported from either one unconditionally, as that would
cause errors in mercurial 3.0 without changing the syntax, and errors in
mercurial 3.0 if we do change it.

The try:except block at the beginning of git-remote-hg.py tries first to
import mercurial.changegroup.getbundle, and if that fails we set the
function 'getbundle' to work correctly with mercurial.repo.getbundle by
removing the first argument.

Signed-off-by: William Giokas 1007...@gmail.com
---

So, what I had in there before would not work at all on repo.getbundle
because **kwargs only unpacks keyword args, not normal args. Sometimes my
brain works.

 git-remote-hg.py | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/git-remote-hg.py b/git-remote-hg.py
index 34cda02..32eeffb 100755
--- a/git-remote-hg.py
+++ b/git-remote-hg.py
@@ -14,6 +14,13 @@
 
 from mercurial import hg, ui, bookmarks, context, encoding, node, error, 
extensions, discovery, util
 
+try:
+from mercurial.changegroup import getbundle
+
+except ImportError:
+def getbundle(repo, **kwargs):
+return repo.getbundle(**kwargs)
+
 import re
 import sys
 import os
@@ -985,7 +992,8 @@ def push_unsafe(repo, remote, parsed_refs, p_revs):
 if not checkheads(repo, remote, p_revs):
 return None
 
-cg = repo.getbundle('push', heads=list(p_revs), common=common)
+cg = getbundle(repo=repo, source='push', heads=list(p_revs),
+   common=common)
 
 unbundle = remote.capable('unbundle')
 if unbundle:
-- 
2.0.0.rc3

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

Re: [PATCH v6 00/42] Use ref transactions for all ref updates

[Jonathan Nieder]

Hi,

Ronnie Sahlberg wrote:

 This patch series is based on next and expands on the transaction API.

Sorry to take so long to get to this.

For the future, it's easier to review patches based on some particular
branch that got merged into next, since next is a moving target
(series come and go from there depending on what seems to need testing
at a given moment).  Is mh/ref-transaction the relevant branch to
build on?

Trying to apply the series to mh/ref-transaction, I get conflicts in
patch 13 due to absence of rs/ref-update-check-errors-early.

Trying to apply the series to a merge of mh/ref-transaction and
rs/ref-update-check-errors-early, I get a minor conflict in patch 15
but it is easy to resolve and the rest goes smoothly.

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

[GUILT v2 00/29] Teach guilt import-commit how to create legal patch names, and more

[Per Cederqvist]

This is version two of the patch series I sent back on 21 Mar 2014.  I
have addressed all feedback to date, updated the coding style, and
added signed-off-by lines from Jeff Sipek for those commits that I
have received an explicit approval from him (but only if I have not
made any other change to that particular commit).

I have also added one more patch: a very limited coding style guide,
and accompanying settings for Emacs.  See the last commit in the
series.  All other commits have retained their numbering.

To see how the patches have evolved, you might find
http://www.lysator.liu.se/~ceder/guilt-oslo-2014-v2/ useful.  It
displays diffs of all the patches, in pdiffdiff output format.

Here is the original blurb for the series, slightly edited:

I recently found myself sitting on a train with a computer in front of
me.  I tried to use guilt import-commit, which seemed to work, but
when I tried to guilt push the commits I had just imported I got
some errors.  It turned out that guilt import-commit had generated
invalid patch names.

I decided to fix the issue, and write a test case that demonstrated
the problem.

One thing led to another, and here I am, a few late nights at a hotel
and a return trip on the train later, submitting a patch series in 28
parts.  Sorry about the number of patches, but this is what happens
when you uncover a bug while writing a test case for the bug you
uncovered while writing a test case for your original problem.

The patch series consists of:

 - A number of fixes to the test suite.

 - A number of bug fixes which I hope are non-controversial.  Most of
   the fixes have test cases.

 - Changed behavior: guilt push when there is nothing more to push
   now uses exit status 1.  This makes it possible to write shell
   loops such as while guilt push; do make test||break; done.  Also,
   guilt pop when no patches are applied also uses exit status 1.
   (This aligns guilt push and guilt pop with how hg qpush and
   hg qpop has worked for several years.)

 - Changed behavior: by default, guilt no longer changes branch when
   you push a patch.  You need to do git config guilt.reusebranch
   false to re-enable that.  This patch sets the default value of
   guilt.reusebranch to true; it should in my opinion change to false
   a year or two after the next release.

 - The humble beginnings of a coding style guide.

A more detailed overview of the patches:

1. Some tests fail if git config guilt.diffstat true is in effect.

2-4. Some commands fail if run from a directory with spaces in its
 name.

5. guilt new had an overly restrictive argument parser.

6-8. guilt.diffstat could break guilt fold and guilt new.

9-10. The test suite did not test exit status properly.

11. Remove pointless redirections in the test suite.

12-13. guilt header tried to check if a patch existed, but the check
was broken.

14-16. Various parts of guilt tried to ensure that patch names were
   legal git branch names, but failed.

17-20. guilt graph failed when no patch was applied, and when a
   branch name contained a comma or a quote.

21-23. git config log.decorate short caused guilt import-commit,
   guilt patchbomb and guilt rebase to fail in various ways.

24. Patches may contain backslashes, but various informative messages
from guilt did backslash processing.

25-26. guilt push and guilt pop should fail when there is nothing
   to do.

27-28. These two commits were things I intended to contribute a while
   back, when contributing the Change git branch when patches are
   applied change (commit 67d3af63f422).  These commits makes
   that behavior optional, and it defaults to being disabled, so
   that you can use both Guilt v0.35 (and earlier) and the current
   Guilt code against the same repo.  These commits add some code
   complexity, and you might want to skip them if you think the
   current behavior is better.

29. A coding style guide, with Emacs support.

This patch series is also available on
http://repo.or.cz/w/guilt/ceder.git in the oslo-2014-v2 branch.  If
you already have a copy of guilt, you should be able to fetch that
branch with something like:

git remote add ceder http://repo.or.cz/r/guilt/ceder.git
git fetch ceder refs/heads/oslo-2014-v2:refs/remotes/ceder/oslo-2014-v2

A few of the regression/t-*.out files contain non-ASCII characters.  I
hope they survive the mail transfer; if not, please use the repo above
to fetch the commits.


Per Cederqvist (29):
  The tests should not fail if guilt.diffstat is set.
  Allow guilt delete -f to run from a dir which contains spaces.
  Added test case for guilt delete -f.
  Allow guilt import-commit to run from a dir which contains spaces.
  guilt new: Accept more than 4 arguments.
  Fix the do_get_patch function.
  Added test cases for guilt fold.
  Added more test cases for guilt new: empty patches.
  Test suite: properly check the exit status of commands.
  Run 

[GUILT v2 01/29] The tests should not fail if guilt.diffstat is set.

[Per Cederqvist]

Explicitly set guilt.diffstat to its default value.  Without this, the
027 test (and possibly others) fail if guilt.diffstat is set to true
in ~/.gitconfig.

Signed-off-by: Per Cederqvist ced...@opera.com
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
---
 regression/scaffold | 1 +
 1 file changed, 1 insertion(+)

diff --git a/regression/scaffold b/regression/scaffold
index 546d8c6..5c8b73e 100644
--- a/regression/scaffold
+++ b/regression/scaffold
@@ -87,6 +87,7 @@ function setup_git_repo
# Explicitly set config that the tests rely on.
git config log.date default
git config log.decorate no
+   git config guilt.diffstat false
 }
 
 function setup_guilt_repo
-- 
1.8.3.1

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

[GUILT v2 02/29] Allow guilt delete -f to run from a dir which contains spaces.

[Per Cederqvist]

Signed-off-by: Per Cederqvist ced...@opera.com
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
---
 guilt-delete | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/guilt-delete b/guilt-delete
index 3e394f8..967ac10 100755
--- a/guilt-delete
+++ b/guilt-delete
@@ -49,7 +49,7 @@ series_remove_patch $patch
 
 guilt_hook delete $patch
 
-[ ! -z $force ]  rm -f $GUILT_DIR/$branch/$patch
+[ ! -z $force ]  rm -f $GUILT_DIR/$branch/$patch
 
 exit 0
 
-- 
1.8.3.1

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

[GUILT v2 03/29] Added test case for guilt delete -f.

[Per Cederqvist]

Ensure that the file really is deleted.

Signed-off-by: Per Cederqvist ced...@opera.com
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
---
 regression/t-026.out | 15 +++
 regression/t-026.sh  |  5 -
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/regression/t-026.out b/regression/t-026.out
index 3b9fb14..be50b48 100644
--- a/regression/t-026.out
+++ b/regression/t-026.out
@@ -29,3 +29,18 @@ f 7848194fd2e9ee0eb6589482900687d799d60a12  
.git/patches/master/series
 f 9c18cc7abe6b87f18503714a80a677b4094eb457  .git/patches/master/add
 f bc9ab2e0f5db99d483961e956e814d963f0309f8  .git/patches/master/modify
 f da39a3ee5e6b4b0d3255bfef95601890afd80709  .git/patches/master/status
+% guilt new delete-me
+% guilt pop
+All patches popped.
+% guilt delete -f delete-me
+% list_files
+d .git/patches
+d .git/patches/master
+d .git/refs/patches
+d .git/refs/patches/master
+f 22930c6d1f1938f298a4fca51c57e4b47171db21  .git/patches/master/mode
+f 413390f3906f16f30b054a4fb86c1e014b964504  .git/patches/master/remove
+f 7848194fd2e9ee0eb6589482900687d799d60a12  .git/patches/master/series
+f 9c18cc7abe6b87f18503714a80a677b4094eb457  .git/patches/master/add
+f bc9ab2e0f5db99d483961e956e814d963f0309f8  .git/patches/master/modify
+f da39a3ee5e6b4b0d3255bfef95601890afd80709  .git/patches/master/status
diff --git a/regression/t-026.sh b/regression/t-026.sh
index 0ccdf85..e25d0df 100755
--- a/regression/t-026.sh
+++ b/regression/t-026.sh
@@ -20,4 +20,7 @@ cmd guilt delete add
 
 cmd list_files
 
-# FIXME: test delete -f
+cmd guilt new delete-me
+cmd guilt pop
+cmd guilt delete -f delete-me
+cmd list_files
-- 
1.8.3.1

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

[GUILT v2 04/29] Allow guilt import-commit to run from a dir which contains spaces.

[Per Cederqvist]

Signed-off-by: Per Cederqvist ced...@opera.com
---
 guilt-import-commit | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/guilt-import-commit b/guilt-import-commit
index 20dcee2..f14647c 100755
--- a/guilt-import-commit
+++ b/guilt-import-commit
@@ -23,7 +23,7 @@ if ! must_commit_first; then
 fi
 
 disp About to begin conversion... 2
-disp Current head: `cat $GIT_DIR/refs/heads/\`git_branch\`` 2
+disp Current head: `git rev-parse \`git_branch\`` 2
 
 for rev in `git rev-list $rhash`; do
s=`git log --pretty=oneline -1 $rev | cut -c 42-`
@@ -46,7 +46,7 @@ for rev in `git rev-list $rhash`; do
do_make_header $rev
echo 
git diff --binary $rev^..$rev
-   )  $GUILT_DIR/$branch/$fname
+   )  $GUILT_DIR/$branch/$fname
 
# FIXME: grab the GIT_AUTHOR_DATE from the commit object and set the
# timestamp on the patch
@@ -68,6 +68,6 @@ for rev in `git rev-list $rhash`; do
 done
 
 disp Done. 2
-disp Current head: `cat $GIT_DIR/refs/heads/\`git_branch\`` 2
+disp Current head: `git rev-parse \`git_branch\`` 2
 
 }
-- 
1.8.3.1

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

[GUILT v2 06/29] Fix the do_get_patch function.

[Per Cederqvist]

A patch file consists of:

(1) the description
(2) optional diffstat
(3) the patches

When extracting the patch, we only want part 3.  The do_get_patch used
to give us part 2 and part 3.  That made for instance this series of
operations fail if guilt.diffstat is true:

guilt push empty-1
guilt push empty-2
guilt pop
guilt fold empty-2
guilt pop
guilt push

Signed-off-by: Per Cederqvist ced...@opera.com
---
 guilt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/guilt b/guilt
index 8701481..3fc524e 100755
--- a/guilt
+++ b/guilt
@@ -334,7 +334,7 @@ do_get_patch()
 {
awk '
 BEGIN{}
-/^(diff |---$|--- )/ {patch = 1}
+/^(diff |--- )/ {patch = 1}
 patch == 1 {print $0}
 END{}
 '  $1
-- 
1.8.3.1

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

[GUILT v2 05/29] guilt new: Accept more than 4 arguments.

[Per Cederqvist]

The argument parser arbitrarily refused to accept more than 4
arguments.  That made it impossible to run guilt new -f -s -m msg
patch.  Removed the checks for the number of arguments from the
guilt new parser -- the other checks that are already there are
enough to catch all errors.

Give a better error message if -m isn't followed by a message
argument.

Signed-off-by: Per Cederqvist ced...@opera.com
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
---
 guilt-new | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/guilt-new b/guilt-new
index bb68924..9528438 100755
--- a/guilt-new
+++ b/guilt-new
@@ -11,10 +11,6 @@ fi
 
 _main() {
 
-if [ $# -lt 1 ] || [ $# -gt 4 ]; then
-   usage
-fi
-
 while [ $# -gt 0 ] ; do
case $1 in
-f)
@@ -31,6 +27,9 @@ while [ $# -gt 0 ] ; do
fi
;;
-m)
+   if [ $# -eq 1 ]; then
+   usage
+   fi
msg=$2
shift
 
-- 
1.8.3.1

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

[GUILT v2 08/29] Added more test cases for guilt new: empty patches.

[Per Cederqvist]

Test that empty patches are handled correctly, both with and without
the guilt.diffstat configuration option.

Signed-off-by: Per Cederqvist ced...@opera.com
---
 regression/t-020.out | 250 +++
 regression/t-020.sh  |  60 +
 2 files changed, 310 insertions(+)

diff --git a/regression/t-020.out b/regression/t-020.out
index af45734..7e07efa 100644
--- a/regression/t-020.out
+++ b/regression/t-020.out
@@ -1128,3 +1128,253 @@ f 9c18cc7abe6b87f18503714a80a677b4094eb457  
.git/patches/master/add
 f bacb4aad8a55fe4e7aa58a9ae169990bb764069f  .git/patches/master/series
 f bc9ab2e0f5db99d483961e956e814d963f0309f8  .git/patches/master/modify
 f da39a3ee5e6b4b0d3255bfef95601890afd80709  .git/patches/master/status
+% guilt new empty.patch
+% guilt pop
+All patches popped.
+% guilt push
+Applying patch..empty.patch
+Patch applied.
+% list_files
+d .git/patches
+d .git/patches/master
+d .git/refs/patches
+d .git/refs/patches/master
+f 22930c6d1f1938f298a4fca51c57e4b47171db21  .git/patches/master/mode
+f 413390f3906f16f30b054a4fb86c1e014b964504  .git/patches/master/remove
+f 9c18cc7abe6b87f18503714a80a677b4094eb457  .git/patches/master/add
+f bc9ab2e0f5db99d483961e956e814d963f0309f8  .git/patches/master/modify
+f d15a1d2d34493f790c78ddacb8815b0b9536ee2b  .git/patches/master/series
+f da39a3ee5e6b4b0d3255bfef95601890afd80709  .git/patches/master/empty.patch
+f e90b964f01cbef60bbe00c38c55d9ea86618a66a  .git/patches/master/status
+r c7a139f532a43c3c8b0e068cac04f8f6af0f94e1  
.git/refs/patches/master/empty.patch
+% git log -p
+commit c7a139f532a43c3c8b0e068cac04f8f6af0f94e1
+Author: Author Name author@email
+Date:   Mon Jan 1 00:00:00 2007 +
+
+patch empty.patch
+
+commit d4850419ccc1146c7169f500725ce504b9774ed0
+Author: Author Name author@email
+Date:   Mon Jan 1 00:00:00 2007 +
+
+initial
+
+Signed-off-by: Commiter Name commiter@email
+
+diff --git a/def b/def
+new file mode 100644
+index 000..8baef1b
+--- /dev/null
 b/def
+@@ -0,0 +1 @@
++abc
+% git config guilt.diffstat true
+% guilt refresh
+Patch empty.patch refreshed
+% guilt pop
+All patches popped.
+% guilt push
+Applying patch..empty.patch
+Patch applied.
+% list_files
+d .git/patches
+d .git/patches/master
+d .git/refs/patches
+d .git/refs/patches/master
+f 22930c6d1f1938f298a4fca51c57e4b47171db21  .git/patches/master/mode
+f 413390f3906f16f30b054a4fb86c1e014b964504  .git/patches/master/remove
+f 7d261b8caad0f161c21daf5de65eeb521ff8c067  .git/patches/master/empty.patch
+f 9c18cc7abe6b87f18503714a80a677b4094eb457  .git/patches/master/add
+f bc9ab2e0f5db99d483961e956e814d963f0309f8  .git/patches/master/modify
+f d15a1d2d34493f790c78ddacb8815b0b9536ee2b  .git/patches/master/series
+f da39a3ee5e6b4b0d3255bfef95601890afd80709  .git/patches/master/empty.patch~
+f e90b964f01cbef60bbe00c38c55d9ea86618a66a  .git/patches/master/status
+r c7a139f532a43c3c8b0e068cac04f8f6af0f94e1  
.git/refs/patches/master/empty.patch
+% git log -p
+commit c7a139f532a43c3c8b0e068cac04f8f6af0f94e1
+Author: Author Name author@email
+Date:   Mon Jan 1 00:00:00 2007 +
+
+patch empty.patch
+
+commit d4850419ccc1146c7169f500725ce504b9774ed0
+Author: Author Name author@email
+Date:   Mon Jan 1 00:00:00 2007 +
+
+initial
+
+Signed-off-by: Commiter Name commiter@email
+
+diff --git a/def b/def
+new file mode 100644
+index 000..8baef1b
+--- /dev/null
 b/def
+@@ -0,0 +1 @@
++abc
+% git config guilt.diffstat false
+---
+
+% guilt pop
+All patches popped.
+% guilt push
+Applying patch..empty.patch
+Patch applied.
+% list_files
+d .git/patches
+d .git/patches/master
+d .git/refs/patches
+d .git/refs/patches/master
+f 22930c6d1f1938f298a4fca51c57e4b47171db21  .git/patches/master/mode
+f 413390f3906f16f30b054a4fb86c1e014b964504  .git/patches/master/remove
+f 7d261b8caad0f161c21daf5de65eeb521ff8c067  .git/patches/master/empty.patch
+f 9c18cc7abe6b87f18503714a80a677b4094eb457  .git/patches/master/add
+f bc9ab2e0f5db99d483961e956e814d963f0309f8  .git/patches/master/modify
+f d15a1d2d34493f790c78ddacb8815b0b9536ee2b  .git/patches/master/series
+f da39a3ee5e6b4b0d3255bfef95601890afd80709  .git/patches/master/empty.patch~
+f e90b964f01cbef60bbe00c38c55d9ea86618a66a  .git/patches/master/status
+r c7a139f532a43c3c8b0e068cac04f8f6af0f94e1  
.git/refs/patches/master/empty.patch
+% git log -p
+commit c7a139f532a43c3c8b0e068cac04f8f6af0f94e1
+Author: Author Name author@email
+Date:   Mon Jan 1 00:00:00 2007 +
+
+patch empty.patch
+
+commit d4850419ccc1146c7169f500725ce504b9774ed0
+Author: Author Name author@email
+Date:   Mon Jan 1 00:00:00 2007 +
+
+initial
+
+Signed-off-by: Commiter Name commiter@email
+
+diff --git a/def b/def
+new file mode 100644
+index 000..8baef1b
+--- /dev/null
 b/def
+@@ -0,0 +1 @@
++abc
+% git config guilt.diffstat true
+% guilt refresh
+Patch empty.patch refreshed
+% guilt pop
+All patches popped.
+% guilt push
+Applying patch..empty.patch

[GUILT v2 07/29] Added test cases for guilt fold.

[Per Cederqvist]

Test that we can combine any combination of patches with empty and
non-empty messages, both with and without guilt.diffstat.  (All
patches are empty.)

Signed-off-by: Per Cederqvist ced...@opera.com
---
 regression/t-035.out | 467 +++
 regression/t-035.sh  |  62 +++
 2 files changed, 529 insertions(+)
 create mode 100644 regression/t-035.out
 create mode 100755 regression/t-035.sh

diff --git a/regression/t-035.out b/regression/t-035.out
new file mode 100644
index 000..cc16fb4
--- /dev/null
+++ b/regression/t-035.out
@@ -0,0 +1,467 @@
+% setup_repo
+% git config guilt.diffstat true
+%% empty + empty (diffstat=true)
+% guilt new empty-1
+% guilt pop
+All patches popped.
+% guilt push
+Applying patch..empty-1
+Patch applied.
+% guilt new empty-2
+% guilt pop
+Now at empty-1.
+% guilt push
+Applying patch..empty-2
+Patch applied.
+% guilt pop
+Now at empty-1.
+% guilt fold empty-2
+% guilt pop
+All patches popped.
+% guilt push
+Applying patch..empty-1
+Patch applied.
+% list_files
+d .git/patches
+d .git/patches/master
+d .git/refs/patches
+d .git/refs/patches/master
+f 22930c6d1f1938f298a4fca51c57e4b47171db21  .git/patches/master/mode
+f 413390f3906f16f30b054a4fb86c1e014b964504  .git/patches/master/remove
+f 4ea806e306f0228a8ef41f186035e7b04097f1f2  .git/patches/master/status
+f 7d261b8caad0f161c21daf5de65eeb521ff8c067  .git/patches/master/empty-1
+f 9c18cc7abe6b87f18503714a80a677b4094eb457  .git/patches/master/add
+f bc9ab2e0f5db99d483961e956e814d963f0309f8  .git/patches/master/modify
+f d28d87b88c1e24d637e390dc3603cfa7c1715711  .git/patches/master/series
+f da39a3ee5e6b4b0d3255bfef95601890afd80709  .git/patches/master/empty-1~
+f da39a3ee5e6b4b0d3255bfef95601890afd80709  .git/patches/master/empty-2~
+r bde3d337af70f36836ad606c800d194006f883b3  .git/refs/patches/master/empty-1
+% guilt pop
+All patches popped.
+% guilt delete -f empty-1
+% list_files
+d .git/patches
+d .git/patches/master
+d .git/refs/patches
+d .git/refs/patches/master
+f 22930c6d1f1938f298a4fca51c57e4b47171db21  .git/patches/master/mode
+f 413390f3906f16f30b054a4fb86c1e014b964504  .git/patches/master/remove
+f 9c18cc7abe6b87f18503714a80a677b4094eb457  .git/patches/master/add
+f bacb4aad8a55fe4e7aa58a9ae169990bb764069f  .git/patches/master/series
+f bc9ab2e0f5db99d483961e956e814d963f0309f8  .git/patches/master/modify
+f da39a3ee5e6b4b0d3255bfef95601890afd80709  .git/patches/master/empty-1~
+f da39a3ee5e6b4b0d3255bfef95601890afd80709  .git/patches/master/empty-2~
+f da39a3ee5e6b4b0d3255bfef95601890afd80709  .git/patches/master/status
+%% empty + nonempty (diffstat=true)
+% guilt new empty
+% guilt pop
+All patches popped.
+% guilt push
+Applying patch..empty
+Patch applied.
+% guilt new -f -s -m A commit message. nonempty
+% guilt pop
+Now at empty.
+% guilt push
+Applying patch..nonempty
+Patch applied.
+% guilt pop
+Now at empty.
+% guilt fold nonempty
+% guilt pop
+All patches popped.
+% guilt push
+Applying patch..empty
+Patch applied.
+% list_files
+d .git/patches
+d .git/patches/master
+d .git/refs/patches
+d .git/refs/patches/master
+f 15aab0fd8b937eb3bb01841693f35dcb75da2faf  .git/patches/master/status
+f 22930c6d1f1938f298a4fca51c57e4b47171db21  .git/patches/master/mode
+f 413390f3906f16f30b054a4fb86c1e014b964504  .git/patches/master/remove
+f 51fcfcf16db2903f19ab4a4a3caacd297ea9f6cd  .git/patches/master/empty~
+f 51fcfcf16db2903f19ab4a4a3caacd297ea9f6cd  .git/patches/master/nonempty~
+f 683678040eef9334d6329e00d5b9babda3e65b57  .git/patches/master/empty
+f 9c18cc7abe6b87f18503714a80a677b4094eb457  .git/patches/master/add
+f a26a22287b500a2a372e42c2bab03599bbe37cdf  .git/patches/master/series
+f bc9ab2e0f5db99d483961e956e814d963f0309f8  .git/patches/master/modify
+f da39a3ee5e6b4b0d3255bfef95601890afd80709  .git/patches/master/empty-1~
+f da39a3ee5e6b4b0d3255bfef95601890afd80709  .git/patches/master/empty-2~
+r 4eedaa32894fc07af3298d8c1178052942a3ca6a  .git/refs/patches/master/empty
+% guilt pop
+All patches popped.
+% guilt delete -f empty
+% list_files
+d .git/patches
+d .git/patches/master
+d .git/refs/patches
+d .git/refs/patches/master
+f 22930c6d1f1938f298a4fca51c57e4b47171db21  .git/patches/master/mode
+f 413390f3906f16f30b054a4fb86c1e014b964504  .git/patches/master/remove
+f 51fcfcf16db2903f19ab4a4a3caacd297ea9f6cd  .git/patches/master/empty~
+f 51fcfcf16db2903f19ab4a4a3caacd297ea9f6cd  .git/patches/master/nonempty~
+f 9c18cc7abe6b87f18503714a80a677b4094eb457  .git/patches/master/add
+f bacb4aad8a55fe4e7aa58a9ae169990bb764069f  .git/patches/master/series
+f bc9ab2e0f5db99d483961e956e814d963f0309f8  .git/patches/master/modify
+f da39a3ee5e6b4b0d3255bfef95601890afd80709  .git/patches/master/empty-1~
+f da39a3ee5e6b4b0d3255bfef95601890afd80709  .git/patches/master/empty-2~
+f da39a3ee5e6b4b0d3255bfef95601890afd80709  .git/patches/master/status
+%% nonempty + empty (diffstat=true)
+% guilt new -f -s -m A commit message. nonempty
+% guilt pop
+All patches popped.
+% guilt 

Re: [PATCH] remote-hg: getbundle changed in mercurial 3.0

[Junio C Hamano]

William Giokas 1007...@gmail.com writes:

 In mercurial 3.0, getbundle was moved to the changegroup module, and
 gained a new argument. Due to this we cannot simply start using
 getbundle(...) imported from either one unconditionally, as that would
 cause errors in mercurial 3.0 without changing the syntax, and errors in
 mercurial 3.0 if we do change it.

 The try:except block at the beginning of git-remote-hg.py tries first to
 import mercurial.changegroup.getbundle, and if that fails we set the
 function 'getbundle' to work correctly with mercurial.repo.getbundle by
 removing the first argument.

 Signed-off-by: William Giokas 1007...@gmail.com
 ---

 I have tested this briefly with mercurial 3.0, but have not yet really run
 through its paces. The tests that are included in next do pass with
 mercurial 3.0.

  git-remote-hg.py | 9 -
  1 file changed, 8 insertions(+), 1 deletion(-)

 diff --git a/git-remote-hg.py b/git-remote-hg.py
 index 34cda02..3dc9e11 100755
 --- a/git-remote-hg.py
 +++ b/git-remote-hg.py
 @@ -14,6 +14,13 @@
  
  from mercurial import hg, ui, bookmarks, context, encoding, node, error, 
 extensions, discovery, util
  
 +try:
 +from mercurial.changegroup import getbundle
 +
 +except ImportError:
 +def getbundle(__empty__, **kwargs):
 +return repo.getbundle(**kwargs)
 +
  import re
  import sys
  import os
 @@ -985,7 +992,7 @@ def push_unsafe(repo, remote, parsed_refs, p_revs):
  if not checkheads(repo, remote, p_revs):
  return None
  
 -cg = repo.getbundle('push', heads=list(p_revs), common=common)
 +cg = getbundle(repo, 'push', heads=list(p_revs), common=common)
  
  unbundle = remote.capable('unbundle')
  if unbundle:

Thanks.  I agree with you that this would probably be a better
way to future-proof the code without unconditionally including what
may not be used at all, as you said in the other thread.

I can be pursuaded to queue this particular patch for maintenance
tracks after 2.0 final to my tree, but I do not think I would want
to keep taking patches to this area myself in the longer run.

The way I envision the longer term shape of git-remote-hg.py in the
contrib/ is either one of these two:

 (1) manage it just like contrib/hooks/multimail/, keeping a
 reasonably fresh and known-to-be-good snapshot, while making it
 clear that my tree is not the authoritative copy people should
 work off of;

 (2) treat it just like contrib/emacs/vc-git.el, which found a
 better home and left my tree at 78513869 (emacs: Remove the no
 longer maintained vc-git package., 2009-02-07); or

The first one may be more preferrable between the two, if only
because distros would need time to adjust where they pick up the
source material to package up, but it still needs cooperation with
the authoritative upstream and this project to allow us to at
least learn when the good time to import such good snapshots.  In
the case of vc-git, the separation was not about lack of will to
coordinate, but because it was not necessary to keep it in my tree,
as the better home was where the users expect to find the latest
and greatest anyway.

If Felipe turns out to be a maintainer users do not trust for
remote-hg, it is possible that you and other people can maintain it
and work with git.git better by forking off of his tree.  It is far
early to know if that will be the case at this point, and I
personally do not think that will happen (no, I do not have a
concrete reason I can cite to explain why I think that way).

But even in such an extreme hypothetical case, the difference it
makes to my tree would only be to take (1) or (2) and point to that
forked remote-hg repository, instead of Felipe's, as the source of
the authoritative copy.  And I wouldn't be taking individual
patches directly to my tree in either case.
--
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

[GUILT v2 09/29] Test suite: properly check the exit status of commands.

[Per Cederqvist]

The cmd and shouldfail functions checked the exit status of the
replace_path function instead of the actual command that was running.
(The $? construct checks the exit status of the last command in a
pipeline, not the first command.)

Updated t-032.sh, which used shouldfail instead of cmd in one
place.  (The comment in the script makes it clear that the command is
expected to succeed.)

Signed-off-by: Per Cederqvist ced...@opera.com
---
 regression/scaffold | 17 +++--
 regression/t-032.sh |  2 +-
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/regression/scaffold b/regression/scaffold
index 5c8b73e..e4d7487 100644
--- a/regression/scaffold
+++ b/regression/scaffold
@@ -51,18 +51,23 @@ function filter_dd
 function cmd
 {
echo % $@
-   $@ 21 | replace_path  return 0
-   return 1
+   (
+   exec 31
+   rv=`(($@ 21; echo $? 4) | replace_path 3 ) 41`
+   exit $rv
+   )
+   return $?
 }
 
 # usage: shouldfail cmd..
 function shouldfail
 {
echo % $@
-   (
-   $@ 21 || return 0
-   return 1
-   ) | replace_path
+   ! (
+   exec 31
+   rv=`(($@ 21; echo $? 4) | replace_path 3 ) 41`
+   exit $rv
+   )
return $?
 }
 
diff --git a/regression/t-032.sh b/regression/t-032.sh
index b1d5f19..bba401e 100755
--- a/regression/t-032.sh
+++ b/regression/t-032.sh
@@ -28,7 +28,7 @@ shouldfail guilt import -P foo3 foo
 cmd guilt import -P foo2 foo
 
 # ok
-shouldfail guilt import foo
+cmd guilt import foo
 
 # duplicate patch name (implicit)
 shouldfail guilt import foo
-- 
1.8.3.1

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

[GUILT v2 13/29] Check that guilt header '.*' fails.

[Per Cederqvist]

Signed-off-by: Per Cederqvist ced...@opera.com
---
 regression/t-028.out | 7 +++
 regression/t-028.sh  | 4 
 2 files changed, 11 insertions(+)

diff --git a/regression/t-028.out b/regression/t-028.out
index 1564c09..ea72a3a 100644
--- a/regression/t-028.out
+++ b/regression/t-028.out
@@ -49,3 +49,10 @@ Signed-off-by: Commiter Name commiter@email
 
 % guilt header non-existant
 Patch non-existant is not in the series
+% guilt header .*
+.* does not uniquely identify a patch. Did you mean any of these?
+  modify
+  add
+  remove
+  mode
+  patch-with-some-desc
diff --git a/regression/t-028.sh b/regression/t-028.sh
index 88e9adb..2ce0378 100755
--- a/regression/t-028.sh
+++ b/regression/t-028.sh
@@ -31,4 +31,8 @@ done
 
 shouldfail guilt header non-existant
 
+# This is an evil variant of a non-existant patch.  However, this
+# patch name is a regexp that just happens to match an existing patch.
+shouldfail guilt header '.*'
+
 # FIXME: how do we check that -e works?
-- 
1.8.3.1

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

[GUILT v2 11/29] test suite: remove pointless redirection.

[Per Cederqvist]

The shouldfail function already redirects stderr to stdout, so there
is no need to do the same in t-028.sh and t-021.sh.

Signed-off-by: Per Cederqvist ced...@opera.com
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
---
 regression/t-021.sh | 2 +-
 regression/t-025.sh | 2 +-
 regression/t-028.sh | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/regression/t-021.sh b/regression/t-021.sh
index 6337d7b..614e870 100755
--- a/regression/t-021.sh
+++ b/regression/t-021.sh
@@ -61,7 +61,7 @@ for n in `_seq -2 $npatches`; do
if [ $n -gt 0 ]; then
cmd guilt pop -n $n
else
-   shouldfail guilt pop -n $n 21
+   shouldfail guilt pop -n $n
fi
 
cmd list_files
diff --git a/regression/t-025.sh b/regression/t-025.sh
index 3824608..985fed4 100755
--- a/regression/t-025.sh
+++ b/regression/t-025.sh
@@ -44,7 +44,7 @@ shouldfail guilt new white space
 cmd list_files
 
 for pname in prepend mode /abc ./blah ../blah abc/./blah abc/../blah abc/. 
abc/.. abc/ ; do
-   shouldfail guilt new $pname 21
+   shouldfail guilt new $pname
 
cmd list_files
 done
diff --git a/regression/t-028.sh b/regression/t-028.sh
index 8480100..88e9adb 100755
--- a/regression/t-028.sh
+++ b/regression/t-028.sh
@@ -29,6 +29,6 @@ guilt series | while read n; do
cmd guilt header $n
 done
 
-shouldfail guilt header non-existant 21
+shouldfail guilt header non-existant
 
 # FIXME: how do we check that -e works?
-- 
1.8.3.1

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

[GUILT v2 10/29] Run test_failed if the exit status of a test script is bad.

[Per Cederqvist]

There were two problems with the old code:

 - Since set -e is in effect (that is set in scaffold) the run-test
   script exited immediately if a t-*.sh script failed.  This is not
   nice, as we want the error report that test_failed prints.

 - The code ran cd - between running the t-*.sh script and checking
   the exit status, so the exit status was lost.  (Actually, the exit
   status was saved in $ERR, but nothing ever looked at $ERR.)

Signed-off-by: Per Cederqvist ced...@opera.com
---
 regression/run-tests | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/regression/run-tests b/regression/run-tests
index a10e796..8e0af9f 100755
--- a/regression/run-tests
+++ b/regression/run-tests
@@ -55,11 +55,15 @@ function run_test
 
# run the test
cd $REPODIR  /dev/null
-   $REG_DIR/t-$1.sh 21  $LOGFILE
-   ERR=$?
+   if $REG_DIR/t-$1.sh 21  $LOGFILE; then
+   ERR=false
+   else
+   ERR=true
+   fi
+
cd -  /dev/null
 
-   [ $? -ne 0 ]  test_failed
+   $ERR  test_failed
diff -u t-$1.out $LOGFILE || test_failed
 
echo done.
-- 
1.8.3.1

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

[GUILT v2 12/29] guilt header: more robust header selection.

[Per Cederqvist]

If you run something like guilt header '.*' the command would crash,
because the grep comand that tries to ensure that the patch exist
would detect a match, but the later code expected the match to be
exact.

Fixed by comparing exact strings.

And as a creeping feature guilt header will now try to use the
supplied patch name as an unachored regexp if no exact match was
found.  If the regexp yields a unique match, it is used; if more than
one patch matches, the names of all patches are listed and the command
fails.  (Exercise left to the reader: generalized this so that guilt
push also accepts a unique regular expression.)

Signed-off-by: Per Cederqvist ced...@opera.com
---
 guilt-header | 28 +---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/guilt-header b/guilt-header
index 41e00cc..4701b31 100755
--- a/guilt-header
+++ b/guilt-header
@@ -45,10 +45,32 @@ esac
 [ -z $patch ]  die No patches applied.
 
 # check that patch exists in the series
-ret=`get_full_series | grep -e ^$patch\$ | wc -l`
-if [ $ret -eq 0 ]; then
-   die Patch $patch is not in the series
+full_series=`get_tmp_file series`
+get_full_series  $full_series
+found_patch=
+while read x; do
+   if [ $x = $patch ]; then
+   found_patch=$patch
+   break
+   fi
+done  $full_series
+if [ -z $found_patch ]; then
+   TMP_MATCHES=`get_tmp_file series`
+   grep $patch  $full_series  $TMP_MATCHES
+   nr=`wc -l  $TMP_MATCHES`
+   if [ $nr -gt 1 ]; then
+   echo $patch does not uniquely identify a patch. Did you mean 
any of these? 2
+   sed 's/^/  /' $TMP_MATCHES 2
+   rm -f $TMP_MATCHES
+   exit 1
+   elif [ $nr -eq 0 ]; then
+   rm -f $TMP_MATCHES
+   die Patch $patch is not in the series
+   fi
+   found_patch=`cat $TMP_MATCHES`
+   rm -f $TMP_MATCHES
 fi
+patch=$found_patch
 
 # FIXME: warn if we're editing an applied patch
 
-- 
1.8.3.1

--
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: Error using git-remote-hg

[Felipe Contreras]

William Giokas wrote:
 On Tue, May 13, 2014 at 02:09:55PM -0500, Felipe Contreras wrote:
  William Giokas wrote:
   On Tue, May 13, 2014 at 10:30:26AM -0700, Junio C Hamano wrote:
  
Why do we import changegroup unconditionally, even though it
is only used in the new codepath meant only for version 3.0 or
higher, not inside the if block that decides if we need that
module? 
  
   changegroup is prefectly /okay/ to import unconditionally, though as you
   say it will never be used.
  
  As you say, it's perfectly OK.
 
 But wrong. Yes, it works, but it's not how it should be done when we
 have a code review such as this. It should simply not be done and makes
 no sense to do with an 'if check ver; else' kind of thing later in the
 application.

That's exactly how it should be done. Maybe this visualization helps

  from mercurial import hg, ui, bookmarks, ...# for hg = 3.0
  from mercurial import node, error, extensions, ...  # for hg = 3.0
  from mercurial import changegroup   # for hg = 3.0

  if check_version(3, 0):
  cg = changegroup.getbundle(repo, 'push', ...# for hg = 3.0
  else:
  cg = repo.getbundle('push', heads=list(p_revs)  # for hg  3.0

Eventually the code would become:

  from mercurial import hg, ui, bookmarks, ...# for hg = 3.0
  from mercurial import node, error, extensions, ...  # for hg = 3.0
  from mercurial import changegroup   # for hg = 3.0

  cg = changegroup.getbundle(repo, 'push', ...# for hg = 3.0

The fact that at some point 'import changegroup' was not needed is
irrelevant.

Primarily we want to support the current version of Mercurial.
Secondarily we want to support older versions. That's why we add the
repo.getbundle() code (ass an addendum to the core part).

   We can also be even more explicit with what we import by doing something
   like::
   
 try:
 from mercurial.changegroup import getbundle
   
 except ImportError:
 def getbundle(__empty__, **kwargs):
 return repo.getbundle(**kwargs)
  
  We could try that, but that would assume we want to maintain getbundle()
  for the long run, and I personally don't want to do that. I would rather
  contact the Mercurial developers about ways in which the push() method
  can be improved so we don't need to have our own version. Hopefully
  after it's improved we wouldn't have to call getbundle().
 
 Assuming that mercurial 3.0 will not change, then this should never
 need to change.

But it should. Otherwise the code will keep having more and more hacks
and it will become less and less maintainable.

That's why we don't have code for hg 1.0; because it would require too
many hacks, and nobody cared anyway.

Just like nobody cares about hg 1.0, eventually nobody will care about
hg 2.0.

 Changes in 'getbundle' upstream would require changes either way.

I doubt we will see any more changes in getbundle, at least not until
4.0, and hopefully by then we wouldn't be using it anyway. I am willing
 to bet we won't see those changes.

  Moreover, eventually there will be a Mercurial 4.0, even 5.0, and at
  some point we would want to remove the hacks for older versions. When we
  do so we would want the import to remain unconditionally, and remove the
  'check_version(3, 0)' which is already helping to explain what the code
  is for without the need of comments.
 
 The same exact thing can be done with this. In fact, it would probably
 allow us to have better future-proofing with regards to new versions of
 mercurial, there would just be different try:except statements at the
 beginning.

No, your code doesn't show that this is for versiosn = 3.0,
'check_version(3, 0)' does.

Plus, when we remove this code my version makes it straight forward:

-if check_version(3, 0):
-cg = changegroup.getbundle(repo, 'push', ...
-else:
-cg = repo.getbundle('push', heads=list(p_revs), ...
+cg = changegroup.getbundle(repo, 'push', ...

Not so with your code:

-
-try:
-from mercurial.changegroup import getbundle
-
-except ImportError:
-def getbundle(__empty__, **kwargs):
-return repo.getbundle(**kwargs)
+from mercurial import getbundle
 
 import re
 import sys
@@ -1036,7 +1030,7 @@ def push_unsafe(repo, remote, ...
 if not checkheads(repo, remote, p_revs):
 return None
 
-cg = getbundle(repo, 'push', heads=list(p_revs), ...
+cg = changegroup.getbundle(repo, 'push', ...


   I was really sad to see that, and didn't have time to really look at it
   because of work and other projects, but I hope this presents a better
   solution than the current patch.
  
  Either way Junio doesn't maintain this code, I do. And it's not
  maintained in git.git, git's maintained out-of-tree (thanks to Junio's
  decisions).
 
 I still see it in git.git, and I will contribute this upstream for as
 long as it is in the tree.

And what happens when it's eventually removed?

 If you 

[GUILT v2 14/29] Use git check-ref-format to validate patch names.

[Per Cederqvist]

The valid_patchname now lets git check-ref-format do its job instead
of trying (and failing) to implement the same rules.  See
git-check-ref-format(1) for a list of the rules.

Refer to the git-check-ref-format(1) man page in the error messages
produced when valid_patchname indicates that the name is bad.

Added testcases that breaks most of the rules in that man-page.

Git version 1.8.5 no longer allows the single character @ as a
branch name.  Guilt always rejects that name, for increased
compatibility.

Signed-off-by: Per Cederqvist ced...@opera.com
---
 guilt|  21 ++-
 guilt-fork   |   2 +-
 guilt-import |   2 +-
 guilt-new|   2 +-
 regression/t-025.out | 426 +--
 regression/t-025.sh  |  12 +-
 regression/t-032.out |   4 +-
 7 files changed, 446 insertions(+), 23 deletions(-)

diff --git a/guilt b/guilt
index 3fc524e..23cc2da 100755
--- a/guilt
+++ b/guilt
@@ -132,14 +132,19 @@ fi
 # usage: valid_patchname patchname
 valid_patchname()
 {
-   case $1 in
-   /*|./*|../*|*/./*|*/../*|*/.|*/..|*/|*\ *|*\*)
-   return 1;;
-   *:*)
-   return 1;;
-   *)
-   return 0;;
-   esac
+   if git check-ref-format --allow-onelevel $1; then
+   # Starting with Git version 1.8.5, a branch cannot be
+   # the single character @.  Make sure guilt rejects
+   # that name even if we are currently using an older
+   # version of Git.  This ensures that the test suite
+   # runs fine using any version of Git.
+   if [ $1 = @ ]; then
+   return 1
+   fi
+   return 0
+   else
+   return 1
+   fi
 }
 
 get_branch()
diff --git a/guilt-fork b/guilt-fork
index a85d391..6447e55 100755
--- a/guilt-fork
+++ b/guilt-fork
@@ -37,7 +37,7 @@ else
 fi
 
 if ! valid_patchname $newpatch; then
-   die The specified patch name contains invalid characters (:).
+   die The specified patch name is invalid according to 
git-check-ref-format(1).
 fi
 
 if [ -e $GUILT_DIR/$branch/$newpatch ]; then
diff --git a/guilt-import b/guilt-import
index 3e9b3bb..928e325 100755
--- a/guilt-import
+++ b/guilt-import
@@ -40,7 +40,7 @@ if [ -e $GUILT_DIR/$branch/$newname ]; then
 fi
 
 if ! valid_patchname $newname; then
-   die The specified patch name contains invalid characters (:).
+   die The specified patch name is invalid according to 
git-check-ref-format(1).
 fi
 
 # create any directories as needed
diff --git a/guilt-new b/guilt-new
index 9528438..9f7fa44 100755
--- a/guilt-new
+++ b/guilt-new
@@ -64,7 +64,7 @@ fi
 
 if ! valid_patchname $patch; then
disp Patchname is invalid. 2
-   die it cannot begin with '/', './' or '../', or contain /./, /../, or 
whitespace
+   die It must follow the rules in git-check-ref-format(1).
 fi
 
 # create any directories as needed
diff --git a/regression/t-025.out b/regression/t-025.out
index 7811ab1..01bc406 100644
--- a/regression/t-025.out
+++ b/regression/t-025.out
@@ -141,7 +141,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709  
.git/patches/master/prepend
 r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8  .git/refs/patches/master/prepend
 % guilt new white space
 Patchname is invalid.
-it cannot begin with '/', './' or '../', or contain /./, /../, or whitespace
+It must follow the rules in git-check-ref-format(1).
 % list_files
 d .git/patches
 d .git/patches/master
@@ -211,7 +211,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709  
.git/patches/master/prepend
 r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8  .git/refs/patches/master/prepend
 % guilt new /abc
 Patchname is invalid.
-it cannot begin with '/', './' or '../', or contain /./, /../, or whitespace
+It must follow the rules in git-check-ref-format(1).
 % list_files
 d .git/patches
 d .git/patches/master
@@ -235,7 +235,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709  
.git/patches/master/prepend
 r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8  .git/refs/patches/master/prepend
 % guilt new ./blah
 Patchname is invalid.
-it cannot begin with '/', './' or '../', or contain /./, /../, or whitespace
+It must follow the rules in git-check-ref-format(1).
 % list_files
 d .git/patches
 d .git/patches/master
@@ -259,7 +259,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709  
.git/patches/master/prepend
 r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8  .git/refs/patches/master/prepend
 % guilt new ../blah
 Patchname is invalid.
-it cannot begin with '/', './' or '../', or contain /./, /../, or whitespace
+It must follow the rules in git-check-ref-format(1).
 % list_files
 d .git/patches
 d .git/patches/master
@@ -283,7 +283,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709  
.git/patches/master/prepend
 r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8  .git/refs/patches/master/prepend
 % guilt new abc/./blah
 Patchname is invalid.

[GUILT v2 15/29] Produce legal patch names in guilt-import-commit.

[Per Cederqvist]

Try harder to create patch names that adhere to the rules in
git-check-ref-format(1) when deriving a patch name from the commit
message.  Verify that the derived name using git check-ref-format,
and as a final fallback simply use the patch name x (to ensure that
the code is future-proof in case new rules are added in the future).

Always append a .patch suffix to the patch name.

Added test cases.

Signed-off-by: Per Cederqvist ced...@opera.com
---
 guilt-import-commit  |  20 +-
 regression/t-034.out | 567 +++
 regression/t-034.sh  |  71 +++
 3 files changed, 656 insertions(+), 2 deletions(-)
 create mode 100644 regression/t-034.out
 create mode 100755 regression/t-034.sh

diff --git a/guilt-import-commit b/guilt-import-commit
index f14647c..6260c56 100755
--- a/guilt-import-commit
+++ b/guilt-import-commit
@@ -28,19 +28,35 @@ disp Current head: `git rev-parse \`git_branch\`` 2
 for rev in `git rev-list $rhash`; do
s=`git log --pretty=oneline -1 $rev | cut -c 42-`
 
+   # Try to convert the first line of the commit message to a
+   # valid patch name.
fname=`echo $s | sed -e s//and/g -e s/[ :]/_/g -e s,[/\\],-,g \
-e s/['\\[{}]//g -e 's/]//g' -e 's/\*/-/g' \
-   -e 's/\?/-/g' | tr A-Z a-z`
+   -e 's/\?/-/g' -e 's/\.\.\.*/./g' -e 's/^\.//' \
+   -e 's/\.patch$//' -e 's/\.$//' | tr A-Z a-z`
+
+   if ! valid_patchname $fname; then
+   # Try harder to make it a legal commit name by
+   # removing all but a few safe characters.
+   fname=`echo $fname|tr -d -c _a-zA-Z0-9---/\\n`
+   fi
+   if ! valid_patchname $fname; then
+   # If we failed to derive a legal patch name, use the
+   # name x.  (If this happens, we likely have to
+   # append a suffix to make the name unique.)
+   fname=x
+   fi
 
disp Converting `echo $rev | cut -c 1-8` as $fname
 
mangle_prefix=1
fname_base=$fname
-   while [ -f $GUILT_DIR/$branch/$fname ]; do
+   while [ -f $GUILT_DIR/$branch/$fname.patch ]; do
fname=$fname_base-$mangle_prefix
mangle_prefix=`expr $mangle_prefix + 1`
disp Patch under that name exists...trying '$fname'
done
+   fname=$fname.patch
 
(
do_make_header $rev
diff --git a/regression/t-034.out b/regression/t-034.out
new file mode 100644
index 000..7bc9459
--- /dev/null
+++ b/regression/t-034.out
@@ -0,0 +1,567 @@
+% setup_git_repo
+% git tag base
+% create_commit a The sequence /. is forbidden.
+[master eebb76e] The sequence /. is forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+ create mode 100644 a
+% create_commit a The sequence .lock/ is forbidden.
+[master 45e81b5] The sequence .lock/ is forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a A/component/may/not/end/in/foo.lock
+[master bbf3f59] A/component/may/not/end/in/foo.lock
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a Two consecutive dots (..) is forbidden.
+[master 1535e67] Two consecutive dots (..) is forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a Check/multiple/../dots/./foo..patch
+[master 48eb60c] Check/multiple/../dots/./foo..patch
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a Space is forbidden.
+[master 10dea83] Space is forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a Tilde~is~forbidden.
+[master 70a83b7] Tilde~is~forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a Caret^is^forbidden.
+[master ee6ef2c] Caret^is^forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a Colon:is:forbidden.
+[master c077fe2] Colon:is:forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a Delisforbidden.
+[master 589ee30] Delisforbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% git branch some-branch
+% git tag some-tag
+% create_commit a Ctrl
isforbidden.
+[master e63cdde] Ctrl
isforbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a CR
is
also
forbidden.
+[master 21ad093] CR
is
also
forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a Question-mark?is?forbidden.
+[master be2fa9b] Question-mark?is?forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a Asterisk*is*forbidden.
+[master af7b50f] Asterisk*is*forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a Open[bracket[is[forbidden.
+[master 689f618] 

[GUILT v2 16/29] Fix backslash handling when creating names of imported patches.

[Per Cederqvist]

The 'echo %s' construct sometimes processes escape sequences.  (This
happens, for instance, under Ubuntu 14.04 when /bin/sh is actually
dash.)  We don't want that to happen when we are importing commits, so
use 'printf %s $s' instead.

(The -E option of bash that explicitly disables backslash expansion is
not portable; it is not supported by dash.)

Signed-off-by: Per Cederqvist ced...@opera.com
---
 guilt-import-commit  |  2 +-
 regression/t-034.out | 14 +++---
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/guilt-import-commit b/guilt-import-commit
index 6260c56..45f2404 100755
--- a/guilt-import-commit
+++ b/guilt-import-commit
@@ -30,7 +30,7 @@ for rev in `git rev-list $rhash`; do
 
# Try to convert the first line of the commit message to a
# valid patch name.
-   fname=`echo $s | sed -e s//and/g -e s/[ :]/_/g -e s,[/\\],-,g \
+   fname=`printf %s $s | sed -e s//and/g -e s/[ :]/_/g -e 
s,[/\\],-,g \
-e s/['\\[{}]//g -e 's/]//g' -e 's/\*/-/g' \
-e 's/\?/-/g' -e 's/\.\.\.*/./g' -e 's/^\.//' \
-e 's/\.patch$//' -e 's/\.$//' | tr A-Z a-z`
diff --git a/regression/t-034.out b/regression/t-034.out
index 7bc9459..bda4399 100644
--- a/regression/t-034.out
+++ b/regression/t-034.out
@@ -236,7 +236,7 @@ Date:   Mon Jan 1 00:00:00 2007 +
 About to begin conversion...
 Current head: 2a8b1889aa5066193bac978e6bf5073ffcfa6541
 Converting 2a8b1889 as can-have-embedded-single-slashes
-Converting 0a46f8fa as backslash-isorbidden
+Converting 0a46f8fa as backslash-is-forbidden
 Converting aedb74fd as x
 Converting 30187ed0 as cannot@have@the@sequence@at-brace
 Converting 106e8e5a as cannot_end_in_
@@ -300,7 +300,7 @@ Applying patch..cannot@have@the@sequence@at-brace.patch
 Patch applied.
 Applying patch..x.patch
 Patch applied.
-Applying patch..backslash-isorbidden.patch
+Applying patch..backslash-is-forbidden.patch
 Patch applied.
 Applying patch..can-have-embedded-single-slashes.patch
 Patch applied.
@@ -311,7 +311,7 @@ Date:   Mon Jan 1 00:00:00 2007 +
 
 Can/have/embedded/single/slashes
 
-commit 7c3ffa4f940c862e9f11f5d4a5ae421f7a8d3141 
(refs/patches/master/backslash-isorbidden.patch)
+commit 7c3ffa4f940c862e9f11f5d4a5ae421f7a8d3141 
(refs/patches/master/backslash-is-forbidden.patch)
 Author: Author Name author@email
 Date:   Mon Jan 1 00:00:00 2007 +
 
@@ -518,8 +518,6 @@ d .git/patches/master
 d .git/refs/patches
 d .git/refs/patches/master
 f 06beca7069b9e576bd431f65d13862ed5d3e2a0f  
.git/patches/master/ctrlisforbidden.patch
-f 08267ec6783ea9d1adae55b275198f7594764ed0  .git/patches/master/series
-f 08267ec6783ea9d1adae55b275198f7594764ed0  .git/patches/master/status
 f 09b7e9be44ae5ec3a4bb30f5ee9d4ebc2c042f64  
.git/patches/master/two_consecutive_dots_(.)_is_forbidden.patch
 f 0b971c9a17aeca2319c93d700ffd98acc2a93451  
.git/patches/master/question-mark-is-forbidden.patch
 f 2b8392f63d61efc12add554555adae30883993cc  
.git/patches/master/cannot-end-in-slash-.patch
@@ -529,7 +527,7 @@ f 34e07c584032df137f19bdb66d93f316f00a5ac8  
.git/patches/master/tildeisforbidden
 f 49bab499826b63deb2bd704629d60c7268c57aee  
.git/patches/master/the_sequence_-._is_forbidden.patch
 f 5bcddb8ccb6e6e5e8a61e9e56cb2e0f70cbab2f5  
.git/patches/master/cannot@have@the@sequence@at-brace.patch
 f 637b982fe14a240de181ae63226b27e0c406b3dc  
.git/patches/master/asterisk-is-forbidden.patch
-f 698f8a7d41a64e3b6be1a3eba86574078b22a5f3  
.git/patches/master/backslash-isorbidden.patch
+f 698f8a7d41a64e3b6be1a3eba86574078b22a5f3  
.git/patches/master/backslash-is-forbidden.patch
 f 7b103c3c7ae298cd2334f6f49da48bae1424f77b  
.git/patches/master/crisalsoforbidden.patch
 f 9b810b8c63779c51d2e7f61ab59cd49835041563  .git/patches/master/x.patch
 f a22958d9ae9976fd7b2b5a9d0bcd44bf7ad9b08a  
.git/patches/master/caretisforbidden.patch
@@ -537,6 +535,8 @@ f ab325bf5a432937fc6f231d3e8a773a62d53952b  
.git/patches/master/multiple-slashes
 f cb9cffbd4465bddee266c20ccebd14eb687eaa89  
.git/patches/master/delisforbidden.patch
 f d0885a1a1fdee0fd1e4fedce3f7acd3100540bc4  
.git/patches/master/openbracketisforbidden.patch
 f d2903523fb66a346596eabbdd1bda4e52b266440  
.git/patches/master/check-multiple-.-dots-.-foo.patch
+f da90de1c84138194524994e0bc3bc4ca8189c999  .git/patches/master/series
+f da90de1c84138194524994e0bc3bc4ca8189c999  .git/patches/master/status
 f dfc11f76394059909671af036598c5fbe33001ba  
.git/patches/master/space_is_forbidden.patch
 f e47474c52d6c893f36d0457f885a6dd1267742bb  
.git/patches/master/colon_is_forbidden.patch
 f e7a5f8912592d9891e6159f5827c8b4f372cc406  
.git/patches/master/the_sequence_.lock-_is_forbidden.patch
@@ -548,7 +548,7 @@ r 1626a11d979a1e9e775c766484172212277153df  
.git/refs/patches/master/asterisk-is
 r 3a0d5ccef0359004fcaa9cee98fbd6a2c4432e74  
.git/refs/patches/master/tildeisforbidden.patch
 r 434e07cacdd8e7eb4723e67cb2d100b3a4121a3a  

[GUILT v2 17/29] guilt graph no longer loops when no patches are applied.

[Per Cederqvist]

Give an error message if no patches are applied.  Added a test case
that never terminates unless this fix is applied.

Signed-off-by: Per Cederqvist ced...@opera.com
---
 guilt-graph  |  9 +++--
 regression/t-033.out |  3 +++
 regression/t-033.sh  | 13 +
 3 files changed, 23 insertions(+), 2 deletions(-)
 create mode 100644 regression/t-033.out
 create mode 100755 regression/t-033.sh

diff --git a/guilt-graph b/guilt-graph
index b3469dc..56d0e77 100755
--- a/guilt-graph
+++ b/guilt-graph
@@ -17,8 +17,13 @@ fi
 
 patchname=$1
 
-bottom=`git rev-parse refs/patches/$branch/$(head_n 1  $applied)`
-base=`git rev-parse $bottom^`
+bottompatch=$(head_n 1  $applied)
+if [ -z $bottompatch ]; then
+   echo No patch applied. 2
+   exit 1
+fi
+
+base=`git rev-parse refs/patches/${branch}/${bottompatch}^`
 
 if [ -z $patchname ]; then
top=`git rev-parse HEAD`
diff --git a/regression/t-033.out b/regression/t-033.out
new file mode 100644
index 000..76613f9
--- /dev/null
+++ b/regression/t-033.out
@@ -0,0 +1,3 @@
+% setup_repo
+% guilt graph
+No patch applied.
diff --git a/regression/t-033.sh b/regression/t-033.sh
new file mode 100755
index 000..a3a8981
--- /dev/null
+++ b/regression/t-033.sh
@@ -0,0 +1,13 @@
+#!/bin/bash
+#
+# Test the graph code
+#
+
+source $REG_DIR/scaffold
+
+cmd setup_repo
+
+# Check that guilt graph gives a proper No patch applied error
+# message when no patches are applied.  (An older version of guilt
+# used to enter an endless loop in this situation.)
+shouldfail guilt graph
-- 
1.8.3.1

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

[GUILT v2 19/29] Check that guilt graph works when working on a branch with a comma.

[Per Cederqvist]

git branch names can contain commas.  Check that guilt graph works
even in that case.

Signed-off-by: Per Cederqvist ced...@opera.com
---
 regression/t-033.out | 65 
 regression/t-033.sh  | 39 +++
 2 files changed, 104 insertions(+)

diff --git a/regression/t-033.out b/regression/t-033.out
index 76613f9..3d1c61f 100644
--- a/regression/t-033.out
+++ b/regression/t-033.out
@@ -1,3 +1,68 @@
 % setup_repo
 % guilt graph
 No patch applied.
+%% Testing branch a,graph
+% git checkout -b a,graph master
+Switched to a new branch 'a,graph'
+% guilt init
+% guilt new a.patch
+% guilt pop
+All patches popped.
+% guilt push
+Applying patch..a.patch
+Patch applied.
+% guilt graph
+digraph G {
+# checking rev 95275d7c05c6a6176d3941374115b91272877f6c
+   95275d7c05c6a6176d3941374115b91272877f6c [label=a.patch]
+}
+% git add file.txt
+% guilt refresh
+Patch a.patch refreshed
+% guilt pop
+All patches popped.
+% guilt push
+Applying patch..a.patch
+Patch applied.
+% guilt graph
+digraph G {
+# checking rev ff2775f8d1dc753f635830adcc3a067e0b681e2d
+   ff2775f8d1dc753f635830adcc3a067e0b681e2d [label=a.patch]
+}
+%% Adding an unrelated file in a new patch. No deps.
+% guilt new b.patch
+% git add file2.txt
+% guilt refresh
+Patch b.patch refreshed
+% guilt pop
+Now at a.patch.
+% guilt push
+Applying patch..b.patch
+Patch applied.
+% guilt graph
+digraph G {
+# checking rev c7014443c33d2b0237293687ceb9cbd38313df65
+   c7014443c33d2b0237293687ceb9cbd38313df65 [label=b.patch]
+# checking rev ff2775f8d1dc753f635830adcc3a067e0b681e2d
+   ff2775f8d1dc753f635830adcc3a067e0b681e2d [label=a.patch]
+}
+%% Changing a file already changed in the first patch adds a dependency.
+% guilt new c.patch
+% git add file.txt
+% guilt refresh
+Patch c.patch refreshed
+% guilt pop
+Now at b.patch.
+% guilt push
+Applying patch..c.patch
+Patch applied.
+% guilt graph
+digraph G {
+# checking rev 891bc14b5603474c9743fd04f3da888644413dc5
+   891bc14b5603474c9743fd04f3da888644413dc5 [label=c.patch]
+# checking rev c7014443c33d2b0237293687ceb9cbd38313df65
+   c7014443c33d2b0237293687ceb9cbd38313df65 [label=b.patch]
+# checking rev ff2775f8d1dc753f635830adcc3a067e0b681e2d
+   ff2775f8d1dc753f635830adcc3a067e0b681e2d [label=a.patch]
+   891bc14b5603474c9743fd04f3da888644413dc5 - 
ff2775f8d1dc753f635830adcc3a067e0b681e2d; // ?
+}
diff --git a/regression/t-033.sh b/regression/t-033.sh
index a3a8981..fac081e 100755
--- a/regression/t-033.sh
+++ b/regression/t-033.sh
@@ -3,6 +3,13 @@
 # Test the graph code
 #
 
+function fixup_time_info
+{
+   cmd guilt pop
+   touch -a -m -t $TOUCH_DATE .git/patches/a,graph/$1
+   cmd guilt push
+}
+
 source $REG_DIR/scaffold
 
 cmd setup_repo
@@ -11,3 +18,35 @@ cmd setup_repo
 # message when no patches are applied.  (An older version of guilt
 # used to enter an endless loop in this situation.)
 shouldfail guilt graph
+
+echo %% Testing branch a,graph
+cmd git checkout -b a,graph master
+
+cmd guilt init
+
+cmd guilt new a.patch
+
+fixup_time_info a.patch
+cmd guilt graph
+
+cmd echo a  file.txt
+cmd git add file.txt
+cmd guilt refresh
+fixup_time_info a.patch
+cmd guilt graph
+
+echo %% Adding an unrelated file in a new patch. No deps.
+cmd guilt new b.patch
+cmd echo b  file2.txt
+cmd git add file2.txt
+cmd guilt refresh
+fixup_time_info b.patch
+cmd guilt graph
+
+echo %% Changing a file already changed in the first patch adds a dependency.
+cmd guilt new c.patch
+cmd echo c  file.txt
+cmd git add file.txt
+cmd guilt refresh
+fixup_time_info c.patch
+cmd guilt graph
-- 
1.8.3.1

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

[GUILT v2 20/29] guilt graph: Handle patch names containing quotes.

[Per Cederqvist]

Quote quotes with a backslash in the guilt graph output.  Otherwise,
the dot file could contain syntax errors.

Added a test case.

Signed-off-by: Per Cederqvist ced...@opera.com
---
 guilt-graph  |  2 ++
 regression/t-033.out | 22 ++
 regression/t-033.sh  |  9 +
 3 files changed, 33 insertions(+)

diff --git a/guilt-graph b/guilt-graph
index 924a63e..0857e0d 100755
--- a/guilt-graph
+++ b/guilt-graph
@@ -57,6 +57,8 @@ while [ $current != $base ]; do
 }`
[ -z $pname ]  pname=?
 
+   pname=`printf \%s\ \$pname\ | sed 's/\/\/g'`
+
disp # checking rev $current
disp   \$current\ [label=\$pname\]
 
diff --git a/regression/t-033.out b/regression/t-033.out
index 3d1c61f..c120d4f 100644
--- a/regression/t-033.out
+++ b/regression/t-033.out
@@ -66,3 +66,25 @@ digraph G {
ff2775f8d1dc753f635830adcc3a067e0b681e2d [label=a.patch]
891bc14b5603474c9743fd04f3da888644413dc5 - 
ff2775f8d1dc753f635830adcc3a067e0b681e2d; // ?
 }
+% guilt new a-betterquicker'-patch.patch
+% git add file.txt
+% guilt refresh
+Patch a-betterquicker'-patch.patch refreshed
+% guilt pop
+Now at c.patch.
+% guilt push
+Applying patch..a-betterquicker'-patch.patch
+Patch applied.
+% guilt graph
+digraph G {
+# checking rev bc7df666a646739eaf559af23cab72f2bfd01f0e
+   bc7df666a646739eaf559af23cab72f2bfd01f0e 
[label=a-\betterquicker'-patch.patch]
+# checking rev 891bc14b5603474c9743fd04f3da888644413dc5
+   891bc14b5603474c9743fd04f3da888644413dc5 [label=c.patch]
+   bc7df666a646739eaf559af23cab72f2bfd01f0e - 
891bc14b5603474c9743fd04f3da888644413dc5; // ?
+# checking rev c7014443c33d2b0237293687ceb9cbd38313df65
+   c7014443c33d2b0237293687ceb9cbd38313df65 [label=b.patch]
+# checking rev ff2775f8d1dc753f635830adcc3a067e0b681e2d
+   ff2775f8d1dc753f635830adcc3a067e0b681e2d [label=a.patch]
+   891bc14b5603474c9743fd04f3da888644413dc5 - 
ff2775f8d1dc753f635830adcc3a067e0b681e2d; // ?
+}
diff --git a/regression/t-033.sh b/regression/t-033.sh
index fac081e..9fe1827 100755
--- a/regression/t-033.sh
+++ b/regression/t-033.sh
@@ -50,3 +50,12 @@ cmd git add file.txt
 cmd guilt refresh
 fixup_time_info c.patch
 cmd guilt graph
+
+# A patch name that contains funky characters, including unbalanced
+# quotes.
+cmd guilt new a-\betterquicker'-patch.patch
+cmd echo d  file.txt
+cmd git add file.txt
+cmd guilt refresh
+fixup_time_info a-\betterquicker'-patch.patch
+cmd guilt graph
-- 
1.8.3.1

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

[GUILT v2 18/29] guilt-graph: Handle commas in branch names.

[Per Cederqvist]

This fix relies on the fact that git branch names can not contain :.

Signed-off-by: Per Cederqvist ced...@opera.com
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
---
 guilt-graph | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/guilt-graph b/guilt-graph
index 56d0e77..924a63e 100755
--- a/guilt-graph
+++ b/guilt-graph
@@ -51,7 +51,7 @@ safebranch=`echo $branch|sed 's%/%/%g'`
 while [ $current != $base ]; do
pname=`git show-ref | sed -n -e 
 /^$current refs\/patches\/$safebranch/ {
-   s,^$current refs/patches/$branch/,,
+   s:^$current refs/patches/$branch/::
p
q
 }`
-- 
1.8.3.1

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

[GUILT v2 25/29] guilt push now fails when there are no more patches to push.

[Per Cederqvist]

This makes it easier to script operations on the entire queue, for
example run the test suite on each patch in the queue:

guilt pop -a;while guilt push; do make test||break; done

This brings guilt push in line with the push operation in Mercurial
Queues (hg qpush), which fails when there are no patches to apply.

Updated the test suite.

guilt push -a still does not fail.  (It successfully manages to
ensure that all patches are pushed, even if it did not have to do
anything to make it so.)

Signed-off-by: Per Cederqvist ced...@opera.com
---
 guilt-push   | 19 ++-
 regression/t-020.out | 89 
 regression/t-020.sh  | 13 +++-
 3 files changed, 113 insertions(+), 8 deletions(-)

diff --git a/guilt-push b/guilt-push
index 67687e7..39c125e 100755
--- a/guilt-push
+++ b/guilt-push
@@ -56,6 +56,12 @@ fi
 patch=$1
 [ ! -z $all ]  patch=-a
 
+# Treat guilt push as guilt push -n 1.
+if [ -z $patch ]; then
+   patch=1
+   num=t
+fi
+
 if [ $patch = -a ]; then
# we are supposed to push all patches, get the last one out of
# series
@@ -65,7 +71,7 @@ if [ $patch = -a ]; then
die There are no patches to push.
fi
 elif [ ! -z $num ]; then
-   # we are supposed to pop a set number of patches
+   # we are supposed to push a set number of patches
 
[ $patch -lt 0 ]  die Invalid number of patches to push.
 
@@ -78,11 +84,6 @@ elif [ ! -z $num ]; then
# clamp to minimum
[ $tidx -lt $eidx ]  eidx=$tidx
 
-elif [ -z $patch ]; then
-   # we are supposed to push only the next patch onto the stack
-
-   eidx=`wc -l  $applied`
-   eidx=`expr $eidx + 1`
 else
# we're supposed to push only up to a patch, make sure the patch is
# in the series
@@ -109,7 +110,11 @@ if [ $sidx -gt $eidx ]; then
else
disp File series fully applied, ends at patch `get_series | 
tail -n 1`
fi
-   exit 0
+   if [ -n $all ]; then
+   exit 0
+   else
+   exit 1
+   fi
 fi
 
 get_series | sed -n -e ${sidx},${eidx}p | while read p
diff --git a/regression/t-020.out b/regression/t-020.out
index 7e07efa..23cb9db 100644
--- a/regression/t-020.out
+++ b/regression/t-020.out
@@ -270,6 +270,95 @@ index 000..8baef1b
 +++ b/def
 @@ -0,0 +1 @@
 +abc
+% guilt push
+File series fully applied, ends at patch mode
+% guilt push -a
+File series fully applied, ends at patch mode
+% list_files
+d .git/patches
+d .git/patches/master
+d .git/refs/patches
+d .git/refs/patches/master
+f 22930c6d1f1938f298a4fca51c57e4b47171db21  .git/patches/master/mode
+f 413390f3906f16f30b054a4fb86c1e014b964504  .git/patches/master/remove
+f 71596bf71b72c2717e1aee378aabefbfa19ab7c8  .git/patches/master/status
+f 9c18cc7abe6b87f18503714a80a677b4094eb457  .git/patches/master/add
+f bacb4aad8a55fe4e7aa58a9ae169990bb764069f  .git/patches/master/series
+f bc9ab2e0f5db99d483961e956e814d963f0309f8  .git/patches/master/modify
+r 33633e7a1aa31972f125878baf7807be57b1672d  .git/refs/patches/master/modify
+r 37d588cc39848368810e88332bd03b083f2ce3ac  .git/refs/patches/master/add
+r ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba  .git/refs/patches/master/mode
+r ffb7faa126a6d91bcdd44a494f76b96dd860b8b9  .git/refs/patches/master/remove
+% git log -p
+commit ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba
+Author: Author Name author@email
+Date:   Mon Jan 1 00:00:00 2007 +
+
+patch mode
+
+diff --git a/def b/def
+old mode 100644
+new mode 100755
+
+commit ffb7faa126a6d91bcdd44a494f76b96dd860b8b9
+Author: Author Name author@email
+Date:   Mon Jan 1 00:00:00 2007 +
+
+patch remove
+
+diff --git a/abd b/abd
+deleted file mode 100644
+index fd3896d..000
+--- a/abd
 /dev/null
+@@ -1 +0,0 @@
+-‰öuؽáZâñeÏÈE„£WÀV¼/›U?Ú|¢@6¤8'H¸1G_˜Í§*·ðRҙ¤
ªÂ~·
+\ No newline at end of file
+
+commit 37d588cc39848368810e88332bd03b083f2ce3ac
+Author: Author Name author@email
+Date:   Mon Jan 1 00:00:00 2007 +
+
+patch add
+
+diff --git a/abd b/abd
+new file mode 100644
+index 000..fd3896d
+--- /dev/null
 b/abd
+@@ -0,0 +1 @@
++‰öuؽáZâñeÏÈE„£WÀV¼/›U?Ú|¢@6¤8'H¸1G_˜Í§*·ðRҙ¤
ªÂ~·
+\ No newline at end of file
+
+commit 33633e7a1aa31972f125878baf7807be57b1672d
+Author: Author Name author@email
+Date:   Mon Jan 1 00:00:00 2007 +
+
+patch modify
+
+diff --git a/def b/def
+index 8baef1b..7d69c2f 100644
+--- a/def
 b/def
+@@ -1 +1,2 @@
+ abc
++asjhfksad
+
+commit d4850419ccc1146c7169f500725ce504b9774ed0
+Author: Author Name author@email
+Date:   Mon Jan 1 00:00:00 2007 +
+
+initial
+
+Signed-off-by: Commiter Name commiter@email
+
+diff --git a/def b/def
+new file mode 100644
+index 000..8baef1b
+--- /dev/null
 b/def
+@@ -0,0 +1 @@
++abc
 % guilt pop --all
 All patches popped.
 % guilt push
diff --git a/regression/t-020.sh b/regression/t-020.sh
index 906aec6..0f9f85d 100755
--- a/regression/t-020.sh
+++ 

[GUILT v2 27/29] Minor testsuite fix.

[Per Cederqvist]

Fix remove_topic() in t-061.sh so that it doesn't print a git hash.

Signed-off-by: Per Cederqvist ced...@opera.com
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
---
 regression/t-061.out | 1 -
 regression/t-061.sh  | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/regression/t-061.out b/regression/t-061.out
index ef0f335..60ad56d 100644
--- a/regression/t-061.out
+++ b/regression/t-061.out
@@ -381,7 +381,6 @@ ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba commit 
refs/patches/master/mode
 ffb7faa126a6d91bcdd44a494f76b96dd860b8b9 commit
refs/patches/master/remove
 % guilt pop -a
 No patches applied.
-ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba
 % git checkout guilt/master
 Switched to branch guilt/master
 % guilt pop -a
diff --git a/regression/t-061.sh b/regression/t-061.sh
index 6192f1b..db26e12 100755
--- a/regression/t-061.sh
+++ b/regression/t-061.sh
@@ -15,7 +15,7 @@ old_style_branch() {
 
 remove_topic() {
cmd guilt pop -a
-   if git rev-parse --verify --quiet guilt/master
+   if git rev-parse --verify --quiet guilt/master /dev/null
then
cmd git checkout guilt/master
else
-- 
1.8.3.1

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

[GUILT v2 26/29] guilt pop now fails when there are no more patches to pop.

[Per Cederqvist]

This is analogous to how guilt push now fails when there are no more
patches to push.  Like push, the --all argument still succeeds even
if there was no need to pop anything.

Updated the test suite.

Signed-off-by: Per Cederqvist ced...@opera.com
---
 guilt-pop| 17 +++--
 regression/t-021.out |  2 ++
 regression/t-021.sh  |  6 ++
 regression/t-061.sh  |  6 +-
 4 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/guilt-pop b/guilt-pop
index f0e647f..191313e 100755
--- a/guilt-pop
+++ b/guilt-pop
@@ -49,9 +49,19 @@ fi
 patch=$1
 [ ! -z $all ]  patch=-a
 
+# Treat guilt pop as guilt pop -n 1.
+if [ -z $patch ]; then
+   patch=1
+   num=t
+fi
+
 if [ ! -s $applied ]; then
disp No patches applied.
-   exit 0
+   if [ $patch = -a ]; then
+   exit 0
+   else
+   exit 1
+   fi
 elif [ $patch = -a ]; then
# we are supposed to pop all patches
 
@@ -68,11 +78,6 @@ elif [ ! -z $num ]; then
# catch underflow
[ $eidx -lt 0 ]  eidx=0
[ $eidx -eq $sidx ]  die No patches requested to be removed.
-elif [ -z $patch ]; then
-   # we are supposed to pop only the current patch on the stack
-
-   sidx=`wc -l  $applied`
-   eidx=`expr $sidx - 1`
 else
# we're supposed to pop only up to a patch, make sure the patch is
# in the series
diff --git a/regression/t-021.out b/regression/t-021.out
index 9b42d9c..58be12f 100644
--- a/regression/t-021.out
+++ b/regression/t-021.out
@@ -287,6 +287,8 @@ index 000..8baef1b
 +++ b/def
 @@ -0,0 +1 @@
 +abc
+% guilt pop
+No patches applied.
 % guilt push --all
 Applying patch..modify
 Patch applied.
diff --git a/regression/t-021.sh b/regression/t-021.sh
index 614e870..e0d2dc1 100755
--- a/regression/t-021.sh
+++ b/regression/t-021.sh
@@ -23,6 +23,12 @@ guilt series | _tac | while read n ; do
 done
 
 #
+# pop when there is nothing to pop
+#
+
+shouldfail guilt pop
+
+#
 # push all
 #
 cmd guilt push --all
diff --git a/regression/t-061.sh b/regression/t-061.sh
index 1411baa..6192f1b 100755
--- a/regression/t-061.sh
+++ b/regression/t-061.sh
@@ -48,7 +48,11 @@ cmd list_files
 
 for i in `seq 5`
 do
-   cmd guilt pop
+   if [ $i -ge 5 ]; then
+   shouldfail guilt pop
+   else
+   cmd guilt pop
+   fi
cmd git for-each-ref
cmd guilt push
cmd git for-each-ref
-- 
1.8.3.1

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

[GUILT v2 22/29] The log.decorate setting should not influence patchbomb.

[Per Cederqvist]

Signed-off-by: Per Cederqvist ced...@opera.com
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
---
 guilt-patchbomb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/guilt-patchbomb b/guilt-patchbomb
index 1231418..164b10c 100755
--- a/guilt-patchbomb
+++ b/guilt-patchbomb
@@ -47,7 +47,7 @@ if [ $? -ne 0 ]; then
 fi
 
 # display the list of commits to be sent as patches
-git log --pretty=oneline $r | cut -c 1-8,41- | $pager
+git log --no-decorate --pretty=oneline $r | cut -c 1-8,41- | $pager
 
 _disp Are these what you want to send? [Y/n] 
 read n
-- 
1.8.3.1

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

[GUILT v2 23/29] The log.decorate setting should not influence guilt rebase.

[Per Cederqvist]

Signed-off-by: Per Cederqvist ced...@opera.com
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
---
 guilt-rebase | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/guilt-rebase b/guilt-rebase
index fd28e48..a1714a0 100755
--- a/guilt-rebase
+++ b/guilt-rebase
@@ -66,7 +66,7 @@ pop_all_patches
 git merge --no-commit $upstream  /dev/null 2 /dev/null
 
 disp 
-log=`git log -1 --pretty=oneline`
+log=`git log -1 --no-decorate --pretty=oneline`
 disp HEAD is now at `echo $log | cut -c 1-7`... `echo $log | cut -c 41-`
 
 #
-- 
1.8.3.1

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

[GUILT v2 21/29] The log.decorate setting should not influence import-commit.

[Per Cederqvist]

Use --no-decorate in the call to git log that tries to read the commit
message to produce patch names.  Otherwise, if the user has set
log.decorate to short or full, the patch name will be less useful.

Modify the t-034.sh test case to demonstrate that this is needed.

Signed-off-by: Per Cederqvist ced...@opera.com
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
---
 guilt-import-commit  | 2 +-
 regression/t-034.out | 2 ++
 regression/t-034.sh  | 2 ++
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/guilt-import-commit b/guilt-import-commit
index 45f2404..1da7c8e 100755
--- a/guilt-import-commit
+++ b/guilt-import-commit
@@ -26,7 +26,7 @@ disp About to begin conversion... 2
 disp Current head: `git rev-parse \`git_branch\`` 2
 
 for rev in `git rev-list $rhash`; do
-   s=`git log --pretty=oneline -1 $rev | cut -c 42-`
+   s=`git log --no-decorate --pretty=oneline -1 $rev | cut -c 42-`
 
# Try to convert the first line of the commit message to a
# valid patch name.
diff --git a/regression/t-034.out b/regression/t-034.out
index bda4399..5d81bd4 100644
--- a/regression/t-034.out
+++ b/regression/t-034.out
@@ -232,6 +232,7 @@ Date:   Mon Jan 1 00:00:00 2007 +
 
 Signed-off-by: Commiter Name commiter@email
 % guilt init
+% git config log.decorate short
 % guilt import-commit base..HEAD
 About to begin conversion...
 Current head: 2a8b1889aa5066193bac978e6bf5073ffcfa6541
@@ -259,6 +260,7 @@ Converting 45e81b51 as the_sequence_.lock-_is_forbidden
 Converting eebb76e9 as the_sequence_-._is_forbidden
 Done.
 Current head: d4850419ccc1146c7169f500725ce504b9774ed0
+% git config log.decorate no
 % guilt push -a
 Applying patch..the_sequence_-._is_forbidden.patch
 Patch applied.
diff --git a/regression/t-034.sh b/regression/t-034.sh
index f41f958..648d009 100755
--- a/regression/t-034.sh
+++ b/regression/t-034.sh
@@ -57,7 +57,9 @@ cmd git log
 
 # Import all the commits to guilt.
 cmd guilt init
+cmd git config log.decorate short
 cmd guilt import-commit base..HEAD
+cmd git config log.decorate no
 
 for patch in .git/patches/master/*.patch; do
touch -a -m -t $TOUCH_DATE $patch
-- 
1.8.3.1

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

[GUILT v2 24/29] disp no longer processes backslashes.

[Per Cederqvist]

Only one invocation of disp or _disp actually needed backslash
processing.  In quite a few instances, it was wrong to do backslash
processing, as the message contained data derived from the user.

Created the new function disp_e that should be used when backslash
processing is required, and changed disp and _disp to use printf
code %s instead of %b.

Signed-off-by: Per Cederqvist ced...@opera.com
---
 guilt | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/guilt b/guilt
index 23cc2da..9947acc 100755
--- a/guilt
+++ b/guilt
@@ -36,15 +36,24 @@ usage()
exit 1
 }
 
-# echo -n is a bashism, use printf instead
+# Print arguments, but no trailing newline.
+# (echo -n is a bashism, use printf instead)
 _disp()
 {
-   printf %b $*
+   printf %s $*
 }
 
-# echo -e is a bashism, use printf instead
+# Print arguments.
+# (echo -E is a bashism, use printf instead)
 disp()
 {
+   printf %s\n $*
+}
+
+# Print arguments, processing backslash sequences.
+# (echo -e is a bashism, use printf instead)
+disp_e()
+{
printf %b\n $*
 }
 
@@ -117,7 +126,7 @@ else
 
disp 
disp Example:
-   disp \tguilt push
+   disp_e \tguilt push
 
# now, let's exit
exit 1
-- 
1.8.3.1

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

[GUILT v2 28/29] Added guilt.reusebranch configuration option.

[Per Cederqvist]

When the option is true (the default), Guilt does not create a new Git
branch when patches are applied.  This way, you can switch between
Guilt 0.35 and the current version of Guilt with no issues.

At a future time, maybe a year after Guilt with guilt.reusebranch
support is released, the default should be changed to false to take
advantage of the ability to use a separate Git branch when patches are
applied.

Signed-off-by: Per Cederqvist ced...@opera.com
---
 guilt|  28 +++-
 regression/scaffold  |   1 +
 regression/t-062.out | 441 +++
 regression/t-062.sh  | 137 
 4 files changed, 601 insertions(+), 6 deletions(-)
 create mode 100644 regression/t-062.out
 create mode 100755 regression/t-062.sh

diff --git a/guilt b/guilt
index 9947acc..7c830eb 100755
--- a/guilt
+++ b/guilt
@@ -853,6 +853,9 @@ guilt_push_diff_context=1
 # default diffstat value: true or false
 DIFFSTAT_DEFAULT=false
 
+# default old_style_prefix value: true or false
+REUSE_BRANCH_DEFAULT=true
+
 # Prefix for guilt branches.
 GUILT_PREFIX=guilt/
 
@@ -864,6 +867,10 @@ GUILT_PREFIX=guilt/
 diffstat=`git config --bool guilt.diffstat`
 [ -z $diffstat ]  diffstat=$DIFFSTAT_DEFAULT
 
+# reuse Git branch?
+reuse_branch=`git config --bool guilt.reusebranch`
+[ -z $reuse_branch ]  reuse_branch=$REUSE_BRANCH_DEFAULT
+
 #
 # The following gets run every time this file is source'd
 #
@@ -928,13 +935,22 @@ else
die Unsupported operating system: $UNAME_S
 fi
 
-if [ $branch = $raw_git_branch ]  [ -n `get_top 2/dev/null` ]
-then
-# This is for compat with old repositories that still have a
-# pushed patch without the new-style branch prefix.
-old_style_prefix=true
+if [ -n `get_top 2/dev/null` ]; then
+   # If there is at least one pushed patch, we set
+   # old_style_prefix according to how it was pushed.  It is only
+   # possible to change the prefix style while no patches are
+   # applied.
+   if [ $branch = $raw_git_branch ]; then
+   old_style_prefix=true
+   else
+   old_style_prefix=false
+   fi
 else
-old_style_prefix=false
+   if $reuse_branch; then
+   old_style_prefix=true
+   else
+   old_style_prefix=false
+   fi
 fi
 
 _main $@
diff --git a/regression/scaffold b/regression/scaffold
index e4d7487..e4d2f35 100644
--- a/regression/scaffold
+++ b/regression/scaffold
@@ -93,6 +93,7 @@ function setup_git_repo
git config log.date default
git config log.decorate no
git config guilt.diffstat false
+   git config guilt.reusebranch false
 }
 
 function setup_guilt_repo
diff --git a/regression/t-062.out b/regression/t-062.out
new file mode 100644
index 000..727b436
--- /dev/null
+++ b/regression/t-062.out
@@ -0,0 +1,441 @@
+% setup_repo
+% git config guilt.reusebranch true
+% guilt push -a
+Applying patch..modify
+Patch applied.
+Applying patch..add
+Patch applied.
+Applying patch..remove
+Patch applied.
+Applying patch..mode
+Patch applied.
+% list_files
+d .git/patches
+d .git/patches/master
+d .git/refs/patches
+d .git/refs/patches/master
+f 22930c6d1f1938f298a4fca51c57e4b47171db21  .git/patches/master/mode
+f 413390f3906f16f30b054a4fb86c1e014b964504  .git/patches/master/remove
+f 71596bf71b72c2717e1aee378aabefbfa19ab7c8  .git/patches/master/status
+f 9c18cc7abe6b87f18503714a80a677b4094eb457  .git/patches/master/add
+f bacb4aad8a55fe4e7aa58a9ae169990bb764069f  .git/patches/master/series
+f bc9ab2e0f5db99d483961e956e814d963f0309f8  .git/patches/master/modify
+r 33633e7a1aa31972f125878baf7807be57b1672d  .git/refs/patches/master/modify
+r 37d588cc39848368810e88332bd03b083f2ce3ac  .git/refs/patches/master/add
+r ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba  .git/refs/patches/master/mode
+r ffb7faa126a6d91bcdd44a494f76b96dd860b8b9  .git/refs/patches/master/remove
+% git for-each-ref
+ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba commitrefs/heads/master
+37d588cc39848368810e88332bd03b083f2ce3ac commitrefs/patches/master/add
+ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba commitrefs/patches/master/mode
+33633e7a1aa31972f125878baf7807be57b1672d commit
refs/patches/master/modify
+ffb7faa126a6d91bcdd44a494f76b96dd860b8b9 commit
refs/patches/master/remove
+% git for-each-ref
+ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba commitrefs/heads/master
+37d588cc39848368810e88332bd03b083f2ce3ac commitrefs/patches/master/add
+ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba commitrefs/patches/master/mode
+33633e7a1aa31972f125878baf7807be57b1672d commit
refs/patches/master/modify
+ffb7faa126a6d91bcdd44a494f76b96dd860b8b9 commit
refs/patches/master/remove
+% list_files
+d .git/patches
+d .git/patches/master
+d .git/refs/patches
+d .git/refs/patches/master
+f 22930c6d1f1938f298a4fca51c57e4b47171db21  .git/patches/master/mode
+f 413390f3906f16f30b054a4fb86c1e014b964504  .git/patches/master/remove

[GUILT v2 29/29] Added a short style guide, and Emacs settings.

[Per Cederqvist]

Signed-off-by: Per Cederqvist ced...@opera.com
---
 .dir-locals.el |  3 +++
 Documentation/Contributing | 15 +++
 2 files changed, 18 insertions(+)
 create mode 100644 .dir-locals.el

diff --git a/.dir-locals.el b/.dir-locals.el
new file mode 100644
index 000..50ef2b7
--- /dev/null
+++ b/.dir-locals.el
@@ -0,0 +1,3 @@
+((nil . ((indent-tabs-mode . t)
+(tab-width . 8)))
+ (sh-mode . ((sh-basic-offset . 8
diff --git a/Documentation/Contributing b/Documentation/Contributing
index abf3480..0da49d6 100644
--- a/Documentation/Contributing
+++ b/Documentation/Contributing
@@ -4,6 +4,21 @@ Documentation/SubmittingPatches file. :)
 
 1) Hack on the code a bit
 
+Please follow this style guide:
+
+ - Use tabs for indentation.
+
+ - Put then on the same line as if.
+
+ - Follow the style of the existing code, except if it breaks the
+   above guidlines.
+
+ - If you change the code to conform to the style guide, please do so
+   in a separate commit that does not change anything else.
+
+Please check that you change does not break make test.  Please add
+new testcases for any new functionality, and if you fix a bug.
+
 2) Make a patch:
 
 Use diff -up or diff -uprN to create patches. Or simply use git to
-- 
1.8.3.1

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