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

Specify a julia binary instead of a julia prefix #3243

Merged

Conversation

sebasguts
Copy link
Member

and let julia collect the information about all other paths

@fingolfin
Copy link
Member

This breaks all existing users of --with-julia, and also will complicate bisecting. At the very least, it'd be nice if we supported both giving a prefix or an executable path.

Ah, this remonds me: I think gc=julia should imply --with-julia. Not a request for this PR, it just should be done eventually

@codecov
Copy link

codecov bot commented Jan 28, 2019

Codecov Report

Merging #3243 into master will decrease coverage by 7.93%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3243      +/-   ##
==========================================
- Coverage   85.09%   77.16%   -7.94%     
==========================================
  Files         695      688       -7     
  Lines      344336   338935    -5401     
==========================================
- Hits       293024   261524   -31500     
- Misses      51312    77411   +26099
Impacted Files Coverage Δ
src/modules.h 16.66% <0%> (-83.34%) ⬇️
lib/ctbllatt.gi 4.94% <0%> (-79.62%) ⬇️
lib/ctblauto.gi 5.98% <0%> (-78.04%) ⬇️
lib/algliess.gi 5.95% <0%> (-72%) ⬇️
lib/proto.gi 13.4% <0%> (-71.14%) ⬇️
lib/ctblpope.gi 8% <0%> (-69.47%) ⬇️
lib/contfrac.gi 31.42% <0%> (-67.15%) ⬇️
src/saveload.c 3.26% <0%> (-64.58%) ⬇️
lib/lierep.gi 23.58% <0%> (-62.55%) ⬇️
lib/zlattice.gi 20.97% <0%> (-60.53%) ⬇️
... and 287 more

@coveralls
Copy link

coveralls commented Jan 28, 2019

Coverage Status

Coverage decreased (-0.0004%) to 84.934% when pulling 29e0c60 on sebasguts:sg/use_julia_binary_for_config into c381e0c on gap-system:master.

@sebasguts
Copy link
Member Author

@fingolfin I thought about making this backwards-compatible, but I decided against because I wanted to set a default.

--with-gc=julia kind of implies --with-julia= to be set, because the configure will fail. We could have a better error message, though. Or did you mean --with-julia should imply --with-gc=julia?

@sebasguts sebasguts force-pushed the sg/use_julia_binary_for_config branch from c8a4088 to fae156e Compare January 29, 2019 08:59
and let julia collect the information about all other paths
@fingolfin fingolfin added the topic: julia Julia GC integration and related matters label Jan 29, 2019
@fingolfin fingolfin merged commit d5e7625 into gap-system:master Jan 29, 2019
@fingolfin
Copy link
Member

Backported to stable-4.10 in 3c74918

[],[with_julia=no]
)
[AS_HELP_STRING([--with-julia@<:@=PATH@:>@],
[specify a prefix for a julia binary or a julia binary])],
Copy link
Member

Choose a reason for hiding this comment

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

@sebasguts do you really mean [specify a prefix for a julia binary or a julia binary]?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I mean it that way.

You can either directly specify a julia binary, or its prefix, e.g., /usr/local

Copy link
Member

Choose a reason for hiding this comment

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

Ah, now I got it. Parsing plain text without parentheses or commas failed. Maybe

specify a prefix for a julia binary, or a julia binary

or

specify a julia binary or a prefix for a julia binary 

would be easier to read.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I understand the problem now, sorry. I will provide a PR for that.

@olexandr-konovalov olexandr-konovalov added this to the GAP 4.10.1 milestone Feb 23, 2019
@olexandr-konovalov olexandr-konovalov added the release notes: added PRs introducing changes that have since been mentioned in the release notes label Feb 23, 2019
@olexandr-konovalov
Copy link
Member

See also #3316 for an updated comment.

@fingolfin fingolfin removed this from the GAP 4.10.1 milestone Jun 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-4.10-DONE release notes: added PRs introducing changes that have since been mentioned in the release notes topic: build system topic: julia Julia GC integration and related matters
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants