macOS build failure for v57.5 (Homebrew)

p-linnane commented 1 year ago

Hello 👋 . I'm a maintainer for the Homebrew project. While packaging v57.5 we are encountering build failures. The relevant GitHub Actions run can be found here: https://github.com/Homebrew/homebrew-core/actions/runs/4557146919/jobs/8038523624

==> make install -f NONWINDOWS/Makefile BINDIR=/opt/homebrew/Cellar/zpaqfranz/57.5/bin/zpaqfranz
  objc[41729]: Class AMSupportURLConnectionDelegate is implemented in both /usr/lib/libauthinstall.dylib (0x1fbffb480) and /System/Library/PrivateFrameworks/MobileDevice.framework/Versions/A/MobileDevice (0x110e902b8). One of the two will be used. Which one is undefined.
  objc[41729]: Class AMSupportURLSession is implemented in both /usr/lib/libauthinstall.dylib (0x1fbffb4d0) and /System/Library/PrivateFrameworks/MobileDevice.framework/Versions/A/MobileDevice (0x110e90308). One of the two will be used. Which one is undefined.
  clang  -DNOJIT -O3 -Dunix zpaqfranz.cpp -o zpaqfranz -pthread -lstdc++ -lm
  zpaqfranz.cpp:37744:128: error: a space is required between consecutive right angle brackets (use '> >')
          int64_t         franzparallelhashfiles(string i_hashtype,int64_t i_totalsize,vector<string> i_thefiles,vector<std::pair<string,string>>& o_hashname);
                                                                                                                                               ^~
                                                                                                                                               > >
  zpaqfranz.cpp:64457:34: error: a space is required between consecutive right angle brackets (use '> >')
          vector<std::pair<uint64_t,string>> bysize;
                                          ^~
                                          > >
  zpaqfranz.cpp:64493:37: error: a space is required between consecutive right angle brackets (use '> >')
                                  vector<std::pair<uint64_t,string>>::iterator basso      = std::lower_bound(bysize.begin(), bysize.end(), p->second.size,mycompareuint64string());
                                                                  ^~
                                                                  > >
  zpaqfranz.cpp:64502:38: error: a space is required between consecutive right angle brackets (use '> >')
                                          vector<std::pair<uint64_t,string>>::iterator alto       = std::upper_bound(bysize.begin(), bysize.end(), p->second.size,mycompareuint64string());
                                                                          ^~
                                                                          > >
  zpaqfranz.cpp:64503:43: error: a space is required between consecutive right angle brackets (use '> >')
                                          for (vector<std::pair<uint64_t,string>>::iterator a=basso; a!=alto; a++)
                                                                               ^~
                                                                               > >
  zpaqfranz.cpp:64529:32: error: a space is required between consecutive right angle brackets (use '> >')
          vector<std::pair<string,string>> tobehashedsource_hashname;
                                        ^~
                                        > >
  zpaqfranz.cpp:64544:32: error: a space is required between consecutive right angle brackets (use '> >')
          vector<std::pair<string,string>> tobehasheddest_hashname;
                                        ^~
                                        > >
  zpaqfranz.cpp:64575:33: error: a space is required between consecutive right angle brackets (use '> >')
                  vector<std::pair<string,string>>::iterator basso        = std::lower_bound(tobehasheddest_hashname.begin(), tobehasheddest_hashname.end(), cerco,mycomparestringstring());
                                                ^~
                                                > >
  zpaqfranz.cpp:64578:34: error: a space is required between consecutive right angle brackets (use '> >')
                          vector<std::pair<string,string>>::iterator alto                 = std::upper_bound(tobehasheddest_hashname.begin(), tobehasheddest_hashname.end(), cerco,mycomparestringstring());
                                                        ^~
                                                        > >
  zpaqfranz.cpp:64580:39: error: a space is required between consecutive right angle brackets (use '> >')
                          for (vector<std::pair<string,string>>::iterator a=basso; a<alto; a++)
                                                             ^~
                                                             > >
  zpaqfranz.cpp:64606:33: error: a space is required between consecutive right angle brackets (use '> >')
                  vector<std::pair<string,string>>::iterator basso        = std::lower_bound(tobehasheddest_hashname.begin(), tobehasheddest_hashname.end(), cerco,mycomparestringstring());
                                                ^~
                                                > >
  zpaqfranz.cpp:64609:34: error: a space is required between consecutive right angle brackets (use '> >')
                          vector<std::pair<string,string>>::iterator alto                 = std::upper_bound(tobehasheddest_hashname.begin(), tobehasheddest_hashname.end(), cerco,mycomparestringstring());
                                                        ^~
                                                        > >
  zpaqfranz.cpp:64613:39: error: a space is required between consecutive right angle brackets (use '> >')
                          for (vector<std::pair<string,string>>::iterator a=basso; a<alto; a++)
                                                             ^~
                                                             > >
  zpaqfranz.cpp:64712:133: error: a space is required between consecutive right angle brackets (use '> >')
  int64_t Jidac::franzparallelhashfiles(string i_hashtype,int64_t i_totalsize,vector<string> i_thefiles,vector<std::pair<string,string>>& o_hashname)
                                                                                                                                      ^~
                                                                                                                                      > >
  14 errors generated.
  make: *** [zpaqfranz] Error 1

Relates to https://github.com/Homebrew/homebrew-core/pull/126991

fcorbelli commented 1 year ago

Thank you seems a include issue

I'll check tomorrow

ZhongRuoyu commented 1 year ago

(Sorry for the noise above.)

This is actually because the C++ program is compiled with a C compiler. Double right angle brackets has been in the standards since C++11.

