Multiple issues with patching symlinks

[quyse]

The initial issue I've encountered:

Basically the Linux version of the game has the following structure of symlinks:

The newest version:

libSDL2-2.0.so -> libSDL2-2.0.so.0
libSDL2-2.0.so.0 -> libSDL2-2.0.so.0.10.0
libSDL2.so -> libSDL2-2.0.so
libSDL2-2.0.so.0.10.0 - real file

The current version on my PC:

libSDL2-2.0.so -> libSDL2-2.0.so.0
libSDL2-2.0.so.0 -> libSDL2-2.0.so.0.9.0
libSDL2.so -> libSDL2-2.0.so
libSDL2-2.0.so.0.9.0 - real file

(Build process just copies whatever /usr/lib/libSDL2* to the result build, so that's why it looks like that.)

So looks like the error happens because itch can't re-target symlink libSDL2-2.0.so.0 to the new 0.10.0 version of the file.

I tried to reproduce that with test project, and encountered some interesting issues with symlinks.

Issue 1. Changing from symlink to normal file - does not change file type, rewrites wrong file. Version 1: test2.txt is normal file with content A, test.txt is a symlink to test2.txt. Version 2: test2.txt is normal file (with the same content A), test.txt is normal file (with new content B). Updating from version 1 to version 2 results in: test2.txt is normal file with content B (???), test.txt is still a symlink (???) to test2.txt

Issue 2. Multi-level symlinks - silent failures Version 1: has some files. Version 2: has the same files unchanged, plus the following new ones:

aaa.txt -> aa.txt
aa.txt -> a.txt
a.txt - normal file

Updating from version 1 to version 2 results in:

aa.txt -> a.txt
a.txt

There's no aaa.txt at all. I've added some other stuff, but without changing these files, and on updating to new version aaa.txt finally appeared correctly.

I tried it again with added symlink chain in reverse order of names:

bbb.txt - normal file
bb.txt -> bbb.txt
b.txt -> bb.txt

And on update only bbb.txt appeared!

Then I removed the package and re-installed it from scratch, and this time all three b-files were installed correctly.

I wanted to try more stuff, but now I'm receiving errors like: when trying to install the project, did I broke the backend somehow?

Anyhow, looks like handling symlinks is broken enough already.

Everything was done on Linux. itch 25.4.0 butler 15.17.0 itch-setup 1.18.0

[fasterthanlime]

For this part:

I wanted to try more stuff, but now I'm receiving errors like: when trying to install the project, did I broke the backend somehow?

Nothing's broken, the upload was just deleted, so nobody (including you & admins) can install it.

---

For the symlink issues, I think I know what's happening. Symlinks are handled last, so for issue 1, it writes to test.txt which is a symlink to test2.txt, so test2.txt actually gets changed.

For issue 2, I'm not sure exactly. The good news is that all that is pretty easy to codify as unit tests for wharf/butler, so a fix shouldn't be hard at all. The only tricky bit is to exclude them on Windows.

I believe the proper file for this is https://github.com/itchio/wharf/blob/master/pwr/patching_test.go - I really want to say I'll take a look soon but I don't what my availability will be like (even on weekdays).

[quyse]

Nothing's broken, the upload was just deleted, so nobody (including you & admins) can install it.

Interesting, it works now, but it didn't work before. I did delete the upload, but then I created/uploaded it again, and it was displaying on the game's page, but was not possible to install.

For issue 2, I'm not sure exactly.

I guess the issue is that currently symlink doesn't get created if its target is missing, and with symlinks pointing to other symlinks you need to either topologically sort the whole symlink graph to create them in correct order (but that would not allow circular symlinks), or better - create them without checking target's existence (I believe that's what POSIX symlink does).

[fasterthanlime]

Interesting, it works now, but it didn't work before. I did delete the upload, but then I created/uploaded it again, and it was displaying on the game's page, but was not possible to install.

Right, butler caches /uploads responses, maybe the TTL is too long and/or maybe it should retry a fresh fetch if it turns out the upload has been deleted since.

I guess the issue is that currently symlink doesn't get created if its target is missing, and with symlinks pointing to other symlinks you need to either topologically sort the whole symlink graph to create them in correct order (but that would not allow circular symlinks), or better - create them without checking target's existence (I believe that's what POSIX symlink does).

Yep that seems like the most likely explanation but golang's os.Symlink refusing to create dangling symlinks would be sorta surprising to me (although not impossible).

[fasterthanlime]

Update: I am just now getting the opportunity to write test cases for this, I'll keep you posted.

It doesn't fail exactly as I would expect it to so far, maybe the validator itself doesn't check that files are indeed regular files and that's why it mistakenly thinks everything is fine:

(I would expect it to fail before "Healing to (v1)")

[fasterthanlime]

Issue 1. Changing from symlink to normal file - does not change file type, rewrites wrong file. Version 1: test2.txt is normal file with content A, test.txt is a symlink to test2.txt. Version 2: test2.txt is normal file (with the same content A), test.txt is normal file (with new content B). Updating from version 1 to version 2 results in: test2.txt is normal file with content B (???), test.txt is still a symlink (???) to test2.txt

I'm not able to reproduce this one for now, everything seems to work as expected:

 symlinks-test ✖ butler ls v1
v1: directory
Lrwxrwxrwx          - test.txt -> test2.txt
-rw-r--r-- 1024.00 KiB test2.txt
 symlinks-test ツ sha256sum v1/test2.txt
774a6278cfe70740e88337e3503916cb5b827554070da78deff3b78c9cc8d1fa  v1/test2.txt
 symlinks-test ツ butler ls v2
v2: directory
-rw-r--r--   2.00 MiB test.txt
-rw-r--r-- 1024.00 KiB test2.txt
 symlinks-test ツ sha256sum v2/*.txt
774a6278cfe70740e88337e3503916cb5b827554070da78deff3b78c9cc8d1fa  v2/test2.txt
cdd3adcb76b92c91a59291bd7f466c2338a651144d5d8163d363ebd7616159e7  v2/test.txt
 symlinks-test ツ butler diff v1 v2
• Hashing v1
✓ 1024.00 KiB (1 files, 0 dirs, 1 symlinks) @ 417.49 MiB/s                      

• Diffing v2
✓ 3.00 MiB (2 files, 0 dirs, 0 symlinks) @ 71.09 MiB/s                          

✓ Re-used 33.33% of old, added 2.00 MiB fresh data
✓ 2.00 MiB patch (66.67% of the full size) in 42.202581ms
 symlinks-test ツ butler ditto v1 out
Counting files in v1...
Mirroring...
 symlinks-test ツ butler apply2 --staging-dir staging patch.pwr --signature patch.pwr.sig out
• Patching out (in-place)
Save interval: 2s
✓ Before commit, staging dir is 2.00 MiB                                        
• Committing...
✓ Patched 3.00 MiB (2 files, 0 dirs, 0 symlinks) @ 616.67 MiB/s (a few seconds total)
• Verifying against signature...
✓ Phew, everything checks out!                                                  
 symlinks-test ツ butler verify patch.pwr.sig out
• Verifying out
✓ 3.00 MiB (2 files, 0 dirs, 0 symlinks) @ 571.75 MiB/s                         

 symlinks-test ツ butler ls out
out: directory
-rw-r--r--   2.00 MiB test.txt
-rw-rw-rw- 1024.00 KiB test2.txt
 symlinks-test ツ sha256sum out/*.txt
774a6278cfe70740e88337e3503916cb5b827554070da78deff3b78c9cc8d1fa  out/test2.txt
cdd3adcb76b92c91a59291bd7f466c2338a651144d5d8163d363ebd7616159e7  out/test.txt

[fasterthanlime]

Oh, I think I see why you had that error - the actual patching didn't fail, but "reverting to the older version" uses heal, and heal does have the issues you brought up. Working on a fix now.

[fasterthanlime]

Okay that multi-level symlink bug is stupid:

• There was a foreach loop that made sure all symlinks existed and pointed to the right file
• If they didn't exist, they were created - but the logic continued
• The next bit of the logic was if they had the wrong destination - which if they were missing would be "", so it would be wrong
• In which case it would remove the existing link and call Symlink again, this time with the right path
• and then return (instead of continuing the loop), which would ignore all further symlinks.

So yeah.