Re: [PATCH v4] filter-branch: fix errors caused by refs that point at non-committish

2018-04-08 Thread Junio C Hamano
Yuki Kokubun  writes:

> References: <1521996898-7052-1-git-send-email-orga.chem@gmail.com>
> Content-Type: text/plain
>
> Sorry, I forgot add a line of "Reviewed-by".
> I'm gonna send the fixed patch again.

Do not worry too much about this.  Reviewed-by: added by patch
author is seen rather rarely, because the following sequence of
events need to happen:

 - The author sends a patch vN

 - A reviewer reviews vN and says "This is now Reviewed-by: me"

 - The author re-submits patch vN+1, where the only difference
   between vN and vN+1 is essentially _nothing_.  Except for the
   addition of "Reviewed-by:" by the reviewer.

But our typical patch injestion cycle is much faster and often the
last step does not happen ;-)


Re: [PATCH v4] filter-branch: fix errors caused by refs that point at non-committish

2018-03-25 Thread Junio C Hamano
Yuki Kokubun  writes:

> "git filter-branch -- --all" prints error messages when processing refs that
> point at objects that are not committish. Such refs can be created by
> "git replace" with trees or blobs. And also "git tag" with trees or blobs can
> create such refs.
>
> Filter these problematic refs out early, before they are seen by the logic to
> see which refs have been modified and which have been left intact (which is
> where the unwanted error messages come from), and warn that these refs are 
> left
> unwritten while doing so.
>
> Signed-off-by: Yuki Kokubun 
> ---
>  git-filter-branch.sh | 14 --
>  t/t7003-filter-branch.sh | 14 ++
>  2 files changed, 26 insertions(+), 2 deletions(-)

Good.  Will queue.  Thanks.

>
> diff --git a/git-filter-branch.sh b/git-filter-branch.sh
> index 1b7e4b2cd..41efecb28 100755
> --- a/git-filter-branch.sh
> +++ b/git-filter-branch.sh
> @@ -251,8 +251,18 @@ done < "$tempdir"/backup-refs
>  
>  # The refs should be updated if their heads were rewritten
>  git rev-parse --no-flags --revs-only --symbolic-full-name \
> - --default HEAD "$@" > "$tempdir"/raw-heads || exit
> -sed -e '/^^/d' "$tempdir"/raw-heads >"$tempdir"/heads
> + --default HEAD "$@" > "$tempdir"/raw-refs || exit
> +while read ref
> +do
> + case "$ref" in ^?*) continue ;; esac
> +
> + if git rev-parse --verify "$ref"^0 >/dev/null 2>&1
> + then
> + echo "$ref"
> + else
> + warn "WARNING: not rewriting '$ref' (not a committish)"
> + fi
> +done >"$tempdir"/heads <"$tempdir"/raw-refs
>  
>  test -s "$tempdir"/heads ||
>   die "You must specify a ref to rewrite."
> diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
> index 7cb60799b..04f79f32b 100755
> --- a/t/t7003-filter-branch.sh
> +++ b/t/t7003-filter-branch.sh
> @@ -470,4 +470,18 @@ test_expect_success 'tree-filter deals with object name 
> vs pathname ambiguity' '
>   git show HEAD:$ambiguous
>  '
>  
> +test_expect_success 'rewrite repository including refs that point at 
> non-commit object' '
> + test_when_finished "git reset --hard original" &&
> + tree=$(git rev-parse HEAD^{tree}) &&
> + test_when_finished "git replace -d $tree" &&
> + echo A >new &&
> + git add new &&
> + new_tree=$(git write-tree) &&
> + git replace $tree $new_tree &&
> + git tag -a -m "tag to a tree" treetag $new_tree &&
> + git reset --hard HEAD &&
> + git filter-branch -f -- --all >filter-output 2>&1 &&
> + ! fgrep fatal filter-output
> +'
> +
>  test_done


Re: [PATCH v4] filter-branch: fix errors caused by refs that point at non-committish

2018-03-25 Thread Yuki Kokubun
References: <1521996898-7052-1-git-send-email-orga.chem@gmail.com>
Content-Type: text/plain

Sorry, I forgot add a line of "Reviewed-by".
I'm gonna send the fixed patch again.


[PATCH v4] filter-branch: fix errors caused by refs that point at non-committish

2018-03-25 Thread Yuki Kokubun
"git filter-branch -- --all" prints error messages when processing refs that
point at objects that are not committish. Such refs can be created by
"git replace" with trees or blobs. And also "git tag" with trees or blobs can
create such refs.

Filter these problematic refs out early, before they are seen by the logic to
see which refs have been modified and which have been left intact (which is
where the unwanted error messages come from), and warn that these refs are left
unwritten while doing so.

Signed-off-by: Yuki Kokubun 
---
 git-filter-branch.sh | 14 --
 t/t7003-filter-branch.sh | 14 ++
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 1b7e4b2cd..41efecb28 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -251,8 +251,18 @@ done < "$tempdir"/backup-refs
 
 # The refs should be updated if their heads were rewritten
 git rev-parse --no-flags --revs-only --symbolic-full-name \
-   --default HEAD "$@" > "$tempdir"/raw-heads || exit
-sed -e '/^^/d' "$tempdir"/raw-heads >"$tempdir"/heads
+   --default HEAD "$@" > "$tempdir"/raw-refs || exit
+while read ref
+do
+   case "$ref" in ^?*) continue ;; esac
+
+   if git rev-parse --verify "$ref"^0 >/dev/null 2>&1
+   then
+   echo "$ref"
+   else
+   warn "WARNING: not rewriting '$ref' (not a committish)"
+   fi
+done >"$tempdir"/heads <"$tempdir"/raw-refs
 
 test -s "$tempdir"/heads ||
die "You must specify a ref to rewrite."
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 7cb60799b..04f79f32b 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -470,4 +470,18 @@ test_expect_success 'tree-filter deals with object name vs 
pathname ambiguity' '
git show HEAD:$ambiguous
 '
 
+test_expect_success 'rewrite repository including refs that point at 
non-commit object' '
+   test_when_finished "git reset --hard original" &&
+   tree=$(git rev-parse HEAD^{tree}) &&
+   test_when_finished "git replace -d $tree" &&
+   echo A >new &&
+   git add new &&
+   new_tree=$(git write-tree) &&
+   git replace $tree $new_tree &&
+   git tag -a -m "tag to a tree" treetag $new_tree &&
+   git reset --hard HEAD &&
+   git filter-branch -f -- --all >filter-output 2>&1 &&
+   ! fgrep fatal filter-output
+'
+
 test_done
-- 
2.16.2.18.g09cb46d