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

Failed build returns 0 exit code #1169

Merged
merged 1 commit into from
Jun 27, 2014
Merged

Conversation

ludekstepan
Copy link
Contributor

When ember build fails a non-zero error code should be returned. This is similar to #1150

Steps to reproduce

ember init
# builds project and outputs OK
ember build && echo "OK"
# remove some dependency to break the build
rm -rf vendor/jquery
ember build && echo "OK"

Output:

version: 0.0.36
Build failed.
Path or pattern "vendor/jquery/dist/jquery.js" did not match any files
Error: Path or pattern "vendor/jquery/dist/jquery.js" did not match any files
    at Object.multiGlob (./node_modules/ember-cli/node_modules/broccoli-concat/node_modules/broccoli-kitchen-sink-helpers/index.js:216:13)
    at ./node_modules/ember-cli/node_modules/broccoli-concat/index.js:41:30
    at tryCatch (./node_modules/ember-cli/node_modules/rsvp/dist/commonjs/rsvp/-internal.js:163:16)
    at invokeCallback (./node_modules/ember-cli/node_modules/rsvp/dist/commonjs/rsvp/-internal.js:172:17)
    at publish (./node_modules/ember-cli/node_modules/rsvp/dist/commonjs/rsvp/-internal.js:150:13)
    at flush (./node_modules/ember-cli/node_modules/rsvp/dist/commonjs/rsvp/asap.js:51:9)
    at process._tickCallback (node.js:419:13)OK

@stefanpenner
Copy link
Contributor

Can you try 0.0.37. Improvements have been made

@ludekstepan
Copy link
Contributor Author

Same problem with 0.0.37

@stefanpenner
Copy link
Contributor

yup it does, good catch.

we should propagate this style solution throughout the CLI 3c8aebf

@stefanpenner
Copy link
Contributor

actually, just tried to reproduce in 0.0.37 and it works as expected. ensure you npm -g uninstall ember-cli as the fix was in the executable itself.

I do believe we have more issues here, but it appears this scenario does work.

@stefanpenner
Copy link
Contributor

easy way to confirm is

cat $(which ember) | grep '}).then(process.exit);'

@ludekstepan
Copy link
Contributor Author

Thanks for you assistance, I've just did:

npm -g uninstall ember-cli
npm -g install ember-cli
cat $(which ember) | grep '}).then(process.exit);'
# prints:     }).then(process.exit);

But I can still reproduce:

ember build; echo "Exit code $?"
version: 0.0.37
Buildingapp.import(vendor/ember-data/ember-data.js) - Passing modules object is deprecated. Please pass an option object with modules as export key (see http://git.io/H1GsPw for more info).
Build failed.
Path or pattern "vendor/jquery/dist/jquery.js" did not match any files
Error: Path or pattern "vendor/jquery/dist/jquery.js" did not match any files
    at Object.multiGlob (/Users/ludek/Projects/tmp/pokus/node_modules/ember-cli/node_modules/broccoli-concat/node_modules/broccoli-kitchen-sink-helpers/index.js:216:13)
    at /Users/ludek/Projects/tmp/pokus/node_modules/ember-cli/node_modules/broccoli-concat/index.js:41:30
    at tryCatch (/Users/ludek/Projects/tmp/pokus/node_modules/ember-cli/node_modules/rsvp/dist/commonjs/rsvp/-internal.js:163:16)
    at invokeCallback (/Users/ludek/Projects/tmp/pokus/node_modules/ember-cli/node_modules/rsvp/dist/commonjs/rsvp/-internal.js:172:17)
    at publish (/Users/ludek/Projects/tmp/pokus/node_modules/ember-cli/node_modules/rsvp/dist/commonjs/rsvp/-internal.js:150:13)
    at flush (/Users/ludek/Projects/tmp/pokus/node_modules/ember-cli/node_modules/rsvp/dist/commonjs/rsvp/asap.js:51:9)
    at process._tickCallback (node.js:419:13)Exit code 0

@stefanpenner
Copy link
Contributor

@ludekstepan sounds good, i suspect the failure I was seeing was going down a working path.

looking at https://github.com/stefanpenner/ember-cli/blob/master/lib/tasks/build.js#L46-L58 it clearly doesn't return the correct status code.

@ludekstepan
Copy link
Contributor Author

Yep, I've been just looking at the same thing, there's no return statement. :-)

@stefanpenner
Copy link
Contributor

@ludekstepan the reason i haven't submitted a fix yet is lack of time to add the test so we don't regress again. If you have time to help, I would be appreciative :)

@ludekstepan
Copy link
Contributor Author

I'll try and eventually open a pull request :) thanks!

@vire
Copy link

vire commented Jun 26, 2014

👍 @ludekstepan

@@ -1,5 +1,6 @@
# ember-cli Changelog

* [BUGFIX] Failed build should return non-zero exit code. [#1169](https://github.com/stefanpenner/ember-cli/pull/1169)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New items should be at the end of the listing generally.

@stefanpenner
Copy link
Contributor

LGTM

rwjblue added a commit that referenced this pull request Jun 27, 2014
Failed build returns 0 exit code
@rwjblue rwjblue merged commit 0185478 into ember-cli:master Jun 27, 2014
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.

None yet

4 participants