* Re: inotify to minimize stat() calls
From: Magnus Bäck @ 2013-02-14 14:36 UTC (permalink / raw)
To: demerphq
Cc: Duy Nguyen, Ramkumar Ramachandra, Robert Zeh, Junio C Hamano,
Git List, finnag
In-Reply-To: <CANgJU+WYSD8RHb19EP0M89=Y_XskfjDtFWf51qjg4ur+rDb3ug@mail.gmail.com>
On Sunday, February 10, 2013 at 08:26 EST,
demerphq <demerphq@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
baeck@google.com
^ permalink raw reply
* Re: inotify to minimize stat() calls
From: Ævar Arnfjörð Bjarmason @ 2013-02-14 15:16 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List
In-Reply-To: <CALkWK0=EP0Lv1F_BArub7SpL9rgFhmPtpMOCgwFqfJmVE=oa=A@mail.gmail.com>
On Fri, Feb 8, 2013 at 10:10 PM, Ramkumar Ramachandra
<artagnon@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.
^ permalink raw reply
* [PATCH] git.txt: update description of the configuration mechanism
From: Matthieu Moy @ 2013-02-14 15:36 UTC (permalink / raw)
To: git, gitster; +Cc: 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.Moy@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
^ permalink raw reply related
* [PATCH 0/5] gpg_interface: use the status
From: Michael J Gruber @ 2013-02-14 16:04 UTC (permalink / raw)
To: git; +Cc: Mariusz Gronczewski
In-Reply-To: <CAJ9Ak2pLqUesOWKNtjwG7P_BW7-EqJP4Kaj0FF_+72Wn=oGoew@mail.gmail.com>
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
^ permalink raw reply
* [PATCH 2/5] log-tree: rely upon the check in the gpg_interface
From: Michael J Gruber @ 2013-02-14 16:04 UTC (permalink / raw)
To: git; +Cc: Mariusz Gronczewski
In-Reply-To: <cover.1360857415.git.git@drmicha.warpmail.net>
It's just so much betterer.
Signed-off-by: Michael J Gruber <git@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
^ permalink raw reply related
* [PATCH 1/5] gpg-interface: check good signature in a reliable way
From: Michael J Gruber @ 2013-02-14 16:04 UTC (permalink / raw)
To: git; +Cc: Mariusz Gronczewski
In-Reply-To: <cover.1360857415.git.git@drmicha.warpmail.net>
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 <git@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_tmpXXXXXX");
@@ -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
^ permalink raw reply related
* [PATCH 3/5] gpg_interface: allow to request status return
From: Michael J Gruber @ 2013-02-14 16:04 UTC (permalink / raw)
To: git; +Cc: Mariusz Gronczewski
In-Reply-To: <cover.1360857415.git.git@drmicha.warpmail.net>
Currently, verify_signed_buffer() returns the user facing output only.
Allow callers to request the status output also.
Signed-off-by: Michael J Gruber <git@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_tmpXXXXXX");
@@ -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 eae57ad..2a1e174 100644
--- a/pretty.c
+++ b/pretty.c
@@ -984,7 +984,7 @@ 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);
+ &gpg_output, NULL);
if (status && !gpg_output.len)
goto out;
ctx->signature.gpg_output = strbuf_detach(&gpg_output, NULL);
--
1.8.1.3.797.ge0260c7
^ permalink raw reply related
* [PATCH 4/5] pretty: parse the gpg status lines rather than the output
From: Michael J Gruber @ 2013-02-14 16:04 UTC (permalink / raw)
To: git; +Cc: Mariusz Gronczewski
In-Reply-To: <cover.1360857415.git.git@drmicha.warpmail.net>
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 <git@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
^ permalink raw reply related
* [PATCH 5/5] pretty: make %GK output the signing key for signed commits
From: Michael J Gruber @ 2013-02-14 16:04 UTC (permalink / raw)
To: git; +Cc: Mariusz Gronczewski
In-Reply-To: <cover.1360857415.git.git@drmicha.warpmail.net>
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 <git@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
^ permalink raw reply related
* Re: [PATCH] git.txt: update description of the configuration mechanism
From: Michael J Gruber @ 2013-02-14 16:15 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git, gitster
In-Reply-To: <1360856214-934-1-git-send-email-Matthieu.Moy@imag.fr>
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.Moy@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
>
^ permalink raw reply
* Re: [PATCH] git.txt: update description of the configuration mechanism
From: Matthieu Moy @ 2013-02-14 16:23 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, gitster
In-Reply-To: <511D0D88.6010302@drmicha.warpmail.net>
Michael J Gruber <git@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/
^ permalink raw reply
* Re: inotify to minimize stat() calls
From: Junio C Hamano @ 2013-02-14 16:31 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: Ramkumar Ramachandra, Git List
In-Reply-To: <CACBZZX6BVuQWtrLuTVXZo+77sT4yZQ3pvN=_fMma24-zd0NNqA@mail.gmail.com>
Ævar Arnfjörð Bjarmason <avarab@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.
^ permalink raw reply
* Re: [BUG] Veryfing signatures in git log fails when language is not english
From: Junio C Hamano @ 2013-02-14 16:47 UTC (permalink / raw)
To: Mariusz Gronczewski; +Cc: Michael J Gruber, git
In-Reply-To: <CAJ9Ak2pLqUesOWKNtjwG7P_BW7-EqJP4Kaj0FF_+72Wn=oGoew@mail.gmail.com>
Mariusz Gronczewski <xani666@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 %G<anything> 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.
^ permalink raw reply
* Re: [PATCH] git.txt: update description of the configuration mechanism
From: Junio C Hamano @ 2013-02-14 16:52 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git
In-Reply-To: <1360856214-934-1-git-send-email-Matthieu.Moy@imag.fr>
Matthieu Moy <Matthieu.Moy@imag.fr> writes:
> The old Git version where it appeared is not useful only to historians,
> not to normal users.
That statement cannot be generally true, unless the "old" is really
old and no longer relevant but 0.99.9 is sufficiently old so the
value of the part of the message you removed that warns users with
ancient versions that they may not be able to use the feature
without upgrading has diminished.
> +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.
Both look sensible.
^ permalink raw reply
* Re: What's cooking in git.git (Feb 2013, #05; Tue, 12)
From: Junio C Hamano @ 2013-02-14 16:59 UTC (permalink / raw)
To: Andrew Ardill; +Cc: git@vger.kernel.org
In-Reply-To: <CAH5451mMG-U8qETAy_6pRJLbtOjtAPhbapVA9RLbrrS2yy7rCw@mail.gmail.com>
Andrew Ardill <andrew.ardill@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.
^ permalink raw reply
* Re: [PATCH v4 3/4] count-objects: report garbage files in pack directory too
From: Junio C Hamano @ 2013-02-14 17:02 UTC (permalink / raw)
To: Duy Nguyen; +Cc: git
In-Reply-To: <CACsJy8DxRuML1+gSCZaGC1hA4MU9sWnoGJGZMbeUVdaipOH7vA@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> On Wed, Feb 13, 2013 at 10:55 PM, Junio C Hamano <gitster@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.
^ permalink raw reply
* Re: [PATCH] git.txt: update description of the configuration mechanism
From: Junio C Hamano @ 2013-02-14 17:10 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Michael J Gruber, git
In-Reply-To: <vpqzjz6ampm.fsf@grenoble-inp.fr>
Matthieu Moy <Matthieu.Moy@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 = "junkio@twinsun.com"
+ email = "gitster@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
^ permalink raw reply related
* Re: [PATCH 1/5] gpg-interface: check good signature in a reliable way
From: Junio C Hamano @ 2013-02-14 17:22 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Mariusz Gronczewski
In-Reply-To: <e0f3c48e474c64e33c67c772468bbf7bba0169af.1360857415.git.git@drmicha.warpmail.net>
Michael J Gruber <git@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 <git@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_tmpXXXXXX");
> @@ -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;
> }
^ permalink raw reply
* Re: [PATCH] git.txt: update description of the configuration mechanism
From: Matthieu Moy @ 2013-02-14 17:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael J Gruber, git
In-Reply-To: <7v4nhe3joq.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@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/
^ permalink raw reply
* Re: [PATCH v4.1 09/12] sequencer.c: teach append_signoff to avoid adding a duplicate newline
From: John Keeping @ 2013-02-14 17:58 UTC (permalink / raw)
To: Brandon Casey; +Cc: git, gitster, pclouds, jrnieder, Brandon Casey
In-Reply-To: <1360665222-3166-1-git-send-email-drafnel@gmail.com>
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 <bcasey@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,
^ permalink raw reply
* Re: [PATCH] git.txt: update description of the configuration mechanism
From: Junio C Hamano @ 2013-02-14 18:03 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Michael J Gruber, git
In-Reply-To: <vpqy5eq94rq.fsf@grenoble-inp.fr>
Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> Junio C Hamano <gitster@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.
^ permalink raw reply
* Re: [PATCH v2 02/15] user-manual: Update for receive.denyCurrentBranch=refuse
From: Junio C Hamano @ 2013-02-14 18:57 UTC (permalink / raw)
To: W. Trevor King; +Cc: Git
In-Reply-To: <7v1ucnssnw.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> "W. Trevor King" <wking@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" <wking@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 <wking@tremily.us>
Signed-off-by: Junio C Hamano <gitster@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
^ permalink raw reply related
* [PATCH] Documentation: filter-branch env-filter example
From: Tadeusz Andrzej Kadłubowski @ 2013-02-14 19:34 UTC (permalink / raw)
To: git
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 = john@old.example.com ]
+ then
+ GIT_AUTHOR_EMAIL=john@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
^ permalink raw reply related
* Re: [PATCH] Documentation: filter-branch env-filter example
From: Junio C Hamano @ 2013-02-14 20:29 UTC (permalink / raw)
To: Tadeusz Andrzej Kadłubowski; +Cc: git, Jeff King
In-Reply-To: <511D3C33.6070902@hell.org.pl>
Tadeusz Andrzej Kadłubowski <yess@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 = john@old.example.com ]
Quote the variable in double-quotes, like this:
if [ "$GIT_AUTHOR_EMAIL" = john@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=john@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=yess@example.com
export GIT_AUTHOR_EMAIL
fi
if test "$GIT_COMMITTER_EMAIL" = "root@localhost"
then
GIT_COMMITTER_EMAIL=yess@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=yess@example.com
- export GIT_AUTHOR_EMAIL
fi
if test "$GIT_COMMITTER_EMAIL" = "root@localhost"
then
GIT_COMMITTER_EMAIL=yess@example.com
- export GIT_COMMITTER_EMAIL
fi
' -- --all
--------------------------------------------------------
^ permalink raw reply related
* Re: [PATCH] Documentation: filter-branch env-filter example
From: Jeff King @ 2013-02-14 21:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Tadeusz Andrzej Kadłubowski, git
In-Reply-To: <7vy5eqy6z3.fsf@alter.siamese.dyndns.org>
On Thu, Feb 14, 2013 at 12:29:36PM -0800, Junio C Hamano wrote:
> Quote the variable in double-quotes, like this:
>
> if [ "$GIT_AUTHOR_EMAIL" = john@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?
Yeah, definitely. If you are cleaning up a broken ident that cannot be
parsed, the failure mode is to put the whole author line in
$GIT_AUTHOR_EMAIL, which would almost certainly include spaces.
> 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=yess@example.com
> export GIT_AUTHOR_EMAIL
> fi
> if test "$GIT_COMMITTER_EMAIL" = "root@localhost"
> then
> GIT_COMMITTER_EMAIL=yess@example.com
> export GIT_COMMITTER_EMAIL
> fi
> ' -- --all
> --------------------------------------------------------
That looks better, though there are a few English nits; here's my edited
version:
The `--env-filter` option can be used to modify committer and/or
author identity. For example, if you found out that your
commits have the wrong identity due to a misconfigured
user.email, you can make a correction, before publishing the
project, like this:
> 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.
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.
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. Apple's
shell documentation seems to indicate that /bin/sh in older versions of
OS X had this behavior:
https://developer.apple.com/library/mac/documentation/opensource/conceptual/shellscripting/shell_scripts/shell_scripts.html#//apple_ref/doc/uid/TP40004268-CH237-SW11
which makes me think that BSD ash may behave that way. It is certainly
not necessary to re-export under bash or dash. I shudder to think what
horrible, 1980's-era behavior is codified in Solaris /bin/sh.
We could explicitly re-export all of the ident variables preemptively
before calling commit-tree, just to save the user the hassle of
remembering to do so. It would be a no-op on sane shells, and I doubt
the runtime cost is very high. I suppose it would break somebody who
explicitly did:
unset GIT_COMMITTER_NAME ;# use the value from user.name
in their env-filter, but that seems like a pretty unlikely corner case.
-Peff
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox