commonsmachinery / blockhash

blockhash.io
MIT License
86 stars 28 forks source link

Support for ImageMagick >= 7 #19

Closed rakkesh closed 7 years ago

rakkesh commented 8 years ago

Build fails when ImageMagick 7.0.2 is present during compilation . I could fix the failure by changing the path to MagickWand/MagickWand.h when the following error showed up -

../blockhash.c:14:10: fatal error: 'wand/MagickWand.h' file not found
#include <wand/MagickWand.h>

Complete build logs - https://gist.github.com/02477d5bcbbbf2b4697991d18cd48add

Could you please add support for ImageMagick 7.0.x?

ilovezfs commented 7 years ago

@jonasob @artfwo ping on this. The issue looks like it's only the location of the header, but otherwise it builds.

artfwo commented 7 years ago

Hi and thanks for the heads up! It looks like the majority of Linux distros are still relying on the version 6 of ImageMagick. We can probably support both versions 6 and 7 though. For the record, here's the version 7 porting guide: https://www.imagemagick.org/script/porting.php

ilovezfs commented 7 years ago

@artfwo here's what I did so that we could avoid "boneyarding" blockhash: https://github.com/Homebrew/homebrew-core/blob/master/Formula/blockhash.rb#L25-L28

artfwo commented 7 years ago

Yeah, that will work. But I think the issue is best solved by adding a configure option or auto-detecting version 7 during configure step and choosing the header via a compile-time flag.

ilovezfs commented 7 years ago

@artfwo right. That is just a stop-gap for brew, not an actual solution.

ilovezfs commented 7 years ago

@artfwo thanks! Could I sweet talk you into tagging a new release? :innocent:

artfwo commented 7 years ago

@ilovezfs already done :)

ilovezfs commented 7 years ago

Neat. Thanks. Running our test block for HEAD, I see

==> /usr/local/Cellar/blockhash/HEAD-cc2c8be_2/bin/blockhash /tmp/blockhash-test-20170113-83389-5ecol5/clipper_ship.jpg
Error: blockhash: failed
Expected /00007ffe7ffe7ffe7ffe7ffe7ffe77fe77fe600e7f5e00000000000000000000/ to match "00007ff07fe07fe07fe67ff07520600077fe601e7f5e000079fd40410001fffe  /tmp/blockhash-test-20170113-83389-5ecol5/clipper_ship.jpg\n".

Is that expected that the end would have changed like that since 0.1?

ilovezfs commented 7 years ago

The test has been

  test do
    resource("testdata").stage testpath
    hash = "00007ffe7ffe7ffe7ffe7ffe7ffe77fe77fe600e7f5e00000000000000000000"
    # Exit status is not meaningful, so use pipe_output instead of shell_output
    # for now
    # https://github.com/commonsmachinery/blockhash/pull/9
    result = pipe_output("#{bin}/blockhash #{testpath}/clipper_ship.jpg", nil, nil)
    assert_match hash, result
  end
artfwo commented 7 years ago

@ilovezfs the algorithm has been updated for improved accurary, so yeah, this is expected.

ilovezfs commented 7 years ago

Cool. I'll update the hash then!

ilovezfs commented 7 years ago

Voilà: https://github.com/Homebrew/homebrew-core/pull/8815.

artfwo commented 7 years ago

Awesome, thanks!