-
Notifications
You must be signed in to change notification settings - Fork 605
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
Add support for automatic configuration of llvm-config under MacPorts. #3297
Add support for automatic configuration of llvm-config under MacPorts. #3297
Conversation
New method adds the ability to check for MacPorts separately from Homebrew.
- MacPorts adds versioned binaries into the path, rather than a single binary named llvm-config. - This method queries MacPorts for the path to the latest installed version of llvm-config.
If MacPorts is installed, query MacPorts for the correct llvm-config* binary to use. Otherwise, use the existing brew lookup.
- Replace comments with descriptive temporary variables. - Limit selectable LLVM ports to those within the currently-supported range of llvm-3.0 to llvm-3.5.
|
||
# Returns true if MacPorts is installed in its default location. | ||
def macports? | ||
File.exists? '/opt/local/bin/port' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not test for which port
in case it's installed in a non-default location?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jc00ke The reason is that MacPorts is intended to be installed under /opt/local, while it's possible that there could be some other command named "port" in the PATH. Looking for the canonical location of the MacPorts command seems safe enough; simply assuming that a successful which port
means MacPorts is actually installed might lead to odd errors later. Checking the full canonical path seems like the optimal way to verify that it's really the MacPorts port command, short of trying to pull version strings or the like for comparison.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While the canonical location might be /opt/local/bin/port
we have no control over it, and we shouldn't enforce the user to always place MacPorts in a fixed place. One option would be to default to the output of which port
but allow users to specify a custom path (using something like --macports=...
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, while the argument of duplicate port
commands in PATH makes sense, this wouldn't work anyway due to the shell commands using a non-absolute path for this command.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yorickpeterse That's fine; I'll switch it to just checking the PATH for the port command. The current code for Homebrew doesn't even do that; it just calls brew and hopes for the best.
I'll post a patch shortly to make this change.
…or_macports Add support for automatic configuration of llvm-config under MacPorts.
- Use *which* to search the PATH for the *port* command, rather than checking the canonical installation location. - See <rubinius#3297> for a discussion of the pros and cons of this approach.
What does this pull request do?
Configure#macports?
.Configure#macports_llvm_config
.CC=clang
must still be passed explicitly.How were the changes tested?
Tested build of configuration on Yosemite with
CC=clang ./configure
:Tested new Rubinius build with with test suite using
bundle exec rake
: