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

build-tool-depends silently ignored for non-existent executable #4781

Closed
hvr opened this issue Sep 21, 2017 · 7 comments · Fixed by #4884
Closed

build-tool-depends silently ignored for non-existent executable #4781

hvr opened this issue Sep 21, 2017 · 7 comments · Fixed by #4884

Comments

@hvr
Copy link
Member

hvr commented Sep 21, 2017

Consider the following specification,

name:                dummypkg
version:             0
build-type:          Simple
cabal-version:       >=2.0

library
  default-language: Haskell2010
  build-tool-depends: mtl:imaginary-exe

when invoking new-build, this silently succeeds. And also, plan.json doesn't record any exe-depends from dummypkg-0-inplace on mtl as it would if build-tool-depends referred to an actually existing executable:

    {
      "type": "configured",
      "id": "dummypkg-0-inplace",
      "pkg-name": "dummypkg",
      "pkg-version": "0",
      "flags": {},
      "style": "local",
      "dist-dir": ".../dist-newstyle/build/x86_64-linux/ghc-8.2.1/dummypkg-0",
      "depends": [],
      "exe-depends": [],
      "component-name": "lib"
    },

For a build-tool-depends declaration, I'd expect the solver to require the existence of that executable in the respective package as a constraint to solve for. The reason being is that packages may start out as a library, and in later releases start adding an executable. Or sometimes executables may get renamed or removed.

/cc @grayjay @Ericson2314

@hvr hvr added the type: bug label Sep 21, 2017
@grayjay
Copy link
Collaborator

grayjay commented Sep 23, 2017

It looks like cabal doesn't solve for specific executables yet:

-- TODO do something about the name of the exe component itself
convExeDep :: DependencyReason PN -> ExeDependency -> LDep PN
convExeDep dr (ExeDependency pn _ vr) = LDep dr $ Dep (IsExe True) pn (Constrained vr)

That will probably require #4087.

@Ericson2314
Copy link
Collaborator

@grayjay yeah that was a stop-gap in lieu of per-component solving. Is a better solution possible now?

@phadej
Copy link
Collaborator

phadej commented Sep 23, 2017

Can we make specific constraints, including only versions of package with an executable requested. That should work even we don't have per-component solving

@Ericson2314
Copy link
Collaborator

That would be nice. To be clear, this still fails on the Cabal library side of things, right? I added a bunch of tests and stuff for those failure cases.

@grayjay
Copy link
Collaborator

grayjay commented Sep 25, 2017

@Ericson2314 I don't know of a better solution now.

@phadej Where would the constraints be specified? It seems like that would be easier to implement than complete per-component dependency solving.

@phadej
Copy link
Collaborator

phadej commented Sep 25, 2017

@grayjay when the build-tool-depends executable is requested, we could elaborate that into more precise constraints than just package (+ bounds). I don't know how it happens now though :(

@grayjay
Copy link
Collaborator

grayjay commented Sep 25, 2017

I think that would need to happen in the solver, after it chooses to build a package that has a build-tool-depends dependency. That corresponds to the first tree transformation that I described under "Component dependency validation steps" in #4087 (comment). I don't think we need to add the other tree transformations as long as we continue to require the dependencies for all executables for each package.

@grayjay grayjay self-assigned this Oct 14, 2017
grayjay added a commit to grayjay/cabal that referenced this issue Nov 11, 2017
This commit adds two checks to the validation phase of the solver:

1. It checks that each newly chosen package instance contains all executables
   that are required from that package so far.

2. It checks that each new build tool dependency that refers to a previously
   chosen package can be satisfied by the executables in that package.

This commit also fixes a TODO related to solver log messages. Previously, it was
possible for the log to associate an incorrect executable name with a
dependency.
grayjay added a commit to grayjay/cabal that referenced this issue Nov 12, 2017
This commit adds two checks to the validation phase of the solver:

1. It checks that each newly chosen package instance contains all executables
   that are required from that package so far.

2. It checks that each new build tool dependency that refers to a previously
   chosen package can be satisfied by the executables in that package.

This commit also fixes a TODO related to solver log messages. Previously, it was
possible for the log to associate an incorrect executable name with a
dependency.
23Skidoo added a commit that referenced this issue Nov 22, 2017
Solver: Enforce dependencies on executables (fixes #4781).
grayjay added a commit to grayjay/cabal that referenced this issue May 6, 2018
This commit generalizes the fix for issue haskell#4781
(e86f838) by tracking dependencies on
components instead of dependencies on executables.  Associating each dependency
with a component also moves towards the design for component-based dependency
solving described in issue haskell#4087.
grayjay added a commit to grayjay/cabal that referenced this issue May 6, 2018
This commit generalizes the fix for issue haskell#4781
(e86f838) by tracking dependencies on
components instead of dependencies on executables.  That means that the solver
always checks whether a package contains a library before using it to satisfy a
build-depends dependency.  If a version of a package doesn't contain a library,
the solver can try other versions.  Associating each dependency with a component
also moves towards the design for component-based dependency solving described
in issue haskell#4087.
23Skidoo pushed a commit that referenced this issue May 9, 2018
This commit generalizes the fix for issue #4781
(e86f838) by tracking dependencies on
components instead of dependencies on executables.  That means that the solver
always checks whether a package contains a library before using it to satisfy a
build-depends dependency.  If a version of a package doesn't contain a library,
the solver can try other versions.  Associating each dependency with a component
also moves towards the design for component-based dependency solving described
in issue #4087.
hvr pushed a commit that referenced this issue Jun 8, 2018
This commit generalizes the fix for issue #4781
(e86f838) by tracking dependencies on
components instead of dependencies on executables.  That means that the solver
always checks whether a package contains a library before using it to satisfy a
build-depends dependency.  If a version of a package doesn't contain a library,
the solver can try other versions.  Associating each dependency with a component
also moves towards the design for component-based dependency solving described
in issue #4087.

(cherry picked from commit 6efb5e2)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants