Re: [PATCH v4 3/4] count-objects: report garbage files in pack directory too

2013-02-14 Thread Duy Nguyen
On Wed, Feb 13, 2013 at 10:55 PM, Junio C Hamano gits...@pobox.com wrote:
 + default:
 + return;
 + }
 + for (; first = last; first++)

 This looks odd.  If you use the usual last+1 convention between the
 caller and callee, you do not have to do this, or call this function
 with i - 1 and list-nr -1 as the last parameter.

I know. I just don't know how to name the variable to say the element
after the last one.
-- 
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [BUG] Veryfing signatures in git log fails when language is not english

2013-02-14 Thread Michael J Gruber
XANi venit, vidit, dixit 14.02.2013 01:18:
 Hi,
 
 any functionality that depends on exact exit msg of program
  can potentially fail because of that
 ᛯ export |grep LANG
 declare -x LANG=pl_PL.UTF-8
 
 ᛯ ~/src/os/git/git log --format=%G? %h |head -2 
  0d19377
  5b9d7f8
 
 ᛯ unset LANG
 ᛯ ~/src/os/git/git log --format=%G? %h |head -2
 G 0d19377
 G 5b9d7f8
 
 tested against maint (d32805d) and master (5bf72ed)
 
 maybe git should set up some output-changing variables before calling
 external programs? I think setting LC_ALL=C should be enougth.
 

There are really multiple problems here:

1. git calls gpg without setting LANG but expects output in LANG=C

2. git looks at the textual output from gpg to check the validity.

3. In fact, it does so only for %G and the display of signed merge
commits, in all other cases it checks the return code only.

gpg is not supposed to be used like that.

Since the callers of verify_signed_buffer do that craziness there is
some refactoring to be done.

A false hotfix would be to set LANG=C when calling gpg from git, but
that wouldn't solve the real problem. Besides, we do want LANG dependent
output for the user.

I'll have a closer look.

BTW: Thanks for the clear report :)

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


Re: [RFC v2] git-multimail: a replacement for post-receive-email

2013-02-14 Thread Matthieu Moy
Michael Haggerty mhag...@alum.mit.edu writes:

 On 02/13/2013 03:56 PM, Matthieu Moy wrote:

 Installation troubles:
 
 I had an old python installation (Red Hat package, and I'm not root),
 that did not include the email.utils package, so I couldn't use my
 system's python. I found no indication about python version in README,
 so I installed the latest python by hand, just to find out that
 git-multimail wasn't compatible with Python 3.x. 2to3 can fix
 automatically a number of 3.x compatibility issues, but not all of them
 so I gave up and installed Python 2.7.

 What version of Python was it that caused problems?

Python 2.4.3, installed with RHEL 5.9.

 I just discovered that the script wouldn't have worked with Python
 2.4, where email.utils used to be called email.Utils.

Indeed, import email.Utils works with this Python.

 But I pushed a fix to GitHub:

 ddb1796660 Accommodate older versions of Python's email module.

Not sufficient, but I added a pull request that works for me with 2.4.

 @@ -835,6 +837,17 @@ class ReferenceChange(Change):
  for line in self.expand_lines(NO_NEW_REVISIONS_TEMPLATE):
  yield line
  
 +if adds and self.showlog:
 +yield '\n'
 +yield 'Detailed log of added commits:\n\n'
 +for line in read_lines(
 +['git', 'log']
 ++ self.logopts
 ++ ['%s..%s' % (self.old.commit, self.new.commit,)],
 +keepends=True,
 +):
 +yield line
 +
  # The diffstat is shown from the old revision to the new
  # revision.  This is to show the truth of what happened in
  # this change.  There's no point showing the stat from the
 

 Thanks for the patch.  I like the idea, but I think the implementation
 is incorrect.  Your code will not only list new commits but will also
 list commits that were already in the repository on another branch
 (e.g., if an existing feature branch is merged into master, all of the
 commits on the feature branch will be listed).  (Or was that your
 intention?)

I did not think very carefully about this case, but the behavior of my
code seems sensible (although not uncontroversial): it's just showing
the detailed log for the same commits as the summary at the top of the
email. I have no personnal preferences.

 But even worse, it will fail to list commits that were
 added at the same time that a branch was created (e.g., if I create a
 feature branch with a number of commits on it and then push it for the
 first time).

Right.

 Probably the Push object has to negotiate with its constituent
 ReferenceChange objects to figure out which one is responsible for
 summarizing each of the commits newly added by the push (i.e., the ones
 returned by push.get_new_commits(None)).

I updated the pull request with a version that works for new branches,
and takes the list of commits to display from the call to
get_new_commits (which were already there for other purpose). Then, it
essentially calls git log --no-walk $list_of_sha1s.

This should be better.

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


[PATCH/RFC] clone: suggest index version 4 when the index is bigger than 512 KiB

2013-02-14 Thread Nguyễn Thái Ngọc Duy
I just realized that many of my big repos are still on index v2 while
v4 should reduce its size significantly (3.8M - 2.9M for linux-2.6
and 25M - 14M for webkit, for example). I wanted to propose index v4
as the new default version, because I guess that not many people
outside git@vger are aware of it, but perhaps this approach is less
drastic. It only suggest index v4 when the index size after clone hits
512K limit.

I have 170 git repositories (most of them Gnome projects) and only big
ones exceed the limit: webkit, linux-2.6, libreoffice-core, wine
(530K), gentoo-x86. Gimp and banshee are close (510K and 321K). The
rest barely hits 256K. So this hint won't show up often. On second
thought, maybe we should lower it down to 300K to show more often and
raise awareness on index v4 :)

Something I should have done in this patch too, is state that index v4
is only supported since vXXX, that git clients older than that will
not work. But maybe just put that in update-index man page and refer
there.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 builtin/clone.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/builtin/clone.c b/builtin/clone.c
index e0aaf13..7cd1b60 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -543,6 +543,7 @@ static int checkout(void)
struct tree *tree;
struct tree_desc t;
int err = 0, fd;
+   struct stat st;
 
if (option_no_checkout)
return 0;
@@ -591,6 +592,15 @@ static int checkout(void)
if (!err  option_recursive)
err = run_command_v_opt(argv_submodule, RUN_GIT_CMD);
 
+   if (!err 
+   !stat(git_path(index), st) 
+   st.st_size  512 * 1024)
+   advise(_(Your index is quite large (%d KiB).\n
+You may want to update to index version 4 to reduce 
its size,\n
+   as large index files may affect performance, using the 
command:\n
+  git update-index --index-version 4),
+  st.st_size / 1024);
+
return err;
 }
 
-- 
1.8.1.2.536.gf441e6d

--
To unsubscribe from this list: send the line 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: inotify to minimize stat() calls

2013-02-14 Thread Magnus Bäck
On Sunday, February 10, 2013 at 08:26 EST,
 demerphq demer...@gmail.com wrote:

 Is windows stat really so slow?

Well, the problem is that there is no such thing as Windows stat :-)

 I encountered this perception in windows Perl in the past, and I know
 that on windows Perl stat *appears* slow compared to *nix, because in
 order to satisfy the full *nix stat interface, specifically the nlink
 field, it must open and close the file*. As of 5.10 this can be
 disabled by setting a magic var ${^WIN32_SLOPPY_STAT} to a true value,
 which makes a significant improvement to the performance of the Perl
 level stat implementation.  I would not be surprised if the cygwin
 implementation of stat() has the same issue as Perl did, and that stat
 appears much slower than it actually need be if you don't care about
 the nlink field.

I suggested a few years ago that FindFirstFile() be used to implement
stat() since it's way faster than opening and closing the file, but
FindFirstFile() apparently produces unreliable mtime results when DST
shifts are involved.

http://thread.gmane.org/gmane.comp.version-control.git/114041
(The reference link in Johannes Sixt's first email is broken, but I'm
sure the information can be dug up.)

Based on a quick look it seems GetFileAttributesEx() is still used for
mingw and cygwin Git.

-- 
Magnus Bäck
ba...@google.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: inotify to minimize stat() calls

2013-02-14 Thread Ævar Arnfjörð Bjarmason
On Fri, Feb 8, 2013 at 10:10 PM, Ramkumar Ramachandra
artag...@gmail.com wrote:
 For large repositories, many simple git commands like `git status`
 take a while to respond.  I understand that this is because of large
 number of stat() calls to figure out which files were changed.  I
 overheard that Mercurial wants to solve this problem using itnotify,
 but the idea bothers me because it's not portable.  Will Git ever
 consider using inotify on Linux?  What is the downside?

There's one relatively easy sub-task of this that I haven't seen
mentioned: Improving the speed of interactive rebase on large (as in
lots of checked out files) repositories.

That's the single biggest thing that bothers me when I use Git with
large repos, not the speed of git status. When you git rebase -i
HEAD~100 re-arrange some patches and save the TODO list it takes say
0.5-1s for each patch to be applied, but at least 10x less than that
on a small repository. E.g. try this on linux-2.6.git v.s. some small
project with a few dozen files.

I looked into this a long while ago and remembered that rebase was
doing something like a git status for every commit that it made to
check the dirtyness.

This could be vastly improved by having an unsafe option to git-rebase
where it just assumes that the starting state + whatever it wrote out
is the current state, i.e. it would break if someone stuck up on your
checkout during an interactive rebase and changed a file, but the
common case of the user having exclusive access to the repo and
waiting for the rebase would be much faster.
--
To unsubscribe from this list: send the line 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.txt: update description of the configuration mechanism

2013-02-14 Thread Matthieu Moy
The old Git version where it appeared is not useful only to historians,
not to normal users. Also, the text was mentioning only the per-repo
config file, so add a mention of ~/.gitconfig. Describing in details the
system-wide, XDG and all would be counter-productive here, so reword the
description of the link to git-config to make it clear that it is not
only a list of configuration options.

Signed-off-by: Matthieu Moy matthieu@imag.fr
---
 Documentation/git.txt | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 0b681d9..e332947 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -535,10 +535,11 @@ include::cmds-purehelpers.txt[]
 Configuration Mechanism
 ---
 
-Starting from 0.99.9 (actually mid 0.99.8.GIT), `.git/config` file
-is used to hold per-repository configuration options.  It is a
-simple text file modeled after `.ini` format familiar to some
-people.  Here is an example:
+Git uses a simple text file format modeled after `.ini` format
+familiar to some people to store its configuration. The `.git/config`
+file is used to hold per-repository configuration options, and
+per-user configuration can be stored in a `~/.gitconfig` file.
+Here is an example:
 
 
 #
@@ -559,7 +560,7 @@ people.  Here is an example:
 
 Various commands read from the configuration file and adjust
 their operation accordingly.  See linkgit:git-config[1] for a
-list.
+list and more details about the configuration mechanism.
 
 
 Identifier Terminology
-- 
1.8.1.3.572.g35e1b60

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


[PATCH 0/5] gpg_interface: use the status

2013-02-14 Thread Michael J Gruber
Currently, we look at the user facing output of gpg, which is LANG dependent as
well as insecure.

After this series, we look at the status output (--status-fd) which is designed
for that purpose. As an additional benefit, we can read off the key which was 
used
for the signature, which is important for assigning trust.

All existing tests pass with this.

BTW: git branch --set-upstream-to= coredumps when on a detached head.

Michael J Gruber (5):
  gpg-interface: check good signature in a reliable way
  log-tree: rely upon the check in the gpg_interface
  gpg_interface: allow to request status return
  pretty: parse the gpg status lines rather than the output
  pretty: make %GK output the signing key for signed commits

 Documentation/pretty-formats.txt |  1 +
 builtin/fmt-merge-msg.c  |  2 +-
 builtin/verify-tag.c |  2 +-
 gpg-interface.c  | 18 +++---
 gpg-interface.h  |  2 +-
 log-tree.c   | 27 ---
 pretty.c | 19 +++
 7 files changed, 46 insertions(+), 25 deletions(-)

-- 
1.8.1.3.797.ge0260c7

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


[PATCH 1/5] gpg-interface: check good signature in a reliable way

2013-02-14 Thread Michael J Gruber
Currently, verify_signed_buffer() only checks the return code of gpg,
and some callers implement additional unreliable checks for Good
signature in the gpg output meant for the user.

Use the status output instead and parse for a line beinning with
[GNUPG:] GOODSIG . This is the only reliable way of checking for a
good gpg signature.

If needed we can change this easily to [GNUPG:] VALIDSIG  if we want
to take into account the trust model.

Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net
---
 gpg-interface.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/gpg-interface.c b/gpg-interface.c
index 4559033..c582b2e 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -96,15 +96,17 @@ int sign_buffer(struct strbuf *buffer, struct strbuf 
*signature, const char *sig
 /*
  * Run gpg to see if the payload matches the detached signature.
  * gpg_output, when set, receives the diagnostic output from GPG.
+ * gpg_status, when set, receives the status output from GPG.
  */
 int verify_signed_buffer(const char *payload, size_t payload_size,
 const char *signature, size_t signature_size,
 struct strbuf *gpg_output)
 {
struct child_process gpg;
-   const char *args_gpg[] = {NULL, --verify, FILE, -, NULL};
+   const char *args_gpg[] = {NULL, --status-fd=1, --verify, FILE, 
-, NULL};
char path[PATH_MAX];
int fd, ret;
+   struct strbuf buf = STRBUF_INIT;
 
args_gpg[0] = gpg_program;
fd = git_mkstemp(path, PATH_MAX, .git_vtag_tmpXX);
@@ -119,9 +121,10 @@ int verify_signed_buffer(const char *payload, size_t 
payload_size,
memset(gpg, 0, sizeof(gpg));
gpg.argv = args_gpg;
gpg.in = -1;
+   gpg.out = -1;
if (gpg_output)
gpg.err = -1;
-   args_gpg[2] = path;
+   args_gpg[3] = path;
if (start_command(gpg)) {
unlink(path);
return error(_(could not run gpg.));
@@ -134,9 +137,15 @@ int verify_signed_buffer(const char *payload, size_t 
payload_size,
strbuf_read(gpg_output, gpg.err, 0);
close(gpg.err);
}
+   strbuf_read(buf, gpg.out, 0);
+   close(gpg.out);
+
ret = finish_command(gpg);
 
unlink_or_warn(path);
 
+   ret |= !strstr(buf.buf, \n[GNUPG:] GOODSIG );
+   strbuf_release(buf);
+
return ret;
 }
-- 
1.8.1.3.797.ge0260c7

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


[PATCH 2/5] log-tree: rely upon the check in the gpg_interface

2013-02-14 Thread Michael J Gruber
It's just so much betterer.

Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net
---
 log-tree.c | 25 +++--
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/log-tree.c b/log-tree.c
index 5dc45c4..912fe08 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -508,20 +508,17 @@ static void show_one_mergetag(struct rev_info *opt,
gpg_message_offset = verify_message.len;
 
payload_size = parse_signature(extra-value, extra-len);
-   if ((extra-len = payload_size) ||
-   (verify_signed_buffer(extra-value, payload_size,
- extra-value + payload_size,
- extra-len - payload_size,
- verify_message) 
-verify_message.len = gpg_message_offset)) {
-   strbuf_addstr(verify_message, No signature\n);
-   status = -1;
-   }
-   else if (strstr(verify_message.buf + gpg_message_offset,
-   : Good signature from ))
-   status = 0;
-   else
-   status = -1;
+   status = -1;
+   if (extra-len  payload_size)
+   if (verify_signed_buffer(extra-value, payload_size,
+extra-value + payload_size,
+extra-len - payload_size,
+verify_message)) {
+   if (verify_message.len = gpg_message_offset)
+   strbuf_addstr(verify_message, No 
signature\n);
+   else
+   status = 0;
+   }
 
show_sig_lines(opt, status, verify_message.buf);
strbuf_release(verify_message);
-- 
1.8.1.3.797.ge0260c7

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


[PATCH 3/5] gpg_interface: allow to request status return

2013-02-14 Thread Michael J Gruber
Currently, verify_signed_buffer() returns the user facing output only.

Allow callers to request the status output also.

Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net
---
 builtin/fmt-merge-msg.c |  2 +-
 builtin/verify-tag.c|  2 +-
 gpg-interface.c | 11 +++
 gpg-interface.h |  2 +-
 log-tree.c  |  4 ++--
 pretty.c|  2 +-
 6 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index b49612f..265a925 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -492,7 +492,7 @@ static void fmt_merge_msg_sigs(struct strbuf *out)
 
if (size == len)
; /* merely annotated */
-   else if (verify_signed_buffer(buf, len, buf + len, size - len, 
sig)) {
+   else if (verify_signed_buffer(buf, len, buf + len, size - len, 
sig, NULL)) {
if (!sig.len)
strbuf_addstr(sig, gpg verification 
failed.\n);
}
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index a8eee88..9cdf332 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -29,7 +29,7 @@ static int run_gpg_verify(const char *buf, unsigned long 
size, int verbose)
if (size == len)
return error(no signature found);
 
-   return verify_signed_buffer(buf, len, buf + len, size - len, NULL);
+   return verify_signed_buffer(buf, len, buf + len, size - len, NULL, 
NULL);
 }
 
 static int verify_tag(const char *name, int verbose)
diff --git a/gpg-interface.c b/gpg-interface.c
index c582b2e..8b0e874 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -100,13 +100,14 @@ int sign_buffer(struct strbuf *buffer, struct strbuf 
*signature, const char *sig
  */
 int verify_signed_buffer(const char *payload, size_t payload_size,
 const char *signature, size_t signature_size,
-struct strbuf *gpg_output)
+struct strbuf *gpg_output, struct strbuf *gpg_status)
 {
struct child_process gpg;
const char *args_gpg[] = {NULL, --status-fd=1, --verify, FILE, 
-, NULL};
char path[PATH_MAX];
int fd, ret;
struct strbuf buf = STRBUF_INIT;
+   struct strbuf *pbuf = buf;
 
args_gpg[0] = gpg_program;
fd = git_mkstemp(path, PATH_MAX, .git_vtag_tmpXX);
@@ -137,15 +138,17 @@ int verify_signed_buffer(const char *payload, size_t 
payload_size,
strbuf_read(gpg_output, gpg.err, 0);
close(gpg.err);
}
-   strbuf_read(buf, gpg.out, 0);
+   if (gpg_status)
+   pbuf = gpg_status;
+   strbuf_read(pbuf, gpg.out, 0);
close(gpg.out);
 
ret = finish_command(gpg);
 
unlink_or_warn(path);
 
-   ret |= !strstr(buf.buf, \n[GNUPG:] GOODSIG );
-   strbuf_release(buf);
+   ret |= !strstr(pbuf-buf, \n[GNUPG:] GOODSIG );
+   strbuf_release(buf); /* no matter it was used or not */
 
return ret;
 }
diff --git a/gpg-interface.h b/gpg-interface.h
index b9c3608..cf99021 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -2,7 +2,7 @@
 #define GPG_INTERFACE_H
 
 extern int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const 
char *signing_key);
-extern int verify_signed_buffer(const char *payload, size_t payload_size, 
const char *signature, size_t signature_size, struct strbuf *gpg_output);
+extern int verify_signed_buffer(const char *payload, size_t payload_size, 
const char *signature, size_t signature_size, struct strbuf *gpg_output, struct 
strbuf *gpg_status);
 extern int git_gpg_config(const char *, const char *, void *);
 extern void set_signing_key(const char *);
 extern const char *get_signing_key(void);
diff --git a/log-tree.c b/log-tree.c
index 912fe08..3d88823 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -444,7 +444,7 @@ static void show_signature(struct rev_info *opt, struct 
commit *commit)
 
status = verify_signed_buffer(payload.buf, payload.len,
  signature.buf, signature.len,
- gpg_output);
+ gpg_output, NULL);
if (status  !gpg_output.len)
strbuf_addstr(gpg_output, No signature\n);
 
@@ -513,7 +513,7 @@ static void show_one_mergetag(struct rev_info *opt,
if (verify_signed_buffer(extra-value, payload_size,
 extra-value + payload_size,
 extra-len - payload_size,
-verify_message)) {
+verify_message, NULL)) {
if (verify_message.len = gpg_message_offset)
strbuf_addstr(verify_message, No 
signature\n);
else
diff --git a/pretty.c b/pretty.c
index 

[PATCH 4/5] pretty: parse the gpg status lines rather than the output

2013-02-14 Thread Michael J Gruber
Currently, parse_signature_lines() parses the gpg output for strings
which depend on LANG so it fails to recognize good commit signatures
(and thus does not fill in %G? and the like) in most locales.

Make it parse the status lines from gpg instead, which are the proper
machine interface. This fixes the problem described above.

There is a change in behavior for %GS which we intentionally do not
work around: %GS used to put quotes around the signer's uid (or
rather: it inherited from the gpg user output). We output the uid
without quotes now, just like author and committer names.

Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net
---
 pretty.c | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/pretty.c b/pretty.c
index 2a1e174..973b912 100644
--- a/pretty.c
+++ b/pretty.c
@@ -759,6 +759,7 @@ struct format_commit_context {
unsigned commit_signature_parsed:1;
struct {
char *gpg_output;
+   char *gpg_status;
char good_bad;
char *signer;
} signature;
@@ -948,13 +949,13 @@ static struct {
char result;
const char *check;
 } signature_check[] = {
-   { 'G', : Good signature from  },
-   { 'B', : BAD signature from  },
+   { 'G', \n[GNUPG:] GOODSIG  },
+   { 'B', \n[GNUPG:] BADSIG  },
 };
 
 static void parse_signature_lines(struct format_commit_context *ctx)
 {
-   const char *buf = ctx-signature.gpg_output;
+   const char *buf = ctx-signature.gpg_status;
int i;
 
for (i = 0; i  ARRAY_SIZE(signature_check); i++) {
@@ -963,7 +964,7 @@ static void parse_signature_lines(struct 
format_commit_context *ctx)
if (!found)
continue;
ctx-signature.good_bad = signature_check[i].result;
-   found += strlen(signature_check[i].check);
+   found += strlen(signature_check[i].check)+17;
next = strchrnul(found, '\n');
ctx-signature.signer = xmemdupz(found, next - found);
break;
@@ -975,6 +976,7 @@ static void parse_commit_signature(struct 
format_commit_context *ctx)
struct strbuf payload = STRBUF_INIT;
struct strbuf signature = STRBUF_INIT;
struct strbuf gpg_output = STRBUF_INIT;
+   struct strbuf gpg_status = STRBUF_INIT;
int status;
 
ctx-commit_signature_parsed = 1;
@@ -984,13 +986,15 @@ static void parse_commit_signature(struct 
format_commit_context *ctx)
goto out;
status = verify_signed_buffer(payload.buf, payload.len,
  signature.buf, signature.len,
- gpg_output, NULL);
+ gpg_output, gpg_status);
if (status  !gpg_output.len)
goto out;
ctx-signature.gpg_output = strbuf_detach(gpg_output, NULL);
+   ctx-signature.gpg_status = strbuf_detach(gpg_status, NULL);
parse_signature_lines(ctx);
 
  out:
+   strbuf_release(gpg_status);
strbuf_release(gpg_output);
strbuf_release(payload);
strbuf_release(signature);
-- 
1.8.1.3.797.ge0260c7

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


[PATCH 5/5] pretty: make %GK output the signing key for signed commits

2013-02-14 Thread Michael J Gruber
Because we can.

No, really: In order to employ signed keys in an automated way it is
absolutely necessary to check which keys the signatures come from. Now
you can.

Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net
---
 Documentation/pretty-formats.txt | 1 +
 pretty.c | 9 -
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 105f18a..2939655 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -133,6 +133,7 @@ The placeholders are:
 - '%GG': raw verification message from GPG for a signed commit
 - '%G?': show either G for Good or B for Bad for a signed commit
 - '%GS': show the name of the signer for a signed commit
+- '%GK': show the key used to sign a signed commit
 - '%gD': reflog selector, e.g., `refs/stash@{1}`
 - '%gd': shortened reflog selector, e.g., `stash@{1}`
 - '%gn': reflog identity name
diff --git a/pretty.c b/pretty.c
index 973b912..b57adef 100644
--- a/pretty.c
+++ b/pretty.c
@@ -762,6 +762,7 @@ struct format_commit_context {
char *gpg_status;
char good_bad;
char *signer;
+   char *key;
} signature;
char *message;
size_t width, indent1, indent2;
@@ -964,7 +965,9 @@ static void parse_signature_lines(struct 
format_commit_context *ctx)
if (!found)
continue;
ctx-signature.good_bad = signature_check[i].result;
-   found += strlen(signature_check[i].check)+17;
+   found += strlen(signature_check[i].check);
+   ctx-signature.key = xmemdupz(found, 16);
+   found += 17;
next = strchrnul(found, '\n');
ctx-signature.signer = xmemdupz(found, next - found);
break;
@@ -1204,6 +1207,10 @@ static size_t format_commit_one(struct strbuf *sb, const 
char *placeholder,
if (c-signature.signer)
strbuf_addstr(sb, c-signature.signer);
break;
+   case 'K':
+   if (c-signature.key)
+   strbuf_addstr(sb, c-signature.key);
+   break;
}
return 2;
}
-- 
1.8.1.3.797.ge0260c7

--
To unsubscribe from this list: send the line 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.txt: update description of the configuration mechanism

2013-02-14 Thread Michael J Gruber
Matthieu Moy venit, vidit, dixit 14.02.2013 16:36:
 The old Git version where it appeared is not useful only to historians,
 not to normal users. Also, the text was mentioning only the per-repo

I do not think you meant to not remove so many nots ;)

Besides, if history is uninteresting, then so is sociology: familiar to
some people can go, too.

 config file, so add a mention of ~/.gitconfig. Describing in details the
 system-wide, XDG and all would be counter-productive here, so reword the

Hmpf, I think this gives a way too prominent role to ~/.gitconfig. The
config files most people will have to deal with are:

- the repo config file
- the one set by config --global

And really, it would often be best if the latter was the XDG thing.

So, I'm all for improving git.txt, but somewhat differently ;)

 description of the link to git-config to make it clear that it is not
 only a list of configuration options.
 
 Signed-off-by: Matthieu Moy matthieu@imag.fr
 ---
  Documentation/git.txt | 11 ++-
  1 file changed, 6 insertions(+), 5 deletions(-)
 
 diff --git a/Documentation/git.txt b/Documentation/git.txt
 index 0b681d9..e332947 100644
 --- a/Documentation/git.txt
 +++ b/Documentation/git.txt
 @@ -535,10 +535,11 @@ include::cmds-purehelpers.txt[]
  Configuration Mechanism
  ---
  
 -Starting from 0.99.9 (actually mid 0.99.8.GIT), `.git/config` file
 -is used to hold per-repository configuration options.  It is a
 -simple text file modeled after `.ini` format familiar to some
 -people.  Here is an example:
 +Git uses a simple text file format modeled after `.ini` format
 +familiar to some people to store its configuration. The `.git/config`
 +file is used to hold per-repository configuration options, and
 +per-user configuration can be stored in a `~/.gitconfig` file.
 +Here is an example:
  
  
  #
 @@ -559,7 +560,7 @@ people.  Here is an example:
  
  Various commands read from the configuration file and adjust
  their operation accordingly.  See linkgit:git-config[1] for a
 -list.
 +list and more details about the configuration mechanism.
  
  
  Identifier Terminology
 

--
To unsubscribe from this list: send the line 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.txt: update description of the configuration mechanism

2013-02-14 Thread Matthieu Moy
Michael J Gruber g...@drmicha.warpmail.net writes:

 Matthieu Moy venit, vidit, dixit 14.02.2013 16:36:
 The old Git version where it appeared is not useful only to historians,
 not to normal users. Also, the text was mentioning only the per-repo

 I do not think you meant to not remove so many nots ;)

The first was meant to be now, indeed.

 Besides, if history is uninteresting, then so is sociology: familiar to
 some people can go, too.

It can, but I'm fine with keeping it too. It may help some users to
realize ah, OK, the same ini file I'm used to. And it doesn't really
harm.

 config file, so add a mention of ~/.gitconfig. Describing in details the
 system-wide, XDG and all would be counter-productive here, so reword the

 Hmpf, I think this gives a way too prominent role to ~/.gitconfig. The
 config files most people will have to deal with are:

 - the repo config file
 - the one set by config --global

 And really, it would often be best if the latter was the XDG thing.

That's a different question. For now, ~/.gitconfig is the default
destination of config --global and we should wait for XDG-aware Gits
to be widely deployed before reconsidering that.

If the XDG config file ever become the default, then sure, it will have
to be promoted instead of ~/.gitconfig in git.txt (and I'll be all for
it when it's time, even though I can foresee a few flamewars ;-) ), but
I don't think we should do that now.

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


Re: inotify to minimize stat() calls

2013-02-14 Thread Junio C Hamano
Ævar Arnfjörð Bjarmason ava...@gmail.com writes:

 I looked into this a long while ago and remembered that rebase was
 doing something like a git status for every commit that it made to
 check the dirtyness.

 This could be vastly improved by having an unsafe option to git-rebase
 where it just assumes that the starting state + whatever it wrote out
 is the current state, i.e. it would break if someone stuck up on your
 checkout during an interactive rebase and changed a file,...

You could make it a lot safer than just assumes, and the result
may become generally usable, I think.  For example, you can set a
magic bit somewhere in $GIT_DIR/rebase-i while you are in I am
doing pick/pick/pick and the user will not interfere me mode, and
clear that bit upon rebase --continue.  And you cheat only while
that magic bit is set.

--
To unsubscribe from this list: send the line 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] Veryfing signatures in git log fails when language is not english

2013-02-14 Thread Junio C Hamano
Mariusz Gronczewski xani...@gmail.com writes:

 What is really missing is an ability to display used key ID without
 hammering git log output with regexps, it would be much easier to
 validate incoming commits if there was format option to just display
 key ID instead of signer name. %GS isn't really good solution for that
 because it will show only one of email addresses used in the key and
 script checking signatures would have to always pick right one.

The %Ganything pretty modifiers other than %GG were done mostly as
placeholders.

I think the following would be a good way to refine them:

- %GG, and possibly log --show-signature should run GPG under
  the user's LANG.

- %G? is mostly useless, unless it is made to always mean does
  it verify crypto-wise and nothing else.  One bit is simply
  too small to represent all the cases where you may or may not
  have the signer's key, or you may have the key but you do not
  have enough trust in it (e.g. the key may be expired, revoked,
  or not enough confidence in your web of trust).

- The right one you mention for %GS is easier than you might
  think.  If you just verify against the accompanying tagger
  identity, that should be sufficient.  It of course cannot be
  generally solved, as you could tag as person A while signing
  with key for person B, but a simple social convention would
  help us out there: if you tag as Mariusz Gronczewski, your
  signature should also say so.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Feb 2013, #05; Tue, 12)

2013-02-14 Thread Junio C Hamano
Andrew Ardill andrew.ard...@gmail.com writes:

 If that is the change we are going to make, and if you can guarantee
 that nobody who is used to the historical behaviour will complain,
 then I am fine with it, but I think the latter part of the condition
 will not hold.

 Does the impossibility of asserting that no-one will complain put this
 in the 'too hard' bucket?

Basically, yes.  Cannot be done without UI regression.

It could be a Git 2.0 item, if you plan the transition right, though.

 The implication here is that a relatively small number of people will
 be inconvenienced by needing to specify extra flags/set up an alias.
 Compare this to the many for whom the expected behaviour is now
 default, and we have a net win.

We take backward compatibility a lot more seriously; it is not even
a democracy.

Net win does not mean an iota.  Even if small number is 47 and
large majority is 4 million, it does not change the fact that you
are breaking things these 47 people have depended on working in an
expected (the expected does not have to be intuitive in this
sentence; what counts more is that it is the way they are accustomed
to) way and introducing a UI regression.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 3/4] count-objects: report garbage files in pack directory too

2013-02-14 Thread Junio C Hamano
Duy Nguyen pclo...@gmail.com writes:

 On Wed, Feb 13, 2013 at 10:55 PM, Junio C Hamano gits...@pobox.com wrote:
 + default:
 + return;
 + }
 + for (; first = last; first++)

 This looks odd.  If you use the usual last+1 convention between the
 caller and callee, you do not have to do this, or call this function
 with i - 1 and list-nr -1 as the last parameter.

 I know. I just don't know how to name the variable to say the element
 after the last one.

In case it was unclear, by the usual last+1 convention, I meant
that it is perfectly normal to write

for (i = first; i  last; i++)
for (i = begin; i  end; i++)

in C.  Saying these as

for (i = first; i  beyond_last; i++)
for (i = begin; i  beyond_end; i++)

look non-C.
--
To unsubscribe from this list: send the line 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.txt: update description of the configuration mechanism

2013-02-14 Thread Junio C Hamano
Matthieu Moy matthieu@grenoble-inp.fr writes:

 If the XDG config file ever become the default, then sure, it will have
 to be promoted instead of ~/.gitconfig in git.txt (and I'll be all for
 it when it's time, even though I can foresee a few flamewars ;-) ), but
 I don't think we should do that now.

We are giving an overview to list what things are possible and what
words the reader should be familiar with in this part of the manual.
The fact that configuration can be done per user and per repository
is a relevant thing to know at the conceptual level.

But the exact location of per-user and per-repository configuration
files does not matter in this context and is best left to the
git-config documentation.


 Documentation/git.txt | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 0b681d9..2d975e3 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -535,10 +535,9 @@ include::cmds-purehelpers.txt[]
 Configuration Mechanism
 ---
 
-Starting from 0.99.9 (actually mid 0.99.8.GIT), `.git/config` file
-is used to hold per-repository configuration options.  It is a
-simple text file modeled after `.ini` format familiar to some
-people.  Here is an example:
+Git uses a simple text format to store customizations that are per
+repository and are per user.  Such a configuration file may look
+like this:
 
 
 #
@@ -553,13 +552,13 @@ people.  Here is an example:
 ; user identity
 [user]
name = Junio C Hamano
-   email = jun...@twinsun.com
+   email = gits...@pobox.com
 
 
 
 Various commands read from the configuration file and adjust
 their operation accordingly.  See linkgit:git-config[1] for a
-list.
+list and more details about the configuration mechanism.
 
 
 Identifier Terminology
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/5] gpg-interface: check good signature in a reliable way

2013-02-14 Thread Junio C Hamano
Michael J Gruber g...@drmicha.warpmail.net writes:

 Currently, verify_signed_buffer() only checks the return code of gpg,
 and some callers implement additional unreliable checks for Good
 signature in the gpg output meant for the user.

 Use the status output instead and parse for a line beinning with
 [GNUPG:] GOODSIG . This is the only reliable way of checking for a
 good gpg signature.

 If needed we can change this easily to [GNUPG:] VALIDSIG  if we want
 to take into account the trust model.

Thanks.  I didn't look beyond man gpg nor bother looking at
DETAILS file in its source, which the manpage refers to.

I think GOODSIG is a good starting point.  Depending on the context
(e.g. %G?) we may also want to consider EXPSIG (but not EXPKEYSIG
or REVKEYSIG) acceptable, while reading log --show-signature on
ancient part of the history, no?

 Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net
 ---
  gpg-interface.c | 13 +++--
  1 file changed, 11 insertions(+), 2 deletions(-)

 diff --git a/gpg-interface.c b/gpg-interface.c
 index 4559033..c582b2e 100644
 --- a/gpg-interface.c
 +++ b/gpg-interface.c
 @@ -96,15 +96,17 @@ int sign_buffer(struct strbuf *buffer, struct strbuf 
 *signature, const char *sig
  /*
   * Run gpg to see if the payload matches the detached signature.
   * gpg_output, when set, receives the diagnostic output from GPG.
 + * gpg_status, when set, receives the status output from GPG.
   */
  int verify_signed_buffer(const char *payload, size_t payload_size,
const char *signature, size_t signature_size,
struct strbuf *gpg_output)
  {
   struct child_process gpg;
 - const char *args_gpg[] = {NULL, --verify, FILE, -, NULL};
 + const char *args_gpg[] = {NULL, --status-fd=1, --verify, FILE, 
 -, NULL};
   char path[PATH_MAX];
   int fd, ret;
 + struct strbuf buf = STRBUF_INIT;
  
   args_gpg[0] = gpg_program;
   fd = git_mkstemp(path, PATH_MAX, .git_vtag_tmpXX);
 @@ -119,9 +121,10 @@ int verify_signed_buffer(const char *payload, size_t 
 payload_size,
   memset(gpg, 0, sizeof(gpg));
   gpg.argv = args_gpg;
   gpg.in = -1;
 + gpg.out = -1;
   if (gpg_output)
   gpg.err = -1;
 - args_gpg[2] = path;
 + args_gpg[3] = path;
   if (start_command(gpg)) {
   unlink(path);
   return error(_(could not run gpg.));
 @@ -134,9 +137,15 @@ int verify_signed_buffer(const char *payload, size_t 
 payload_size,
   strbuf_read(gpg_output, gpg.err, 0);
   close(gpg.err);
   }
 + strbuf_read(buf, gpg.out, 0);
 + close(gpg.out);
 +
   ret = finish_command(gpg);
  
   unlink_or_warn(path);
  
 + ret |= !strstr(buf.buf, \n[GNUPG:] GOODSIG );
 + strbuf_release(buf);
 +
   return ret;
  }
--
To unsubscribe from this list: send the line 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.txt: update description of the configuration mechanism

2013-02-14 Thread Matthieu Moy
Junio C Hamano gits...@pobox.com writes:

 But the exact location of per-user and per-repository configuration
 files does not matter in this context and is best left to the
 git-config documentation.

I'm OK with your version.

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


Re: [PATCH v4.1 09/12] sequencer.c: teach append_signoff to avoid adding a duplicate newline

2013-02-14 Thread John Keeping
On Tue, Feb 12, 2013 at 02:33:42AM -0800, Brandon Casey wrote:
 Teach append_signoff to detect whether a blank line exists at the position
 that the signed-off-by line will be added, and refrain from adding an
 additional one if one already exists.  Or, add an additional line if one
 is needed to make sure the new footer is separated from the message body
 by a blank line.
 
 Signed-off-by: Brandon Casey bca...@nvidia.com
 ---

As Jonathan Nieder wondered before [1], this changes the behaviour when
the commit message is empty.  Before this commit, there is an empty line
followed by the S-O-B line; now the S-O-B is on the first line of the
commit.

The previous behaviour seems better to me since the empty line is
hinting that the user should fill something in.  It looks particularly
strange if your editor has syntax highlighting for commit messages such
that the first line is in a different colour.

[1] http://article.gmane.org/gmane.comp.version-control.git/214796

 diff --git a/sequencer.c b/sequencer.c
 index 3364faa..084573b 100644
 --- a/sequencer.c
 +++ b/sequencer.c
 @@ -1127,8 +1127,19 @@ void append_signoff(struct strbuf *msgbuf, int 
 ignore_footer, unsigned flag)
   else
   has_footer = has_conforming_footer(msgbuf, sob, ignore_footer);
  
 - if (!has_footer)
 - strbuf_splice(msgbuf, msgbuf-len - ignore_footer, 0, \n, 1);
 + if (!has_footer) {
 + const char *append_newlines = NULL;
 + size_t len = msgbuf-len - ignore_footer;
 +
 + if (len  msgbuf-buf[len - 1] != '\n')
 + append_newlines = \n\n;
 + else if (len  1  msgbuf-buf[len - 2] != '\n')
 + append_newlines = \n;

To restore the old behaviour this needs something like this:

else if (!len)
append_newlines = \n;

 + if (append_newlines)
 + strbuf_splice(msgbuf, msgbuf-len - ignore_footer, 0,
 + append_newlines, strlen(append_newlines));
 + }
  
   if (has_footer != 3  (!no_dup_sob || has_footer != 2))
   strbuf_splice(msgbuf, msgbuf-len - ignore_footer, 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.txt: update description of the configuration mechanism

2013-02-14 Thread Junio C Hamano
Matthieu Moy matthieu@grenoble-inp.fr writes:

 Junio C Hamano gits...@pobox.com writes:

 But the exact location of per-user and per-repository configuration
 files does not matter in this context and is best left to the
 git-config documentation.

 I'm OK with your version.

I already queued your original with one s/not/now/; perhaps I will
redo it 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 v2 02/15] user-manual: Update for receive.denyCurrentBranch=refuse

2013-02-14 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 W. Trevor King wk...@tremily.us writes:

 On Sun, Feb 10, 2013 at 01:24:47PM -0800, Junio C Hamano wrote:
 I would not be surprised if some readers felt as if then why not
 update it instead of rejecting? were a valid question, without a
 bit more explanation.

 You can also push to a repository that has a working tree,
   …

 Looks good to me :).  Shall I just drop this patch from v3 and leave
 it to you?

 No.

 Others need to object to, comment on and polish what you saw from
 me, before it turns into a commit.  And you need to be credited for
 identifying the problem, initiating the discussion, and collecting
 responses to result in the final patch.

I did not think the detailed discussion belongs there in the first
place, so I re-read the context.  I think the only thing the reader
of the user manual needs to learn at that point of the flow is that
they can push to a non-bare but cannot push to update the currently
checked out branch by default.  So let's tone everything down and do
this instead:

-- 8 --
From: W. Trevor King wk...@tremily.us
Date: Fri, 8 Feb 2013 12:04:20 -0500
Subject: [PATCH] user-manual: Update for receive.denyCurrentBranch=refuse

acd2a45 (Refuse updating the current branch in a non-bare repository
via push, 2009-02-11) changed the default to refuse such a push, but
it forgot to update the docs.

7d182f5 (Documentation: receive.denyCurrentBranch defaults to
'refuse', 2010-03-17) updated Documentation/config.txt, but forgot to
update the user manual.

Signed-off-by: W. Trevor King wk...@tremily.us
Signed-off-by: Junio C Hamano gits...@pobox.com
---
 Documentation/user-manual.txt | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index 85651b5..7c534dc 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -1986,9 +1986,10 @@ handling this case.
 
 Note that the target of a push is normally a
 def_bare_repository,bare repository.  You can also push to a
-repository that has a checked-out working tree, but the working tree
-will not be updated by the push.  This may lead to unexpected results if
-the branch you push to is the currently checked-out branch!
+repository that has a checked-out working tree, but a push to update the
+currently checked-out branch is denied by default to prevent confusion.
+See the description ofthe receive.denyCurrentBranch option
+in linkgit:git-config[1] for details.
 
 As with `git fetch`, you may also set up configuration options to
 save typing; so, for example, after
--
To unsubscribe from this list: send the line 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: filter-branch env-filter example

2013-02-14 Thread Tadeusz Andrzej Kadłubowski
filter-branch --env-filter example that shows how to change the email address
in all commits by a certain developer.
---
 Documentation/git-filter-branch.txt | 13 +
 1 file changed, 13 insertions(+)

diff --git a/Documentation/git-filter-branch.txt 
b/Documentation/git-filter-branch.txt
index dfd12c9..2664cec 100644
--- a/Documentation/git-filter-branch.txt
+++ b/Documentation/git-filter-branch.txt
@@ -329,6 +329,19 @@ git filter-branch --msg-filter '
 ' HEAD~10..HEAD
 

+You can modify committer/author personal information using `--env-filter`.
+For example, to update some developer's email address use this command:
+
+
+git filter-branch --env-filter '
+   if [ $GIT_AUTHOR_EMAIL = j...@old.example.com ]
+   then
+   GIT_AUTHOR_EMAIL=j...@new.example.com
+   fi
+   export GIT_AUTHOR_EMAIL
+' -- --all
+
+
 To restrict rewriting to only part of the history, specify a revision
 range in addition to the new branch name.  The new branch name will
 point to the top-most revision that a 'git rev-list' of this range
-- 
1.7.11.7
--
To unsubscribe from this list: send the line 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] Documentation: filter-branch env-filter example

2013-02-14 Thread Junio C Hamano
Tadeusz Andrzej Kadłubowski  y...@hell.org.pl writes:

 filter-branch --env-filter example that shows how to change the email address
 in all commits by a certain developer.
 ---

Thanks.  Sign-off?

  Documentation/git-filter-branch.txt | 13 +
  1 file changed, 13 insertions(+)

 diff --git a/Documentation/git-filter-branch.txt 
 b/Documentation/git-filter-branch.txt
 index dfd12c9..2664cec 100644
 --- a/Documentation/git-filter-branch.txt
 +++ b/Documentation/git-filter-branch.txt
 @@ -329,6 +329,19 @@ git filter-branch --msg-filter '
  ' HEAD~10..HEAD
  

 +You can modify committer/author personal information using `--env-filter`.
 +For example, to update some developer's email address use this command:
 +
 +
 +git filter-branch --env-filter '
 + if [ $GIT_AUTHOR_EMAIL = j...@old.example.com ]

Quote the variable in double-quotes, like this:

if [ $GIT_AUTHOR_EMAIL = j...@old.example.com ]

Otherwise the comparison will break, if the e-mail part had a
whitespace in it, or if it were empty, which is an example of a more
likely situation where you would want to fix commits using a
procedure like this, no?

But more on the example later...

 + then
 + GIT_AUTHOR_EMAIL=j...@new.example.com
 + fi
 + export GIT_AUTHOR_EMAIL
 +' -- --all
 +
 +

I do not think an illustration of env-filter is a bad addition
per-se, but the sample scenario is not a realistic one.  No sane
project should be encouraged to rewrite their entire history every
time one of the contributors change his e-mail address.  That is
what the mailmap mechanism is for.

The only scenario that justifies use of the given sample I can think
of is to rewrite the author and committer in an unpublished project
because you noticed that you forgot to set user.name and user.email
up before you created these commits correctly.

Taking all of the above, the added text may look more like this, I
think:

The `--env-filter` can be used to modify committer and/or
author identity.  For example, if you found out that your
commits have wrong identity of yours due to misconfigured
user.email, you can make correction, before publishing the
project, like this:


git filter-branch --env-filter '
if test $GIT_AUTHOR_EMAIL = root@localhost
then
GIT_AUTHOR_EMAIL=y...@example.com
export GIT_AUTHOR_EMAIL
fi
if test $GIT_COMMITTER_EMAIL = root@localhost
then
GIT_COMMITTER_EMAIL=y...@example.com
export GIT_COMMITTER_EMAIL
fi
' -- --all


By the way, I left the export in; git filter-branch --help
explicitly says that you need to re-export it.  But I am not sure if
they are necessary, especially after 3c730fab2cae (filter-branch:
use git-sh-setup's ident parsing functions, 2012-10-18) by Peff,
which added extra export to make sure all six identity variables
are exported.  After applying the above rewrite, we may want to do
the following as a separate, follow-up patch.

 Documentation/git-filter-branch.txt | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/Documentation/git-filter-branch.txt 
b/Documentation/git-filter-branch.txt
index 8ebe999..066548e 100644
--- a/Documentation/git-filter-branch.txt
+++ b/Documentation/git-filter-branch.txt
@@ -83,8 +83,7 @@ OPTIONS
This filter may be used if you only need to modify the environment
in which the commit will be performed.  Specifically, you might
want to rewrite the author/committer name/email/time environment
-   variables (see linkgit:git-commit-tree[1] for details).  Do not forget
-   to re-export the variables.
+   variables (see linkgit:git-commit-tree[1] for details).
 
 --tree-filter command::
This is the filter for rewriting the tree and its contents.
@@ -340,12 +339,10 @@ git filter-branch --env-filter '
if test $GIT_AUTHOR_EMAIL = root@localhost
then
GIT_AUTHOR_EMAIL=y...@example.com
-   export GIT_AUTHOR_EMAIL
fi
if test $GIT_COMMITTER_EMAIL = root@localhost
then
GIT_COMMITTER_EMAIL=y...@example.com
-   export GIT_COMMITTER_EMAIL
fi
 ' -- --all
 



--
To unsubscribe from this list: send the line 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] Documentation: filter-branch env-filter example

2013-02-14 Thread Jeff King
On Thu, Feb 14, 2013 at 04:09:10PM -0500, Jeff King wrote:

 I think the advice in the documentation about re-exporting is because
 some versions of the bourne shell will not reliably pass the new version
 of the variable when you do this:
 
   VAR=old
   export VAR
   VAR=new
   some_subprocess ;# we see $VAR=old here!
 
 I do not recall ever running across such a shell myself, but rather
 hearing about it third-hand in a portability guide somewhere.

The closest I could find in the autoconf shell guidelines[1] is that the
automagic export marking for incoming variables is not always accurate:

export
The builtin export dubs a shell variable environment
variable. Each update of exported variables corresponds to
an update of the environment variables.  Conversely, each
environment variable received by the shell when it is
launched should be imported as a shell variable marked as
exported.

Alas, many shells, such as Solaris 2.5, IRIX 6.3, IRIX 5.2,
AIX 4.1.5, and Digital UNIX 4.0, forget to export the
environment variables they receive. As a result, two
variables coexist: the environment variable and the shell
variable. The following code demonstrates this failure:

#! /bin/sh
echo $FOO
FOO=bar
echo $FOO
exec /bin/sh $0

when run with `FOO=foo' in the environment, these shells
will print alternately `foo' and `bar', although it should
only print `foo' and then a sequence of `bar's.

Therefore you should export again each environment variable
that you update.

I don't know what the behavior would be on such shells of:

#!/bin/sh
echo $FOO
FOO=bar
export FOO
echo $FOO
exec /bin/sh $0

I.e., would the export correctly reconcile the local and environment
copies of the variable, or are they forever broken? I don't have such a
system to test on. But that would more closely match what we are doing.

-Peff

[1] 
https://www.gnu.org/software/autoconf/manual/autoconf.html#Limitations-of-Builtins
--
To unsubscribe from this list: send the line 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] Documentation: filter-branch env-filter example

2013-02-14 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 I think it has always been the case that we export them after setting
 them; just look at the preimage from 3c730fab, and you can see exports
 there.

Yeah, I think the only difference is a broken commit case where sed
expression did not find what it was looking for, in which case we do
not do the export.

 I think the advice in the documentation about re-exporting is because
 some versions of the bourne shell will not reliably pass the new version
 of the variable ...

Ahh, old and painful memory of Solaris days comes back to me.  OK,
let's keep the export then.

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


What's cooking in git.git (Feb 2013, #06; Thu, 14)

2013-02-14 Thread Junio C Hamano
Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.

A preview of the upcoming release 1.8.2-rc0 is expected to be tagged
late this week.  At around that time, we may want to discard
long-stalled topics that did not see activities as well.

You can find the changes described here in the integration branches of the
repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[Graduated to master]

* al/mergetool-printf-fix (2013-02-10) 2 commits
  (merged to 'next' on 2013-02-11 at 5f9bc4e)
 + difftool--helper: fix printf usage
 + git-mergetool: print filename when it contains %


* bw/get-tz-offset-perl (2013-02-09) 3 commits
  (merged to 'next' on 2013-02-11 at b9c8893)
 + cvsimport: format commit timestamp ourselves without using strftime
 + perl/Git.pm: fix get_tz_offset to properly handle DST boundary cases
 + Move Git::SVN::get_tz to Git::get_tz_offset

 git-cvsimport and git-svn miscomputed TZ offset at DST boundary.


* dg/subtree-fixes (2013-02-05) 6 commits
  (merged to 'next' on 2013-02-09 at 8f19ebe)
 + contrib/subtree: make the manual directory if needed
 + contrib/subtree: honor DESTDIR
 + contrib/subtree: fix synopsis
 + contrib/subtree: better error handling for 'subtree add'
 + contrib/subtree: use %B for split subject/body
 + contrib/subtree: remove test number comments

 contrib/subtree updates, but here are only the ones that looked
 ready.


* jc/extended-fake-ancestor-for-gitlink (2013-02-05) 1 commit
  (merged to 'next' on 2013-02-09 at 2d3547b)
 + apply: verify submodule commit object name better

 Instead of requiring the full 40-hex object names on the index
 line, we can read submodule commit object names from the textual
 diff when synthesizing a fake ancestore tree for git am -3.


* jk/diff-graph-cleanup (2013-02-12) 6 commits
  (merged to 'next' on 2013-02-12 at 6e764c1)
 + combine-diff.c: teach combined diffs about line prefix
 + diff.c: use diff_line_prefix() where applicable
 + diff: add diff_line_prefix function
 + diff.c: make constant string arguments const
 + diff: write prefix to the correct file
 + graph: output padding for merge subsequent parents

 Refactors a lot of repetitive code sequence from the graph drawing
 code and adds it to the combined diff output.


* jk/error-const-return (2013-02-08) 1 commit
  (merged to 'next' on 2013-02-11 at ba8dba3)
 + Use __VA_ARGS__ for all of error's arguments


* jx/utf8-printf-width (2013-02-11) 1 commit
  (merged to 'next' on 2013-02-11 at 968b4e2)
 + Add utf8_fprintf helper that returns correct number of columns

 Use a new helper that prints a message and counts its display width
 to align the help messages parse-options produces.


* mg/bisect-doc (2013-02-11) 1 commit
  (merged to 'next' on 2013-02-11 at 6125304)
 + git-bisect.txt: clarify that reset quits bisect


* mm/remote-mediawiki-build (2013-02-08) 2 commits
  (merged to 'next' on 2013-02-11 at 4ebb902)
 + git-remote-mediawiki: use toplevel's Makefile
 + Makefile: make script-related rules usable from subdirectories


* nd/status-show-in-progress (2013-02-05) 1 commit
  (merged to 'next' on 2013-02-11 at 5ffcbc2)
 + status: show the branch name if possible in in-progress info


* tz/perl-styles (2013-02-06) 1 commit
  (merged to 'next' on 2013-02-09 at c8cff17)
 + Update CodingGuidelines for Perl

 Add coding guidelines for writing Perl scripts for Git.

--
[New Topics]

* mk/make-rm-depdirs-could-be-empty (2013-02-13) 1 commit
  (merged to 'next' on 2013-02-14 at d966248)
 + Makefile: don't run rm without any files

 make COMPUTE_HEADER_DEPENDENCIES=no clean would try to run rm
 -rf $(dep_dirs) with an empty dep_dir, but some implementations of
 rm -rf barf on an empty argument list.

 Will merge to 'master'.


