Closure property for multiple home units is not enforced nor checked #3422

mpickering · 2022-12-23T10:25:27Z

The blog post [The Interface for Multiple Home Units](https://well-typed.com/blog/2022/01/multiple-home-units/#closure-property-for-home-units) explains the closure property needed for setting up a multiple component session.

HLS does not ensure this holds which leads to broken sessions when loading multiple components. For example.. random overlapping instance errors, panics in checkFamConst etc etc. It is critical that any session which HLS constructs obeys the closure property.

Stated simply the closure property is:

Any dependency which is not a home unit must not (transitively) depend on a home unit.

And in terms of implementation there is a function in GHC called checkHomeUnitsClosed which implements this check and reports the reason why it's not closed.

My proposal is

If a new unit is added and the closure property doesn't hold, then the error is reported prominently to the user and the new component is ignored completely by the IDE.
If a user then subsequently adds the units needed for closure then the session can resume in a multi-component manner.

My experience is that multi-component works without problem as long as the closure property. I spent the last week using the support whilst developing cabal-install, which has 5 components.

In the future:

There should be a better interface for initialising a session which contains multiple components rather than having to create the session incrementally one component at a time.

The text was updated successfully, but these errors were encountered:

michaelpj · 2022-12-23T16:57:33Z

Users don't "add units", so I'm not sure that the workflow you lay out can map into HLS. I guess the closest thing that would correspond to is:

If a user opens a file in a unit such that adding it would break the closure property, then we give them an error and ignore the file.
If a user later opens a different files (leading to more units being added) such that the original file is now fine, then when they revisit it we should try to load it again and succeed.

This is a slightly odd situation anyway: it supposes that we have two units A and B such that A depends on B and the user can open files from both. In that scenario why would we not immediately load B as a home unit as soon as the user opens a file in A?

The problematic case is I guess where we have A -> B -> C where A and C are local but B is not. That does seem tricky and we'll probably have to give an error. But if all the units are local I don't see why we can't just load them all as home units to begin with?

pepeiborra · 2022-12-23T19:00:46Z

I made a similar proposal a while ago: haskell/hie-bios#269

The problem back then is that the user doesn't really know which dependency is breaking the closure property. It sounds like checkHomeUnitsClosed addresses this, which is awesome.

Agree with Michael, it would be best if HLS could parse the result of checkHomeUnitsClosed and automatically load the necessary units to restore the closure property, where possible, otherwise report the error.

@unitA

…ttps://downloads.haskell.org/ghc/9.4.4/docs/users_guide/using.html#multiple-home-units We now support arguments of the form ``` -unit @unitA -unit @unitb ``` where the response files `unitA` and `unitB` contain the actual list of arguments for that unit: ``` -this-unit-id a-0.1.0.0 -i -isrc A1 A2 ``` Also refactor the session loader and simplify it. Also adds error messages on GHC 9.4 if the units are not closed (#3422).

@unitA

…ttps://downloads.haskell.org/ghc/9.4.4/docs/users_guide/using.html#multiple-home-units We now support arguments of the form ``` -unit @unitA -unit @unitb ``` where the response files `unitA` and `unitB` contain the actual list of arguments for that unit: ``` -this-unit-id a-0.1.0.0 -i -isrc A1 A2 ``` Also refactor the session loader and simplify it. Also adds error messages on GHC 9.4 if the units are not closed (#3422).

@unitA

…ttps://downloads.haskell.org/ghc/9.4.4/docs/users_guide/using.html#multiple-home-units We now support arguments of the form ``` -unit @unitA -unit @unitb ``` where the response files `unitA` and `unitB` contain the actual list of arguments for that unit: ``` -this-unit-id a-0.1.0.0 -i -isrc A1 A2 ``` Also refactor the session loader and simplify it. Also adds error messages on GHC 9.4 if the units are not closed (#3422).

@unitA

…ttps://downloads.haskell.org/ghc/9.4.4/docs/users_guide/using.html#multiple-home-units We now support arguments of the form ``` -unit @unitA -unit @unitb ``` where the response files `unitA` and `unitB` contain the actual list of arguments for that unit: ``` -this-unit-id a-0.1.0.0 -i -isrc A1 A2 ``` Also refactor the session loader and simplify it. Also adds error messages on GHC 9.4 if the units are not closed (#3422).

@unitA

…ttps://downloads.haskell.org/ghc/9.4.4/docs/users_guide/using.html#multiple-home-units We now support arguments of the form ``` -unit @unitA -unit @unitb ``` where the response files `unitA` and `unitB` contain the actual list of arguments for that unit: ``` -this-unit-id a-0.1.0.0 -i -isrc A1 A2 ``` Also refactor the session loader and simplify it. Also adds error messages on GHC 9.4 if the units are not closed (#3422).

@unitA

…ttps://downloads.haskell.org/ghc/9.4.4/docs/users_guide/using.html#multiple-home-units We now support arguments of the form ``` -unit @unitA -unit @unitb ``` where the response files `unitA` and `unitB` contain the actual list of arguments for that unit: ``` -this-unit-id a-0.1.0.0 -i -isrc A1 A2 ``` Also refactor the session loader and simplify it. Also adds error messages on GHC 9.4 if the units are not closed (#3422).

@unitA

…ttps://downloads.haskell.org/ghc/9.4.4/docs/users_guide/using.html#multiple-home-units We now support arguments of the form ``` -unit @unitA -unit @unitb ``` where the response files `unitA` and `unitB` contain the actual list of arguments for that unit: ``` -this-unit-id a-0.1.0.0 -i -isrc A1 A2 ``` Also refactor the session loader and simplify it. Also adds error messages on GHC 9.4 if the units are not closed (#3422).

@unitA

…ttps://downloads.haskell.org/ghc/9.4.4/docs/users_guide/using.html#multiple-home-units We now support arguments of the form ``` -unit @unitA -unit @unitb ``` where the response files `unitA` and `unitB` contain the actual list of arguments for that unit: ``` -this-unit-id a-0.1.0.0 -i -isrc A1 A2 ``` Also refactor the session loader and simplify it. Also adds error messages on GHC 9.4 if the units are not closed (#3422). fixes Fix closure check session-loader: override old units with new in multi-unit support

@unitA

…ttps://downloads.haskell.org/ghc/9.4.4/docs/users_guide/using.html#multiple-home-units We now support arguments of the form ``` -unit @unitA -unit @unitb ``` where the response files `unitA` and `unitB` contain the actual list of arguments for that unit: ``` -this-unit-id a-0.1.0.0 -i -isrc A1 A2 ``` Also refactor the session loader and simplify it. Also adds error messages on GHC 9.4 if the units are not closed (#3422). fixes Fix closure check session-loader: override old units with new in multi-unit support

@unitA

…ttps://downloads.haskell.org/ghc/9.4.4/docs/users_guide/using.html#multiple-home-units We now support arguments of the form ``` -unit @unitA -unit @unitb ``` where the response files `unitA` and `unitB` contain the actual list of arguments for that unit: ``` -this-unit-id a-0.1.0.0 -i -isrc A1 A2 ``` Also refactor the session loader and simplify it. Also adds error messages on GHC 9.4 if the units are not closed (#3422). fixes Fix closure check session-loader: override old units with new in multi-unit support

@unitA

…ttps://downloads.haskell.org/ghc/9.4.4/docs/users_guide/using.html#multiple-home-units We now support arguments of the form ``` -unit @unitA -unit @unitb ``` where the response files `unitA` and `unitB` contain the actual list of arguments for that unit: ``` -this-unit-id a-0.1.0.0 -i -isrc A1 A2 ``` Also refactor the session loader and simplify it. Also adds error messages on GHC 9.4 if the units are not closed (#3422). fixes Fix closure check session-loader: override old units with new in multi-unit support

@unitA

…ttps://downloads.haskell.org/ghc/9.4.4/docs/users_guide/using.html#multiple-home-units We now support arguments of the form ``` -unit @unitA -unit @unitb ``` where the response files `unitA` and `unitB` contain the actual list of arguments for that unit: ``` -this-unit-id a-0.1.0.0 -i -isrc A1 A2 ``` Also refactor the session loader and simplify it. Also adds error messages on GHC 9.4 if the units are not closed (#3422). fixes Fix closure check session-loader: override old units with new in multi-unit support

@unitA

…ttps://downloads.haskell.org/ghc/9.4.4/docs/users_guide/using.html#multiple-home-units We now support arguments of the form ``` -unit @unitA -unit @unitb ``` where the response files `unitA` and `unitB` contain the actual list of arguments for that unit: ``` -this-unit-id a-0.1.0.0 -i -isrc A1 A2 ``` Also refactor the session loader and simplify it. Also adds error messages on GHC 9.4 if the units are not closed (#3422). fixes Fix closure check session-loader: override old units with new in multi-unit support

@unitA

…ttps://downloads.haskell.org/ghc/9.4.4/docs/users_guide/using.html#multiple-home-units We now support arguments of the form ``` -unit @unitA -unit @unitb ``` where the response files `unitA` and `unitB` contain the actual list of arguments for that unit: ``` -this-unit-id a-0.1.0.0 -i -isrc A1 A2 ``` Also refactor the session loader and simplify it. Also adds error messages on GHC 9.4 if the units are not closed (#3422). fixes Fix closure check session-loader: override old units with new in multi-unit support Remove implicit-hie session-loader: remember which files caused old components to be loaded, and also pass them on to hie-bios so it can in turn pass them to `cabal repl` when loading newer components. This allows us to create valid set of build flags encompassing both the old and new components, and the closure of all components in between. The observation is that if you want to load some components X, Y, Z and so on, cabal repl X Y Z ... will be more likely to give you a valid multi component build plan/flags than cabal repl all, or any way of combining the results of cabal repl X, cabal repl Y ... Use new hie-bios Move implicit cradles to HLS Fix build on 9.0 Werror Improve handling of specialTarget

@unitA

* Add support for the multi unit argument syntax introduced in GHC 9.4: https://downloads.haskell.org/ghc/9.4.4/docs/users_guide/using.html#multiple-home-units We now support arguments of the form ``` -unit @unitA -unit @unitb ``` where the response files `unitA` and `unitB` contain the actual list of arguments for that unit: ``` -this-unit-id a-0.1.0.0 -i -isrc A1 A2 ``` Also refactor the session loader and simplify it. Also adds error messages on GHC 9.4 if the units are not closed (#3422). fixes Fix closure check session-loader: override old units with new in multi-unit support Remove implicit-hie session-loader: remember which files caused old components to be loaded, and also pass them on to hie-bios so it can in turn pass them to `cabal repl` when loading newer components. This allows us to create valid set of build flags encompassing both the old and new components, and the closure of all components in between. The observation is that if you want to load some components X, Y, Z and so on, cabal repl X Y Z ... will be more likely to give you a valid multi component build plan/flags than cabal repl all, or any way of combining the results of cabal repl X, cabal repl Y ... Use new hie-bios Move implicit cradles to HLS Fix build on 9.0 Werror Improve handling of specialTarget * hie-bios doesn't mention the component name in the message anymore * stack fixes * wrapper: remove unused argument * werror * werror * Implicit cradle: match implicit-hie-cradle logic * Fix eval plugin * ignore multi unit tests on 9.2 * Some fixes for 9.2 * Add hie.yaml for call-hierarchy-plugin tests * Add hie.yaml for explicit-record-fields-plugin * Add hie.yaml for hls-overloaded-record-dot-plugin

michaelpj · 2024-01-16T18:32:26Z

Sounds like we added a check.

mpickering added type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc.. status: needs triage labels Dec 23, 2022

mpickering changed the title ~~Closure property for multiple home units is not enforced or checked~~ Closure property for multiple home units is not enforced nor checked Dec 23, 2022

michaelpj removed the status: needs triage label Jan 10, 2023

wz1000 mentioned this issue Jan 23, 2023

Add support for multi unit argument syntax #3462

Merged

brprice mentioned this issue May 17, 2023

chore: bump HLS pin hackworthltd/primer#995

Closed

georgefst mentioned this issue May 17, 2023

Weird errors when a local intermediate component is not loaded #1370

Closed

michaelpj closed this as completed Jan 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Closure property for multiple home units is not enforced nor checked #3422

Closure property for multiple home units is not enforced nor checked #3422

mpickering commented Dec 23, 2022

michaelpj commented Dec 23, 2022

pepeiborra commented Dec 23, 2022

michaelpj commented Jan 16, 2024

Closure property for multiple home units is not enforced nor checked #3422

Closure property for multiple home units is not enforced nor checked #3422

Comments

mpickering commented Dec 23, 2022

michaelpj commented Dec 23, 2022

pepeiborra commented Dec 23, 2022

michaelpj commented Jan 16, 2024