Thanks for tackling this.

It's strange that GitPython is still using this backend for storing files, I thought it's all done by the Git (CLI) based backend.

The python implementation is pretty slow and I definitely wouldn't want to trust it, so it makes me sad it's still in useand gets to meddle with your work.

Thanks! There are a couple of nits that should probably be fixed before merging.

It comes to mind that ingitoxide tempfiles are created with the correct mode right away. This is also how Git does it, and a way to completely avoid the issue here. In Python that doesn't seem quite as straightforward, but I wanted to mention it in case you know how.

Yes, great catch, that's true! It would be better to do that only in the rename case.

Besides that, and beyond this PR, the logic there could use a cleanup, as it really does object writes in the slowest possible way. In order to not slow to a crawl objects need to be written and hashed to memory. Then one won't write the object at all if it already exists in the database.

Independently of what might be right, this change seems to be the smallest possible change to fix this particular issue you have been encountering, so I think it's good to merge.

I will see to create a new release, too.

(This is only posted as a review because that lets me include a comment on a specific part of the changed code. This pull request has already been merged, and rightly so. You are under no obligation to reply!)

After therename(tmp_path, obj_path) operation, the file atobj_path might not be fully committed to disk immediately. If chmod is called before the system fully registers obj_path as available, a PermissionError may occur.

I'm curious if this is specific to bind mounts to the outside filesystem in Docker Desktop on macOS, or if it is broader. Is this related todocker/for-mac#6812? I ask because:

1. Not being able to operate on the target ofos.rename immediately after it returns successfully is unintuitive, and if this is not a bug in Docker, then maybe it should be documented somewhere. If it is in practice specific toos.rename, then it could be documented for that. If it is already documented, we could add a link tothe comment here.

2. Maybe the delays could be made specific to that situation, so as to avoid waiting before reporting otherPermissionErrors in most circumstances.

3. If creating a file with the desired permissions initially, as suggested in#115 (review), would work, then I think it would be worthwhile. But I worry that some ways of doing this mightalso be limited by whatever limitations apply to the filesystem when using containers on macOS.

4. If the file cannot be created with the desired permissions initially, I wonder if it might still be possible to create it in a way that would allow the desired permissions to be set without delay. The operation that was not permitted includes accessincreases:msktemp creates files with 0600 permissions, but they are then usually changed to 0444. If permissions are changed in a way that onlydecreases access, by usually creating the file with 0644 permissions and then changing them to 0444, would the change still have to wait? It seems odd to think those operations might have different constraints, butdocker/for-mac#6812 makes me think that might be so.

   Even if we need to avoid path collisions before the file is moved into place as robustly asmkstemp does, we can still create the file initially with less restrictive permissions, by usingmkdtemp orTemporaryDirectory to make a temporary directory, and then creating the file with a predictable name inside that directory.

My motivation in bringing up (2), (3), and (4) is that I worry that, for some callers, the delays could accumulate. Callers should probably not callstore in a loop without stopping on errors, so ideally the added delay shouldn't exceed one second. But if a program makes independent attempts to store many objects and then reports errors, then there could be a long delay ifPermissionError is being raised for other reasons such as the user not having enough access to the repository. In principle this can already happen on Windows because of the retry logic forremove, but as far as I know it has not been expected on other systems.