Re: [PATCH 2/2] filter-branch: use git-sh-setup's ident parsing functions
On Thu, Oct 18, 2012 at 06:22:17AM -0400, Jeff King wrote: > Hrm. We _do_ test this in t7003. Weirder, if I instrument filter-branch > like this: > [...] > and run t7003, it shows that the variable is properly exported to the > sub-process! But I can't seem to figure out why. Confused... Oh, I see. The variables are already exported by test-lib.sh. You can see the breakage with: diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh index 4d13e10..1e7a209 100755 --- a/t/t7003-filter-branch.sh +++ b/t/t7003-filter-branch.sh @@ -167,10 +167,11 @@ test_expect_success 'author information is preserved' ' test_tick && GIT_AUTHOR_NAME="B V Uips" git commit -m bvuips && git branch preserved-author && - git filter-branch -f --msg-filter "cat; \ + (sane_unset GIT_AUTHOR_NAME && +git filter-branch -f --msg-filter "cat; \ test \$GIT_COMMIT != $(git rev-parse master) || \ echo Hallo" \ - preserved-author && + preserved-author) && test 1 = $(git rev-list --author="B V Uips" preserved-author | wc -l) ' -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] filter-branch: use git-sh-setup's ident parsing functions
On Thu, Oct 18, 2012 at 03:54:29AM -0400, Jeff King wrote:
> 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).
Hrm. We _do_ test this in t7003. Weirder, if I instrument filter-branch
like this:
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 69406ae..1b504ce 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -298,8 +298,13 @@ while read commit parents; do
git cat-file commit "$commit" >../commit ||
die "Cannot read commit $commit"
+ echo >&2 pre: $GIT_AUTHOR_NAME
+ sh -c 'echo >&2 pre, subshell: $GIT_AUTHOR_NAME'
+
eval "$(set_ident <../commit)" ||
die "setting author/committer failed for commit $commit"
+ echo >&2 post: $GIT_AUTHOR_NAME
+ sh -c 'echo >&2 post, subshell: $GIT_AUTHOR_NAME'
eval "$filter_env" < /dev/null ||
die "env filter failed: $filter_env"
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 4d13e10..ce57fc5 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -174,6 +174,8 @@ test_expect_success 'author information is preserved' '
test 1 = $(git rev-list --author="B V Uips" preserved-author | wc -l)
'
+test_done
+
test_expect_success "remove a certain author's commits" '
echo i > i &&
test_tick &&
and run t7003, it shows that the variable is properly exported to the
sub-process! But I can't seem to figure out why. Confused...
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] filter-branch: use git-sh-setup's ident parsing functions
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] filter-branch: use git-sh-setup's ident parsing functions
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] filter-branch: use git-sh-setup's ident parsing functions
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
---
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 ] [--tree-filter ]
@@ -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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
