Per-component new-build support (no Custom support yet) #3662

ezyang · 2016-08-02T09:47:12Z

A bit of a megapatch. Here's what's in it:

First, a few miscellaneous utility functions and reexports
in Cabal. I could have split these into a separate commit
but I was too lazy to.
Distribution.Client.Install got refactored:
instead of using PackageFixedDeps, it uses PackageInstalled
instead. That's because the install plan was NOT using
ComponentDeps in any nontrivial way. And it shouldn't.
So use a more specific type-class. I also removed the
invariant checking and error reporting because it was
being annoying (we check the invariants already in
SolverInstallPlan).
Look at Distribution.Client.ProjectPlanning.Types.
This contains the primary type change: ElaboratedConfiguredPackage
is now EITHER a monolithic ElaboratedPackage, or a per-component
ElaboratedComponent (it should get renamed but I didn't do that
in this patch.) These are what we're going to store in our
plans: if a package we're building has a Setup script which supports
per-component builds, we'll explode it into a component. Otherwise
we'll keep it as a package. We'll see codepaths for both
throughout.
OK, so the expansion happens in ProjectPlanning, mostly in
'elaborateAndExpandSolverPackage'. You should review the
package hash computation code closely. When we can separate
components, we compute a hash for each INDEPENDENTLY. This
is good: we get more sharing.
We need to adjust the target resolution and pruning code
in ProjectOrchestration and ProjectPlanning. I did a dumb
but easy idea: if a user mentions 'packagename' in a
target name, I spray the PackageTarget on every
possibly relevant IPID in buildTargets', and then pare
it down later.
And of course there's code in ProjectBuilding to actual
do a configure and then build.

Custom doesn't work yet because I need to give them their own
separate component, and teach Cabal how to build them specially.

phadej · 2016-08-02T09:54:08Z

Cabal/Distribution/Package.hs

@@ -176,6 +177,10 @@ mkUnitId = SimpleUnitId . ComponentId
 mkLegacyUnitId :: PackageId -> UnitId
 mkLegacyUnitId = SimpleUnitId . ComponentId . display

+-- | Extract ComponentId from UnitId.


--| Extract 'ComponentId' from 'UnitId'. formatting :P

ezyang · 2016-08-02T11:04:03Z

OK, looked at all comments @phadej.

ezyang · 2016-08-02T11:06:53Z

CC @dcoutts

dcoutts · 2016-08-02T13:59:30Z

Will have a look.

ezyang · 2016-08-04T00:50:21Z

I just realized this PR breaks internal build-tools. So I guess I will at least fix this enough so that feature works again.

ezyang · 2016-08-04T07:25:35Z

Although, it's hard to trigger, because most uses of build-tools induced ordering also require a Custom setup, at which point we turn off the feature.

ezyang · 2016-08-09T10:01:43Z

Note: this breaks running the package-tests test suite: we use the LBI to look in the inplace database for the library, but obviously there won't be any because each component gets its own inplace. I'll rewrite this code to be more robust.

hvr · 2016-08-09T13:04:22Z

Just to document one nice thing about this PR:

I've got a package which consists of one lib stanza, and 28 exe/bench/test stanzas (which depend on the lib stanza). With this PR, build-time on a quad-core i7-3770 is reduced from a total of 3m40s down to 2m40s. Building the lib-part takes up 2m05s, so this provides a 4-fold speed-up (from 1m35s down to 0m35s) for the executable build+linking phase which can exploit parallelism.

dcoutts · 2016-08-12T03:20:01Z

Cabal/Distribution/Simple/Setup.hs

@@ -347,6 +347,9 @@ relaxDepsPrinter (Just (RelaxDepsSome pkgs)) = map (Just . display) $ pkgs
 -- IMPORTANT: every time a new flag is added, 'D.C.Setup.filterConfigureFlags'
 -- should be updated.
 data ConfigFlags = ConfigFlags {
+    -- This is the same hack as in 'buildArgs' and 'copyArgs'.
+    configArgs :: [String],


Can we have a TODO like we do for buildArgs so they all get removed when we break the UserHooks API.

Sure why not.

dcoutts · 2016-08-12T04:55:51Z

Cabal/Distribution/PackageDescription/Parse.hs

+                        (fromMaybe (error "subLibFieldDescrs") . libName)
+                        (\xs lib -> lib{libName=Just xs})
+  ] ++ libFieldDescrs
+


