Use text package to decode UTF-8 instead of doing it ourselves #11462

Bodigrim · 2026-01-24T22:41:00Z

Template Α: This PR modifies behaviour or interface

Include the following checklist in your PR:

Patches conform to the coding conventions.
Any changes that could be relevant to users have been recorded in the changelog.
- Is the change significant? If so, remember to add significance: significant in the changelog file.
The documentation has been updated, if necessary.
Manual QA notes have been included.
Tests have been added. (Ask for help if you don’t know how to write them! Ask for an exemption if tests are too complex for too little coverage!)

ulysses4ever · 2026-01-25T14:52:09Z

In principle, this looks like the right direction of travel. But I'm wary of the CPP and dependence on an internal text module.

Bodigrim · 2026-01-25T16:47:50Z

CPP can be avoided if everyone is happy to require text >= 2.0. This would require regenerating bootstrap build plan for GHC 9.2.8, but otherwise I hardly see any drawbacks. Shall we?

geekosaur · 2026-01-25T17:15:59Z

With all the stuff we already need for bootstrapping, and 9.2.8 being disrecommended due to bugs, I think we should go ahead and bump it if it avoids CPP.

This currently requires CPP for text<2, required for Cabal bootstrap plan on GHC 9.2. Hopefully we will be able to get rid of it in a not so distant future: `text-2.0` itself is buildable with GHC >= 8.0 and shipped as a boot package starting from GHC 9.4.

Bodigrim · 2026-01-25T17:55:04Z

One does not simply update a bootstrap plan. Raised #11464 to discuss.

geekosaur · 2026-01-25T17:55:26Z

And for what it's worth, I don't see any other text version checks in the codebase.

ulysses4ever · 2026-01-25T19:43:58Z

Since it's a change to the library, not the tool, I'd be more careful. In particular, I'd only require text>=2.0 after the last GHC shipping text<2.0 falls out of our support window. Thoughts @Mikolaj @mpickering @ffaf1?

Bodigrim · 2026-01-25T19:55:02Z

In particular, I'd only require text>=2.0 after the last GHC shipping text<2.0 falls out of our support window.

Could you elaborate why? What's the relationship?

geekosaur · 2026-01-25T20:16:25Z

All I can think of is someone building such an older ghc from source, since that would require the newer text to be a bootlib.

Bodigrim · 2026-01-25T20:47:08Z

All I can think of is someone building such an older ghc from source, since that would require the newer text to be a bootlib.

If they are building an old GHC from source, surely they are building an old Cabal boot library from the GHC source tree, so all is fine.

(For the record, not a hill I'm going to die on; I'm genuinely curious)

Mikolaj · 2026-01-26T10:20:11Z

This would require regenerating bootstrap build plan for GHC 9.2.8

We can drop the plan for GHC 9.2.8, even because we already have more plans than we normally offer.

Re requiring new text vs CPP, that's a tough one, in particular, because CPP (and non-strict deps) make it much harder to test all paths in CI. Text 2.0 is 4 years old, so maybe it's time to switch? That obviously needs a major version, asking major lib users ahead of time (especially @mpilgrem and the illustrious distro maintainers) and I may still be missing some old promise or custom around this that @ulysses4ever remembers.

ulysses4ever · 2026-01-26T15:51:38Z

I don't have any reasoning, just things I came to believe over time, overhearing various talks and reading various ancient threads on this bug tracker (no links, sorry). I'd ask Oleg hadn't he explicitly asked not to ping him on ongoing cabal matters.

For distributors, apart from Mike, I think it'd be prudent to ping @juhp for feedback. Maybe @maralorn could also opine. (Apologies in advance for a rather vague ping.) The question is whether you guys see any issue in Cabal-the-library requiring text>=2.

Bodigrim · 2026-01-26T22:25:58Z

Text 2.0 is 4 years old, so maybe it's time to switch? That obviously needs a major version, asking major lib users ahead of time (especially @mpilgrem and the illustrious distro maintainers) and I may still be missing some old promise or custom around this that @ulysses4ever remembers.

Stack already requires text>=2.1.3.

text-2.0 has been a boot package since GHC 9.4 and pretty much all major Linux distributives are already shipping it or newer.

maralorn · 2026-01-27T07:31:28Z

Yeah, I this would be no problem on Nixos.

Bodigrim · 2026-01-29T01:09:42Z

@ulysses4ever @Mikolaj @geekosaur is this enough of assurances to go ahead with this or do you have further reservations?

ulysses4ever · 2026-01-29T01:14:51Z

With respect to text 2, yes, thank you!

Is there no way to avoid depending on an internal module?

geekosaur · 2026-01-29T01:40:30Z

Cabal-syntax/src/Distribution/Parsec/FieldLineStream.hs

-              bs1
-              ((acc `shiftL` 6) .|. fromIntegral cn .&. 0x3F)
-        | otherwise -> (replacementChar, f bs1)
+    go decoderResult bs = case decoderResult of


Given that we're using the same replacementChar, couldn't we just use Data.Text.Encoding.decodeUtf8Lenient? (It's also in 2.0, if you're worried about that.)

This is all a bit awkward for historical reasons. In the modern Haskell ecosystem Cabal-syntax should not be parsing ByteString but with UTF-8 semantics, this is obviously stupid. And all FieldLineStream and FieldLine should wrap Text, not ByteString. But fixing it properly is a significant breaking change and I don't want to disrupt ongoing work on Cabal formatters.

Bodigrim · 2026-02-01T17:10:07Z

Is there no way to avoid depending on an internal module?

It's possible with decodeUtf8Chunk and decodeUtf8More, but very awkward, to a degree that I'd rather revert changes to Distribution.Parsec.FieldLineStream if it proves to be a sticking point. What do you think?

mpilgrem · 2026-02-01T20:24:26Z

Stack would be unaffected:

Stack builds (by default) with the version of Cabal that is the boot package of the specified GHC version
Stack's own reliance on Cabal is on a single version - most often the version in the most recent Stackage LTS Haskell snapshot (unless there is a compelling reason to jump forwards).

Bodigrim · 2026-02-03T23:04:23Z

It's possible with decodeUtf8Chunk and decodeUtf8More, but very awkward, to a degree that I'd rather revert changes to Distribution.Parsec.FieldLineStream if it proves to be a sticking point. What do you think?

@ulysses4ever @Mikolaj @geekosaur @ffaf1?

(While my preference is to keep changes, I will not be grossly offended if you ask me to revert. I'd rather move forward one way or another then drain myself on a protracted dicussion about a relatively minor change)

ulysses4ever

I don't have an informed opinion and will trust your judgement w.r.t. internals.

Otherwise, great job!

geekosaur · 2026-02-03T23:46:27Z

Artem was the one concerned about it; I was just making a suggestion, but in thinking about it the whole Text thing probably means a different internal function to access the resulting Text directly to make it a ByteString.

So probably just go with it as is until we have a chance to redesign the APIs to use Text like they should have in the first place.

Bodigrim force-pushed the utf8 branch 3 times, most recently from 52b3ba5 to 550ea8f Compare January 25, 2026 14:14

Bodigrim marked this pull request as ready for review January 25, 2026 14:14

Bodigrim added 2 commits January 25, 2026 17:43

Use text package to decode UTF-8

99fd8e7

Bodigrim force-pushed the utf8 branch from 550ea8f to 9c985e5 Compare January 25, 2026 17:44

Bodigrim force-pushed the utf8 branch from 9c985e5 to d53f2d3 Compare January 25, 2026 19:16

Require text>=2.0.2 to eliminate CPP

0c8ecea

Bodigrim force-pushed the utf8 branch from d53f2d3 to 0c8ecea Compare January 25, 2026 19:20

geekosaur reviewed Jan 29, 2026

View reviewed changes

ulysses4ever approved these changes Feb 3, 2026

View reviewed changes

Use text package to decode UTF-8 instead of doing it ourselves #11462

Are you sure you want to change the base?

Use text package to decode UTF-8 instead of doing it ourselves #11462

Conversation

Bodigrim commented Jan 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ulysses4ever commented Jan 25, 2026

Uh oh!

Bodigrim commented Jan 25, 2026

Uh oh!

geekosaur commented Jan 25, 2026

Uh oh!

Bodigrim commented Jan 25, 2026

Uh oh!

geekosaur commented Jan 25, 2026

Uh oh!

ulysses4ever commented Jan 25, 2026

Uh oh!

Bodigrim commented Jan 25, 2026

Uh oh!

geekosaur commented Jan 25, 2026

Uh oh!

Bodigrim commented Jan 25, 2026

Uh oh!

Mikolaj commented Jan 26, 2026

Uh oh!

ulysses4ever commented Jan 26, 2026

Uh oh!

Bodigrim commented Jan 26, 2026

Uh oh!

maralorn commented Jan 27, 2026

Uh oh!

Bodigrim commented Jan 29, 2026

Uh oh!

ulysses4ever commented Jan 29, 2026

Uh oh!

geekosaur Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

Bodigrim Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

Bodigrim commented Feb 1, 2026

Uh oh!

mpilgrem commented Feb 1, 2026

Uh oh!

Bodigrim commented Feb 3, 2026

Uh oh!

ulysses4ever left a comment

Choose a reason for hiding this comment

Uh oh!

geekosaur commented Feb 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Bodigrim commented Jan 24, 2026 •

edited

Loading