GitHub Action for non-UTF-8 locales

[aitap]

Following #7681 (comment): test data.table in the Latin-1 locale (where CE_NATIVE strings should be byte-to-byte equal to CE_LATIN1), GB18030 (which is fully Unicode-compatible, but the mapping from code points to byte sequences is very non-uniform), KOI8-R (which can represent some math symbols but not extended Latin or CJK).

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.01%. Comparing base (94649d5) to head (060f490).
⚠️ Report is 11 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #7821   +/-   ##
=======================================
  Coverage   99.01%   99.01%           
=======================================
  Files          88       88           
  Lines       17234    17234           
=======================================
  Hits        17065    17065           
  Misses        169      169           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[MichaelChirico]

I think it could be merged as-is, PTAL at the feedback and see what you agree is worth addressing. Thanks!

[MichaelChirico]

BTW, in #7832 (R-CMD-check-occasional GHA), we also run lv_LV and zh_CN locales, and are finding a number of CI errors -- I'm surprised they aren't throwing here, could you explain the discrepancy?

[aitap]

Here I'm running tests with only LC_CTYPE set in order to catch encoding bugs; LC_COLLATE stays unchanged. Meanwhile, Latvian conveniently uses the Latin script and defines a complex collation sequence on their alphabet. The same will happen with Azerbaijani, Turkish, Czech, Danish, Norwegian, Estonian, Hungarian, and Lithuanian, but not Spanish, Swedish, Finnish, Polish, Icelandic, French, or German.

(Try installing locales-all and feeding multi-character ASCII strings to the following function:

function(locale, strings) {
 old_collate = Sys.getlocale('LC_COLLATE')
 Sys.setlocale('LC_COLLATE', locale)
 on.exit(Sys.setlocale('LC_COLLATE', old_collate), add = TRUE)
 old_ctype = Sys.getlocale('LC_CTYPE')
 Sys.setlocale('LC_CTYPE', locale)
 on.exit(Sys.setlocale('LC_CTYPE', old_ctype), add = TRUE)
 data.table::data.table(strings, locale = base::order(strings), forderv = data.table:::forderv(strings))
}

)

[MichaelChirico]

Yea, lv_LV was specifically chosen for the collation rules (which indeed caught some issues in the tests).

I'm wondering if we should combine the efforts here with the R-CMD-check-occasional GHA, just add more encodings over there, add a collation check encoding on the every-push GHA, or just leave things as-is (i.e. with these two PRs unchanged), WDYT?

[aitap]

It's probably fine to move the encoding tests into the R-CMD-check-occasional action, together with the LC_COLLATE tests.

[MichaelChirico]

OK, moved the changes there. The behemoth grows :)

[ben-schwen]

LGTM besides the potential clash of 'fr_CA.ISO-8859-1' and 'fr_CA'

[aitap]

It's not completely reliable to rely on the implicit default encoding. It's possible to have a UTF-8 fr_CA locale with the right settings in /etc/locale.gen:

echo 'fr_CA UTF-8' | sudo tee -a /etc/locale.gen
sudo locale-gen --keep-existing
LANG=fr_CA Rscript -e 'l10n_info()'

$MBCS
[1] TRUE

$`UTF-8`
[1] TRUE

$`Latin-1`
[1] FALSE

$codeset
[1] "UTF-8"

[aitap]

This will cause child R processes run by R CMD check to encode their output to UTF-8 as well. Then the overall output from the parent process will be encoded once again, with some outputs requiring echo "$unicode_text_from_browser" | iconv -t $source_encoding | iconv -t $source_encoding to read them as UTF-8 (implying some encoding from $source_encoding to UTF-8 has happened thrice, some even more).

[MichaelChirico]

Yea, I got as far as that, but not fixing it.

In ru_RU.KOI8-R, it looks like the root issue is {bit} not installing, possibly because of UTF-8 in R/ comments like:

https://github.com/r-lib/bit/blob/d128f0735f3c15ac67070a1398fea3411de5eab8/R/zzz.R#L2

This per LLM:

The Recursive Mojibake (Why the text is so garbled)

1. Child Process: Encounters an error and correctly outputs Russian text in KOI8-R (e.g. Предупреждение). Its iconv wrapper converts this to UTF-8.
2. Parent Process (R CMD INSTALL): Captures that UTF-8 text and echoes it to the console. The parent's iconv wrapper intercepts these UTF-8 bytes, incorrectly assumes they are KOI8-R, and converts them to UTF-8 again.
3. Grandparent Process (R CMD check): Captures the double-mojibake, assumes it is KOI8-R, and converts it a third time.

If we reverse the triple-encoding of the string in your log (п©ц╥я▐Б■─...), it translates perfectly to the standard R error:

"Error in parse(con, keep.source = FALSE, srcfile = NULL) : invalid input found on input connection..."

[aitap]

{0} probably included quotes or something. You can safely pass arbitrary arguments to shell commands like this:

Suggested change

	shell: bash -c 'set -o pipefail; Rscript {0} 2>&1 | iconv -c -t UTF-8'
	shell: bash -c 'set -o pipefail; Rscript "$@" 2>&1 | iconv -c -t UTF-8' -- {0}

[aitap]

This is so silly. What GHA runner actually runs is not the command line specified in the shell argument, oh no. It splits the shell line by whitespace and makes every word into a separate argument. As a result, the command being run is

bash "-c" "'set" "-o" "pipefail;" "Rscript" '"$@"' "2>&1" "|" "iconv" "-c" "-t" "UTF-8'" "--" "/home/runner/work/_temp/21fe02f1-a9e0-4138-ba8c-875e7bb7469a.sh"