I don't understand why this is in this patch. It's just for the pretty-printer apparently. Is this fixing something that was already broken previously? Is this change supposed to live in the patch about the pretty printer?

Oh yes this got killed in the pretty-printer patch. Let me see if I can swizzle the hunks around.

dcoutts · 2016-08-14T15:49:01Z

Cabal/Distribution/Simple/Configure.hs

                    installedPackageSet
                    internalPackageSet
                    requiredDepsMap)
                comp
                compPlatform
                pkg_descr0

+    debug verbosity $ "Finalized package description:\n"
+                  ++ showPackageDescription pkg_descr


Do we want to keep this or was this only during hacking? I ask since it's quite a lot of text to dump.

It's debug mode, and it really isn't that much text (compared to some of the other stuff we spew, even in verbose non-debug mode!) And I've only ever found this information useful.

hvr · 2016-08-21T08:51:04Z

I've used the latest PR code for the last two hours and couldn't find anything obviously broken. So let's merge this and iron out any fallout when we notice it. Keeping this branch unmerged for much longer is only going to risk merge conflicts which are tedious to fix for such a large stack of commits.

Signed-off-by: Edward Z. Yang <[email protected]>

Described in: ghc-proposals/ghc-proposals#4 ./Setup configure now takes an argument to specify a specific component name that should solely be configured. Most of the gyrations in Configure are all about making it so that we can feed in internal dependencies via --dependency. I dropped the package name match sanity check to handle convenience library package name munging. Consider an internal library named 'q' in package 'p'. When we install it to the package database, we munged the package name into 'z-p-z-q', so that it doesn't conflict with the actual package named 'q'. Now consider when we feed it in with --dependency q=p-0.1-hash-q. Previously, Cabal checked that the 'q' in --dependency matched the package name in the database... which it doesn't. So I dropped the check. I also had to make register/copy unconditionally install internal libraries; otherwise you can't refer to them from later builds. Also a miscellaneous refactor: convenience libraries are printed with a "header" stanza now (not really a stanza header). Signed-off-by: Edward Z. Yang <[email protected]>

Signed-off-by: Edward Z. Yang <[email protected]>

The previous approach I took, though correct, was quite confusing. If I refactor InstallPlan to operate on a per-component basis, then we'll automatically get support for convenience libraries, which will ultimately cleaner. (While we won't be able to get rid of support for whole package installs, it will be safe to assume packages using convenience libraries also support one-shot configure.) I didn't revert the support in cabal install; I'm not planning on componentizing it. Signed-off-by: Edward Z. Yang <[email protected]>

A bit of a megapatch. Here's what's in it: * First, a few miscellaneous utility functions and reexports in Cabal. I could have split these into a separate commit but I was too lazy to. * Distribution.Client.Install got refactored: instead of using PackageFixedDeps, it uses IsUnit instead. This is because we weren't using ComponentDeps in a nontrivial way; we just need some graph structure and IsNode (with UnitId keys) fulfills that. I also removed the invariant checking and error reporting because it was being annoying (we check the invariants already in SolverInstallPlan). * Look at Distribution.Client.ProjectPlanning.Types. This contains the primary type change: ElaboratedConfiguredPackage is now EITHER a monolithic ElaboratedPackage, or a per-component ElaboratedComponent (it should get renamed but I didn't do that in this patch.) These are what we're going to store in our plans: if a package we're building has a Setup script which supports per-component builds, we'll explode it into a component. Otherwise we'll keep it as a package. We'll see codepaths for both throughout. * OK, so the expansion happens in ProjectPlanning, mostly in 'elaborateAndExpandSolverPackage'. You should review the package hash computation code closely. When we can separate components, we compute a hash for each INDEPENDENTLY. This is good: we get more sharing. * We need to adjust the target resolution and pruning code in ProjectOrchestration and ProjectPlanning. I did a dumb but easy idea: if a user mentions 'packagename' in a target name, I spray the PackageTarget on every possibly relevant IPID in buildTargets', and then pare it down later. * And of course there's code in ProjectBuilding to actual do a configure and then build. * We change the layout of build directories so that we can track each component separately. While I was doing that, I also added compiler and platform information. Custom doesn't work yet because I need to give them their own separate component, and teach Cabal how to build them specially. Signed-off-by: Edward Z. Yang <[email protected]>

