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

fendor · 2021-07-27T11:13:28Z

This is the first step for implementing Approach 3 of #7489.
Mainly extracted from #7478

In particular, add a flag --enable-build-info to configure of Setup.hs and generate build-info.json right next to setup-config.

Readable by tooling.

Cabal Tests using build-type: Simple
Cabal Tests using build-type: Custom
Optional: Add cabal configure --enable-dump-buildinfo (e.g. enable/disable the generation)
Add buildInfoJson to plan.json output
Guard against older Cabal versions

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

fendor · 2021-07-30T13:18:46Z

Cabal/src/Distribution/Simple/BuildPaths.hs


+-- | Build info json file, generated in every build
+buildInfoPref :: FilePath -> FilePath
+buildInfoPref distPref = distPref </> "build-info.json"


Possible inconsistency, file name is build-info but other flags are buildinfo. Any preferences?

As far as I'm concerned buildinfo is a word.

While I also find "buildinfo" more natural I found some precedent on the cmdline: --enable-debug-info is a thing so maybe we should be consistent with that?

I choose consistency with debug-info, everything will be renamed to build-info

hasufell · 2021-07-30T15:08:24Z

Cabal/src/Distribution/Simple/Setup.hs

                "Don't emit debug info"
         ]

+      , multiOption "dump-buildinfo"


Why conditionally? Is there a reason not to write it always?

I did not want to change the default behaviour, but it would be indeed better if it was always generated. Subject to bikeshed.

performance is not a problem I suppose

I woudn't be so sure that performance couldn't be a problem, best to err on the side of caution when people with pitchforks are sure to follow if build speed regresses ;)

If it turns out to be unproblematic it's an easy default to change: just ignore the flag going forward, but the other way around isn't so easy since tooling would break if we introduce a flag.

Lastly my understanding is that hie-bios currently always does at least one (implicit) build call anyway though the repl command so we're just maintaining the status quo if it has to do one with a special flag to get the buildinfo.

Lastly my understanding is that hie-bios currently always does at least one (implicit) build call anyway though the repl command so we're just maintaining the status quo if it has to do one with a special flag to get the buildinfo.

yeah but it would be nice if we get to make no one if possible, though

DanielG

This is looking pretty good. I'm super excited that we figured out how to roll this into build rather than introducing a new command.

Apart from nits and bikeshedding the only real points that need addressing are the non-nullability of the compiler "path", the nullability of "build-info" in plan.json or an alternate mechanism for downstreams to detect if the "lib:Cabal too old" case is happening and as always moar docs ;)

Cabal/src/Distribution/Simple/Build.hs

Cabal/src/Distribution/Simple/Setup.hs

DanielG · 2021-07-30T21:32:03Z

Cabal/src/Distribution/Simple/BuildPaths.hs


+-- | Build info json file, generated in every build
+buildInfoPref :: FilePath -> FilePath
+buildInfoPref distPref = distPref </> "build-info.json"


While I also find "buildinfo" more natural I found some precedent on the cmdline: --enable-debug-info is a thing so maybe we should be consistent with that?

DanielG · 2021-07-30T21:35:47Z

Cabal/src/Distribution/Simple/ShowBuildInfo.hs

 -- information about a Cabal package and the compiler configuration Cabal
 -- would use to build it. This can be produced with the
-- @cabal new-show-build-info@ command.
+-- @cabal build --enable-dump-buildinfo@ command.


Seeing that one would pass the (configure) enable flag to build like this makes me wonder if --dump-buildinfo should imply --enable-buildinfo on the build cmdline? cabal build --enable-buildinfo --dump-buildinfo=PATH seems quite redundant.

no flag --dump-buildinfo exists yet, I would leave that to a follow-up PR, unless someone objects

Cabal/src/Distribution/Simple/ShowBuildInfo.hs

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

DanielG · 2021-07-30T21:59:40Z

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

+    flags_3_5_0 = flags_latest {
+      -- Cabal < 3.5 does not understand --dump-build-info
+        configDumpBuildInfo = NoFlag
+      }


I think we ought to emit a warning about this somewhere.

Sure but where?

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

DanielG

Just another per-commit review pass. Something seems off about the commit messages not being in sync with the diffs.

Cabal/src/Distribution/Simple/BuildTarget.hs

Cabal/Cabal.cabal

Cabal/src/Distribution/Simple/Utils/Json.hs

doc/setup-commands.rst

DanielG · 2021-09-04T12:08:34Z

doc/cabal-project.rst

+    on how to build an individual component, such as compiler version, modules of a component and
+    how to compile the component.
+
+    The output format is in json, and the exact location can be discovered from ``plan.json``.


This still doesn't seem to mention what the field name in plan.json is or where that field can be found like I suggested above.

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

Cabal/src/Distribution/Simple/Setup.hs

DanielG

We should add some comments to make sure code, json schema and docs stay in sync in the face of other people changing this code in the future.

doc/setup-commands.rst

doc/json-schemas/build-info.schema.json

Cabal/src/Distribution/Simple/ShowBuildInfo.hs

Adds missing file information for test and benchmark stanzas.

Removes 'show-build-info' command from 'lib:Cabal' and replaces it by generating build-info whenever a build happens. Add flag '--dump-buildinfo' to signal the build step to dump build information for the package/component we are currently building.

…to build

Refactor the API slightly s.t. a ConfiguredProgram for the Compiler is passed to build-info generation directly.

jneira · 2021-09-08T10:52:34Z

are you scared of pressing the button? 😝

fendor · 2021-09-08T14:48:35Z

Well, someone else should press the merge button :)

DanielG · 2021-09-08T14:51:56Z

Alright then, been eyeing that button all day :D

andreasabel · 2021-09-11T09:43:26Z

@fendor @DanielG Commit 024c060 of this PR broke make users-guide:

doc/developing-packages.rst:259: WARNING: Unknown target name: "common stanzas".

doc/developing-packages.rst:351: WARNING: Unknown target name: "package versioning policy".

doc/setup-commands.rst:1057: WARNING: Unexpected indentation.
doc/setup-commands.rst:1060: WARNING: Block quote ends without a blank line; unexpected unindent.
doc/setup-commands.rst:1062: WARNING: Unexpected indentation.
doc/setup-commands.rst:1067: WARNING: Unexpected indentation.
doc/setup-commands.rst:1068: WARNING: Block quote ends without a blank line; unexpected unindent.
doc/setup-commands.rst:1070: WARNING: Unexpected indentation.
doc/setup-commands.rst:1071: WARNING: Block quote ends without a blank line; unexpected unindent.
doc/setup-commands.rst:1073: WARNING: Unexpected indentation.
doc/setup-commands.rst:1074: WARNING: Block quote ends without a blank line; unexpected unindent.
doc/setup-commands.rst:1076: WARNING: Unexpected indentation.
doc/setup-commands.rst:1077: WARNING: Block quote ends without a blank line; unexpected unindent.
doc/setup-commands.rst:1081: WARNING: Block quote ends without a blank line; unexpected unindent.
doc/setup-commands.rst:1082: WARNING: Definition list ends without a blank line; unexpected unindent.

doc/setup-commands.rst:1084: WARNING: Unknown directive type "jsonschema".

.. jsonschema:: ./json-schemas/build-info.schema.json

Mikolaj · 2021-09-11T10:27:40Z

Uhoh. We do need make users-guide in CI.

DanielG · 2021-09-11T13:35:18Z

Indeed fendor seems to have forgotten to add the jsonschema extension to conf.py, but the users-guide target was already broken before the merge AFAICT :]

According to my bisect 0f041b6 is to blame originally.

Mikolaj · 2021-09-11T13:54:22Z

See #7637.

fendor force-pushed the feature/sbi-build-sideeffect branch 4 times, most recently from 9567d55 to 7744fe4 Compare July 28, 2021 10:52

fendor requested review from Mikolaj, emilypi, fgaz and jneira and removed request for Mikolaj July 29, 2021 11:52

fendor force-pushed the feature/sbi-build-sideeffect branch 2 times, most recently from 535591f to 500d0bd Compare July 30, 2021 10:35

fendor commented Jul 30, 2021

View reviewed changes

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

fendor force-pushed the feature/sbi-build-sideeffect branch 3 times, most recently from 3c7272f to f9844d6 Compare July 30, 2021 13:15

fendor commented Jul 30, 2021

View reviewed changes

fendor force-pushed the feature/sbi-build-sideeffect branch 2 times, most recently from 4fc7f04 to bda7743 Compare July 30, 2021 14:02

hasufell reviewed Jul 30, 2021

View reviewed changes

DanielG reviewed Jul 30, 2021

View reviewed changes

Cabal/src/Distribution/Simple/BuildTarget.hs Show resolved Hide resolved

Cabal/Cabal.cabal Show resolved Hide resolved

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

fendor force-pushed the feature/sbi-build-sideeffect branch 8 times, most recently from 53de4fb to 38f189f Compare August 1, 2021 18:21

fendor force-pushed the feature/sbi-build-sideeffect branch from e03c434 to 7a9f973 Compare September 4, 2021 12:13

DanielG reviewed Sep 4, 2021

View reviewed changes

doc/setup-commands.rst Show resolved Hide resolved

doc/json-schemas/build-info.schema.json Show resolved Hide resolved

Cabal/src/Distribution/Simple/ShowBuildInfo.hs Show resolved Hide resolved

fendor added 5 commits September 7, 2021 17:09

Enhance show-build-info API

b1c02f1

Adds missing file information for test and benchmark stanzas.

Enhance show-build-info command API

fb53623

Add build-info location to plan.json

a2413e5

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

705228e

fendor force-pushed the feature/sbi-build-sideeffect branch 4 times, most recently from c26077d to 9b56d10 Compare September 7, 2021 16:21

fendor added 5 commits September 7, 2021 18:25

Add user-guide documentation for 'build-info'

6b954b0

Always write build-info, even before building

c444ad8

Add test-cases for extracting build-info, even if a component failed …

b68243c

…to build

Extract 'dumpBuildInfo' function

66669b1

Make compiler path not nullable in dumped build-info

0702193

Refactor the API slightly s.t. a ConfiguredProgram for the Compiler is passed to build-info generation directly.

fendor force-pushed the feature/sbi-build-sideeffect branch from 9b56d10 to 0702193 Compare September 7, 2021 16:25

DanielG merged commit c66a126 into haskell:master Sep 8, 2021

andreasabel mentioned this pull request Sep 11, 2021

Additional examples in doc of tested-with (fix #4894) #7604

Merged

andreasabel mentioned this pull request Sep 20, 2021

Testing readthedocs' pr integration (do not merge) #7647

Closed

jneira mentioned this pull request Oct 18, 2021

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

Closed

fendor mentioned this pull request May 3, 2022

Renaming multi component support haskell/haskell-language-server#2193

Open

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

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

Uh oh!

Conversation

fendor commented Jul 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

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!

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!

DanielG left a comment

Choose a reason for hiding this comment

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!

Choose a reason for hiding this comment

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!

DanielG left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

DanielG left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jneira commented Sep 8, 2021

Uh oh!

fendor commented Sep 8, 2021

Uh oh!

DanielG commented Sep 8, 2021

Uh oh!

andreasabel commented Sep 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mikolaj commented Sep 11, 2021

Uh oh!

DanielG commented Sep 11, 2021

Uh oh!

Mikolaj commented Sep 11, 2021

Uh oh!

Reviewers

Assignees

fendor commented Jul 27, 2021 •

edited

Loading

andreasabel commented Sep 11, 2021 •

edited

Loading