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

Complete quoting for parameters of some CMake commands. #1104

Conversation

elfring
Copy link
Contributor

@elfring elfring commented Apr 2, 2014

A wiki article pointed out that whitespace will only be preserved for parameters in CMake commands if passed strings will be appropriately quoted or escaped.

Quoting can be added so that more places should also handle file names correctly which contain space characters or semicolons eventually.

…e commands

A wiki article pointed out that whitespace will only be preserved for parameters
in CMake commands if passed strings will be appropriately quoted or escaped.
http://cmake.org/Wiki/CMake/Language_Syntax#CMake_splits_arguments_unless_you_use_quotation_marks_or_escapes.

Quoting was added so that more places should also handle file names correctly
which contain space characters or semicolons eventually.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
@hkaiser hkaiser added this to the 0.9.9 milestone Apr 2, 2014
hkaiser added a commit that referenced this pull request Apr 2, 2014
…_of_some_CMake_commands

Complete quoting for parameters of some CMake commands.
@hkaiser hkaiser merged commit dd50c0c into STEllAR-GROUP:master Apr 2, 2014
@hkaiser
Copy link
Member

hkaiser commented Apr 2, 2014

I'm impressed by the amount of work you put into this! Thanks a lot!

@hkaiser
Copy link
Member

hkaiser commented Apr 2, 2014

This patch causes compilation of one of the examples to fail. See here: http://hermione.cct.lsu.edu/builders/hpx_gcc46_x8664_boost152_debug/builds/306/steps/build_core/logs/stdio. Would you be able to fix this?

@elfring
Copy link
Contributor Author

elfring commented Apr 3, 2014

I admit that I did not test all possible software combinations for my update suggestion.

I imagine that I can not fix this unexpected build failure alone.

...
CMake Error: File /opt/buildbot/slave/hpx_gcc46_x8664_boost152_debug/build/gcc-46_boost-1520_debug/tests/performance/network/network_storage/"/opt/buildbot/slave/hpx_gcc46_x8664_boost152_debug/build/tests/performance/network/network_storage"/slurm-test-HPX-storage.sh.in does not exist.
CMake Error at /opt/buildbot/slave/hpx_gcc46_x8664_boost152_debug/build/tests/performance/network/network_storage/copy_script.cmake:4 (configure_file):
  configure_file Problem configuring file
...

It seems that the text "slurm-test-HPX-storage" is referenced at one place. (Do you know a bit more about this component?)
Do the changes for a single script trigger "this build surprise"?

@hkaiser
Copy link
Member

hkaiser commented Apr 3, 2014

The issue above seems to be fixed now, but there is another one: http://hermione.cct.lsu.edu/builders/hpx_docs/builds/172/steps/build_docs/logs/stdio which looks like is related to the quoting changes.

@elfring
Copy link
Contributor Author

elfring commented Apr 3, 2014

Please tell me how the issue was fixed around the "Slurm script generator".

Next issue:

@hkaiser
Copy link
Member

hkaiser commented Apr 3, 2014

Frankly, I have no idea how the issue above was solved :-P. The only change I made was to avoid that the target was built always (f75896a).

The buildbot is just a automated tool executing the build scripts, I doubt it's involved (it worked before). The variable hpx_SOURCE_DIR is set by CMake itself and is derived from the directory where the main CMakeLists.txt is located. So currently I'm a bit clueless what's causing the issue.

@elfring
Copy link
Contributor Author

elfring commented Apr 3, 2014

Do we need to clarify the passing of directory names with quotation marks in more detail for two build failures?

@hkaiser
Copy link
Member

hkaiser commented Apr 3, 2014

I'm not sure I understand what you mean. Could you elaborate, please?

@elfring
Copy link
Contributor Author

elfring commented Apr 3, 2014

Did you notice that the error messages from the two build logs mentioned directories with quotation marks in the names?
(I am also unsure at the moment where these quotes come from.)

@elfring
Copy link
Contributor Author

elfring commented Apr 3, 2014

Did you report build failures that are two permanent open issues at the moment?

@elfring
Copy link
Contributor Author

elfring commented Apr 4, 2014

I find it interesting here that you renamed an involved variable.

@elfring
Copy link
Contributor Author

elfring commented Apr 4, 2014

Does a specific display from your build bot indicate that my update suggestion did not damage your software generation process more than I hoped to make it a bit safer?

@elfring
Copy link
Contributor Author

elfring commented Apr 4, 2014

@hkaiser
Copy link
Member

hkaiser commented Apr 4, 2014

I find it interesting here that you renamed an involved variable.
Yes, this was a genuine problem. I have no idea how it could have ever worked :/

@hkaiser
Copy link
Member

hkaiser commented Apr 4, 2014

Does a specific display from your build bot indicate that my update
suggestion did not damage your software generation process more
than I hoped to make it a bit safer?
Yes, thanks again. You patch is fine, except for those problems we discussed (and where I don't even know what's causing them).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants