Quote path variables that may contain spaces. #84
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Make sure that references to variables that may be paths (which in turn may contain spaces) are quoted, so cmake won't break on the spaces. This fixes #79.
|
👍 |
| @@ -151,7 +151,7 @@ ExternalProject_Add(lpeg | |||
| DEPENDS luajit | |||
| URL "http://s3.amazonaws.com/download.draios.com/dependencies/lpeg-1.0.0.tar.gz" | |||
| URL_MD5 "0aec64ccd13996202ad0c099e2877ece" | |||
| BUILD_COMMAND LUA_INCLUDE=${LUAJIT_INCLUDE} ${PROJECT_SOURCE_DIR}/scripts/build-lpeg.sh | |||
| BUILD_COMMAND LUA_INCLUDE=${LUAJIT_INCLUDE} "${PROJECT_SOURCE_DIR}/scripts/build-lpeg.sh" | |||
There was a problem hiding this comment.
How do you think about to enclose also this include parameter by double quotes?
There was a problem hiding this comment.
I could be wrong, but I though it wasn't necessary as LUAJIT_INCLUDE is defined in terms of LUAJIT_SRC, which is quoted:
set(LUAJIT_INCLUDE "${LUAJIT_SRC}")
There was a problem hiding this comment.
I suggest to apply quoting rules consistently. It does not matter that the previous assignment of the mentioned variable is fine while its use for an other parameter might be a bit unsafe.
|
Thanks for your improvement of build scripts. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Make sure that references to variables that may be paths (which in turn
may contain spaces) are quoted, so cmake won't break on the spaces.
This fixes #79.
@henridf