Replace flag-defaulting hack by proper solution #4886

hvr · 2017-11-12T08:56:15Z

This is not finished yet; this is supposed to address issues like #4737 or #5139

Regressions:

(note: this is specific to the all target)

$ cabal new-haddock all
Resolving dependencies...
fromFlag NoFlag. Use fromFlagOrDefault
CallStack (from HasCallStack):
  error, called at ./Distribution/Simple/Setup.hs:172:21 in Cabal-2.1.0.0-inplace:Distribution.Simple.Setup
  fromFlag, called at ./Distribution/Client/CmdHaddock.hs:156:32 in cabal-install-2.1.0.0-inplace:Distribution.Client.CmdHaddock

fgaz · 2017-11-12T12:06:46Z

Note to self: this may affect #4879

hvr · 2017-11-12T13:07:30Z

That CI passes is quite deceptive, as with the PR as it stands, cabal new-haddock all runs into a flag-related bottom rightaway... (cabal new-haddock libs however works fine)

fgaz · 2017-11-12T13:31:16Z

Can we remove that LANGUAGE ViewPatterns now or is it needed elsewhere?

hvr · 2017-11-12T13:32:43Z

@fgaz which one? my patch removes already a couple of those...

fgaz · 2017-11-12T15:46:12Z

@hvr I looked at CmdInstall and assumed you leaved it on the other modules too, but apparently I picked the exception, not the rule

23Skidoo · 2018-01-03T15:09:57Z

@hvr What's the status of this?

hvr · 2018-01-07T08:55:25Z

@23Skidoo I've just rebased this, and will try to use it locally and try to observe regressions (which I'll report here in this ticket); but generally I think we want to merge this, and fix fromFlag-related exceptions as they get observed/reported.

See haskell#4737

See haskell#4737 This reverts commit 71131cf.

See haskell#4737 This reverts commit 1e90ae4.

hvr · 2018-02-08T09:59:26Z

cabal-install/Distribution/Client/Setup.hs

@@ -1627,7 +1620,10 @@ installCommand = CommandUI {
     ++ "  " ++ (map (const ' ') pname)
                      ++ "                         "
     ++ "    Change installation destination\n",
-  commandDefaultFlags = (mempty, mempty, mempty, mempty),
+  commandDefaultFlags = (commandDefaultFlags configureCommand,


This is possibly wrong

nevermind, the commits were in the wrong orde

hvr · 2018-02-08T09:59:41Z

cabal-install/Distribution/Client/Setup.hs

@@ -1118,7 +1108,10 @@ upgradeCommand = configureCommand {
    commandSynopsis     = "(command disabled, use install instead)",
    commandDescription  = Nothing,
    commandUsage        = usageFlagsOrPackages "upgrade",
-    commandDefaultFlags = (mempty, mempty, mempty, mempty),
+    commandDefaultFlags = (commandDefaultFlags configureCommand,


This is possibly wrong

nevermind, the commits were in the wrong order

hvr · 2018-02-08T12:04:45Z

After brainstorming w/ @alexbiehl it appears like we want (c.f. #4737) something for the haddock flags in the style of

resolveSolverSettings :: ProjectConfig -> SolverSettings

resolveBuildTimeSettings :: Verbosity -> CabalDirLayout -> ProjectConfig -> BuildTimeSettings

specifically,

resolveHaddockSettings :: _ -> ProjectConfig -> HaddockSettings

(where _ is yet to be determined; could be empty) ...and a new type HaddockSettings to hold all/most haddock* fields from ProjectConfig.

EDIT: after some more thought; I'm not so sure about this anymore. It's more tricky

hvr · 2018-02-12T09:37:21Z

cabal-install/Distribution/Client/CmdHaddock.hs

-    isRequested _ ExeKind    = fromFlag (haddockExecutables haddockFlags)
-    isRequested _ TestKind   = fromFlag (haddockTestSuites  haddockFlags)
-    isRequested _ BenchKind  = fromFlag (haddockBenchmarks  haddockFlags)
+    isRequested _ FLibKind   = fromFlagOrDefault False (haddockForeignLibs haddockFlags)


in my local version I had a comment along the lines of

-- TODO/HACK, we encode some defaults here as new-haddock's logic; -- make sure this matches the defaults applied in -- "Distribution.Client.ProjectPlanning"; this may need more work -- to be done properly -- -- See also https://github.com/haskell/cabal/pull/4886

which may need to be extended even further to point out that this gives CLI flags unduly importance over per-package cabal.project settings; which is the thing we'd need to do properly in future work.

Will add the comment

hvr · 2018-02-12T18:37:23Z

I've merged this as this enables other work to be unblocked (such as #4840)

23Skidoo mentioned this pull request Nov 13, 2017

Global configuration for project commands #4888

Closed

4 tasks

hvr mentioned this pull request Dec 8, 2017

ld-options is not taken into account when passed by command line or cabal.project.local #4925

Closed

23Skidoo added the blocked: action-required label Dec 19, 2017

hvr force-pushed the pr/issue-4737 branch from eaf2763 to 4117304 Compare January 7, 2018 08:53

hvr added 3 commits February 8, 2018 10:54

Revert applyDefaults introduced via new-install & new-update

45b7334

See haskell#4737

Revert "Introduce applyFlagsDefault and use ViewPatterns"

5a31fda

See haskell#4737 This reverts commit 71131cf.

Revert "Use default flags rather than mempty."

f72eb1d

See haskell#4737 This reverts commit 1e90ae4.

hvr force-pushed the pr/issue-4737 branch from 4117304 to f72eb1d Compare February 8, 2018 09:55

hvr commented Feb 8, 2018

View reviewed changes

CmdHaddock: use fromFlagOrDefault

f36a01d

hvr commented Feb 12, 2018

View reviewed changes

CmdHaddock: Suggested comment

28b0b9e

hvr changed the title ~~WIP: Replace flag-defaulting hack by proper solution~~ Replace flag-defaulting hack by proper solution Feb 12, 2018

hvr added this to the 2.2 milestone Feb 12, 2018

hvr removed the blocked: action-required label Feb 12, 2018

hvr mentioned this pull request Feb 12, 2018

cabal new-configure dumps redundant settings into cabal.project.local file #5139

Closed

hvr merged commit abaf365 into haskell:master Feb 12, 2018

hvr deleted the pr/issue-4737 branch February 12, 2018 18:35

fgaz mentioned this pull request Sep 29, 2021

Install test executables with cabal install when "--enable-tests" present #7693

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace flag-defaulting hack by proper solution #4886

Replace flag-defaulting hack by proper solution #4886

hvr commented Nov 12, 2017 •

edited

Loading

fgaz commented Nov 12, 2017 •

edited

Loading

hvr commented Nov 12, 2017 •

edited

Loading

fgaz commented Nov 12, 2017

hvr commented Nov 12, 2017

fgaz commented Nov 12, 2017

23Skidoo commented Jan 3, 2018

hvr commented Jan 7, 2018 •

edited

Loading

hvr Feb 8, 2018

hvr Feb 8, 2018

hvr Feb 8, 2018

hvr Feb 8, 2018

hvr commented Feb 8, 2018 •

edited

Loading

hvr Feb 12, 2018

alexbiehl Feb 12, 2018

hvr commented Feb 12, 2018

Replace flag-defaulting hack by proper solution #4886

Replace flag-defaulting hack by proper solution #4886

Conversation

hvr commented Nov 12, 2017 • edited Loading

fgaz commented Nov 12, 2017 • edited Loading

hvr commented Nov 12, 2017 • edited Loading

fgaz commented Nov 12, 2017

hvr commented Nov 12, 2017

fgaz commented Nov 12, 2017

23Skidoo commented Jan 3, 2018

hvr commented Jan 7, 2018 • edited Loading

hvr Feb 8, 2018

Choose a reason for hiding this comment

hvr Feb 8, 2018

Choose a reason for hiding this comment

hvr Feb 8, 2018

Choose a reason for hiding this comment

hvr Feb 8, 2018

Choose a reason for hiding this comment

hvr commented Feb 8, 2018 • edited Loading

hvr Feb 12, 2018

Choose a reason for hiding this comment

alexbiehl Feb 12, 2018

Choose a reason for hiding this comment

hvr commented Feb 12, 2018

hvr commented Nov 12, 2017 •

edited

Loading

fgaz commented Nov 12, 2017 •

edited

Loading

hvr commented Nov 12, 2017 •

edited

Loading

hvr commented Jan 7, 2018 •

edited

Loading

hvr commented Feb 8, 2018 •

edited

Loading