Signed-off-by: Edward Z. Yang <[email protected]>

Two big ideas: * @--dependency@ takes a ComponentId, not UnitId. I used to think it should be a UnitId but it is now clear that you want to finger the indefinite unit id, which can be uniquely identified with a ComponentId * When hashing for an InstalledPackageId in new-build, we should produce a ComponentId, not a UnitId. While cleaning up the results, for any codepaths which we don't plan on implementing Backpack (Distribution.Client.Install, I'm looking at you), just coerce ComponentId into UnitIds as necessary. Signed-off-by: Edward Z. Yang <[email protected]>

…ds are destructed. Signed-off-by: Edward Z. Yang <[email protected]>

@hvr

This causes strange output when we output the install plan. Thanks @hvr for reporting. Signed-off-by: Edward Z. Yang <[email protected]>

Signed-off-by: Edward Z. Yang <[email protected]>

As requested by Duncan, the majority of fields that originally lived in ElaboratedPackage now are moved to ElaboratedConfiguredPackage under the prefix 'elab'. Some code has gotten simpler as a result. Signed-off-by: Edward Z. Yang <[email protected]>

Signed-off-by: Edward Z. Yang <[email protected]>

This fixes haskell#220: new-build now builds, installs and adds executables to PATH automatically if they show up in 'build-tools'. However, there is still more that could be done: the new behavior only applies to a specific list of 'build-tools' (alex, happy, etc) which Cabal recognizes out of the box. The plan is to introduce a new 'tool-depends' field to allow dependencies on other executables as well. Signed-off-by: Edward Z. Yang <[email protected]>

Signed-off-by: Edward Z. Yang <[email protected]>

Actually we could probably do this a bit more properly with UnitId in the Backpack patchset. Signed-off-by: Edward Z. Yang <[email protected]>

- New "exe-depends" field - Dropped "depends" when it's a package; you can use "components" to get the information Signed-off-by: Edward Z. Yang <[email protected]>

phadej reviewed Aug 2, 2016
View reviewed changes

ezyang force-pushed the pr/per-component-install branch from 58a7e58 to 3b6b885 Compare August 3, 2016 00:51

ezyang force-pushed the pr/per-component-install branch from c2399c3 to cb56193 Compare August 5, 2016 07:06

ezyang mentioned this pull request Aug 6, 2016

Backpack #3672

Merged

ezyang force-pushed the pr/per-component-install branch 2 times, most recently from aa537f1 to 143218e Compare August 7, 2016 21:13

ezyang mentioned this pull request Aug 11, 2016

One-component configure #3644

Closed

ezyang force-pushed the pr/per-component-install branch 2 times, most recently from 2640d8d to a1f1884 Compare August 11, 2016 22:15

dcoutts reviewed Aug 12, 2016
View reviewed changes

ezyang mentioned this pull request Aug 12, 2016

copy command fails when building only some components #2780

Closed

dcoutts reviewed Aug 12, 2016
View reviewed changes

ezyang force-pushed the pr/per-component-install branch 2 times, most recently from eaab985 to e3dda99 Compare August 12, 2016 05:13

dcoutts reviewed Aug 14, 2016
View reviewed changes

ezyang mentioned this pull request Aug 20, 2016

Need some clarification re qualifiers and linking #3706

Closed

ezyang force-pushed the pr/per-component-install branch from 85950be to c24ec41 Compare August 21, 2016 00:15

ezyang force-pushed the pr/per-component-install branch from fbe5d8e to ea42701 Compare August 21, 2016 09:00

ezyang added 23 commits August 21, 2016 02:02

Add configArgs parameter to ConfigFlags.

a06460c

