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

Projects
None yet
2 participants
@elfring
Copy link
Contributor

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.

Bug #1070: Completed quoting for parameters of some CMake 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

Merge pull request #1104 from elfring/Complete_quoting_for_parameters…
…_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

This comment has been minimized.

Copy link
Member

commented Apr 2, 2014

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

@hkaiser

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Contributor Author

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Contributor Author

commented Apr 3, 2014

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

Next issue:

@hkaiser

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Contributor Author

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

This comment has been minimized.

Copy link
Member

commented Apr 3, 2014

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

@elfring

This comment has been minimized.

Copy link
Contributor Author

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

This comment has been minimized.

Copy link
Contributor Author

commented Apr 3, 2014

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

@elfring

This comment has been minimized.

Copy link
Contributor Author

commented Apr 4, 2014

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

@elfring

This comment has been minimized.

Copy link
Contributor Author

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

This comment has been minimized.

Copy link
Contributor Author

commented Apr 4, 2014

@hkaiser

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Member

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
You can’t perform that action at this time.