commonsmachinery / blockhash

blockhash.io
MIT License
86 stars 28 forks source link

Build using CMake #21

Closed marlls1989 closed 7 years ago

marlls1989 commented 7 years ago

Instead of including a binary blob in the git tree for building the executable, cmake which is readily available in most platforms could be used instead

jonasob commented 7 years ago

Thank you for suggesting this. We do appreciate the pull request, and would love to see more, but I would be careful about this particular change. We've tended to use waf (which is written in Python) extensively through all our projects and making the switch to Cmake feels as something which should be done project-wide, rather than on individual repositories.

I'd be happy to hear from @artfwo as well though.

artfwo commented 7 years ago

Hey @marlls1989, thanks for the PR. I've noticed that your CMakeLists.txt doesn't include the check for ImageMagick 7, does that work when building against V7?

Regarding waf, well, waf is a compressed Python script, not an executable binary. I like waf because it's simple, fast and does its work. CMake, on the other hand, is a Makefile generator and thus requires both make and cmake to be installed in order to build anything. While those can indeed be installed on most systems, it seems an overkill for me switch over the entire project.

We've been trying to keep things simple and blockhash can actually be built without any of those tools by running gcc blockhash.c `pkg-config --cflags --libs MagickWand` -lm. Waf is there for a bit of cross-platform convenience and saving typing :) I don't think moving to cmake will make things any better for such a small project, so I'd rather not do that.

marlls1989 commented 7 years ago

Hi! Thanks for your time reviewing and replying my PR, I really appreciate your time and attention...

Regarding testing against ImageMagick 7, I haven't tested. This PR was just a quick hack done under 30 minutes on Fedora 26 and was built using ImageMagick 6.9.3. It is not "master branch" quality work yet.

It is just a hack I did to build the package as I was suspicious of running unknown code on my machine, sent my hack back to you as a pull request to make it more readily available to those that like me, were suspicious of running what appeared as a foreigner binary on their machines.

Again, thank you for your time replying my PR. I understand and respect your reasoning for choosing Waf.

Cheers

artfwo commented 7 years ago

Hey, @marlls1989! Thanks for understanding. Looking forward to seeing more from you in the future!