Skip to content

[pull] master from git:master#190

Merged
pull[bot] merged 8 commits intoturkdevops:masterfrom
git:master
Apr 4, 2026
Merged

[pull] master from git:master#190
pull[bot] merged 8 commits intoturkdevops:masterfrom
git:master

Conversation

@pull
Copy link
Copy Markdown

@pull pull bot commented Apr 4, 2026

See Commits and Changes for more details.


Created by pull[bot] (v2.0.0-alpha.4)

Can you help keep this open source service alive? 💖 Please sponsor : )

pks-t and others added 8 commits April 2, 2026 11:39
When executing our test suite with Dash v0.5.13.2 one can observe
several test failures that all have the same symptoms: we have a quoted
heredoc that contains multibyte characters, but the final data does not
match what we actually wanted to write. One such example is in t0300,
where we see the diffs like the following:

  --- expect-stdout	2026-04-01 07:25:45.249919440 +0000
  +++ stdout	2026-04-01 07:25:45.254919509 +0000
  @@ -1,5 +1,5 @@
   protocol=https
   host=example.com
  -path=perú.git
  +path=perú.git
   username=foo
   password=bar

While seemingly the same, the data that we've written via the heredoc
contains some invisible bytes. The expected hex representation of the
string is:

  7065 72c3 ba2e 6769 74                 per...git

But what we actually get instead is this string:

  7065 7285 02c3 ba02 852e 6769 74       per.......git

What's important to note here is that the multibyte character exists in
both versions. But in the broken version we see that the bytes are
wrapped in a sequence of "85 02" and "02 85". This is the CTLMBCHAR byte
sequence of Dash, which it uses internally to quote multibyte sequences.

As it turns out, this bug was introduced in c5bf970 (expand: Add
multi-byte support to pmatch, 2024-06-02), which adds multibyte support
to more contexts of Dash. One of these contexts seems to be in heredocs,
and Dash _does_ correctly unquote these multibyte sequences when using
an unquoted heredoc. But the bug seems to be that this unquoting does
not happen in quoted heredocs, and the bug still exists on the latest
"master" branch.

For now, work around the bug by using unquoted heredocs instead.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When executing t9300 with Dash v0.5.13.1 we can see that the test hangs
completely with the following (condensed) trace:

  git fast-import
  + error=1
  + read output
  + cat input
  + echo checkpoint
  + echo progress checkpoint
  + test rogress checkpoint = progress checkpoint
  + test rogress checkpoint = UNEXPECTED
  + echo cruft: rogress checkpoint
  cruft: rogress checkpoint
  + read output
  + test  = progress checkpoint
  + test  = UNEXPECTED
  + echo cruft:
  cruft:
  + read output

Basically, what's happening here is that we spawn git-fast-import(1) and
wait for it to output a certain string, "progress checkpoint". Curiously
though, what we end up reading is "rogress checkpoint" -- so the first
byte of the expected string is missing.

Same as in the preceding commit, this seems to be a bug in Dash itself
that bisects to c5bf970 (expand: Add multi-byte support to pmatch,
2024-06-02). But other than in the preceding commit, this bug has
already been fixed upstream in 079059a (input: Fix heap-buffer-overflow
in preadbuffer on long lines, 2026-02-11), which is part of v0.5.13.2.

For now though, work around the bug by waiting for the expected output
in a different way. There is no good reason why one version should work
better than the other, but at least the new version doesn't exhibit the
bug. And, if you ask me, it's also slightly easier to read.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This is a companion patch of 3b9b2c2 (compat/posix: introduce
writev(3p) wrapper, 2026-03-13) where support for using the `writev()`
wrapper was introduced in the `Makefile` and the Meson-based build, but
the CMake build still needs that treatment, too.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The `mingw_strftime()` wrapper exists to work around msvcrt.dll's
incomplete `strftime()` implementation by dynamically loading the
version from ucrtbase.dll at runtime via `LoadLibrary()` +
`GetProcAddress()`. When the binary is already linked against UCRT
(i.e. when building in the UCRT64 environment), the linked-in
`strftime()` is the ucrtbase.dll version, making the dynamic loading
needless churn: It's calling the very same code.

Simply guard both the declaration and implementation so that the
unnecessary work-around is skipped in UCRT builds.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
MinGW build updates.

* js/mingw-use-strftime-directly-in-ucrt-build:
  mingw: use strftime() directly in UCRT builds
Build instruction for recently added writev() compatibility wrapper
has been also added to cmake.

* js/cmake-needs-writev-compat-too:
  cmake: use writev(3p) wrapper as needed
The way dash 0.5.13 handles non-ASCII contents in here-doc
is buggy and breaks our existing tests, which unfortunately
have been rewritten to avoid triggering the bug.

* ps/dash-buggy-0.5.13-workaround:
  t9300: work around partial read bug in Dash v0.5.13
  t: work around multibyte bug in quoted heredocs with Dash v0.5.13
Signed-off-by: Junio C Hamano <gitster@pobox.com>
@pull pull bot locked and limited conversation to collaborators Apr 4, 2026
@pull pull bot added the ⤵️ pull label Apr 4, 2026
@pull pull bot merged commit 2855562 into turkdevops:master Apr 4, 2026
2 of 3 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants