show-build-info (exe:cabal part plus tests) by fendor · Pull Request #6241 · haskell/cabal

fendor · 2019-09-14T19:38:59Z

Please include the following checklist in your PR:

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.
If the change is docs-only, [ci skip] is used to avoid triggering the build bots.

Please also shortly describe how you tested your change. Bonus points for added tests!

Since #6108 merged parts of #5954, the original pr #5954 got obsolete.
This now is the second part that contains only changes to cabal-install and the test-suite.
This is a push to get s-b-i merged for exe:cabal, enabling further improvements to it.

cc @hvr, @DanielG, @mpickering

cabal-install/Distribution/Client/CmdShowBuildInfo.hs

fendor · 2019-09-15T15:51:30Z

cabal-install/main/Main.hs

      , hiddenCmd  actAsSetupCommand actAsSetupAction
      , hiddenCmd  manpageCommand (manpageAction commandSpecs)
-
+      , hiddenCmd CmdShowBuildInfo.showBuildInfoCommand


should it be a hidden command?

I don't see any reason to hide it. We're treating it like an API, right? What is our API guarantee for the returned JSON anyways? Should probably write that down somewhere.

No breaking changes within a Cabal major version? No breaking changes ever?

Someone else needs to define this, I dont know enough of the process and the ramifications of such promises.

fendor · 2019-09-15T19:57:50Z

CI fail seems to be unrelated to this pr.

DanielG

Thanks for pushing this through! Functionality loooks good apart from the fact that we still don't have any tests that actually use the build info we get to run GHC or anything.

While testing out the lib:Cabal side show-build-info I noticed that what we get back from that doesn't include the inplace package-db because that's added very late by the build command. See the createInternalPackageDB calls in Simple/Build.hs.

Some pretty basic tests that call GHC would have cought this, just saying ;) I found it because I have such tests in cabal-helper.

I think we should definetly fix that issue before merging but we can just fix it up on the cabal-install side for now, I'm not sure it even makes sense to fix that on the Cabal side anyways.

Other minor things:

The code is still a bit messy and needs quite some cleanup.
I'd like to see some deduplication of the test code.

cabal-testsuite/PackageTests/ShowBuildInfo/A/build-info-exe-exact.test.hs

cabal-testsuite/PackageTests/ShowBuildInfo/B/B.cabal

cabal-testsuite/PackageTests/ShowBuildInfo/B/Setup.hs

cabal-install/Distribution/Client/CmdShowBuildInfo.hs

cabal-install/main/Main.hs

DanielG · 2019-09-20T22:04:43Z

cabal-testsuite/PackageTests/ShowBuildInfo/A/build-info-unknown.out

@@ -0,0 +1,12 @@
+# cabal show-build-info
+cabal: Internal error in target matching. It should always be possible to find a syntax that's sufficiently qualified to give an unambiguous match. However when matching 'exe:B'  we found exe:B (unknown-component) which does not have an unambiguous syntax. The possible syntax and the targets they match are as follows:


Um, this sounds like a cabal bug to me. Certainly nothing that should be expected test output or ever seen by users! Please find out what's going on here.

That error message looks like #5081.

cabal-testsuite/PackageTests/ShowBuildInfo/A/cabal.project

lukel97 · 2020-06-01T15:31:32Z

I've been trying to integrate this with hie-bios and it's been going well so far. There are some things I would like to see addressed first though:

runghc Setup.hs show-build-info returns one JSON object, whereas cabal show-build-info returns an array of JSON objects, one for each component/unit it plans on building, namely due to how it is calling the wrapper several times. But this seems to me to defeat the purpose of the components array, as it is possible to get runghc Setup.hs show-build-info to return one object containing all the components needed
With runghc Setup.hs show-build-info, you need to configure it first with --enable-tests/--enable-benchmarks to see the build info for tests and benchmarks respectively. This makes sense for the Setup.hs method, but you also need to do the same for cabal show-build-info. For the sake of tooling integration I think cabal show-build-info should default to including tests and benchmarks without having to pass --enable-tests/--enable-benchmarks

Cabal/Distribution/Utils/Json.hs

lukel97 · 2020-06-03T15:11:45Z

@phadej @fendor I've changed this to avoid calling Setup.hs to get the show-build-info result (It still uses it for configuring the components). That way we can collect the component information separately and use the correct compiler information, and return just one build info object rather than several, whilst also avoiding the temporary files. So there's a bit of poking about done with LocalBuildInfo here which I'm aware seems to be taboo within cabal-install

lukel97 · 2020-06-03T15:16:27Z

test.hs

+build-depends: base
+with-compiler: ghc-8.6.5
+-}
+main = putStrLn "hey"


Ugh accidentally added this and hie.yaml during a rebase. Will fix

lukel97 · 2020-06-03T22:03:01Z

I think cabal show-build-info should implicitly build dependencies, much like cabal repl, otherwise it will fail on a clean project when trying to configure. It should behave equivalently to

cabal build --only-dependencies <targets>  && cabal show-build-info <targets>

Passing the targets to the build part will be important, as with cabal repl only the dependencies of the target need to be buildable before ghci can be launched with the right flags

phadej · 2020-06-03T23:13:44Z

--only-dependencies could fail easily: #6106

If project contains packages A and C, and there is third package B (on Hackage),
then if dependency graph is A -> B -> C, you essentially need to build also C to build B to configure A.

We don't have such more of operation currently. Would be good to have. Actually both:

build all dependencies you can without building local packages
build all dependencies, incl. some local packages, so all external dependencies are built. I.e. you can configure packages.

lukel97 · 2020-06-04T15:56:53Z

I ran into this whilst testing out integrating this with hie-bios, #6874 and I think there should be a mechanism to allow for ambiguous target selectors to be resolved, e.g. by just selecting the first one. Namely, hie-bios isn't going to care what component's flags cabal show-build-info returns, as long as it can actually get the flags in the first place

lukel97 · 2020-06-06T15:24:33Z

Todo

Make AmbiguousTargetResolver a command line flag so other commands can use it. --pick-first-target
Fix Haddock output interfering with stdout
Address autogen files

lukel97 · 2020-06-12T13:34:02Z

Tricky situation with dependencies: Given this simple cabal file

cabal-version: 2.4
name:          C
version:       0.1.0.0
license:       BSD-3-Clause

library
  exposed-modules:  Lib
  build-depends:    base
  default-language: Haskell2010

test-suite tests
  type: exitcode-stdio-1.0
  main-is: Test.hs
  build-depends: base, C
  default-language: Haskell2010

If we are to run cabal show-build-info verbatim without passing any targets to it at all, it will fail at configuring the test-suite because the library C will not have been built. cabal show-build-info Test.hs does work though, as it seems to count the library C as a dependency in this scenario

Edit: this has been solved with a new prunePlanToAllDependencies function, a special case of prunePlanToDependencies

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.

Every other command defaults to what they used to do. show-build-info now just chooses the first choice, since it doesn't care about ambiguity.

lukel97 · 2020-07-07T19:00:47Z

@DanielG Phew, reworked it to now use the nix stuff. Hopefully ci passes

These are needed by tooling to setup GHC sessions.

lukel97 · 2020-07-08T10:37:39Z

New and noteworthy things in this PR:

Added cabal-install show-build-info
--pick-first-target flag to resolve ambiguous target selectors to just the first target
show-build-info now returns the location of the cabal file as well as the unpacked source directory of the package (not the hs-src-directory, which is different)
show-build-info now uses Text rather than String internally
show-build-info has the buildinfo-components-only flag which spits out each component's json line by line. Used by cabal-install to piece together components that are split across multiple per-component packages

phadej · 2020-07-08T10:43:06Z

Cabal/Distribution/Utils/Json.hs

+          | JsonString !Text
+
+-- | A type to mirror 'ShowS'
+type ShowT = Text -> Text


I spotted this via your comment. Why this and not https://hackage.haskell.org/package/text-1.2.4.0/docs/Data-Text-Lazy-Builder.html

The ShowT = Text -> Text is worse than ShowS, as it copies all intermediate values, causing quadratic behavior.

Yet, if you doing this for performance reasons, you should use ByteString and its builder, like aeson.

Accidentally quadratic, how embarrassing! I'll switch this all over to ByteString, didn't realise Text was also locale dependent

phadej · 2020-07-08T10:43:37Z

cabal-install/Distribution/Client/CmdShowBuildInfo.hs

+
+  case fileOutput of
+    Nothing -> T.putStrLn res
+    Just fp -> T.writeFile fp res


This is locale dependent, better to serialise to ByteString and write it.

phadej · 2020-07-08T10:44:52Z

Separate JSON utility into own PR, it is independent and can be reviewed and committed independently.

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. It also 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. 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.

jneira · 2021-10-18T07:44:34Z

@fendor has been this superseeded so we could close?

masaeedu · 2021-11-05T00:38:07Z

@jneira @fendor Sorry, would you mind expanding on what this has been superseded with? There seems to now be a network of issues about show-build-info that are all pointing at each other saying X is superseded by Y and Y is superseded by Z and so on :)

Is there a "can i run cabal show-build-info yet" issue somewhere?

fendor · 2021-11-05T07:31:23Z

This issue summarises the current ideas, discussions and efforts: #7489

Specifically this is a PR overview: #7489 (comment)

fendor force-pushed the rebase-cabal-sbi branch from 2e70d3d to c946e9a Compare September 15, 2019 10:01

fendor commented Sep 15, 2019

View reviewed changes

fendor force-pushed the rebase-cabal-sbi branch from c946e9a to e4c648d Compare September 15, 2019 16:08

fendor requested a review from DanielG September 19, 2019 16:25

DanielG requested changes Sep 20, 2019

View reviewed changes

fendor mentioned this pull request Oct 1, 2019

Add new-show-build-info command #5954

Closed

4 tasks

robrix mentioned this pull request Nov 1, 2019

Multi component ghcide setup github/semantic#374

Merged

3 tasks

jneira mentioned this pull request Apr 8, 2020

Auto generating hie.yaml / better implicit configuration haskell/hie-bios#122

Closed

lukel97 force-pushed the rebase-cabal-sbi branch from e4c648d to d33e665 Compare May 1, 2020 14:41

lukel97 force-pushed the rebase-cabal-sbi branch from 49e8cc1 to f10457e Compare June 2, 2020 18:58

fendor commented Jun 3, 2020

View reviewed changes

Cabal/Distribution/Utils/Json.hs Outdated Show resolved Hide resolved

lukel97 force-pushed the rebase-cabal-sbi branch from a75aa0b to de2a4d6 Compare June 3, 2020 15:04

lukel97 reviewed Jun 3, 2020

View reviewed changes

lukel97 force-pushed the rebase-cabal-sbi branch from de2a4d6 to b1cdcea Compare June 3, 2020 15:19

lukel97 force-pushed the rebase-cabal-sbi branch 2 times, most recently from fb02daa to 0dff8a6 Compare June 8, 2020 16:14

lukel97 mentioned this pull request Jun 8, 2020

A mechanical way to report build progress #6886

Open

lukel97 force-pushed the rebase-cabal-sbi branch 3 times, most recently from 9a5d4bd to 11de41c Compare June 8, 2020 22:37

This was referenced Jun 11, 2020

cabal wrapper uses always global ghc in PATH haskell/hie-bios#194

Closed

Cabal cradle causes me to constantly need to run cabal configure haskell/hie-bios#195

Closed

lukel97 added 7 commits July 7, 2020 19:59

Modernize CmdShowBuildInfo

9708c4b

Fix haddock parsing in TargetProblem

554f679

Build dependencies in show-build-info

08a5e93

Update .prod/.zinz templates

4c1978f

Silence Haddock output

6180494

Add AmbiguityResolver to decide how to resolve ambiguty

6daaa52

Every other command defaults to what they used to do. show-build-info now just chooses the first choice, since it doesn't care about ambiguity.

lukel97 force-pushed the rebase-cabal-sbi branch from 36c3f8b to 6e70ee3 Compare July 7, 2020 19:00

lukel97 added 3 commits July 7, 2020 20:16

Add --pick-first-target flag

d52bdde

Don't output haddock stdout if verbosity is silent

23b8870

Generate autogen files

25aef99

These are needed by tooling to setup GHC sessions.

lukel97 force-pushed the rebase-cabal-sbi branch 2 times, most recently from 5084eb1 to 00ba53b Compare July 8, 2020 10:29

phadej reviewed Jul 8, 2020

View reviewed changes

lukel97 force-pushed the rebase-cabal-sbi branch from 00ba53b to 84aa560 Compare July 8, 2020 11:30

lukel97 mentioned this pull request Jul 8, 2020

Convert Json module from String to ByteString #6948

Closed

jneira mentioned this pull request Sep 23, 2020

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

Closed

9 tasks

jneira mentioned this pull request Dec 22, 2020

cabal new-path #4661

Open

fendor mentioned this pull request Jun 2, 2021

Proposal: better multi component support (including show-build-info) haskell/hie-bios#269

Open

fendor mentioned this pull request Jul 8, 2021

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

Closed

7 tasks

fendor mentioned this pull request Aug 6, 2021

Implementation and purpose of Show-Build-Info #7489

Open

fendor closed this Oct 18, 2021

		@@ -0,0 +1,12 @@
		# cabal show-build-info
		cabal: Internal error in target matching. It should always be possible to find a syntax that's sufficiently qualified to give an unambiguous match. However when matching 'exe:B' we found exe:B (unknown-component) which does not have an unambiguous syntax. The possible syntax and the targets they match are as follows:

Conversation

fendor commented Sep 14, 2019

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fendor commented Sep 15, 2019

Uh oh!

DanielG left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lukel97 commented Jun 1, 2020

Uh oh!

Uh oh!

lukel97 commented Jun 3, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lukel97 commented Jun 3, 2020

Uh oh!

phadej commented Jun 3, 2020

Uh oh!

lukel97 commented Jun 4, 2020

Uh oh!

lukel97 commented Jun 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Todo

Uh oh!

lukel97 commented Jun 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lukel97 commented Jul 7, 2020

Uh oh!

lukel97 commented Jul 8, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phadej commented Jul 8, 2020

Uh oh!

jneira commented Oct 18, 2021

Uh oh!

masaeedu commented Nov 5, 2021

Uh oh!

fendor commented Nov 5, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

lukel97 commented Jun 6, 2020 •

edited

Loading

lukel97 commented Jun 12, 2020 •

edited

Loading