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

Handle apparent package dependency cycles due to test suites / benchmarks #1575

Open
tibbe opened this issue Nov 11, 2013 · 32 comments
Open

Handle apparent package dependency cycles due to test suites / benchmarks #1575

tibbe opened this issue Nov 11, 2013 · 32 comments

Comments

@tibbe
Copy link
Member

@tibbe tibbe commented Nov 11, 2013

Update by @ezyang: See @edsko's comment at #1575 (comment) we don't actually want to dep solver for each package individually, according to @dcoutts.

Update by @dcoutts: the latest iteration of idea 1 below is as follows:

Suppose for sake of argument we're looking at bytestring which has a test suite that uses tasty, and tasty depends indirectly on bytestring. So the original proposal was to resolve this cycle by building the under-development bytestring and then building tasty against that, and then building the bytestring testsuite against the freshly built tasty package. But actually this is crazy, it means every time I make a small edit to bytestring and want to re-run the tests, cabal has to go and rebuild tasty (and all the other deps of tasty that use bytestring). This would get really annoying really quickly. Also, we don't really want to build tasty against an in-development version of bytestring, we want tasty to use a stable version of bytestring. So much better is to have tasty be built using an existing stable bytestring. Then tasty etc does not need to be rebuilt as we make minor changes to the in-development version of bytestring.

So this requires we have a solution using two different version of bytestring. We already have something like this for dependencies of Setup scripts, which we call "independent" dependencies. So we can treat the dependencies of the test-suite /which it doesn't share with the library/ as independent. See #3422 for a partial implementation of this idea and a discussion of the difficulties.

Idea 2 below would require a different approach. In general it is important to have consistent behaviour of code in one component (e.g. exe) vs another component in the same package. This is what authors generally expect. This means that two exes both use some lib then they need to use the same version of that lib. If authors really want to allow two exes in the same package to use different versions of some dep then this ought to be explicit and we should come up with some mechanism to do so. If we go down this route then we should also consider the case of the same component using two different version of a lib. We might handle that like package qualified module imports, giving each one a different local name. This also relates to backpack.


I think we should create the install plan on a component (i.e. library, executable, test-suite, and benchmark) basis instead of creating one for the whole package.

There are two use cases I can think of:

  1. There's a growing number of packages (anything that's a dependency of either test-frame-work and its related packages or of criterion) that can't have test suite or benchmark sections, as that would create a dependency cycle.
  2. Users have requested being able to build executables (in a single package) that have conflicting dependencies.

I suggest that we treat each component as if it was a "package" with name package-x.y.z.w:component-name, for the purpose of dependency resolution.

@23Skidoo
Copy link
Member

@23Skidoo 23Skidoo commented Nov 11, 2013

+1, this would be nice to have.

@ygale
Copy link
Contributor

@ygale ygale commented Nov 28, 2013

#1532 might be another reason this would be a good idea.

@kosmikus
Copy link
Contributor

@kosmikus kosmikus commented Nov 28, 2013

I don't think that the solver-part would be really hard. But we need to be able to build and install the components separately, otherwise this is worthless.

@23Skidoo
Copy link
Member

@23Skidoo 23Skidoo commented Nov 29, 2013

But we need to be able to build and install the components separately, otherwise this is worthless.

