mono / ngit

Automated jgit port to c#
261 stars 151 forks source link

Crash when home folder can't be located #14

Closed Kukkimonsuta closed 12 years ago

Kukkimonsuta commented 13 years ago

When trying to open repository on IIS NGit.Util.FS_Win32.UserHome fails to find home folder and returns null, which causes null reference exception in NGit.Util.SystemReader.UpenUserConfig. I'm not sure if ngit can live without user config internally, but it would be nice to have the possibility to at least redirect home folder somewhere else programatically.

[NullReferenceException: Object reference not set to an instance of an object.]
   Sharpen.FilePath..ctor(FilePath other, String child) in C:\Users\Administrator\Desktop\ngit-xpaulbettsx\Sharpen\Sharpen\FilePath.cs:25
   NGit.Util._SystemReader_65.OpenUserConfig(Config parent, FS fs) in E:\Projects\slluis-ngit-54a43da\NGit\NGit.Util\SystemReader.cs:116
   NGit.Storage.File.FileRepository..ctor(BaseRepositoryBuilder options) in E:\Projects\slluis-ngit-54a43da\NGit\NGit.Storage.File\FileRepository.cs:141
   NGit.BaseRepositoryBuilder`2.Build() in E:\Projects\slluis-ngit-54a43da\NGit\NGit\BaseRepositoryBuilder.cs:700
   NGit.Api.Git.Open(FilePath dir, FS fs) in E:\Projects\slluis-ngit-54a43da\NGit\NGit.Api\Git.cs:129
   NGit.Api.Git.Open(FilePath dir) in E:\Projects\slluis-ngit-54a43da\NGit\NGit.Api\Git.cs:109
harryhazza77 commented 12 years ago

We solved the issue locally by changing the call in Sharpen.RunTime.cs.Getenv(string var) to Environment.GetEnvironmentVariable(var, EnvironmentVariableTarget.User)

harryhazza77 commented 12 years ago

Also worth noting that the IIS 7.5 settings for the website/webservice need to be Anonymous Auth: Enabled ASP.Net Impersonation: Enabled (and there has to be a named account) (everything else Disabled)

Then you'll also need to add env variables under the named account on the server for HOMEDRIVE and HOMEPATH

alanmcgovern commented 12 years ago

Is this still an issue with the latest NGit code? We fixed something similar recently so if it does not fix your issue, please let me know and we'll get a fix merged in.

harryhazza77 commented 12 years ago

Hi Alan,

Many thanks for your post.

When you mention a similar issue, I think you are referring to "user.home should come from Environment.SpecialFolder.UserProfile". This does not address this issue which occurs in FS_Win32.cs.UserHomeImpl() which eventually returns a null reference.

On a IIS Web Server, it currently forces you to have Environment variables which when combined resolve to an existing folder which does NOT have to a have a git configuration file.

We detect that it the code is running under IIS and simply return a new FilePath instance constructed with a fixed folder path which means we have a no installation footprint other than installing Git.

Hope that helps.

alanmcgovern commented 12 years ago

I was actually thinking of https://github.com/mono/ngit/commit/eed3e5d285b9dce81217002fcd31eeb156b916d5 , though after further examination the fix in that commit will not help in this scenario.

At the moment the win32 path checks for a bunch of env vars and if they are all null it tries Runtime.GetProperty ("user.home"). By default this is the same as the UserProfile special directory. What you could do is simply use Runtime.SetProperty ("user.home", "the_path_you_want_to_use"). This will allow you to override the default in a 'sane' way.

Would this solve your problem?

harryhazza77 commented 12 years ago

Yes it would work for us although would that not be a 'permanent' override? We were trying to ideally avoid breaking other runtime scenarios nGit provides by adding detection of the IIS context and then branching accordingly. This way it should (hopefully) work under more contexts.

alanmcgovern commented 12 years ago

You need something like this in your C# code:

if (running_in_iis) Runtime.SetProperty ("user.home", path_to_use_as_home_dir);

harryhazza77 commented 12 years ago

Hi,

yes indeed that it was we are doing but just somewhere else (see earlier post). Is this something that can be looked into further for the best fix?

linquize commented 12 years ago

In pull request #31, I mentioned that for a normal Windows desktop user, environment variables HOMEDRIVE and HOMEPATH are used in NGit to determine the home directory. For ASP.NET user, the problem should have been resolved by b811c68d479ebf620aa036799f86c44bbf76fed4 and 24e3cde76eb94bd36676b4fb35626577c425a66c.

I also found that passing ("C:", "\Users\User") to Sharpen.FilePath(string, string) constructor simply call System.IO.Path.Combine(string, string), which differs from the behaviour of java.io.File class in Java. Should we either 1) learn from Java source code to reimplement Sharpen.FilePath(string, string) constructor? or 2) Patch NGit.Util/FS_Win32.cs to concat %HOMEDRIVE%%HOMEPATH% instead of calling Sharpen.FilePath(string, string) constructor? like what he did in https://github.com/sopolenius/ngit/commit/194291f5b8a5dbc549cab0690a7e27d065daa2cc

alanmcgovern commented 12 years ago

harry, the general rule for ngit is that we do our best to not patch the c# files to alter their behaviour. Every modification we make increases the burden when updating ngit from the latest java source. If your environment doesn't export the env variables that jgit requires, then realistically there are two approaches:

1) Export the required environment variables before launching the process using ngit 2) When your process launches, manually fix up the paths stored in Runtime

I don't think there is anything we can do from within ngit itself to fix this problem other than make a random guess as to what path we can use.

linquize, could you file a separate bug for this? We'll need to do option 1 there as there could be more than 1 place which relies on this behaviour.

Kukkimonsuta commented 12 years ago

For anyone interested I resolved this issue without setting any environment variables with little reflection:

var type = typeof(Sharpen.FilePath).Assembly.GetTypes().Where(ta => ta.Name == "Runtime").FirstOrDefault();
var method = type.GetMethod("GetProperties", System.Reflection.BindingFlags.Public | System.Reflection.BindingFlags.Static);
method.Invoke(null, null);
var field = type.GetField("properties", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static);
var properties = (System.Collections.Hashtable)field.GetValue(null);
properties["user.home"] = Path.Combine(Path.Combine(HostingEnvironment.ApplicationPhysicalPath, "App_Data"), "git");
linquize commented 12 years ago

Why don't let it fallback (in FS_Win32 when all environment variables are null) to the FS class to get "user.home"? That is to use Environment.GetFolderPath (Environment.SpecialFolder.UserProfile) which already fixed in 24e3cde76eb94bd36676b4fb35626577c425a66c

alanmcgovern commented 12 years ago

Sharpen.Runtime is now public and exposes GetProperty/SetProperty methods which can be used to modify user.home (or whatever) at runtime. I hope this helps!

dougrathbone commented 11 years ago

This doesn't seem to be the case with the version that comes packaged with ngit on nuget. is this package up to date?

alanmcgovern commented 11 years ago

I don't know who's involved in the nuget packaging nor how to update it myself. It's quite possible it's a very old version.