Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NEW FEATURE] make git-for-windows supports posix file mode bits in WSL's way #4438

Merged
merged 2 commits into from
Jul 7, 2023

Conversation

swigger
Copy link

@swigger swigger commented May 23, 2023

In WSL(2), we can set the mode for an NTFS file, for example:

$ chmod 0755 /mnt/d/test/a.sh

provided that WSL has enabled metadata mounting NTFS volumes.

In order to facilitate better collaboration between the Windows
version of Git and the WSL version of Git, we can make the Windows
version of Git also support reading and writing NTFS file modes
in a manner compatible with WSL.

In this pull request, I've incorporated two commits. The initial commit
introduces two helper functions for reading and writing file mode bits.
The subsequent commit enhances Git with WSL compatibility for
improved collaboration.

Copy link
Member

@dscho dscho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for contributing this feature! It is pretty well-written, and I only have one suggestion regarding its form: to separate out the WSL-related file mode functions into their own file.

Having said that, I have a major concern about the design of the feature: it requires calling CreateFileW() for potentially many, many files, and we know for a fact that this API function is quite slow and therefore the impact would be noticeable for monorepo users.

Maybe we can strike a compromise here? I would suggest a new config option that is opt-in, enabling WSL compatibility (which I would probably use myself on a couple of repositories of mine), and leaving the original behavior intact for users who do not need WSL compatibility.

@swigger what do you think?

compat/mingw.c Outdated Show resolved Hide resolved
compat/win32/fscache.c Outdated Show resolved Hide resolved
compat/win32/fscache.c Outdated Show resolved Hide resolved
Copy link
Member

@dscho dscho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks already pretty awesome! I offer a couple of recommendations how to polish this PR further. In addition, I would like to ask for two things:

  • Please use your real name in the author information and the Signed-off-by: line (it is totally allowed, even encouraged, to use non-ASCII characters there).
  • Please rebase the branch onto the latest main: Git for Windows v2.41.0 was released in the meantime (and this was the reason why I did not have time to review this PR in-depth earlier than today).

Documentation/config/core.txt Outdated Show resolved Hide resolved
Documentation/config/core.txt Outdated Show resolved Hide resolved
compat/mingw.c Outdated Show resolved Hide resolved
compat/mingw.c Outdated Show resolved Hide resolved
compat/mingw.c Outdated Show resolved Hide resolved
compat/win32/fscache.c Outdated Show resolved Hide resolved
compat/win32/wsl.c Show resolved Hide resolved
compat/win32/wsl.c Outdated Show resolved Hide resolved
compat/win32/wsl.c Outdated Show resolved Hide resolved
compat/win32/wsl.c Outdated Show resolved Hide resolved
In Git for Windows v2.39.0, we fixed a regression where `git.exe` would
no longer work in Windows Nano Server (frequently used in Docker
containers).

This GitHub workflow can be used to verify manually that the Git/Scalar
executables work in Nano Server.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho dscho added this to the v2.41.0(2) milestone Jul 6, 2023
@dscho
Copy link
Member

dscho commented Jul 6, 2023

This commit should probably still mention https://devblogs.microsoft.com/commandline/chmod-chown-wsl-improvements/, and I would like to verify for myself that it works with my WSL setup. Sadly, sudo mount -o remount,metadata,umask=22,fmask=111 seems not to do what I want, and I have currently a couple of processes open that use /mnt/c therefore cannot unmount. Hopefully I'll get to testing this PR later today.

@dscho
Copy link
Member

dscho commented Jul 6, 2023

Hopefully I'll get to testing this PR later today.

I got to test this, and can testify that it works! (You'll need to set core.fileMode to true if you choose to do this in any existing repository.)

It's a bit too late over here to start releasing Git for Windows with this PR today, but I'll do it first thing in the morning tomorrow.

The Windows Subsystem for Linux (WSL) version 2 allows to use `chmod` on
NTFS volumes provided that they are mounted with metadata enabled (see
https://devblogs.microsoft.com/commandline/chmod-chown-wsl-improvements/
for details), for example:

	$ chmod 0755 /mnt/d/test/a.sh

In order to facilitate better collaboration between the Windows
version of Git and the WSL version of Git, we can make the Windows
version of Git also support reading and writing NTFS file modes
in a manner compatible with WSL.

Since this slightly slows down operations where lots of files are
created (such as an initial checkout), this feature is only enabled when
`core.WSLCompat` is set to true. Note that you also have to set
`core.fileMode=true` in repositories that have been initialized without
enabling WSL compatibility.

There are several ways to enable metadata loading for NTFS volumes
in WSL, one of which is to modify `/etc/wsl.conf` by adding:

```
[automount]
enabled = true
options = "metadata,umask=027,fmask=117"
```

And reboot WSL.

It can also be enabled temporarily by this incantation:

	$ sudo umount /mnt/c &&
	  sudo mount -t drvfs C: /mnt/c -o metadata,uid=1000,gid=1000,umask=22,fmask=111

It's important to note that this modification is compatible with, but
does not depend on WSL. The helper functions in this commit can operate
independently and functions normally on devices where WSL is not
installed or properly configured.

Signed-off-by: xungeng li <xungeng@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho
Copy link
Member

dscho commented Jul 7, 2023

/git-artifacts

The tag-git workflow run was started

@dscho
Copy link
Member

dscho commented Jul 7, 2023

/add release note feature To support interoperability with Windows Subsystem for Linux (WSL) better, it is now possible to let Git set e.g. the executable bits of files (this needs core.WSLCompat to be set, and the NTFS volume needs to be mounted in WSL using the appropriate options).

The workflow run was started

github-actions bot pushed a commit to git-for-windows/build-extra that referenced this pull request Jul 7, 2023
To support interoperability with Windows Subsystem for Linux (WSL)
better, it [is now possible to let Git
set](git-for-windows/git#4438) e.g. the
executable bits of files (this needs `core.WSLCompat` to be set, and
[the NTFS volume needs to be mounted in WSL using the appropriate
options](https://devblogs.microsoft.com/commandline/chmod-chown-wsl-improvements/)).

Signed-off-by: gitforwindowshelper[bot] <gitforwindowshelper-bot@users.noreply.github.com>
@dscho
Copy link
Member

dscho commented Jul 7, 2023

/release

The release-git workflow run was started

@dscho dscho merged commit 330483e into git-for-windows:main Jul 7, 2023
33 checks passed
dscho added a commit that referenced this pull request Aug 7, 2024
Support for wsl mode bits was previously added to git, but there was a
bug because the filenames provided by fscache did not contain paths.

This commit fixes the issue.

The previous feature is added in PR #4438,
but at that time I didn't tested so much.
Sorry to have this bug.

To test this feature, set core.wslcompat to ture and core.filemode to
true and make sure repo is on NTFS.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants