Skip to content

Use UTF-8 encoding for GHCi -unit response file arguments - #12574

Open
fendor wants to merge 3 commits into
haskell:masterfrom
fendor:wip/fendor/fix-unit-file-encoding
Open

fendor wants to merge 3 commits into
haskell:masterfrom
fendor:wip/fendor/fix-unit-file-encoding

Conversation

@fendor

@fendor fendor commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator

Fixes #12573

Updates the cabal-testsuite to use UTF-8 encoding appropriately, otherwise the tests crash due to the same encoding error that plagues -unit @rsp.


Template B: This PR does not modify behaviour or interface

E.g. the PR only touches documentation or tests, does refactorings, etc.

Include the following checklist in your PR:

  • Patches conform to the coding conventions.
  • Is this a PR that fixes CI? If so, it will need to be backported to older cabal release branches (ask maintainers for directions).

@fendor

fendor commented Nov 11, 2026 •

Copy link
Copy Markdown
Collaborator Author

I think, in theory, it is worth reviewing every occurrence of Data.ByteString.Char8 and review it for its correctness.
While skimming the codebase, I found multiple locations where the usage is dubious (i.e. I wasn't immediately convinced the usage was correct).

@fendor
fendor force-pushed the wip/fendor/fix-unit-file-encoding branch 3 times, most recently from f4e9d49 to bf904ca Compare November 11, 2026 14:00
@fendor
fendor force-pushed the wip/fendor/fix-unit-file-encoding branch 2 times, most recently from 98f951d to 9cdd305 Compare November 11, 2026 14:35
@fendor fendor changed the title Use UTF-8 encoding to for GHCi -unit response file arguments Use UTF-8 encoding for GHCi -unit response file arguments Nov 11, 2026
@fendor
fendor force-pushed the wip/fendor/fix-unit-file-encoding branch from 9cdd305 to dabcb3d Compare November 11, 2026 15:53
@Mikolaj

Mikolaj commented Nov 11, 2026

Copy link
Copy Markdown
Member

I think, in theory, it is worth reviewing every occurrence of Data.ByteString.Char8 and review it for its correctness. While skimming the codebase, I found multiple locations where the usage is dubious (i.e. I wasn't immediately convinced the usage was correct).

Would it fit this PR or would a new PR be better? If the latter, could you kindly open a ticket so that we don't forget?

@Mikolaj Mikolaj left a comment

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.

Thank you!

@mergify mergify Bot added the 1️⃣ more approval needed Set by Mergify: has one of the two required approvals label Nov 11, 2026
@mergify mergify Bot removed the 1️⃣ more approval needed Set by Mergify: has one of the two required approvals label Nov 11, 2026
@fendor

fendor commented Nov 12, 2026

Copy link
Copy Markdown
Collaborator Author

Would it fit this PR or would a new PR be better? If the latter, could you kindly open a ticket so that we don't forget?

I think another PR is better, this fixes a real bug, the other occurrences could be benign. Also, I am not committing to review the whole cabal codebase 😬

@fendor

fendor commented Nov 12, 2026

Copy link
Copy Markdown
Collaborator Author

Thank you all for the thorough review, not my proudest PR, given the amount of oversights :D

@fendor
fendor force-pushed the wip/fendor/fix-unit-file-encoding branch 2 times, most recently from 3404405 to 296efc5 Compare November 12, 2026 08:55
@fendor

fendor commented Nov 12, 2026

Copy link
Copy Markdown
Collaborator Author

I created an issue: #12580

@fendor
fendor force-pushed the wip/fendor/fix-unit-file-encoding branch from 296efc5 to 6a5dba8 Compare November 12, 2026 09:17
@fendor fendor added the merge me Tell Mergify Bot to merge label Nov 12, 2026
@mergify mergify Bot added the ready and waiting Mergify is waiting out the cooldown period label Nov 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge me Tell Mergify Bot to merge ready and waiting Mergify is waiting out the cooldown period

Projects

None yet

Development

Successfully merging this pull request may close these issues.

multi-repl crashes when the directory contains utf-8 symbols

4 participants