Implement `show-build-info` for cabal-install #7478

fendor · 2021-07-08T19:34:52Z

Rebase, squash and further development of #6241.

Basically brings the PR back into shape, building on top of the work of @bubba and others.

This is a new PR since the old one has quite a lot of outdated review comments and I thought it might be easier to review here.

However, there are quite a lot of insights in the previous PR, so it is still a must-read for understanding details and implementation details.

So, what works? Basically, the command works for build-type: Simple and on top of that, hie-bios has a POC powered by cabal's show-build-info support (HLS POC is also on the way).

(I haven't tested build-type: Custom yet, so no promises at the moment)

~~What doesn't work? The caching is pretty broken, e.g. whenever you invoke show-build-info, the state cache changes and build will rebuild local packages, but I think we can fix that.~~
Additionally, I want to add a caching mechanism for show-build-info, and implemented here a POC where show-build-info for each component is written to dist-newstyle/cache/buildinfo and it is re-generated whenever a local package needs to be rebuilt.
The intention is that show-build-info is lightning fast on the happy path. That commit can be reverted though if that behaviour is undesired.
Writes json of show-build-info to a file in dist-newstyle/cache/buildinfo but it is not really necessary.

TODOS:

Merge Json refactoring Json bytestring #7477
Merge Haddock output silencer Don't output haddock stdout if verbosity is silent #7483
Discuss fate of --pick-first-target Add AmbiguityResolver to decide how to resolve ambiguity #7482
Patches conform to the coding conventions.
Any changes that could be relevant to users have been recorded in the changelog.
The documentation has been updated, if necessary.
Enough tests?

jneira · 2021-07-08T21:15:27Z

The third time is the right time!

fendor · 2021-07-09T10:17:54Z

Cabal/src/Distribution/Simple/ShowBuildInfo.hs

+  , "unit-id"       .= JsonString (T.pack $ prettyShow $ componentUnitId clbi)
+  , "compiler-args" .= JsonArray (map JsonString $ getCompilerArgs bi lbi clbi)
+  , "modules"       .= JsonArray (map (JsonString . T.pack . display) modules)
+  , "src-files"     .= JsonArray (map (JsonString . T.pack) sourceFiles)


this is not ideal as sourceFiles are relative to hs-src-dirs. Getting the final Filepath for the source file requires more post-processing in tooling.

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

This allows users to get a JSON representation of various information about how Cabal would go about building a package. The output of this command is intended for external tools and therefore the format should remain stable.

Add (currently nonfunctional) new-show-build-info Fix compile error Make new-show-build-info functional Use silent verbosity by default on showBuildInfo commands to keep output json clean Make show-build-info commands hidden Implement write-autogen-files Make new-write-autogen-files work Make new-write-autogen-files configure if necessary Use target selectors for new-show-build-info Don't prune plan for new-show-build-info Only configure in new-show-build-info and new-write-autogen-files if no persist build info file is found Wrap multiple target output of new-show-build-info in json list

Mikolaj

Low confidence accept from me, since I didn't read the issues and old PRs (much).

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

Also, rename new-show-build-info to show-build-info in ShowBuildInfo Get cabal-install building again Fix typo Don't hardcode cabal version in showbuildinfo tests Remove some unnecessary files from test package Refactor show-build-info tests Undo some changes no longer needed in Main.hs Add back explicit exports and fix typos Tidy up imports Update showBuildInfoAction documentation Cosmetic fixes

Mikolaj · 2021-07-12T12:24:39Z

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

    -- Also shut up haddock since it dumps warnings to stdout
-    flags' = flags { haddockFlags = haddockFlags { haddockVerbosity = Flag silent }
-                   , configFlags = configFlags { Cabal.configTests = Flag True
-                                               , Cabal.configBenchmarks = Flag True
-                                               }
-                   }
-    cliConfig = commandLineFlagsToProjectConfig globalFlags flags'
+    -- flags' = flags { haddockFlags = haddockFlags { haddockVerbosity = Flag silent }
+    --                , configFlags = configFlags { Cabal.configTests = Flag True
+    --                                            , Cabal.configBenchmarks = Flag True
+    --                                            }
+    --                }
+    cliConfig = commandLineFlagsToProjectConfig globalFlags flags


Why is this not a problem any more?

Verbosity is still set to silent by default for the aforementioned reason (E.g. pollutes output and makes piping the results harder). However, since we do have a file output flag, I am not against changing the default verbosity to normal.

This used to enable tests and benchmarks by default, which I dislike, as we might ignore user configuration for basically no reason. There might be even a good reason why these have been disabled. So, I prefer explicit over implicit, and I think this is the more reasonable behaviour. People can and should explicitly enable these if they want information about them. (Although I just found out that tests and benchmarks are automagically enabled iff all their dependencies are already built. Still, my point stands :D)

Haddock verbosity might be still an issue, have to test that code-path.

I've noticed some extra lines in test output with HADDOCK in them. Not sure if related.

Probably, but I personally don't believe HADDOCK is really an issue.

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

fendor · 2021-07-12T12:18:29Z

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

@@ -1313,10 +1316,24 @@ buildInplaceUnpackedPackage verbosity
              Tar.createTarGzFile dest docDir name
              notice verbosity $ "Documentation tarball created: " ++ dest

+        -- Build info phase
+        {- buildInfo <- -}


fendor · 2021-07-12T12:18:47Z

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

+        -- Build info phase
+        {- buildInfo <- -}
+        whenBuildInfo $ do
+          -- Write the json to a temporary file to read it, since stdout can get


Update comment

fendor · 2021-07-12T12:19:49Z

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

+          -- Write the json to a temporary file to read it, since stdout can get
+          -- cluttered
+          let dir = distProjectCacheDirectory </> "buildinfo"
+          let fp = dir </> (unUnitId $ elabUnitId pkg) <.> "json"


Filenames look like this: hie-bios-0.8.0-inplace-hie-bios.json
Not really stable and not nice to look at. Besides that these files are not strictly necessary, would there be a way to improve them?

Problem? I thought that is a feature? But probably no.
However, I am thinking about removing it and maybe we can talk about this in a follow-up pr.

fendor · 2021-07-12T12:20:09Z

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

+       buildResultDocs      :: DocsResult,
+       buildResultTests     :: TestsResult,
+       buildResultLogFile   :: Maybe FilePath,
+       buildResultBuildInfo :: Maybe Text


Unused, also what do these BuildResults really do?

Good question. I'd like this answered too

It seems like they were intended to be cached somehow, but it kind of looks like they are always created on the fly.
Probably going to re-activate the usage in favour of the cache files in dist-newstyle/cache/buildinfo.

cabal-testsuite/PackageTests/ShowBuildInfo/Complex/src/Lib.hs

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

jneira

As @Mikolaj approving by pure trust and a quick look 😄
It would be nice if reviewers of the superseded pr (@phadej, @DanielG) could take a last look to this one

This means that cabal-install now extracts the LocalBuildInfo etc. itself for each component, and now assembles the JSON without the need for writing to temporary files. It also means that one build info JSON object can be returned instead of an array. It works by configuring each component separately as before, and instead of making its own build info object, it just collects the component information. This one build info object now reports the compiler used with the ElaboratedSharedConfig, which is shared across all components. Fix haddock parsing in TargetProblem Build dependencies in show-build-info Update .prod/.zinz templates Silence Haddock output

This fixes a lot of edge cases for example where the package db wasn't created at the time of configuring. Manually doing the setup.hs wrapper stuff was hairy. Turns out we do need to keep the show-build-info part inside Cabal as we rely on LocalBuildInfo which can change between versions, and we would need to do this anyway if we wanted to utilise the ProjectPlanning/Building infrastructure.

Changes the internal representation of JSON to Text rather than String, and introduces the buildinfo-components-only flag in the Cabal part to make it easier to stitch back the JSON into an array in cabal-install.

Simplifies writing tests for show-build-info Add trailing path separator to make testing simpler.

emilypi · 2021-07-14T20:22:58Z

Cabal/Cabal.cabal

@@ -257,6 +257,7 @@ library
    Distribution.Utils.NubList
    Distribution.Utils.ShortText
    Distribution.Utils.Progress
+    Distribution.Utils.Json


Thanks 👍

fendor · 2021-08-15T07:39:41Z

Closed in favour of #7498

fendor force-pushed the cabal-sbi-expirement branch from bebb70e to c44cb87 Compare July 8, 2021 19:35

fendor mentioned this pull request Jul 9, 2021

WIP for cabal show-build-info haskell/hie-bios#301

Closed

fendor commented Jul 9, 2021

View reviewed changes

fendor mentioned this pull request Jul 9, 2021

Draft: Add POC for HLS loading multiple components on startup haskell/haskell-language-server#2009

Closed

fendor force-pushed the cabal-sbi-expirement branch 5 times, most recently from f544427 to d2f0035 Compare July 12, 2021 08:10

fendor mentioned this pull request Jul 12, 2021

Add AmbiguityResolver to decide how to resolve ambiguity #7482

Draft

3 tasks

fendor force-pushed the cabal-sbi-expirement branch from d2f0035 to 3693a56 Compare July 12, 2021 08:37

bgamari and others added 2 commits July 12, 2021 10:37

Add show-build-info command

0bdf9e5

This allows users to get a JSON representation of various information about how Cabal would go about building a package. The output of this command is intended for external tools and therefore the format should remain stable.

fendor force-pushed the cabal-sbi-expirement branch 3 times, most recently from 8e3d211 to d0471c2 Compare July 12, 2021 09:27

fendor requested review from fgaz, Mikolaj, emilypi and jneira July 12, 2021 09:40

Mikolaj approved these changes Jul 12, 2021

View reviewed changes

cabal-install/src/Distribution/Client/Setup.hs Outdated Show resolved Hide resolved

fendor force-pushed the cabal-sbi-expirement branch from d0471c2 to cf4ec5d Compare July 12, 2021 10:48

fendor force-pushed the cabal-sbi-expirement branch from cf4ec5d to 4bc4f15 Compare July 12, 2021 10:54

fendor changed the title ~~WIP: Implement show-build-info for cabal-install~~ Implement show-build-info for cabal-install Jul 12, 2021

Mikolaj reviewed Jul 12, 2021

View reviewed changes

fendor commented Jul 12, 2021

View reviewed changes

jneira approved these changes Jul 13, 2021

View reviewed changes

fendor mentioned this pull request Jul 13, 2021

Don't output haddock stdout if verbosity is silent #7483

Draft

3 tasks

fendor force-pushed the cabal-sbi-expirement branch 2 times, most recently from f52d06c to aee85f5 Compare July 14, 2021 07:11

fendor added 2 commits July 14, 2021 09:30

Generate autogen files

a524314

fendor force-pushed the cabal-sbi-expirement branch 2 times, most recently from 07ed561 to b3cfd45 Compare July 14, 2021 07:38

fendor added 5 commits July 14, 2021 11:19

Port JSON module to Text based implementation

9794067

Changes the internal representation of JSON to Text rather than String, and introduces the buildinfo-components-only flag in the Cabal part to make it easier to stitch back the JSON into an array in cabal-install.

Extend s-b-i information in Cabal

b79e4a1

Cache show-build-info results in cache directory

8bd4431

Add verbose output marker for show-build-info

8ce27f2

Simplifies writing tests for show-build-info Add trailing path separator to make testing simpler.

fendor force-pushed the cabal-sbi-expirement branch 4 times, most recently from 6bef1c7 to 3240ecc Compare July 14, 2021 14:15

emilypi reviewed Jul 14, 2021

View reviewed changes

Re-design test-cases for show-build-info

2b51b5e

fendor force-pushed the cabal-sbi-expirement branch from 3240ecc to 2b51b5e Compare July 15, 2021 08:07

This was referenced Jul 20, 2021

Improve performance of discovering the ghc version used by the project haskell/haskell-language-server#2010

Open

Major steps required (Roadmap) haskell/haskell-language-server#1

Closed

This was referenced Jul 21, 2021

Implementation and purpose of Show-Build-Info #7489

Open

Remove show-build-info command and generate build-info as a side-effect of 'build' #7498

Merged

fendor closed this Aug 15, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement `show-build-info` for cabal-install #7478

Implement `show-build-info` for cabal-install #7478

fendor commented Jul 8, 2021 •

edited

Loading

jneira commented Jul 8, 2021

fendor Jul 9, 2021

Mikolaj left a comment

Mikolaj Jul 12, 2021

fendor Jul 12, 2021

fendor Jul 12, 2021

Mikolaj Jul 12, 2021

fendor Jul 12, 2021 •

edited

Loading

fendor Jul 12, 2021

fendor Jul 12, 2021

fendor Jul 12, 2021

fendor Jul 14, 2021

fendor Jul 12, 2021

emilypi Jul 14, 2021

fendor Jul 14, 2021

jneira left a comment

emilypi Jul 14, 2021

fendor commented Aug 15, 2021

Implement show-build-info for cabal-install #7478

Implement show-build-info for cabal-install #7478

Conversation

fendor commented Jul 8, 2021 • edited Loading

jneira commented Jul 8, 2021

Choose a reason for hiding this comment

Mikolaj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fendor Jul 12, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jneira left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fendor commented Aug 15, 2021

Implement `show-build-info` for cabal-install #7478

Implement `show-build-info` for cabal-install #7478

fendor commented Jul 8, 2021 •

edited

Loading

fendor Jul 12, 2021 •

edited

Loading