-
Notifications
You must be signed in to change notification settings - Fork 31
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
Per component building, and hackage DB #6
Per component building, and hackage DB #6
Conversation
Only reasons I wouldn't merge this straight away are:
|
|
lib/Cabal2Nix.hs
Outdated
@@ -94,7 +95,6 @@ shakeBranch :: (Foldable t, Foldable f) => CondBranch v (t c) (f a) -> Maybe (Co | |||
shakeBranch (CondBranch c t f) = case (shakeTree t, f >>= shakeTree) of | |||
(Nothing, Nothing) -> Nothing | |||
(Nothing, Just f') -> shakeBranch (CondBranch (CNot c) f' Nothing) | |||
(Just (CondNode d _c [(CondBranch c' t' f')]), Nothing) | null d -> Just $ CondBranch (CAnd c c') t' f' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@angerman Oh, I removed this line because it was clearly causing things to be mistranslated. IIRC, there was some package that depended on Win32
conditionally that this broke, causing it to always depend on it. Looking at this line, it seems wrong, but I admit I don't really fully understand this function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ElvishJerricco this could absolutely be wrong. The idea is to simplify the tree and drop unnecessary branching.
So assuming we are in a conditional branch, and clean up both t
rue and f
alse subtrees,
now we have only the t
rue side, and nothing left for the f
alse side. We should be able to ignore this, and move the conditional with an AND one node down.
Before shaking:
- c - <true sub tree: t>
'-- <false sub tree: f>
after shaking:
- c - <some node with no interesting data, but a another branch>
'-- Nothing
so this looks a bit like this:
- c - c' - <true subtree: t'>
| '--- <false subtree: f'>
'------- Nothing
So we try to simplify this as
- (c && c') - <true subtree: t'>
'------ <false subtree: f'>
And this is where the logic is wrong I guess. If c
is false
, we don't want f'
, however the simplification would yield that instead of nothing.
@ElvishJerricco I'll go ahead an selectively merge parts of this (e.g. the incorrect For the NixOS/hackage-db#9 issue, that seems like it's not too hard to fix? |
@angerman Example usage from this branch (dunno about changes in master): cabal new-run hackage-to-nix -- ./hackage # Takes a while
cabal new-run lts-to-nix -- ./lts-haskell/lts-11.4.yaml | tail -n+2 > stackage.nix # cabal outputs "Up to date"
nix build -f nix/default.nix \
--arg planFunc "import ./stackage.nix" \
--arg hackage "import ./hackage" \
hsPkgs.cabal-install.components.exes.cabal |
You can use the output of |
#6 included "Per component building, and hackage DB", this PR only pulls in the relevant hackage-db logic. The per component building (nix) will be a separate PR.
58bbcae
to
4ebd797
Compare
This rather crudely integrates most of the haskell changes from elvishjerricco/per-component-new-names.
Closing in favor of #9 and https://github.com/angerman/haskell.nix/pull/2 |
#6 included "Per component building, and hackage DB", this PR only pulls in the relevant hackage-db logic. The per component building (nix) will be a separate PR.
This rather crudely integrates most of the haskell changes from elvishjerricco/per-component-new-names.
No description provided.