Re: Patch from git-for-windows for SSH hangs
Hi Jeremy, hi Johannes, On Feb 24 13:22, Jeremy Drake via Cygwin-patches wrote: > This patch exists in the git-for-windows fork of msys2-runtime (which is > itself a fork of cygwin). There have been complaints and requests to > apply this patch to msys2-runtime (such as > https://github.com/msys2/MSYS2-packages/issues/4962), but it makes more > sense to me to try to figure this out upstream, so everyone can benefit. > I did not write the patch, nor do I personally encounter the bug it is > intended to fix, so I can't really advocate for its approach, but the > commit message is pretty detailed as to the investigation that led to it. > > This is the original patch from > https://github.com/git-for-windows/msys2-runtime/pull/75, if necessary I > can rebase it on cygwin's master branch, it does so cleanly (or you can > try git am -3, that tends to work for me in cases where straight git am > does not). It applies cleanly. I just wonder why Johannes hasn't sent this already upstream, given the patch is from October. *puzzled* > >From cbe555e054cefeccd65250bb11dc56f82196301f Mon Sep 17 00:00:00 2001 > From: Johannes Schindelin > Date: Thu, 10 Oct 2024 19:52:47 +0200 > Subject: [PATCH] Fix SSH hangs > > It was reported in https://github.com/git-for-windows/git/issues/5199 > that as of v3.5.4, cloning or fetching via SSH is hanging indefinitely. > > Bisecting the problem points to 555afcb2f3 (Cygwin: select: set pipe > writable only if PIPE_BUF bytes left, 2024-08-18). That commit's > intention seems to look at the write buffer, and only report the pipe as > writable if there are more than one page (4kB) available. > > However, the number that is looked up is the number of bytes that are > already in the buffer, ready to be read, and further analysis > shows that in the scenario described in the report, the number of > available bytes is substantially below `PIPE_BUF`, but as long as they > are not handled, there is apparently a dead-lock. > > Since the old logic worked, and the new logic causes a dead-lock, let's > essentially revert 555afcb2f3 (Cygwin: select: set pipe writable only if > PIPE_BUF bytes left, 2024-08-18). > > Note: This is not a straight revert, as the code in question has been > modified subsequently, and trying to revert the original commit would > cause merge conflicts. Therefore, the diff looks very different from the > reverse diff of the commit whose logic is reverted. Ok, so the patch shows that reporting writable from select only if PIPE_BUF bytes are free is not working as desired. Too bad. The patch is just kind of incomplete. pipe_data_available() returns PIPE_BUF in case the actual available bytes can't be evaluated. However, it only does so if called from select(). If called from elsewhere, it returns 1. If select() now always returns writability if at least 1 byte is available, there's no reason left to special case being called from select(), because it's now sufficient to return 1 to select() as well. I added a second patch on top to drop the PDA_SELECT handling and pushed both. Thanks, Corinna
[PATCH v2] Cygwin: Add spawn family of functions to docs
In the doc tree, add a new section "Other system interfaces[...]" that lists the spawn family of functions, most of the exposed cygwin internal functions that a user might have use for, and some other functions duplicating Windows or DOS interfaces that might have some utility. --- winsup/doc/posix.xml | 35 +++ 1 file changed, 35 insertions(+) diff --git a/winsup/doc/posix.xml b/winsup/doc/posix.xml index 43e860b0d..b9443eaae 100644 --- a/winsup/doc/posix.xml +++ b/winsup/doc/posix.xml @@ -1762,6 +1762,41 @@ ISO®/IEC DIS 9945 Information technology +Other system interfaces, some from Windows: + + +_alloca(Windows) +_feinitialise +_get_osfhandle (Windows) +_pipe (Windows) +_setmode (Windows) +cwait (Windows) +cygwin_attach_handle_to_fd +cygwin_conv_path +cygwin_conv_path_list +cygwin_create_path +cygwin_detach_dll +cygwin_dll_init +cygwin_internal +cygwin_logon_user +cygwin_posix_path_list_p +cygwin_set_impersonation_token +cygwin_split_path +cygwin_stackdump +cygwin_umount +cygwin_winpid_to_pid +spawnl (Windows) +spawnle(Windows) +spawnlp(Windows) +spawnlpe (Windows) +spawnv (Windows) +spawnve(Windows) +spawnvp(Windows) +spawnvpe (Windows) + + + + Implementation Notes chroot only emulates a chroot function call -- 2.45.1
Re: [PATCH] Cygwin: Add spawn family of functions to docs
On Feb 24 17:51, Mark Geisert wrote: > Hi Corinna, > > On 2/24/2025 3:58 AM, Corinna Vinschen wrote: > > On Feb 17 11:22, Corinna Vinschen wrote: > > > On Feb 16 13:46, Mark Geisert wrote: > > > > In the doc tree, change the title of section "Other UNIX system > > > > interfaces..." to "Other system interfaces...". Add the spawn family of > > > > functions noting their origin as Windows. > [...] > > > > Actually, Jon raised some reservations against adding historical > > msvcrt functions to the set of documented POSIX functions on the > > IRC channel. > > > > We also have functions like _get_osfhandle and stuff like that. > > Do we really want them documented in the list of POSIXy functions? > > I didn't see Jon's comments unless the "1999" reference covered them ;-). > > I have no issue with the Windows-derived functions going on a separate list. > I only suggested the UNIX-* list because of the small number of > Windows-derived functions being added. > > BTW The MSDN documentation of the spawn family of functions has their names > all starting with an underscore character. Should we follow that or not? We don't export them with underscore. > On the question of documenting these funcs at all (was that being raised?), > I don't feel very strongly about it. Maybe it would save one out of ten > posts asking why our POSIX environment doesn't do this Windows thing? /s Yeah, good point. > If the final decision is to document in a separate list for the doc pages, I > can submit a revised patch for that. We could add a "Non-POSIX, non-UNIX Windowy functions added for one reason or another" kind of section to the end, I guess. Even the section name isn't quite clear to me. It's kind of "Cygwin-only", and that would then also include the stuff from sys/cygwin.h, i.e. cygwin_conv_path, cygwin_internal, ... Corinna
