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

openblas build broken on mac when not using binarybuilder #42519

Closed
ViralBShah opened this issue Oct 6, 2021 · 9 comments · Fixed by #42538
Closed

openblas build broken on mac when not using binarybuilder #42519

ViralBShah opened this issue Oct 6, 2021 · 9 comments · Fixed by #42538
Labels
building Build system, or building Julia or its dependencies system:mac Affects only macOS

Comments

@ViralBShah
Copy link
Member

➜  julia git:(master) ✗ make USE_BINARYBUILDER_OPENBLAS=0 VERBOSE=1
make[1]: *** No rule to make target `scratch/objconv/build-compiled', needed by `scratch/openblas-d909f9f3d4fc4ccff36d69f178558df154ba1002/build-compiled'.  Stop.
make: *** [julia-deps] Error 2
@ViralBShah ViralBShah added building Build system, or building Julia or its dependencies system:mac Affects only macOS labels Oct 6, 2021
@ViralBShah
Copy link
Member Author

I have to do

make USE_BINARYBUILDER_OPENBLAS=0 USE_BINARYBUILDER_OBJCONV=0 VERBOSE=1

It seems that openblas on darwin is unable to use the BB provided objconv (presumably some makefile stuff needs fixing).

@ViralBShah
Copy link
Member Author

ViralBShah commented Oct 6, 2021

