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

> 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


Back to the top