Spaces in project paths break BLT

swichers commented 7 years ago

My system information:

Operating system type: macOS
Operating system version: 10.12.5
BLT version: 8.7 and earlier

When I run this command:

  blt doctor
  <or any blt command that passes or uses raw paths>

I get the following output:

BUILD FAILED<path with space>/.vendor/acquia/blt/phing/build.xml:25:51: <path with space>/.vendor/acquia/blt/phing/tasks/properties.xml:42:175: Task exited with code 1; 0.2125 seconds

Note: Line number and file may vary depending on command used.

There are several places where paths are passed raw to the shell without escaping or quoting, and this causes the command to fail.

malikkotob commented 7 years ago

@swichers, I was able to reproduce this issue when the repo name itself has a space in it (i.e. the top-level directory), but not otherwise. Are you seeing the same behavior?

Steps:

Renamed my top-level directory to include a space
Ran blt doctor within it, saw the error
Reverted top-level directory name to remove the space
Created a directory called test directory within my top-level directory
Ran blt doctor within that, did not see the error

grasmash commented 7 years ago

@swichers is this a new issue with 8.7.0 or did it exist prior? I can't seem to reproduce it exactly, though I do get this error when the repo dir contains a space: Error: You must run this command from within a BLT-generated project repository!

grasmash commented 7 years ago

I updated my alias to this, and then could reproduce the error:

function blt() {

  POSSIBLE_ROOTS=( "$(pwd)" "$(git rev-parse --show-cdup 2> /dev/null)" )

  FOUND_ROOT=0
  for i in "${POSSIBLE_ROOTS[@]}"
  do
     if [ -f "$i/vendor/bin/blt" ]; then
       "$i/vendor/bin/blt" "$@"
       FOUND_ROOT=1
       break
     fi
  done

  if [ $FOUND_ROOT == 0 ]; then
    echo "Error: You must run this command from within a BLT-generated project repository!"
    return 1
  fi
}

swichers commented 7 years ago

@grasmash I saw the issue on 8.7 and 8.6.14 but this was an existing project with established BLT aliases.

My bash alias is:

function blt() {
  if [ "`git rev-parse --show-cdup 2> /dev/null`" != "" ]; then
    GIT_ROOT=$(git rev-parse --show-cdup)
  else
    GIT_ROOT="."
  fi

  if [ -f "$GIT_ROOT/vendor/bin/blt" ]; then
    $GIT_ROOT/vendor/bin/blt "$@"
  else
    echo "You must run this command from within a BLT-generated project repository."
    exit 1
  fi
}

Maybe the exact error message depends on the command that is being run? I was testing with blt doctor but saw similar behavior with other commands.

@malikkotob We saw the error with the following folder structure: ~/Work Projects/projectname

I took my working repo at ~/Projects/projectname, moved it to ~/Projects/test space/projectname and the error surfaced.

This is both 8.6.14 and 8.7:

~/Projects  $ cd client/
~/P/client  $ blt doctor
blt > doctor:
Changed current directory to /home/vagrant/.composer
<snip>
BUILD FINISHED; 7.8865 seconds
~/P/client  $
~/P/client  $ cd ..
~/Projects  $ mv client "client space"
~/Projects  $ cd client\ space/
~/P/client space  $ blt doctor

BUILD FAILED/~/Projects/client space/./vendor/acquia/blt/phing/build.xml:24:51: /~/Projects/client space/./vendor/acquia/blt/phing/tasks/properties.xml:42:175: Task exited with code 1
; 0.2087 seconds
/Projects  $ mv client\ space/ client
~/Projects  $ cd client
~/P/client  $ blt doctor
blt > doctor:
Changed current directory to /home/vagrant/.composer
<snip>
BUILD FINISHED; 7.8865 seconds
~/P/client  $
~/P/client  $ cd ..
~/Projects  $ mkdir "test space"
~/Projects  $ mv client "test space/client"
~/Projects  $ cd test\ space/client/
~/P/t/client  $ blt doctor

BUILD FAILED/~/Projects/test space/client/./vendor/acquia/blt/phing/build.xml:24:51: /~/Projects/test space/client/./vendor/acquia/blt/phing/tasks/properties.xml:42:175: Task exited with code 1
; 0.1042 seconds

Note that I only snipped the successful output, not anything that failed.

The developer scenario here is onboarding a new developer to an existing project. We did notice that even after fixing the space in his project path there was still 'weirdness' sticking around from the space (cached/generated files?) causing errors. We had him rebuild the project from scratch (but still using the existing repo files) and it worked.

swichers commented 7 years ago

I did some hacking on this and it's not any blt command, but instead any blt command that winds up passing its arguments directly to the shell. In 55d6d95 I have some minor changes that allow me to run blt doctor without errors.

