Skip to content

Copy README.md/amalgamate_demo.cpp only if output dir isn't the same as ${SCRIPTPATH} - #1113

Merged
lemire merged 1 commit into
simdjson:masterfrom
pps83:bugfix/Suppress-error-code-if-copying-of-README.md-amalgamate_demo.cpp-isnt-needed
Aug 18, 2020
Merged

Copy README.md/amalgamate_demo.cpp only if output dir isn't the same as ${SCRIPTPATH}#1113
lemire merged 1 commit into
simdjson:masterfrom
pps83:bugfix/Suppress-error-code-if-copying-of-README.md-amalgamate_demo.cpp-isnt-needed

Conversation

@pps83

@pps83 pps83 commented Aug 13, 2020

Copy link
Copy Markdown
Contributor

No description provided.

@lemire

lemire commented Aug 14, 2020

Copy link
Copy Markdown
Member

If we are concerned with the case where they are the same file... and that's the only scenario we care about, then we can check add a check.

What do you think of the following...

#1115

Is there any other instances where you'd like for us to ignore the error?

@lemire

lemire commented Aug 14, 2020

Copy link
Copy Markdown
Member

Note that FreeBSD remains broken with this PR.

@pps83
pps83 force-pushed the bugfix/Suppress-error-code-if-copying-of-README.md-amalgamate_demo.cpp-isnt-needed branch from 6a9e5de to 11cfd78 Compare August 15, 2020 13:30
@pps83 pps83 changed the title Suppress error code if copying of README.md/amalgamate_demo.cpp isn't needed Copy README.md/amalgamate_demo.cpp only if output dir isn't the same as ${SCRIPTPATH} Aug 15, 2020
@pps83

pps83 commented Aug 15, 2020

Copy link
Copy Markdown
Contributor Author

Note that FreeBSD remains broken with this PR.

PR updated

@lemire

lemire commented Aug 15, 2020

Copy link
Copy Markdown
Member

You do a string comparison when comparing the paths, but that is not safe. It can still be the same directory.

@pps83

pps83 commented Aug 15, 2020

Copy link
Copy Markdown
Contributor Author

You do a string comparison when comparing the paths, but that is not safe. It can still be the same directory.

If #1115 doesn't have the issue, then it's a better fix.

@jkeiser

jkeiser commented Aug 15, 2020

Copy link
Copy Markdown
Member

If neither of them ends up being acceptable, we have code in jsonexamples/CMakeLists.txt that uses cmake for this purpose :)

@pps83
pps83 force-pushed the bugfix/Suppress-error-code-if-copying-of-README.md-amalgamate_demo.cpp-isnt-needed branch from 11cfd78 to bcf7c9d Compare August 16, 2020 06:50
@pps83

pps83 commented Aug 16, 2020

Copy link
Copy Markdown
Contributor Author

PR updated to expand ${SCRIPTPATH} and ${AMALGAMATE_OUTPUT_PATH} to ensure that equality works even with relative paths

@pps83
pps83 force-pushed the bugfix/Suppress-error-code-if-copying-of-README.md-amalgamate_demo.cpp-isnt-needed branch from 940f52f to 72391c2 Compare August 16, 2020 09:33
…as ${SCRIPTPATH}

 + use test's `-ef` file operator to check if ${SCRIPTPATH} and ${AMALGAMATE_OUTPUT_PATH} are not the same
@pps83
pps83 force-pushed the bugfix/Suppress-error-code-if-copying-of-README.md-amalgamate_demo.cpp-isnt-needed branch from 72391c2 to bae36aa Compare August 16, 2020 09:36
@lemire

lemire commented Aug 18, 2020

Copy link
Copy Markdown
Member

Let us try this.

Yes. CMake can definitive copy files and that would be the right move... but let us go with this PR for now.

@lemire
lemire merged commit 78ce2b4 into simdjson:master Aug 18, 2020
@pps83
pps83 deleted the bugfix/Suppress-error-code-if-copying-of-README.md-amalgamate_demo.cpp-isnt-needed branch August 26, 2020 14:16
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.

3 participants