run.sh: Check for spaces in the path to the current directory. #1079
Conversation
The shell wrapper script for the ODK is assuming that the directory containing the repository does not contain spaces in its path. This is required for the logic that binds the repository to the /work directory inside the container to work, and we may not change that without breaking the requirement that the wrapper script shall be POSIX compliant (supporting space-containing paths would require using array-typed variables, which are not specified by POSIX). This commit adds an explicit check at the beginning of the wrapper script to ensure that the current working directory does not contain any spaces, and immediately abort if it does. closes #1078
|
The fact that the repository must not be at the end of a space-containing path should be documented somewhere. Since |
|
Good idea. Feel free to do it yourself, if you dont have time, just drop an issue and assign to me! Thanks a lot! |
|
Will do that next week. My work machine is off until Monday. 🌴 |
| @@ -16,6 +16,14 @@ | |||
|
|
|||
| set -e | |||
|
|
|||
| # Check for spaces in the current directory path | |||
| for item in $PWD ; do | |||
| if [ "$item" != "$PWD" ]; then | |||
There was a problem hiding this comment.
I don't get how this fixes the issue, but I trust that it does and I'm just misunderstanding something. Thanks for addressing #1078!
There was a problem hiding this comment.
This is a way of checking whether the current directory (as stored in the PWD variable) contains spaces.
To illustrate how it works, let’s first suppose that PWD does not contain any space (for example, let’s say it contains /home/alice/myont). The first line will be expanded as for item in /home/alice/myont. Because /home/alice/myont cannot be split by the shell (since it contains no spaces), the for loop will be executed only once, where the loop variable item will be set to /home/alice/myont.
Inside the loop, the "$item" != "$PWD" condition will be evaluated to false (since item contains the same value as PWD), so we’ll leave the loop without having done anything.
But suppose that PWD contains (at least) one space (for example /home/alice/my ont). The first line will be expanded as for item in /home/alice/my ont. The shell will split /home/alice/my ont in two (notice that $PWD is not enclosed in quotes), so the for loop will (in principle) be executed twice: first with item set to /home/alice/my, and then with item set to ont.
During the first turn in the loop, the "$item" != "$PWD" condition will now be evaluated to true, so the script will print the error message and terminate.
That’s a slightly convoluted way of doing if echo $PWD | grep ' ' ; then echo "There are spaces in the current directory path ; fi, with the advantage that we don’t need to spawn a grep process. :)
There was a problem hiding this comment.
Wow, thanks for taking the time on the explanation!
I only needed to read up to:
The first line will be expanded as
for item in /home/alice/myont.
I thought that $PWD would get interpolated as a string, so I thought it would be iterating over characters in that string.
The shell wrapper script for the ODK is assuming that the directory containing the repository does not contain spaces in its path. This is required for the logic that binds the repository to the
/workdirectory inside the container to work, and we may not change that without breaking the requirement that the wrapper script shall be POSIX compliant (supporting space-containing paths would require using array-typed variables, which are not specified by POSIX).This commit adds an explicit check at the beginning of the wrapper script to ensure that the current working directory does not contain any spaces, and immediately abort if it does.
closes #1078