Re: [PATCH 1/2] add setup step to filter-branch

[Junio C Hamano <gitster@pobox.com>]

Andreas Heiduk <asheiduk@gmail.com> writes:

> A specific `--setup` step in `git filter-branch` makes it much easier
> to define the initial values of variables used in the real filters.
> Also sourcing/defining utility functions here instead of
> `--env-filter` improves performance and minimizes clogging the output
> in case of errors.
>
> Signed-off-by: Andreas Heiduk <asheiduk@gmail.com>
> ---

I was placed on To: line, but I do not have a strong opinion on this
change, either for or against.

"filter-branch" program itself may probably already be hard to port
to C, but I need to point out that this makes it even harder than it
currently is [*1*], and it is likely that it has to stay implemented
in shell forever, though.  I do not mind that future myself, but
those on platforms with weaker implementation of shells might.

[Footnote]

 *1* The issue is *not* that these individual filter commands expect
     <command> written as a shell scriptlet; it is that these
     scriptlets expect to be evaled inside a single shell process,
     making an update to a shell variable in one command visible to
     the next command that runs.


>  Documentation/git-filter-branch.txt | 16 +++++++++++-----
>  git-filter-branch.sh                | 18 +++++++++++++-----
>  2 files changed, 24 insertions(+), 10 deletions(-)
>
> diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt
> index 6e4bb0220..45c849d8c 100644
> --- a/Documentation/git-filter-branch.txt
> +++ b/Documentation/git-filter-branch.txt
> @@ -8,11 +8,11 @@ git-filter-branch - Rewrite branches
>  SYNOPSIS
>  --------
>  [verse]
> -'git filter-branch' [--env-filter <command>] [--tree-filter <command>]
> -	[--index-filter <command>] [--parent-filter <command>]
> -	[--msg-filter <command>] [--commit-filter <command>]
> -	[--tag-name-filter <command>] [--subdirectory-filter <directory>]
> -	[--prune-empty]
> +'git filter-branch' [--setup <command>] [--env-filter <command>]
> +	[--tree-filter <command>] [--index-filter <command>]
> +	[--parent-filter <command>] [--msg-filter <command>]
> +	[--commit-filter <command>] [--tag-name-filter <command>]
> +	[--subdirectory-filter <directory>] [--prune-empty]
>  	[--original <namespace>] [-d <directory>] [-f | --force]
>  	[--] [<rev-list options>...]
>  
> @@ -82,6 +82,12 @@ multiple commits.
>  OPTIONS
>  -------
>  
> +--setup <command>::
> +	This is not a real filter executed for each commit but a one
> +	time setup just before the loop. Therefore no commit-specific
> +	variables are defined yet.  Functions or variables defined here
> +	can be used or modified in the following filter steps.
> +
>  --env-filter <command>::
>  	This filter may be used if you only need to modify the environment
>  	in which the commit will be performed.  Specifically, you might
> diff --git a/git-filter-branch.sh b/git-filter-branch.sh
> index aafaf708d..2758ae5eb 100755
> --- a/git-filter-branch.sh
> +++ b/git-filter-branch.sh
> @@ -81,11 +81,12 @@ set_ident () {
>  	finish_ident COMMITTER
>  }
>  
> -USAGE="[--env-filter <command>] [--tree-filter <command>]
> -	[--index-filter <command>] [--parent-filter <command>]
> -	[--msg-filter <command>] [--commit-filter <command>]
> -	[--tag-name-filter <command>] [--subdirectory-filter <directory>]
> -	[--original <namespace>] [-d <directory>] [-f | --force]
> +USAGE="[--setup <command>] [--env-filter <command>]
> +	[--tree-filter <command>] [--index-filter <command>]
> +	[--parent-filter <command>] [--msg-filter <command>]
> +	[--commit-filter <command>] [--tag-name-filter <command>]
> +	[--subdirectory-filter <directory>] [--original <namespace>]
> +	[-d <directory>] [-f | --force]
>  	[<rev-list options>...]"
>  
>  OPTIONS_SPEC=
> @@ -96,6 +97,7 @@ if [ "$(is_bare_repository)" = false ]; then
>  fi
>  
>  tempdir=.git-rewrite
> +filter_setup=
>  filter_env=
>  filter_tree=
>  filter_index=
> @@ -148,6 +150,9 @@ do
>  	-d)
>  		tempdir="$OPTARG"
>  		;;
> +	--setup)
> +		filter_setup="$OPTARG"
> +		;;
>  	--env-filter)
>  		filter_env="$OPTARG"
>  		;;
> @@ -317,6 +322,9 @@ else
>  	need_index=
>  fi
>  
> +eval "$filter_setup" < /dev/null ||
> +	die "filter setup failed: $filter_setup"
> +
>  while read commit parents; do
>  	git_filter_branch__commit_count=$(($git_filter_branch__commit_count+1))