Skip to content

replace memory with ram#573

Closed
jappeace wants to merge 3 commits intosnoyberg:masterfrom
jappeace:replace-mem-with-ram
Closed

replace memory with ram#573
jappeace wants to merge 3 commits intosnoyberg:masterfrom
jappeace:replace-mem-with-ram

Conversation

@jappeace
Copy link

@jappeace jappeace commented Mar 5, 2026

attempt to get rid of basement
we can't modify memory but I can replace it with a copy which no longer uses basement.

for context: https://discourse.haskell.org/t/fork-basement-as-baseplate/12415/72
cf: kazu-yamamoto/crypton#67

my little worker bee asserted the tests pass and it builds: jappeace#1

@kazu-yamamoto
Copy link
Collaborator

Cc: me

@Yuras
Copy link
Collaborator

Yuras commented Mar 7, 2026

I'm not a maintainer, so the following is just a personal opinion.
If I understand correctly, now htttp-client-tls will build with tls-2.3.0, but not with tls-2.2.2. Is it correct?
Is there way to avoid it, e.g. put memory vs ram behind a flag? If not, then IMO tls version bound needs to be updated.

@jappeace
Copy link
Author

jappeace commented Mar 7, 2026

@Yuras the idea is that we swap all of these libraries over in one go.
use tight upper bounds if you don't want to switch yet.

NB: I wanted to get memory maintainership preferably, but that doesn't seem possible right now, so this is the next best thing.

@Yuras
Copy link
Collaborator

Yuras commented Mar 7, 2026

the idea is that we swap all of these libraries over in one go.

@jappeace OK, but then it makes sense to update tls upper lower bound, doesn't it? Right now it's tls >= 2.1.2, why would you keep it if you know http-client-tls won't build with tls earlier than 2.3.0?

this is the next best thing.

Probably I'm misunderstanding what's going on here, but won't it be better if http-client-tls would support both? That way we'll avoid ripple effect throughout the ecosystem. Assuming it's possible to support both of course. Am I missing something?

@jappeace
Copy link
Author

jappeace commented Mar 7, 2026

but then it makes sense to update tls upper bound, doesn't it?

yes it will allow cabal to make better plans. by default it'll pick "as high as possible".
one issue is that previous versions would've needed upper bounds for this to be opt in as well.

Someone would have to go trough all these http-client-tls hackage releases and add upperbounds to tls for example.

(I think hackage maintainers allow bound changes for this exact reason).

Probably I'm misunderstanding what's going on here, but won't it be better if http-client-tls would support both?

it's not trivial to do. ram exposes the same api as memory (it's a little different because we rely on base and ghc-prim instead of basement),
however you'll get type errors if you mix types from ram and memory even though they have the same name.
It'd be confusing!

For this reason by default cabal doesn't allow you to install multiple versions of the same library. But this rename has caused this situation to occur anyway. 😔

if you're interested there is a discussion on all this: https://discourse.haskell.org/t/fork-basement-as-baseplate/12415/77

@Yuras
Copy link
Collaborator

Yuras commented Mar 7, 2026

I have an impression we are talking past each other. I guess a typo in my previous comment contributed to the confusion, sorry for that.
I'm not suggesting to update previous releases, just to change tls >= 2.1.2 here to tls >= 2.3.0 to reflect the reality.

it's not trivial to do.

OK, if you say so. Though I don't see why it won't work. Just to make sure we are talking about the same thing, by "hiding it behind a flag" I meant adding cabal flag in http-client-tls.caba, something like this:

flag memory
  default: false
  manual: true -- or even false to let cabal decide?
-- <..>
library
  if flag(memory)
    build-depends: memory
  else
    build-depends: ram

That way someone can e.g. temporary set http-client-tls +memory in their cabal.project.lock file while waiting for the ecosystem to catch up.

if you're interested there is a discussion on all this: https://discourse.haskell.org/t/fork-basement-as-baseplate/12415/77

I didn't find any discussion of how to avoid ripple effect there. Though the thread is quite long, I could've missed it.

ADDED:
Probably even this:

flag memory
  default: false
  manual: false
-- <..>
library
  if flag(memory)
    build-depends: memory,   tls < 2.3.0
  else
    build-depends: ram,   tls >= 2.3.0

@sol
Copy link
Collaborator

sol commented Mar 7, 2026

@Yuras @jappeace I did not read the whole discussion, nor do I intend to, but:

  1. We don't want to support ram and memory at the same time (even if it were possible, it's just a waste of time and effort)
  2. Somebody has to add Hackage upper bounds to affected package, to rule out broken install plans. Right now, many things are just broken, which IMHO is not acceptable.

tls 2.3.0 is the first version using ram instead of memory.
Older tls versions (< 2.3.0) use memory and will cause type errors
when combined with this package which now depends on ram.
The upper bound ensures compatibility with the current tls API.

Prompt: add bounds to the PR such that this branch has a tls with
the ram change already included, but also a tls with a tight upper bound

Tokens used: ~200k

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tighten tls bounds to >= 2.3.0 && < 2.4
@jappeace
Copy link
Author

jappeace commented Mar 7, 2026

We've a tight bound on tls now for the next release in this PR.

it doesn't solve the issue where previous versions don't have upper bounds and I don't have permissions to change that either.

mpilgrem added a commit to sol/hpack that referenced this pull request Mar 7, 2026
See:
* #640
* #641
* snoyberg/http-client#573

The immediate problem is that direct dependency `http-client-tls <=
0.3.6.4` does not support newly-released `crypton-1.1.0` (an indirect
dependency of `hpack`) but does not, itself, specify an upper bound on
`crypton`.
@kazu-yamamoto
Copy link
Collaborator

My intention is:

  • If one package selects tls v2.2, memory is used.
  • If it chooses tls v2.3, ram is used.

The boundaries in tls.cabal of v2.2.2 was loose and I have just updated it on Hackage.
I hope this update implements my intention correctly.
If not, I should dig the source of this problem deeply.

@kazu-yamamoto
Copy link
Collaborator

@Yuras Do you still see the problem?

@Yuras
Copy link
Collaborator

Yuras commented Mar 7, 2026

@Yuras Do you still see the problem?

@kazu-yamamoto Sorry, I'm not sure which problem you are referring too. If it's about the lower bound on tls in http-client-tls, then @jappeace already corrected it, so I don't see anything wrong with the PR.
If you are asking about the cabal flag to support both older and newer versions of tls, then it was just a suggestion, there were no problem to begin with. And @sol clearly doesn't want it anyway.

@kazu-yamamoto
Copy link
Collaborator

@Yuras I guess I misunderstood and thought the problem was still there. Thank you for the clarification.

@snoyberg Could you merge this PR and release a new version?

@snoyberg
Copy link
Owner

snoyberg commented Mar 9, 2026

I'm not going to be able to do any uploads myself for a bit. I'll be happy to add you as a maintainer on Hackage, but for some reason I'm not able to load any pages on Hackage at the moment. I'll try again later. In the meanwhile, I've invited you as a maintainer on this repo as well.

@snoyberg
Copy link
Owner

snoyberg commented Mar 9, 2026

@kazu-yamamoto I've made you a maintainer on Hackage for all four packages in the repo.

@kazu-yamamoto
Copy link
Collaborator

@snoyberg Thanks!

Hackage is unstable recently: haskell/hackage-server#1467

May I ask to give me the Write permission of this repository to merge this PR?
Optionally, if it is hard for you to maintain these libraries, you might consider transferring this repository to yesodweb with new maintainers.

@Yuras
Copy link
Collaborator

Yuras commented Mar 9, 2026

May I ask to give me the Write permission of this repository to merge this PR?

@kazu-yamamoto Just to make sure you didn't miss it: the CI is failing because warp-tls is still using old tls: https://github.com/yesodweb/wai/blob/master/warp-tls/warp-tls.cabal#L25 I thought you are aware of it since you seems to be the maintainer of warp-tls, sorry if my assumption was wrong. (Was it the problem you were referring above? I might have misunderstood you.)
IIRC warp-tls is used in tests, so in theory we can merge the PR without waiting for warp-tls to be fixed, but IMO it's a bad idea.

Also I think technically I have right to merge into this repository, though I'd prefer to avoid doing this without maintainer's approval.

@kazu-yamamoto
Copy link
Collaborator

@Yuras I did not notice this. I think that relaxing the boundary of tls in warp-tls on Hackage is good enough.
I will do it.
But because Hackage is unstable, it would take time.

@kazu-yamamoto
Copy link
Collaborator

I should do the same thing for warp-quic.

@kazu-yamamoto
Copy link
Collaborator

@Yuras I think that this problem has been fixed.

@Yuras
Copy link
Collaborator

Yuras commented Mar 10, 2026

@kazu-yamamoto Indeed the code compiles now, but we have another problem. Tests are failing with the following error:

GHC.Internal.Event.Thread.getSystemTimerManager: the TimerManager requires linking against the threaded runtime

I managed to pin the issue down to warp. Specifically, tests pass with cabal test -c 'warp==3.4.9' all but fail with cabal test -c 'warp==3.4.10' all
I don't see anything in the changelog for warp-3.4.10 except a general note about dependencies. But in the repository I see a number of changes related to time manages. In particular this seems to be relevant: yesodweb/wai@32719ae

As a maintainer of warp, could you please clarify what might've caused the issue. Does warp not support non-threaded runtime anymore? I confirmed that, at least locally, enabling -threaded makes tests pass again.

@kazu-yamamoto
Copy link
Collaborator

time-manager v0.3 or later require threaded RTS.
You should use time-manager < 0.3.

@snoyberg
Copy link
Owner

@kazu-yamamoto it's just waiting on you accepting the invite

image

@kazu-yamamoto
Copy link
Collaborator

Oh. I just found your invitation! Thanks.

@kazu-yamamoto
Copy link
Collaborator

I think #572 is better than this PR.
So, I'm now working for #572.

@jappeace
Copy link
Author

yes that's better, thanks

@kazu-yamamoto
Copy link
Collaborator

Closing.

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.

5 participants