Re: [PATCH v2] filter-branch: use printf instead of echo -e
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&m=152111905428554&w=2 (so that the script could run without problems if there's nothing to do) and https://marc.info/?l=git&m=152147095416175&w=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
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&m=152147479517707&w=2 Junio already queued that PATCH-v2. -- Michele
[PATCH v2] filter-branch: use printf instead of echo -e
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 --- 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
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 --- 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
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 --- 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
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
2018-03-15 15:12 GMT+01:00 Jeff King : > 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
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 --- 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?
2018-03-09 14:23 GMT+01:00 Ian Campbell : > 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?
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?
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.