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

RFC: Run the tests on julia-debug if JULIA_DEBUG==1 #11155

Closed
wants to merge 1 commit into from

Conversation

garrison
Copy link
Member

@garrison garrison commented May 6, 2015

This modifies the Makefile so that make test and its siblings will use a debug build of julia if JULIA_DEBUG is set to 1. Previously, make test would always make a release build of julia and then run the tests, even if make had just made a debug build of julia due to JULIA_DEBUG being set.

@timholy
Copy link
Member

timholy commented May 6, 2015

👍 for this idea, though I'm not enough of a Makefile expert to count as a reviewer. I had the impression that make testall was running a debug build; or is that only on Travis?

@garrison
Copy link
Member Author

garrison commented May 6, 2015

From the looks of things, make testall will never use a debug build. In fact, if you compile julia in debug mode, running make test[all] will build "release" julia and overwrite your symlink to point to julia instead of julia-debug.

I just created a new (simpler, and more correct) patch, so would appreciate it if somebody with actual Makefile knowledge could review.

@garrison garrison changed the title WIP: Run the tests on julia-debug if JULIA_DEBUG==1 RFC: Run the tests on julia-debug if JULIA_DEBUG==1 May 6, 2015
@pao
Copy link
Member

pao commented May 6, 2015

I'd opt for @vtjnash or @ViralBShah for build system.

@pao pao added building Build system, or building Julia or its dependencies test This change adds or pertains to unit tests labels May 6, 2015
@garrison
Copy link
Member Author

garrison commented May 6, 2015

@timholy Actually we're both right: make testall will never use the debug build, but travis.yml bypasses that and calls julia-debug explicitly.

@timholy
Copy link
Member

timholy commented May 6, 2015

Thanks for finding that. I knew I had seen something like that somewhere.

@vtjnash
Copy link
Member

vtjnash commented May 6, 2015

lgtm

@vtjnash
Copy link
Member

vtjnash commented May 6, 2015

however, if you really want to have some fun with makefiles, replace JULIA_DEBUG=1 with ifneq (,$(findstring release,$(MAKECMDGOALS))) (interpretation: if the targets given on the command line by the user contained the string release), and let us know if that can be made to work.

https://www.gnu.org/software/make/manual/html_node/Goals.html

@garrison
Copy link
Member Author

garrison commented May 7, 2015

@vtjnash I'm not quite sure if I'm up for that now, and also I do not quite understand the benefit (could you please elaborate?). If I understand correctly, with that a debug build would be built by default, unless somebody expicitly says "release". Do we want e.g. the perf tests to run on a debug build?

@vtjnash
Copy link
Member

vtjnash commented May 7, 2015

good catch, i wanted to express "debug & !release". so, maybe it should be:

JULIA_DEBUG = 0
ifeq (,$(findstring release,$(MAKECMDGOALS)))
ifneq (,$(findstring debug,$(MAKECMDGOALS)))
JULIA_DEBUG = 1
endif
endif

@vtjnash
Copy link
Member

vtjnash commented May 11, 2015

merged as a4d019e

i decided to go ahead and make the tweaks i was suggesting as a followup commit. now make debug test does the "right thing"

@tkelman
Copy link
Contributor

tkelman commented May 11, 2015

would've been ideal to do that in a pr first, given the appveyor breakage

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 test This change adds or pertains to unit tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants