Re: [PATCH] Add a new lstat implementation based on Win32 API, and make stat use that implementation too.
- From
- Reece Dunn <msclrhd@googlemail.com>
- Date
- Sep 2, 2007, 16:33 UTC
- Message-ID
- <3f4fd2640709020933sa2bdec0g532500ea49c179b5@mail.gmail.com>
- In-Reply-To
- <46DAE025.900@trolltech.com>
On 02/09/07, Marius Storm-Olsen <marius@trolltech.com> wrote:
Show 36 quoted lines
> Reece Dunn wrote: > > On 02/09/07, Marius Storm-Olsen <marius@trolltech.com> wrote: > >> This gives us a significant speedup when adding, committing and stat'ing files. > >> (Also, since Windows doesn't really handle symlinks, it's fine that stat just uses lstat) > >> > >> + if (ext && (!_stricmp(ext, ".exe") || > >> + !_stricmp(ext, ".com") || > >> + !_stricmp(ext, ".bat") || > >> + !_stricmp(ext, ".cmd"))) > >> + fMode |= S_IEXEC; > >> + } > > > > This breaks executable mode reporting for things like configure > > scripts and other shell scripts that may, or may not, be executable. > > Also, you may want to turn off the executable state for some of these > > extensions (for example if com or cmd were not actually executable > > files). This makes it impossible to manipulate git repositories > > properly on the MinGW platform. > > Actually, you don't really need the EXEC bit for Git to work. I just > added it for completeness. (We _could_ remove that too, since it's > slowing us down slightly ;-) > > Remember that Git isn't using MSys for its builtins, so MinGW Git > doesn't understand the MSys notion of executable files anyways. > The MinGW port actually peeks at the beginning of a file (ignoring exe > files), and sees if there's an interpreter there. If there is, it will > expand > git-foo args... > into > sh git-foo args... > and execute the command. So, it's not really affected by this change. > > I haven't had any problems with this patch on my system, so could you > explain what you mean with 'this makes it impossible to manipulate git > repositories'?
You pull a repository that contains executable scripts that are required to work in order to build the system. You then make some modifications to the local repository and run the 'git add .' command. Since this patch is reporting executable bits differently, the mode change is stored as well as the local modifications. Now the changes are pushed upstream (along with the file mode changes).
Someone running a Linux machine, pulls your changes. When those files are checked out, the executable state of those scripts has now changed, preventing the Linux user from running those scripts. _That_ is what I meant. Or am I misunderstanding how git works in this case?
Show 14 quoted lines
> > Would it be possible to use the git tree to manage the executable > > state? That way, all files would not have their executable state set > > by default on Windows. The problem with this is how then to set the > > executable state? Having a git version of chmod may not be a good > > idea, but then how else are you going to reliably and efficiently > > modify the files permissions on Windows? > > The file-state-in-git-tree belongs in a different discussion. Have a > look at the '.gitignore, .gitattributes, .gitmodules, .gitprecious?, > .gitacls? etc.' and 'tracking perms/ownership [was: empty directories]' > threads. Permissions are not a trivial topic, since systems represent > them differently. This patch just tries to reflect the read, write and > execute permissions as normal Windows would; and it only cares about > file extensions (and the PE header, if it exists).
I understand that this is not a trivial topic. I was thinking that this different behaviour w.r.t. the executable permission will break things when you have developers on both Linux and Windows, such as the cairo developers, for current git usage.
I have not really been tracking those threads, but I will take a look at them.
> Also note that my patch totally ignores the Group & Others part of the > permission bits. Again, we're on Windows so we don't really care. We > _could_ make it reflect the ACLs in Windows, but then we'd have to make > it optional, since that's _really_ slow to 'stat'.
Sure. Cygwin does use ACLs to implement stat which is why it is slow. So anything that can speed git up here, without any breakage in functionality, is a good thing.
- Reece