Bug 39114

Summary: Auto-rebase script does not deal correctly with deleted files
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: Pushed to main --- QA Contact: Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski>
Severity: normal    
Priority: P5 - low CC: baptiste.wojtkowski, nick
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
25.05.00
Circulation function:
Bug Depends on: 38664    
Bug Blocks:    
Attachments: Bug 39114: The auto-rebase script - Fix files removal
Bug 39114: The auto-rebase script - Fix files removal
Bug 39114: The auto-rebase script - Fix files removal
Bug 39114: The auto-rebase script - Fix files removal
Bug 39114: The auto-rebase script - Fix files removal
Bug 39114: The auto-rebase script - Fix files removal

Description Jonathan Druart 2025-02-12 19:10:18 UTC
It's using `$git->rm($file)` but this does not work.
Comment 1 Jonathan Druart 2025-02-12 19:13:39 UTC
Created attachment 177970 [details] [review]
Bug 39114: The auto-rebase script - Fix files removal

It's using `$git->rm($file)` but this does not work.

Test plan:
git checkout HEAD~100 (so you are before 38664)
git rm mainpage.pl
edit C4/Auth.pm and make some changes
git commit -a --no-verify -m"Bug 12345: test"
then rebase your branch
Retrieve the auto_rebase.pl version from this patch and run it
perl auto_rebase.pl

The resulted commit should not contain mainpage.pl
Comment 2 Matt Blenkinsop 2025-02-13 09:12:56 UTC
Created attachment 177986 [details] [review]
Bug 39114: The auto-rebase script - Fix files removal

It's using `$git->rm($file)` but this does not work.

Test plan:
git checkout HEAD~100 (so you are before 38664)
git rm mainpage.pl
edit C4/Auth.pm and make some changes
git commit -a --no-verify -m"Bug 12345: test"
then rebase your branch
Retrieve the auto_rebase.pl version from this patch and run it
perl auto_rebase.pl

The resulted commit should not contain mainpage.pl

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 3 Jonathan Druart 2025-02-13 09:37:27 UTC
Created attachment 177988 [details] [review]
Bug 39114: The auto-rebase script - Fix files removal

Wrong handling of the return of the try-catch, $git->rm has an output
("rm /file/path").

This patch retrieves the status from the `git diff-tree` output

Test plan:
git checkout HEAD~100 (so you are before 38664)
git rm mainpage.pl
edit C4/Auth.pm and make some changes
git commit -a --no-verify -m"Bug 12345: test"
then rebase your branch
Retrieve the auto_rebase.pl version from this patch and run it
perl auto_rebase.pl

The resulted commit should not contain mainpage.pl

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 4 Katrin Fischer 2025-02-13 10:22:19 UTC
Thanks Matt!
Comment 5 Jonathan Druart 2025-02-13 11:20:46 UTC
Created attachment 177989 [details] [review]
Bug 39114: The auto-rebase script - Fix files removal

Wrong handling of the return of the try-catch, $git->rm has an output
("rm /file/path").

This patch retrieves the status from the `git diff-tree` output

Test plan:
git checkout HEAD~100 (so you are before 38664)
git rm mainpage.pl
edit C4/Auth.pm and make some changes
git commit -a --no-verify -m"Bug 12345: test"
then rebase your branch
Retrieve the auto_rebase.pl version from this patch and run it
perl auto_rebase.pl

The resulted commit should not contain mainpage.pl
Comment 6 Nick Clemens (kidclamp) 2025-02-13 14:15:28 UTC
Created attachment 178012 [details] [review]
Bug 39114: The auto-rebase script - Fix files removal

Wrong handling of the return of the try-catch, $git->rm has an output
("rm /file/path").

This patch retrieves the status from the `git diff-tree` output

Test plan:
git checkout HEAD~100 (so you are before 38664)
git rm mainpage.pl
edit C4/Auth.pm and make some changes
git commit -a --no-verify -m"Bug 12345: test"
then rebase your branch
Retrieve the auto_rebase.pl version from this patch and run it
perl auto_rebase.pl

The resulted commit should not contain mainpage.pl

WNC amended patch, tidied, ironically

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 7 Nick Clemens (kidclamp) 2025-02-13 14:16:01 UTC
Not sure why Matt's sign off got lost, but feel free to move to PQA and add it back
Comment 8 Jonathan Druart 2025-02-13 14:19:26 UTC
(In reply to Nick Clemens (kidclamp) from comment #7)
> Not sure why Matt's sign off got lost, but feel free to move to PQA and add
> it back

He tested a previous version of the patch.
Comment 9 Baptiste Wojtkowski (bwoj) 2025-02-13 16:04:58 UTC
Created attachment 178035 [details] [review]
Bug 39114: The auto-rebase script - Fix files removal

Wrong handling of the return of the try-catch, $git->rm has an output
("rm /file/path").

This patch retrieves the status from the `git diff-tree` output

Test plan:
git checkout HEAD~100 (so you are before 38664)
git rm mainpage.pl
edit C4/Auth.pm and make some changes
git commit -a --no-verify -m"Bug 12345: test"
then rebase your branch
Retrieve the auto_rebase.pl version from this patch and run it
perl auto_rebase.pl

The resulted commit should not contain mainpage.pl

WNC amended patch, tidied, ironically

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
Comment 10 Katrin Fischer 2025-02-13 16:15:26 UTC
Pushed for 25.05!

Well done everyone, thank you!