(Note this is against 8.6.14 but the issue is in 8.7 as well.)

malikkotob commented 7 years ago

Submitted the work done here + an additional commit in PR https://github.com/acquia/blt/pull/1349. Build currently failing - https://travis-ci.org/acquia/blt/builds/220627065. Yet to dig in to see why that is, but am unable to reproduce locally.

blt > drupal:install:
     [echo] Installing Drupal...
Configuration import directory ../config/default does not contain any[warning]
configuration; will skip import.
exception 'Drush\Sql\SqlException' with message 'Unable to find a    [error]
matching SQL Class. Drush cannot find your database connection
details.' in
/home/travis/build/acquia/blt-project/vendor/drush/drush/commands/sql/sql.drush.inc:541
Stack trace:
#0
/home/travis/build/acquia/blt-project/vendor/drush/drush/commands/core/site_install.drush.inc(122):
drush_sql_get_class()
#1 [internal function]: drush_core_pre_site_install('lightning',
'install_configu...')
#2
/home/travis/build/acquia/blt-project/vendor/drush/drush/includes/command.inc(422):
call_user_func_array('drush_core_pre_...', Array)
#3
/home/travis/build/acquia/blt-project/vendor/drush/drush/includes/command.inc(231):
_drush_invoke_hooks(Array, Array)
#4 [internal function]: drush_command('lightning',
'install_configu...')
#5
/home/travis/build/acquia/blt-project/vendor/drush/drush/includes/command.inc(199):
call_user_func_array('drush_command', Array)
#6
/home/travis/build/acquia/blt-project/vendor/drush/drush/lib/Drush/Boot/BaseBoot.php(67):
drush_dispatch(Array)
#7
/home/travis/build/acquia/blt-project/vendor/drush/drush/includes/preflight.inc(66):
Drush\Boot\BaseBoot->bootstrap_and_dispatch()
#8
/home/travis/build/acquia/blt-project/vendor/drush/drush/drush.php(12):
drush_main()
#9 {main}
[phingcall] /home/travis/build/acquia/blt-project/./vendor/acquia/blt/phing/tasks/drupal.xml:15:10: /home/travis/build/acquia/blt-project/./vendor/acquia/blt/phing/tasks/drupal.xml:31:29: Drush exited with code 1
BUILD FAILED/home/travis/build/acquia/blt-project/./vendor/acquia/blt/phing/tasks/ci.xml:4:22: Execution of the target buildfile failed. Aborting.
; 18.1913 seconds

swichers commented 7 years ago

I think there are more changes necessary just based on this:

(8.6 version)

~/P/n/v/a/b/phing  $ ack 'cat '

build.xml
87:    <exec dir="${repo.root}" command="cat ${blt.root}/scripts/blt/ascii-art.txt" logoutput="true" passthru="true" checkreturn="false"/>

tasks/blt.xml
16:    <exec dir="${repo.root}" command="cat ${blt.root}/scripts/blt/ascii-art.txt" logoutput="true" passthru="true" checkreturn="false"/>
26:    <exec dir="${repo.root}" command="cat ${blt.root}/scripts/blt/ascii-art.txt" logoutput="true" passthru="true" checkreturn="false"/>

tasks/properties.xml
42:  <exec dir="${repo.root}" command="cat ${blt.config-files.schema-version}" outputProperty="blt.schema-version" checkreturn="true" logoutput="false" level="${blt.exec_level}"/>

tasks/vm.xml
54:    <exec dir="${repo.root}" command="cat ${blt.root}/scripts/drupal-vm/drupal-vm.aliases.drushrc.php >> ${repo.root}/drush/site-aliases/aliases.drushrc.php" logoutput="true" checkreturn="true" level="${blt.exec_level}"/>

If one of those has problems (the tasks/properties.xml) I would assume they all would.

malikkotob commented 7 years ago

Ah, thanks for pointing that out @swichers, I read your comment above as:

In 55d6d95 I have some minor changes that allow me to run blt without errors.

instead of:

...blt doctor without errors

swichers commented 7 years ago

Yeah I was just aiming to get a more concrete example of what was broken (and what could fix it), not an all-encompassing fix. That said,

@grasmash is work on this ticket useful given the planned changes from Phing?

grasmash commented 7 years ago

Fixing it in Robo would be worthwhile, if it's actually an issue there.

arosboro commented 7 years ago

Probably related, I get:

Creating BLT templated files...

/Volumes/Macintosh HD/Users/arosboro/git/myproject/vendor/acquia/blt/bin/blt add-to-project sh: /Volumes/Macintosh: No such file or directory

acquia / blt

Spaces in project paths break BLT #1328