Signed-off-by: Edward Z. Yang <[email protected]>

Fix pretty-printing PackageDescription for good.

0589974

Signed-off-by: Edward Z. Yang <[email protected]>

showComponentTarget remove dependence on ElaboratedPackage.

5c410b3

Signed-off-by: Edward Z. Yang <[email protected]>

Refactor showBuildTarget to not require QualLevel, making it total.

36a186a

Signed-off-by: Edward Z. Yang <[email protected]>

Docs and modest safety improvements.

929679c

Signed-off-by: Edward Z. Yang <[email protected]>

Fix haskell#1541, by adding internal build-tools to PATH.

6764810

Signed-off-by: Edward Z. Yang <[email protected]>

Add record selectors for PackageIndex, and use them when not all fiel…

56bb80b

…ds are destructed. Signed-off-by: Edward Z. Yang <[email protected]>

Prevent nodeNeighbors from returning a node multiple times

15dd845

This causes strange output when we output the install plan. Thanks @hvr for reporting. Signed-off-by: Edward Z. Yang <[email protected]>

Use per-component install directories.

4e782d6

Signed-off-by: Edward Z. Yang <[email protected]>

Don't provide --constraints when pkgComponent is Just.

bb08006

Signed-off-by: Edward Z. Yang <[email protected]>

Only put library dependencies in elabComponentDependencies.

18cb246

Signed-off-by: Edward Z. Yang <[email protected]>

Correctly track internal executable dependencies as exe deps.

4b4690b

Signed-off-by: Edward Z. Yang <[email protected]>

Fix build-tools PATH usage with per-component new-build

da15a6f

Signed-off-by: Edward Z. Yang <[email protected]>

Introduce InstallPlan.depends = nodeNeighbors and use it.

f63273d

Signed-off-by: Edward Z. Yang <[email protected]>

Remove stale comment.

0e40859

Signed-off-by: Edward Z. Yang <[email protected]>

Try to not redo building executables if we see the cid in the store.

10a9c4a

Actually we could probably do this a bit more properly with UnitId in the Backpack patchset. Signed-off-by: Edward Z. Yang <[email protected]>

Tweaks to plan.json format

4a9f11e

- New "exe-depends" field - Dropped "depends" when it's a package; you can use "components" to get the information Signed-off-by: Edward Z. Yang <[email protected]>

ezyang force-pushed the pr/per-component-install branch from ea42701 to 4a9f11e Compare August 21, 2016 09:02

ezyang merged commit 4a9f11e into haskell:master Aug 21, 2016

grayjay mentioned this pull request Nov 5, 2016

Per-component dependency solving #4087

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Per-component new-build support (no Custom support yet) #3662

Per-component new-build support (no Custom support yet) #3662

ezyang commented Aug 2, 2016

phadej Aug 2, 2016

ezyang commented Aug 2, 2016

ezyang commented Aug 2, 2016

dcoutts commented Aug 2, 2016

ezyang commented Aug 4, 2016

ezyang commented Aug 4, 2016

ezyang commented Aug 9, 2016

hvr commented Aug 9, 2016 •

edited

Loading

dcoutts Aug 12, 2016 •

edited

Loading

ezyang Aug 12, 2016

dcoutts Aug 12, 2016

ezyang Aug 12, 2016

ezyang Aug 12, 2016

dcoutts Aug 14, 2016

ezyang Aug 14, 2016

dcoutts Aug 14, 2016

hvr commented Aug 21, 2016

Per-component new-build support (no Custom support yet) #3662

Per-component new-build support (no Custom support yet) #3662

Conversation

ezyang commented Aug 2, 2016

Choose a reason for hiding this comment

ezyang commented Aug 2, 2016

ezyang commented Aug 2, 2016

dcoutts commented Aug 2, 2016

ezyang commented Aug 4, 2016

ezyang commented Aug 4, 2016

ezyang commented Aug 9, 2016

hvr commented Aug 9, 2016 • edited Loading

dcoutts Aug 12, 2016 • 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

hvr commented Aug 21, 2016

hvr commented Aug 9, 2016 •

edited

Loading

dcoutts Aug 12, 2016 •

edited

Loading