Adding a -std=c++11 flag should solve this, but I suggest that a C++ compiler be used instead. Please take a look at #51.

fcorbelli commented 1 year ago

Building on Mac should be (info in the source)

MacOS 11.0 gcc (clang) 12.0.5, INTEL
MacOS 12.6 gcc (clang) 13.1.6, INTEL
Please note:
The -std=c++11 is required, otherwise you have to change half a dozen lines (or -DANCIENT). 
No -static here
"Apple does not support statically linked binaries on Mac OS X. 
(...) Rather, we strive to ensure binary 
compatibility in each dynamically linked system library and framework
(AHAHAHAHAHAH, note by me)
Warning: Shipping a statically linked binary entails a significant compatibility risk. 
We strongly recommend that you not do this..."
Short version: Apple does not like -static, so compile with
g++ -Dunix  -O3 -march=native zpaqfranz.cpp -o zpaqfranz -pthread  -std=c++11

Mac PowerPC with gcc4.x
Look at -DBIG (for BIG ENDIAN) and -DANCIENT (old-compiler)
g++ -O3 -DBIG -DANCIENT -Dunix -DNOJIT zpaqfranz.cpp -o zpaqfranz -pthread

Apple Macintosh (M1/M2)
Untested (yet), should be
g++ -Dunix  -O3 -DNOJIT zpaqfranz.cpp -o zpaqfranz -pthread  -std=c++11

The source is designed to be compiled with old C++ compilers (I mean from over 20 years ago), while maintaining a decent level of performance There are often 'strange' things, such as the use of ordered maps instead of unordered maps, simply because the latter are not always available

Similarly, no make, as on some systems, there is simply no make

The proposed Makefile is a kind of "wildcard", i.e. the one that - based on empirical experience - compiles on the largest number of non-Windows systems

It is trivial to make a Makefile for a specific target, much less so for FreeBSD, OpenBSD, Solaris, OmniOS, Linux, Arch Linux (not the same as Linux), Mac, Sparc, Android, Haiku, Qnap, Synology, ESXi

In short, if something seems strange, there's usually a reason for it :)

I don't have any modern Macs (Mx), so I only test for PPC and Intel. If it compiles on FreeBSD I normally consider it OK on Macs too

Thank you for the change, I'll put in the greetings sections

Short version:

compiling on Intel Macs, -Dunix turn on optimizations,whatever, link pthread, enable c++11
compiling on M1/M2 whatever NOT Intel, -Dunix -DNOJIT (turn off the just-in-type compiler in x64 assembly)
compiling on PPC, -DNOJIT -DBIG -DANCIENT (no jit because non intel, BIG endian, ancient compiler)

On other "things" (BSD) the "pseudocode" is something like
"by default the JIT is turned off, UNLESS you are on Intel-amd64"

ZPAQFLAGS=      -Dunix -DNOJIT
(...)
.if (${ARCH} == "amd64")
ZPAQFLAGS=      -Dunix
.endif

If I can help, I am at your disposal

ZhongRuoyu commented 1 year ago

Thanks @fcorbelli for the explanation. I was aware of the build instructions in the README, but I did not have enough context to understand why. If the patch broke anything: please feel free to revert it.

fcorbelli commented 1 year ago

I am rewriting from scratch a couple of functions, to work even without -std=c++11 A couple of days and I'll check better Can I add you like that?

20 Thanks to https://github.com/ZhongRuoyu              for __linux__ instead of older #defines

ZhongRuoyu commented 1 year ago

A couple of days and I'll check better

Thanks!

Can I add you like that?

Yes, I'm happy with that. And thanks for doing so! :)

fcorbelli commented 1 year ago

The "real" thing I do not know for Homebrew is conditional compiling.
On "everything" different from amd64 CPU a -DNOJIT is mandatory (M1, M2, PPC...) Is it possible? I really know nothing of "brew"
If it is impossible then a -DNOJIT must be enforced, even on Intel chips
This will slow down (a lot), but better than nothing

"spiegone". zpaq runs compression and extraction in PAQL, a custom-made language running on a virtual machine embedded in the software (! yes !).
On amd64 (Intel) there is a JIT (!) that translate the code into Intel machine language (! yes !). It is the default.
On "non Intel" the JIT must be excluded, running on software emulation (! yes !), with -DNOJIT

If you think this is crazy, remember that dr. Mahoney, the "father" of zpaq, is one of the world's top compression expert (today retired)

In fact the JIT use some extension, therefore on 32 bit Intel the situation is even more complex (! yessssss !), but I think there aren't Mac 32 bit Intel (all amd64)

ZhongRuoyu commented 1 year ago

On "everything" different from amd64 CPU a -DNOJIT is mandatory (M1, M2, PPC...) Is it possible?

Thanks for the info @fcorbelli. Yes, it is possible, and actually, it's already there: https://github.com/Homebrew/homebrew-core/blob/6a10c64ef2b12349ad8dbdc7fae2118c03940636/Formula/zpaqfranz.rb#L33-L34

    # JIT only works on Intel (might work on Linux aarch64, but not Apple Silicon)
    ENV.append_to_cflags "-DNOJIT" unless Hardware::CPU.intel?

The code snippet above means that -DNOJIT is added to the compile flags on non-Intel (amd64) platforms.

but I think there aren't Mac 32 bit Intel

And 32-bit Intel is already out of the scope of Homebrew. Homebrew does not support 32-bit Intel at all. Neither do recent versions of macOS.

fcorbelli / zpaqfranz

macOS build failure for v57.5 (Homebrew) #50