Fix windows race condition when writing image with duplicate layers

[dgannon991]

This is a potential solution for #1920, any feedback would be greatly appreciated.

[slonopotamus]

I'd say a more robust solution is "try to rename, if it fails, but destination exists, just remove the source and pretend you're fine". This would also cover the case when multiple crane processes stomp on the same file.

[slonopotamus]

I had a very similar issue in git-lfs in the past. Here's the analysis: git-lfs/git-lfs#3880 (comment) and here's the final fix that solved the issue: git-lfs/git-lfs@662a624

[dgannon991]

I'll give that a go, thank you for the feedback.

[dgannon991]

@slonopotamus I've written this up and given it a test, and I can confirm it works:

	err = os.Rename(w.Name(), renamePath)
	var badInputErr *os.LinkError
	if err != nil && errors.As(err, &badInputErr) && badInputErr.Err == syscall.ERROR_ACCESS_DENIED {
		if _, err := os.Stat(renamePath); err == nil {
			// windows os.Rename() issue #1920
			// the destination already exists, just remove the target and continue
			return os.Remove(w.Name())
		}
	}
	return err

I do have to say though of the two solutions I think I'm leaning towards the first one, as it makes os.Rename behave more like linux (in that it's an atomic transaction.) Would you mind if I left this up for a second opinion? I'm more than happy to swap over to this approach, but I worry it would mask other issues outside of the scope of this ticket (as you mentioned, running two crane processes at the same time, it got me thinking that realistically that /should/ be an error!)

[slonopotamus]

Sure, I'm just passing by, and simply gave my thoughts.

[dgannon991]

@slonopotamus - thank you for taking the time, the thoughts were definitely appreciated, and it was great to have a confirm that it was an issue in other places!

[dgannon991]

Hi Guys,
Is there anything more I need to do on this one? Just aware it will auto close at some point!
Cheers,
David