Also seems like ZLIB is a problem (USE_BINARYBUILDER_ZLIB=0 doesn't help either):

Undefined symbols for architecture x86_64:
  "_compress2", referenced from:
      llvm::zlib::compress(llvm::StringRef, llvm::SmallVectorImpl<char>&, int) in libLLVMSupport.a(Compression.cpp.o)
  "_compressBound", referenced from:
      llvm::zlib::compress(llvm::StringRef, llvm::SmallVectorImpl<char>&, int) in libLLVMSupport.a(Compression.cpp.o)
  "_crc32", referenced from:
      llvm::zlib::crc32(llvm::StringRef) in libLLVMSupport.a(Compression.cpp.o)
      llvm::crc32(unsigned int, llvm::ArrayRef<unsigned char>) in libLLVMSupport.a(CRC.cpp.o)
      llvm::crc32(llvm::ArrayRef<unsigned char>) in libLLVMSupport.a(CRC.cpp.o)
      llvm::JamCRC::update(llvm::ArrayRef<unsigned char>) in libLLVMSupport.a(CRC.cpp.o)
     (maybe you meant: _ijl_crc32c_sw, _jl_crc32c )
  "_uncompress", referenced from:
      llvm::zlib::uncompress(llvm::StringRef, char*, unsigned long&) in libLLVMSupport.a(Compression.cpp.o)
      llvm::zlib::uncompress(llvm::StringRef, llvm::SmallVectorImpl<char>&, unsigned long) in libLLVMSupport.a(Compression.cpp.o)
     (maybe you meant: _ijl_uncompress_argname_n, _ijl_uncompress_ir , _ijl_uncompress_argnames )
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[1]: *** [/Users/viral/julia/usr/lib/libjulia-internal.1.8.dylib] Error 1
make: *** [julia-src-release] Error 2

@vchuravy
Copy link
Member

vchuravy commented Oct 6, 2021

Ideally we need to have CI for the non-BB builds as well. Otherwise this will keep happening.

@DilumAluthge
Copy link
Member

If we run non-BB builds on CI for every PR, that'll eat up a lot of CI time.

Maybe we just have a scheduled CI job that runs once per day and does a non-BB build on each platform? That would use up fewer resources, but we'd still detect the failures within 24 hours of the bad commit being merged to master.

cc: @staticfloat

The risk with this approach (as @JeffBezanson and I were discussing on Slack) is that in theory, a PR can be passing all PR CI (because that PR builds just fine with the PR CI), and then you merge the PR (because CI is green), and then the next day, the scheduled CI job fails because the non-BB build fails.

So I don't know what the right answer is here.

@ViralBShah
Copy link
Member Author

ViralBShah commented Oct 6, 2021

IMO, the right answer here is to fix these as we find them. Once in 24 hour CI is reasonable, if it is possible to set up something like that.

@ViralBShah
Copy link
Member Author

Note that the objconv issue is mac only.

staticfloat added a commit that referenced this issue Oct 7, 2021
We didn't properly move the OpenBLAS -> Objconv dependency chain
over to manifest files.  This was not visible until now because we
rarely build OpenBLAS from source without also building Objconv from
source.

Fixes #42519
@staticfloat
Copy link
Member

I think adding a 24-hour scheduled build with USE_BINARYBUILDER=0 is a good idea, but note that it still wouldn't have found this issue, since this is only triggered if USE_BINARYBUILDER_OPENBLAS=0 USE_BINARYBUILDER_OBJCONV=1 was true. There's just too much surface area in our build system for proper CI of this stuff, we just have to focus on keeping the happy path alive, and fix other issues as we go.

@vchuravy
Copy link
Member

vchuravy commented Oct 8, 2021

Yeah having USE_BINARYBUILDER=0 is sufficient for a smoke tests, but as you said we are facing an infinite build variation.

@DilumAluthge
Copy link
Member

DilumAluthge commented Oct 11, 2021

I think adding a 24-hour scheduled build with USE_BINARYBUILDER=0 is a good idea

Implemented in #42592, for the purpose of being a very basic smoke test.

ViralBShah pushed a commit that referenced this issue Oct 16, 2021
* Fix USE_BINARYBUILDER_OPENBLAS=0 on macOS

We didn't properly move the OpenBLAS -> Objconv dependency chain
over to manifest files.  This was not visible until now because we
rarely build OpenBLAS from source without also building Objconv from
source.

Fixes #42519

* Also update path to `objconv` binary
LilithHafner pushed a commit to LilithHafner/julia that referenced this issue Feb 22, 2022
* Fix USE_BINARYBUILDER_OPENBLAS=0 on macOS

We didn't properly move the OpenBLAS -> Objconv dependency chain
over to manifest files.  This was not visible until now because we
rarely build OpenBLAS from source without also building Objconv from
source.

Fixes JuliaLang#42519

* Also update path to `objconv` binary
LilithHafner pushed a commit to LilithHafner/julia that referenced this issue Mar 8, 2022
* Fix USE_BINARYBUILDER_OPENBLAS=0 on macOS

We didn't properly move the OpenBLAS -> Objconv dependency chain
over to manifest files.  This was not visible until now because we
rarely build OpenBLAS from source without also building Objconv from
source.

Fixes JuliaLang#42519

* Also update path to `objconv` binary
KristofferC pushed a commit that referenced this issue May 23, 2022
* Fix USE_BINARYBUILDER_OPENBLAS=0 on macOS

We didn't properly move the OpenBLAS -> Objconv dependency chain
over to manifest files.  This was not visible until now because we
rarely build OpenBLAS from source without also building Objconv from
source.

Fixes #42519

* Also update path to `objconv` binary

(cherry picked from commit 6af7584)
KristofferC pushed a commit that referenced this issue May 23, 2022
* Fix USE_BINARYBUILDER_OPENBLAS=0 on macOS

We didn't properly move the OpenBLAS -> Objconv dependency chain
over to manifest files.  This was not visible until now because we
rarely build OpenBLAS from source without also building Objconv from
source.

Fixes #42519

* Also update path to `objconv` binary

(cherry picked from commit 6af7584)
KristofferC pushed a commit that referenced this issue Jul 4, 2022
* Fix USE_BINARYBUILDER_OPENBLAS=0 on macOS

We didn't properly move the OpenBLAS -> Objconv dependency chain
over to manifest files.  This was not visible until now because we
rarely build OpenBLAS from source without also building Objconv from
source.

Fixes #42519

* Also update path to `objconv` binary

(cherry picked from commit 6af7584)
KristofferC pushed a commit that referenced this issue Dec 21, 2022
* Fix USE_BINARYBUILDER_OPENBLAS=0 on macOS

We didn't properly move the OpenBLAS -> Objconv dependency chain
over to manifest files.  This was not visible until now because we
rarely build OpenBLAS from source without also building Objconv from
source.

Fixes #42519

* Also update path to `objconv` binary

(cherry picked from commit 6af7584)
staticfloat added a commit that referenced this issue Dec 23, 2022
* Fix USE_BINARYBUILDER_OPENBLAS=0 on macOS

We didn't properly move the OpenBLAS -> Objconv dependency chain
over to manifest files.  This was not visible until now because we
rarely build OpenBLAS from source without also building Objconv from
source.

Fixes #42519

* Also update path to `objconv` binary

(cherry picked from commit 6af7584)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
building Build system, or building Julia or its dependencies system:mac Affects only macOS
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants