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

Fix #3345 by having Install manually register packages #3356

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@ezyang
Copy link
Contributor

ezyang commented Apr 17, 2016

Just the last commit.

cabal sandbox init
cabal sandbox add-source custom
cabal sandbox add-source client
# Set HOME to something goofy so that we force the user

This comment has been minimized.

@23Skidoo

23Skidoo Apr 19, 2016

Member

Can't you just use --package-db=clear --package-db=global?

This comment has been minimized.

@ezyang

ezyang Apr 20, 2016

Contributor

No, that does not work, because these settings don't apply to Setup.hs build.

$ ghc-pkg-7.6 list
/opt/ghc/7.6.3/lib/ghc-7.6.3/package.conf.d
   Cabal-1.16.0
/home/ezyang/.ghc/x86_64-linux-7.6.3/package.conf.d
   Cabal-1.22.3.0
   Cabal-1.23.0.0
   Cabal-1.23.1.0
   Cabal-1.23.2.0
$ cabal configure -v --package-db=clear --package-db=global -w ghc-7.6
...
/srv/prod/bin/ghc-7.6 --make -odir ./dist/setup -hidir ./dist/setup -i -i. -package-id Cabal-1.22.3.0-9afd7fdd87eadc07ff9f73c86e22d4fe ./dist/setup/setup.hs -o ./dist/setup/setup -threaded

I'll add a comment to this effect.

maybePkgConfs <- maybeGenPkgConfs mLogPath

return (Right (BuildOk docsResult testsResult maybePkgConfs))
-- TODO: This is duplicated with

This comment has been minimized.

@23Skidoo

23Skidoo Apr 19, 2016

Member

I couldn't find the fragment you're referring to, would be nice if this comment was a bit more expanded.

This comment has been minimized.

@ezyang

ezyang Apr 20, 2016

Contributor

OK.

@23Skidoo

This comment has been minimized.

Copy link
Member

23Skidoo commented Apr 19, 2016

/cc @dcoutts

@ezyang ezyang force-pushed the ezyang:pr/T3345 branch from d053c59 to 8732c2b Apr 21, 2016

@dcoutts

This comment has been minimized.

Copy link
Member

dcoutts commented Apr 23, 2016

Having cabal call setup to get the package reg info and then doing the registration ourselves is in principle the right thing to do (and what the new-build code does).

I've not looked at the code in detail, but it's the right strategy. @23Skidoo so if you're happy with the code details then go for it.

@23Skidoo

This comment has been minimized.

Copy link
Member

23Skidoo commented Apr 23, 2016

Yes, LGTM. @ezyang, feel free to merge the last commit.

@ezyang

This comment has been minimized.

Copy link
Contributor

ezyang commented Apr 27, 2016

We can't merge this without also merging #3355 (otherwise the patch needs to be substantially reworked.)

ezyang added some commits Apr 17, 2016

Remove support for root-cmd, fixes #3353.
Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>
Fix #3345 by having Install manually register packages
Unfortunately, it was too difficult to factor out the common code
between ProjectBuilding and Install.

Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>
Give a message that --root-cmd is not supported and --global is depre…
…cated.

Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment