LocalBuildInfo fields are poorly named or designed

[ttuegel]

Problems

with fields

These field names have the prefix with:

• withPrograms :: ProgramConfiguration
• withPackageDB :: PackageDBStack
• withVanillaLib :: Bool
• withProfLib :: Bool
• withSharedLib :: Bool
• withDynExe :: Bool
• withProfExe :: Bool
• withProfLibDetail :: ProfDetailLevel
• withProfExeDetail :: ProfDetailLevel
• withOptimization :: OptimisationLevel
• withDebugInfo :: DebugInfoLevel
• withGHCiLib :: Bool

In usual Haskell parlance, with indicates a function in continuation passing style, but these are simple accessors.

Singular/Plural

These fields refer to executables, libraries, or package databases in the singular, which is incorrect because we handle multiple of each simultaneously:

• withPackageDB :: PackageDBStack
• withVanillaLib :: Bool
• withProfLib :: Bool
• withSharedLib :: Bool
• withDynExe :: Bool
• withProfExe :: Bool
• withProfLibDetail :: ProfDetailLevel
• withProfExeDetail :: ProfDetailLevel

Duplicated fields

We have

• withProfLib :: Bool and withProfLibDetail :: ProfDetailLevel
• withProfExe :: Bool and withProfExeDetail :: ProfDetailLevel

when libProfiling :: Maybe ProfDetailLevel and exeProfiling :: Maybe ProfDetailLevel would be more idiomatic.

Essentially-unused fields

The program configuration in withPrograms :: ProgramConfiguration can't actually be serialized. Also, the programs in use can and often are reconfigured after the package is configured! Package configuration and program configuration are separate issues.

Proposed Solutions

1. Rename the with-prefixed fields.
2. Add fields libProfiling :: Maybe ProfDetailLevel and exeProfiling :: Maybe ProfDetailLevel.
3. Split LocalBuildInfo into PackageConfig (which would have most of the same fields as LocalBuildInfo) and a separate ProgramConfiguration.

Compatibility

I can add deprecated accessors for all the renamed and removed fields. However, code which assigns those fields will have to change.

Splitting up LocalBuildInfo internally does not need to affect external code because we can always reconstruct a LocalBuildInfo for legacy interfaces.

[ttuegel]

Related to refactoring discussion in #3589.

/cc @ezyang @23Skidoo @dcoutts

[ezyang]

• withPrograms has always been a giant wart. Good to get rid of this.
• OK, what are we going to rename the with prefixed fields to? I have started using the local* prefix to indicate LocalBuildInfo accessors but if you call it PackageConfig the name is less apt. Note that cabal-install already defines a PackageConfig so best not to clobber the name.
• Are you suggesting we keep around an old style legacy type LocalBuildInfo for BC purposes? (I suppose we could.)
• I get the feeling that the LocalBuildInfo split should be co-designed with the hooks API change. Do the hooks now take both PackageConfig and ProgramConfiguration? I guess there should be some environment type passed to the hook where it can get various information from?

[23Skidoo]

local* feels too generic, what about lbi*? Though I think that the with* prefix is not that bad in this case, since those fields correspond to various --with-* flags of configure.

2 and 3 sound good.

[ttuegel]

local* feels too generic, what about lbi_? Though I think that the with_ prefix is not that bad in this case, since those fields correspond to various --with-* flags of configure.

I was thinking of removing the prefixes entirely and using RecordWildCards, i.e.

• packageDBs :: PackageDBStack
• vanillaLibs :: Bool
• sharedLibs :: Bool
• ghciLibs :: Bool
• dynamicExes :: Bool
• optimisation :: OptimisationLevel

to facilitate LocalBuildInfo {..} in function headers, but I don't know how everyone feels about that.

I actually don't like how similar they are to the configure flags' names; they are emphatically NOT the flags passed to configure. In fact, we have some code that skims the ConfigFlags at build time, when it should really be reading settings from here.

[ezyang]

My uninformed opinion about record wild cards is they should only be used when it is manifestly obvious where the names come from. Otherwise I find it is too confusing to figure out the binding site of a record wild card.

[ttuegel]

I agree, but I think that it is unambiguous because the LocalBuildInfo should be the canonical source of all the settings it contains.

[ezyang]

I mean, I'm not going to block you from using record wild cards if you really want to, but I will personally only use them in boilery-platey code; if it's a function like configure I'd rather play nice and use the accessor manually.

[ttuegel]

I mean, I'm not going to block you from using record wild cards if you really want to, but I will personally only use them in boilery-platey code; if it's a function like configure I'd rather play nice and use the accessor manually.

Well, I will think more about how best to do this. It's not much of a "solution" if it doesn't meet everyone's needs! RecordWildCards was also my boilerplate-reduction strategy for the command-line flag structs. I definitely want to get this right; there's no need for haste!

[ezyang]

I think the biggest problem people frequently have with RecordWildCards is that these bindings do shadow, and Cabal currently strives to be shadow free.

[dcoutts]

Separating the program config is absolutely the right thing to do. The only reason they're currently mixed up was to avoid breaking the user hooks API.