Write separate statefiles per prefix.

[chrahunt]

Previously, we kept selfcheck info for all pip instances in the same file.

Now, we use a separate file per pip instance, simplifying the process of recording updated state.

Also added several tests that are a little less intrusive into the selfcheck implementation details.

Makes progress on #6954.

[cjerdonek]

A couple comments.

[chrahunt]

Unrelated failure on Appveyor and it doesn't appear to have "Rebuild" buttons like Travis, so closing/reopening.

[pradyunsg]

Unrelated failure on Appveyor and it doesn't appear to have "Rebuild" buttons like Travis, so closing/reopening.

Yea, AppVeyor <-> GitHub permissions is weird. I think one needs to be an Admin on AppVeyor at the organization level to do that.

Aside, if you see an unrelated failure from our test suite (not the CI's bootstrapping/setup) please file an issue -- ideally our tests would be reasonably resilient to the occasional network issues, system-specific configuration and cosmic-ray-induced bit flips. Okay, maybe not that last one.

[chrahunt]

👍

@cjerdonek, any other comments or concerns?

[chrahunt]

Refactored and split into separate commits so it should hopefully be easier to review. Please forgive my mess before. 🙏

[cjerdonek]

@cjerdonek, any other comments or concerns?

I haven't read the thread about the underlying rationale, but on the surface the code and change looks okay to me.

My only question / concern is around the length of the filename (79 characters). I'm wondering if that could ever cause problems. It doesn't strike me as something that needs cryptographic strength, but again, I haven't read the thread to know for sure.

[chrahunt]

The only property we need is being able to map to the same file, unique per prefix, independently on each invocation of the same pip. A cryptographically secure hash requires less consideration than one that may be considered broken or insecure, so it's a reasonable default IMO.

I could switch to sha224 as used in cache control, which would bring our path length to 70 (comparable to the 66 used for one of our http cache entries, which are kept in the same directory).

[chrahunt]

I have switched to sha224, moved the files to a subdirectory, and dropped the file extension. Now the file path length should be the same as for any of our HTTP cache files.

[chrahunt]

@cjerdonek, in case there are any concerns I have submitted #6879 so it is more clear where this is going and how it is related to #4766. Specifically, once we simplify our usage of the selfcheck state file from "read, merge, write" to just "write" then we do not need file locking and can just overwrite the file.

[cjerdonek]

@chrahunt I don't have any concerns... I trust and am sure the underlying idea is sound. It's just that I haven't had time to look at it. I was kind of hoping that the people participating in the #4766 discussion would, since it's not a discussion I really read / participated in.

[cjerdonek]

I appreciate the thought and additional write-up though! If I can get to it, it will help me get up to speed on what you're planning.

[chrahunt]

If it's okay, we can treat this PR independently of the underlying lockfile issue (similar to #6845). I think it's only necessary to consider it in the context of the larger issue if there's not enough justification for these changes on their own.

[chrahunt]

The discussion on #6879 indicates that the general approach is OK, and it is just a matter of deciding where to put the new utility methods. We will need the changes from this PR regardless of the outcome of that decision.

[chrahunt]

OK to merge?

[pradyunsg]

@chrahunt yep!