Skip to content
This repository has been archived by the owner on Nov 9, 2017. It is now read-only.

Run git-daemon on windows without closing socket immediately. #70

Closed

Conversation

toshiyuki-ogawa
Copy link

git client had exit with 'read error' some time, if connected git-daemon
on windows. The cause was that git-daemon close socket immediately.
I fixed this problem.
Here is detail the explanation.

[https://github.com/toshiyuki-ogawa/msysgit/wiki/A-consideration-of--'fatal:-read-error-(invalid-argument)']

Signed-off-by: Toshiyuki Ogawa Toshiyuki.Ogawa.2.71@gmail.com

git client had exit with 'read error' some time, if connected git-daemon
on windows. The cause was that git-daemon close socket immediately.
I fixed this problem.
Here is detail the explanation.

https://github.com/toshiyuki-ogawa/msysgit/wiki/A-consideration-of--'fatal:-read-error-(invalid-argument)'

Signed-off-by: Toshiyuki Ogawa <Toshiyuki.Ogawa.2.71@gmail.com>
@buildhive
Copy link

MSysGit - the development behind Git for Windows » git #83 FAILURE
Looks like there's a problem with this pull request
(what's this?)

@dscho
Copy link
Member

dscho commented May 8, 2013

@toshiyuki-ogawa looks like a file is missing here, or maybe Windows-specific stuff is not hidden appropriately behind #if ... #endif guards.

@linquize
Copy link

linquize commented May 9, 2013

Does this fix the instability problem of git-daemon on Windows?

@toshiyuki-ogawa
Copy link
Author

I would think that this fix it. On my windows, my fixed version git works fine.

@dscho
Copy link
Member

dscho commented May 9, 2013

@linquize the proof lies in the pudding. Please check it out (and you know that it is easy with the net installer).

@toshiyuki-ogawa did you get a chance to address the concerns raised by Jenkins (Cloudbees' buildhive)?

@toshiyuki-ogawa
Copy link
Author

I got account of the Cloudbee's buildhive now. It would take two or three days to understand it.

@dscho
Copy link
Member

dscho commented May 9, 2013

Have you actually looked at the link buildhive attached above? If so, have you followed the "Console Output" link? Please notice this line at the end of said output:

upload-pack.c:16:26: fatal error: winsock-proc.h: No such file or directory

Following the link to your commit attached by GitHub itself, I cannot fail to see the responsible line: toshiyuki-ogawa@48f7556#L30L15

There is actually a file of the name winsock-proc.h which you add, but it is in compat! So the line should read"

#include "compat/winsock-proc.h"

if at all. But as you can see from

git grep include.*compat/

there are precious few users of that construct in Git outside of git-compat-util.h. The reason is that we try to avoid littering the code with platform-specific #define's and rather try to use POSIX functions and provide platform-specific overrides if at all needed. Please have a look at the way it is done with, say, opendir.

By the way, this is not at all to hassle you, but to make the code more maintainable and therefore robust. I hope you understand!

@toshiyuki-ogawa
Copy link
Author

I had looked Jenkins Console output and understood to modify include of 'winsok-proc.h'. I would like to commit the modification In two days.
I'm sorry that I am in late to fix error because I have only one hour in a day to spend for git source.

@buildhive
Copy link

MSysGit - the development behind Git for Windows » git #84 FAILURE
Looks like there's a problem with this pull request
(what's this?)

Toshiyuki Ogawa added 2 commits May 10, 2013 21:00
Signed-off-by: Toshiyuki Ogawa <Toshiyuki.Ogawa.2.71@gmail.com>
Signed-off-by: Toshiyuki Ogawa <Toshiyuki.Ogawa.2.71@gmail.com>
@buildhive
Copy link

MSysGit - the development behind Git for Windows » git #85 SUCCESS
This pull request looks good
(what's this?)

The option could be to specify close socket at start time. But I had not
need it.

Signed-off-by: Toshiyuki Ogawa <Toshiyuki.Ogawa.2.71@gmail.com>
@buildhive
Copy link

MSysGit - the development behind Git for Windows » git #86 SUCCESS
This pull request looks good
(what's this?)

compat/win-fd.h and compat/winsock-proc.h have to be after mingw.h.

Signed-off-by: Toshiyuki Ogawa <Toshiyuki.Ogawa.2.71@gmail.com>
@buildhive
Copy link

MSysGit - the development behind Git for Windows » git #87 SUCCESS
This pull request looks good
(what's this?)

@toshiyuki-ogawa
Copy link
Author

I did some commits to compie collectly on both linux and msysgit.
dscho and linquize, Thank you for some advice. I could fix some errors on buildhive and kept git guildlines.
As an aside, I carelessly clicked Close botton a few hours ago. I did not know tha 'close' means close 'pull request' .

@dscho
Copy link
Member

dscho commented Dec 17, 2013

@toshiyuki-ogawa sorry for the long delay (I cannot always work as much on Git as I want to). I will have a look at this soon!

@toshiyuki-ogawa
Copy link
Author

Ok, I will pull the socket-test on my pc and do my best.

@dscho
Copy link
Member

dscho commented Mar 22, 2014

Thank you, @toshiyuki-ogawa!

@dscho
Copy link
Member

dscho commented Apr 2, 2014

@toshiyuki-ogawa did you have any luck with the minimal socket test?

@toshiyuki-ogawa
Copy link
Author

Yes I did, I am going to push some modification to work it. The push will be done 2 or 3 days later. Now I am writing some descriptions for it.

@linquize
Copy link

linquize commented May 6, 2015

Any followup?

@dscho
Copy link
Member

dscho commented May 6, 2015

@linquize how about providing a Minimal, Complete and Verifiable Example, seeing as you are able to reproduce the issue.

@dscho dscho mentioned this pull request Jul 9, 2015
@linquize
Copy link

What things are outstanding in this pull request?

@dscho
Copy link
Member

dscho commented Jul 10, 2015

@linquize pretty much everything: the patch is very, very large, and it appears that all that is needed is a grace period of a couple of seconds after closing the socket before exiting. If that is true -- and I have not been able to verify this myself, I do not even have a way to reproduce here -- then a substantially smaller patch should do the job (and be much easier to review).

@dscho
Copy link
Member

dscho commented Aug 22, 2015

I am afraid that I have to close this ticket because

  1. I cannot reproduce it, so
  2. I cannot verify it, and
  3. I am really, really convinced that a much smaller patch can fix the same issue, and finally,
  4. Git for Windows 1.x has been retired, in favor of Git for Windows 2.x

I will try to find some time to patch the client so it waits a couple of seconds before reading each chunk of data; hopefully I will get an MCVE that way. If I can, I might take a crack at solving this issue myself.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants