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

Only choose buildable components for build-depends and build-tool-depends dependencies. #5325

Open
grayjay opened this issue May 15, 2018 · 9 comments
Labels
cabal-install: solver re: build-tool Concerning `build-tools` and `build-tool-depends` type: enhancement

Comments

@grayjay
Copy link
Collaborator

grayjay commented May 15, 2018

See #5304 (comment) and #3978 (comment) for an example involving http-client-0.5.12.1. The library stanza contains

  -- See build failure at https://travis-ci.org/snoyberg/http-client/jobs/359573631
  if impl(ghc < 7.10)
    buildable: False

The solver doesn't backtrack to find an older version with a buildable library, and cabal fails with (from #5304 (comment)):

$ cabal new-build -w ghc-7.8.4 --index-state='2018-05-09T06:20:23Z'
Error:
    Dependency on unbuildable library from http-client
    In the stanza 'library'
    In the inplace package 'zzz-0'

This issue is a continuation of #3978, which described a bad error message for an unbuildable component. Its regression test tests the current error message:

# cabal new-build
Resolving dependencies...
Error:
Dependency on unbuildable library from p
In the stanza 'library'
In the inplace package 'q-1.0'

This issue should probably be fixed as part of #4087.

@hvr
Copy link
Member

hvr commented May 19, 2018

@grayjay I'm promoting this issue to high prio as this is effectively causing a lot of install-plans to regress on Hackage currently, and adding a build-depends: base>=4.8 constraint (as @phadej did) to help the solver avoid running into the impl(ghc >= 7.10) buildable:False case unfortunately doesn't work because of a property that @ezyang already diagnosed 2 years ago in the referenced #3978:

when buildable: False we ignore the build-depends. This is desirable for executables but not for libraries.

Would something like ezyang@272b504 constitute a reasonable short-term stopgap solution (assuming that #4087 may take longer to materialise) for improving the situation for this http-client incident?

@phadej
Copy link
Collaborator

phadej commented May 19, 2018

I made a PR #5332, and it seems to solve my http-client related issue.

grayjay added a commit to grayjay/cabal that referenced this issue May 20, 2018
This commit handles the most common case of issue haskell#5325 by checking that each
package is buildable in the current environment.  The solver replaces
non-buildable packages with failure nodes.  It allows two types of buildable
packages:

Buildable non-test package:
The package has at least one buildable library or executable.

Buildable test package:
The package has no libraries or executables, but it does have at least one
buildable test or benchmark.

The buildable check can give false-positives, because it ignores all flags, and
it doesn't check whether the intra-package dependencies of a component are
buildable.  The check is also incomplete because it is performed before any
flags are assigned.  It is possible for the solver to later choose a value for a
flag that makes the package unbuildable.
@grayjay
Copy link
Collaborator Author

grayjay commented May 20, 2018

I'm still worried that that commit could prevent cabal from finding a solution in cases where it found a valid solution previously, such as installing a package that provides an executable and an optional, flag-controlled library. I would rather make a safer change, especially if it needs to be applied to the release branch.

I tried implementing my idea from #3988 (comment), which checks whether any of the package's components are buildable in the current environment: https://github.com/grayjay/cabal/tree/issue-5325. I think it would handle the issue with http-client, though I haven't tested it with GHC 7.8.4 yet. Do you think that could work as a short-term solution?

@phadej
Copy link
Collaborator

phadej commented May 20, 2018

That works even too well: https://github.com/haskell-servant/servant/blob/f88cfd0740be8b2e3085584ee59f20e034168e58/doc/cookbook/jwt-and-basic-auth/jwt-and-basic-auth.cabal#L17
errors with

cabal: Could not resolve dependencies:
[__0] next goal: cookbook-jwt-and-basic-auth (user goal)
[__0] rejecting: cookbook-jwt-and-basic-auth-0.0.1 (not buildable in the
current environment)
[__0] fail (backjumping, conflict set: cookbook-jwt-and-basic-auth)
After searching the rest of the dependency tree exhaustively, these were the
goals I've had most trouble fulfilling: cookbook-jwt-and-basic-auth

Without cookbook packages (some of which are !impl(ghc >= 7.10) buildable: False), cabal manages to find an install plan.


I agree the use of buildable: False in servant's cookbook is phishy, so maybe it's good it broke.

@phadej
Copy link
Collaborator

phadej commented May 20, 2018

To be clear: either (my PR or @grayjay's commit) works for me, I have no preference.

@hvr
Copy link
Member

hvr commented May 20, 2018

I've been dogfooding @phadej's PR a bit yesterday; but I like @grayjay's approach better and I'll dogfood grayjay@238286f today

@hvr
Copy link
Member

hvr commented May 20, 2018

Btw, @phadej's PR would refuse to build cabal-install, because we have that lib:cabal-install hack in there for CI which is by default (conditional on a manual flag) buildable:False. @grayjay's patch handles this better.

grayjay added a commit to grayjay/cabal that referenced this issue May 21, 2018
…ment.

This commit handles the most common case of issue haskell#5325 by checking that each
component that is required as a dependency is buildable in the current
environment, where environment refers to the compiler, os, arch, and global flag
constraints.  The solver records whether each component is buildable in the
package's PInfo.

The buildable check can give false-positives, because it only considers flags
that are set by unqualified flag constraints, and it doesn't check whether the
intra-package dependencies of a component are buildable.  The check is also
incomplete because it is performed before any automatic flags are assigned.  It
is possible for the solver to later choose a value for a flag that makes the
package unbuildable.
grayjay added a commit to grayjay/cabal that referenced this issue May 21, 2018
…ment.

This commit handles the most common case of issue haskell#5325 by checking that each
component that is required as a dependency is buildable in the current
environment, where environment refers to the compiler, os, arch, and global flag
constraints.  The solver records whether each component is buildable in the
package's PInfo during index conversion.  Then it checks that each required
component is buildable in the validation phase, similar to the check for missing
components.

The buildable check can give false-positives, because it only considers flags
that are set by unqualified flag constraints, and it doesn't check whether the
intra-package dependencies of a component are buildable.  The check is also
incomplete because it is performed before any automatic flags are assigned.  It
is possible for the solver to later choose a value for a flag that makes the
package unbuildable.
@grayjay
Copy link
Collaborator Author

grayjay commented May 21, 2018

Thanks for the feedback. I realized that I can also use the component dependency information from #5304 to only reject the package when the component is unbuildable and required by another package. That should allow cabal to handle the cookbook package, as long as nothing depends on it. I'll make a PR.

hvr pushed a commit that referenced this issue Jun 8, 2018
…ment.

This commit handles the most common case of issue #5325 by checking that each
component that is required as a dependency is buildable in the current
environment, where environment refers to the compiler, os, arch, and global flag
constraints.  The solver records whether each component is buildable in the
package's PInfo during index conversion.  Then it checks that each required
component is buildable in the validation phase, similar to the check for missing
components.

The buildable check can give false-positives, because it only considers flags
that are set by unqualified flag constraints, and it doesn't check whether the
intra-package dependencies of a component are buildable.  The check is also
incomplete because it is performed before any automatic flags are assigned.  It
is possible for the solver to later choose a value for a flag that makes the
package unbuildable.
hvr pushed a commit that referenced this issue Jun 8, 2018
…ment.

This commit handles the most common case of issue #5325 by checking that each
component that is required as a dependency is buildable in the current
environment, where environment refers to the compiler, os, arch, and global flag
constraints.  The solver records whether each component is buildable in the
package's PInfo during index conversion.  Then it checks that each required
component is buildable in the validation phase, similar to the check for missing
components.

The buildable check can give false-positives, because it only considers flags
that are set by unqualified flag constraints, and it doesn't check whether the
intra-package dependencies of a component are buildable.  The check is also
incomplete because it is performed before any automatic flags are assigned.  It
is possible for the solver to later choose a value for a flag that makes the
package unbuildable.

(cherry picked from commit b747032)
@hvr
Copy link
Member

hvr commented Jun 9, 2018

Current status:

@andreasabel andreasabel added the re: build-tool Concerning `build-tools` and `build-tool-depends` label Nov 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cabal-install: solver re: build-tool Concerning `build-tools` and `build-tool-depends` type: enhancement
Projects
None yet
Development

No branches or pull requests

4 participants