Add new-haddock support for `--haddock-for-hackage` #4490

noisegul · 2017-05-03T16:22:23Z

Greetings everyone, this is my first pull request here.
I added support for the --haddock-for-hackage flag, the associated issue is #4412 .
I'm happy for any kind of feedback.

mention-bot · 2017-05-03T16:22:27Z

@bitrauser, thanks for your PR! By analyzing the history of the files in this pull request, we identified @grayjay, @dcoutts and @ezyang to be potential reviewers.

noisegul · 2017-05-03T18:17:15Z

There are still two Orphan instances for instance Binary HaddockTarget in my PR, located at:
Distribution/Client/ProjectConfig/Types.hs and Distribution/Client/ProjectPlanning/Types.hs

Currently trying to fix those.

ezyang · 2017-05-05T03:34:59Z

Re the test suite failures, look at legacyPackageConfigFieldDescrs, you need to add your new flag to this block of code:

  ( liftFields
      legacyHaddockFlags
      (\flags conf -> conf { legacyHaddockFlags = flags })
  . mapFieldNames
      ("haddock-"++)
  . filterFields
      [ "hoogle", "html", "html-location"
      , "foreign-libraries"
      , "executables", "tests", "benchmarks", "all", "internal", "css"
      , "hyperlink-source", "hscolour-css"
      , "contents-location", "keep-temp-files"
      ]
  . commandOptionsToFields
  ) (haddockOptions ParseArgs)

Unfortunately the need to make the change here is irritatingly non-discoverable.

noisegul · 2017-05-05T11:17:03Z

Thanks! That one is hard to find indeed.

I added it and updated my PR, however the unit-tests still fail at ProjectConfig printing/parsing round trip (local/specific/all), the same way as before, as of now.

ezyang · 2017-05-05T14:06:24Z

I don't see any obvious problems, so it's time to put on the debugging gloves.

First, you should read the test, and make sure you understand what it is actually testing for. The test properties are all reasonably simple.
Next, you should look carefully at the failing counterexample, and verify that it is indeed related to the new flag.
This test involves a number of stages, so it might be helpful to see the intermediate results. There are a number of ways you might do this; printf debugging with Debug.Trace works, but you can also try cabal new-repl cabal. Here's a sample session (I don't have your branch built so I couldn't test your particular problem)

ezyang@sabre:~/Dev/cabal-shake$ cabal new-repl cabal
In order, the following will be built (use -v for more details):
 - cabal-install-2.1.0.0 +lib (ephemeral targets)
Preprocessing library for cabal-install-2.1.0.0..
Building library for cabal-install-2.1.0.0..
Preprocessing executable 'cabal' for cabal-install-2.1.0.0..
GHCi, version 8.0.1: http://www.haskell.org/ghc/  :? for help
*** WARNING: .ghci is writable by someone else, IGNORING!
Suggested fix: execute 'chmod go-w .ghci'
[1 of 2] Compiling Paths_cabal_install ( /srv/code/cabal-shake/dist-newstyle/build/x86_64-linux/ghc-8.0.1/cabal-install-2.1.0.0/noopt/build/cabal/autogen/Paths_cabal_install.hs, interpreted )
[2 of 2] Compiling Main             ( main/Main.hs, interpreted )
Ok, modules loaded: Paths_cabal_install, Main.
*Main> import Distribution.Client.ProjectConfig
*Main Distribution.Client.ProjectConfig> mempty :: ProjectConfig
ProjectConfig {projectPackages = [], projectPackagesOptional = [], projectPackagesRepo = [], projectPackagesNamed = [], projectConfigBuildOnly = ProjectConfigBuildOnly {projectConfigVerbosity = NoFlag, projectConfigDryRun = NoFlag, projectConfigOnlyDeps = NoFlag, projectConfigSummaryFile = [], projectConfigLogFile = NoFlag, projectConfigBuildReports = NoFlag, projectConfigReportPlanningFailure = NoFlag, projectConfigSymlinkBinDir = NoFlag, projectConfigOneShot = NoFlag, projectConfigNumJobs = NoFlag, projectConfigKeepGoing = NoFlag, projectConfigOfflineMode = NoFlag, projectConfigKeepTempFiles = NoFlag, projectConfigHttpTransport = NoFlag, projectConfigIgnoreExpiry = NoFlag, projectConfigCacheDir = NoFlag, projectConfigLogsDir = NoFlag}, projectConfigShared = ProjectConfigShared {projectConfigDistDir = NoFlag, projectConfigProjectFile = NoFlag, projectConfigHcFlavor = NoFlag, projectConfigHcPath = NoFlag, projectConfigHcPkg = NoFlag, projectConfigHaddockIndex = NoFlag, projectConfigRemoteRepos = [], projectConfigLocalRepos = [], projectConfigIndexState = NoFlag, projectConfigConstraints = [], projectConfigPreferences = [], projectConfigCabalVersion = NoFlag, projectConfigSolver = NoFlag, projectConfigAllowOlder = Nothing, projectConfigAllowNewer = Nothing, projectConfigMaxBackjumps = NoFlag, projectConfigReorderGoals = NoFlag, projectConfigCountConflicts = NoFlag, projectConfigStrongFlags = NoFlag, projectConfigAllowBootLibInstalls = NoFlag, projectConfigPerComponent = NoFlag}, projectConfigProvenance = fromList [], projectConfigLocalPackages = PackageConfig {packageConfigProgramPaths = MapLast {getMapLast = fromList []}, packageConfigProgramArgs = MapMappend {getMapMappend = fromList []}, packageConfigProgramPathExtra = [], packageConfigFlagAssignment = [], packageConfigVanillaLib = NoFlag, packageConfigSharedLib = NoFlag, packageConfigDynExe = NoFlag, packageConfigProf = NoFlag, packageConfigProfLib = NoFlag, packageConfigProfExe = NoFlag, packageConfigProfDetail = NoFlag, packageConfigProfLibDetail = NoFlag, packageConfigConfigureArgs = [], packageConfigOptimization = NoFlag, packageConfigProgPrefix = NoFlag, packageConfigProgSuffix = NoFlag, packageConfigExtraLibDirs = [], packageConfigExtraFrameworkDirs = [], packageConfigExtraIncludeDirs = [], packageConfigGHCiLib = NoFlag, packageConfigSplitObjs = NoFlag, packageConfigStripExes = NoFlag, packageConfigStripLibs = NoFlag, packageConfigTests = NoFlag, packageConfigBenchmarks = NoFlag, packageConfigCoverage = NoFlag, packageConfigRelocatable = NoFlag, packageConfigDebugInfo = NoFlag, packageConfigRunTests = NoFlag, packageConfigDocumentation = NoFlag, packageConfigHaddockHoogle = NoFlag, packageConfigHaddockHtml = NoFlag, packageConfigHaddockHtmlLocation = NoFlag, packageConfigHaddockForeignLibs = NoFlag, packageConfigHaddockExecutables = NoFlag, packageConfigHaddockTestSuites = NoFlag, packageConfigHaddockBenchmarks = NoFlag, packageConfigHaddockInternal = NoFlag, packageConfigHaddockCss = NoFlag, packageConfigHaddockHscolour = NoFlag, packageConfigHaddockHscolourCss = NoFlag, packageConfigHaddockContents = NoFlag}, projectConfigSpecificPackage = MapMappend {getMapMappend = fromList []}}
*Main Distribution.Client.ProjectConfig> mempty { packageConfigHaddockBenchmarks = Flag True }
PackageConfig {packageConfigProgramPaths = MapLast {getMapLast = fromList []}, packageConfigProgramArgs = MapMappend {getMapMappend = fromList []}, packageConfigProgramPathExtra = [], packageConfigFlagAssignment = [], packageConfigVanillaLib = NoFlag, packageConfigSharedLib = NoFlag, packageConfigDynExe = NoFlag, packageConfigProf = NoFlag, packageConfigProfLib = NoFlag, packageConfigProfExe = NoFlag, packageConfigProfDetail = NoFlag, packageConfigProfLibDetail = NoFlag, packageConfigConfigureArgs = [], packageConfigOptimization = NoFlag, packageConfigProgPrefix = NoFlag, packageConfigProgSuffix = NoFlag, packageConfigExtraLibDirs = [], packageConfigExtraFrameworkDirs = [], packageConfigExtraIncludeDirs = [], packageConfigGHCiLib = NoFlag, packageConfigSplitObjs = NoFlag, packageConfigStripExes = NoFlag, packageConfigStripLibs = NoFlag, packageConfigTests = NoFlag, packageConfigBenchmarks = NoFlag, packageConfigCoverage = NoFlag, packageConfigRelocatable = NoFlag, packageConfigDebugInfo = NoFlag, packageConfigRunTests = NoFlag, packageConfigDocumentation = NoFlag, packageConfigHaddockHoogle = NoFlag, packageConfigHaddockHtml = NoFlag, packageConfigHaddockHtmlLocation = NoFlag, packageConfigHaddockForeignLibs = NoFlag, packageConfigHaddockExecutables = NoFlag, packageConfigHaddockTestSuites = NoFlag, packageConfigHaddockBenchmarks = Flag True, packageConfigHaddockInternal = NoFlag, packageConfigHaddockCss = NoFlag, packageConfigHaddockHscolour = NoFlag, packageConfigHaddockHscolourCss = NoFlag, packageConfigHaddockContents = NoFlag}
*Main Distribution.Client.ProjectConfig> mempty { projectConfigLocalPackages = mempty { packageConfigHaddockBenchmarks = Flag True } }
ProjectConfig {projectPackages = [], projectPackagesOptional = [], projectPackagesRepo = [], projectPackagesNamed = [], projectConfigBuildOnly = ProjectConfigBuildOnly {projectConfigVerbosity = NoFlag, projectConfigDryRun = NoFlag, projectConfigOnlyDeps = NoFlag, projectConfigSummaryFile = [], projectConfigLogFile = NoFlag, projectConfigBuildReports = NoFlag, projectConfigReportPlanningFailure = NoFlag, projectConfigSymlinkBinDir = NoFlag, projectConfigOneShot = NoFlag, projectConfigNumJobs = NoFlag, projectConfigKeepGoing = NoFlag, projectConfigOfflineMode = NoFlag, projectConfigKeepTempFiles = NoFlag, projectConfigHttpTransport = NoFlag, projectConfigIgnoreExpiry = NoFlag, projectConfigCacheDir = NoFlag, projectConfigLogsDir = NoFlag}, projectConfigShared = ProjectConfigShared {projectConfigDistDir = NoFlag, projectConfigProjectFile = NoFlag, projectConfigHcFlavor = NoFlag, projectConfigHcPath = NoFlag, projectConfigHcPkg = NoFlag, projectConfigHaddockIndex = NoFlag, projectConfigRemoteRepos = [], projectConfigLocalRepos = [], projectConfigIndexState = NoFlag, projectConfigConstraints = [], projectConfigPreferences = [], projectConfigCabalVersion = NoFlag, projectConfigSolver = NoFlag, projectConfigAllowOlder = Nothing, projectConfigAllowNewer = Nothing, projectConfigMaxBackjumps = NoFlag, projectConfigReorderGoals = NoFlag, projectConfigCountConflicts = NoFlag, projectConfigStrongFlags = NoFlag, projectConfigAllowBootLibInstalls = NoFlag, projectConfigPerComponent = NoFlag}, projectConfigProvenance = fromList [], projectConfigLocalPackages = PackageConfig {packageConfigProgramPaths = MapLast {getMapLast = fromList []}, packageConfigProgramArgs = MapMappend {getMapMappend = fromList []}, packageConfigProgramPathExtra = [], packageConfigFlagAssignment = [], packageConfigVanillaLib = NoFlag, packageConfigSharedLib = NoFlag, packageConfigDynExe = NoFlag, packageConfigProf = NoFlag, packageConfigProfLib = NoFlag, packageConfigProfExe = NoFlag, packageConfigProfDetail = NoFlag, packageConfigProfLibDetail = NoFlag, packageConfigConfigureArgs = [], packageConfigOptimization = NoFlag, packageConfigProgPrefix = NoFlag, packageConfigProgSuffix = NoFlag, packageConfigExtraLibDirs = [], packageConfigExtraFrameworkDirs = [], packageConfigExtraIncludeDirs = [], packageConfigGHCiLib = NoFlag, packageConfigSplitObjs = NoFlag, packageConfigStripExes = NoFlag, packageConfigStripLibs = NoFlag, packageConfigTests = NoFlag, packageConfigBenchmarks = NoFlag, packageConfigCoverage = NoFlag, packageConfigRelocatable = NoFlag, packageConfigDebugInfo = NoFlag, packageConfigRunTests = NoFlag, packageConfigDocumentation = NoFlag, packageConfigHaddockHoogle = NoFlag, packageConfigHaddockHtml = NoFlag, packageConfigHaddockHtmlLocation = NoFlag, packageConfigHaddockForeignLibs = NoFlag, packageConfigHaddockExecutables = NoFlag, packageConfigHaddockTestSuites = NoFlag, packageConfigHaddockBenchmarks = Flag True, packageConfigHaddockInternal = NoFlag, packageConfigHaddockCss = NoFlag, packageConfigHaddockHscolour = NoFlag, packageConfigHaddockHscolourCss = NoFlag, packageConfigHaddockContents = NoFlag}, projectConfigSpecificPackage = MapMappend {getMapMappend = fromList []}}
*Main Distribution.Client.ProjectConfig> convertToLegacyProjectConfig $ mempty { projectConfigLocalPackages = mempty { packageConfigHaddockBenchmarks = Flag True } }

<interactive>:7:1: error:
    Variable not in scope:
      convertToLegacyProjectConfig :: ProjectConfig -> t
*Main Distribution.Client.ProjectConfig> import Distribution.Client.ProjectConfig.Legacy
*Main Distribution.Client.ProjectConfig Distribution.Client.ProjectConfig.Legacy> convertToLegacyProjectConfig $ mempty { projectConfigLocalPackages = mempty { packageConfigHaddockBenchmarks = Flag True } }

<interactive>:10:1: error:
    • No instance for (Show LegacyProjectConfig)
        arising from a use of ‘print’
    • In a stmt of an interactive GHCi command: print it
*Main Distribution.Client.ProjectConfig Distribution.Client.ProjectConfig.Legacy> showLegacyProjectConfig . convertToLegacyProjectConfig $ mempty { projectConfigLocalPackages = mempty { packageConfigHaddockBenchmarks = Flag True } }
"haddock-benchmarks: True\n"

Bonus points if you fix the test to print this for you.

noisegul · 2017-05-08T08:02:21Z

Ok so apparently the problem is that the printparse roundtrip test fails. It checks whether the ProjectConfig stays unaltered after parsing it, by printing it out again and comparing.

The check fails then because packageConfigHaddockForHackage = Flag ForDevelopment while it expects it to be NoFlag. I need to find where and why during the convert, parse, show and convertTo process this is altered. If I observe everything correctly.