which gives the same error:

-o: -c: line 1: unexpected EOF while looking for matching `''

Naturally, this is not documented at all.

[MichaelChirico]

That approach is kinda doomed because this is shared with windows runners too, right?

[aitap]

Quite.

I'm testing a solution in 1e27ce0 (https://github.com/Rdatatable/data.table/actions/runs/31110427909), limiting the time spent running with a non-default locale to R CMD check only.

[aitap]

There were two problems:

• R-3.5 being unable to parse bit source in the ru_RU.KOI8-R locale, failing R CMD check with a warning due to bit64 not being installed
• rcmdcheck::rcmdcheck calling desc::desc before building the package, which reads DESCIPTION with Encoding(.) == 'bytes, then feeds it to read.dcf() via a text connection. Somehow, read.dcf() then fed the byte-encoded string to startsWith() (despite there's code to prevent that). In a non-UTF-8 MBCS locale like zh_CN.18030, startsWith() needs to call translateCharUTF8() for non-ASCII strings, which is an error for CE_BYTES.

The former is avoided by only setting LC_ALL before checking the package and the latter is avoided by launching R CMD check manually instead of via rcmdcheck when the locale encoding is not UTF-8. This also lets us use the system2(stdout = TRUE, stderr = TRUE) code path to re-encode the output back to UTF-8.

Now let's see what I missed.

[MichaelChirico]

re: desc::description I have been trying for a long time to reproduce that locally to no avail, did you find a reprex? One thing was the read.dcf/write.dcf round trip but that should be avoided by deferring LC_ALL to just prior to R CMD check

[aitap]

No so far. At least in R-devel since r90200 (July 1), read.dcf should convert strings from CE_BYTES before trying to match them. I'd try to reduce the GHA example, but they are down right now.

Just noticed: some packages fail to install on macOS (e.g. hexbin on R-4.2) because there's no gfortran.

[MichaelChirico]

because there's no gfortran

Nice spot, let's add it. I guess the implication is that we have worse other.Rraw coverage on macOS.

At least in R-devel since r90200

Yea, I was seeing the same issue on r-release though. Really elusive: https://github.com/Rdatatable/data.table/actions/runs/31077783084/job/92539498421. I can't quite find the exact revision r-devel is running on, maybe that's something we should log.

[MichaelChirico]

but they are down right now.

well, that was quite serious lol

[aitap]

Really elusive

At least we can dissect it now. I can reproduce this with LC_CTYPE=zh_CN.gb18030 luit R-4.6.0/bin/R but not latest R-devel.

In desc:::idesc_create_text(self, private, lines), the lines are UTF-8 text marked as CE_NATIVE (GB18030). desc runs:

con <- textConnection(text, local = TRUE, encoding = "bytes")
on.exit(close(con), add = TRUE)
dcf <- read_dcf(con)

desc:::read_dcf runs:

lines <- readLines(file)

Encoding(lines) is once again unknown, and the contents are still UTF-8 bytes. Eventually the code needs to run

con <- textConnection(lines, local = TRUE)
read.dcf(con, all = TRUE)

In R 4.6 and R-devel until July, read.dcf(all = TRUE) skips the call to .Internal(readDCF(...)) and tries to run:

lines <- readLines(file, skipNul = TRUE, encoding = "bytes")
lines <- lines[!startsWith(lines, "#")]

Which fails in non-UTF-8 MBCS locales. In latest R-devel, lines is first converted by .enc2utf8_sub().

Even in latest R-devel, encoding="bytes" looks a bit derpy:

> readLines("DESCRIPTION", encoding='bytes') |> (\(.) .[Encoding(.) != 'unknown'])()
[1] "  person(\"Iago\", \"Gin\\xc3\\xa9-V\\xc3\\xa1zquez\",   role=\"ctb\"),\0T\001\004\xf2\r"
[2] "  person(\"Elise\", \"Maign\\xc3\\xa9\",        role=\"ctb\"),\0\x86\031辿脹"
[3] "  person(\"Alja\\xc5\\xbe\", \"Sluga\",         role=\"ctb\"),\0阃驤\x80"
[4] "  person(\"Ildik\\xc3\\xb3\", \"Czeller\",      role=\"ctb\"),\0x骹\005繤"

Surprisingly, ASan is not protesting (maybe with a newer compiler it would?); Valgrind says the memory contents are uninitialised, so maybe it's not a buffer overrun, but an over-allocation?

==16199== Conditional jump or move depends on uninitialised value(s)
==16199==    at 0x8A0344C: from_gb18030 (loop.c:335)
==16199==    by 0x8A0344C: gconv (skeleton.c:619)
==16199==    by 0x52178D3: mbrtowc (mbrtowc.c:85)
==16199==    by 0x2CBBD0: Rf_EncodeString (printutils.c:692)

Reported.

[MichaelChirico]

the contents are still UTF-8 bytes.

is that true also after our read.dcf/write.dcf loop to edit the DESCRIPTION to count the other.Rraw requirements as Suggests?

[aitap]

I think that was true even when running in weird locales, because as long as the invalid-encoding string stays marked as CE_NATIVE, R doesn't feel the need to convert it. A newer R-devel would enforce UTF-8 more strongly, but since it both reads and writes the file as UTF-8, there shouldn't be any mismatch. Now that only the child process sees the non-default locale, we can even be more sure.

[MichaelChirico]

amazing!!

[MichaelChirico]

might be slightly nicer to have UTF-8 returned by default here? I'm also thinking to invert it to avoid !non* below. merging now but if you agree we can do a quick follow up.