* Re: [PATCH] Add new git-remote-hd helper
From: Johannes Schindelin @ 2012-10-18 9:10 UTC (permalink / raw)
To: Felipe Contreras
Cc: Sverre Rabbelier, git, Junio C Hamano, Ilari Liusvaara,
Daniel Barkalow
In-Reply-To: <CAMP44s2wOX0-UrTM5t5ewZ4yhOJJU=+SfB9PoTQKdsP4Pzsnpg@mail.gmail.com>
Hi Felipe,
On Thu, 18 Oct 2012, Felipe Contreras wrote:
> On Thu, Oct 18, 2012 at 10:47 AM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>
> So your idea of collaboration is accept that your code is awesome, and
> my code sucks, and that I should fix your code, and throw my code to the
> trash, while you do absolutely nothing but complain about the whole
> situation.
I said no such thing. I like to keep things professional and keep emotions
out.
Hth,
Johannes
^ permalink raw reply
* Re: [PATCH 3/6] pretty: prepare notes message at a centralized place
From: Junio C Hamano @ 2012-10-18 9:17 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20121018074921.GC9999@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> It's possible that switching it would create bugs elsewhere (there are
> over 100 uses of strbuf_detach, so maybe somebody really does want this
> NULL behavior), but I tend to think it is just as likely to be fixing
> undiscovered bugs.
Yeah, I tend to agree.
This "format-patch --notes" is obviously a post 1.8.0 topic, and so
is the strbuf_detach() clean-up. Let me bookmark this thread in
case it hasn't been resolved when I came back from my vacation, so
that I won't forget ;-).
^ permalink raw reply
* Re: [PATCH] Add new git-remote-hd helper
From: Felipe Contreras @ 2012-10-18 9:10 UTC (permalink / raw)
To: Sverre Rabbelier
Cc: Jeff King, git, Junio C Hamano, Johannes Schindelin,
Ilari Liusvaara, Daniel Barkalow
In-Reply-To: <CAGdFq_hsCjL5uunwg=N-y+FkFt-ZxORtrEcxAAHt-rDfA04cZw@mail.gmail.com>
On Thu, Oct 18, 2012 at 8:12 AM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
> On Wed, Oct 17, 2012 at 10:18 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> Right now I've just added an error when using remote repositories. But
>> it seems there's no way around it; if we want to have support for
>> remote repos, we need to make a local clone.
>
> My git-remote-hg does the local clone into .git/ using a hash of the
> url (although you could just as well use urlencode, basically any way
> to safely use a url as a directory name). Have a look if you want.
Can you point to the version you are talking about? I've been checking
the remote-hg branch of fingolfin.
https://github.com/fingolfin/git/tree/remote-hg/
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH] Add new git-remote-hd helper
From: Felipe Contreras @ 2012-10-18 9:03 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Sverre Rabbelier, git, Junio C Hamano, Ilari Liusvaara,
Daniel Barkalow
In-Reply-To: <alpine.DEB.1.00.1210181031320.3049@bonsai2>
On Thu, Oct 18, 2012 at 10:47 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi Felipe,
>
> On Wed, 17 Oct 2012, Felipe Contreras wrote:
>
>> On Wed, Oct 17, 2012 at 8:18 PM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
>> > On Wed, Oct 17, 2012 at 11:12 AM, Felipe Contreras
>> > <felipe.contreras@gmail.com> wrote:
>> >> But fine, lets remove the tests out of the equation (150 lines), the
>> >> number of lines of code still exceeds 3000.
>> >
>> > I don't think it's fair to just look at LOC, git-remote-hg when it was
>> > just parsing was fairly simple. Most of the current code is our copy
>> > of the python fast-import library which is only used to support
>> > pushing to mercurial.
>>
>> Well, as a rule of thumb more code means more places for bugs to hide.
>
> Everybody on this list knows that. But it is equally true that more
> functionality requires more code.
Not necessarily. There's projects with more code and less
functionality than their alternatives.
>> It is also quite frankly rather difficult to navigate; very
>> spaghetti-like. I have the feeling [...]
>
> Yours truly always welcomes constructive criticism. Other types of
> criticism, not so much.
>
> As to the functionality you seek: git-remote-hg found in
> git://github.com/msysgit/git works. It has the following advantages over
> every other solution, including the one proposed in this thread:
>
> - it works
>
> - no really, it works
Not for me.
> - it supports pushes, too
I don't care. When I need that I'll implement that with probably much less code.
> - it matured over a long time
So has CVS.
> - there are tests
Only a dozen of them. If I write the same tests for my solution would
you be happier? I didn't think so.
> - whenever we fixed bugs, we also added tests for the bug fixes
Like this one?
https://github.com/msysgit/git/commit/9f934c9987981cbecf4ebaf8eb4a8e9f1d002caf
I don't see any tests for that.
> - it is in constant use
So you say, my impression is different.
> Without push support, remote-hg is useless to me.
Different people have different needs.
Without an easy way to setup, remote-hg is useless to me.
> If there are concerns about code style or unnecessary code (insofar it is
> really unnecessary, testgit for example is not, unless you want to avoid
> robust regression tests), I will discuss issues and collaborate. If the
> idea was not to collaborate, but to show off how much shorter code can be
> when it lacks functionality and proof of robustness I require for my
> everyday use of the program, dismissing existing code and concepts, less
> so.
So your idea of collaboration is accept that your code is awesome, and
my code sucks, and that I should fix your code, and throw my code to
the trash, while you do absolutely nothing but complain about the
whole situation. I have at least looked at your code. Have you even
looked at mine?
I've done my part in making my code easily available and ready for
review. I will not reply to you anymore until you show your
willingness to collaborate that you seem to demand for me, and:
1) Point to a remote-hg branch that is independent of msysgit stuff,
or any other irrelevant stuff
2) Is based on top of a recent version of git
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH] Add new git-remote-hd helper
From: Felipe Contreras @ 2012-10-18 8:48 UTC (permalink / raw)
To: Jeff King
Cc: git, Junio C Hamano, Sverre Rabbelier, Johannes Schindelin,
Ilari Liusvaara, Daniel Barkalow
In-Reply-To: <CAMP44s0+Fhtj2rMQ1Av-49Koa=DumX8JZs5angOFSRzqtDc+9Q@mail.gmail.com>
On Thu, Oct 18, 2012 at 5:44 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Thu, Oct 18, 2012 at 12:59 AM, Jeff King <peff@peff.net> wrote:
>> The first thing I tried was:
>>
>> $ git clone hg::https://code.google.com/p/dactyl/
>
> Right, doesn't look like it works for remote repositories. I think
> that's the next feature I want to implement, but to be honest, I don't
> think it's a big issue. To replace this:
Done, now you should be able to clone and fetch remote repositories :)
https://github.com/felipec/git/commit/783e4b380ab4fabb4e2fb200722c92afc8494a83
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH] Add new git-remote-hd helper
From: Johannes Schindelin @ 2012-10-18 8:47 UTC (permalink / raw)
To: Felipe Contreras
Cc: Sverre Rabbelier, git, Junio C Hamano, Ilari Liusvaara,
Daniel Barkalow
In-Reply-To: <CAMP44s2-BCuA5v7jE8S3d-Gg+DxKf6Yte9MvTDBDr4jEkLKZ9g@mail.gmail.com>
Hi Felipe,
On Wed, 17 Oct 2012, Felipe Contreras wrote:
> On Wed, Oct 17, 2012 at 8:18 PM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
> > On Wed, Oct 17, 2012 at 11:12 AM, Felipe Contreras
> > <felipe.contreras@gmail.com> wrote:
> >> But fine, lets remove the tests out of the equation (150 lines), the
> >> number of lines of code still exceeds 3000.
> >
> > I don't think it's fair to just look at LOC, git-remote-hg when it was
> > just parsing was fairly simple. Most of the current code is our copy
> > of the python fast-import library which is only used to support
> > pushing to mercurial.
>
> Well, as a rule of thumb more code means more places for bugs to hide.
Everybody on this list knows that. But it is equally true that more
functionality requires more code.
Besides, we are talking about concrete code, so there is no need at all
for handwaving arguments. GitHub makes it easy to point at exact line
numbers in exact file names in exact revisions, as you know, and we should
use that to discuss code.
> It is also quite frankly rather difficult to navigate; very
> spaghetti-like. I have the feeling [...]
Yours truly always welcomes constructive criticism. Other types of
criticism, not so much.
As to the functionality you seek: git-remote-hg found in
git://github.com/msysgit/git works. It has the following advantages over
every other solution, including the one proposed in this thread:
- it works
- no really, it works
- it supports pushes, too
- it matured over a long time
- there are tests
- whenever we fixed bugs, we also added tests for the bug fixes
- it is rock solid
- it is in constant use
Without push support, remote-hg is useless to me. Without regression tests
proving that it is rock solid, I will not use remote-hg. And I will not
indulge in efforts to duplicate work.
If there are concerns about code style or unnecessary code (insofar it is
really unnecessary, testgit for example is not, unless you want to avoid
robust regression tests), I will discuss issues and collaborate. If the
idea was not to collaborate, but to show off how much shorter code can be
when it lacks functionality and proof of robustness I require for my
everyday use of the program, dismissing existing code and concepts, less
so.
Hth,
Johannes
^ permalink raw reply
* Re: [PATCH 2/2] filter-branch: use git-sh-setup's ident parsing functions
From: Jeff King @ 2012-10-18 7:54 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Junio C Hamano, Ilya Basin, git
In-Reply-To: <507FB470.2060404@viscovery.net>
On Thu, Oct 18, 2012 at 09:49:04AM +0200, Johannes Sixt wrote:
> > - s/.*/GIT_'$uid'_NAME='\''&'\''; export GIT_'$uid'_NAME/p
>
> Didn't you lose the export GIT_$uid_{NAME,EMAIL,DATE} parts somewhere on
> the way?
Yikes, you're right. I didn't even notice, as the test suite still
passes. I can see how the env filter would still be able to see the
variables, but the commit-tree call wouldn't. I guess it happens to work
because we do not test alternate idents in our filter branch tests (IOW,
we are silently rewriting each commit during the filter-branch, but it
happens to have the same identities).
I'll investigate.
-Peff
^ permalink raw reply
* Re: [PATCH 3/6] pretty: prepare notes message at a centralized place
From: Jeff King @ 2012-10-18 7:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1350539128-21577-4-git-send-email-gitster@pobox.com>
On Wed, Oct 17, 2012 at 10:45:25PM -0700, Junio C Hamano wrote:
> + if (opt->show_notes) {
> + int raw;
> + struct strbuf notebuf = STRBUF_INIT;
> +
> + raw = (opt->commit_format == CMIT_FMT_USERFORMAT);
> + format_display_notes(commit->object.sha1, ¬ebuf,
> + get_log_output_encoding(), raw);
> + ctx.notes_message = notebuf.len
> + ? strbuf_detach(¬ebuf, NULL)
> + : xcalloc(1, 1);
> + }
This last line seems like it is caused by a bug in the strbuf API.
Detaching an empty string will sometimes get you NULL and sometimes not.
For example, this:
struct strbuf foo = STRBUF_INIT;
strbuf_detach(&foo, NULL);
will return NULL. But this:
struct strbuf foo = STRBUF_INIT;
strbuf_addstr(&foo, "bar");
strbuf_reset(&foo);
strbuf_detach(&foo, NULL);
will get you a zero-length string. Which just seems insane to me. The
whole point of strbuf_detach is that you do not have to care about the
internal representation. It should probably always return a newly
allocated zero-length string.
Looking through a few uses of strbuf_detach, it looks like callers
assume they will always get a pointer from strbuf_detach, and we are
saved by implementation details. For example, sha1_file_to_archive might
have an empty file, but the fact that strbuf_attach always allocates a
byte means that the detach will never return NULL. Similarly,
argv_array_pushf would never want to turn an empty string into an
accidental NULL; it is saved by the fact that strbuf_vaddf will always
preemptively allocate 64 bytes.
It's possible that switching it would create bugs elsewhere (there are
over 100 uses of strbuf_detach, so maybe somebody really does want this
NULL behavior), but I tend to think it is just as likely to be fixing
undiscovered bugs.
-Peff
^ permalink raw reply
* Re: [PATCH 2/2] filter-branch: use git-sh-setup's ident parsing functions
From: Johannes Sixt @ 2012-10-18 7:49 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Ilya Basin, git
In-Reply-To: <20121018072535.GB9999@sigill.intra.peff.net>
Am 10/18/2012 9:25, schrieb Jeff King:
> -# When piped a commit, output a script to set the ident of either
> -# "author" or "committer
> +# Ensure non-empty id name.
> +fallback_name() {
> + echo "case \"\$GIT_$1_NAME\" in \"\") GIT_$1_NAME=\"\${GIT_$1_EMAIL%%@*}\" && export GIT_$1_NAME;; esac"
> +}
>
> set_ident () {
> - lid="$(echo "$1" | tr "[A-Z]" "[a-z]")"
> - uid="$(echo "$1" | tr "[a-z]" "[A-Z]")"
> - pick_id_script='
> - /^'$lid' /{
> - s/'\''/'\''\\'\'\''/g
> - h
> - s/^'$lid' \([^<]*\) <[^>]*> .*$/\1/
> - s/'\''/'\''\'\'\''/g
> - s/.*/GIT_'$uid'_NAME='\''&'\''; export GIT_'$uid'_NAME/p
> -
> - g
> - s/^'$lid' [^<]* <\([^>]*\)> .*$/\1/
> - s/'\''/'\''\'\'\''/g
> - s/.*/GIT_'$uid'_EMAIL='\''&'\''; export GIT_'$uid'_EMAIL/p
> -
> - g
> - s/^'$lid' [^<]* <[^>]*> \(.*\)$/@\1/
> - s/'\''/'\''\'\'\''/g
> - s/.*/GIT_'$uid'_DATE='\''&'\''; export GIT_'$uid'_DATE/p
> -
> - q
> - }
> - '
> -
> - LANG=C LC_ALL=C sed -ne "$pick_id_script"
> - # Ensure non-empty id name.
> - echo "case \"\$GIT_${uid}_NAME\" in \"\") GIT_${uid}_NAME=\"\${GIT_${uid}_EMAIL%%@*}\" && export GIT_${uid}_NAME;; esac"
> + parse_ident_from_commit author AUTHOR committer COMMITTER
> + fallback_name AUTHOR
> + fallback_name COMMITTER
> }
Didn't you lose the export GIT_$uid_{NAME,EMAIL,DATE} parts somewhere on
the way?
Thanks for working on this!
-- Hannes
^ permalink raw reply
* Re: [PATCH] transport-helper: call git fast-import properly
From: Felipe Contreras @ 2012-10-18 7:39 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: git, Junio C Hamano, Daniel Barkalow
In-Reply-To: <CAGdFq_jNs_CNN8PGbbgvJbmNTSSN9TaEDxCSyDEpScZL4zyiwg@mail.gmail.com>
On Thu, Oct 18, 2012 at 7:13 AM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
> On Wed, Oct 17, 2012 at 1:27 AM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> The marks options are being ignored right now.
>
> It seems unlikely to me that this never worked, surely no reviewer
> would accept a patch that doesn't actually implement the feature?
> What's the history here?
Now I see, the {import,export}-marks options are only meant for
fast-export, for fast-import one should use the 'feature' commands. It
took me a while because the git_remote_helper code for python is very
confusing: it uses testgit.marks for the marks that git generates, and
git.marks for the marks that testgit generates.
It's not very convenient for remote-helpers that can export single
branches as opposed to the whole repo:
http://github.com/felipec/git/commit/0961fdf8231a4ac057eec8a306a708e66f7b6ae9
But it works, so this patch is not needed.
--
Felipe Contreras
^ permalink raw reply
* [PATCH 2/2] filter-branch: use git-sh-setup's ident parsing functions
From: Jeff King @ 2012-10-18 7:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Ilya Basin, git
In-Reply-To: <20121018072207.GA1605@sigill.intra.peff.net>
This saves us some code, but it also reduces the number of
processes we start for each filtered commit. Since we can
parse both author and committer in the same sed invocation,
we save one process. And since the new interface avoids tr,
we save 4 processes.
It also avoids using "tr", which has had some odd
portability problems reported with from Solaris's xpg6
version.
Signed-off-by: Jeff King <peff@peff.net>
---
git-filter-branch.sh | 42 +++++++++---------------------------------
1 file changed, 9 insertions(+), 33 deletions(-)
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 178e453..69406ae 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -64,37 +64,15 @@ set_ident () {
eval "$functions"
-# When piped a commit, output a script to set the ident of either
-# "author" or "committer
+# Ensure non-empty id name.
+fallback_name() {
+ echo "case \"\$GIT_$1_NAME\" in \"\") GIT_$1_NAME=\"\${GIT_$1_EMAIL%%@*}\" && export GIT_$1_NAME;; esac"
+}
set_ident () {
- lid="$(echo "$1" | tr "[A-Z]" "[a-z]")"
- uid="$(echo "$1" | tr "[a-z]" "[A-Z]")"
- pick_id_script='
- /^'$lid' /{
- s/'\''/'\''\\'\'\''/g
- h
- s/^'$lid' \([^<]*\) <[^>]*> .*$/\1/
- s/'\''/'\''\'\'\''/g
- s/.*/GIT_'$uid'_NAME='\''&'\''; export GIT_'$uid'_NAME/p
-
- g
- s/^'$lid' [^<]* <\([^>]*\)> .*$/\1/
- s/'\''/'\''\'\'\''/g
- s/.*/GIT_'$uid'_EMAIL='\''&'\''; export GIT_'$uid'_EMAIL/p
-
- g
- s/^'$lid' [^<]* <[^>]*> \(.*\)$/@\1/
- s/'\''/'\''\'\'\''/g
- s/.*/GIT_'$uid'_DATE='\''&'\''; export GIT_'$uid'_DATE/p
-
- q
- }
- '
-
- LANG=C LC_ALL=C sed -ne "$pick_id_script"
- # Ensure non-empty id name.
- echo "case \"\$GIT_${uid}_NAME\" in \"\") GIT_${uid}_NAME=\"\${GIT_${uid}_EMAIL%%@*}\" && export GIT_${uid}_NAME;; esac"
+ parse_ident_from_commit author AUTHOR committer COMMITTER
+ fallback_name AUTHOR
+ fallback_name COMMITTER
}
USAGE="[--env-filter <command>] [--tree-filter <command>]
@@ -320,10 +298,8 @@ while read commit parents; do
git cat-file commit "$commit" >../commit ||
die "Cannot read commit $commit"
- eval "$(set_ident AUTHOR <../commit)" ||
- die "setting author failed for commit $commit"
- eval "$(set_ident COMMITTER <../commit)" ||
- die "setting committer failed for commit $commit"
+ eval "$(set_ident <../commit)" ||
+ die "setting author/committer failed for commit $commit"
eval "$filter_env" < /dev/null ||
die "env filter failed: $filter_env"
--
1.8.0.rc3.3.gba630e1
^ permalink raw reply related
* [PATCH 1/2] git-sh-setup: refactor ident-parsing functions
From: Jeff King @ 2012-10-18 7:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Ilya Basin, git
In-Reply-To: <20121018072207.GA1605@sigill.intra.peff.net>
The only ident-parsing function we currently provide is
get_author_ident_from_commit. This is not very
flexible for two reasons:
1. It takes a commit as an argument, and can't read from
commit headers saved on disk.
2. It will only parse authors, not committers.
This patch provides a more flexible interface which will
parse multiple idents from a commit provide on stdin. We can
easily use it as a building block for the current function
to retain compatibility.
Signed-off-by: Jeff King <peff@peff.net>
---
Since we are counting processes in this series, I should note that this
actually adds a subshell invocation for each call, since it went from:
script='...'
sed $script
to:
sed "$(make_script)"
For filter-branch, which is really the only high-performance caller we
have, this is negated by the fact that it will do author and committer
at the same time, saving us an extra subshell (in addition to an extra
sed invocation).
git-sh-setup.sh | 62 +++++++++++++++++++++++++++++++++++++++------------------
1 file changed, 43 insertions(+), 19 deletions(-)
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index ee0e0bc..22f0aed 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -191,28 +191,52 @@ get_author_ident_from_commit () {
fi
}
+# Generate a sed script to parse identities from a commit.
+#
+# Reads the commit from stdin, which should be in raw format (e.g., from
+# cat-file or "--pretty=raw").
+#
+# The first argument specifies the ident line to parse (e.g., "author"), and
+# the second specifies the environment variable to put it in (e.g., "AUTHOR"
+# for "GIT_AUTHOR_*"). Multiple pairs can be given to parse author and
+# committer.
+pick_ident_script () {
+ while test $# -gt 0
+ do
+ lid=$1; shift
+ uid=$1; shift
+ printf '%s' "
+ /^$lid /{
+ s/'/'\\\\''/g
+ h
+ s/^$lid "'\([^<]*\) <[^>]*> .*$/\1/'"
+ s/.*/GIT_${uid}_NAME='&'/p
+
+ g
+ s/^$lid "'[^<]* <\([^>]*\)> .*$/\1/'"
+ s/.*/GIT_${uid}_EMAIL='&'/p
+
+ g
+ s/^$lid "'[^<]* <[^>]*> \(.*\)$/@\1/'"
+ s/.*/GIT_${uid}_DATE='&'/p
+ }
+ "
+ done
+ echo '/^$/q'
+}
+
+# Create a pick-script as above and feed it to sed. Stdout is suitable for
+# feeding to eval.
+parse_ident_from_commit () {
+ LANG=C LC_ALL=C sed -ne "$(pick_ident_script "$@")"
+}
+
+# Parse the author from a commit given as an argument. Stdout is suitable for
+# feeding to eval to set the usual GIT_* ident variables.
get_author_ident_from_commit () {
- pick_author_script='
- /^author /{
- s/'\''/'\''\\'\'\''/g
- h
- s/^author \([^<]*\) <[^>]*> .*$/\1/
- s/.*/GIT_AUTHOR_NAME='\''&'\''/p
-
- g
- s/^author [^<]* <\([^>]*\)> .*$/\1/
- s/.*/GIT_AUTHOR_EMAIL='\''&'\''/p
-
- g
- s/^author [^<]* <[^>]*> \(.*\)$/@\1/
- s/.*/GIT_AUTHOR_DATE='\''&'\''/p
-
- q
- }
- '
encoding=$(git config i18n.commitencoding || echo UTF-8)
git show -s --pretty=raw --encoding="$encoding" "$1" -- |
- LANG=C LC_ALL=C sed -ne "$pick_author_script"
+ parse_ident_from_commit author AUTHOR
}
# Clear repo-local GIT_* environment variables. Useful when switching to
--
1.8.0.rc3.3.gba630e1
^ permalink raw reply related
* [PATCH 0/2] clean up filter-branch ident parsing
From: Jeff King @ 2012-10-18 7:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Ilya Basin, git
In-Reply-To: <20121018060847.GB6308@sigill.intra.peff.net>
On Thu, Oct 18, 2012 at 02:08:47AM -0400, Jeff King wrote:
> Working on it now. git-sh-setup works, but chasing an annoying bug in
> filter-branch. I'm sure it's something silly and stupid.
Ugh. I was being caught by crazy dash-versus-bash stuff.
Try this:
$ bash -c 'echo "\\\\"'
\\
$ dash -c 'echo "\\\\"'
\
It's the "echo will automatically do backslash escaping" magic we have
so often enjoyed. I solved it with printf.
Patches to follow.
My primary motivation was cleanup, but it also has a net reduction of 5
fork+execs for each commit that filter-branch processes. This dropped
the run-time of "git filter-branch HEAD -1000" on my linux box from 62s
to 47s. A real filter-branch would do more work in the filters, of
course, but that translates to 7.5 minutes of time saved if you are
filtering all 30,000 commits of git.git.
[1/2]: git-sh-setup: refactor ident-parsing functions
[2/2]: filter-branch: use git-sh-setup's ident parsing functions
-Peff
^ permalink raw reply
* Re: [PATCH] Add new git-remote-hd helper
From: Sverre Rabbelier @ 2012-10-18 6:12 UTC (permalink / raw)
To: Felipe Contreras
Cc: Jeff King, git, Junio C Hamano, Johannes Schindelin,
Ilari Liusvaara, Daniel Barkalow
In-Reply-To: <CAMP44s1b=dNaCbm1WkFBv6368Y+jDYdkPuscEFdiUnVJBStfVg@mail.gmail.com>
On Wed, Oct 17, 2012 at 10:18 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> Right now I've just added an error when using remote repositories. But
> it seems there's no way around it; if we want to have support for
> remote repos, we need to make a local clone.
My git-remote-hg does the local clone into .git/ using a hash of the
url (although you could just as well use urlencode, basically any way
to safely use a url as a directory name). Have a look if you want.
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: What can cause empty GIT_AUTHOR_NAME for 'git filter-branch --tree-filter' on Solaris?
From: Jeff King @ 2012-10-18 6:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Ilya Basin, git
In-Reply-To: <7vk3uomi0c.fsf@alter.siamese.dyndns.org>
On Wed, Oct 17, 2012 at 11:06:11PM -0700, Junio C Hamano wrote:
> >> - eval "$(set_ident AUTHOR <../commit)" ||
> >> + eval "$(set_ident AUTHOR author <../commit)" ||
> >
> > I cringe a little at losing DRY-ness to avoid processes.
>
> Well, the header field token "author" and the middle word of the
> variable GIT_AUTHOR_NAME _happen_ to be the same modulo case, but
> they did not have to be, so you could argue the updated set_ident
> implementation is more generally useful (you could even argue that
> we should spell the first parameter out as "GIT_AUTHOR_NAME" and
> "GIT_AUTHOR_EMAIL", two separate parameters).
True, though that is even more work for the caller (and *_DATE, too). We
could make it "GIT_AUTHOR", but I don't think there is much point in
being that level of half-way general. The caller can always pick it out
of the variables if they really want to do something tricky.
> > Speaking of repetition, this seems like almost the exact same parsing
> > that happens in git-sh-setup's get_author_ident_from_commit. Maybe it's
> > worth merging them. I suspect you could also avoid another process
> > by parsing out both author and committer information in the same sed
> > invocation.
>
> Yes, yes and yes.
Working on it now. git-sh-setup works, but chasing an annoying bug in
filter-branch. I'm sure it's something silly and stupid.
-Peff
^ permalink raw reply
* Re: What can cause empty GIT_AUTHOR_NAME for 'git filter-branch --tree-filter' on Solaris?
From: Junio C Hamano @ 2012-10-18 6:06 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Sixt, Ilya Basin, git
In-Reply-To: <20121018053656.GA6308@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Thu, Oct 18, 2012 at 07:31:35AM +0200, Johannes Sixt wrote:
>
>> Right. But we should really be doing something like this instead to save a
>> few subprocesses.
>> [...]
>> - eval "$(set_ident AUTHOR <../commit)" ||
>> + eval "$(set_ident AUTHOR author <../commit)" ||
>
> I cringe a little at losing DRY-ness to avoid processes.
Well, the header field token "author" and the middle word of the
variable GIT_AUTHOR_NAME _happen_ to be the same modulo case, but
they did not have to be, so you could argue the updated set_ident
implementation is more generally useful (you could even argue that
we should spell the first parameter out as "GIT_AUTHOR_NAME" and
"GIT_AUTHOR_EMAIL", two separate parameters).
> Speaking of repetition, this seems like almost the exact same parsing
> that happens in git-sh-setup's get_author_ident_from_commit. Maybe it's
> worth merging them. I suspect you could also avoid another process
> by parsing out both author and committer information in the same sed
> invocation.
Yes, yes and yes.
^ permalink raw reply
* [PATCH 6/6] format-patch --notes: show notes after three-dashes
From: Junio C Hamano @ 2012-10-18 5:45 UTC (permalink / raw)
To: git
In-Reply-To: <1350539128-21577-1-git-send-email-gitster@pobox.com>
When inserting the note after the commit log message to format-patch
output, add three dashes before the note. Record the fact that we
did so in the rev_info and omit showing duplicated three dashes in
the usual codepath that is used when notes are not being shown.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
log-tree.c | 15 +++++++++++----
revision.h | 1 +
t/t4014-format-patch.sh | 7 +++++--
3 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index 4390b11..712a22b 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -677,8 +677,13 @@ void show_log(struct rev_info *opt)
append_signoff(&msgbuf, opt->add_signoff);
if ((ctx.fmt != CMIT_FMT_USERFORMAT) &&
- ctx.notes_message && *ctx.notes_message)
+ ctx.notes_message && *ctx.notes_message) {
+ if (ctx.fmt == CMIT_FMT_EMAIL) {
+ strbuf_addstr(&msgbuf, "---\n");
+ opt->shown_dashes = 1;
+ }
strbuf_addstr(&msgbuf, ctx.notes_message);
+ }
if (opt->show_log_size) {
printf("log size %i\n", (int)msgbuf.len);
@@ -710,6 +715,7 @@ void show_log(struct rev_info *opt)
int log_tree_diff_flush(struct rev_info *opt)
{
+ opt->shown_dashes = 0;
diffcore_std(&opt->diffopt);
if (diff_queue_is_empty()) {
@@ -737,10 +743,11 @@ int log_tree_diff_flush(struct rev_info *opt)
opt->diffopt.output_prefix_data);
fwrite(msg->buf, msg->len, 1, stdout);
}
- if ((pch & opt->diffopt.output_format) == pch) {
- printf("---");
+ if (!opt->shown_dashes) {
+ if ((pch & opt->diffopt.output_format) == pch)
+ printf("---");
+ putchar('\n');
}
- putchar('\n');
}
}
diff_flush(&opt->diffopt);
diff --git a/revision.h b/revision.h
index a95bd0b..059bfff 100644
--- a/revision.h
+++ b/revision.h
@@ -111,6 +111,7 @@ struct rev_info {
/* Format info */
unsigned int shown_one:1,
+ shown_dashes:1,
show_merge:1,
show_notes:1,
show_notes_given:1,
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index bea6381..9750ba6 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -623,9 +623,12 @@ test_expect_success 'format-patch --signoff' '
test_expect_success 'format-patch --notes --signoff' '
git notes --ref test add -m "test message" HEAD &&
git format-patch -1 --signoff --stdout --notes=test >out &&
- # Notes message must come after S-o-b
+ # Three dashes must come after S-o-b
! sed "/^Signed-off-by: /q" out | grep "test message" &&
- sed "1,/^Signed-off-by: /d" out | grep "test message"
+ sed "1,/^Signed-off-by: /d" out | grep "test message" &&
+ # Notes message must come after three dashes
+ ! sed "/^---$/q" out | grep "test message" &&
+ sed "1,/^---$/d" out | grep "test message"
'
echo "fatal: --name-only does not make sense" > expect.name-only
--
1.8.0.rc3.112.gdb88a5e
^ permalink raw reply related
* [PATCH 5/6] format-patch: append --signature after notes
From: Junio C Hamano @ 2012-10-18 5:45 UTC (permalink / raw)
To: git
In-Reply-To: <1350539128-21577-1-git-send-email-gitster@pobox.com>
When appending a new signature with "format-patch --signature", if
the "--notes" option is also in effect, the location of the new
signature (and if the signature should be added in the first place)
should be decided using the contents of the original commit log
message, before the message from the notes is added.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
log-tree.c | 6 ++++--
t/t4014-format-patch.sh | 12 ++++++++++--
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index e7e08f4..4390b11 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -672,12 +672,14 @@ void show_log(struct rev_info *opt)
ctx.reflog_info = opt->reflog_info;
ctx.fmt = opt->commit_format;
pretty_print_commit(&ctx, commit, &msgbuf);
+
+ if (opt->add_signoff)
+ append_signoff(&msgbuf, opt->add_signoff);
+
if ((ctx.fmt != CMIT_FMT_USERFORMAT) &&
ctx.notes_message && *ctx.notes_message)
strbuf_addstr(&msgbuf, ctx.notes_message);
- if (opt->add_signoff)
- append_signoff(&msgbuf, opt->add_signoff);
if (opt->show_log_size) {
printf("log size %i\n", (int)msgbuf.len);
graph_show_oneline(opt->graph);
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 959aa26..bea6381 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -616,8 +616,16 @@ test_expect_success 'format-patch --in-reply-to' '
'
test_expect_success 'format-patch --signoff' '
- git format-patch -1 --signoff --stdout |
- grep "^Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
+ git format-patch -1 --signoff --stdout >out &&
+ grep "^Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" out
+'
+
+test_expect_success 'format-patch --notes --signoff' '
+ git notes --ref test add -m "test message" HEAD &&
+ git format-patch -1 --signoff --stdout --notes=test >out &&
+ # Notes message must come after S-o-b
+ ! sed "/^Signed-off-by: /q" out | grep "test message" &&
+ sed "1,/^Signed-off-by: /d" out | grep "test message"
'
echo "fatal: --name-only does not make sense" > expect.name-only
--
1.8.0.rc3.112.gdb88a5e
^ permalink raw reply related
* [PATCH 4/6] pretty_print_commit(): do not append notes message
From: Junio C Hamano @ 2012-10-18 5:45 UTC (permalink / raw)
To: git
In-Reply-To: <1350539128-21577-1-git-send-email-gitster@pobox.com>
The only case pretty_print_commit() appends notes message to the log
message taken from the commit is when show_log() calls it with the
notes_message field set, and the output format is not the userformat
(i.e. when substituting "%N"). No other users of this function sets
this field in the pretty_print_context, as can be easily verified in
the previous step.
Hoist the code to append the notes message to the caller.
Up to this point, no functionality change is intended.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
log-tree.c | 3 +++
pretty.c | 3 ---
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index 84e9f5b..e7e08f4 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -672,6 +672,9 @@ void show_log(struct rev_info *opt)
ctx.reflog_info = opt->reflog_info;
ctx.fmt = opt->commit_format;
pretty_print_commit(&ctx, commit, &msgbuf);
+ if ((ctx.fmt != CMIT_FMT_USERFORMAT) &&
+ ctx.notes_message && *ctx.notes_message)
+ strbuf_addstr(&msgbuf, ctx.notes_message);
if (opt->add_signoff)
append_signoff(&msgbuf, opt->add_signoff);
diff --git a/pretty.c b/pretty.c
index a53eb53..1925e9c 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1417,9 +1417,6 @@ void pretty_print_commit(const struct pretty_print_context *pp,
if (pp->fmt == CMIT_FMT_EMAIL && sb->len <= beginning_of_body)
strbuf_addch(sb, '\n');
- if (pp->notes_message && *pp->notes_message)
- strbuf_addstr(sb, pp->notes_message);
-
free(reencoded);
}
--
1.8.0.rc3.112.gdb88a5e
^ permalink raw reply related
* [PATCH 3/6] pretty: prepare notes message at a centralized place
From: Junio C Hamano @ 2012-10-18 5:45 UTC (permalink / raw)
To: git
In-Reply-To: <1350539128-21577-1-git-send-email-gitster@pobox.com>
Instead of passing a boolean show_notes around, pass an optional
string that is to be inserted after the log message proper is shown.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
commit.h | 2 +-
log-tree.c | 14 +++++++++++++-
pretty.c | 9 ++++-----
3 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/commit.h b/commit.h
index a822af8..7b43e45 100644
--- a/commit.h
+++ b/commit.h
@@ -86,7 +86,7 @@ struct pretty_print_context {
enum date_mode date_mode;
unsigned date_mode_explicit:1;
int need_8bit_cte;
- int show_notes;
+ char *notes_message;
struct reflog_walk_info *reflog_info;
const char *output_encoding;
};
diff --git a/log-tree.c b/log-tree.c
index c894930..84e9f5b 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -540,7 +540,6 @@ void show_log(struct rev_info *opt)
struct pretty_print_context ctx = {0};
opt->loginfo = NULL;
- ctx.show_notes = opt->show_notes;
if (!opt->verbose_header) {
graph_show_commit(opt->graph);
@@ -648,6 +647,18 @@ void show_log(struct rev_info *opt)
if (!commit->buffer)
return;
+ if (opt->show_notes) {
+ int raw;
+ struct strbuf notebuf = STRBUF_INIT;
+
+ raw = (opt->commit_format == CMIT_FMT_USERFORMAT);
+ format_display_notes(commit->object.sha1, ¬ebuf,
+ get_log_output_encoding(), raw);
+ ctx.notes_message = notebuf.len
+ ? strbuf_detach(¬ebuf, NULL)
+ : xcalloc(1, 1);
+ }
+
/*
* And then the pretty-printed message itself
*/
@@ -689,6 +700,7 @@ void show_log(struct rev_info *opt)
}
strbuf_release(&msgbuf);
+ free(ctx.notes_message);
}
int log_tree_diff_flush(struct rev_info *opt)
diff --git a/pretty.c b/pretty.c
index 735cf0f..a53eb53 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1033,9 +1033,8 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
}
return 0; /* unknown %g placeholder */
case 'N':
- if (c->pretty_ctx->show_notes) {
- format_display_notes(commit->object.sha1, sb,
- get_log_output_encoding(), 1);
+ if (c->pretty_ctx->notes_message) {
+ strbuf_addstr(sb, c->pretty_ctx->notes_message);
return 1;
}
return 0;
@@ -1418,8 +1417,8 @@ void pretty_print_commit(const struct pretty_print_context *pp,
if (pp->fmt == CMIT_FMT_EMAIL && sb->len <= beginning_of_body)
strbuf_addch(sb, '\n');
- if (pp->show_notes)
- format_display_notes(commit->object.sha1, sb, encoding, 0);
+ if (pp->notes_message && *pp->notes_message)
+ strbuf_addstr(sb, pp->notes_message);
free(reencoded);
}
--
1.8.0.rc3.112.gdb88a5e
^ permalink raw reply related
* [PATCH 2/6] format_note(): simplify API
From: Junio C Hamano @ 2012-10-18 5:45 UTC (permalink / raw)
To: git
In-Reply-To: <1350539128-21577-1-git-send-email-gitster@pobox.com>
We either stuff the notes message without modification for %N
userformat, or format it for human consumption. Using two bits
is an overkill that does not benefit anybody.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
notes.c | 13 +++++++------
notes.h | 6 +-----
pretty.c | 5 ++---
revision.c | 2 +-
4 files changed, 11 insertions(+), 15 deletions(-)
diff --git a/notes.c b/notes.c
index bc454e1..97097db 100644
--- a/notes.c
+++ b/notes.c
@@ -1204,10 +1204,11 @@ void free_notes(struct notes_tree *t)
* If the given notes_tree is NULL, the internal/default notes_tree will be
* used instead.
*
- * 'flags' is a bitwise combination of the flags for format_display_notes.
+ * (raw != 0) gives the %N userformat; otherwise, the note message is given
+ * for human consumption.
*/
static void format_note(struct notes_tree *t, const unsigned char *object_sha1,
- struct strbuf *sb, const char *output_encoding, int flags)
+ struct strbuf *sb, const char *output_encoding, int raw)
{
static const char utf8[] = "utf-8";
const unsigned char *sha1;
@@ -1244,7 +1245,7 @@ static void format_note(struct notes_tree *t, const unsigned char *object_sha1,
if (msglen && msg[msglen - 1] == '\n')
msglen--;
- if (flags & NOTES_SHOW_HEADER) {
+ if (!raw) {
const char *ref = t->ref;
if (!ref || !strcmp(ref, GIT_NOTES_DEFAULT_REF)) {
strbuf_addstr(sb, "\nNotes:\n");
@@ -1260,7 +1261,7 @@ static void format_note(struct notes_tree *t, const unsigned char *object_sha1,
for (msg_p = msg; msg_p < msg + msglen; msg_p += linelen + 1) {
linelen = strchrnul(msg_p, '\n') - msg_p;
- if (flags & NOTES_INDENT)
+ if (!raw)
strbuf_addstr(sb, " ");
strbuf_add(sb, msg_p, linelen);
strbuf_addch(sb, '\n');
@@ -1270,13 +1271,13 @@ static void format_note(struct notes_tree *t, const unsigned char *object_sha1,
}
void format_display_notes(const unsigned char *object_sha1,
- struct strbuf *sb, const char *output_encoding, int flags)
+ struct strbuf *sb, const char *output_encoding, int raw)
{
int i;
assert(display_notes_trees);
for (i = 0; display_notes_trees[i]; i++)
format_note(display_notes_trees[i], object_sha1, sb,
- output_encoding, flags);
+ output_encoding, raw);
}
int copy_note(struct notes_tree *t,
diff --git a/notes.h b/notes.h
index 3592b19..3324c48 100644
--- a/notes.h
+++ b/notes.h
@@ -237,10 +237,6 @@ void prune_notes(struct notes_tree *t, int flags);
*/
void free_notes(struct notes_tree *t);
-/* Flags controlling how notes are formatted */
-#define NOTES_SHOW_HEADER 1
-#define NOTES_INDENT 2
-
struct string_list;
struct display_notes_opt {
@@ -274,7 +270,7 @@ void init_display_notes(struct display_notes_opt *opt);
* You *must* call init_display_notes() before using this function.
*/
void format_display_notes(const unsigned char *object_sha1,
- struct strbuf *sb, const char *output_encoding, int flags);
+ struct strbuf *sb, const char *output_encoding, int raw);
/*
* Load the notes tree from each ref listed in 'refs'. The output is
diff --git a/pretty.c b/pretty.c
index c311a68..735cf0f 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1035,7 +1035,7 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
case 'N':
if (c->pretty_ctx->show_notes) {
format_display_notes(commit->object.sha1, sb,
- get_log_output_encoding(), 0);
+ get_log_output_encoding(), 1);
return 1;
}
return 0;
@@ -1419,8 +1419,7 @@ void pretty_print_commit(const struct pretty_print_context *pp,
strbuf_addch(sb, '\n');
if (pp->show_notes)
- format_display_notes(commit->object.sha1, sb, encoding,
- NOTES_SHOW_HEADER | NOTES_INDENT);
+ format_display_notes(commit->object.sha1, sb, encoding, 0);
free(reencoded);
}
diff --git a/revision.c b/revision.c
index a09e60b..ddfba11 100644
--- a/revision.c
+++ b/revision.c
@@ -2236,7 +2236,7 @@ static int commit_match(struct commit *commit, struct rev_info *opt)
if (!buf.len)
strbuf_addstr(&buf, commit->buffer);
format_display_notes(commit->object.sha1, &buf,
- get_log_output_encoding(), 0);
+ get_log_output_encoding(), 1);
}
/* Find either in the commit object, or in the temporary */
--
1.8.0.rc3.112.gdb88a5e
^ permalink raw reply related
* [PATCH 1/6] pretty: remove reencode_commit_message()
From: Junio C Hamano @ 2012-10-18 5:45 UTC (permalink / raw)
To: git
In-Reply-To: <1350539128-21577-1-git-send-email-gitster@pobox.com>
This function has only two callsites, and is a thin wrapper whose
usefulness is dubious. When the caller needs to learn the log
output encoding, it should be able to do so by directly calling
get_log_output_encoding() and calling the underlying
logmsg_reencode() with it.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/blame.c | 5 +++--
commit.h | 2 --
pretty.c | 13 ++-----------
3 files changed, 5 insertions(+), 15 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index c27ef21..cfae569 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1425,7 +1425,7 @@ static void get_commit_info(struct commit *commit,
int detailed)
{
int len;
- const char *subject;
+ const char *subject, *encoding;
char *reencoded, *message;
static char author_name[1024];
static char author_mail[1024];
@@ -1446,7 +1446,8 @@ static void get_commit_info(struct commit *commit,
die("Cannot read commit %s",
sha1_to_hex(commit->object.sha1));
}
- reencoded = reencode_commit_message(commit, NULL);
+ encoding = get_log_output_encoding();
+ reencoded = logmsg_reencode(commit, encoding);
message = reencoded ? reencoded : commit->buffer;
ret->author = author_name;
ret->author_mail = author_mail;
diff --git a/commit.h b/commit.h
index 9f21313..a822af8 100644
--- a/commit.h
+++ b/commit.h
@@ -99,8 +99,6 @@ extern int has_non_ascii(const char *text);
struct rev_info; /* in revision.h, it circularly uses enum cmit_fmt */
extern char *logmsg_reencode(const struct commit *commit,
const char *output_encoding);
-extern char *reencode_commit_message(const struct commit *commit,
- const char **encoding_p);
extern void get_commit_format(const char *arg, struct rev_info *);
extern const char *format_subject(struct strbuf *sb, const char *msg,
const char *line_separator);
diff --git a/pretty.c b/pretty.c
index 8b1ea9f..c311a68 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1341,16 +1341,6 @@ void pp_remainder(const struct pretty_print_context *pp,
}
}
-char *reencode_commit_message(const struct commit *commit, const char **encoding_p)
-{
- const char *encoding;
-
- encoding = get_log_output_encoding();
- if (encoding_p)
- *encoding_p = encoding;
- return logmsg_reencode(commit, encoding);
-}
-
void pretty_print_commit(const struct pretty_print_context *pp,
const struct commit *commit,
struct strbuf *sb)
@@ -1367,7 +1357,8 @@ void pretty_print_commit(const struct pretty_print_context *pp,
return;
}
- reencoded = reencode_commit_message(commit, &encoding);
+ encoding = get_log_output_encoding();
+ reencoded = logmsg_reencode(commit, encoding);
if (reencoded) {
msg = reencoded;
}
--
1.8.0.rc3.112.gdb88a5e
^ permalink raw reply related
* [PATCH 0/6] Bring "format-patch --notes" closer to a real feature
From: Junio C Hamano @ 2012-10-18 5:45 UTC (permalink / raw)
To: git
This replaces the earlier "wip" with a real thing.
We never advertised the "--notes" option to format-patch (or
anything related to the pretty format options for that matter)
because the behaviour of these options was whatever they happened to
do, not what they were designed to do.
It had a few obvious glitches:
* The notes section was appended immediately after the log message,
and then the three-dash line was added. Such a supplimental
material should come after the three-dash line.
* The logic to append a new sign-off with "format-patch --signoff"
worked on the message after the note was added, which made the
detection of existing sign-off lines incorrect.
This updates the handling of "--notes" option to correct these, in
an attempt to bring it closer to a real feature.
Junio C Hamano (6):
pretty: remove reencode_commit_message()
format_note(): simplify API
pretty: prepare notes message at a centralized place
pretty_print_commit(): do not append notes message
format-patch: append --signature after notes
format-patch --notes: show notes after three-dashes
builtin/blame.c | 5 +++--
commit.h | 4 +---
log-tree.c | 32 ++++++++++++++++++++++++++++----
notes.c | 13 +++++++------
notes.h | 6 +-----
pretty.c | 22 ++++------------------
revision.c | 2 +-
revision.h | 1 +
t/t4014-format-patch.sh | 15 +++++++++++++--
9 files changed, 59 insertions(+), 41 deletions(-)
--
1.8.0.rc3.112.gdb88a5e
^ permalink raw reply
* Re: What can cause empty GIT_AUTHOR_NAME for 'git filter-branch --tree-filter' on Solaris?
From: Jeff King @ 2012-10-18 5:36 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Ilya Basin, git
In-Reply-To: <507F9437.2070501@viscovery.net>
On Thu, Oct 18, 2012 at 07:31:35AM +0200, Johannes Sixt wrote:
> Right. But we should really be doing something like this instead to save a
> few subprocesses.
> [...]
> - eval "$(set_ident AUTHOR <../commit)" ||
> + eval "$(set_ident AUTHOR author <../commit)" ||
I cringe a little at losing DRY-ness to avoid processes. But the
repetition is pretty straightforward and obvious, and I know that some
platforms are really hurt by extra processes (and this is being called
for every commit).
Speaking of repetition, this seems like almost the exact same parsing
that happens in git-sh-setup's get_author_ident_from_commit. Maybe it's
worth merging them. I suspect you could also avoid another process
by parsing out both author and committer information in the same sed
invocation.
-Peff
^ permalink raw reply
* Re: What can cause empty GIT_AUTHOR_NAME for 'git filter-branch --tree-filter' on Solaris?
From: Johannes Sixt @ 2012-10-18 5:31 UTC (permalink / raw)
To: Jeff King; +Cc: Ilya Basin, git
In-Reply-To: <20121017220912.GA21742@sigill.intra.peff.net>
Am 10/18/2012 0:09, schrieb Jeff King:
> - lid="$(echo "$1" | tr "[A-Z]" "[a-z]")"
> - uid="$(echo "$1" | tr "[a-z]" "[A-Z]")"
> + lid="$(echo "$1" | tr ABCDEFGHIJKLMNOPQRSTUVWXYZ abcdefghijklmnopqrstuvwxyz)"
> + uid="$(echo "$1" | tr abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ)"
>
> That seems like crazy overkill, but it at least will let us double-check
> that the tr sequences are the problem.
Right. But we should really be doing something like this instead to save a
few subprocesses.
-- Hannes
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 178e453..018e56e 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -68,8 +68,8 @@ eval "$functions"
# "author" or "committer
set_ident () {
- lid="$(echo "$1" | tr "[A-Z]" "[a-z]")"
- uid="$(echo "$1" | tr "[a-z]" "[A-Z]")"
+ uid=$1
+ lid=$2
pick_id_script='
/^'$lid' /{
s/'\''/'\''\\'\'\''/g
@@ -320,9 +320,9 @@ while read commit parents; do
git cat-file commit "$commit" >../commit ||
die "Cannot read commit $commit"
- eval "$(set_ident AUTHOR <../commit)" ||
+ eval "$(set_ident AUTHOR author <../commit)" ||
die "setting author failed for commit $commit"
- eval "$(set_ident COMMITTER <../commit)" ||
+ eval "$(set_ident COMMITTER committer <../commit)" ||
die "setting committer failed for commit $commit"
eval "$filter_env" < /dev/null ||
die "env filter failed: $filter_env"
^ permalink raw reply related
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