Working on it.

*Distribution.Client.BuildReports.Anonymous Distribution.Client.ProjectConfig
λ> import Distribution.Simple.Setup
*Distribution.Client.BuildReports.Anonymous Distribution.Client.ProjectConfig Distribution.Simple.Setup
λ> mempty { projectConfigLocalPackages = mempty { packageConfigHaddockBenchmarks = Flag True } }
ProjectConfig {projectPackages = [], projectPackagesOptional = [], projectPackagesRepo = [], projectPackagesNamed = [], projectConfigBuildOnly = ProjectConfigBuildOnly {projectConfigVerbosity = NoFlag, projectConfigDryRun = NoFlag, projectConfigOnlyDeps = NoFlag, projectConfigSummaryFile = [], projectConfigLogFile = NoFlag, projectConfigBuildReports = NoFlag, projectConfigReportPlanningFailure = NoFlag, projectConfigSymlinkBinDir = NoFlag, projectConfigOneShot = NoFlag, projectConfigNumJobs = NoFlag, projectConfigKeepGoing = NoFlag, projectConfigOfflineMode = NoFlag, projectConfigKeepTempFiles = NoFlag, projectConfigHttpTransport = NoFlag, projectConfigIgnoreExpiry = NoFlag, projectConfigCacheDir = NoFlag, projectConfigLogsDir = NoFlag}, projectConfigShared = ProjectConfigShared {projectConfigDistDir = NoFlag, projectConfigProjectFile = NoFlag, projectConfigHcFlavor = NoFlag, projectConfigHcPath = NoFlag, projectConfigHcPkg = NoFlag, projectConfigHaddockIndex = NoFlag, projectConfigRemoteRepos = [], projectConfigLocalRepos = [], projectConfigIndexState = NoFlag, projectConfigConstraints = [], projectConfigPreferences = [], projectConfigCabalVersion = NoFlag, projectConfigSolver = NoFlag, projectConfigAllowOlder = Nothing, projectConfigAllowNewer = Nothing, projectConfigMaxBackjumps = NoFlag, projectConfigReorderGoals = NoFlag, projectConfigCountConflicts = NoFlag, projectConfigStrongFlags = NoFlag, projectConfigAllowBootLibInstalls = NoFlag, projectConfigPerComponent = NoFlag, projectConfigIndependentGoals = NoFlag}, projectConfigProvenance = fromList [], projectConfigLocalPackages = PackageConfig {packageConfigProgramPaths = MapLast {getMapLast = fromList []}, packageConfigProgramArgs = MapMappend {getMapMappend = fromList []}, packageConfigProgramPathExtra = [], packageConfigFlagAssignment = [], packageConfigVanillaLib = NoFlag, packageConfigSharedLib = NoFlag, packageConfigDynExe = NoFlag, packageConfigProf = NoFlag, packageConfigProfLib = NoFlag, packageConfigProfExe = NoFlag, packageConfigProfDetail = NoFlag, packageConfigProfLibDetail = NoFlag, packageConfigConfigureArgs = [], packageConfigOptimization = NoFlag, packageConfigProgPrefix = NoFlag, packageConfigProgSuffix = NoFlag, packageConfigExtraLibDirs = [], packageConfigExtraFrameworkDirs = [], packageConfigExtraIncludeDirs = [], packageConfigGHCiLib = NoFlag, packageConfigSplitObjs = NoFlag, packageConfigStripExes = NoFlag, packageConfigStripLibs = NoFlag, packageConfigTests = NoFlag, packageConfigBenchmarks = NoFlag, packageConfigCoverage = NoFlag, packageConfigRelocatable = NoFlag, packageConfigDebugInfo = NoFlag, packageConfigRunTests = NoFlag, packageConfigDocumentation = NoFlag, packageConfigHaddockHoogle = NoFlag, packageConfigHaddockHtml = NoFlag, packageConfigHaddockHtmlLocation = NoFlag, packageConfigHaddockForeignLibs = NoFlag, packageConfigHaddockExecutables = NoFlag, packageConfigHaddockTestSuites = NoFlag, packageConfigHaddockBenchmarks = Flag True, packageConfigHaddockInternal = NoFlag, packageConfigHaddockCss = NoFlag, packageConfigHaddockHscolour = NoFlag, packageConfigHaddockHscolourCss = NoFlag, packageConfigHaddockContents = NoFlag, packageConfigHaddockForHackage = NoFlag}, projectConfigSpecificPackage = MapMappend {getMapMappend = fromList []}}
*Distribution.Client.BuildReports.Anonymous Distribution.Client.ProjectConfig Distribution.Simple.Setup

