Skip to content

[test-release.sh] Fix sed encoding issues on macOS #105989

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.

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

Merged

Conversation

keith
Copy link
Member

@keith keith commented Aug 25, 2024

When using bsd sed that ships with macOS on the object files for comparison, every command would error with

sed: RE error: illegal byte sequence

This was potentially fixed for an older version in 6c52b02 but even the commands in the example there still have this error. You can repro this with any binary:

$ sed s/a/b/ /bin/ls >/dev/null
sed: RE error: illegal byte sequence

Where LC_CTYPE appears to no longer solve the issue:

$ LC_CTYPE=C sed s/a/b/ /bin/ls >/dev/null
sed: RE error: illegal byte sequence

But this change with LC_ALL does:

$ LC_ALL=C sed s/a/b/ /bin/ls >/dev/null; echo $?
0

It seems like the difference here is that if you have LC_ALL set to something else, LC_CTYPE does not override it. More info: https://stackoverflow.com/a/23584470/902968

When using bsd sed that ships with macOS on the object files for
comparison, every command would error with

```
sed: RE error: illegal byte sequence
```

This was potentially fixed for an older version in
6c52b02 but even the commands in the
example there still have this error. You can repro this with any binary:

```
$ sed s/a/b/ /bin/ls >/dev/null
sed: RE error: illegal byte sequence
```

Where LC_CTYPE appears to no longer solve the issue:

```
$ LC_CTYPE=C sed s/a/b/ /bin/ls >/dev/null
sed: RE error: illegal byte sequence
```

But this change with LC_ALL does:

```
$ LC_ALL=C sed s/a/b/ /bin/ls >/dev/null; echo $?
0
```

It seems like the difference here is that if you have LC_ALL set to
something else, LC_CTYPE does not override it. More info:
https://stackoverflow.com/a/23584470/902968
@keith keith requested a review from tstellar August 25, 2024 15:27
@keith keith changed the title test-release.sh: fix sed encoding issues on macOS [test-release.sh] Fix sed encoding issues on macOS Aug 25, 2024
@keith keith requested a review from tru August 28, 2024 04:30
Copy link
Collaborator

@tru tru left a comment

Choose a reason for hiding this comment

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

If this was tested on other platforms I am fine with it.

@keith keith merged commit 51e0a99 into llvm:main Sep 29, 2024
9 checks passed
@keith keith deleted the ks/test-release.sh-fix-sed-encoding-issues-on-macos branch September 29, 2024 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants