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

composer now outputs to stderr #19

Closed
stunami opened this issue Mar 18, 2015 · 13 comments · Fixed by #20
Closed

composer now outputs to stderr #19

stunami opened this issue Mar 18, 2015 · 13 comments · Fixed by #20

Comments

@stunami
Copy link

stunami commented Mar 18, 2015

Hi,

composer recently changed to output from using stdout to stderr as you can see from this PR

https://github.com/composer/composer/pull/3715/files

this breaks composer::project as the grep fails on the onlyif

https://github.com/Brainsware/puppet-composer/blob/master/manifests/project.pp#L125

  exec { "composer_install_${title}":
    command => "${composer} install ${base_opts} ${install_opts}",
    onlyif  => "${composer} install ${install_opts} --dry-run | grep -E -- '- (Install|Updat)ing '",
  }

Piping stderr to stdout fixes this as below, though I haven't tested it with older composer versions.

    onlyif  => "${composer} install ${install_opts} --dry-run 2>&1 >/dev/null | grep -E -- '- (Install|Updat)ing '",

@mbrodala
Copy link
Contributor

What's the >/dev/null in your fix for? This drops everything (stdout & stderr) thus this command can never succeed.

The general redirection of stderr to stdout to have its output available for pattern matching looks fine though.

If this was executed on an older Composer version and a real error occurred, the pattern match would just not succeed which is the same as if all required packages are already installed.

@mbrodala
Copy link
Contributor

Uh, apparently >/dev/null only drops the original stdout, not the stderr redirected to stdout. But this would break on older Composer versions where the output went to stdout.

Thus without this part, this check should work on both old and new Composer versions.

@stunami
Copy link
Author

stunami commented Mar 18, 2015

Yeah I figured it wasn't the complete solution which is why there was no PR from me.

According to the composer dev's the correct check is the exit code composer/composer#3795 (comment)

@mbrodala
Copy link
Contributor

Yeah, if we where checking for an error. But in both cases (packages installed/updated or nothing) the exit code is 0, so nothing we can check for.

mbrodala added a commit that referenced this issue Mar 19, 2015
Recently Composer has moved a lot of status messages from stdout
to stderr, thus we need to perform a redirect here to keep pattern
matching working.

See composer/composer#3715

Fixes #19
mbrodala added a commit that referenced this issue Mar 19, 2015
Recently Composer has moved a lot of status messages from stdout
to stderr, thus we need to perform a redirect here to keep pattern
matching working.

See composer/composer#3715

Fixes #19
@yakatz
Copy link
Contributor

yakatz commented Oct 12, 2015

Will this make it into a release anytime soon?
The version on puppetforge needs this.

@mbrodala
Copy link
Contributor

There are some refactoring steps outstanding before a new release can be made. You can pull this version explicitly e.g. in your Puppetfile:

mod 'brainsware-composer',
  :git => 'git://github.com/Brainsware/puppet-composer.git',
  :ref => 'f8b74dfdf9e5176eb586b897c84afaf89b94607b'

@igalic
Copy link
Contributor

igalic commented Oct 13, 2015

sorry for chiming in so late: I can make a new release, OR, I make travis do it, so anyone with merge access can!

@goreilly
Copy link

I this project still maintained? I just encountered this error from the version (0.2.5) on puppet forge.

@mbrodala
Copy link
Contributor

@goreilly Sure this is still maintained. See my previous comment to pull in the development version which fixes the issue. @igalic Maybe make a release with the current state after all?

@igalic
Copy link
Contributor

igalic commented Jan 14, 2016

will do!

@igalic
Copy link
Contributor

igalic commented Jan 14, 2016

released 0.2.6!

@mbrodala
Copy link
Contributor

@igalic Thanks. :-)

@goreilly
Copy link

Thanks you guys 😄

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 a pull request may close this issue.

5 participants