Some fixes for multi component stuff#3686
Conversation
wz1000
force-pushed
the
wip/multi-comp-fixes
branch
from
bf4419b to
021ecf4
Compare
fendor
left a comment
fendor
left a comment
There was a problem hiding this comment.
Minor comments, looks good to me!
| case unitIdString (homeUnitId_ df) of | ||
| -- cabal uses main for the unit id of all executable packages | ||
| -- This makes multi-component sessions confused about what | ||
| -- options to use for that component. | ||
| -- Solution: hash the options and use that as part of the unit id | ||
| -- This works because there won't be any dependencies on the | ||
| -- executable unit. |
There was a problem hiding this comment.
I think haskell/cabal#8726 fixes this, right? If so, I think we should include that here.
There was a problem hiding this comment.
We don't have any way of knowing what cabal version we're working with? In the future will we support MHU only with a sufficiently new cabal? If so then perhaps this needs a note saying when we can throw it away.
There was a problem hiding this comment.
In the future, we will likely have support for older cabal versions as well, but it will be slightly hacky.
Is this a cabal bug? Presumably this means that e.g. a multi-repl with multiple executables will go wrong also? |
|
Ah I see, Fendor says it might be fixed. |
| case unitIdString (homeUnitId_ df) of | ||
| -- cabal uses main for the unit id of all executable packages | ||
| -- This makes multi-component sessions confused about what | ||
| -- options to use for that component. | ||
| -- Solution: hash the options and use that as part of the unit id | ||
| -- This works because there won't be any dependencies on the | ||
| -- executable unit. |
There was a problem hiding this comment.
We don't have any way of knowing what cabal version we're working with? In the future will we support MHU only with a sufficiently new cabal? If so then perhaps this needs a note saying when we can throw it away.
wz1000
force-pushed
the
wip/multi-comp-fixes
branch
from
021ecf4 to
60703f0
Compare
wz1000
force-pushed
the
wip/multi-comp-fixes
branch
from
60703f0 to
3123d59
Compare
|
Test failures seem maybe genuine? |
|
I don't think so, different tests fail every time |
wz1000
force-pushed
the
wip/multi-comp-fixes
branch
from
08624ea to
fda63c5
Compare
|
IDK, it looks like it's almost always the |
|
Yeah, you are probably right, I see consistent failures in the 9.4+ jobs, I was looking at the <=9.2 jobs and saw random failures earlier. I will investigate. |
…fused multi component sessions. Solution: include the hash of the options in the unit id when the unit id is called "main". Fixes #3513
wz1000
force-pushed
the
wip/multi-comp-fixes
branch
from
fda63c5 to
8ea0fee
Compare
|
Turned out to be a missing fallthrough case that snuck in while cherry picking the commit 🤦 |
|
I've had my eye on this PR for a while since #3513 has been really annoying me, and for the record, it now works perfectly for my projects, where previous iterations before today would hang at initialisation. Thanks for the fix! |
4 participants
mainas the unit id of all executable packages. This confused multi component sessions.Solution: include the hash of the options in the unit id when the unit id is called "main".
Fixes #3513