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 [email protected] # some note for the first change
change2 2021-04-01T12:19:21Z UserName [email protected] # some other note for the second change
change3 2021-04-01T12:20:21Z UserName [email protected] # just another note for the third change
@v0.1.0 2021-04-16T21:55:56Z UserName [email protected] # tagged this as v0.1.0
change2 [[email protected]] 2021-04-19T06:00:32Z UserName [email protected] # reworked something to change 2 as it was tagged in v0.1.0
change4 2021-04-20T14:48:05Z UserName [email protected] # 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/[email protected], not deploy/change2.sql. Have you made a change to deploy/[email protected]? Because all rework does is, effectively:

cp deploy/change2.sql deploy/[email protected]

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

[theory]

Closing due to inactivity.