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

pugz 0.8.0 #644

Closed
wants to merge 1 commit into from
Closed

pugz 0.8.0 #644

wants to merge 1 commit into from

Conversation

tseemann
Copy link
Contributor

  • Have you followed the guidelines for contributing?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source FORMULA, where FORMULA is the name of the formula you're submitting?
  • Does your build pass brew audit --strict FORMULA (after doing brew install FORMULA)?

@tseemann
Copy link
Contributor Author

/usr/bin/ld: /tmp/ccNbQPgH.ltrans0.ltrans.o: undefined reference to symbol 'pthread_create@@GLIBC_2.2.5'
/lib/x86_64-linux-gnu/libpthread.so.0: error adding symbols: DSO missing from command line
collect2: error: ld returned 1 exit status
Makefile:192: recipe for target 'gunzip' failed
make: *** [gunzip] Error 1

I assume this in OpenMP related; will depend on gcc and see.

@tseemann
Copy link
Contributor Author

tseemann commented May 24, 2019

Now with make V=1:
I notice the -lrt in the final link?

ar cr libdeflate.a 
CC="g++-5" CFLAGS=" -std=c++14 -I. -Icommon -lpthread -Iexternal/type_safe/include -Iexternal/type_safe/external/debug_assert -Wall -Wundef -Wrestrict -Wnull-dereference -Wuseless-cast -Wshadow -Weffc++ -Wpedantic -Wvla -O4 -flto -march=native -mtune=native -g -D_POSIX_C_SOURCE=200809L -D_FILE_OFFSET_BITS=64 -DHAVE_CONFIG_H" programs/detect.sh > programs/config.h
g++-5 -o programs/gunzip.o -c  -std=c++14 -I. -Icommon -lpthread -Iexternal/type_safe/include -Iexternal/type_safe/external/debug_assert -Wall -Wundef -Wrestrict -Wnull-dereference -Wuseless-cast -Wshadow -Weffc++ -Wpedantic -Wvla -O4 -flto -march=native -mtune=native -g -D_POSIX_C_SOURCE=200809L -D_FILE_OFFSET_BITS=64 -DHAVE_CONFIG_H programs/gunzip.cpp
g++-5 -o programs/prog_util.o -c  -std=c++14 -I. -Icommon -lpthread -Iexternal/type_safe/include -Iexternal/type_safe/external/debug_assert -Wall -Wundef -Wrestrict -Wnull-dereference -Wuseless-cast -Wshadow -Weffc++ -Wpedantic -Wvla -O4 -flto -march=native -mtune=native -g -D_POSIX_C_SOURCE=200809L -D_FILE_OFFSET_BITS=64 -DHAVE_CONFIG_H programs/prog_util.cpp
g++-5 -o programs/tgetopt.o -c  -std=c++14 -I. -Icommon -lpthread -Iexternal/type_safe/include -Iexternal/type_safe/external/debug_assert -Wall -Wundef -Wrestrict -Wnull-dereference -Wuseless-cast -Wshadow -Weffc++ -Wpedantic -Wvla -O4 -flto -march=native -mtune=native -g -D_POSIX_C_SOURCE=200809L -D_FILE_OFFSET_BITS=64 -DHAVE_CONFIG_H programs/tgetopt.cpp
g++-5 -o gunzip   -std=c++14 -I. -Icommon -lpthread -Iexternal/type_safe/include -Iexternal/type_safe/external/debug_assert -Wall -Wundef -Wrestrict -Wnull-dereference -Wuseless-cast -Wshadow -Weffc++ -Wpedantic -Wvla -O4 -flto -march=native -mtune=native -g -D_POSIX_C_SOURCE=200809L -D_FILE_OFFSET_BITS=64 -DHAVE_CONFIG_H programs/gunzip.o programs/prog_util.o programs/tgetopt.o libdeflate.a -lrt
/usr/bin/ld: /tmp/cclubzas.ltrans0.ltrans.o: undefined reference to symbol 'pthread_create@@GLIBC_2.2.5'
/lib/x86_64-linux-gnu/libpthread.so.0: error adding symbols: DSO missing from command line
collect2: error: ld returned 1 exit status

@sjackman
Copy link
Member

g++-5 -o gunzip   -std=c++14 -I. -Icommon -lpthread -Iexternal/type_safe/include -Iexternal/type_safe/external/debug_assert -Wall -Wundef -Wrestrict -Wnull-dereference -Wuseless-cast -Wshadow -Weffc++ -Wpedantic -Wvla -O4 -flto -march=native -mtune=native -g -D_POSIX_C_SOURCE=200809L -D_FILE_OFFSET_BITS=64 -DHAVE_CONFIG_H programs/gunzip.o programs/prog_util.o programs/tgetopt.o libdeflate.a -lrt

The -lpthread is in the wrong location on this command line. All the -l options need to occur after the object files programs/gunzip.o programs/prog_util.o programs/tgetopt.o where the -lrt is currently found.

@sjackman
Copy link
Member

sjackman commented May 24, 2019

A hack fix for now would be either

inreplace "Makefile", "-lpthread", "-pthread"

or

inreplace "Makefile", "-lrt", "-lpthread -lrt"

@sjackman
Copy link
Member

It'd be good to report this bug upstream. It should be easily reproducible on any Ubuntu system.

@tseemann
Copy link
Contributor Author

@tseemann reported upstream, added hack w/ note - thank you!

@tseemann tseemann requested a review from sjackman May 26, 2019 23:28
@tseemann
Copy link
Contributor Author

Fails on macos now:

==> make V=1
ar cr libdeflate.a 
CC="g++-9" CFLAGS=" -std=c++14 -I. -Icommon -lpthread -Iexternal/type_safe/include -Iexternal/type_safe/external/debug_assert -Wall -Wundef -Wrestrict -Wnull-dereference -Wuseless-cast -Wshadow -Weffc++ -Wpedantic -Wvla -O4 -flto -march=native -mtune=native -g -D_POSIX_C_SOURCE=200809L -D_FILE_OFFSET_BITS=64 -DHAVE_CONFIG_H" programs/detect.sh > programs/config.h
ar: no archive members specified
usage:  ar -d [-TLsv] archive file ...
	ar -m [-TLsv] archive file ...
	ar -m [-abiTLsv] position archive file ...
	ar -p [-TLsv] archive [file ...]
	ar -q [-cTLsv] archive file ...
	ar -r [-cuTLsv] archive file ...
	ar -r [-abciuTLsv] position archive file ...
	ar -t [-TLsv] archive [file ...]
	ar -x [-ouTLsv] archive [file ...]
make: *** [libdeflate.a] Error 1
make: *** Waiting for unfinished jobs....

@Piezoid
Copy link

Piezoid commented May 27, 2019

The (empty) static lib is now disabled on master.
Link argument order should also be fixed.

Imho, it is a bit early for a distributed release. But at least this will allow to sort out bugs on macos.

I didn't try at all to build on macos, nor do I have the hardware to do so.
The linux specific syscalls (non posix madvises) should be disabled by the preprocessor.

Edit: I think I got this pass in CircleCI

diff --git a/Formula/pugz.rb b/Formula/pugz.rb
index 2b4d60e..94c54c3 100644
--- a/Formula/pugz.rb
+++ b/Formula/pugz.rb
@@ -1,19 +1,12 @@
 class Pugz < Formula
   desc "Truly parallel gzip decompression"
   homepage "https://github.com/Piezoid/pugz"
-  url "https://github.com/Piezoid/pugz/archive/recombseq19-demo.tar.gz"
+  url "https://github.com/Piezoid/pugz/archive/97be6839c48878b498112933655506800af3b8e2.tar.gz"
   version "0.8.0"
-  sha256 "1005519d7ea80c4f59593b0e9bd2e9337c02a2b5c52dfe1732c16f75e4eeecb3"
-
-  depends_on "gcc" if OS.mac? # for openmp
-
-  fails_with :clang # needs openmp
+  sha256 "e0ec4763a9aa64d7ebf333ec333ba72ebfc5797c051a56fd9dd1ce6fca040dc8"
 
   def install
-    # https://github.com/Piezoid/pugz/issues/10
-    inreplace "Makefile", "-lrt", "-lpthread -lrt"
-
-    system "make", "V=1"
+    system "make", "asserts=0", "V=1"
     bin.install "gunzip" => "pugz"
     pkgshare.install "example"
     doc.install "paper/paper.pdf"

Not sure why you put a requirement on gcc for openmp, pugz uses plain pthread and compile fine with clang on linux.
Tell me if I should make a 0.8.1 release.

@sjackman
Copy link
Member

Thanks for your help, Maël!

Imho, it is a bit early for a distributed release. But at least this will allow to sort out bugs on macos.

In your opinion, should we close this PR, and wait for a stable release?

@tseemann
Copy link
Contributor Author

I would just add caveats section about stability; but i will defer to you

@sjackman
Copy link
Member

Not sure why you put a requirement on gcc for openmp, pugz uses plain pthread and compile fine with clang on linux.

@tseemann Could you please remove the gcc dependency and fails_with :clang?

@stale
Copy link

stale bot commented Jun 19, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale label Jun 19, 2019
@stale stale bot closed this Jun 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

3 participants