[PATCH] filter-branch: use sh -c instead of eval

[PATCH] filter-branch: use sh -c instead of eval

[Matthias Lederhofer]

If filters use variables with the same name as variables
used in the script the script breaks.  Executing the filters
in a separate process prevents accidential modification of
the variables in the main process.

Signed-off-by: Matthias Lederhofer <matled@gmx.net>
---
This one goes on top of the last patch, adding the < /dev/null.

Example:
% git filter-branch --tree-filter 'commit=foo' bar 
94ddd5151901a2b62820facc1bcf578abf842c8a (1/2) fatal: ambiguous argument 'foo': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions
fatal: ambiguous argument 'foo': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions
94ddd5151901a2b62820facc1bcf578abf842c8a
[..]
head: cannot open `../map/81208e18e22e0f1c7c73a4ea5bbd5150c0ee65c2'
for reading: No such file or directory
usage: git-update-ref [-m <reason>] (-d <refname> <value> | [--no-deref] <refname> <value> [<oldval>])
---
 git-filter-branch.sh |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 73e7c01..b446011 100644
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -54,9 +54,9 @@
 # Filters
 # ~~~~~~~
 # The filters are applied in the order as listed below. The COMMAND
-# argument is always evaluated in shell using the 'eval' command.
-# The $GIT_COMMIT environment variable is permanently set to contain
-# the id of the commit being rewritten. The author/committer environment
+# argument is always evaluated in shell using sh -c "$filter".  The
+# $GIT_COMMIT environment variable is permanently set to contain the id
+# of the commit being rewritten. The author/committer environment
 # variables are set before the first filter is run.
 #
 # A 'map' function is available that takes an "original sha1 id" argument
@@ -349,21 +349,21 @@ while read commit; do
 
 	eval "$(set_ident AUTHOR <../commit)"
 	eval "$(set_ident COMMITTER <../commit)"
-	eval "$filter_env" < /dev/null
+	sh -c "$filter_env" < /dev/null
 
 	if [ "$filter_tree" ]; then
 		git-checkout-index -f -u -a
 		# files that $commit removed are now still in the working tree;
 		# remove them, else they would be added again
 		git-ls-files -z --others | xargs -0 rm -f
-		eval "$filter_tree" < /dev/null
+		sh -c "$filter_tree" < /dev/null
 		git-diff-index -r $commit | cut -f 2- | tr '\n' '\0' | \
 			xargs -0 git-update-index --add --replace --remove
 		git-ls-files -z --others | \
 			xargs -0 git-update-index --add --replace --remove
 	fi
 
-	eval "$filter_index" < /dev/null
+	sh -c "$filter_index" < /dev/null
 
 	parentstr=
 	for parent in $(get_parents $commit); do
@@ -376,11 +376,11 @@ while read commit; do
 		fi
 	done
 	if [ "$filter_parent" ]; then
-		parentstr="$(echo "$parentstr" | eval "$filter_parent")"
+		parentstr="$(echo "$parentstr" | sh -c "$filter_parent")"
 	fi
 
 	sed -e '1,/^$/d' <../commit | \
-		eval "$filter_msg" | \
+		sh -c "$filter_msg" | \
 		sh -c "$filter_commit" git-commit-tree $(git-write-tree) $parentstr | \
 		tee ../map/$commit
 done <../revs
@@ -410,7 +410,7 @@ if [ "$filter_tag_name" ]; then
 		[ -f "../map/$sha1" ] || continue
 		new_sha1="$(cat "../map/$sha1")"
 		export GIT_COMMIT="$sha1"
-		new_ref="$(echo "$ref" | eval "$filter_tag_name")"
+		new_ref="$(echo "$ref" | sh -c "$filter_tag_name")"
 
 		echo "$ref -> $new_ref ($sha1 -> $new_sha1)"
 
-- 
1.5.2.1.860.g78ab5-dirty

Re: [PATCH] filter-branch: use sh -c instead of eval

[Johannes Sixt]

Matthias Lederhofer wrote:

> If filters use variables with the same name as variables
> used in the script the script breaks.  Executing the filters
> in a separate process prevents accidential modification of
> the variables in the main process.
> @@ -349,21 +349,21 @@ while read commit; do
>  
>  eval "$(set_ident AUTHOR <../commit)"
>  eval "$(set_ident COMMITTER <../commit)"
> -     eval "$filter_env" < /dev/null
> +     sh -c "$filter_env" < /dev/null

NACK.

The eval is on purpose here. $filter_env must be able export GIT_AUTHOR* and
GIT_COMMITTER* variables here.

Generally, it might be useful that one filter sets or exports variables that
are then available for subsequent filters or the next commit. Therefore, I
think it's actually a feature to have eval instead of sh -c even if there
is a chance that the filter overwrites internal variables. 

-- Hannes

Re: [PATCH] filter-branch: use sh -c instead of eval

[Junio C Hamano]

Johannes Sixt <johannes.sixt@telecom.at> writes:

> Matthias Lederhofer wrote:
>
>> If filters use variables with the same name as variables
>> used in the script the script breaks.  Executing the filters
>> in a separate process prevents accidential modification of
>> the variables in the main process.
>> @@ -349,21 +349,21 @@ while read commit; do
>>  
>>  eval "$(set_ident AUTHOR <../commit)"
>>  eval "$(set_ident COMMITTER <../commit)"
>> -     eval "$filter_env" < /dev/null
>> +     sh -c "$filter_env" < /dev/null
>
> NACK.
>
> The eval is on purpose here. $filter_env must be able export GIT_AUTHOR* and
> GIT_COMMITTER* variables here.

True.  The other hunks may be improvements, though.