Make cargo test work on Ubuntu 24.04 regardless of umask

[mathijs81]

🧢 Changes

I cloned the repo and tried cargo test but ran into issues because my default umask seems to be 0002 (I'm running Ubuntu 24.04).
These changes make cargo test run through and I think should also work on all other systems.

[vercel]

@mathijs81 is attempting to deploy a commit to the GitButler Team on Vercel.

A member of the Team first needs to authorize it.

[Byron]

Thanks a lot for the fix, it's much appreciated!

I wonder if @slarse also ran into this? If not, maybe it really is only about the umask somehow.
In any case, it's probably OK to wait until git2 is no more.

[mathijs81]

Even with the right umask, the forced_changes_with_snapshot_and_directory_to_file test kept failing as described in the comment, with libgit2 complaining that the Oid is null/not found when it's supposed to do the checkout, I couldn't figure out either why that's happening.

[Byron]

Do you want to add more commits/linux specific exclusions to make this work?

Maybe the CI specific checks should really be Linux checks?

[mathijs81]

I tested on a second 24.04 machine and needed to add a couple more umask normalizations.
With these changes all tests pass for me on both those Ubuntu 24.04 machines (including the two other ones in checkout.rs that are disabled for the CI), so I think it's not needed to exclude linux further?

[Byron]

That's good to hear, thanks so much!

Let's have it!

[slarse]

@Byron I had umask issues as well, but not exactly the same ones as my umask is different. In fact, I still have umask issues if I set a more restrictive umask (e.g. 0077).

Isn't it more convenient to just look at the filetype and normalize based on that, instead of building up a sort of library of possible outcomes from different umasks? When it comes to Git, the permission bits are really only interesting when it comes to regular files anyway.

Edit: Concrete suggestion in #11375. It should more generally cover different umasks.

[Byron]

Thanks for flagging this hackery that has been going on here. The mapping was originally made by me as a sure-fire way of making the test work on CI, so I wanted it to work for sure, going with stupid simple.
Now that crutch is growing, and that should not continue :).

Thanks also for the fix, I am heading over there now.