Potential bug - Reverting based on modifications to earlier scripts in the plans does not take into account tags

[dlawrences]

Say we have the following plan:

change1 2021-04-01T12:18:21Z UserName user@domain.com # some note for the first change
change2 2021-04-01T12:19:21Z UserName user@domain.com # some other note for the second change
change3 2021-04-01T12:20:21Z UserName user@domain.com # just another note for the third change
@v0.1.0 2021-04-16T21:55:56Z UserName user@domain.com # tagged this as v0.1.0
change2 [change1@v0.1.0] 2021-04-19T06:00:32Z UserName user@domain.com # reworked something to change 2 as it was tagged in v0.1.0
change4 2021-04-20T14:48:05Z UserName user@domain.com # some note for the fourth change

Running the following:

sqitch revert --target <some target> --modified

Will revert the target to change1, prior to v0.1.0, because it detects changes in the deployment scripts of change2, even though the changes have been done through sqitch rework after a clear tag.

Thoughts?

[theory]

Yes, this is a known issue, though I can't find another instance of it right now, so let's keep this one. When I first wrote the plan parser and searcher, rework did not yet exist and I was thinking linearly, thinking “you must mean the earliest instance if you don't specify a tag”. I think that may be a mistake, and it should be “the latest instance”. But in the meantime, you can always append @HEAD to the change to revert to.

Er, except you're using --modified. That makes it even worse. :-( Now there are two bugs. Will have to make some time this summer to fix these and other issues.

[theory]

Okay, I think I see the issue with --modified at line 1163 of Engine.pm:

sqitch/lib/App/Sqitch/Engine.pm

Lines 1160 to 1164 in f22c32f

	foreach my $change (@deployed_changes) {
	$i++;
	return $i if $i + $from_idx >= $plan->count
	|| $change->script_hash ne $plan->change_at($i + $from_idx)->script_hash;
	}

As you said, it's detecting that the deploy script has changed. However, it should not be changed! For the first instance of change2 in the list of deployed changes, $change->script_hash should return the hash for deploy/change2@v0.1.0.sql, not deploy/change2.sql. Have you made a change to deploy/change2@v0.1.0.sql? Because all rework does is, effectively:

cp deploy/change2.sql deploy/change2@v0.1.0.sql

So its hash should not change at all unless you edited that file. Did you?