Re: [PATCH] mingw: make stderr unbuffered again
Johannes Schindelinwrites: > ... there is a different problem here: you stated very explicitly > that it is okay for `pu` to be broken [*1*]. If it were different, if any > breakage would imply that a fix is really, really required lest the patch > series be evicted from `pu`, we could easily modify my Continuous Testing > setup to report more visibly what is broken. I think you misread me. Regarding 'pu', there is a chicken-and-egg problem involved. Most of the days, the tip of 'pu' passes the test at least for me. At the end of day's integration cycle, I may find that 'pu' does not pass the test for me [*1*]. It is not unusual to see new multiple topics come to 'pu', each of which may test well in isolation but have hidden interactions with topics already in 'next' or 'pu' that are exposed only when merged to 'pu', and it may be too late in the day for me to find which one of these new topics is problematic. I could choose not to merge any of them and push 'pu' out with no new topics. Or I may choose to push it out anyway, so that other people who are working during the remainder of 24-hour in different timezones can notice and find which new topic is broken [*2*]. And that is why I explicitly say that 'pu' may not even build. It shouldn't be taken as a discouragement to people from looking at it, which seems to be how I read you to misinterpret it. It is the opposite---a breakage at the tip of 'pu' is an invitation for people to help. If nobody steps up and says "the tip of 'pu' does not build and that is because of this topic", I'd be irritated enough to find which topic is broken myself and then ask the author of the offencing topic for help. If the topic is left broken after that, it will be ejected from 'pu', because I cannot use 'pu' for helping to polish other new topics that wants to cook there unless I do so. Of course there are platform- or environment-dependent breakages that would not cause my tests to fail [*3*]. Again, people with different environment can step up to help, if these topics are included in 'pu'. You would recall that you called out one topic [*4*] that did not build in your environment and the topic was ejected after your message (but not before---there wasn't a way for Window-less folks to tell that it was broken IIRC) from 'pu'. In a near-by thread, somebody wondered if we can add an integration branch 'pr' that is beyond 'pu' and includes everything ever posted to the list, and I explained why I won't have time for that. But I think the spirit of suggested 'pr' is what 'pu' already is---it is a collection of promising topics that can be merged into a seemingly consistent whole, which may or may not build and the purpose of the branch is to contain them in one place so that people can find which ones need unbreaking. Testing the tip of it alone and complaining that it is broken does not help much, but figuiring out which topic merged to it is broken does, by unblocking other topics in 'pu'. [Footnote] *1* If any other integration branch (including 'jch', which is somewhere between 'master' and 'pu' and beyond the commit with a tree that matches 'next' aka "pu^{/^### match next}") does not build, I pull overtime ;-) *2* When there is only a new topic that breaks 'pu' for me, it is easy to decide not to queue it and send a note to the author of the offending topic, and that does happen a lot, but not always. *3* I do not have p4 and I may not have svn installed so my tests may not even cover them. *4* I think it was nd/worktree-move but may have been another topic.
Re: [PATCH] mingw: make stderr unbuffered again
Hi Junio, On Wed, 15 Feb 2017, Junio C Hamano wrote: > Junio C Hamanowrites: > > > Johannes Schindelin writes: > > > >> FWIW I wish it were different, that git.git's `master` reflected more > >> closely what the current Git for Windows version has. > > > > Well, we two wishing the same thing together without doing anything > > else would make it happen. > > ehh, would *not* make it happen, of course. That is the reason why set aside a substantial part of my maintainer time to upstream those patches. You may not see how much time it costs me, say, to get the putty-w-args stuff upstream, but rest assured that I would not be able to do this were it not for a company paying me to do exactly that. > > As an experiment to see if our process can be improved, I've been > > meaning to suggest (which is what was behind my "question at a bit > > higher level" to Hannes [*1*]) asking you to throw me occasional > > pull requests for changes that are only about Windows specific > > issues, bypassing "patches on the list" for things like a hotfix to > > js/mingw-isatty [*2*] and use of OpenSSL SHA-1 on Windows [*3*], > > essentially treating Windows specific changes as "a sub-maintainer > > makes pull requests" we already do with Paul, Eric and Pat. The problem here is that Git for Windows is not a subsystem. It touches pretty much all of Git. That is very different from the Tcl/Tk code, or from git-svn (whose code is not shared by anything else in Git's source code, despite the fact that it is written in Perl). And even if you were to accept the occasional Pull Request from me, it would not solve the even bigger problem that we essentially reject so much expertise out there, so many potential contributions by very competent developers who just have no desire to fight the contribution process. > While this may ease the flow of upstreaming windows specific > changes, we need a separate thing to address the on-going issue you > raised in your message. A Windows-less person would not know his ... or her... > change to a generic code that is innocuous-looking has fallouts on > Windows (read this sentence with "Windows" replaced with any > specific platform name). When somebody writes c == '/' that should > have been written as is_dir_sep(c), you or Hannes often finds it > during the review here, and after repeatedly seeing such reviews, > that (slowly) rubs off on other Window-less folks. A new code may > still hit 'next' and 'master' with such an issue if it goes > unnoticed during the review. Apart from the fact that we have no prayer at coming up with a system that keeps track of open issues (because we do not use any tracker, but instead rely on people to remember that some thing still may not have been addressed), there is a different problem here: you stated very explicitly that it is okay for `pu` to be broken [*1*]. If it were different, if any breakage would imply that a fix is really, really required lest the patch series be evicted from `pu`, we could easily modify my Continuous Testing setup to report more visibly what is broken. But since it is okay for `pu` to be broken, that would just annoy everybody and people would learn to ignore/procmail those reports. > The CI you are setting up [*1*] which *1*? And... I already set it up. I just did not bother to make it more public because the builds were broken more often than not. IIRC the entire month of October was a solid red. > may certainly be a step in the good direction. Having more people like > Hannes working off of upstream may also be a great way to help the > "forget 'next' and upstream in general" issue. Any other ideas? The Continuous Integration is actually not so much a Continuous Integration as it is a Continuous Testing. If it became more of a CI, that would certainly reduce the impression that Git's bleeding edge only ever works on Linux. Ciao, Johannes Footnote *1*: You probably read between the lines that this is unfortunate, in my opinion. It sets the mood that lets experimental (if useful) features such as worktrees be broken for the better part of a year.
Re: [PATCH] mingw: make stderr unbuffered again
Johannes Sixtwrites: > Am 16.02.2017 um 18:10 schrieb Johannes Schindelin: >> On Wed, 15 Feb 2017, Johannes Sixt wrote: >>> I do not see how dup2() causes a change in stdio state. I >>> must be missing something (and that may be a basic misunderstanding of how >>> stdio is initialized). >> >> It appears that dup2()ing fd 2 resets that stdio state. > > OK, thanks. It's surprising, but so be it. > > Thank you for the quick fix! I am happy to see two Windows folks being happy. I tentatively marked this as "undecided" after merging it to 'next', but let's have it in the -rc2 and final. Two people familiar with Windows agreeing that there is no longer mystery why it fixes, after seeing that the update fixes a regression, is a strong enough sign to me that including it is better than leaving it out. Thanks.
Re: [PATCH] mingw: make stderr unbuffered again
Am 16.02.2017 um 18:10 schrieb Johannes Schindelin: On Wed, 15 Feb 2017, Johannes Sixt wrote: I do not see how dup2() causes a change in stdio state. I must be missing something (and that may be a basic misunderstanding of how stdio is initialized). It appears that dup2()ing fd 2 resets that stdio state. OK, thanks. It's surprising, but so be it. Thank you for the quick fix! -- Hannes
Re: [PATCH] mingw: make stderr unbuffered again
Hi Hannes, On Wed, 15 Feb 2017, Johannes Sixt wrote: > Am 15.02.2017 um 13:32 schrieb Johannes Schindelin: > > On Tue, 14 Feb 2017, Johannes Sixt wrote: > > > Am 14.02.2017 um 15:47 schrieb Johannes Schindelin: > > > > On Mon, 13 Feb 2017, Junio C Hamano wrote: > > > > > Johannes Schindelinwrites: > > > > > > What we forgot was to mark stderr as unbuffered again. > > > > > > I do not see how the earlier patch turned stderr from unbuffered to > > > buffered, as it did not add or remove any setvbuf() call. Can you > > > explain? > > > > [ motivation and history of js/mingw-isatty snipped ] > > > > So instead of "bending" the target HANDLE of the existing > > stdout/stderr (which would *naturally* have kept the > > buffered/unbuffered nature as-is), we now redirect with correct API > > calls. > > Your statement implies that at the time when winansi_init() begins, > stdio is already initialized and the buffered/unbuffered state has been > set for stderr. I would think that this is true. > > Then we swap out the file handle underlying stderr in swap_osfhnd() > using dup2(). Why would that change the buffered state of stdio? The file handle we swap in for stderr points to the pipe that a freshly-started thread consumes for parsing the ANSI color sequences. This handle is used both for stdout and stderr. The dup2() call then implicitly reopens stderr, with the default buffering. > > And the patch I provided at the bottom of this mail thread reinstates > > the unbuffered nature of stderr now that it gets reopened. > > > > Hopefully that makes it clear why the setvbuf() call is required now, > > but was previously unnecessary? > > Unfortunately, no. I do not see how dup2() causes a change in stdio state. I > must be missing something (and that may be a basic misunderstanding of how > stdio is initialized). It appears that dup2()ing fd 2 resets that stdio state. Ciao, Dscho
Re: [PATCH] mingw: make stderr unbuffered again
Junio C Hamanowrites: > Johannes Schindelin writes: > >> FWIW I wish it were different, that git.git's `master` reflected more >> closely what the current Git for Windows version has. > > Well, we two wishing the same thing together without doing anything > else would make it happen. ehh, would *not* make it happen, of course. > As an experiment to see if our process can be improved, I've been > meaning to suggest (which is what was behind my "question at a bit > higher level" to Hannes [*1*]) asking you to throw me occasional > pull requests for changes that are only about Windows specific > issues, bypassing "patches on the list" for things like a hotfix to > js/mingw-isatty [*2*] and use of OpenSSL SHA-1 on Windows [*3*], > essentially treating Windows specific changes as "a sub-maintainer > makes pull requests" we already do with Paul, Eric and Pat. While this may ease the flow of upstreaming windows specific changes, we need a separate thing to address the on-going issue you raised in your message. A Windows-less person would not know his change to a generic code that is innocuous-looking has fallouts on Windows (read this sentence with "Windows" replaced with any specific platform name). When somebody writes c == '/' that should have been written as is_dir_sep(c), you or Hannes often finds it during the review here, and after repeatedly seeing such reviews, that (slowly) rubs off on other Window-less folks. A new code may still hit 'next' and 'master' with such an issue if it goes unnoticed during the review. The CI you are setting up [*1*] may certainly be a step in the good direction. Having more people like Hannes working off of upstream may also be a great way to help the "forget 'next' and upstream in general" issue. Any other ideas?
Re: [PATCH] mingw: make stderr unbuffered again
Johannes Schindelinwrites: > Hi Junio, > ... > The hat of a person who sees how patches are reviewed before they enter > pu/next/master/maint of git.git. > ... > make sense. This makes my life harder, but I believe that the alternative > would be *not* to have those patches in git.git *at all*. You wrote a lot, what you wrote were readable, and perhaps were good material to support your answer/conclusion, but you forgot to answer a simple question I asked. I think your description of where things currently are would support any of the possible answers below, and I cannot guess which one it would be. The question was: Is our position "unless you are extremely competent and are willing to cherry-pick missing things from Git for Windows tree as needed, we recommend you to build Git for Windows source instead"? Or is it "please ignore upstream and work off of Git for Windows?" Or is it "please try to work with upstream and if you find Windows specific glitches, after checking to see if Git for Windows has already a fix for it and otherwise feed your fix to Git for Windows, so that we can upstream your fix for you, together with changes from others"? Or is it "please try to work with upstream and if you find Windows specific glitches, after checking to see if Git for Windows has already a fix for it and otherwise upstream your fix, so that next pull from upstream into Git for Windows would have your fix"? Or is it something else? > FWIW I wish it were different, that git.git's `master` reflected more > closely what the current Git for Windows version has. Well, we two wishing the same thing together without doing anything else would make it happen. As an experiment to see if our process can be improved, I've been meaning to suggest (which is what was behind my "question at a bit higher level" to Hannes [*1*]) asking you to throw me occasional pull requests for changes that are only about Windows specific issues, bypassing "patches on the list" for things like a hotfix to js/mingw-isatty [*2*] and use of OpenSSL SHA-1 on Windows [*3*], essentially treating Windows specific changes as "a sub-maintainer makes pull requests" we already do with Paul, Eric and Pat. Interested parties would instead see only the pull request sent by you to me, either on the list or directly CC'ed to them, and do their own fetch to assess if it is a good idea for me to actually pull. Suggestions to the changes from bystanders like Peff's comment [*4*] may need to reproduce the patch text when sent to the list, which would burden reviewers a bit more, but they still are possible. Such a pull-request workflow would either hide the communication from the list or force people to go to multiple places (i.e. both to the list and to GitHub comments) in order to view the whole picture, and I am still reluctant to extend it to other areas (e.g. a change that adds "#if WINDOWS" to a more generic codepath), but a trial on a limited scope may give us a better feel of how well such an updated workflow would work for us. [References] *1* *2* *3* <6a29f8c60d315a24292c1fa9f5e84df4dfdbf813.1486679254.git.johannes.schinde...@gmx.de> *4* <20170210050237.gajicliueuvk6...@sigill.intra.peff.net>
Re: [PATCH] mingw: make stderr unbuffered again
Am 15.02.2017 um 13:32 schrieb Johannes Schindelin: On Tue, 14 Feb 2017, Johannes Sixt wrote: Am 14.02.2017 um 15:47 schrieb Johannes Schindelin: On Mon, 13 Feb 2017, Junio C Hamano wrote: Johannes Schindelinwrites: What we forgot was to mark stderr as unbuffered again. I do not see how the earlier patch turned stderr from unbuffered to buffered, as it did not add or remove any setvbuf() call. Can you explain? [ motivation and history of js/mingw-isatty snipped ] So instead of "bending" the target HANDLE of the existing stdout/stderr (which would *naturally* have kept the buffered/unbuffered nature as-is), we now redirect with correct API calls. Your statement implies that at the time when winansi_init() begins, stdio is already initialized and the buffered/unbuffered state has been set for stderr. I would think that this is true. Then we swap out the file handle underlying stderr in swap_osfhnd() using dup2(). Why would that change the buffered state of stdio? And the patch I provided at the bottom of this mail thread reinstates the unbuffered nature of stderr now that it gets reopened. Hopefully that makes it clear why the setvbuf() call is required now, but was previously unnecessary? Unfortunately, no. I do not see how dup2() causes a change in stdio state. I must be missing something (and that may be a basic misunderstanding of how stdio is initialized). -- Hannes
Re: [PATCH] mingw: make stderr unbuffered again
Hi Junio, On Tue, 14 Feb 2017, Junio C Hamano wrote: > Johannes Schindelinwrites: > > >> OK. Should this go directly to 'master', as the isatty thing is > >> already in? > > > > From my point of view, it is not crucial. The next Git for Windows > > version will have it, of course, and Hannes is always running with his > > set of patches, he can easily cherry-pick this one, too. > > What hat were you wearing when you said the above "From my point of > view"? The hat of a person who sees how patches are reviewed before they enter pu/next/master/maint of git.git. If that review had anything to do with Windows, and refused to accept patches unless they work correctly on Windows, I would agree that it is a wise idea to fast-track important fixes for Windows. But that is not the case. Quite often `pu`, sometimes `next`, and rarely even `master` are regularly broken on Windows. The only branch that is tested very stringently on Windows, and into which nothing is allowed that breaks on Windows, is git-for-windows/git's `master` branch. BTW this is not just an opinion, this is just an account of the current state. Once you accept that this is reality, you will understand why I *dared* to say that a criticial Windows-specific fix needs to be fast-tracked to git-for-windows/git's `master`, but not into git.git's `master` branch. FWIW I wish it were different, that git.git's `master` reflected more closely what the current Git for Windows version has. If you are attentive, you will have noticed that I continuously work toward that goal. I frequently "upstream patches" from Git for Windows[*1*], even if it seems that the influx of new patches is higher than the rate of patches that make it into git.git's `master`. And even if I am often asked to change these patches so much that it is virtually guaranteed that they regress (hence my recently increasing reluctance to accept each and every reviewer's suggestions as "must implement"). Ciao, Johannes Footnote *1*: Yes, I even "upstream patches" from developers other than myself, trying to shield contributors from having to send their patches as mails and to cope with the reviewers' suggestions that may, or may not, make sense. This makes my life harder, but I believe that the alternative would be *not* to have those patches in git.git *at all*.
Re: [PATCH] mingw: make stderr unbuffered again
Hi Hannes, On Tue, 14 Feb 2017, Johannes Sixt wrote: > Am 14.02.2017 um 15:47 schrieb Johannes Schindelin: > > On Mon, 13 Feb 2017, Junio C Hamano wrote: > > > Johannes Schindelinwrites: > > > > When removing the hack for isatty(), we actually removed more than > > > > just an isatty() hack: we removed the hack where internal data > > > > structures of the MSVC runtime are modified in order to redirect > > > > stdout/stderr. > > > > > > > > Instead of using that hack (that does not work with newer versions > > > > of the runtime, anyway), we replaced it by reopening the > > > > respective file descriptors. > > > > > > > > What we forgot was to mark stderr as unbuffered again. > > I do not see how the earlier patch turned stderr from unbuffered to > buffered, as it did not add or remove any setvbuf() call. Can you > explain? Certainly. I hoped that the commit message would do the job, but then, I am under time pressure these days, so it was probably a bit terse. The hack we used to make isatty() work used to change data structures directly that are *internal* to the MSVC runtime. That is, instead of *really* redirecting stdout/stderr, we simply changed the target of the existing stdout/stderr and thereby could fool MSVC into believing that it is *still* writing to the terminal (because the bit is set) and that it is *not* a pipe (because we forcibly unset that bit). Needless to say, this meddling with internal data structures is not only prone to break with future updates of the MSVC runtime, it is also inappropriate because the implementation may rely on certain side effects (or not so side effects) that may very well cause crashes or data loss. Imagine, for example, that the internal data structure were variable-size, based on the HANDLE type. That is totally legitimate for an internal data structure. And if we meddle with the HANDLE, we can easily cause data corruption. As GCC is basically tied to using an older version of the MSVC runtime (unlike GLIBC, multiple versions of the MSVC runtime can coexist happily, making it relatively easy to maintain backwards-compatibility, but that concept is foreign to GCC), this used to not be a problem. However, with our recent push to allow developing, building, debugging and performance profiling in Visual Studio, that limitation no longer holds true: if you develop with Visual Studio 2015, you will link to a newer MSVC runtime, and that runtime changed those internal data structures rather dramatically. That means that we had to come up with a non-hacky way to redirect stdout/stderr (e.g. to parse ESC color sequences and apply them to the Win32 Console as appropriate) and still have isatty() report what we want it to report. That is, if we redirect stdout/stderr to a pipe that parses the color sequences for use in the Win32 Console, we want isatty() to report that we are writing to a Terminal Typewriter (a charming anachronism, isn't it?). My colleague Jeff Hostetler worked on this and figured out that the only way to do this properly is to wrap isatty() and override it via a #define, and simply remember when stdout/stderr referred to a terminal before redirecting, say, to that pipe. As my famously broken patch to override isatty() (so that /dev/null would not be treated as a terminal) *already* overrode isatty() with a custom wrapper, and as it was broken and needed fixing, I decided to reconcile the two approaches into what is now the version in `master`. So instead of "bending" the target HANDLE of the existing stdout/stderr (which would *naturally* have kept the buffered/unbuffered nature as-is), we now redirect with correct API calls. And the patch I provided at the bottom of this mail thread reinstates the unbuffered nature of stderr now that it gets reopened. Hopefully that makes it clear why the setvbuf() call is required now, but was previously unnecessary? Ciao, Dscho
Re: [PATCH] mingw: make stderr unbuffered again
Johannes Sixtwrites: > Am 14.02.2017 um 15:47 schrieb Johannes Schindelin: > >>>From my point of view, it is not crucial. The next Git for Windows version >> will have it, of course, and Hannes is always running with his set of >> patches, he can easily cherry-pick this one, too. > > The patch fixes the problem for me here. > > I am a bit hesitant to call it "not crucial": The symptom I observed > is certainly not a show stopper; but who knows where else it is > important that stderr goes to the terminal earlier than other > output. As it is a new regression, it should be included in the next > release. I tend to agree with you on the "not crucial" bit. I applied the patch with "Tested-by: Johannes Sixt " on top of 1d3f065e0e ("mingw: follow-up to "replace isatty() hack"", 2017-01-18), which was the tip of js/mingw-isatty topic that was merged to both master and maint during this cycle. Unless I hear the "fix" turns out to be undesirable in the coming few days, let's plan to merge it to 'master' by the final. Thanks.
Re: [PATCH] mingw: make stderr unbuffered again
Johannes Schindelinwrites: >> OK. Should this go directly to 'master', as the isatty thing is >> already in? > > From my point of view, it is not crucial. The next Git for Windows version > will have it, of course, and Hannes is always running with his set of > patches, he can easily cherry-pick this one, too. What hat were you wearing when you said the above "From my point of view"? Were you the "Git for Windows" maintainer, or were you a contributor and a member of the Git development community that works to improve the upstream? If it was not clear, I was asking the question to you wearing the latter hat. To put it differently, what is our position, as the upstream developers, toward those who are on Windows and wants to build from the source? It's not just Hannes. Is our position "unless you are extremely competent and are willing to cherry-pick missing things from Git for Windows tree as needed, we recommend you to build Git for Windows source instead"? It is inevitable that the upstream lags behind in Windows specific changes. Even though you have been trickling Windows specific changes (both things in compat/ and also changes to the generic parts, updating "c == '/'" to "is_dir_sep(c)") in, and I have been accepting them for the past few years, in order to reduce the size of the patch pile Git for Windows needs on top of the upstream, until the patch pile becomes empty, it will always be the case. So I won't object if that were our position. I just need to know what it is to adjust my expectations, and as far as I am concerned, you and Hannes are the two people whose thinking I'd trust regarding what to do with/to Windows users; even though I keep saying "our" position, I am asking yours and Hannes's. Thanks.
Re: [PATCH] mingw: make stderr unbuffered again
Am 14.02.2017 um 15:47 schrieb Johannes Schindelin: On Mon, 13 Feb 2017, Junio C Hamano wrote: Johannes Schindelinwrites: When removing the hack for isatty(), we actually removed more than just an isatty() hack: we removed the hack where internal data structures of the MSVC runtime are modified in order to redirect stdout/stderr. Instead of using that hack (that does not work with newer versions of the runtime, anyway), we replaced it by reopening the respective file descriptors. What we forgot was to mark stderr as unbuffered again. I do not see how the earlier patch turned stderr from unbuffered to buffered, as it did not add or remove any setvbuf() call. Can you explain? OK. Should this go directly to 'master', as the isatty thing is already in? From my point of view, it is not crucial. The next Git for Windows version will have it, of course, and Hannes is always running with his set of patches, he can easily cherry-pick this one, too. The patch fixes the problem for me here. I am a bit hesitant to call it "not crucial": The symptom I observed is certainly not a show stopper; but who knows where else it is important that stderr goes to the terminal earlier than other output. As it is a new regression, it should be included in the next release. Thanks, -- Hannes
Re: [PATCH] mingw: make stderr unbuffered again
Hi Junio, On Mon, 13 Feb 2017, Junio C Hamano wrote: > Johannes Schindelinwrites: > > > When removing the hack for isatty(), we actually removed more than just > > an isatty() hack: we removed the hack where internal data structures of > > the MSVC runtime are modified in order to redirect stdout/stderr. > > > > Instead of using that hack (that does not work with newer versions of > > the runtime, anyway), we replaced it by reopening the respective file > > descriptors. > > > > What we forgot was to mark stderr as unbuffered again. > > > > Reported by Hannes Sixt. Fixed with Jeff Hostetler's assistance. > > > > Signed-off-by: Johannes Schindelin > > --- > > Published-As: > > https://github.com/dscho/git/releases/tag/mingw-unbuffered-stderr-v1 > > Fetch-It-Via: git fetch https://github.com/dscho/git > > mingw-unbuffered-stderr-v1 > > OK. Should this go directly to 'master', as the isatty thing is > already in? >From my point of view, it is not crucial. The next Git for Windows version will have it, of course, and Hannes is always running with his set of patches, he can easily cherry-pick this one, too. Ciao, Johannes P.S.: Could you please cut the remainder of the mail that you are not responding to? Thanks.
Re: [PATCH] mingw: make stderr unbuffered again
Johannes Schindelinwrites: > When removing the hack for isatty(), we actually removed more than just > an isatty() hack: we removed the hack where internal data structures of > the MSVC runtime are modified in order to redirect stdout/stderr. > > Instead of using that hack (that does not work with newer versions of > the runtime, anyway), we replaced it by reopening the respective file > descriptors. > > What we forgot was to mark stderr as unbuffered again. > > Reported by Hannes Sixt. Fixed with Jeff Hostetler's assistance. > > Signed-off-by: Johannes Schindelin > --- > Published-As: > https://github.com/dscho/git/releases/tag/mingw-unbuffered-stderr-v1 > Fetch-It-Via: git fetch https://github.com/dscho/git > mingw-unbuffered-stderr-v1 OK. Should this go directly to 'master', as the isatty thing is already in? > > compat/winansi.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/compat/winansi.c b/compat/winansi.c > index 82b89ab1376..793420f9d0d 100644 > --- a/compat/winansi.c > +++ b/compat/winansi.c > @@ -510,6 +510,8 @@ static HANDLE swap_osfhnd(int fd, HANDLE new_handle) >*/ > close(new_fd); > > + if (fd == 2) > + setvbuf(stderr, NULL, _IONBF, BUFSIZ); > fd_is_interactive[fd] |= FD_SWAPPED; > > return duplicate; > @@ -547,6 +549,8 @@ static void detect_msys_tty(int fd) > !wcsstr(name, L"-pty")) > return; > > + if (fd == 2) > + setvbuf(stderr, NULL, _IONBF, BUFSIZ); > fd_is_interactive[fd] |= FD_MSYS; > } > > > base-commit: 5588dbffbd61e4906e453808c6ad32f792fea521
[PATCH] mingw: make stderr unbuffered again
When removing the hack for isatty(), we actually removed more than just an isatty() hack: we removed the hack where internal data structures of the MSVC runtime are modified in order to redirect stdout/stderr. Instead of using that hack (that does not work with newer versions of the runtime, anyway), we replaced it by reopening the respective file descriptors. What we forgot was to mark stderr as unbuffered again. Reported by Hannes Sixt. Fixed with Jeff Hostetler's assistance. Signed-off-by: Johannes Schindelin--- Published-As: https://github.com/dscho/git/releases/tag/mingw-unbuffered-stderr-v1 Fetch-It-Via: git fetch https://github.com/dscho/git mingw-unbuffered-stderr-v1 compat/winansi.c | 4 1 file changed, 4 insertions(+) diff --git a/compat/winansi.c b/compat/winansi.c index 82b89ab1376..793420f9d0d 100644 --- a/compat/winansi.c +++ b/compat/winansi.c @@ -510,6 +510,8 @@ static HANDLE swap_osfhnd(int fd, HANDLE new_handle) */ close(new_fd); + if (fd == 2) + setvbuf(stderr, NULL, _IONBF, BUFSIZ); fd_is_interactive[fd] |= FD_SWAPPED; return duplicate; @@ -547,6 +549,8 @@ static void detect_msys_tty(int fd) !wcsstr(name, L"-pty")) return; + if (fd == 2) + setvbuf(stderr, NULL, _IONBF, BUFSIZ); fd_is_interactive[fd] |= FD_MSYS; } base-commit: 5588dbffbd61e4906e453808c6ad32f792fea521 -- 2.11.1.windows.1