Integrate Hero Version: Disable usage of hardlinks, but allow enabling via settings

BigRoy commented 3 months ago

Changelog Description

Additional info

There are 'known issues' with hardlinks on Windows where Windows disallows deleting any of the links if ANY of the files are in use. As such, when e.g. hero version and latest v010 publish are hardlinked, a machine actively has v010 in use (e.g. on renderfarm) and we try to delete an swap the hero version with a v011 hardlink when publishing a new version then Windows disallows the removal of the hero version's files - even if actually that particular file isn't in use.

Historically there have been issues with hardlinks when publishing looks. More details scattered across:

And yes, hardlinks have bitten our asses before as you can see.

Testing notes:

Integrating hero version should not use hardlinks by default
Setting to enable hardlink usage should work.

On Windows you should be able to 'detect' hardlinks using fsutil hardlink:

fsutil hardlink list MyFileName.txt

m-u-r-p-h-y commented 3 months ago

just to show different ways of finding the hardlinks on Windows

fsutil hardlink list <fileName>

using Linux subsystem on Windows

find . -links +1

LiborBatek commented 3 months ago

Not sure how to approach this PR and its testing... could you elaborate more on exact testing steps pls?

I was trying to render animation on DL and also publish animation products while rendering and never happened to me that hero version integration failed (tested outside of this PR). So Im not sure how I should prove this PR fixes something.

So more elaborate test steps might help me a lot...thx

BigRoy commented 3 months ago

Not sure how to approach this PR and its testing... could you elaborate more on exact testing steps pls?

I was trying to render animation on DL and also publish animation products while rendering and never happened to me that hero version integration failed (tested outside of this PR). So Im not sure how I should prove this PR fixes something.

So more elaborate test steps might help me a lot...thx

The bug the client is facing may be hard to reproduce - I think the best test for you here, outside of their production enviromment, is to ensure the setting does what it is intended to do and that is "choose" between writing hero versions as hardlinks or regular copies.

So I suppose, with the setting on and off do a few hero publishes, all should work fine. Then each time check whether the files are hardlinks or not - using e.g. the techniques @m-u-r-p-h-y showed in the screenshots (for which a windows variant is also in the testing notes). Those will have to be done through command line most likely since I don't know a way on windows to detect whether a file is hardlink or not without command line.

LiborBatek commented 3 months ago

...it seems that the Use Hardlinks knob does not have any effect atm

BigRoy commented 3 months ago

@LiborBatek your test screenshot with the forcing copy versus hardlink seems to be for Look publishing from Maya - which is unrelated to this PR. The only thing this PR changes is the Integration of Hero Versions

Then, the logic currently was always reporting it's copying the file instead of reporting it's hardlinking - which may be confusing since you then can't really check the log but must check the files on disk itself to confirm whether it hardlinked or not. I've changed that with https://github.com/ynput/ayon-core/pull/678/commits/0ab3653f360e530402d17bc1c78fbeea4f2cd9fd

It should now debug log "Hardlinking file {source} to {destination}" if hardlink is enabled for Integrate Hero Versions and will debug log "Copying file {source} to {destination}". If for whatever reason the hardlink failed and it falls back to copying the file it will now debug log that as well.

BigRoy commented 3 months ago

Test run on my side. Setting enabled:

// pyblish.IntegrateHeroVersion : Hardlinking file "C:\projects\ayontest\asset\char_hero\publish\pointcache\pointcacheMain\v022\ynts_char_hero_pointcacheMain_v022.abc" to "C:/projects/ayontest/asset/char_hero/publish/pointcache/pointcacheMain/hero/ynts_char_hero_modeling_hero.abc"

Setting disabled:

// pyblish.IntegrateHeroVersion : Copying file "C:\projects\ayontest\asset\char_hero\publish\pointcache\pointcacheMain\v023\ynts_char_hero_pointcacheMain_v023.abc" to "C:/projects/ayontest/asset/char_hero/publish/pointcache/pointcacheMain/hero/ynts_char_hero_modeling_hero.abc"

m-u-r-p-h-y commented 3 months ago

should not the hardlinking be more generic setting (the storage either supports it or not)

having this option scattered across different families and DCCs does not sound right to me. The solution should not be part of this PR just thinking aloud . . .

BigRoy commented 3 months ago

should not the hardlinking be more generic setting (the storage either supports it or not)

having this option scattered across different families and DCCs does not sound right to me. The solution should not be part of this PR just thinking aloud . . .

I understand your point. But at the same time I don't see an issue with a studio wanting looks to NOT be hardlinked, but hero versions they want. Which is a customization option you would lose then. These are now two options - that's not too bad. If this was scattered all over for 10s of options.

It could be worth a separate issue if you think it's very problematic in design currently.

ynput / ayon-core