[
Date Prev][
Date Next][
Thread Prev][
Thread Next][
Date Index][
Thread Index]
[
List Home]
Re: [jgit-dev] FS_Win32_Cygwin instantiation
|
> 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.
The problem is that the FS is a static instance which is accessed from
other core classes. So the constructor parameter has to passed through
these classes. No problem in my opinion, but before adding a new
constructor parameter to Repository and a couple of other classes I
wanted to ask.
As mentioned, a system property in the static{}-block within FS can do
the job nearly as good, except of being able to re-configure your git
installation while the application is running: you may think of a
Preferences dialog where you can specify a Git executable. Changing this
Git executable from Cygwin to non-Cygwin or vice versa would mean to
restart the application to have these changes taking effect in FS.
That's the drawback of the static instance I would like to avoid.
> I do not see much benefit in moving userHome() to yet another utitlity class.
It's a utility method and unrelated to "file system abstraction".
--
Best regards,
Marc Strapetz
=============
syntevo GmbH
http://www.syntevo.com
http://blog.syntevo.com
Matthias Sohn wrote:
> 2010/4/20 Johannes Schindelin <Johannes.Schindelin@xxxxxx
> <mailto:Johannes.Schindelin@xxxxxx>>
>
> On Tue, 20 Apr 2010, Matthias Sohn wrote:
>
> > 2010/4/20 Marc Strapetz <marc.strapetz@xxxxxxxxxxx
> <mailto: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.
>
> --
> Matthias