no longer ignores absent revert scripts with log-only

[DimitriyKh]

Previous versions of sqitch ignore absent revert scripts when revert --log-only was issued. Now it's failing (see logs below). This behaviour was usefull when on development changes are reconsidered or moved between scripts or/and branches.

Now I need to either touch revert/script.sql and them remove them or dance with checkout between branches to run verify-revert-deploy or rebase commands. I think since log-only not suppose to actually run script, it's ok to ignore missing files.

sqitch -t wf1 revert -y --log-only --to-change TK68448 -vvv
    DBI 1.647-ithread default trace level set to 0x0/1 (pid 24643 pi 12f808800) at DBIEngine.pm line 39 via (eval 362) line 16
    -> DBI->connect(dbi:MariaDB:host=127.0.0.1;port=3306;database=wf1_sqitch, wf1, ****, HASH(0x128c45de8))
    -> DBI->install_driver(MariaDB) for darwin perl=5.040001 pid=24643 ruid=501 euid=501
       install_driver: DBD::MariaDB version 1.23 loaded from /opt/homebrew/Cellar/sqitch/1.5.0/libexec/../lib/perl5/darwin-thread-multi-2level/DBD/MariaDB.pm
    <- install_driver= DBI::dr=HASH(0x128c3ba78)
    !! The warn '0' was CLEARED by call to connect method
    <- connect('host=127.0.0.1;port=3306;database=wf1_sqitch', 'wf1', ...)= ( DBI::db=HASH(0x128c3b028) ) [1 items] at DBI.pm line 683
    <- STORE('RaiseError', 0)= ( 1 ) [1 items] at DBI.pm line 735
    <- STORE('PrintError', 0)= ( 1 ) [1 items] at DBI.pm line 735
    <- STORE('AutoCommit', 1)= ( 1 ) [1 items] at DBI.pm line 735
    <- STORE('Callbacks', HASH(0x128c46070))= ( 1 ) [1 items] at DBI.pm line 738
    <- STORE('HandleError', CODE(0x128b6f008))= ( 1 ) [1 items] at DBI.pm line 738
    <- STORE('Username', 'wf1')= ( 1 ) [1 items] at DBI.pm line 738
    {{ connected callback CODE(0x128874ae0) being invoked with 5 args
    }} connected callback CODE(0x128874ae0) returned
    <- connected('dbi:MariaDB:host=127.0.0.1;port=3306;database=wf1_sqitch', 'wf1', ...)= ( ) [0 items] at DBI.pm line 745
    <- connect= DBI::db=HASH(0x128c3b028)
    <- STORE('dbi_connect_closure', CODE(0x128c3b790))= ( 1 ) [1 items] at DBI.pm line 754
    <- FETCH('mariadb_serverinfo')= ( '8.0.40' ) [1 items] at mysql.pm line 109
    <- FETCH('mariadb_serverversion')= ( 80040 ) [1 items] at mysql.pm line 117
    <- DESTROY(DBI::st=HASH(0x128942628))= ( ) [0 items] at DBI.pm line 1705
    <- selectcol_arrayref('SELECT CAST(ROUND(MAX(version), 1) AS CHAR) FROM releases')= ( [ '1.1' ] ) [1 items] at DBIEngine.pm line 96
    <- DESTROY(DBI::st=HASH(0x128c373f8))= ( ) [0 items] at DBI.pm line 1705
    <- selectcol_arrayref('
            SELECT COUNT(*)
            FROM information_schema.tables
            WHERE table_schema = ?
            AND table_name   = ?
        ', undef, ...)= ( [ 1 ] ) [1 items] at mysql.pm line 258
    <- DESTROY(DBI::st=HASH(0x12886c050))= ( ) [0 items] at DBI.pm line 1705
    <- selectcol_arrayref('SELECT get_lock(?, ?)', undef, ...)= ( [ 1 ] ) [1 items] at mysql.pm line 321
    <- DESTROY(DBI::st=HASH(0x128bbb090))= ( ) [0 items] at DBI.pm line 1705
    <- selectcol_arrayref('
            SELECT change_id
              FROM changes
             WHERE project = ?
               AND changes.change  = ?
             ORDER BY changes.committed_at ASC
        ', undef, ...)= ( [ '07b97f8876309caaacf6a9fe3b4500ba2848f190' ] ) [1 items] at DBIEngine.pm line 948
    !! The ERROR '2000' was CLEARED by call to fetch method
    <- DESTROY(DBI::st=HASH(0x1299321b8))= ( ) [0 items] at DBI.pm line 1643
    <- selectrow_hashref('
        SELECT c.change_id AS id, c.change AS name, c.project, c.note,
               date_format(c.planned_at, 'year:%Y:month:%m:day:%d:hour:%H:minute:%i:second:%S:time_zone:UTC') AS "timestamp", c.planner_name, c.planner_email,
                GROUP_CONCAT(t.tag ORDER BY t.tag SEPARATOR ' ') AS tags, c.script_hash
          FROM changes   c
          LEFT JOIN tags t ON c.change_id = t.change_id
         WHERE c.change_id = ?
         GROUP BY c.change_id, c.change, c.project, c.note, c.planned_at,
               c.planner_name, c.planner_email, c.script_hash
    ', undef, ...)= ( HASH(0x129932470)9keys ) [1 items] at DBIEngine.pm line 823
    <- DESTROY(DBI::st=HASH(0x1299321e8))= ( ) [0 items] at DBI.pm line 1676
    <- selectall_arrayref('
        SELECT c.change_id AS id, c.change AS name, c.project, c.note,
               date_format(c.planned_at, 'year:%Y:month:%m:day:%d:hour:%H:minute:%i:second:%S:time_zone:UTC') AS "timestamp", c.planner_name, c.planner_email,
               GROUP_CONCAT(t.tag ORDER BY t.tag SEPARATOR ' ') AS tags, c.script_hash
          FROM changes   c
          LEFT JOIN tags t ON c.change_id = t.change_id
         WHERE c.project = ?
           AND c.committed_at > (SELECT committed_at FROM changes WHERE change_id = ?)
         GROUP BY c.change_id, c.change, c.project, c.note, c.planned_at,
               c.planner_name, c.planner_email, c.committed_at, c.script_hash
         ORDER BY c.committed_at ASC
    ', HASH(0x1299324e8), ...)= ( [ HASH(0x129424d10) ] ) [1 items] at DBIEngine.pm line 805
Reverting changes to TK68448 @10.0.1 from wf1
debug: Will revert the following changes:
debug: IN57523
    <- DESTROY(DBI::st=HASH(0x129424ce0))= ( ) [0 items] at DBI.pm line 1676
    <- selectall_arrayref('
        SELECT c.change_id, c.project, c.change, (
            SELECT tag
              FROM changes c2
              JOIN tags ON c2.change_id = tags.change_id
             WHERE c2.project       = c.project
               AND c2.committed_at >= c.committed_at
             ORDER BY c2.committed_at
             LIMIT 1
        ) AS asof_tag
          FROM dependencies d
          JOIN changes c ON c.change_id = d.change_id
         WHERE d.dependency_id = ?
    ', HASH(0x129424d58), ...)= ( [ ] ) [1 items] at DBIEngine.pm line 641
  - IN57523 ..     <- do("LOCK TABLES releases WRITE, changes WRITE, dependencies WRITE, events WRITE, projects WRITE, tags WRITE")= ( '0E0' ) [1 items] at mysql.pm line 298
    <- begin_work= ( ) [0 items] at mysql.pm line 300
not ok
    <- commit= ( 1 ) [1 items] at mysql.pm line 341
    <- do("UNLOCK TABLES")= ( '0E0' ) [1 items] at mysql.pm line 342
    <- DESTROY(DBI::db=HASH(0x128c3b148))= ( ) [0 items] at Tiny.pm line 97
Revert script revert/IN57523.sql does not exist
trace: Trace begun at /opt/homebrew/Cellar/sqitch/1.5.0/libexec/../lib/perl5/App/Sqitch/Engine.pm line 1093
trace: App::Sqitch::Engine::try {...}  at /opt/homebrew/Cellar/sqitch/1.5.0/libexec/../lib/perl5/Try/Tiny.pm line 102
trace: eval {...} at /opt/homebrew/Cellar/sqitch/1.5.0/libexec/../lib/perl5/Try/Tiny.pm line 93
trace: Try::Tiny::try('CODE(0x129424d58)', 'Try::Tiny::Finally=REF(0x11f871ff0)', 'Try::Tiny::Catch=REF(0x128ca33d8)') called at /opt/homebrew/Cellar/sqitch/1.5.0/libexec/../lib/perl5/App/Sqitch/Engine.pm line 1112
trace: App::Sqitch::Engine::revert_change('App::Sqitch::Engine::mysql=HASH(0x128bbb360)', 'App::Sqitch::Plan::Change=HASH(0x128c58530)') called at /opt/homebrew/Cellar/sqitch/1.5.0/libexec/../lib/perl5/App/Sqitch/Engine.pm line 433
trace: App::Sqitch::Engine::revert('App::Sqitch::Engine::mysql=HASH(0x128bbb360)', 'TK68448', '', 1) called at /opt/homebrew/Cellar/sqitch/1.5.0/libexec/../lib/perl5/App/Sqitch/Command/revert.pm line 172
trace: App::Sqitch::Command::revert::execute(undef) called at /opt/homebrew/Cellar/sqitch/1.5.0/libexec/../lib/perl5/App/Sqitch.pm line 207
trace: App::Sqitch::try {...}  at /opt/homebrew/Cellar/sqitch/1.5.0/libexec/../lib/perl5/Try/Tiny.pm line 100
trace: eval {...} at /opt/homebrew/Cellar/sqitch/1.5.0/libexec/../lib/perl5/Try/Tiny.pm line 93
trace: Try::Tiny::try('CODE(0x11fa33660)', 'Try::Tiny::Catch=REF(0x11fb8b368)') called at /opt/homebrew/Cellar/sqitch/1.5.0/libexec/../lib/perl5/App/Sqitch.pm line 227
trace: App::Sqitch::go('App::Sqitch') called at /opt/homebrew/Cellar/sqitch/1.5.0/libexec/sqitch line 8

    <- disconnect_all= ( 1 ) [1 items] at DBI.pm line 762
!   <- DESTROY(DBI::dr=HASH(0x128c3b9b8))= ( undef ) [1 items] during global destruction

[theory]

This behavior was changed in 7388a13, in response to #828. I could see an argument to add an option to disable it when using --log-only. Is that something you could work on, @DimitriyKh?

[DimitriyKh]

The #828 looks quite reasonable since it needs to calculate hash at least. But for revert with --log-only that's probably overkill, so disabling the check for this case is good option.

[DimitriyKh]

is there something I can do?
so far I resolve to run touch revert/...sql anytime I see an error.

[theory]

Sqitch wants to encourage best practices, including adding revert scripts for every change. But if you wanted to add an option on revert to skip the check, I think that would be fine. Or maybe it shouldn't bother checking on revert with --log-only? That seems okay, too. A patch with something like this:

--- a/lib/App/Sqitch/Engine.pm
+++ b/lib/App/Sqitch/Engine.pm
@@ -1090,11 +1090,13 @@ sub revert_change {
 
     try {
         my $file = $change->revert_file;
-        hurl revert => __x(
-            'Revert script {file} does not exist',
-            file => $file,
-        ) unless -e $file;
-        $self->run_revert($file) unless $self->log_only;
+        unless ($self->log_only) {
+            hurl revert => __x(
+                'Revert script {file} does not exist',
+                file => $file,
+            ) unless -e $file;
+            $self->run_revert($file);
+         }
         try {
             $self->log_revert_change($change);
             $sqitch->info(__ 'ok');