Skip to content
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

Convenience libraries #3022

Merged
merged 30 commits into from Mar 29, 2016

Conversation

Projects
None yet
4 participants
@ezyang
Copy link
Contributor

ezyang commented Jan 3, 2016

Implement "convenience libraries", fixes #269.

Convenience libraries are package-private libraries
that can be used as part of executables, libraries, etc
without being exposed to the external world. Private
libraries are signified using the

library foo

stanza. Within a Cabal package, the name convenience library
shadows the conventional meaning of package name in
build-depends, so that references to "foo" do not indicate
foo in Hackage, but the convenience library defined in the
same package. (So, don't shadow Hackage packages!)

This commit implements convenience libraries such that they
ARE installed the package database (this prevents us from
having to special case dynamically linked executables);
in GHC 7.10 and later they are installed under the same
package name as the package that contained them, but have
a distinct "component ID" (one pay off of making the distinction
between component IDs and installed package IDs.)

There is a "default" library which is identified by the fact
that its library name coincides with the package name. There
are some new convenience functions to permit referencing this.

TODO: if we REALLY don't want to install a convenience library,
we could add a "installable:" flag which toggles whether or
not copy/register should install it. Extra sanity checking
is necessary in this case.

TODO: docs

TODO: cabal-install must ignore convenience libraries when
dep solving

NB: there are a few extra commits in this PR which should be split into their own PRs. Annoying to do slightly.

Signed-off-by: Edward Z. Yang ezyang@cs.stanford.edu

@ezyang ezyang force-pushed the ezyang:convenience-libraries branch 4 times, most recently from a2ae14b to 735d7cb Jan 4, 2016

@dcoutts

This comment has been minimized.

Copy link
Member

dcoutts commented Jan 10, 2016

In principle this is a great feature.

From brief discussion we think not installing the convenience library when it's not needed is a good improvement, and we can determine exactly when it's needed or not so no user visible flag is needed.

@dcoutts

This comment has been minimized.

Copy link
Member

dcoutts commented Jan 10, 2016

For other people reviewing this: the important point is that these libs are not available for other packages to depend on, so it's still the case that one package provides at most one library. And the installed package id for private libs is picked such that it can never clash with primary public libs from any other package.

@ezyang ezyang force-pushed the ezyang:convenience-libraries branch from 735d7cb to cee286b Jan 10, 2016

@23Skidoo

This comment has been minimized.

Copy link
Member

23Skidoo commented Jan 11, 2016

@ezyang Do you want this to go into 1.24? Otherwise I think I'll postpone merging this PR until the 1.24 branch is created.

@ezyang

This comment has been minimized.

Copy link
Contributor Author

ezyang commented Jan 11, 2016

Yeah, I'm fine with postponing this after 1.24.

@ezyang

This comment has been minimized.

Copy link
Contributor Author

ezyang commented Jan 11, 2016

Found a bug: internal libraries and the main libraries all get installed to the same directory.

@ezyang ezyang force-pushed the ezyang:convenience-libraries branch from cee286b to 0ecdcea Jan 12, 2016

@ezyang

This comment has been minimized.

Copy link
Contributor Author

ezyang commented Jan 12, 2016

So, I fixed all those bugs, and along the way also fixed #1893 and also did a bit of refactoring. The patchset got longer but these are all self-contained commits.

@ezyang ezyang force-pushed the ezyang:convenience-libraries branch from 0ecdcea to 2c3f3ca Jan 12, 2016

@ezyang ezyang force-pushed the ezyang:convenience-libraries branch from be5222b to 590f505 Jan 13, 2016

@ezyang ezyang referenced this pull request Jan 13, 2016

Closed

Backpack design ticket #3038

@ezyang ezyang force-pushed the ezyang:convenience-libraries branch 4 times, most recently from d66990d to 20584e7 Jan 13, 2016

@ezyang

This comment has been minimized.

Copy link
Contributor Author

ezyang commented Jan 14, 2016

Blah, I broke buildinfo parsing. I'll fix it and add a test.

@ezyang ezyang force-pushed the ezyang:convenience-libraries branch 2 times, most recently from 9a0936c to 2389f80 Jan 14, 2016

@ezyang

This comment has been minimized.

Copy link
Contributor Author

ezyang commented Jan 15, 2016

(NB: this branch has been rebased on top of #3047)

This was referenced Jan 19, 2016

@ezyang ezyang force-pushed the ezyang:convenience-libraries branch from 2389f80 to 10b5140 Feb 2, 2016

ezyang added some commits Jan 13, 2016

Warning about compatibility requirements on Setup.hs in cabal-install.
Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>
Always generate CURRENT_COMPONENT_ID macros.
Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>
Minor renaming to avoid conflicts.
Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>
Warn if user attempts to define a package with a reserved name.
Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>
Minor refactoring of compatibility keys, streamline library keys.
Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>
Fix HookedBuildInfo parsing to support old-style format (needs test.)
Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>
Add NFData instance to ModuleName.
Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>
Ignore bak files.
Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>
Enforce unique naming for internal libraries as well.
Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>
Take advantage of unique section naming to simplify some paths.
Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>
Fix test-case after simplified component IDs.
Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>
Accept components to copy in ./Setup copy, fixes #2780.
Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>
Test case for the Configure build-type.
This test-case was lifted straight from the Cabal manual.
It also tests if buildinfo is handled correctly.

Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>
Disable tests involving shared libraries on Windows.
Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>
Make GHC flag comment less cryptic.
Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>
Generalize HookedBuildInfo to work with any type of component.
Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>
Use ADT for MultiInstance boolean argument.
Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>

@ezyang ezyang force-pushed the ezyang:convenience-libraries branch from 36106ae to 40d6f0a Mar 29, 2016

@ezyang ezyang merged commit 40d6f0a into haskell:master Mar 29, 2016

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@ezyang ezyang referenced this pull request Mar 31, 2016

Open

Testing plan for cabal-install and convenience libraries #3257

4 of 6 tasks complete
@23Skidoo

This comment has been minimized.

Copy link
Member

23Skidoo commented Mar 31, 2016

@ezyang

Can you please create tickets for the remaining TODOs?

As far as I can tell (it's a little difficult since when I rebased we lost the old comments) all comments have been addressed. I'll merge.

I meant the TODOs you mentioned in the top comment, i.e. docs, a change to the solver and the "installable" flag. Though I see that docs have now been added.

@ezyang

This comment has been minimized.

Copy link
Contributor Author

ezyang commented Apr 1, 2016

Oh yeah, those comments are out of date. The "installable" flag is irrelevant now, since Cabal just "does the right thing" now to determine if a library should be installed or not. Solver changes are being tracked at #3264/#3257.

@23Skidoo

This comment has been minimized.

Copy link
Member

23Skidoo commented Apr 1, 2016

Great.

@ezyang ezyang deleted the ezyang:convenience-libraries branch Apr 8, 2016

@mgsloan

This comment has been minimized.

Copy link
Collaborator

mgsloan commented on Cabal/tests/PackageTests/Tests.hs in f88f502 Aug 12, 2016

Why does this deviate from the syntax for configuring components? Doesn't make sense.

This comment has been minimized.

Copy link
Contributor Author

ezyang replied Aug 12, 2016

I don't quite understand the comment. I think you are commenting, why ist he Lib test case using lib:p, but the Exe test case using myprog? Actually both syntaxes work; I could have written the lib case as cabal build p and that would have been fine. The code for parsing the targets is equivalent.

There is one case when you might need to qualify, and that is when the package that has a library has an executable with the same name as a package. Then p might be lib:p or exe:p.

Let me know if there's some change you want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.