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

Use the FFTW binaries from fftw-builder #30

Merged
merged 6 commits into from
Aug 31, 2017
Merged

Use the FFTW binaries from fftw-builder #30

merged 6 commits into from
Aug 31, 2017

Conversation

ararslan
Copy link
Member

Let's see what happens...

@ararslan ararslan force-pushed the aa/new-deps branch 4 times, most recently from 6582425 to 7e93acd Compare August 16, 2017 02:54
@ararslan
Copy link
Member Author

At this point, this looks like a failure of the BinDeps setup rather than one of the binaries themselves. Any thoughts or suggestions, @tkelman and/or @staticfloat?

deps/build.jl Outdated
provides(Zypper, "libfftw3_threads3", [libfftw, libfftwf], os=:Linux)
provides(Yum, "fftw-libs", [libfftw, libfftwf], os=:Linux)
provides(BSDPkg, "fftw3", [libfftw, libfftwf], os=:FreeBSD)
url = "https://github.com/ararslan/fftw-builder/releases/download/v\$FFTW_VER/libfftw-\$FFTW_VER"
Copy link
Contributor

@musm musm Aug 16, 2017

Choose a reason for hiding this comment

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

I don't think you meant \$ -> $ ?

Copy link
Contributor

@musm musm Aug 16, 2017

Choose a reason for hiding this comment

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

nvm I just saw the huge include_string above.... 😃

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, this huge include_string() is nasty; we should instead just use include() and put the newer build instructions in a separate file

deps/build.jl Outdated
provides(Zypper, "libfftw3_threads3", [libfftw, libfftwf], os=:Linux)
provides(Yum, "fftw-libs", [libfftw, libfftwf], os=:Linux)
provides(BSDPkg, "fftw3", [libfftw, libfftwf], os=:FreeBSD)
url = "https://github.com/ararslan/fftw-builder/releases/download/v\$FFTW_VER/libfftw-\$FFTW_VER"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, this huge include_string() is nasty; we should instead just use include() and put the newer build instructions in a separate file

deps/build.jl Outdated
"67b8084cfed5f240aff70890369111f38fed624e8fbae58719c3d1ef58ca02ff"
end
provides(Binaries, URI("\$url-win-\$(Sys.ARCH).zip"), [libfftw, libfftwf],
unpacked_dir="usr/lib", SHA=sha, os=:Windows)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would suggest something a little more ordered, like what Tom did for Sundials.jl. Maybe with a check to see if there just aren't any platforms available at all.

@staticfloat
Copy link
Collaborator

Overall looks reasonable, and the errors you're seeing are, ironically, due to BinDeps.jl, not BinDeps2.jl. I don't know why it's crashing with an InterruptException. That's weird.

@tkelman
Copy link
Contributor

tkelman commented Aug 17, 2017

because it's spawning a process to check if sudo works and that spawning fails

@ararslan
Copy link
Member Author

not BinDeps2.jl

Dunno why they would be coming from BinDeps2 anyway, since that isn't being used here...


# Mapping of (Sys.KERNEL, Sys.ARCH) to (url, sha) for precompiled binaries from fftw-builder
const downloads = Dict(
(:Linux, :x86_64) => ("$URL-linux-x86_64.tar.gz",
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to check the full GNU triple of Sys.MACHINE, just kernel and arch aren't good enough. If someone is running Julia on a non-glibc Linux distro, these binaries won't work and this should use a source build.

Copy link
Member Author

Choose a reason for hiding this comment

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

Am I correct that the appropriate keys are then:

  • x86_64-pc-linux-gnu
  • i686-pc-linux-gnu
  • x86_64-w64-mingw32
  • i686-w64-mingw32
  • x86_64-apple-darwin (+ some numbers at the end)

Copy link
Member Author

Choose a reason for hiding this comment

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

The macOS binaries were built with XCode 8 and should work with other versions of macOS...

Copy link
Contributor

Choose a reason for hiding this comment

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

looks right - there will be more possible varieties that should map to "glibc version of linux that would work" but those are going to need to be collected in a single place

@ararslan
Copy link
Member Author

It keeps trying to build from scratch on Linux. It's like it's not even seeing the provides(Binaries, ...) bit. :( Even on Windows it says none of the declared providers can satisfy the library requirement.

@ararslan
Copy link
Member Author

I see the issue: The binaries built on Travis have a directory structure in the tarball, whereas the rest of them just have the binaries right inside. I just need to not do that and it should be good I think.

@ararslan
Copy link
Member Author

Well, that's an issue at least. Doesn't explain the Windows failure.

@ararslan
Copy link
Member Author

Okay. Summary of current bad things:

  • 64-bit Linux still ignores the binaries and builds from scratch
  • Windows says UndefVarError: libfftw not defined (maybe related to how I set libfftw_name?)
  • macOS says tar: could not chdir to '/Users/travis/.julia/v0.7/FFTW/deps/libfftw3_threads'

Any ideas, BinDeps masters?

@ararslan
Copy link
Member Author

Awesome, Windows works now! Linux x64 is still building from scratch (it isn't even hitting the info I put in there :/) though.

deps/build.jl Outdated
elseif Sys.KERNEL === :FreeBSD
provides(BSDPkg, "fftw3", [libfftw, libfftwf], os=:FreeBSD)
else
info("No precompiled binaries found for your system. Building from scratch...")
Copy link
Member Author

Choose a reason for hiding this comment

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

@staticfloat Any idea why Linux seems to be building from scratch without ever seeing this line?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Put info()'s in the other branches of the if, I bet it's passing the haskey() part and actually running the provides(Binaries, ....) on line 52.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also see if you can't put a BinDeps.debug("FFTW") into the travis script to see if it even sees the provides(Binaries, ...).

Copy link
Member Author

Choose a reason for hiding this comment

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

So apparently it's taking that path with the haskey since with the most recent commit it triggers the info, but it's still ignoring the provides(Binaries, ) and using the provides(Sources, ).

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea!

Copy link
Member Author

Choose a reason for hiding this comment

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

$ julia -e 'Pkg.add("BinDeps"); using BinDeps, FFTW; BinDeps.debug("FFTW")'
INFO: No packages to install, update or remove
INFO: Package database updated
INFO: Reading build script...
ERROR: UndefVarError: include not defined
Stacktrace:
 [1] debug_context(::String) at /home/travis/.julia/v0.7/BinDeps/src/debug.jl:54
 [2] debug(::Base.PipeEndpoint, ::String) at /home/travis/.julia/v0.7/BinDeps/src/debug.jl:59
 [3] debug(::String) at /home/travis/.julia/v0.7/BinDeps/src/debug.jl:71

wtf?

@staticfloat
Copy link
Collaborator

Okay, apparently BinDeps.debug("") doesn't like include(). That's interesting.

I guess next is to debug this locally, on a docker instance or something.

@ararslan ararslan merged commit 16c27b0 into master Aug 31, 2017
@ararslan ararslan deleted the aa/new-deps branch August 31, 2017 01:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants