Skip to content

GitHub Action for non-UTF-8 locales #7821

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
aitap wants to merge 4 commits into
base: master
Choose a base branch
from
+50 −1
Open

GitHub Action for non-UTF-8 locales #7821

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c5ab604
GHA: add locale-specific checks
aitap Jun 1, 2026
52271bb
Fix tests 2298.*
aitap Jun 6, 2026
29ad3b0
Drop unnecessary environment variables
aitap Jul 18, 2026
060f490
Merge branch 'master' into GHA-locales
aitap Jul 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions .github/workflows/R-CMD-check.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,52 @@ jobs:
- uses: yihui/actions/check-r-package@HEAD
with:
check-args: "--no-manual --as-cran"

R-CMD-check-locale:
runs-on: ${{ matrix.os }}

name: ${{ matrix.os }}, R-${{ matrix.r }}, LC_CTYPE=${{ matrix.locale }}

strategy:
fail-fast: true
matrix:
os: [ubuntu-22.04]
r: ['release']
locale: ['fr_CA.ISO-8859-1', 'zh_CN.GB18030', 'ru_RU.KOI8-R']

env:
GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}

steps:
- uses: actions/checkout@v6

- uses: r-lib/actions/setup-r@v2
with:
r-version: ${{ matrix.r }}

- name: Configure locale
run: |
echo ${{ matrix.locale }} `echo ${{ matrix.locale }} | sed 's/.*\.//'` | sudo tee -a /etc/locale.gen
sudo locale-gen --keep-existing

- uses: yihui/actions/setup-r-dependencies@HEAD

- name: Override R for locale-specific check
run: |
sudo mkdir -p /override
# poor man's luit that doesn't require a tty
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

@MichaelChirico MichaelChirico Jul 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 check output will add massive overhead and severely slow down the CI run.
  • Buggy Parsing: The read command is missing the -r flag, meaning it will inadvertently mangle and strip backslashes from R's output. Furthermore, using echo "$line" can result in unexpected behavior if a line begins with a flag like -n or -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

@jangorecki jangorecki Jul 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree, it does not impact code of the package its license.

echo "\$line" | iconv -t UTF-8
done
EOF
sudo chmod +x /override/R
echo PATH=/override:$PATH >> $GITHUB_ENV

@MichaelChirico MichaelChirico Jul 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


- uses: yihui/actions/check-r-package@HEAD
with:
check-args: "--no-manual"
env:
LC_CTYPE: ${{ matrix.locale }}
2 changes: 1 addition & 1 deletion inst/tests/tests.Rraw
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20308,7 +20308,7 @@ local(if (utf8_check(c("\u00e4", "\u00f6", "\u00fc"))) {
# a-umlaut, o-umlaut, u-umlaut
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"))

@MichaelChirico MichaelChirico Jul 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from="" is the default, better to just omit it

test(2298.1, rbindlist(list(x,y), use.names=TRUE), data.table("\u00e4"=c(1,6), "\u00f6"=c(2,4), "\u00fc"=c(3,5)))
test(2298.2, rbindlist(list(y,x), use.names=TRUE), data.table("\u00f6"=c(4,2), "\u00fc"=c(5,3), "\u00e4"=c(6,1)))
set(y, j="\u00e4", value=NULL)
Expand Down
Loading