Skip to content

Conversation

stesie
Copy link
Contributor

@stesie stesie commented Mar 12, 2017

Given a Commandfile like this

command 'configure',
  description: 'executes "configure" to prepare build',
  parameters: {
    cxxflags: { default: "-ggdb -Wall -Wno-write-strings" },
    ldflags: { default: "-ggdb" },
  },
  script:  ...

if I'd like to create a chain "coverage" and set parameter cxxflags to -O0 --coverage and ldflags to --coverage, ... coming from a shell background I intuitivly tried to do this (and after all vagrant run v8-5.8 configure --cxxflags="-O0 --coverage" --ldflags="--coverage" on a shell prompt works):

chain 'coverage',
  commands: [
    { command: 'phpize' },
    { command: 'configure', argv: '--cxxflags="-O0 --coverage" --ldflags="--coverage"' },
    { command: 'clean' },
    { command: 'build' },
    { command: 'test' },
    { command: 'coverage_report' },
  ]

... but this fails.

The reason for this is that argv is simply String.split without a parameter (hence space). So it splits the argv to [ '--cxxflags="-O0', '--coverage"', '--ldflags="--coverage"' ] ... which is obviously wrong.

Since I didn't want to add quotes and escape string parsing I introduced array syntax like this

chain 'coverage',
  commands: [
    { command: 'phpize' },
    { command: 'configure', argv: ['--cxxflags=-O0 --coverage', '--ldflags=--coverage'] },
    { command: 'clean' },
    { command: 'build' },
    { command: 'test' },
    { command: 'coverage_report' },
  ]

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 96.525% when pulling 838b229 on stesie:chain-array-argv into b673fc2 on mneudert:master.

@mneudert mneudert merged commit 706e2d4 into mneudert:master Mar 12, 2017
@mneudert
Copy link
Owner

❤️ 💙 💛 💜 💚

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 96.525% when pulling 445961a on stesie:chain-array-argv into b673fc2 on mneudert:master.

@stesie stesie deleted the chain-array-argv branch March 12, 2017 21:33
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