Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [jgit-dev] FS_Win32_Cygwin instantiation

2010/4/20 Johannes Schindelin <Johannes.Schindelin@xxxxxx>
On Tue, 20 Apr 2010, Matthias Sohn wrote:

> 2010/4/20 Marc Strapetz <marc.strapetz@xxxxxxxxxxx>
> > Some of our users with Cygwin git installation have reported performance
> > problems, even if they are using msysgit.
> >
> > The reason is that FS_Win32_Cygwin is always used if cygpath.exe is on
> > the path, even if our client makes sure that another git.exe is used.
> > I've performed a quick fix by introducing a system property to disallow
> > instantiation of FS_Win32_Cygwin. This works, but anyway I think a clean
> > solution would be to have the FS as a field in Repository, which is
> > propagated to relevant classes. As far as I could see these are not too
> > many. The field would default to current FS.INSTANCE and could be
> > overridden by a new constructor parameters (FS is used in the
> > constructor). FS.userHome() could be moved to some utility class FSUtils
> > to have usages of FS limited to its core functionality. I'm not sure if
> > there are other such singletons which could be converted to parameters.
> > In this case, an aggregating class GitEnvironment (or
> > RepositoryEnvironment) containing the FS would probably make sense.
> >
> How is this question relating to jgit ? To me it looks like
> gmane.comp.version-control.git or the msysgit list would be a better
> place for this question.

"FS.userHome()" indicates that it is Java code. You would never read
something like that in C. So my guess is that JGit does evil things,
discovering cypath.exe and assuming that the user is using Cygwin, and
doing things differently, then.

And indeed, a little "git grep cygpath v0.7.0" later, it looks as my guess
was correct.

YCM, you caught me ;-)

If FS detects cygwin jgit forks to cygpath for file path translation.

Adding a another constructor parameter to allow overriding automatic
detection seems ok to me. I do not see much benefit in moving userHome()
to yet another utitlity class.


Back to the top