refactor: Improve AuthenticationStorage

[pavelzw]

Description

While working on #1008 I noticed that AuthenticationStorage::default() doesn't take RATTLER_AUTH_FILE into account.
This PR fixes this and cleans up a bit.

Also, the credentials.lock wasn't cleaned up properly and since we never concurrently write to it, I removed the lock file and made the code a bit clearer/simpler.

Also, i switched the locking mechanism from creating a separate lockfile to using advisory locks on credentials.json.

[baszalmstra]

The cleanup looks good but what is the downside of keeping the lock file? It guards against multiple processes trying to access it. Maybe it doesnt happen often but I also dont see a downside it keeping it.

[pavelzw]

This will be needed for pixi's authentication-file-override in its global config.

My suggestion would be the following order:

• file storage from $RATTLER_AUTH_FILE (if set)
• file specified in authentication-file-override in pixi global config (if set) (pixi-only that's why we need to insert_backend(1, ...) in pixi)
• file storage from the default location
• keyring storage
• netrc storage

[pavelzw]

this was introduced in #645 but i don't think this is used anywhere so i don't think this brings us much benefit, especially now since RATTLER_AUTH_FILE is handled directly in AuthenticationStorage::default()

[pavelzw]

what is the downside of keeping the lock file

it currently is not working properly 😅
when i do pixi auth login, it creates the credentials.json as well as a credentials.lock but doesn't clean up after itself... at least i had quite many scenarios where there was some credentials.lock floating around in my files

[baszalmstra]

what is the downside of keeping the lock file

it currently is not working properly 😅 when i do pixi auth login, it creates the credentials.json as well as a credentials.lock but doesn't clean up after itself... at least i had quite many scenarios where there was some credentials.lock floating around in my files

That is expected behavior, otherwise deleting the file could create a race condition. However, instead of locking the credentials.lock file we could just lock the credentials.json itself!

[pavelzw]

However, instead of locking the credentials.lock file we could just lock the credentials.json itself

how would that work? AFAICT, lock.lock() and lock.unlock() both erase the contents of the file used as a lock 🤔

[pavelzw]

otherwise deleting the file could create a race condition

makes sense 🤔
although i personally found it more than once irritating that there is a random credentials.lock floating around on my system. also, i can't think of a scenario where you would want to write to a credential file multiple times at once...

IMO it being irritating weighs more than potential race conditions on pixi auth login/logout so i would be in favor of removing it but i'll let you decide :)
for now, i re-added the locking

[baszalmstra]

would

We can use an advisory lock on the file instead.

[baszalmstra]

You can use the async-fd-lock crate to achieve this locking behavior.

[pavelzw]

thanks! 6f93e41

[pavelzw]

Thanks for the pointers @baszalmstra! This is ready from my side

[baszalmstra]

Some final last minute small nitpitcks. We can merge after!

[baszalmstra]

Thanks @pavelzw !