Skip to content

Backpack #3672

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 46 commits into from
Oct 6, 2016
Merged

Backpack #3672

merged 46 commits into from
Oct 6, 2016

Conversation

ezyang
Copy link
Contributor

@ezyang ezyang commented Aug 6, 2016

This patchset is based on #3662, so it really only encompasses the last patch in the set, Backpack.

Where to start when reviewing? Fortunately, I am working on a specification at https://github.com/ezyang/ghc-proposals/blob/backpack/proposals/0000-backpack.rst which contains contains a textual specification of everything this patchset does, including module-by-module descriptions of each part of the patch. The cabal-install part is not done, but Cabal is substantially done, except for an in-depth description of how mix-in linking works.

@ezyang ezyang mentioned this pull request Aug 6, 2016
38 tasks
@ezyang ezyang force-pushed the backpack branch 4 times, most recently from 6e93c4f to a6ec56f Compare August 12, 2016 06:56
@ezyang ezyang force-pushed the backpack branch 10 times, most recently from 26c0592 to 6c376d0 Compare August 21, 2016 22:01
@ezyang ezyang force-pushed the backpack branch 3 times, most recently from 08caa3f to ad3e00f Compare September 8, 2016 06:56
compatPackageKey = "",
license = UnspecifiedLicense,
instantiatedWith = [],
compatPackageKey = "", license = UnspecifiedLicense,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

accidental joining of lines

++ ". To use this new syntax, the package needs to specify at least"
++ "'cabal-version: >= 1.21'."
"To use the 'backpack-includes' field the package needs to specify "
++ "at least 'cabal-version: >= 1.25'."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we getting rid of thinning and renaming in the build-depends? Are there released packages using this or can we just drop it?

Copy link
Contributor Author

@ezyang ezyang Sep 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're just dropping it. (Last I checked) there don't seem to be any released packages using this.

@dcoutts
Copy link
Contributor

dcoutts commented Sep 19, 2016

What's your schedule here? Do you want to get in the early changes so we can minimise the diff a bit?

@ezyang
Copy link
Contributor Author

ezyang commented Sep 19, 2016

These bits need to get merged before GHC, because GHC needs Cabal changes. Perhaps we should just go ahead and merge? I need to talk to Derek about possibly bringing back required versus exposed signatures, to support PVP with Backpack (the capsule summary is that in the current design, it's not possible to do version ranges on packages which have requirements in a PVP-abiding way; you always have to fix a specific version.)

@dcoutts
Copy link
Contributor

dcoutts commented Sep 19, 2016

What part does GHC need? Is this in the build system? The compiler part doesn't depend on Cabal anymore. Or is it for ghc-pkg?

@ezyang
Copy link
Contributor Author

ezyang commented Sep 26, 2016

It needs build system, as well as things for ghc-pkg.

@ezyang ezyang force-pushed the backpack branch 3 times, most recently from 98a262c to f57676b Compare September 27, 2016 08:16
@dcoutts
Copy link
Contributor

dcoutts commented Sep 27, 2016

First 5 patches still ok. :-)

Can we really not break up the big one any further? I'm just wondering how to properly tackle it for review...

@ezyang
Copy link
Contributor Author

ezyang commented Sep 27, 2016

Here's the dependency order of module changes in Cabal:

Distribution/Utils/Base62.hs
Distribution/Utils/UnionFind.hs
Distribution/Utils/MapAccum.hs
Distribution/Utils/Progress.hs
Distribution/Package.hs
Distribution/Backpack.hs
Distribution/Backpack/ModSubst.hs
Distribution/Simple/InstallDirs.hs
Distribution/Types/ModuleRenaming.hs
Distribution/Types/IncludeRenaming.hs
Distribution/Backpack/ModuleScope.hs
Distribution/Types/BuildInfo.hs
Distribution/Types/Library.hs
Distribution/PackageDescription.hs
Distribution/PackageDescription/Configuration.hs
Distribution/InstalledPackageInfo.hs
Distribution/Simple/GHC/IPI642.hs
Distribution/Backpack/ModuleShape.hs
Distribution/Backpack/PreExistingComponent.hs
Distribution/Backpack/UnifyM.hs
Distribution/Backpack/MixLink.hs
Distribution/Types/ComponentLocalBuildInfo.hs
Distribution/PackageDescription/Parse.hs
Distribution/Simple/PackageIndex.hs
Distribution/Utils/LogProgress.hs
Distribution/Simple/Setup.hs
Distribution/Types/LocalBuildInfo.hs
Distribution/Simple/LocalBuildInfo.hs
Distribution/PackageDescription/Check.hs
Distribution/Backpack/ComponentsGraph.hs
Distribution/Backpack/Id.hs
Distribution/Backpack/ConfiguredComponent.hs
Distribution/Backpack/LinkedComponent.hs
Distribution/Backpack/InstantiatedComponent.hs
Distribution/Backpack/Configure.hs
Distribution/Simple/Program/GHC.hs
Distribution/Simple/GHC/Internal.hs
Distribution/Simple/Program/HcPkg.hs
Distribution/Simple/LHC.hs
Distribution/Simple/GHC.hs
Distribution/Simple/GHCJS.hs
Distribution/Simple/Register.hs
Distribution/Simple/PreProcess.hs
Distribution/Simple/Configure.hs
Distribution/Simple/Build.hs
Distribution/Simple/Haddock.hs
Distribution/Simple/SrcDist.hs

