Makefile not portable about sha1collisiondetection

[cr-marcstevens]

All linux and macosx builds on travisci succeed, can I close this issue?

[muellermartin]

I think I could have fixed this by changing the order of those suffix rules. See commit 0b97c4e and related PR #4.

[cr-marcstevens]

@zbeekman Can you verify this fixes this issue?

[johnhawkinson]

The Makefile still seems a bit dodgy. Let me know if I should open a new Issue, but notice these results if I ask Homebrew to do make install into a clean PREFIX after I have previously run make:

==> make HAVEAVX=0 PREFIX=/usr/local/Cellar/sha1dc/HEAD-6b1c12f install
glibtool --tag=CC --mode=link clang -O2 -g -Wall -Werror -Wextra -pedantic -std=c99 -Ilib -DHAVE_MMX -DHAVE_SSE -DNO_HAVE_AVX -DNO_HAVE_NEON -march=native obj_src/main.lo obj_lib/sha1.lo obj_lib/ubc_check.lo obj_lib/ubc_check_verify.lo obj_lib/sha1_simd_mmx64.lo obj_lib/ubc_check_simd_mmx64.lo obj_lib/sha1_simd_sse128.lo obj_lib/ubc_check_simd_sse128.lo -Lbin -ldetectcoll -o bin/sha1dcsum
glibtool: link: clang -O2 -g -Wall -Werror -Wextra -pedantic -std=c99 -Ilib -DHAVE_MMX -DHAVE_SSE -DNO_HAVE_AVX -DNO_HAVE_NEON -march=native obj_src/.libs/main.o obj_lib/.libs/sha1.o obj_lib/.libs/ubc_check.o obj_lib/.libs/ubc_check_verify.o obj_lib/.libs/sha1_simd_mmx64.o obj_lib/.libs/ubc_check_simd_mmx64.o obj_lib/.libs/sha1_simd_sse128.o obj_lib/.libs/ubc_check_simd_sse128.o -o bin/sha1dcsum  -Lbin /private/tmp/sha1dc-20170301-89152-1ixmp42/bin/.libs/libdetectcoll.a 
ln -s sha1dcsum bin/sha1dcsum_partialcoll
ln: bin/sha1dcsum_partialcoll: File exists
make: [sha1dcsum_partialcoll] Error 1 (ignored)
bin/sha1dcsum test/*
a56374e1cf4c3746499bc7c0acb39498ad2ee185  test/sha1_reducedsha_coll.bin
16e96b70000dd1e7c85b8368ee197754400e58ec *coll* test/shattered-1.pdf
e1761773e6a35916d99f891b77663e6405313587 *coll* test/shattered-2.pdf
bin/sha1dcsum_partialcoll test/*
dd39885a2a5d8f59030b451e00cb45da9f9d3828 *coll* test/sha1_reducedsha_coll.bin
d3a1d09969c3b57113fd17b23e01dd3de74a99bb *coll* test/shattered-1.pdf
92246b0b718f4c704d37bb025717cbc66babf102 *coll* test/shattered-2.pdf
install bin/sha1dcsum /usr/local/Cellar/sha1dc/HEAD-6b1c12f/bin
install bin/sha1dcsum_partialcoll /usr/local/Cellar/sha1dc/HEAD-6b1c12f/bin
install bin/libdetectcoll.la /usr/local/Cellar/sha1dc/HEAD-6b1c12f/lib

pb3:Formula jhawk$ ls -ld /usr/local/Cellar/sha1dc/HEAD-6b1c12f/bin
-rwxr-xr-x  1 jhawk  admin  1344340 Mar  1 21:44 /usr/local/Cellar/sha1dc/HEAD-6b1c12f/bin
pb3:Formula jhawk$ ls -ld /usr/local/Cellar/sha1dc/HEAD-6b1c12f/lib
drwxr-xr-x  3 jhawk  admin  102 Mar  1 21:44 /usr/local/Cellar/sha1dc/HEAD-6b1c12f/lib
pb3:Formula jhawk$ ls -l /usr/local/Cellar/sha1dc/HEAD-6b1c12f/lib

Note that:

• Despite make having been run, make install rebuilds sha1dcsum and libdetectcoll.a
• Nothing mkdir'd $prefix/bin, so instead of installing the binary inside it, the install rule installed the binary as a file named bin.
• Installation copied in a .la libtool file. I think, as I suggested in #4 (comment), you need to use libtool to install the library, ala: $(LIBTOOL) --tag=CC --mode=install install -c bin/libdetectcoll.la $(PREFIX)/lib

I don't think any of these issues are MacOS -specific.

[johnhawkinson]

Also, sha1.h is not installed. There is also something wrong with the libtool installation of the library that I don't understand, cf. http://lists.gnu.org/archive/html/libtool/2017-03/msg00000.html

[cr-marcstevens]

Can you look at the simplified_c90 branch, I think it fixes a lot and will go to master soon.
It doesn't yet install the headers, we should still add that.

[johnhawkinson]

Yes, I've been following it. BTW, it would be great to get a tag, on either branch.
i.e. poke re #11 :)

At present, the simplified_c90 branch:

• No longer needs HAVEAVX=0 under MacOS. Also builds much faster (?)
• Correctly resolves the first bullet above (rebuilding at install)
• Still tries to install sha1dcsum as a file named bin
• With that corrected, installs sha1dcsum but not sha1dcsum_partialcoll
• Seems to resolve the 3rd bullet (although I'm not sure it's right).
• Builds sha1dcsum linked against a shared libdetectcoll under MacOS, which is not what I would have done, but might be a totally valid choice.

==> mkdir -p /usr/local/Cellar/sha1dc/git-58cac9/bin /usr/local/Cellar/sha1dc/git-58cac9/lib
==> make PREFIX=/usr/local/Cellar/sha1dc/git-58cac9 install
glibtool --tag=CC --mode=install install bin/libdetectcoll.la /usr/local/Cellar/sha1dc/git-58cac9/lib
glibtool: install: install bin/.libs/libdetectcoll.0.dylib /usr/local/Cellar/sha1dc/git-58cac9/lib/libdetectcoll.0.dylib
glibtool: install: (cd /usr/local/Cellar/sha1dc/git-58cac9/lib && { ln -s -f libdetectcoll.0.dylib libdetectcoll.dylib || { rm -f libdetectcoll.dylib && ln -s libdetectcoll.0.dylib libdetectcoll.dylib; }; })
glibtool: install: install bin/.libs/libdetectcoll.lai /usr/local/Cellar/sha1dc/git-58cac9/lib/libdetectcoll.la
glibtool: install: install bin/.libs/libdetectcoll.a /usr/local/Cellar/sha1dc/git-58cac9/lib/libdetectcoll.a
glibtool: install: chmod 644 /usr/local/Cellar/sha1dc/git-58cac9/lib/libdetectcoll.a
glibtool: install: ranlib /usr/local/Cellar/sha1dc/git-58cac9/lib/libdetectcoll.a
glibtool: warning: remember to run 'glibtool --finish /usr/local/lib'
glibtool --tag=CC --mode=install install bin/sha1dcsum /usr/local/Cellar/sha1dc/git-58cac9/bin
glibtool: warning: '/private/tmp/sha1dc-20170304-25067-1onj6r4/bin/libdetectcoll.la' has not been installed in '/usr/local/lib'
glibtool: install: install bin/.libs/sha1dcsum /usr/local/Cellar/sha1dc/git-58cac9/bin/sha1dcsum

[cr-marcstevens]

Done another commit to fix install dirs and added header installation.

AVX and others have disappeared, because it was still WIP and still needed a runtime selection.
I've moved this into the simd branch to work on this feature.
Indeed it builds much faster now, also because we only compile strictly necessary recompression functions.

[johnhawkinson]

Progress, but install -D is not portable and doesn't work with BSD installs (like MacOS):

glibtool: install: install -D bin/.libs/libsha1detectcoll.0.dylib /usr/local/Cellar/sha1dc/dev-v1.0.1/lib/libsha1detectcoll.0.dylib
install: illegal option -- D
usage: install [-bCcpSsv] [-B suffix] [-f flags] [-g group] [-m mode]
               [-o owner] file1 file2
       install [-bCcpSsv] [-B suffix] [-f flags] [-g group] [-m mode]
               [-o owner] file1 ... fileN directory
       install -d [-v] [-g group] [-m mode] [-o owner] directory ...

Trying to work around it with make INSTALL=ginstall to get GNU install fails because $INSTALL has two meanings, one for "install" and one for libtoolized install:

INSTALL ?= install
...
LT_INSTALL:=$(LIBTOOL) --tag=CC --mode=install $(INSTALL)
...
INSTALL=$(LT_INSTALL)
...
install: all
        $(INSTALL) -D bin/libsha1detectcoll.$(LIB_EXT) $(LIBDIR)/libsha1detectcoll.$(LIB_EXT)

So the result is installation of libtool wrapper scripts:

=> make LIBTOOL=glibtool INSTALL=ginstall PREFIX=/usr/local/Cellar/sha1dc/dev-v1.0.1 install
ginstall -D bin/libsha1detectcoll.la /usr/local/Cellar/sha1dc/dev-v1.0.1/lib/libsha1detectcoll.la
ginstall -D lib/sha1.h /usr/local/Cellar/sha1dc/dev-v1.0.1/include/sha1dc/sha1.h
ginstall -D bin/sha1dcsum /usr/local/Cellar/sha1dc/dev-v1.0.1/bin/sha1dcsum
ginstall -D bin/sha1dcsum_partialcoll /usr/local/Cellar/sha1dc/dev-v1.0.1/bin/sha1dcsum_partialcoll
...
pb3:~ jhawk$ head /usr/local/Cellar/sha1dc/dev-v1.0.1/bin/sha1dcsum
#! /bin/sh

# bin/sha1dcsum - temporary wrapper script for .libs/sha1dcsum
# Generated by libtool (GNU libtool) 2.4.6
#
# The bin/sha1dcsum program cannot be directly executed until all the libtool
# libraries that it depends on are installed.
#
# This wrapper script should never be moved out of the build directory.
# If it is, it will not operate correctly.

Working around that with a Makefile edit, things work! Although there are warnings on install that may or may not matter (I don't understand libtool well enough):

==> make LIBTOOL=glibtool PREFIX=/usr/local/Cellar/sha1dc/dev-v1.0.1 install
glibtool --tag=CC --mode=install ginstall -D bin/libsha1detectcoll.la /usr/local/Cellar/sha1dc/dev-v1.0.1/lib/libsha1detectcoll.la
glibtool: install: ginstall -D bin/.libs/libsha1detectcoll.0.dylib /usr/local/Cellar/sha1dc/dev-v1.0.1/lib/libsha1detectcoll.0.dylib
glibtool: install: (cd /usr/local/Cellar/sha1dc/dev-v1.0.1/lib && { ln -s -f libsha1detectcoll.0.dylib libsha1detectcoll.dylib || { rm -f libsha1detectcoll.dylib && ln -s libsha1detectcoll.0.dylib libsha1detectcoll.dylib; }; })
glibtool: install: ginstall -D bin/.libs/libsha1detectcoll.lai /usr/local/Cellar/sha1dc/dev-v1.0.1/lib/libsha1detectcoll.la
glibtool: install: ginstall -D bin/.libs/libsha1detectcoll.a /usr/local/Cellar/sha1dc/dev-v1.0.1/lib/libsha1detectcoll.a
glibtool: install: chmod 644 /usr/local/Cellar/sha1dc/dev-v1.0.1/lib/libsha1detectcoll.a
glibtool: install: ranlib /usr/local/Cellar/sha1dc/dev-v1.0.1/lib/libsha1detectcoll.a
glibtool: warning: remember to run 'glibtool --finish /usr/local/lib'
glibtool --tag=CC --mode=install ginstall -D lib/sha1.h /usr/local/Cellar/sha1dc/dev-v1.0.1/include/sha1dc/sha1.h
glibtool: install: ginstall -D lib/sha1.h /usr/local/Cellar/sha1dc/dev-v1.0.1/include/sha1dc/sha1.h
glibtool --tag=CC --mode=install ginstall -D bin/sha1dcsum /usr/local/Cellar/sha1dc/dev-v1.0.1/bin/sha1dcsum
glibtool: warning: '/private/tmp/sha1dc-20170304-38979-1he25ul/sha1collisiondetection-development-v1.0.1/bin/libsha1detectcoll.la' has not been installed in '/usr/local/lib'
glibtool: install: ginstall -D bin/.libs/sha1dcsum /usr/local/Cellar/sha1dc/dev-v1.0.1/bin/sha1dcsum
glibtool --tag=CC --mode=install ginstall -D bin/sha1dcsum_partialcoll /usr/local/Cellar/sha1dc/dev-v1.0.1/bin/sha1dcsum_partialcoll
glibtool: warning: '/private/tmp/sha1dc-20170304-38979-1he25ul/sha1collisiondetection-development-v1.0.1/bin/libsha1detectcoll.la' has not been installed in '/usr/local/lib'
glibtool: install: ginstall -D bin/.libs/sha1dcsum_partialcoll /usr/local/Cellar/sha1dc/dev-v1.0.1/bin/sha1dcsum_partialcoll

but the result certainly works and tests ok.

[cr-marcstevens]

Have you tried libtool-less installation?
LIBTOOL=libtool-disabled make install

What are the required edits you've made that also make sense for linux installation?
Use LT_INSTALL instead of INSTALL in install target?
Or should we use := instead of = at some places?

[johnhawkinson]

Have you tried libtool-less installation? LIBTOOL=libtool-disabled make install

I had not. That's a solution that works for a static builds, but it's not really the "right" solution, I don't think. But it does work.

What are the required edits you've made that also make sense for linux installation?
Use LT_INSTALL instead of INSTALL in install target?
Or should we use := instead of = at some places?

Merely that there be some way to override INSTALL (at least until you use a portable syntax for creating the $prefix directories, since install -D is not portable). Using LT_INSTALL would be fine, I think, but the real point is don't redefine INSTALL in two places to mean two different things, because it screws up command-line overrides.

[muellermartin]

For me branch simplified_c90 works.

[zbeekman]

Yes, this is fixed for me. Sorry for the slow reply.