Re: [PATCH v2] filter-branch: use printf instead of echo -e

[Michele Locati]

Il 21/03/2018 22:50, Johannes Schindelin ha scritto:

Hi Michele,

On Mon, 19 Mar 2018, Michele Locati wrote:


[...]
--
2.16.2.windows.1


Yay!

Out of curiosity: did the CONTRIBUTING.md file help that was recently
turned into a guide how to contribute to Git (for Windows) by Derrick
Stolee?


Well, no. Here's what led me here...

The people behind the concrete5 CMS asked support for improving the 
following procedure:


concrete5 is stored in
https://github.com/concrete5/concrete5
In order to make it installable with Composer (a PHP package manager), 
we need to extract its "/concrete" directory, and push the results to 
https://github.com/concrete5/concrete5-core

We previously used "git filter-branch --all" with this script:
https://github.com/concrete5/core_splitter/blob/70879e676b95160f7fc5d0ffc22b8f7420b0580b/bin/splitcore

That script is executed every time someone pushes concrete5/concrete5, 
and it took very long time (3~5 minutes).


I noticed on the git-filter-branch manual page that there's a 
"--state-branch", and it seemed quite interesting.
So, I asked in git@vger.kernel.org how to use it, and the author of that 
feature (Ian Campbell) told me to look at some code he wrote.

So, I wrote
https://github.com/concrete5/incremental-filter-branch
which wraps "git filter-branch --filter-state", and it's used in
https://github.com/concrete5/core_splitter/blob/99bbbcea1ab90572a04864ccb92327532ab6a42c/bin/splitcore
(it not yet in production: Korvin wants to be sure everything works well 
before adopting it)
The time required for the operation dropped from 3~5 minutes to ~10 
seconds ;)


While writing that script, I noticed a couple of things that could be 
improved in "git-filter-branch", so I submitted

https://marc.info/?l=git=152111905428554=2
(so that the script could run without problems if there's nothing to do)
and
https://marc.info/?l=git=152147095416175=2
(so that  --filter-state could be used on Mac and other non-Linux systems).
How did I learn how to submit to git? I searched for "git src", landed 
on https://github.com/git/git, read Documentation/SubmittingPatches and 
used git send-email.


And yes, all that on Windows (and WSL).




Ciao,
Johannes


Ciao!

--
Michele

Re: [PATCH] filter-branch: use printf instead of echo -e

[Michele Locati]

Il 20/03/2018 10:53, Ian Campbell ha scritto:

On Tue, 2018-03-20 at 00:22 -0400, Jeff King wrote:

Author cc'd in case there's something more interesting going on.


That code was written years ago, if I had a good reason at the time
I've forgotten what it was and I can't think of a fresh one now.
Switching to printf seems like a reasonable thing to do.

Perhaps switch the remaining `/bin/echo` (there are two without `-e`)
uses to just `echo` for consistency with the rest of the file?

Ian.



Thanks for reply, Ian.
Charles already suggested to replace /bin/echo with echo, and I already 
updated the patch accordingly: see

https://marc.info/?l=git=152147479517707=2
Junio already queued that PATCH-v2.

--
Michele

[PATCH v2] filter-branch: use printf instead of echo -e

[Michele Locati]

In order to echo a tab character, it's better to use printf instead of
"echo -e", because it's more portable (for instance, "echo -e" doesn't work
as expected on a Mac).

This solves the "fatal: Not a valid object name" error in git-filter-branch
when using the --state-branch option.

Furthermore, let's switch from "/bin/echo" to just "echo", so that the
built-in echo command is used where available.

Signed-off-by: Michele Locati <mich...@locati.it>
---
 git-filter-branch.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 1b7e4b2cd..98c76ec58 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -627,12 +627,12 @@ then
print H "$_:$f\n" or die;
}
close(H) or die;' || die "Unable to save state")
-   state_tree=$(/bin/echo -e "100644 blob $state_blob\tfilter.map" | git 
mktree)
+   state_tree=$(printf '100644 blob %s\tfilter.map\n' "$state_blob" | git 
mktree)
if test -n "$state_commit"
then
-   state_commit=$(/bin/echo "Sync" | git commit-tree "$state_tree" 
-p "$state_commit")
+   state_commit=$(echo "Sync" | git commit-tree "$state_tree" -p 
"$state_commit")
else
-   state_commit=$(/bin/echo "Sync" | git commit-tree "$state_tree" 
)
+   state_commit=$(echo "Sync" | git commit-tree "$state_tree" )
fi
git update-ref "$state_branch" "$state_commit"
 fi
-- 
2.16.2.windows.1

[PATCH] filter-branch: use printf instead of echo -e

[Michele Locati]

In order to echo a tab character, it's better to use printf instead of
"echo -e", because it's more portable (for instance, "echo -e" doesn't work
as expected on a Mac).

This solves the "fatal: Not a valid object name" error in git-filter-branch
when using the --state-branch option.

Signed-off-by: Michele Locati <mich...@locati.it>
---
 git-filter-branch.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 1b7e4b2cd..21d84eff3 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -627,7 +627,7 @@ then
print H "$_:$f\n" or die;
}
close(H) or die;' || die "Unable to save state")
-   state_tree=$(/bin/echo -e "100644 blob $state_blob\tfilter.map" | git 
mktree)
+   state_tree=$(printf '100644 blob %s\tfilter.map\n' "$state_blob" | git 
mktree)
if test -n "$state_commit"
then
state_commit=$(/bin/echo "Sync" | git commit-tree "$state_tree" 
-p "$state_commit")
-- 
2.16.2.windows.1

[PATCH v2] filter-branch: return 2 when nothing to rewrite

[Michele Locati]

Using the --state-branch option allows us to perform incremental filtering.
This may lead to having nothing to rewrite in subsequent filtering, so we need
a way to recognize this case.
So, let's exit with 2 instead of 1 when this "error" occurs.

Signed-off-by: Michele Locati <mich...@locati.it>
---
 Documentation/git-filter-branch.txt | 8 
 git-filter-branch.sh| 2 +-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-filter-branch.txt 
b/Documentation/git-filter-branch.txt
index 3a52e4dce..b63404318 100644
--- a/Documentation/git-filter-branch.txt
+++ b/Documentation/git-filter-branch.txt
@@ -222,6 +222,14 @@ this purpose, they are instead rewritten to point at the 
nearest ancestor that
 was not excluded.
 
 
+EXIT STATUS
+---
+
+On success, the exit status is `0`.  If the filter can't find any commits to
+rewrite, the exit status is `2`.  On any other error, the exit status may be
+any other non-zero value.
+
+
 Examples
 
 
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 1b7e4b2cd..c285fdb90 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -310,7 +310,7 @@ git rev-list --reverse --topo-order --default HEAD \
die "Could not get the commits"
 commits=$(wc -l <../revs | tr -d " ")
 
-test $commits -eq 0 && die "Found nothing to rewrite"
+test $commits -eq 0 && die_with_status 2 "Found nothing to rewrite"
 
 # Rewrite the commits
 report_progress ()
-- 
2.16.2.windows.1

Re: [PATCH] filter-branch: return 2 when nothing to rewrite

[Michele Locati]

2018-03-15 16:55 GMT+01:00 Junio C Hamano :
> Jeff King  writes:
>
>> Hrm. I took the goal to mean that we used to exit with a failing "1" in
>> this case, and now we would switch to a more-specific "2". And I think
>> that matches the behavior of the patch:
>>
>> -test $commits -eq 0 && die "Found nothing to rewrite"
>> +test $commits -eq 0 && die_with_status 2 "Found nothing to rewrite"
>>
>> Am I missing something?
>
> No, other than that I wrote my response before sufficiently
> caffeinated ;-)
>
> Thanks, then other than the lack of doc updates, I do not see an
> issue.


Great! So, I'm ready to update the patch, including the doc changes,
which will be
the one suggested by Jeff:


EXIT STATUS
---

On success, the exit status is `0`.  If the filter can't find any commits to
rewrite, the exit status is `2`.  On any other error, the exit status may be
any other non-zero value.


And yes, I'm a brand new contributor, so here's my question: how should I
send an updated patch? I can't find anything related to this in
https://github.com/git/git/blob/master/Documentation/SubmittingPatches

PS: nice community!

--
Michele

Re: [PATCH] filter-branch: return 2 when nothing to rewrite

[Michele Locati]

2018-03-15 15:12 GMT+01:00 Jeff King <p...@peff.net>:
> On Thu, Mar 15, 2018 at 02:03:59PM +0100, Michele Locati wrote:
>
>> Using the --state-branch option allows us to perform incremental filtering.
>> This may lead to having nothing to rewrite in subsequent filtering, so we 
>> need
>> a way to recognize this case.
>> So, let's exit with 2 instead of 1 when this "error" occurs.
>
> That sounds like a good feature. It doesn't look like we use "2" for
> anything else currently.
>
>> ---
>>  git-filter-branch.sh | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> This should probably get a mention in the manpage at
> Documentation/git-filter-branch.txt, too.


Yes, I agree it would be useful. What about this addition right after the
"Remap to ancestor" section?

EXIT CODE
-

In general, this command will fail with an exit status of `1` in case of errors.
When the filter can't fine anything to rewrite, the exit status is `2`.


--
Michele

[PATCH] filter-branch: return 2 when nothing to rewrite

[Michele Locati]

Using the --state-branch option allows us to perform incremental filtering.
This may lead to having nothing to rewrite in subsequent filtering, so we need
a way to recognize this case.
So, let's exit with 2 instead of 1 when this "error" occurs.

Signed-off-by: Michele Locati <mich...@locati.it>
---
 git-filter-branch.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 1b7e4b2cd..c285fdb90 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -310,7 +310,7 @@ git rev-list --reverse --topo-order --default HEAD \
die "Could not get the commits"
 commits=$(wc -l <../revs | tr -d " ")
 
-test $commits -eq 0 && die "Found nothing to rewrite"
+test $commits -eq 0 && die_with_status 2 "Found nothing to rewrite"
 
 # Rewrite the commits
 report_progress ()
-- 
2.16.2.windows.1

Re: How to use filter-branch with --state-branch?

[Michele Locati]

2018-03-09 14:23 GMT+01:00 Ian Campbell <i...@hellion.org.uk>:
> On Fri, 2018-03-09 at 14:04 +0100, Michele Locati wrote:
>> Just a couple of questions:
>>
>> 1. it seems to me it's not possible to process all the branches in one
>> go. Am I right?
>
> I'm not sure, I've never done such a thing, in fact I didn't know you
> could.
>
> Really all this feature does is record the `.git/rewrite-map` (or
> whatever the correct name is) at the end of the rewrite and reinstate
> it again the next time, so it shouldn't really interact with many of
> the other options.
>
> My method for storeing "last version processed" in a branch does
> conflict I suppose (since that branch would be rewritten) but that's an
> artefact of the surrounding scaffolding -- you could equally well keep
> the record in some file on the local system or in a non-branch-ish ref
> (I guess).
>
>> 2. Why do you have this line in filter.sh?
>> `rm -f .git/refs/original/refs/heads/${UPSTREAM_REWRITTEN}`
>
> TBH I'm not really sure. I originally wrote this patch many years ago,
> it's just recently that I got around to upstreaming, so my memory is
> more fuzzy than might be expected.
>
> I think perhaps I was trying to avoid this error:
>
> A previous backup already exists in $orig_namespace
> Force overwriting the backup with -f"
>
> which comes if there is an existing backup (a safety feature in the
> non-incremental case).
>
> Note quite sure why I didn't use `-f` as the message says, but I guess
> because it forces other things too which I didn't want to do?
>
> Perhaps what I should have done is make that check conditional on the
> use of --state-branch.
>
> I wonder if you could use the `original/refs/...` as the "last version
> processed"? Would be a lot less manual messing around than what I do!
>
> Ian.


I managed to get a general script that seems to work: see
https://github.com/mlocati/incremental-git-filter-branch

Thanks again, Ian.

--
Michele

Re: How to use filter-branch with --state-branch?

[Michele Locati]

2018-03-08 10:40 GMT+01:00 Ian Campbell :
>
> On Thu, 2018-03-08 at 10:25 +0100, Ævar Arnfjörð Bjarmason wrote:
>
> > > The first filter-branch call required 7168 steps, so did the second 
> > > call...
> > > I also tried without the --prune option of remote update (I had to add
> > > --force to the second filter-branch), but nothing changed.
>
> You can see an example of the usage in:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git/
>
> in the `scripts/` sub dir (flow is `cronjob` → `filter.sh` → `git
> filter-branch...`.
>
> I think the big difference is rather than `--all` you need to give it
> the `previous..now` range since that is the update you wish to do
> (first time around you just give it `now`).
>
> The devicetree-rebasing scripting arranges that by keeping the previous
> in a separate branch.
>
> Ian.

Thank you for your quick reply, Ian.

Just a couple of questions:

1. it seems to me it's not possible to process all the branches in one
go. Am I right?

2. Why do you have this line in filter.sh?
`rm -f .git/refs/original/refs/heads/${UPSTREAM_REWRITTEN}`

Thank you again,
Michele

How to use filter-branch with --state-branch?

[Michele Locati]

Recent versions of git filter-branch command introduced the --state-branch
option.
BTW I can't find any info about how this can be actually used.

We have this repository on github:
https://github.com/concrete5/concrete5

When someone pushes to that repo, we clone it and execute
`git filter-branch --subdirectory-filter concrete`
to extract the concrete directory, and we push the result to
https://github.com/concrete5/concrete5-core
(including all the branches and tags)

The script at the moment is this one:
https://github.com/concrete5/core_splitter/blob/70879e676b95160f7fc5d0ffc22b8f7420b0580b/bin/splitcore

I tried to use the --state-branch option on a local mirror, so that we could
do an incremental filtering. Here's the script:

# Executed just one time
git clone --no-checkout --mirror \
   https://github.com/concrete5/concrete5.git work
cd work
git filter-branch \
   --subdirectory-filter concrete \
   --tag-name-filter cat \
   --prune-empty \
   --state-branch FILTERBRANCH_STATE \
   -- --all
# Executed every time the repo is updated
git remote update --prune
git filter-branch \
   --subdirectory-filter concrete \
   --tag-name-filter cat \
   --prune-empty \
   --state-branch FILTERBRANCH_STATE \
   -- --all

The first filter-branch call required 7168 steps, so did the second call...
I also tried without the --prune option of remote update (I had to add
--force to the second filter-branch), but nothing changed.