ezyang · 2017-05-08T13:47:58Z

The bug is probably in defaultHaddockFlags, where haddockForHackage defaults to Flag ForDevelopment. You probably need to change this to NoFlag, and then make sure no one is calling fromFlag on haddockForHackage; they need to call fromFlagOrDefault.

This may also explain why this flag was not supported in the first place :)

noisegul · 2017-05-09T00:49:19Z

Ok so I updated this as well, changed the defaultHaddockFlag for haddockForHackage to NoFlag. I then looked for places where fromFlag is called on haddockForHackage, it seems to be only one, where fromFlagOrDefault is already used:

.//Cabal/Distribution/Simple/Haddock.hs: fromFlagOrDefault ForDevelopment (haddockForHackage flags')

This did also not fix the error. However what did fix the error was changing the arbitrary instance of
HaddockTarget from arbitrary = elements [ForDevelopment, ForHackage] to arbitrary = elements [ForHackage].

Regardless of how defaultHaddockFlag is set, the unit test will fail if ForDevelopment is part of the generator combinator, also if it is only set to elements [ForDevelopment].

@dcoutts

This commit fixes one instances of a general bug where noFlag command line flags do not translate correctly when converted into the field description format (and in fact, cannot be converted correctly.) The bug goes something like this: 1. A noArg command line parser translates into a "choice" command line parser, for which there is only one choice (the flag that would be set when you toggle it on.) 2. Choice command line parsers get translated into the field format by having a rendering for each defined choice, and then default to empty (omit the field entirely) if no choices apply. 3. This means that NoFlag and the "default choice" (if you hadn't specified the flag at all) render identically, because there is never a choice for the default choice in the no arg field. In the interest of getting this to work, I manually made for-hackage work correctly, but there is probably a way to refactor this into a more universal fix. CC @dcoutts Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>

ezyang · 2017-05-09T02:07:31Z

Yes, it looks like I was wrong about what the problem was. More below.

This did also not fix the error. However what did fix the error was changing the arbitrary instance of
HaddockTarget from arbitrary = elements [ForDevelopment, ForHackage] to arbitrary = elements [ForHackage].

Yes, but if you do that, then we won't be testing that ForDevelopment round-trips properly ;)

I built your branch and ran the REPL command on the three different variants of HaddockTarget:

*Main Distribution.Client.ProjectConfig Distribution.Client.ProjectConfig.Legacy> showLegacyProjectConfig . convertToLegacyProjectConfig $ mempty { projectConfigLocalPackages = mempty { packageConfigHaddockForHackage = Flag ForDevelopment } }
""
*Main Distribution.Client.ProjectConfig Distribution.Client.ProjectConfig.Legacy> showLegacyProjectConfig . convertToLegacyProjectConfig $ mempty { projectConfigLocalPackages = mempty { packageConfigHaddockForHackage = Flag ForHackage } }
"haddock-for-hackage: for-hackage\n"
*Main Distribution.Client.ProjectConfig Distribution.Client.ProjectConfig.Legacy> showLegacyProjectConfig . convertToLegacyProjectConfig $ mempty { projectConfigLocalPackages = mempty { packageConfigHaddockForHackage = NoFlag } }
""

In fact, this is enough to determine what this bug is. Recall that this is a round-trip test: we're seeing if we can get exactly the same structure back after serializing it. When we see that Flag ForDevelopment and NoFlag serialize to the same value, we know that we can't possible get the same structure back. When you initially reported that you got packageConfigHaddockForHackage = Flag ForDevelopment while it expects it to be NoFlag, that was the NoFlag case failing to deserialize correctly (because mempty was initializing it to Flag ForDevelopment. When you modified the code I suggested, now the Flag ForDevelopment case stopped working, because you are parsing an empty string: whatever you get, it's going to be the default.

So, the correct fix is to make it so Flag ForDevelopment renders as something like "haddock-for-hackage: for-development\n". The logic for rendering these flags cabal-install/Distribution/Client/ProjectConfig/Legacy.hs in legacyPackageConfigFieldDescrs; remember when you added "for-hackage" to the list here, that was opting into the default rendering based on the command line argument... which apparently is not the rendering you want.

Unfortunately, the way this jiggery-pokery works is completely undocumented, so after having done this investigation I went ahead and wrote the fix. I'll push it onto your branch shortly.

ezyang · 2017-05-09T02:13:51Z

I pushed a387779 to your branch. Take a look at it and see if you agree with the reasoning in the commit message.

noisegul · 2017-05-09T12:53:08Z

Ah I see now! That part confused me as the defaultFlags were the first thing I looked at after following the way the printparse check works, and I got stuck there and fixated on it.

a387779 makes sense in that regard, I agree :)

Yes, but if you do that, then we won't be testing that ForDevelopment round-trips properly ;)

Yeah that I was aware of, I mostly did it to get closer to the actual problem, not a very elegant approach by me though.

hvr · 2017-05-15T08:49:29Z

@ezyang @bitrauser what's the status here? what's the next steps? who needs to do something?

noisegul · 2017-05-15T09:42:23Z

@hvr With the latest update on this PR, the haddock-for-hackage command appears to work as intended and the checks passed in the relation to #4412 , not sure why there was a build failure after merging with upstream afterwards, did not look related to this PR.

So I was waiting for this to be approved :)

