Skip to content

configure: only req CMake if we're building LLVM #38128

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

Merged
merged 2 commits into from
Dec 6, 2016

Conversation

cardoe
Copy link
Contributor

@cardoe cardoe commented Dec 2, 2016

CMake is only necessary if LLVM is going to be built and not in any
other case.

Signed-off-by: Doug Goldstein cardoe@cardoe.com

@rust-highfive
Copy link
Contributor

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@@ -848,7 +848,10 @@ then
fi

# For building LLVM
probe_need CFG_CMAKE cmake
if [ -z $CFG_LLVM_ROOT ]
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be quoted? if [ -z "$CFG_LLVM_ROOT"]?

@alexcrichton
Copy link
Member

Looks good to me, but I agree that we probably need quotes (or at least I think we do...)

CMake is only necessary if LLVM is going to be built and not in any
other case.

Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
@cardoe cardoe force-pushed the req-cmake-only-for-llvm branch from c7711fc to e39c8d6 Compare December 2, 2016 17:23
@cardoe
Copy link
Contributor Author

cardoe commented Dec 2, 2016

Fixed. I agree it should be quoted. I was actually copying the behavior in other places in the script.

    elif [ -z $CFG_LLVM_ROOT ]

I'll add another commit that quotes that.

These should probably be quoted.

Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Collaborator

bors commented Dec 2, 2016

📌 Commit f83eb40 has been approved by alexcrichton

@bors
Copy link
Collaborator

bors commented Dec 6, 2016

⌛ Testing commit f83eb40 with merge f7c93c0...

bors added a commit that referenced this pull request Dec 6, 2016
configure: only req CMake if we're building LLVM

CMake is only necessary if LLVM is going to be built and not in any
other case.

Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
@bors bors merged commit f83eb40 into rust-lang:master Dec 6, 2016
@cardoe cardoe deleted the req-cmake-only-for-llvm branch December 12, 2016 14:52
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.

5 participants