On 07/16/2013 05:36 PM, Ramsay Jones wrote:
Caveats:
1) I don't find any speed improvement of the current patch over the
previous one (the tests actually ran faster with the earlier patch,
though the difference was less than 1%).
2) I still question this whole approach, especially having this
non-POSIX compliant mode be the default. Running in this mode breaks
interoperability with Linux, but providing a Linux environment is the
*primary* goal of Cygwin.
Yes, I agree. Since I _always_ disable the Win32 stat functions (by
setting core.filemode by hand - yes it's a little annoying), I don't
get any "benefit" from the added complexity.
However, I don't use git on cygwin with *large* repositories. git.git
is pretty much as large as it gets. So, the difference in performance
for me amounts to something like 0.120s -> 0.260s, which I can barely
notice.
Other people may not be so lucky ...
I would be happy if my original patch (removing the win32 stat funcs)
was applied, but others may not be. :-P
ATB,
Ramsay Jones
Cygwin 1.7 is very different than the earlier, no longer supported, and
no longer available Cygwin variants in many ways, but stat is one of
them. Cygwin 1.7 uses Windows ACLs to represent file permissions, and
therefore gets the file permissions directly from the underlying OS
calls. Earlier Cygwin versions (attempted to) overlay POSIX permissions
on Windows systems using extended attributes and other means, and in
many cases had to resort to opening the file and examining it to
determine executability. This is not true in 1.7.
Therefore, your later patch would be expected to have much less benefit
for 1.7 than for 1.5 (I don't detect *any* benefit on 1.7 when I set
core.filemode=false). There are many choices, three are:
a) Remove the win32 stat funcs, eliminating all of the troublesome code
paths and maintenance burden (your original patch).
b) Add your latest patch, with attendant complexity and maintenance
burden, to support a version of Cygwin that is no longer available and
was last updated over four years ago.
c) Like b, except make this triggered only by a "CYGWIN_15" macro,
limiting this to use by the legacy cygwin platform.
I strongly vote for a, could support c, but fear b is just going to keep
us chasing down bugs. Especially so when we consider that this patch can
only speed things up when core.filemode=false, which mode:
a) causes git to fail its test suite.
b) breaks compatibility with Linux
c) violates the primary goal of the Cygwin project, which is to provide
a Linux environment on Windows.
Mark
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html