ezyang added 27 commits October 6, 2016 00:10
These may NOT be explicitly specified in the Cabal file;
we read it off of 'componentInstantiatedWith'.
Correctly setup 'instantiatedWith' in 'InstalledPackageInfo'.

Also ensure that there are no duplicate entries in 'depends'
(I couldn't find the root cause for duplication, so I just
put the test here.)
Now that we can build a component multiple times with
different instantiations, we need to make sure we give each
instantiation a different build directory; done in 'buildDir'.
Part of the GHC/Cabal contract is that we always have a source file
for everything GHC works on.
Count signatures as modules that we have to find, and when searching for
modules, look for .hsig or .lhsig files (in addition to the usual .hs
.lhs files)
These are libraries which don't have any code (uses 'whenHasCode'
combinator).  In principle this could also be extended to support
building anything with no code.

Also update 'ghcOptThisUnitId' and 'ghcOptInstantiatedWith' to
have correct values.
1) Bugfix so that we get library source files from the correct
   directories (it was wrong previously; it only ever looked
   in the library directory)
2) Search for hsig/lhsig files when looking for source files
Add a clarification in packageTemplateEnv

Improve the pre-processing message so we can see which unit is being
built, not just the source component name.

Debug output while building to print out the installed package info
when we register information.

Remove a done TODO about making sure the installed package registration
files do not clash with each other, by including the full UnitId. This
did not need any code changes here since dislaying the UnitId does the
right thing.
Here are the main changes:

Distribution/Client/InstallPlan.hs
    New utility function 'fromSolverInstallPlanWithProgress'
    which is a monadic version of 'fromSolverInstallPlan'.
    This is because, with Backpack, the conversion from
    'SolverInstallPlan' to 'InstallPlan' can fail/log.

Distribution/Client/ProjectPlanning/Types.hs
    OK. A bunch of new information we need to track.

    New fields in 'ElaboratedConfiguredPackage':
        elabInstantiatedWith :: ModuleSubst (for --instantiated-with)
        elabLinkedInstantiatedWith :: IndefModuleSubst (intermediate)
        elabModuleShape :: ModuleShape (for mix-in linking)

    Here is how all the dependency functions relate to
    one another:

        elabOrderDependencies :: [UnitId]
            Used for nodeNeighbors, this just specifies what needs
            to be built before we build this module.  These refer
            either to fully instantiated unit ids (hashed unit id)
            or uninstantiated unit ids (effectively component id)
            but never a partially instantiated unit id, since we
            never have an install item in our plan for a partially
            instantiated package.

            These dependencies are factored into two parts:
                elabOrderLibDependencies
                elabOrderExeDependencies
            which soley are used to determine if we need to enable
            executables/libraries of a package we are building
            (this isn't new)

        elabLibDependencies :: [ComponentId]
            These are the things we pass to Setup using the --dependency
            flag; so they are JUST ComponentId, not a full on UnitId.
            The mix-in linking process in Setup will reconstruct the
            necessary UnitId.

        elabExeDependencies :: [ComponentId]
            These are the things that we must add to the PATH to run.
            At the moment, this coincides with elabOrderExeDependencies.

    For components, there is also:

        compLinkedLibDependencies :: [IndefUnitId],
            The partially instantiated unit ids that GHC would be
            invoked with, if we were invoking it directly.
            This is used when we subsequently instantiate components.

        compNonSetupDependencies :: [UnitId]
            Non-setup, ORDER dependencies; i.e., everything that has
            to be built before us that is not a setup script.

Distribution/Client/ProjectPlanning.hs
    The workhorse.

    Essentially, we redo all of the steps from
    Distribution.Backpack.Configure, but in the context of planning an
    entire install plan.  The conversion from SolverInstallPlan
    to InstallPlan is responsible for mix-in linking
    (inside elaborateSolverToComponents); afterwards,
    instantiateInstallPlan is responsible for filling in
    the missing, instantiated packages which we need to compile.

Signed-off-by: Edward Z. Yang <[email protected]>
The DefUnitId invariant says that the UnitId in a DefUnitId
must in fact be a definite package (either with no holes, or
fully instantiated.)  This is in constrast to a UnitId,
which can also identify an indefinite unit identifier.

Signed-off-by: Edward Z. Yang <[email protected]>
@ezyang
Copy link
Contributor Author

ezyang commented Oct 6, 2016

OK after this greens I'm going to merge.

@ezyang ezyang merged commit 8fa4d2e into haskell:master Oct 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants