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

julia 1.6.1 (new formula) #76527

Closed
wants to merge 1 commit into from
Closed

julia 1.6.1 (new formula) #76527

wants to merge 1 commit into from

Conversation

carlocab
Copy link
Member

@carlocab carlocab commented May 3, 2021

  • 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?
  • Is your test running fine brew test <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>)?

This fails the audit because of the curl and openlibm dependencies. Julia seems to need a libcurl and libopenlibm that live in the file system, but those don't exist on Big Sur.

Note that this will also vendor its own libLLVM and utf8proc. We can use brewed llvm and utf8proc, but this requires patches (already upstreamed). We can therefore do one of three things:

  1. Add the formula as-is, and then add dependencies on llvm and utf8proc when those patches land in a stable release. (Might be a while though.)
  2. Use upstreamed patches to allow julia to build with brewed llvm and utf8proc.
  3. Wait until the necessary patches are in a stable release and add the formula then.

Last proposed in Homebrew/legacy-homebrew#11482.

@BrewTestBot BrewTestBot added new formula PR adds a new formula to Homebrew/homebrew-core perl Perl use is a significant feature of the PR or issue labels May 3, 2021
@carlocab
Copy link
Member Author

carlocab commented May 3, 2021

That said, I'm not convinced that openlibm is provided by macOS. These are the contents of openlibm's lib directory:

Permissions Size User     Date Modified Name
.rw-r--r--  181k carlocab 27 Apr 14:38  libopenlibm.3.0.dylib
lrwxr-xr-x    21 carlocab 17 Feb 17:04  libopenlibm.3.dylib -> libopenlibm.3.0.dylib
.r--r--r--  390k carlocab 17 Feb 17:04  libopenlibm.a
lrwxr-xr-x    21 carlocab 17 Feb 17:04  libopenlibm.dylib -> libopenlibm.3.0.dylib
drwxr-xr-x     - carlocab 27 Apr 14:38  pkgconfig

If you try to dlopen a libopenlibm.dylib, you get nothing.

@Bo98
Copy link
Member

Bo98 commented May 3, 2021

Julia seems to need a libcurl and libopenlibm that live in the file system, but those don't exist on Big Sur.

What error do you get when you try use system libcurl?

but this requires patches (already upstreamed)

Can you link the patches?

@carlocab
Copy link
Member Author

carlocab commented May 3, 2021

What error do you get when you try use system libcurl?

System library symlink failure: Unable to locate libcurl.dylib on your system!
make[2]: *** [/private/tmp/julia-20210503-8184-m1lldo/julia-1.6.1/usr/lib/julia/libcurl.dylib] Error 1

https://github.com/carlocab/homebrew-personal/pull/78/checks?check_run_id=2493787747#step:8:574

It wants its library dependencies to live in lib/"julia". If you want it to use system versions of those libraries, then it will create symlinks to system libraries, but it has no symlink to create when you want it to use system libcurl.

Can you link the patches?

For utf8proc:

JuliaLang/julia@ba653ec

For llvm:

JuliaLang/julia@8675648

The llvm patch has not yet been merged, but I have an open PR for it: JuliaLang/julia#40680

@Bo98
Copy link
Member

Bo98 commented May 3, 2021

If you want it to use system versions of those libraries, then it will create symlinks to system libraries

Considering an unversioned libcurl.dylib does exist in the default dyld search path, I wonder what would happen if this symlink just wasn't created.

For llvm:

Ah yes, this all links back to the ABI versioning thing. llvm-config actually expects libLLVM-11.dylib to exist!

@carlocab
Copy link
Member Author

carlocab commented May 3, 2021

Considering an unversioned libcurl.dylib does exist in the default dyld search path, I wonder what would happen if this symlink just wasn't created.

Bad things, because it then points all library searches to lib/"julia" instead of relying on the dynamic linker to find them.

prefix=#{prefix}
USE_SYSTEM_CSL=1
USE_SYSTEM_PCRE=1
USE_SYSTEM_OPENLIBM=1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the benefit of using openlibm over the system libm (USE_SYSTEM_LIBM)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

openlibm is optimised for Julia (and vice-versa). openlibm is actually maintained by Julia maintainers. I don't have specific benchmarks, though.

Copy link
Member Author

@carlocab carlocab May 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It also looks like Julia relies on some symbols that aren't necessarily in a given libm implementation: JuliaLang/julia#26434 (comment)

It seems like it'll also download its own at some point if you don't give it an openlibm: JuliaLang/julia#26434 (comment)

@carlocab
Copy link
Member Author

carlocab commented May 3, 2021

Considering an unversioned libcurl.dylib does exist in the default dyld search path, I wonder what would happen if this symlink just wasn't created.

Bad things, because it then points all library searches to lib/"julia" instead of relying on the dynamic linker to find them.

That said, it seems to manage just fine with zlib. I could try to patch out the libcurl symlinking and see if it breaks anything.

@carlocab carlocab force-pushed the julia branch 4 times, most recently from 599a59d to 90d6639 Compare May 4, 2021 11:06
@carlocab
Copy link
Member Author

carlocab commented May 4, 2021

Ok, this should pass the audit now. Do we want this to use brewed llvm and utf8proc? The patches will make it fail the audit again though.

Or an alternative fix in Homebrew/core would be to create a libLLVM-12.dylib symlink to libLLVM.dylib in the llvm formula, which would allow julia to use brewed llvm without any patching.

@carlocab carlocab mentioned this pull request May 4, 2021
5 tasks
@carlocab
Copy link
Member Author

carlocab commented May 4, 2021

#76646 will also allow use of brewed llvm.

@carlocab carlocab force-pushed the julia branch 4 times, most recently from 8faebe9 to b110e72 Compare May 7, 2021 14:29
carlocab added a commit to carlocab/homebrew-core that referenced this pull request May 7, 2021
`llvm-config` relies on a versioned `libLLVM` (e.g. `libLLVM-12`), but
their build system does not install one for some reason.

With the symlink:

    ❯ llvm-config --libs
    -lLLVM-12

Without:

    ❯ llvm-config --libs
    -lLLVMWindowsManifest -lLLVMXRay -lLLVMLibDriver -lLLVMDlltoolDriver ... -lLLVMRemarks -lLLVMBitstreamReader -lLLVMBinaryFormat -lLLVMTableGen -lLLVMSupport -lLLVMDemangle

This deviates from the intended behaviour, as seen on Linux, where
`llvm-config --libs` returns the output shown above with the symlink.
Attempting to link against the libraries shown by `llvm-config` without
this fix is also leads to build failures due to missing symbols.

This will also allow Julia to use brewed `llvm` instead of a vendored
one. (Homebrew#76527)

Closes Homebrew#76798.
@carlocab carlocab force-pushed the julia branch 4 times, most recently from 4216201 to f27b396 Compare May 11, 2021 01:10
BrewTestBot pushed a commit that referenced this pull request May 14, 2021
`llvm-config` relies on a versioned `libLLVM` (e.g. `libLLVM-12`), but
their build system does not install one for some reason.

With the symlink:

    ❯ llvm-config --libs
    -lLLVM-12

Without:

    ❯ llvm-config --libs
    -lLLVMWindowsManifest -lLLVMXRay -lLLVMLibDriver -lLLVMDlltoolDriver ... -lLLVMRemarks -lLLVMBitstreamReader -lLLVMBinaryFormat -lLLVMTableGen -lLLVMSupport -lLLVMDemangle

This deviates from the intended behaviour, as seen on Linux, where
`llvm-config --libs` returns the output shown above with the symlink.
Attempting to link against the libraries shown by `llvm-config` without
this fix is also leads to build failures due to missing symbols.

This will also allow Julia to use brewed `llvm` instead of a vendored
one. (#76527)

Closes #76798.

Closes #76646.

Signed-off-by: Mike McQuaid <mike@mikemcquaid.com>
Signed-off-by: BrewTestBot <1589480+BrewTestBot@users.noreply.github.com>
@carlocab carlocab force-pushed the julia branch 6 times, most recently from 3c7c835 to f26d4b8 Compare May 15, 2021 02:40
@carlocab
Copy link
Member Author

Switched this back to brewed curl. Encountered some libcurl-related errors locally with system curl.

@BrewTestBot
Copy link
Member

:shipit: @carlocab has triggered a merge.

@carlocab carlocab deleted the julia branch May 15, 2021 08:46
@github-actions github-actions bot added the outdated PR was locked due to age label Jun 15, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
new formula PR adds a new formula to Homebrew/homebrew-core outdated PR was locked due to age perl Perl use is a significant feature of the PR or issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants