ghc-options mega issue

[patrickdoc]

There are a bunch of tickets around some weird behaviors when trying to apply ghc-options. I've linked some below, but there are others that are semi-related as well.

This issue tries to capture most of the problems in a single place to resolve them.

There are a few ways to pass ghc-options (or, more generally ${program}-options for programs like ld, gcc, alex, happy)

cabal.project

In cabal.project:

-- Applies globally, see [1]
program-options
    ghc-options: -fno-full-laziness

-- Applies globally
package *
    ghc-options: -fno-full-laziness

-- Applies only to package foo
package foo
    ghc-options: -fno-full-laziness

[1] The configureCompiler phase pulls the options from the local config, which then gets applied to the entire plan's config, triggering a full rebuild. Reproducer:

cabal clean
cabal build all
# add "program-options" block to cabal.project
cabal build all
# all dependencies get built too

Problematic code:

cabal/cabal-install/src/Distribution/Client/ProjectPlanning.hs

Lines 464 to 515 in 65318cc

	-- Configure the compiler we're using.
	--
	-- This is moderately expensive and doesn't change that often so we cache
	-- it independently.
	--
	phaseConfigureCompiler :: ProjectConfig
	-> Rebuild (Compiler, Platform, ProgramDb)
	phaseConfigureCompiler ProjectConfig {
	projectConfigShared = ProjectConfigShared {
	projectConfigHcFlavor,
	projectConfigHcPath,
	projectConfigHcPkg
	},
	projectConfigLocalPackages = PackageConfig {
	packageConfigProgramPaths,
	packageConfigProgramArgs,
	packageConfigProgramPathExtra
	}
	} = do
	progsearchpath <- liftIO $ getSystemSearchPath
	rerunIfChanged verbosity fileMonitorCompiler
	(hcFlavor, hcPath, hcPkg, progsearchpath,
	packageConfigProgramPaths,
	packageConfigProgramArgs,
	packageConfigProgramPathExtra) $ do
	liftIO $ info verbosity "Compiler settings changed, reconfiguring..."
	result@(_, _, progdb') <- liftIO $
	Cabal.configCompilerEx
	hcFlavor hcPath hcPkg
	progdb verbosity
	-- Note that we added the user-supplied program locations and args
	-- for /all/ programs, not just those for the compiler prog and
	-- compiler-related utils. In principle we don't know which programs
	-- the compiler will configure (and it does vary between compilers).
	-- We do know however that the compiler will only configure the
	-- programs it cares about, and those are the ones we monitor here.
	monitorFiles (programsMonitorFiles progdb')
	return result
	where
	hcFlavor = flagToMaybe projectConfigHcFlavor
	hcPath = flagToMaybe projectConfigHcPath
	hcPkg = flagToMaybe projectConfigHcPkg
	progdb =
	userSpecifyPaths (Map.toList (getMapLast packageConfigProgramPaths))
	. userSpecifyArgss (Map.toList (getMapMappend packageConfigProgramArgs))
	. modifyProgramSearchPath
	(++ [ ProgramSearchPathDir dir
	| dir <- fromNubList packageConfigProgramPathExtra ])
	$ defaultProgramDb

Related tickets:

• Package sections in cabal.project are insufficient, need local section #3579 (includes a comment that the program-options stanza was intended to only apply locally)
• v2-cabal: Don't rebuild dependencies when -O0 or -fno-code is supplied as --ghc-options #7368
• ghc-options applies to all dependencies, not just local packages #3883

---

Command line

Input from the command line suffers a similar fate:

cabal clean
cabal build all
cabal build --ghc-options=-fno-full-laziness all
# all dependencies get built too

This time it is more intentional:

cabal/cabal-install/src/Distribution/Client/ProjectConfig/Legacy.hs

Lines 198 to 220 in 65318cc

	-- split the package config (from command line arguments) into
	-- those applied to all packages and those to local only.
	--
	-- for now we will just copy over the ProgramPaths/Args/Extra into
	-- the AllPackages. The LocalPackages do not inherit them from
	-- AllPackages, and as such need to retain them.
	--
	-- The general decision rule for what to put into allConfig
	-- into localConfig is the following:
	--
	-- - anything that is host/toolchain/env specific should be applied
	-- to all packages, as packagesets have to be host/toolchain/env
	-- consistent.
	-- - anything else should be in the local config and could potentially
	-- be lifted into all-packages vial the `package *` cabal.project
	-- section.
	--
	splitConfig :: PackageConfig -> (PackageConfig, PackageConfig)
	splitConfig pc = (pc
	, mempty { packageConfigProgramPaths = packageConfigProgramPaths pc
	, packageConfigProgramArgs = packageConfigProgramArgs pc
	, packageConfigProgramPathExtra = packageConfigProgramPathExtra pc
	, packageConfigDocumentation = packageConfigDocumentation pc })

This note indicates that the idea behind applying these options globally is that you want a consistent tool-chain for the entire build. While that seems reasonable for program paths, I can't think of any good examples for program args.

Related tickets:

• --prog-option(s) (e.g. gcc-options) CLI flags get duplicated #6183
• Explicitly support building with -fno-code #1176 (the last two comments are about trying to apply local-only flags)

---

Caching and Recompilation Avoidance

These problems are complicated by recompilation avoidance. Some flags do not affect output, so are excluded from the package hash. This same logic is used to determine whether or not the config has changed:

cabal/cabal-install/src/Distribution/Client/ProjectBuilding.hs

Lines 346 to 349 in 65318cc

	pkgFileMonitorConfig =
	FileMonitor {
	fileMonitorCacheFile = distPackageCacheFile dparams "config",
	fileMonitorKeyValid = (==) `on` normaliseConfiguredPackage shared,

This causes some flags to get stuck in the config cache until either a cabal clean or a non-ignored flag is applied. Reproducer:

cabal clean
cabal build --ghc-options=-ddump-simpl all
# -ddump flags are ignored, so don't trigger dependency rebuilds, but are included in fresh builds
# modify a module
cabal build all
# -ddump-simpl still applied

Related tickets:

• Some ghc-options should not be considered part of build hash #4247 (exclusion of certain flags from package hash)
• --ghc-options are cached, but shouldn't be #7816

---

Proposal

1. To fix the local flags applying globally, we should be able to just rip out the two linked pieces of code that duplicate the flags. Note that this code correctly constructs the list of flags anyway:

   cabal/cabal-install/src/Distribution/Client/ProjectPlanning.hs

   Lines 1969 to 1982 in 65318cc

   	lookupPerPkgOption :: (Package pkg, Monoid m)
   	=> pkg -> (PackageConfig -> m) -> m
   	lookupPerPkgOption pkg f =
   	-- This is where we merge the options from the project config that
   	-- apply to all packages, all project local packages, and to specific
   	-- named packages
   	global `mappend` local `mappend` perpkg
   	where
   	global = f allPackagesConfig
   	local | isLocalToProject pkg
   	= f localPackagesConfig
   	| otherwise
   	= mempty
   	perpkg = maybe mempty f (Map.lookup (packageName pkg) perPackageConfig)

This makes only-local flags possible, and the previous functionality is still available with:

package *
    ghc-options: ...

2. At that point it would probably be best to remove this stanza:

program-options
    ghc-options: ...

and make a top-level ghc-options legal to be consistent with all the other package settings. This is technically a breaking change, but the stanza is currently undocumented and broken, so it's unlikely to break too many workflows.

3. To fix the caching, the best option I can think of is:
   a. Only cache the normalized config so that we aren't left with a -ddump-simpl that we can't get rid of
   b. Use the non-normalized config when building so that the user still gets what they want

[jneira]

@patrickdoc hi! could be this closed after #7973?

[patrickdoc]

#7973 only addresses the first item under "proposal" (fixing the scope of command line and program-options to be local-only).

Item 2 is mostly just about cleaning up the docs/user interface for using ghc-options in cabal.project, so not a huge deal.

Item 3 is a "gotcha". Some flags (basically any options like -ddump or -Wall) can only be added/removed from a project by first running cabal clean. Not a huge problem because it has a workaround, but probably going to cause a ticket or two in the future.

It can probably be closed because 2 and 3 are not really bugs, just rough edges.

[jneira]

Oh, so i was wrong about #7973 fixing the cache issue and i have to reopen the linked issues (#4247 and #7816) 🤦

@patrickdoc could you confirm those ones should be reopened, please? thanks!

[jneira]

ok I ve reopened the issues related with caching, sorry for the noise!

[patrickdoc]

Closing #4247 seems correct to me. The original purpose of that ticket is achieved. (and is actually the cause of #7816)

#7816 is tracking the same issue as the one tracked here under "Caching and Recompilation Avoidance". So either one can be picked as the official one to track the issue.