hvr · 2017-05-15T11:01:33Z

@bitrauser Well, the TEST_OTHER_VERSIONS=YES build-config is broken; IOW, if all works except that single build-config, then I'd consider CI passed as far as this PR is concerned.

hvr · 2017-05-15T11:08:20Z

cabal-install/Distribution/Client/ProjectConfig/Types.hs

       packageConfigHaddockHscolourCss  :: Flag FilePath, --TODO: [required eventually] use this
-       packageConfigHaddockContents     :: Flag PathTemplate --TODO: [required eventually] use this
+       packageConfigHaddockContents     :: Flag PathTemplate, --TODO: [required eventually] use this
+       packageConfigHaddockForHackage   :: Flag HaddockTarget


Fwiw, this looks somewhat inconsistent; we got a field named ForHackage which would imply a Bool, but then this holds an enum which redundantly says ForHackage (or ForDevelopment).

Why isn't this simply a Flag Bool?

PS: To answer my own question, now I see that this has been inherited from Distribution.Simple.Setup (haddockForHackage :: Flag HaddockTarget); so this PR just went along with the pre-existing design decision...

...and actually in #2852 this was still a Flag Bool... it was 0975372 which lateron introduced the HaddockTarget data type w/o addressing the introduced naming inconsistency...

ezyang · 2017-05-15T13:54:25Z

