Re: [PATCH] Cygwin: pty: Disable pseudo console if TERM is dumb or not set.
On Wed, 26 Aug 2020 19:36:06 +0200 Corinna Vinschen wrote: > On Aug 26 21:00, Takashi Yano via Cygwin-patches wrote: > > Pseudo console generates escape sequences on execution of non-cygwin > > apps. If the terminal does not support escape sequence, output will > > be garbled. This patch prevents garbled output in dumb terminal by > > disabling pseudo console. > > I'm a bit puzzled by this patch. We had code handling emacs and dumb > terminals explicitely in the early forms of the first incarnation of > the pseudo tty code, but fortunately you found a way to handle this > without hardcoding terminal types into Cygwin. Why do you think we > have to do this now? What previously disccussed was the problem that the clearing screen at pty startup displays garbage (^[[H^[[2J) in emacs. Finally, this was settled by eliminating clear-screen and triggering redraw-screen instead at the first execution of non-cygwin app. However, the problem reported in https://cygwin.com/pipermail/cygwin/2020-August/245983.html still remains. What's worse in the new implementation, pseudo console sends ESC[6n (querying cursor position) internally on startup and waits for a response. This causes hang if pseudo console is started in dumb terminal. This patch is for fixing this issue. -- Takashi Yano
[PATCH] Cygwin: console: Replace WriteConsoleA() with WriteConsoleW().
- To allow sending non-ASCII chars to console, all WriteConsoleA()
are replaced by WriteConsoleW().
Addresses:
https://cygwin.com/pipermail/cygwin-patches/2020q3/010476.html
---
winsup/cygwin/fhandler_console.cc | 89 ---
1 file changed, 47 insertions(+), 42 deletions(-)
diff --git a/winsup/cygwin/fhandler_console.cc
b/winsup/cygwin/fhandler_console.cc
index 02a5996a1..33e40a9f9 100644
--- a/winsup/cygwin/fhandler_console.cc
+++ b/winsup/cygwin/fhandler_console.cc
@@ -59,7 +59,7 @@ static struct fhandler_base::rabuf_t con_ra;
/* Write pending buffer for ESC sequence handling
in xterm compatible mode */
-static unsigned char last_char;
+static wchar_t last_char;
/* simple helper class to accumulate output in a buffer
and send that to the console on request: */
@@ -67,18 +67,20 @@ static class write_pending_buffer
{
private:
static const size_t WPBUF_LEN = 256u;
- unsigned char buf[WPBUF_LEN];
+ char buf[WPBUF_LEN];
size_t ixput;
public:
- inline void put (unsigned char x)
+ inline void put (char x)
{
if (ixput < WPBUF_LEN)
buf[ixput++] = x;
}
inline void empty () { ixput = 0u; }
- inline void send (HANDLE , DWORD *wn = NULL)
+ inline void send (HANDLE )
{
-WriteConsoleA (handle, buf, ixput, wn, 0);
+wchar_t bufw[WPBUF_LEN];
+DWORD len = sys_mbstowcs (bufw, WPBUF_LEN, buf, ixput);
+WriteConsoleW (handle, bufw, len, NULL, 0);
}
} wpbuf;
@@ -291,7 +293,7 @@ fhandler_console::request_xterm_mode_input (bool req)
dwMode |= ENABLE_VIRTUAL_TERMINAL_INPUT;
SetConsoleMode (get_handle (), dwMode);
if (con.cursor_key_app_mode) /* Restore DECCKM */
- WriteConsoleA (get_output_handle (), "\033[?1h", 5, NULL, 0);
+ WriteConsoleW (get_output_handle (), L"\033[?1h", 5, NULL, 0);
}
}
else
@@ -1793,6 +1795,9 @@ fhandler_console::write_console (PWCHAR buf, DWORD len,
DWORD& done)
if (buf[i] >= (unsigned char) '`' && buf[i] <= (unsigned char) '~')
buf[i] = __vt100_conv[buf[i] - (unsigned char) '`'];
+ if (len > 0)
+last_char = buf[len-1];
+
while (len > 0)
{
DWORD nbytes = len > MAX_WRITE_CHARS ? MAX_WRITE_CHARS : len;
@@ -2001,6 +2006,7 @@ fhandler_console::char_command (char c)
{
int x, y, n;
char buf[40];
+ wchar_t bufw[40];
int r, g, b;
if (wincap.has_con_24bit_colors () && !con_is_legacy)
@@ -2035,9 +2041,9 @@ fhandler_console::char_command (char c)
if (wincap.has_con_esc_rep ())
/* Just send the sequence */
wpbuf.send (get_output_handle ());
- else if (last_char && last_char != '\n')
+ else if (last_char && last_char != L'\n')
for (int i = 0; i < con.args[0]; i++)
- WriteConsoleA (get_output_handle (), _char, 1, 0, 0);
+ WriteConsoleW (get_output_handle (), _char, 1, 0, 0);
break;
case 'r': /* DECSTBM */
con.scroll_region.Top = con.args[0] ? con.args[0] - 1 : 0;
@@ -2058,25 +2064,25 @@ fhandler_console::char_command (char c)
{
/* Erase scroll down area */
n = con.args[0] ? : 1;
- __small_sprintf (buf, "\033[%d;1H\033[J\033[%d;%dH",
- srBottom - (n-1) - con.b.srWindow.Top + 1,
- y + 1 - con.b.srWindow.Top, x + 1);
- WriteConsoleA (get_output_handle (),
-buf, strlen (buf), 0, 0);
+ __small_swprintf (bufw, L"\033[%d;1H\033[J\033[%d;%dH",
+ srBottom - (n-1) - con.b.srWindow.Top + 1,
+ y + 1 - con.b.srWindow.Top, x + 1);
+ WriteConsoleW (get_output_handle (),
+bufw, wcslen (bufw), 0, 0);
}
- __small_sprintf (buf, "\033[%d;%dr",
- y + 1 - con.b.srWindow.Top,
- srBottom + 1 - con.b.srWindow.Top);
- WriteConsoleA (get_output_handle (), buf, strlen (buf), 0, 0);
+ __small_swprintf (bufw, L"\033[%d;%dr",
+ y + 1 - con.b.srWindow.Top,
+ srBottom + 1 - con.b.srWindow.Top);
+ WriteConsoleW (get_output_handle (), bufw, wcslen (bufw), 0, 0);
wpbuf.put ('T');
wpbuf.send (get_output_handle ());
- __small_sprintf (buf, "\033[%d;%dr",
- srTop + 1 - con.b.srWindow.Top,
- srBottom + 1 - con.b.srWindow.Top);
- WriteConsoleA (get_output_handle (), buf, strlen (buf), 0, 0);
- __small_sprintf (buf, "\033[%d;%dH",
- y + 1 - con.b.srWindow.Top, x + 1);
- WriteConsoleA (get_output_handle (), buf, strlen (buf), 0, 0);
+ __small_swprintf (bufw,
Re: [Patch] Fix incorrect code page when setting console title on Win10
On Wed, 26 Aug 2020 19:33:45 +0200 Corinna Vinschen wrote: > On Aug 26 09:30, Johannes Schindelin wrote: > > Hi Corinna, > > > > On Wed, 26 Aug 2020, Corinna Vinschen wrote: > > > > > On Aug 26 16:43, 宫大汉 via Cygwin-patches wrote: > > > > When Cygwin sets console titles on Win10 (has_con_24bit_colors > > > > !con_is_legacy), > > > > `WriteConsoleA` is used and causes an error if: > > > > 1. the environment variable of `LANG` is `***.UTF-8` > > > > 2. and the code page of console.exe is not UTF-8 > > > > 1. e.g. on my Computer, it's GB2312, for Chinese text > > > > > > > > > > > > I've done some tests on msys2 and details are on > > > > https://github.com/git-for-windows/git/issues/2738, > > > > and I filed a PR of > > > > https://github.com/git-for-windows/msys2-runtime/pull/25. > > > > Just in case you want to have a look at it, you can download the patch via > > https://github.com/git-for-windows/msys2-runtime/commit/334f52a53a2e6b7f560b0e8810b9f672ebb3ad24.patch > > > > FWIW my original reviewer comment was: "why not fix wpbuf.send() in the > > first place?" but after having a good look around, that method seemed to > > be called from so many places that I "got cold feet" of that approach. > > > > For one, I saw at least one caller that wants to send Escape sequences, > > and I have no idea whether it is a good idea to do that in the `*W()` > > version of the `WriteConsole()` function. > > Yes, it is. There's no good reason to use the A functions, actually. > They are just wrappers calling the W functions and WriteConsoleW > evaluates ESC sequences just fine (just given as UTF-16 chars). > > > So the real question from my side is: how to address properly the many > > uses of `WriteConsoleA()` (which breaks all kinds of encodings in many > > situations because Windows' idea of the current code page and Cygwin's > > idea of the current locale are pretty often at odds). > > > > The patch discussed here circumvents one of those call sites. > > > > However, even though there have not been any callers of `WriteConsoleA()` > > in Cygwin v3.0.7 (but four callers of `WriteConsoleW()` which I suspect > > were converted to `*A()` calls in v3.1.0 by the Pseudo Console patches), > > there are now a whopping 15 callers of that `*A()` function in Cygwin > > v3.1.7. See here: > > [...] > > That cannot be intentional, can it? We should always thrive to use the > > `*W()` functions so that we can be sure that the expected encoding is > > used, right? > > Takashi? Any reason to use WriteConsoleA rather than WriteConsoleW? If > at all, WriteConsoleA should only be used if it's 100% safe to assume > that the buffer only contains ASCII chars < 127. No. I just did not realize that the escapce sequence cound contain non-ASCII chars. I am sorry. I will submit a patch for that issue. -- Takashi Yano
[PATCH] Cygwin: fhandler_fifo::delete_client_handler: improve efficiency
Delete a client handler by swapping it with the last one in the list
instead of calling memmove.
---
winsup/cygwin/fhandler_fifo.cc | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index b3c4c4a25..75c8406fe 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -377,14 +377,14 @@ fhandler_fifo::add_client_handler (bool new_pipe_instance)
return 0;
}
-/* Always called with fifo_client_lock in place. */
+/* Always called with fifo_client_lock in place. Delete a
+ client_handler by swapping it with the last one in the list. */
void
fhandler_fifo::delete_client_handler (int i)
{
fc_handler[i].close ();
if (i < --nhandlers)
-memmove (fc_handler + i, fc_handler + i + 1,
-(nhandlers - i) * sizeof (fc_handler[i]));
+fc_handler[i] = fc_handler[nhandlers];
}
/* Delete handlers that we will never read from. Always called with
--
2.28.0
[PATCH 2/3] Cygwin: Add github action to cross-build on Fedora
This helps avoid unpleasant surprises when we come to actually make a
release (which are cross-built in this manner)
---
.github/workflows/cygwin.yml | 45
1 file changed, 45 insertions(+)
create mode 100644 .github/workflows/cygwin.yml
diff --git a/.github/workflows/cygwin.yml b/.github/workflows/cygwin.yml
new file mode 100644
index 0..cdad8e67b
--- /dev/null
+++ b/.github/workflows/cygwin.yml
@@ -0,0 +1,45 @@
+name: cygwin
+
+on: push
+
+jobs:
+ fedora-build:
+runs-on: ubuntu-latest
+container: fedora:latest
+strategy:
+ fail-fast: false
+ matrix:
+include:
+- target: x86_64-pc-cygwin
+ pkgarch: 64
+- target: i686-pc-cygwin
+ pkgarch: 32
+name: Fedora cross ${{ matrix.target }}
+
+steps:
+- uses: actions/checkout@v2
+
+# install build tools
+- run: dnf install -y make patch perl
+- run: dnf install -y mingw${{ matrix.pkgarch }}-gcc-c++ mingw${{
matrix.pkgarch }}-winpthreads-static mingw${{ matrix.pkgarch }}-zlib-static
+
+# cocom isn't packaged in Fedora, so we install from a 3rd party repo
+- run: dnf install -y
https://github.com/rpmsphere/noarch/raw/master/r/rpmsphere-release-$(rpm -E
%fedora)-1.noarch.rpm
+- run: dnf install -y cocom
+
+# install cross-cygwin toolchain and libs from copr
+- run: dnf install -y dnf-plugins-core
+- run: dnf copr enable -y yselkowitz/cygwin
+- run: dnf install -y cygwin${{ matrix.pkgarch }}-gcc-c++ cygwin${{
matrix.pkgarch }}-gettext cygwin${{ matrix.pkgarch }}-libbfd cygwin${{
matrix.pkgarch }}-libiconv cygwin${{ matrix.pkgarch }}-zlib
+
+# install doc tools
+- run: dnf install -y dblatex docbook2X docbook-xsl xmlto
+- run: dnf install -y python3 python3-lxml python3-ply
+
+# build
+- run: mkdir build install
+- run: cd build && ../configure --target=${{ matrix.target }}
--prefix=$(realpath $(pwd)/../install)
+- run: make -C build
+- run: make -C build/*/newlib info man
+- run: make -C build install
+- run: make -C build/*/newlib install-info install-man
--
2.28.0
[PATCH 3/3] Cygwin: Remove .drone.yml
tea-ci.org was a CI service for MSYS2, but is no longer operating.
---
.drone.yml | 58 --
1 file changed, 58 deletions(-)
delete mode 100644 .drone.yml
diff --git a/.drone.yml b/.drone.yml
deleted file mode 100644
index ad512a031..0
--- a/.drone.yml
+++ /dev/null
@@ -1,58 +0,0 @@
-# Build configuration for https://tea-ci.org
-# Tea CI is a fork of Drone CI with Cygwin/Msys2 support
-# Feel free to share Tea CI to more open source developers
-# https://docs.tea-ci.org/usage/overview/
-
-debug: true
-
-build:
- stage1:
-image: teaci/cygwin$$arch
-pull: true
-shell: cygwin$$arch
-commands:
- - uname -a
- - id
- - C:/cygwin-installer.exe --site http://mirrors.tea-ci.org/cygwin
--local-package-dir Z:/tmp/cygwin -W -P
gettext-devel,zlib-devel,libiconv,libiconv-devel,mingw64-i686-gcc-g++,mingw64-i686-zlib,mingw64-x86_64-gcc-core,mingw64-x86_64-gcc-g++,mingw64-x86_64-zlib,dejagnu,dblatex,docbook-xml45,docbook-xsl,xmlto
-q &> /dev/null
- - srcdir=`pwd`
- - builddir=/oss/build-stage1
- - installdir=/oss/install-stage1
- - mkdir -p ${builddir} ${installdir}
- - cd ${builddir}
- - ${srcdir}/configure --prefix=${installdir} -v
- - make
- - make install
- - sha1sum ${installdir}/bin/cygwin1.dll /bin/cygwin1.dll
- # FIXME: Is there an easy way to package new Cygwin then install locally
using setup_x86{,-64}.exe?
- - cp -vf ${installdir}/bin/cygwin1.dll /bin/cygwin1.dll
-
- test:
-image: teaci/cygwin$$arch
-pull: true
-shell: /bin/bash # Call from Linux native shell, which is a special bonus
of Tea CI.
-commands:
- # In the worst case, new cygwin1.dll might fail to start with exit
status 0, which fools the CI as status success.
- # The following test does not rely on cygwin exit status, instead we
trust Linux grep result.
- - cygwin$$arch -c "uname -a" | grep CYGWIN_NT
- - cygwin$$arch -c "id" | grep uid
- - cygwin$$arch -c "sha1sum --tag /bin/cygwin1.dll" | grep SHA1
-
- # Compile Cygwin again using the new cygwin1.dll
- stage2:
-image: teaci/cygwin$$arch
-pull: true
-shell: cygwin$$arch
-commands:
- - srcdir=`pwd`
- - builddir=/oss/build-stage2
- - installdir=/oss/install-stage2
- - mkdir -p ${builddir} ${installdir}
- - cd ${builddir}
- - ${srcdir}/configure --prefix=${installdir} -v
- - make
- - make install
-
-matrix:
- arch:
-- 64
-- 32
--
2.28.0
[PATCH 1/3] Cygwin: Add .appveyor.yml
This is a slightly more polished version of the configuration being used
for CI builds at https://ci.appveyor.com/project/cygwin/cygwin, which is
not currently under version control.
---
.appveyor.yml | 69 +++
1 file changed, 69 insertions(+)
create mode 100644 .appveyor.yml
diff --git a/.appveyor.yml b/.appveyor.yml
new file mode 100644
index 0..602c189cd
--- /dev/null
+++ b/.appveyor.yml
@@ -0,0 +1,69 @@
+version: '{build}'
+
+branches:
+ only:
+ - master
+ - /cygwin/
+
+skip_tags: true
+shallow_clone: true
+
+environment:
+ APPVEYOR_SAVE_CACHE_ON_ERROR: true
+ CACHE: C:\cache
+ CYGWIN_MIRROR: http://cygwin.mirror.constant.com
+ matrix:
+ - BUILD: x86_64-pc-cygwin
+CYGWIN_ROOT: C:\cygwin64
+PKGARCH: mingw64-x86_64
+SETUP: setup-x86_64.exe
+ - BUILD: i686-pc-cygwin
+CYGWIN_ROOT: C:\cygwin
+PKGARCH: mingw64-i686
+SETUP: setup-x86.exe
+
+cache: C:\cache
+
+install:
+- if not exist %CACHE% mkdir %CACHE%
+- appveyor DownloadFile http://cygwin.com/%SETUP% -FileName %CACHE%\%SETUP%
+- "%CACHE%\\%SETUP% -qnNdO -R %CYGWIN_ROOT% -s %CYGWIN_MIRROR% -l %CACHE% -g
-P \
+gcc-core,\
+gcc-g++,\
+make,\
+perl,\
+patch,\
+cocom,\
+gettext-devel,\
+libiconv-devel,\
+zlib-devel,\
+%PKGARCH%-gcc-core,\
+%PKGARCH%-gcc-g++,\
+%PKGARCH%-zlib,\
+dblatex,\
+docbook2X,\
+docbook-xml45,\
+docbook-xsl,\
+xmlto,\
+python3-lxml,\
+python3-ply"
+
+build_script:
+- '%CYGWIN_ROOT%/bin/bash -lc "cd $APPVEYOR_BUILD_FOLDER; mkdir build install"'
+- '%CYGWIN_ROOT%/bin/bash -lc "cd $APPVEYOR_BUILD_FOLDER/build; ../configure
--prefix=$(realpath $(pwd)/../install) -v"'
+- '%CYGWIN_ROOT%/bin/bash -lc "cd $APPVEYOR_BUILD_FOLDER/build; make"'
+- '%CYGWIN_ROOT%/bin/bash -lc "cd $APPVEYOR_BUILD_FOLDER/build; make install"'
+- '%CYGWIN_ROOT%/bin/bash -lc "cd $APPVEYOR_BUILD_FOLDER/build; cd */newlib;
make info man"'
+- '%CYGWIN_ROOT%/bin/bash -lc "cd $APPVEYOR_BUILD_FOLDER/build; cd */newlib;
make install-info install-man"'
+
+test: off
+deploy: off
+
+# irc notification via notifico
+notifications:
+- provider: Webhook
+ url: http://n.tkte.ch/h/4848/0nqixIBiOFzf-S_N2PY83dGB
+ method: GET
+ on_build_success: false
+ on_build_failure: false
+ on_build_status_changed: true
--
2.28.0
[PATCH 0/3] CI update
Since we recently had the unpleasant surprise of discovering that Cygwin doesn't build on F32 when trying to make a release, this adds some CI to test that. Open issues: Since there don't seem to be RedHat packages for cocom, this grabs a cocom package from some random 3rd party I found on the internet. That might not be the best idea :). This also updates other CI configurations. Jon Turney (3): Cygwin: Add .appveyor.yml Cygwin: Add github action to cross-build on Fedora Cygwin: Remove .drone.yml .appveyor.yml| 69 .drone.yml | 58 -- .github/workflows/cygwin.yml | 45 +++ 3 files changed, 114 insertions(+), 58 deletions(-) create mode 100644 .appveyor.yml delete mode 100644 .drone.yml create mode 100644 .github/workflows/cygwin.yml -- 2.28.0
Re: [PATCH] Cygwin: pty: Disable pseudo console if TERM is dumb or not set.
Hi Takashi, On Aug 26 21:00, Takashi Yano via Cygwin-patches wrote: > Pseudo console generates escape sequences on execution of non-cygwin > apps. If the terminal does not support escape sequence, output will > be garbled. This patch prevents garbled output in dumb terminal by > disabling pseudo console. I'm a bit puzzled by this patch. We had code handling emacs and dumb terminals explicitely in the early forms of the first incarnation of the pseudo tty code, but fortunately you found a way to handle this without hardcoding terminal types into Cygwin. Why do you think we have to do this now? Thanks, Corinna
Re: [Patch] Fix incorrect code page when setting console title on Win10
On Aug 26 09:30, Johannes Schindelin wrote: > Hi Corinna, > > On Wed, 26 Aug 2020, Corinna Vinschen wrote: > > > On Aug 26 16:43, 宫大汉 via Cygwin-patches wrote: > > > When Cygwin sets console titles on Win10 (has_con_24bit_colors > > > !con_is_legacy), > > > `WriteConsoleA` is used and causes an error if: > > > 1. the environment variable of `LANG` is `***.UTF-8` > > > 2. and the code page of console.exe is not UTF-8 > > > 1. e.g. on my Computer, it's GB2312, for Chinese text > > > > > > > > > I've done some tests on msys2 and details are on > > > https://github.com/git-for-windows/git/issues/2738, > > > and I filed a PR of > > > https://github.com/git-for-windows/msys2-runtime/pull/25. > > Just in case you want to have a look at it, you can download the patch via > https://github.com/git-for-windows/msys2-runtime/commit/334f52a53a2e6b7f560b0e8810b9f672ebb3ad24.patch > > FWIW my original reviewer comment was: "why not fix wpbuf.send() in the > first place?" but after having a good look around, that method seemed to > be called from so many places that I "got cold feet" of that approach. > > For one, I saw at least one caller that wants to send Escape sequences, > and I have no idea whether it is a good idea to do that in the `*W()` > version of the `WriteConsole()` function. Yes, it is. There's no good reason to use the A functions, actually. They are just wrappers calling the W functions and WriteConsoleW evaluates ESC sequences just fine (just given as UTF-16 chars). > So the real question from my side is: how to address properly the many > uses of `WriteConsoleA()` (which breaks all kinds of encodings in many > situations because Windows' idea of the current code page and Cygwin's > idea of the current locale are pretty often at odds). > > The patch discussed here circumvents one of those call sites. > > However, even though there have not been any callers of `WriteConsoleA()` > in Cygwin v3.0.7 (but four callers of `WriteConsoleW()` which I suspect > were converted to `*A()` calls in v3.1.0 by the Pseudo Console patches), > there are now a whopping 15 callers of that `*A()` function in Cygwin > v3.1.7. See here: > [...] > That cannot be intentional, can it? We should always thrive to use the > `*W()` functions so that we can be sure that the expected encoding is > used, right? Takashi? Any reason to use WriteConsoleA rather than WriteConsoleW? If at all, WriteConsoleA should only be used if it's 100% safe to assume that the buffer only contains ASCII chars < 127. Thanks, Corinna
Re: [Patch] Fix incorrect code page when setting console title on Win10
Hi Corinna, On Wed, 26 Aug 2020, Corinna Vinschen wrote: > On Aug 26 16:43, 宫大汉 via Cygwin-patches wrote: > > When Cygwin sets console titles on Win10 (has_con_24bit_colors > > !con_is_legacy), > > `WriteConsoleA` is used and causes an error if: > > 1. the environment variable of `LANG` is `***.UTF-8` > > 2. and the code page of console.exe is not UTF-8 > > 1. e.g. on my Computer, it's GB2312, for Chinese text > > > > > > I've done some tests on msys2 and details are on > > https://github.com/git-for-windows/git/issues/2738, > > and I filed a PR of > > https://github.com/git-for-windows/msys2-runtime/pull/25. Just in case you want to have a look at it, you can download the patch via https://github.com/git-for-windows/msys2-runtime/commit/334f52a53a2e6b7f560b0e8810b9f672ebb3ad24.patch FWIW my original reviewer comment was: "why not fix wpbuf.send() in the first place?" but after having a good look around, that method seemed to be called from so many places that I "got cold feet" of that approach. For one, I saw at least one caller that wants to send Escape sequences, and I have no idea whether it is a good idea to do that in the `*W()` version of the `WriteConsole()` function. So the real question from my side is: how to address properly the many uses of `WriteConsoleA()` (which breaks all kinds of encodings in many situations because Windows' idea of the current code page and Cygwin's idea of the current locale are pretty often at odds). The patch discussed here circumvents one of those call sites. However, even though there have not been any callers of `WriteConsoleA()` in Cygwin v3.0.7 (but four callers of `WriteConsoleW()` which I suspect were converted to `*A()` calls in v3.1.0 by the Pseudo Console patches), there are now a whopping 15 callers of that `*A()` function in Cygwin v3.1.7. See here: $ git grep WriteConsoleA winsup/cygwin/fhandler_console.cc:WriteConsoleA (handle, buf, ixput, wn, 0); winsup/cygwin/fhandler_console.cc: WriteConsoleA (get_output_handle (), "\033[?1h", 5, NULL, 0); winsup/cygwin/fhandler_console.cc:WriteConsoleA (get_output_handle (), _char, 1, 0, 0); winsup/cygwin/fhandler_console.cc:WriteConsoleA (get_output_handle (), winsup/cygwin/fhandler_console.cc:WriteConsoleA (get_output_handle (), buf, strlen (buf), 0, 0); winsup/cygwin/fhandler_console.cc:WriteConsoleA (get_output_handle (), buf, strlen (buf), 0, 0); winsup/cygwin/fhandler_console.cc:WriteConsoleA (get_output_handle (), buf, strlen (buf), 0, 0); winsup/cygwin/fhandler_console.cc:WriteConsoleA (get_output_handle (), buf, strlen (buf), 0, 0); winsup/cygwin/fhandler_console.cc:WriteConsoleA (get_output_handle (), buf, strlen (buf), 0, 0); winsup/cygwin/fhandler_console.cc:WriteConsoleA (get_output_handle (), buf, strlen (buf), 0, 0); winsup/cygwin/fhandler_console.cc:WriteConsoleA (get_output_handle (), winsup/cygwin/fhandler_tty.cc:DEF_HOOK (WriteConsoleA); winsup/cygwin/fhandler_tty.cc:WriteConsoleA_Hooked winsup/cygwin/fhandler_tty.cc: return WriteConsoleA_Orig (h, p, l, n, o); winsup/cygwin/fhandler_tty.cc: DO_HOOK (NULL, WriteConsoleA); That cannot be intentional, can it? We should always thrive to use the `*W()` functions so that we can be sure that the expected encoding is used, right? Ciao, Dscho
[PATCH] Cygwin: pty: Disable pseudo console if TERM is dumb or not set.
Pseudo console generates escape sequences on execution of non-cygwin
apps. If the terminal does not support escape sequence, output will
be garbled. This patch prevents garbled output in dumb terminal by
disabling pseudo console.
---
winsup/cygwin/spawn.cc | 36 +---
1 file changed, 29 insertions(+), 7 deletions(-)
diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
index 8308bccf3..b6d58e97a 100644
--- a/winsup/cygwin/spawn.cc
+++ b/winsup/cygwin/spawn.cc
@@ -647,13 +647,35 @@ child_info_spawn::worker (const char *prog_arg, const
char *const *argv,
ZeroMemory (_pcon, sizeof (si_pcon));
STARTUPINFOW *si_tmp =
if (!iscygwin () && ptys_primary && is_console_app (runpath))
- if (ptys_primary->setup_pseudoconsole (_pcon,
-mode != _P_OVERLAY && mode != _P_WAIT))
- {
- c_flags |= EXTENDED_STARTUPINFO_PRESENT;
- si_tmp = _pcon.StartupInfo;
- enable_pcon = true;
- }
+ {
+ bool nopcon = mode != _P_OVERLAY && mode != _P_WAIT;
+ /* If TERM is "dumb" or not set, disable pseudo console */
+ if (envblock)
+ {
+ bool term_is_set = false;
+ for (PWCHAR p = envblock; *p != L'\0'; p += wcslen (p) + 1)
+ {
+ if (wcscmp (p, L"TERM=dumb") == 0)
+ nopcon = true;
+ if (wcsncmp (p, L"TERM=", 5) == 0)
+ term_is_set = true;
+ }
+ if (!term_is_set)
+ nopcon = true;
+ }
+ else
+ {
+ const char *term = getenv ("TERM");
+ if (!term || strcmp (term, "dumb") == 0)
+ nopcon = true;
+ }
+ if (ptys_primary->setup_pseudoconsole (_pcon, nopcon))
+ {
+ c_flags |= EXTENDED_STARTUPINFO_PRESENT;
+ si_tmp = _pcon.StartupInfo;
+ enable_pcon = true;
+ }
+ }
loop:
/* When ruid != euid we create the new process under the current original
--
2.28.0
Re: [Patch] Fix incorrect code page when setting console title on Win10
Hi,
On Aug 26 16:43, 宫大汉 via Cygwin-patches wrote:
> When Cygwin sets console titles on Win10 (has_con_24bit_colors
> !con_is_legacy),
> `WriteConsoleA` is used and causes an error if:
> 1. the environment variable of `LANG` is `***.UTF-8`
> 2. and the code page of console.exe is not UTF-8
> 1. e.g. on my Computer, it's GB2312, for Chinese text
>
>
> I've done some tests on msys2 and details are on
> https://github.com/git-for-windows/git/issues/2738,
> and I filed a PR of https://github.com/git-for-windows/msys2-runtime/pull/25.
> Then what should I do, in order to fix this on Cygwin?
Sending patches to the cygwin-patches mailing list is the right way to
go, so that's fine. However, there are two small problems with your
patch:
- For non-trivial patches we need a 2-clause BSD waiver from you, as per
https://cygwin.com/contrib.html, chapter "Before you get started".
- Your MUA apparently broke the patch. No way to apply it in this form.
Would you mind to resend it together with your BSD waiver, but as
attachment?
Thanks,
Corinna
>
>
> The below is the commit's .patch file:
>
>
> From 334f52a53a2e6b7f560b0e8810b9f672ebb3ad24 Mon Sep 17 00:00:00 2001
> From: Dahan Gong Date: Fri, 31 Jul 2020 22:06:54 +0800
> Subject: [PATCH] Fix incorrect code page in console title
>
>
> `WriteConsoleA` always follows the current code page of a console window, so
> it's not suitable to pass a multi-byte string in `get_ttyp
> ()-term_code_page` to it directly.
>
>
> This PR turns to `WriteConsoleW` so that most characters will be translated
> "as is", and I've tested it on Win 10 v2004 (CMD: ver 10.0.19041.388).
>
>
> Signed-off-by: gdh1995 ---
> winsup/cygwin/fhandler_console.cc | 18 +-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
>
> diff --git a/winsup/cygwin/fhandler_console.cc
> b/winsup/cygwin/fhandler_console.cc
> index dd979fb8e2..7870eeda81 100644
> --- a/winsup/cygwin/fhandler_console.cc
> +++ b/winsup/cygwin/fhandler_console.cc
> @@ -80,6 +80,15 @@ static class write_pending_buffer
> {
> WriteConsoleA (handle, buf, ixput, wn, 0);
> }
> + inline char *c_str (size_t *psize = NULL)
> + {
> + size_t size = ixput < WPBUF_LEN ? ixput : WPBUF_LEN - 1;
> + buf[size] = '\0';
> + if (psize != NULL) {
> + *psize = size;
> + }
> + return (char *) buf;
> + }
> } wpbuf;
>
> static void
> @@ -3203,7 +3212,14 @@ fhandler_console::write (const void *vsrc, size_t len)
> if (*src < ' ')
>{
> if (wincap.has_con_24bit_colors ()
> !con_is_legacy)
> - wpbuf.send (get_output_handle ());
> + {
> + size_t nms;
> + char *ms = wpbuf.c_str(nms);
> + wchar_t write_buf[TITLESIZE + 1];
> + DWORD done;
> + DWORD buf_len = sys_mbstowcs (write_buf,
> TITLESIZE, ms);
> + write_console (write_buf, buf_len, done);
> + }
> else if (*src == '\007' con.state ==
> gettitle)
> set_console_title (con.my_title_buf);
> con.state = normal;
[Patch] Fix incorrect code page when setting console title on Win10
When Cygwin sets console titles on Win10 (has_con_24bit_colors !con_is_legacy), `WriteConsoleA` is used and causes an error if: 1. the environment variable of `LANG` is `***.UTF-8` 2. and the code page of console.exe is not UTF-8 1. e.g. on my Computer, it's GB2312, for Chinese text I've done some tests on msys2 and details are on https://github.com/git-for-windows/git/issues/2738, and I filed a PR of https://github.com/git-for-windows/msys2-runtime/pull/25. Then what should I do, in order to fix this on Cygwin? The below is the commit's .patch file: From 334f52a53a2e6b7f560b0e8810b9f672ebb3ad24 Mon Sep 17 00:00:00 2001 From: Dahan Gong
Re: [PATCH v2 3/3] winsup/doc/faq-api.xml(faq.api.timezone): explain time zone updates
On Aug 25 06:57, Brian Inglis wrote: > based on material from t...@iana.org mailing list sources > --- > winsup/doc/faq-api.xml | 29 + > 1 file changed, 25 insertions(+), 4 deletions(-) > > diff --git a/winsup/doc/faq-api.xml b/winsup/doc/faq-api.xml > index 829e4d7febd8..365e301555a5 100644 > --- a/winsup/doc/faq-api.xml > +++ b/winsup/doc/faq-api.xml > @@ -385,13 +385,34 @@ Cygwin version number details, check out the > > > > -Why isn't timezone set correctly? > +Why isn't time zone set correctly? > > > -(Please note: This section has not yet been > updated for the latest net release.) > - > -Did you explicitly call tzset() before checking the value of timezone? > +Did you explicitly call tzset() before checking the value of time zone? > If not, you must do so. > +Time zone settings are updated by changes to the tzdata package included in > all > +Cygwin installations. Shouldn't a new paragraph start at this point? Actually, maybe this could be changed a bit more. The question might be better called "Why isn't my (or the) time zone set correctly?" and the order inside the FAQ seems a bit upside down now. Starting with a reply only affecting developers with self-written applications is rather weird given the general discussion only follows afterwards. The discussion on how time zones are updated in real life might be the better start, then how to rectify local settings by running Setup, and only then implications for developers. Make sense? Thanks, I pushed the other two patches in the meantime. Corinna
