GitHub Action for non-UTF-8 locales#7821
Conversation
In a non-UTF-8 locale capable of representing a-umlaut, o-umlaut, u-umlaut, encode from the locale encoding, not UTF-8.
aitap
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. 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. 🚀 New features to boost your workflow:
|
| eval(parse(text = ' # eval(parse()) defers parsing to runtime; see utf8_check description | ||
| setnames(x , c("\u00e4", "\u00f6", "\u00fc")) | ||
| setnames(y , iconv(c("\u00f6", "\u00fc", "\u00e4"), from = "UTF-8", to = "latin1")) | ||
| setnames(y , iconv(c("\u00f6", "\u00fc", "\u00e4"), from = "", to = "latin1")) |
There was a problem hiding this comment.
from="" is the default, better to just omit it
| sudo tee /override/R <<EOF # no quotes => expand backticks, dollars | ||
| #!/bin/bash | ||
| set -o pipefail | ||
| exec stdbuf -o L -e L `which R` "\$@" 2>&1 | while read line; do |
There was a problem hiding this comment.
I don't think we care about AI policy in GHA scripts (cc @jangorecki), which are in .Rbuildignore; here's a recommended alternative:
exec stdbuf -o L -e L $(which R) "$@" 2>&1 | iconv -c -t UTF-8
Based on the feedback:
The wrapper script intercepts R's output and pipes it into a while read line loop. Inside this loop, a new iconv subprocess is spawned for every single line of output.
- Performance: Spawning a subprocess per line of
R CMD checkoutput will add massive overhead and severely slow down the CI run. - Buggy Parsing: The
readcommand is missing the-rflag, meaning it will inadvertently mangle and strip backslashes from R's output. Furthermore, usingecho "$line"can result in unexpected behavior if a line begins with a flag like-nor-e.
Suggestion: Replace the loop with a single direct pipe. If the author's intent was to prevent iconv from terminating the pipe upon encountering an invalid character, they can use the -c flag (which discards invalid characters) instead of a line-by-line loop
There was a problem hiding this comment.
agree, it does not impact code of the package its license.
| done | ||
| EOF | ||
| sudo chmod +x /override/R | ||
| echo PATH=/override:$PATH >> $GITHUB_ENV |
There was a problem hiding this comment.
I think we're meant to use GITHUB_PATH: https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-commands#example-of-adding-a-system-path
MichaelChirico
left a comment
MichaelChirico
left a comment
There was a problem hiding this comment.
I think it could be merged as-is, PTAL at the feedback and see what you agree is worth addressing. Thanks!
3 participants
Following #7681 (comment): test
data.tablein the Latin-1 locale (whereCE_NATIVEstrings should be byte-to-byte equal toCE_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).