Sqitch revert fails to revert to correct change when using tags

[cspann]

I have one tag version1 and reworked a change x after it, as rework is only possible when having tags. Later I added another change y which I wanted to revert and modify whilst testing.

But when I issue sqitch revert x, sqitch reverts to the change prior to version1which i consider to be a major critical bug when it would occur in my live db and not in testing. Am I missing something or needs this to be fixed? I tested it in the latest public version 0.9995.

Best regards,
Christian

[theory]

Please paste the contents of your sqitch.plan file.

[cspann]

Hi,

Thanks for the fast reply. I will try to reproduce the Problem with a minimal example as I cannot paste the original file here.

[cspann]

Hello again,

here is what I did:

idmdev@vagrant-ubuntu-trusty-64:~$ cd sqitchbug/
idmdev@vagrant-ubuntu-trusty-64:~/sqitchbug$ ll
insgesamt 16
drwxrwxr-x  2 idmdev idmdev 4096 Aug 19 15:40 ./
drwxr-xr-x 35 idmdev idmdev 4096 Aug 19 15:40 ../
-rw-rw-r--  1 idmdev idmdev  426 Aug 19 15:40 sqitch.conf
-rw-rw-r--  1 idmdev idmdev   42 Aug 19 15:38 sqitch.plan
idmdev@vagrant-ubuntu-trusty-64:~/sqitchbug$ sqitch add A
Created deploy/A.sql
Created revert/A.sql
Created verify/A.sql
Added "A" to sqitch.plan
idmdev@vagrant-ubuntu-trusty-64:~/sqitchbug$ sqitch tag 1.0
Tagged "A" with @1.0 in sqitch.plan
idmdev@vagrant-ubuntu-trusty-64:~/sqitchbug$ sqitch rework A
Added "A [A@1.0]" to sqitch.plan.
Modify these files as appropriate:
  * deploy/A.sql
  * revert/A.sql
  * verify/A.sql
idmdev@vagrant-ubuntu-trusty-64:~/sqitchbug$ sqitch deploy
Adding registry tables to sqitchbug
Deploying changes to sqitchbug
  + A @1.0 .. ok
  + A ....... ERROR:  duplicate key value violates unique constraint "changes_project_script_hash_key"
DETAIL:  Key (project, script_hash)=(sqitchbug, 0376fb90ee0ee8c45a6412db3c91fe7b3dc049dc) already exists.
not ok
Reverting all changes
  - A @1.0 .. ok
Deploy failed
idmdev@vagrant-ubuntu-trusty-64:~/sqitchbug$ sqitch deploy
Deploying changes to sqitchbug
  + A @1.0 .. ok
  + A ....... ok
idmdev@vagrant-ubuntu-trusty-64:~/sqitchbug$ sqitch add B
Created deploy/B.sql
Created revert/B.sql
Created verify/B.sql
Added "B" to sqitch.plan
idmdev@vagrant-ubuntu-trusty-64:~/sqitchbug$ sqitch revert A
Revert changes to A @1.0 from sqitchbug? [Yes] 
  - A .. ok
idmdev@vagrant-ubuntu-trusty-64:~/sqitchbug$ sqitch status
# On database sqitchbug
# Project:  sqitchbug
# Change:   46218a8e416f8ec4b802f2685aaf8402350df3c3
# Name:     A
# Tag:      @1.0
# Deployed: 2016-08-19 15:43:28 +0200
# By:       Idm Entwickler,,, <idmdev@vagrant-ubuntu-trusty-64>
# 
Undeployed changes:
  * A
  * B

It even says now, that it wants to revert to A@1.0 which is wrong in my opinion, isn't it?

I uploaded the resulting sqitch.plan: sqitch.plan.txt

By the way, the hashing algorithm failed when trying to add a non modified reworked change which is identical to the original one - is that intended?

[theory]

Yep, that's a bug, thank you.

By the way, the hashing algorithm failed when trying to add a non modified reworked change which is identical to the original one - is that intended?

Yes. As part of this plan, the hash on the deploy script must be unique so that Sqitch can use it to do deployment merging.

[theory]

Okay, I have a change all ready to commit that fixes this, but then I'm left to wonder. Shouldn't it be that if I say sqitch revert A, it should revert all instances of A, not just the most recent? If you want the most recent, you can say sqitch revert A@HEAD.

[theory]

I've pushed the change to make it work as described in this ticket in 806d633, but I've also posted a query to the mail list soliciting feedback on this change. Actually curious what most people would expect? I'm leaning towards reverting, personally, because if you really mean the most recent version, you can use the @HEAD syntax, but with this change, if you really mean the earliest version, you have to figure out what tag is associated with it.

In any event, one way or another, this ticket is resolved. I'll update with the consensus when it's reached.