With 1.18, we have this for build. I think that we don't need per-component install. The use cases Johan mentions don't actually require splitting packages other than the install target (one can only depend on libraries anyway, and there's guaranteed to be only a single library per package). We only need a way to selectively enable components with configure; for tests and benchmarks we already have that with --enable-tests and --enable-benchmarks.

So it looks to me that both use cases can be supported with no changes to the solver, only by splitting the install target into several virtual packages. Example of the first use case:

$ cd /path/to/$pkgid
$ cabal install --only-dependencies --enable-benchmarks --enable-tests
# Runs 'cabal install --only-dependencies $pkgid:library $pkgid:benchmark[0..N] $pkgid:test[0..N]' under the hood
$ cabal configure --enable-benchmarks --enable-tests
# Succeeds, since each component is treated as a separate package

For building a subset of exes something like configure --enable-component=foo,bar,baz will have to be added. Again, this will be needed only for the current target. Not sure if we want to allow installing such partially-configured packages with cabal install.

@ygale
Copy link
Contributor

@ygale ygale commented Nov 29, 2013

In #1541 you have a use case with two targets, A and B, with A an executable used as a preprocessor for the source code of library or executable B. A must be built and installed (or at least made accessible at some predictable path) before B is built.

@23Skidoo
Copy link
Member

@23Skidoo 23Skidoo commented Nov 29, 2013

@ygale

In #1541, all you want is to build A before B (A and B don't have conflicting dependencies). This should be possible to fix on the component graph level (since they are both a part of the same package), once there's a way to declare that B depends on A.

#1532 seems to be unrelated.

@ygale
Copy link
Contributor

@ygale ygale commented Nov 29, 2013

Yes, you are correct that in our case it happens that there are no conflicting dependencies. But that is certainly possible. So this is another use case.

Also, here you need access to the compiled executable for A before beginning to build B, so there needs to be some kind of "install" step for A before B is built. It doesn't necessarily need to be the same thing as cabal install, but the salted dist directory is not suitable.

@23Skidoo
Copy link
Member

@23Skidoo 23Skidoo commented Nov 29, 2013

Also, here you need access to the compiled executable for A before beginning to build B, so there needs to be some kind of "install" step for A before B is built.

A only needs to be in PATH. I believe that this works now since you said that that worked with 1.16.

@23Skidoo
Copy link
Member

@23Skidoo 23Skidoo commented Apr 16, 2015

Related: @edsko's work on independent goals (#2531, #2530, #2515, #2514, etc.) and this comment by @kosmikus.

@edsko
Copy link
Contributor

@edsko edsko commented Apr 16, 2015

With my work on fine grained dependencies in the solver, the solver part of this should be relatively easy. The harder part would be to change the way the installer works.

@edsko
Copy link
Contributor

@edsko edsko commented Apr 16, 2015

Incidentally, the testsuite use case mentioned by @tibbe can be addressed in a simpler, if not quite as general way: we can treat the dependencies of the test-suite which it doesn't share with the library as independent. This would make it possible to have, say, a tasty testsuite for optparse-applicative even though tasty depends on optparse-applicative.

Without changing anything else, this would mean we can then link tasty to an another (probably older) version of optparse-applicative than the one we are testing. (It would require the optparse-applicative test suite to use the src/ dir of its main library directly, rather than have an internal dependency on the optparse-applicative library).

This is something I've previously discussed with @dcoutts and it should be quite easy to do know do (I'm not sure I'll have time left to do it though).

@BardurArantsson
Copy link
Collaborator

@BardurArantsson BardurArantsson commented Jun 25, 2015

@edsko Please do what it is you do. From my experience the major irritation here is the test-suite case.

@enolan
Copy link
Contributor

@enolan enolan commented Sep 28, 2015

Can anyone give me some guidance on where to start? What modules need to be changed? Cabal has a lot of them and does not come with a map.

enolan added a commit to enolan/cabal that referenced this issue Oct 23, 2015
Added a test for the problem where a library package has a testsuite
that depends on a packge that depends on it. We should be able to handle
this by having separate build plans for the library and its testsuite.

The ticket for separate build plans is haskell#1575
@ezyang
Copy link
Contributor

@ezyang ezyang commented Jan 13, 2016

See also #2725.

@phadej
Copy link
Collaborator

@phadej phadej commented Jun 26, 2017

BTW already in #3824 was mentioned that it's a blocker for 2.0;

yet if 3.0 if the milestone where new-build is the only thing, that's a hard deadline.

Today while going through some packages, few other occurrences:

  • unordered-containers: cannot build benchmarks (criterion in the loop)
  • aeson has a workaround: benchmarks pulled into separate package
  • integer-logarithms cannot build test-suite (quickcheck-instances -> scientific)
  • Also scientific has benchmark suite

This is a real blocker to develop core packages using new-build.


To return comment to the starting post:

Suppose for sake of argument we're looking at bytestring which has a test suite that uses tasty, and tasty depends indirectly on bytestring. So the original proposal was to resolve this cycle by building the under-development bytestring and then building tasty against that, and then building the bytestring testsuite against the freshly built tasty package. But actually this is crazy, it means every time I make a small edit to bytestring and want to re-run the tests, cabal has to go and rebuild tasty

I'd prefer to rebuild the tasty each time. Because currently in some packages' Travis scripts we do

cabal install  --disable-tests --disable-benchmarks
cabal install test-framework criterion ...
cabal configure --enable-tests --enable-benchmarks
cabal test

E.g. hashable: https://github.com/tibbe/hashable/blob/339ae0ad3250c4d6b703e2fc781ce81a190c8d23/.travis.yml#L121

That workaround is impossible with new-build.

Fancier solution using multiple package versions would be cool, but this issue have been open for almost 4 years. I don't need the perfect solution, I'm quite ok with some solution.

@grayjay
Copy link
Collaborator

@grayjay grayjay commented Jun 27, 2017

I'd prefer to rebuild the tasty each time.

That makes sense. The only downside I can think of is that projects might start relying on cyclic dependencies between packages, and then it might be harder to switch to the more limited, two-version solution later.

@phadej
Copy link
Collaborator

@phadej phadej commented Jun 27, 2017

@grayjay I'm not sure I personally even want two version solution. All the cases mentioned in your #1575 (comment) and my #1575 (comment) are "core" packages, which are (possibly transitive) dependencies of criterion, tasty or test-framework.

It would be completely OK for me to explicitly toggle the switch to allow such "loop" (no name idea atm, but some flag to new-build)

@chris-martin
Copy link

@chris-martin chris-martin commented Oct 19, 2017

Sorry I'm about to ask some dumb question here, but I've been reading a lot of discussion on this issue dating back many years, and -- I'm very confused about the state of this bug, so I'm just hoping to understand where we are with it, to figure out whether there's hope of ever testing my projects with Cabal.

  • Do I understand correctly that if my project has three units foo:test -> bar:lib -> foo:lib (with dependency relationships indicated by the arrows), Cabal will always fail to build this project?
  • Is this working in any existing Cabal releases?
  • Is there any plan to make it work in any future Cabal releases?
  • Stack can build a project like this - I understand that Stack uses Cabal under the hood, so how is that possible?
@grayjay
Copy link
Collaborator

@grayjay grayjay commented Oct 22, 2017

Do I understand correctly that if my project has three units foo:test -> bar:lib -> foo:lib (with
dependency relationships indicated by the arrows), Cabal will always fail to build this project?

Is this working in any existing Cabal releases?

cabal can't build the tests in one step, but it is possible to install foo, install bar, and then build foo with tests enabled. cabal new-build can't build the tests, though, because it can't install packages in one step and then use the installed packages as inputs when calculating the next install plan.

Is there any plan to make it work in any future Cabal releases?

Yes, though there are two possible solutions. #4087 would allow install plans to have cycles between packages, but no cycles between components. #3422 would allow install plans to use two versions of the library to break a cycle when building test suites and benchmarks.

Stack can build a project like this - I understand that Stack uses Cabal under the hood, so how is that possible?

I think that stack builds the packages in multiple steps.

@grayjay
Copy link
Collaborator

@grayjay grayjay commented Jun 11, 2018

See #5200 for an example of a project where cabal would need to use consistent versions for the library used by the test suite helper and the library under test.

@grayjay
Copy link
Collaborator

@grayjay grayjay commented Oct 29, 2018

See #5645 for more discussion of this issue.

quasicomputational added a commit to quasicomputational/cabal that referenced this issue Nov 12, 2018
This is with the intention of the new package,
cabal-quickcheck-instances, being the blessed location for these
orphans, as QuickCheck acquiring a Cabal dependency or vice-versa
would be unsuitable.

This reduces some duplication (some presumably deliberate, and some
apparently accidental) and then some drift between the versions of
these instances.

Due to haskell#1575, some tests have had to move from the Cabal package to
cabal-install.
quasicomputational added a commit to quasicomputational/cabal that referenced this issue Nov 16, 2018
This is with the intention of the new package,
cabal-quickcheck-instances, being the blessed location for these
orphans, as QuickCheck acquiring a Cabal dependency or vice-versa
would be unsuitable.

This reduces some duplication (some presumably deliberate, and some
apparently accidental) and then some drift between the versions of
these instances.

Due to haskell#1575, the modules for the new package are shared with Cabal's
test-suite. This is less than ideal, but it's a workable hack.
quasicomputational added a commit to quasicomputational/cabal that referenced this issue Nov 17, 2018
This is with the intention of the new package,
cabal-quickcheck-instances, being the blessed location for these
orphans, as QuickCheck acquiring a Cabal dependency or vice-versa
would be unsuitable.

This reduces some duplication (some presumably deliberate, and some
apparently accidental) and then some drift between the versions of
these instances.

Due to haskell#1575, the modules for the new package are shared with Cabal's
test-suite. This is less than ideal, but it's a workable hack.
@parsonsmatt
Copy link

@parsonsmatt parsonsmatt commented Apr 6, 2019

This issue is blocking the use of cabal new-* on the persistent repository.

@hvr
Copy link
Member

@hvr hvr commented Apr 7, 2019

This issue is blocking the use of cabal new-* on the persistent repository.

Not for me and I wasn't able to confirm your claim. I performed the following steps:

# clone repo
cabal get -s persistent && persistent
# translate `stack.yaml` into `cabal.project`
cat > cabal.project <<EOF
tests: true
packages:
    ./persistent
    ./persistent-template
    ./persistent-sqlite
    ./persistent-test
    ./persistent-mongoDB
    ./persistent-mysql
    ./persistent-postgresql
    ./persistent-redis
    ./persistent-qq
EOF

# try to run the testsuites
cabal test all

# ERROR: When using configuration(s) from cabal.project, the following errors occurred: The package directory './persistent-qq' does not contain any .cabal file

# curse about unnecessarily cabal-hostile use of trivial package.yaml  in the repo; get `hpack` executable from somewhere, and then finally use `hpack` to convert into proper .cabal file:
hpack persistent-qq/package.yaml

# try to build & run the testsuite a 2nd time
$ cabal v2-test all 
Build profile: -w ghc-8.4.4 -O1
In order, the following will be built (use -v for more details):
 - persistent-2.9.2 (lib) (first run)
 - persistent-2.9.2 (test:test) (first run)
 - persistent-template-2.6.0 (lib) (first run)
 - persistent-sqlite-2.9.3 (lib) (first run)
 - persistent-redis-2.5.2.2 (lib) (first run)
 - persistent-template-2.6.0 (test:test) (first run)
 - persistent-sqlite-2.9.3 (test:test) (first run)
 - persistent-redis-2.5.2.2 (test:basic) (first run)
Configuring library for persistent-2.9.2..
Configuring test suite 'test' for persistent-2.9.2..
Preprocessing library for persistent-2.9.2..
Building library for persistent-2.9.2..
...
Preprocessing test suite 'test' for persistent-2.9.2..
Building test suite 'test' for persistent-2.9.2..
Running 1 test suites...
Test suite test: RUNNING...
Test suite test: PASS
Test suite logged to:
/tmp/persistent/dist-newstyle/build/x86_64-linux/ghc-8.4.4/persistent-2.9.2/t/test/test/persistent-2.9.2-test.log
1 of 1 test suites (1 of 1 test cases) passed.
Preprocessing test suite 'basic' for persistent-redis-2.5.2.2..
Building test suite 'basic' for persistent-redis-2.5.2.2..
Running 1 test suites...
Test suite basic: RUNNING...
"Inserting..."
basic: Network.Socket.connect: <socket: 3>: does not exist (Connection refused)
Test suite basic: FAIL
Test suite logged to:
/tmp/persistent/dist-newstyle/build/x86_64-linux/ghc-8.4.4/persistent-redis-2.5.2.2/t/basic/test/persistent-redis-2.5.2.2-basic.log
0 of 1 test suites (0 of 1 test cases) passed.
cabal: Tests failed for test:basic from persistent-redis-2.5.2.2.

So for me the lack of a cabal.project-equivalent to the stack.yaml file combined with the gratuitous use of hpack in one of the 9 local packages was blocking the use of cabal new-build with persistent's repo to be working out of the box; but other than the use of non-standard file formats I didn't notice any problems.

@parsonsmatt
Copy link

@parsonsmatt parsonsmatt commented Apr 8, 2019

@hvr The issue is occurring in this branch which introduces a pseudocircular dependency. The failure occurred after writing an appropriate cabal.project file and using hpack to generate the cabal file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.