Don't use lockfile to protect updates to selfcheck file.

[chrahunt]

Previously, we were using lockfile.LockFile to protect updates to <cache dir>/selfcheck.json.

This uses hard links to create file locks, which are not supported in several contexts (as raised in various issues).

Now, we securely create a temporary file next to the target file, write to it, and rename it to the target path (removing the target path first if it exists).

In general we are graceful to failure, retrying several times over the course of 1 second before giving up. The only expected failure here should be on Windows if another process has an open file handle, and since this file is not very large it is reasonable that another pip instance will be done reading within that time period - otherwise we silently pass.

Closes #6954.

[pradyunsg]

I spent some more time looking at all the options @uranusjr listed -- vistir's (pipenv) helper looks best suited for this to me.

@uranusjr Do you reckon it'd be fine to factor it out into a (yet another) dedicated "atomic file writes via temporary file" library?

[pradyunsg]

(for folks following via email, I removed the incorrect paste in my previous comment)

[chrahunt]

Just to make some differences explicit, so we can decide on their value:

1. Do we want to retry on failure to rename the file? I think this would be more relevant for the HTTP cache use case since it seems more reasonable that another pip process may have it open.
2. Do file permissions for the tempfile matter?
3. On the off chance that all renaming operations fail, do we want to try to remove the temporary file afterwards?

[uranusjr]

Do you reckon it'd be fine to factor it out into a (yet another) dedicated "atomic file writes via temporary file" library?

Sure; would this be more suited to be explicitly hosted by PyPA? (also cc @techalchemy)

[BrownTruck]

Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the master branch into this pull request or rebase this pull request against master then it will be eligible for code review and hopefully merging!

[pradyunsg]

would this be more suited to be explicitly hosted by PyPA?

It's not exactly a packaging-related tool so I'd prefer it live somewhere else.

[chrahunt]

In #6970 there is an example of a likely race condition from concurrent cache access. This kind of usage will cause the move after write to fail, so I think we should retry a few times so as to not lose work we would've already done.

The pipenv helper construction does not support this kind of usage as-is and IMO adding retry-related options to it may be irrelevant for most use cases. I would feel more comfortable proceeding with the current implementation for this and the HTTP cache use case and then factoring it out afterwards if it still looks substantial enough to be worth it. Definitely open to any thoughts on this approach.

[chrahunt]

Rebased. This should be ready for review!

[pradyunsg]

OK with this in principle.

[BrownTruck]

Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the master branch into this pull request or rebase this pull request against master then it will be eligible for code review and hopefully merging!

[chrahunt]

Anything else needed here in order to merge?

[pradyunsg]

One of us clicking merge. :)