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

bisect is not supported for OCaml 4.06 #835

Closed
nekketsuuu opened this Issue Feb 16, 2018 · 9 comments

Comments

Projects
None yet
4 participants
@nekketsuuu
Copy link

nekketsuuu commented Feb 16, 2018

The opam file says that batteries needs bisect for testing. However, bisect is not supported for OCaml 4.06 (ocaml/opam-repository#10751).

In fact, opam install -t batteries failed under OCaml 4.06.0 whereas opam install batteries succeeded.

$ opam switch show
4.06.0
$ opam reinstall -t batteries
The following dependencies couldn't be met:
  - batteries -> bisect
Your request can't be satisfied:
  - bisect is not available because your system doesn't comply with ocaml-version < "4.06.0".

No solution found, exiting
@gasche

This comment has been minimized.

Copy link
Member

gasche commented Feb 16, 2018

Thanks for the report!

We should port bisect to 4.06, migrate to ppx_bisect, or disable coverage testing, whichever is easier.

@rixed

This comment has been minimized.

Copy link
Contributor

rixed commented Feb 16, 2018

@UnixJunkie

This comment has been minimized.

Copy link
Member

UnixJunkie commented Feb 17, 2018

I guess 'opam reinstall batteries' would work.
I guess the -t flag forces bisect to be required.

nekketsuuu added a commit to nekketsuuu/SATySFi that referenced this issue Feb 17, 2018

@UnixJunkie

This comment has been minimized.

Copy link
Member

UnixJunkie commented Feb 20, 2018

@gasche

This comment has been minimized.

Copy link
Member

gasche commented Feb 23, 2018

This was a two-lines change: ocaml/opam-repository#11472

@nekketsuuu

This comment has been minimized.

Copy link

nekketsuuu commented Feb 26, 2018

Thanks to gasche's PR, now opam reinstall -t batteries works on OCaml 4.06.0. Thank you all :)

@nekketsuuu nekketsuuu closed this Feb 26, 2018

@rixed

This comment has been minimized.

Copy link
Contributor

rixed commented Feb 26, 2018

Given that:

  1. the source code for bisect is not easy to patch (using darc on a private repo with no quick way to submit patches),
  2. xclerc does not seem to respond to github or even his own Mantis bugtracker fast enough for us (at least these days),
  3. I believe no one ever paid attention to any code coverage tool

I suggest this dependency be dropped and will be happy to provide a patch doing just that if everyone agrees.

@gasche

This comment has been minimized.

Copy link
Member

gasche commented Feb 26, 2018

Yes, I'm fine with this as well. I did the work of implementing bisect support in Batteries, but we never really started to use coverage information (I think the constraint that any changed or added code must come with tests is enough in practice), so dropping bisect support is fine. If anyone was amused to re-introduce it (it was not a lot of work in the first place), of course bisect_ppx would be a wiser choice today.

@UnixJunkie

This comment has been minimized.

Copy link
Member

UnixJunkie commented Feb 26, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment