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

Automatically disable USE_BINARYBUILDER_xxx when USE_SYSTEM_xxx is set #31725

Merged
merged 2 commits into from
May 23, 2019

Conversation

staticfloat
Copy link
Member

Also renames USE_BINARYBUILDER_UNWIND to USE_BINARYBUILDER_LIBUNWIND
for consistency, as well as providing a little bit of dependency logic
to auto-disable USE_BINARYBUILDER_SUITESPARSE if USE_SYSTEM_BLAS is
set, for example.

Make.inc Outdated Show resolved Hide resolved
@fredrikekre fredrikekre added the building Build system, or building Julia or its dependencies label Apr 15, 2019
@staticfloat staticfloat force-pushed the sf/auto_disable_bb_for_system_usage branch from b3940ce to 03a28d6 Compare April 15, 2019 16:54
@staticfloat staticfloat requested a review from vtjnash April 15, 2019 16:56
@staticfloat
Copy link
Member Author

Requesting review from @vtjnash to ensure that my ?= initializations are equivalent to his previous fix regarding respecting user settings.

@staticfloat staticfloat force-pushed the sf/auto_disable_bb_for_system_usage branch from 03a28d6 to 7376d10 Compare April 15, 2019 17:21
@staticfloat
Copy link
Member Author

This is failing Appveyor because we have an incoherent set of makevars set:

USE_SYSTEM_SUITESPARSE = 1
USE_SYSTEM_ARPACK = 1
USE_SYSTEM_BLAS = 1
USE_SYSTEM_LAPACK = 1
USE_SYSTEM_GMP = 1
USE_SYSTEM_MPFR = 1
USE_SYSTEM_PCRE = 1
USE_SYSTEM_LIBUNWIND = 1
override LIBLAPACK = $(LIBBLAS)
override LIBLAPACKNAME = $(LIBBLASNAME)
override DEP_LIBS = libuv utf8proc
USE_BINARYBUILDER_LLVM = 1
USE_BINARYBUILDER_OPENBLAS = 1
USE_BINARYBUILDER_SUITESPARSE = 1

We can't have USE_SYSTEM_xxx and USE_BINARYBUILDER_xxx set at the same time. Which should it be?

@vchuravy
Copy link
Member

If I recall the appveyor script correctly it should be USE_BINARYBUILDER, USE_SYSTEM is a lie since we are getting them from elsewhere (and sometimes that is the lastest nightly)

@staticfloat
Copy link
Member Author

USE_SYSTEM is a lie since we are getting them from elsewhere (and sometimes that is the latest nightly)

With the new BB system, I don't think this is necessary anymore. We should be able to build from scratch; we don't need to bootstrap off of the previous build's libraries anymore.

@wavexx
Copy link
Contributor

wavexx commented Apr 15, 2019

Would it be possible to set USE_BINARYBUILDER:=0 in Make.user to avoid all pre-compiled binaries without having to set them manually? This seems to be set very late in Make.inc, so there's no way to override that.

When debugging I want to some compiler flags to be set through-out and be sure that either the library comes from the system (which means I also rebuilt them but in a shared location) or is truly rebuild from source.

@staticfloat
Copy link
Member Author

Would it be possible to set USE_BINARYBUILDER:=0 in Make.user to avoid all pre-compiled binaries without having to set them manually?

Yes, absolutely. That's the purpose of Make.user, and the purpose of USE_BINARYBUILDER. Just put the following in your Make.user:

override USE_BINARYBUILDER=0

The override will short-circuit our calculations to try and auto-determine whether USE_BINARYBUILDER should be set on or off.

@wavexx
Copy link
Contributor

wavexx commented Apr 15, 2019 via email

@staticfloat
Copy link
Member Author

Here's an example of how to do it, and how to check that it is working. First, let's check that the buildsystem wants to use BinaryBuilder for LLVM:

$ make print-USE_BINARYBUILDER_LLVM
USE_BINARYBUILDER_LLVM=1

Now, let's tell the buildsystem to not use BinaryBuilder; we'll flip the global switch, then check to see what the build system wants to do with LLVM:

$ echo "override USE_BINARYBUILDER=0" >> Make.user
$ make print-USE_BINARYBUILDER_LLVM
USE_BINARYBUILDER_LLVM=0

This was verified with the latest master, which is c9786e6. If it's not working for you, please open a new issue about USE_BINARYBUILDER=0 not being honored on your system, and we'll try to figure out what's not working for you.

@wavexx
Copy link
Contributor

wavexx commented Apr 15, 2019 via email

@staticfloat
Copy link
Member Author

I guess it's not possible to move the check earlier in Make.inc, before the inclusion of Make.user?

The timing of Make.user is intentional. The reason it's not working without override is because because I neglected to use ?= for the initial definition of USE_BINARYBUILDER, which I have now included in this PR. Often we don't want to pick up values from environment variables (which ?= will do) but in this case USE_BINARYBUILDER is unique enough that I don't think we need to worry about that (whereas we would need to worry about it for things like CC, CFLAGS, etc..., and so the user must use things like override for those in their Make.user)

@staticfloat
Copy link
Member Author

staticfloat commented Apr 16, 2019

Does appveyor not use my .appveyor.yml from my branch, instead using the one from master? EDIT: Nevermind, the appveyor page was cached weirdly; I see the new build now.

@@ -85,37 +85,6 @@ case $(uname) in
;;
esac

# Download most recent Julia binary for dependencies
Copy link
Member

Choose a reason for hiding this comment

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

😂

@KristofferC KristofferC mentioned this pull request Apr 20, 2019
58 tasks
@JeffBezanson
Copy link
Member

Bump. What's the status of this? Ready to merge?

@staticfloat staticfloat force-pushed the sf/auto_disable_bb_for_system_usage branch from b29b5cf to f1a99f3 Compare May 18, 2019 20:04
…s set

Also renames `USE_BINARYBUILDER_UNWIND` to `USE_BINARYBUILDER_LIBUNWIND`
for consistency, as well as providing a little bit of dependency logic
to auto-disable `USE_BINARYBUILDER_SUITESPARSE` if `USE_SYSTEM_BLAS` is
set, for example.
@staticfloat staticfloat force-pushed the sf/auto_disable_bb_for_system_usage branch from f1a99f3 to 5bd7636 Compare May 18, 2019 20:05
@staticfloat
Copy link
Member Author

Needed a rebase and a trim; assuming this passes CI, it's good to merge.

@@ -1,6 +1,6 @@
## UNWIND ##

ifneq ($(USE_BINARYBUILDER_UNWIND),1)
ifneq ($(USE_BINARYBUILDER_LIBUNWIND),1)
Copy link
Member

Choose a reason for hiding this comment

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

This is set in .travis.yml; needs to be renamed there.

@JeffBezanson JeffBezanson merged commit 5f808c6 into master May 23, 2019
@JeffBezanson JeffBezanson deleted the sf/auto_disable_bb_for_system_usage branch May 23, 2019 18:40
KristofferC pushed a commit that referenced this pull request May 23, 2019
…s set (#31725)

Also renames `USE_BINARYBUILDER_UNWIND` to `USE_BINARYBUILDER_LIBUNWIND`
for consistency, as well as providing a little bit of dependency logic
to auto-disable `USE_BINARYBUILDER_SUITESPARSE` if `USE_SYSTEM_BLAS` is
set, for example.

(cherry picked from commit 5f808c6)
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants