Tool is completely broken & useless or just a really bad documentation?

[isbm]

Version: 2.47.0 (pip install into venv)
OS: Ubuntu 22.04.5 LTS
Python 3.10.12
git version 2.34.1

file: callback.py

def commit_callback(c):
    with open("bananas.txt", "a") as f:
        f.write("BANANA!\n")

1. git clone --mirror ...
2. git filter-repo --commit-callback ./callback.py --force

Output:

Parsed 837 commits
New history written in 0.01 seconds; now repacking/cleaning...
Repacking your repo and cleaning out old unneeded objects

[...]

Completely finished after 0.06 seconds.

But no "bananas.txt" at all. If I just do this:

def commit_callback(c):
    raise "AAAA!"

Still the same "clean" output: parsed NNN commits etc. OK, let's do exactly as documentation says:

git filter-repo --commit-callback 'raise Exception("banana")'                                                  

Traceback (most recent call last):

[...]

    self._commit_callback(commit, self.callback_metadata(aux_info))
  File "<string>", line 2, in callback
Exception: banana

OK, we're getting somewhere. How about this:

git filter-repo --commit-callback 'def commit_callback(c): open("bananas.txt", "a").write("BANANA!\n")' --force

Parsed 837 commits
New history written in 0.01 seconds; now repacking/cleaning...
Repacking your repo and cleaning out old unneeded objects

[...]

Completely finished after 0.06 seconds.

OK, I give up, this just makes zero sense to me.

• What I am doing wrong?
• Why this tool crashes/ignores silently and doesn't say anything with regard of given python script?
• How to define commit_callback so I can change the commit data (can't find doc, BTW)?

[isbm]

Update: OK, bad documentation.

It should be better highlighted, that each Python script receives a magic variable ("commit" and "_do_not_use_this_var" in my case) directly in the top level builtins (and bunch of others). The _do_not_use_this_var is basically a state and graph, also bunch of internal functions.

Sure, if I do:

def whatever(c):
    print("Banana!", c.author_name)

whatever(commit)

...then it works. But I wish documentation was better. 😢

[flypenguin]

well, completely useless was a bit harsh. I get it, I am also sometimes really angry at things, in this case – I have used filter-repo often, and without any problems. This is a really good tool, thanks to the author at this point – fyi @newren.

Praise should be given once in a while :) . Having said that, for me --subdirectory-filter just crashes, so I'm hunting for solutions as well ATM 😆

[isbm]

well, completely useless was a bit harsh.

Yeah. I just spent an hour to figuring out that this thing simply runs script at module level. 😆

Praise should be given once in a while :) . Having said that, for me --subdirectory-filter just crashes, so I'm hunting for solutions as well ATM 😆

Fair enough. But I would still make it a bit ID10t-friendly (I am one, in this case). It would be much better if one is using a path to a .py script, it would be awesome to add a "magic" main() function that takes these magic params (like context etc) and say:

"Dear user, you can define def main(**args): and do whatever inside of it, it will be auto-launched"

That would be "pythonic" way: a luser (like me) would simply dir(args) and my "Aha!" would be happening much sooner. 😆

[flypenguin]

@isbm ... uvx run git-filter-repo [...], or maybe brew install git-filter-repo ; git-filter-repo [...] ... that works just fine for me 😬 😉

[newren]

def commit_callback(c):
with open("bananas.txt", "a") as f:
f.write("BANANA!\n")

That'd result in the following code being compiled and called:

def commit_callback(commit):
    def commit_callback(c):
        with open("bananas.txt", "a") as f:
            f.write("BANANA!\n")

Basically, you have a function which defines an inner function (which shadows the outer function name), but never calls the inner function, so it does nothing.

Update: OK, bad documentation.

It should be better highlighted, that each Python script receives a magic variable ("commit" and "_do_not_use_this_var" in my case) directly in the top level builtins (and bunch of others). The _do_not_use_this_var is basically a state and graph, also bunch of internal functions.

So, would you want the documentation to say something like

    Most callback functions are of the same general format.  For a command line
    argument like
      --foo-callback 'BODY'

    the following code will be compiled and called:
      def foo_callback(foo):
        BODY

and then provide an example like

git filter-repo --commit-callback '
  # Remove executable files with three 6s in their name (including
  # from leading directories).
  # Also, undo deletion of sources/foo/bar.txt (change types are
  # either b"D" (deletion) or b"M" (add or modify); renames are
  # handled by deleting the old file and adding a new one)
  commit.file_changes = [
         change for change in commit.file_changes
         if not (change.mode == b"100755" and
                 change.filename.count(b"6") == 3) and
            not (change.type == b"D" and
                 change.filename == b"sources/foo/bar.txt")]
  # Mark all .sh files as executable; modes in git are always one of
  # 100644 (normal file), 100755 (executable), 120000 (symlink), or
  # 160000 (submodule)
  for change in commit.file_changes:
    if change.filename.endswith(b".sh"):
      change.mode = b"100755"
  '

from which you can see that the callback doesn't do its own function definition but only provides the body of the function?

Sure, if I do:

def whatever(c):
print("Banana!", c.author_name)

whatever(commit)
...then it works. But I wish documentation was better. 😢

That would result in code like the following:

     def commit_callback(commit):
        def whatever(c):
            print("Banana!", c.author_name)

        whatever(commit)

which works, I guess, but feels really weird to have a nested function like this. Why not just:

print("Banana!", commit.author_name)

for your callback, which would get translated into

    def commit_callback(commit):
        print("Banana!", commit.author_name)

?