* mw/bash-prompt-show-untracked-config (2013-02-13) 3 commits
  (merged to 'next' on 2013-02-14 at 809dbcf)
 + t9903: add extra tests for bash.showDirtyState
 + t9903: add tests for bash.showUntrackedFiles
 + shell prompt: add bash.showUntrackedFiles option

 Allows skipping the untracked check GIT_PS1_SHOWUNTRACKEDFILES
 asks for the git-prompt (in contrib/) per repository.

 Will merge to 'master'.


* mg/gpg-interface-using-status (2013-02-14) 5 commits
 - pretty: make %GK output the signing key for signed commits
 - pretty: parse the gpg status lines rather than the output
 - gpg_interface: allow to request status return
 - log-tree: rely upon the check in the gpg_interface
 - gpg-interface: check good signature in a reliable way

 Call gpg using the right API when validating the signature on
 tags.


* mm/config-intro-in-git-doc (2013-02-14) 1 commit
 - git.txt: update description of the configuration mechanism

--
[Stalled]

* mb/gitweb-highlight-link-target (2012-12-20) 1 

Re: What's cooking in git.git (Feb 2013, #05; Tue, 12)

2013-02-14 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 Andrew Ardill andrew.ard...@gmail.com writes:

 If that is the change we are going to make, and if you can guarantee
 that nobody who is used to the historical behaviour will complain,
 then I am fine with it, but I think the latter part of the condition
 will not hold.

 Does the impossibility of asserting that no-one will complain put this
 in the 'too hard' bucket?

 Basically, yes.  Cannot be done without UI regression.

 It could be a Git 2.0 item, if you plan the transition right, though.

I have been staring at the patch again, but I do not think of an
easy way out without retraining the old timers to introduce this if
we knew better, we would have done so from day one and the world
would have been a much better place change.  If we were to have
this in the longer term, we would need a proper transition plan,
similar to the one we devised to change the default used for a lazy
git push (and git push $there) from the traditional matching
to simple at Git 2.0 boundary.

The transition would go like this:

 * Introduce git add --ignore-removal option in the release after
   the current cycle (a new feature is too late for this cycle):

   - when git add pathspec is given without --ignore-removal,
 give a warning about upcoming default change, and advise people
 to use either --ignore-removal or --all option, but behave
 as if --ignore-removal were given.

   - when git add --ignore-removal pathspec is given, only add
 additions and modifications, just like the current behaviour.

   - obviously, -u, -A, and --ignore-removal are mutually
 exclusive.

 * Run with the above for a few releases.

 * Change the behaviour of git add pathspec without -u, -A
   nor --ignore-removal to error out with the same warning and
   advise.

 * Run with the above for a few releases.

 * At Git 2.0, change git add pathspec without -u, -A nor
   --ignore-removal to behave as if git add -A pathspec were
   given.

At any point during the above transtion, git add without any
pathspec will not change its meaning; it will stay a no-op.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3] add: warn when -u or -A is used without filepattern

2013-02-14 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 We should probably update the documentation/help for git add, but
 that is entirely a separate topic.

The documentation update in 0fa2eb530fb7 (add: warn when -u or -A is
used without pathspec, 2013-01-28) says:

If no pathspec is given, the current version of Git defaults to
.; in other words, update all tracked files in the current directory
and its subdirectories. This default will change in a future version
of Git, hence the form without filepattern should not be used.

(oops, I just spotted a stray filepattern here, which came from a
semantic mismerge---I'll fix it locally).

The above text says that we currently add what you have in your
current directory and below, before it says this default will
change.  That makes it easier to connect the default will change
and form without pathspec should not be used in readers' mind.  It
does not take that much imagination and intelligence to infer it
will change and will not limit to my current directory, so in the
future I will have to be explicit when I want to do what I just told
git to do.

But the warning text does not sound quite right.  This is what I get:

warning: The behavior of 'git add --update (or -u)' with no path argument 
from a
subdirectory of the tree will change in Git 2.0 and should not be used 
anymore.

There is a logic gap between will change and should not be used
that is not filled like the text in the manual page does.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3] add: warn when -u or -A is used without filepattern

2013-02-14 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 warning: The behavior of 'git add --update (or -u)' with no path argument 
 from a
 subdirectory of the tree will change in Git 2.0 and should not be used 
 anymore.

 There is a logic gap between will change and should not be used
 that is not filled like the text in the manual page does.

I guess it is not so bad after all, if you read the entire message,
not just the first two lines.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC v2] git-multimail: a replacement for post-receive-email

2013-02-14 Thread Michael Haggerty
On 02/14/2013 01:55 PM, Matthieu Moy wrote:
 Michael Haggerty mhag...@alum.mit.edu writes:
 
 On 02/13/2013 03:56 PM, Matthieu Moy wrote:

 Installation troubles:

 I had an old python installation (Red Hat package, and I'm not root),
 that did not include the email.utils package, so I couldn't use my
 system's python. I found no indication about python version in README,
 so I installed the latest python by hand, just to find out that
 git-multimail wasn't compatible with Python 3.x. 2to3 can fix
 automatically a number of 3.x compatibility issues, but not all of them
 so I gave up and installed Python 2.7.

 What version of Python was it that caused problems?
 
 Python 2.4.3, installed with RHEL 5.9.
 
 I just discovered that the script wouldn't have worked with Python
 2.4, where email.utils used to be called email.Utils.
 
 Indeed, import email.Utils works with this Python.
 
 But I pushed a fix to GitHub:

 ddb1796660 Accommodate older versions of Python's email module.
 
 Not sufficient, but I added a pull request that works for me with 2.4.
 
 @@ -835,6 +837,17 @@ class ReferenceChange(Change):
  for line in self.expand_lines(NO_NEW_REVISIONS_TEMPLATE):
  yield line
  
 +if adds and self.showlog:
 +yield '\n'
 +yield 'Detailed log of added commits:\n\n'
 +for line in read_lines(
 +['git', 'log']
 ++ self.logopts
 ++ ['%s..%s' % (self.old.commit, self.new.commit,)],
 +keepends=True,
 +):
 +yield line
 +
  # The diffstat is shown from the old revision to the new
  # revision.  This is to show the truth of what happened in
  # this change.  There's no point showing the stat from the


 Thanks for the patch.  I like the idea, but I think the implementation
 is incorrect.  Your code will not only list new commits but will also
 list commits that were already in the repository on another branch
 (e.g., if an existing feature branch is merged into master, all of the
 commits on the feature branch will be listed).  (Or was that your
 intention?)
 
 I did not think very carefully about this case, but the behavior of my
 code seems sensible (although not uncontroversial): it's just showing
 the detailed log for the same commits as the summary at the top of the
 email. I have no personnal preferences.

I guess it depends a lot on what logopts are used.  If the user
configures logopts to emit full patches, then the repeated reporting of
the same commits would cause a big increase in the bulk of notification
emails.  But if the logopts are set to just emit a brief summary (e.g.,
author and log message), then a bit of repetition might be acceptable.
But since I wouldn't use this feature, I don't personally have a preference.

 But even worse, it will fail to list commits that were
 added at the same time that a branch was created (e.g., if I create a
 feature branch with a number of commits on it and then push it for the
 first time).
 
 Right.
 
 Probably the Push object has to negotiate with its constituent
 ReferenceChange objects to figure out which one is responsible for
 summarizing each of the commits newly added by the push (i.e., the ones
 returned by push.get_new_commits(None)).
 
 I updated the pull request with a version that works for new branches,
 and takes the list of commits to display from the call to
 get_new_commits (which were already there for other purpose). Then, it
 essentially calls git log --no-walk $list_of_sha1s.
 
 This should be better.

I will check it out.

Thanks!

Michael

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