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

OpenCV 4 only, GHC 9.2 support #157

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

nh2
Copy link
Collaborator

@nh2 nh2 commented Sep 6, 2023

In this PR I modernise opencv:

FYI @ocharles

TODO

  • Handle the fact that the aruco API broke between OpenCV 4.6 and 4.7, and the current Ubuntu 22.04 LTS has 4.5 while NixOS has 4.7.
  • Test with multiple versions of OpenCV 4.
    • Done, see stack.yaml's nix section.
  • Check that enable-nonfree compiles fine.
    • Since I can't test that on Ubuntu easily (see above), probably best to be tested enabled in an OpenCV environment from Nix.
      • Done, I added an entry about this in the CHANGELOG.
  • File an issue for Stackage that Stackage 21+ needs repa added back, so that opencv can be added back.
  • File separate issue about QuickCheck flake I observed:
    Repro with stack test opencv --ta --quickcheck-replay=369455:
    getAffineTransform:
      *** Failed! (after 2 tests):
      The points are too far apart; newPoints =
      V3 (V2 0.0 0.0) (V2 0.0 0.0) (V2 0.0 0.0)
      V3 (V2 1.0 1.0) (V2 1.0 1.0) (V2 0.0 (-1.0))
      V3 (V2 0.0 1.0) (V2 0.0 (-1.0)) (V2 (-1.0) 1.0)
      Use --quickcheck-replay=369455 to reproduce.
      Use -p '/getAffineTransform/' to rerun this test only.
    

@ocharles
Copy link
Collaborator

ocharles commented Sep 6, 2023

Thanks @nh2 ! We have a bunch of commits in our circuithub fork. I'm on holiday for the next two weeks, but I'll try and get on top of things when I'm back and we get this all ship shape again

Implementation notes beyond what this commit adds to the CHANGELOG:

* `drawAxis()` was replaced by `drawFrameAxes()`.
  The former is available only until OpenCV <= 4.5,
  while the latter is available for all 4.x.
  * opencv/opencv_contrib@56d492c
  * opencv/opencv_contrib@0f030dd
The main task here is to make `doc-images-opencv` work,
which is this package's way to generate nice documentation images
into the Haddocks, from code that's in the Haddocks.

For that, this commit:

* Deletes copy-pasted + modified module from `haskell-src-meta`,
  `Language.Haskell.Meta.Syntax.Translate`, since as per the commit mentioned in
  * bmillwood/haskell-src-meta#54
  * haskell-party/haskell-src-meta#3 (comment)
  * haskell-party/haskell-src-meta@5c69c3a
  the necessary `DataKinds` changes were merged upstream.
  The commit suggests it's in >= 0.8.3, so that's what's added in the `.cabal` file.

* Replaces all occurrences of `*` by `Type` from `Data.Kind` to fix
  all `-Wstar-is-type` warnings
  (see also https://ghc-proposals.readthedocs.io/en/latest/proposals/0143-remove-star-kind.html).
  This is necessary because the upstream `haskell-src-meta` above
  does not support `*` but does support `Type`; it would print:
      Language.Haskell.Meta.Syntax.Translate.toType: not implemented: TyStar ()
@nh2
Copy link
Collaborator Author

nh2 commented Sep 29, 2023

Another thing that could be done:

The SIFT patent expired in 2020 (https://opencv.org/blog/2020/07/18/opencv-4-4-0/) so we should be able to change our code here to move that out of the enable-nonfree Cabal flag I'm adding in this PR.

@nh2
Copy link
Collaborator Author

nh2 commented Oct 3, 2023

@ocharles Do you want to do a short call regarding this?

I'm also in London starting from tonight in case you want to hang out :)

@ocharles
Copy link
Collaborator

ocharles commented Oct 7, 2023

Hey @nh2, sorry for not getting back earlier! A call sounds good - can you do next week some time? And how long are you in London for?

@nh2
Copy link
Collaborator Author

nh2 commented Oct 7, 2023

@ocharles Afternoons from 16:00 work for me (except Monday), I'm in London at least the whole week, probably more.

else
pkgconfig-depends: opencv >= 3.0.0
build-depends: base64-bytestring >= 1.0.0.1
pkgconfig-depends: opencv4 >= 4.0.0
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, this doesn't work with cabal2nix:

> cabal2nix: post-process: cannot replace name binding Bind (Identifier "opencv") (Path [Identifier "pkgs",Identifier "opencv"]) by Bind (Identifier "opencv3") (Path [Identifier "pkgs",Identifier "opencv3"]) because it's not found in set fromList [Bind (Identifier "opencv4") (Path [Identifier "pkgs",Identifier "opencv4"])]
> CallStack (from HasCallStack):
>   error, called at src/Distribution/Nixpkgs/Haskell/FromCabal/PostProcess.hs:211:27 in cabal2nix-2.19.1-24MEcjNppQdH8xbM8xoUph:Distribution.Nixpkgs.Haskell.FromCabal.PostProcess

This is the problem:
https://github.com/NixOS/cabal2nix/blob/2099a1f4594f621bb1a2879b793b860aefe4c027/cabal2nix/src/Distribution/Nixpkgs/Haskell/FromCabal/PostProcess.hs#L359

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@htngr That is correct, that hardcode in cabal2nix will need to be fixed once this thing here is merged and released.

If you use this branch, you'll have to patch it, or write a manual cabal2nix output for now, like this:

# TODO: Delete this when https://github.com/LumiGuide/haskell-opencv/pull/157 is in nixpkgs
#       and once cabal2nix has removed its hardcode of `opencv3` in
#       https://github.com/NixOS/cabal2nix/blob/0365d9b77086d26ca5197fb48019cedbb0dce5d2/cabal2nix/src/Distribution/Nixpkgs/Haskell/FromCabal/PostProcess.hs#L359
{ mkDerivation, aeson, base, base64-bytestring, bindings-DSL
, bytestring, Cabal, containers, criterion, data-default, deepseq
, directory, Glob, haskell-src-exts, inline-c, inline-c-cpp
, JuicyPixels, lens, linear, opencv4, primitive, QuickCheck, repa
, tasty, tasty-hunit, tasty-quickcheck, template-haskell, text
, transformers, vector
, async
, haskell-src-meta
, fetchFromGitHub
, lib
}:
mkDerivation {
  pname = "opencv";
  version = "0.0.2.1";
  src = "${fetchFromGitHub {
    owner = "nh2";
    repo = "haskell-opencv";
    rev = "fe0b7c8813aa090835ee974d90c0436d8c95efff";
    sha256 = "0xsjvkpss2iilb8dnp2w229pwgskv5r6yqrbfdhc9avjj6x4qj6i";
  }}/opencv";
  setupHaskellDepends = [ base Cabal ];
  libraryHaskellDepends = [
    aeson base base64-bytestring bindings-DSL bytestring containers
    data-default deepseq inline-c inline-c-cpp JuicyPixels linear
    primitive repa template-haskell text transformers vector
    async
    haskell-src-meta
  ];
  libraryPkgconfigDepends = [ opencv4 ];
  testHaskellDepends = [
    base bytestring containers data-default directory Glob
    haskell-src-exts JuicyPixels lens linear primitive QuickCheck repa
    tasty tasty-hunit tasty-quickcheck template-haskell text
    transformers vector
  ];
  benchmarkHaskellDepends = [ base bytestring criterion repa ];
  hardeningDisable = [ "bindnow" ];
  description = "Haskell binding to OpenCV-3.x";
  license = lib.licenses.bsd3;
  hydraPlatforms = lib.platforms.none;
  # Just because when installing from git, there's no license file.
  preInstall = ''
    touch LICENSE
  '';
}

@ocharles
Copy link
Collaborator

Hey @nh2, sorry I never got back to you! Unfortunately time has been very short recently, and as things go that isn't looking to change any time soon 😅 I don't think I have time for a call, but I have viewed all your changes here and they look excellent. I'm very happy for you to just crack on with this and merge/release as necessary. If there's anything in particular you want to talk about, let me know. This looks like a sensible and incremental path forward though, and I can't see anything remotely controversial. Good work!

emilazy added a commit to emilazy/nixpkgs that referenced this pull request Aug 22, 2024
These packages have been broken since before the 24.05 release. There
is a pull request open to update them to a newer OpenCV and get them
working again: <LumiGuide/haskell-opencv#157>.
emilazy added a commit to emilazy/nixpkgs that referenced this pull request Aug 23, 2024
These packages have been broken since before the 24.05 release. There
is a pull request open to update them to a newer OpenCV and get them
working again: <LumiGuide/haskell-opencv#157>.

(cherry picked from commit 79872e0)
RCoeurjoly pushed a commit to RCoeurjoly/nixpkgs that referenced this pull request Aug 23, 2024
These packages have been broken since before the 24.05 release. There
is a pull request open to update them to a newer OpenCV and get them
working again: <LumiGuide/haskell-opencv#157>.

(cherry picked from commit 79872e0)
greg-hellings pushed a commit to greg-hellings/nixpkgs that referenced this pull request Aug 24, 2024
These packages have been broken since before the 24.05 release. There
is a pull request open to update them to a newer OpenCV and get them
working again: <LumiGuide/haskell-opencv#157>.

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

Successfully merging this pull request may close these issues.

3 participants