Yes, CI is independently broken, see #4516

@bitrauser feel free to merge, I'll try to debug and fix the CI failure soon.

noisegul added 2 commits May 3, 2017 18:04

Add support for new-haddock --haddock-for-hackage

e14a9c4

Remove old commented placeholder

4f19f7d

Move instance Binary HaddockTarget to module of class

38d057b

noisegul added 3 commits May 5, 2017 12:53

Add "for-hackage" flag to legacyPackageConfigFieldDescrs

cde8779

Move ForHackage to the end in PackageConfig

7366534

Add for-hackage to unit-tests

ff0e66a

Merge remote-tracking branch 'upstream/master'

4bf497a

ezyang mentioned this pull request May 7, 2017

You're awesome! We're giving you commit bits. #3567

Closed

noisegul added 3 commits May 9, 2017 01:15

Change arbitrary instance of HaddockTarget

fa0d26e

Change default Flag of haddockForHackage to NoFlag

570c65b

Merge remote-tracking branch 'upstream/master'

df53e50

Merge remote-tracking branch 'upstream/master'

46fa1a2

Merge remote-tracking branch 'upstream/master'

c0a03ae

hvr reviewed May 15, 2017

View reviewed changes

hvr approved these changes May 15, 2017

View reviewed changes

noisegul merged commit b3da88b into haskell:master May 15, 2017

Add new-haddock support for --haddock-for-hackage #4490

Add new-haddock support for --haddock-for-hackage #4490

Uh oh!

Conversation

noisegul commented May 3, 2017

Uh oh!

mention-bot commented May 3, 2017

Uh oh!

noisegul commented May 3, 2017

Uh oh!

ezyang commented May 5, 2017

Uh oh!

noisegul commented May 5, 2017

Uh oh!

ezyang commented May 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

noisegul commented May 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ezyang commented May 8, 2017

Uh oh!

noisegul commented May 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ezyang commented May 9, 2017

Uh oh!

ezyang commented May 9, 2017

Uh oh!

noisegul commented May 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hvr commented May 15, 2017

Uh oh!

noisegul commented May 15, 2017

Uh oh!

hvr commented May 15, 2017

Uh oh!

hvr May 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ezyang commented May 15, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Add new-haddock support for `--haddock-for-hackage` #4490

Add new-haddock support for `--haddock-for-hackage` #4490

ezyang commented May 5, 2017 •

edited

Loading

noisegul commented May 8, 2017 •

edited

Loading

noisegul commented May 9, 2017 •

edited

Loading

noisegul commented May 9, 2017 •

edited

Loading

hvr May 15, 2017 •

edited

Loading