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

Remove dependency on tee command #420

Merged
merged 1 commit into from
Mar 18, 2016
Merged

Remove dependency on tee command #420

merged 1 commit into from
Mar 18, 2016

Conversation

seanpdoyle
Copy link
Contributor

Closes #417.

Instead of piping output to the tee command (which was
previously swallowing non-zero exit statuses), capture output
from Open3.capture2e and write to both the specified log file and
STDOUT.

err = StringIO.new
runner = EmberCli::Runner.new(err: err, out: out)
it "writes to all `err` and `out` streams" do
output_streams = 4.times.map { StringIO.new }

Choose a reason for hiding this comment

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

Use Array.new with a block instead of .times.map.

Closes [#417].

Instead of piping output to the [`tee`][tee] command (which was
previously swallowing non-zero exit statuses), capture output
from `Open3.capture2e` and write to both the specified log file and
`STDOUT`.

[#417]: #417
[tee]: http://man7.org/linux/man-pages/man1/tee.1.html
@mike-burns
Copy link

This is my favorite solution yet.

@klaustopher
Copy link

I really like "faking" an IO object that than writes out to multiple other IO objects ... ❤️ the approach!

The approach is working for me 👍

$ ls -lah ~/.config/configstore/ember-cli.json
-rw------- 1 root wheel 56 Mär 17 14:14 /Users/klaustopher/.config/configstore/ember-cli.json

$ bin/rake ember:compile --trace
** Invoke ember:compile (first_time)
** Invoke ember:install (first_time)
** Invoke environment (first_time)
** Execute environment
** Execute ember:install
/Users/klaustopher/Projects/application/feed/node_modules/configstore/index.js:53
                throw err;
                ^

Error: EACCES: permission denied, open '/Users/klaustopher/.config/configstore/ember-cli.json'
You don't have access to this file.

    at Error (native)
    at Object.fs.openSync (fs.js:584:18)
    at Object.fs.readFileSync (fs.js:431:33)
    at Object.create.all.get (/Users/klaustopher/Projects/application/feed/node_modules/configstore/index.js:34:26)
    at Object.Configstore (/Users/klaustopher/Projects/application/feed/node_modules/configstore/index.js:27:44)
    at clientId (/Users/klaustopher/Projects/application/feed/node_modules/ember-cli/lib/cli/index.js:22:21)
    at module.exports (/Users/klaustopher/Projects/application/feed/node_modules/ember-cli/lib/cli/index.js:65:19)
    at /Users/klaustopher/Projects/application/feed/node_modules/ember-cli/bin/ember:26:3
    at /Users/klaustopher/Projects/application/feed/node_modules/resolve/lib/async.js:44:21
    at ondir (/Users/klaustopher/Projects/application/feed/node_modules/resolve/lib/async.js:187:31)
/Users/klaustopher/Projects/application/demandcheck/node_modules/configstore/index.js:53
                throw err;
                ^

Error: EACCES: permission denied, open '/Users/klaustopher/.config/configstore/ember-cli.json'
You don't have access to this file.

    at Error (native)
    at Object.fs.openSync (fs.js:584:18)
    at Object.fs.readFileSync (fs.js:431:33)
    at Object.create.all.get (/Users/klaustopher/Projects/application/demandcheck/node_modules/configstore/index.js:34:26)
    at Object.Configstore (/Users/klaustopher/Projects/application/demandcheck/node_modules/configstore/index.js:27:44)
    at clientId (/Users/klaustopher/Projects/application/demandcheck/node_modules/ember-cli/lib/cli/index.js:22:21)
    at module.exports (/Users/klaustopher/Projects/application/demandcheck/node_modules/ember-cli/lib/cli/index.js:65:19)
    at /Users/klaustopher/Projects/application/demandcheck/node_modules/ember-cli/bin/ember:26:3
    at /Users/klaustopher/Projects/application/demandcheck/node_modules/resolve/lib/async.js:44:21
    at ondir (/Users/klaustopher/Projects/application/demandcheck/node_modules/resolve/lib/async.js:187:31)
** Execute ember:compile
/Users/klaustopher/Projects/application/feed/node_modules/configstore/index.js:53
                throw err;
                ^

Error: EACCES: permission denied, open '/Users/klaustopher/.config/configstore/ember-cli.json'
You don't have access to this file.

    at Error (native)
    at Object.fs.openSync (fs.js:584:18)
    at Object.fs.readFileSync (fs.js:431:33)
    at Object.create.all.get (/Users/klaustopher/Projects/application/feed/node_modules/configstore/index.js:34:26)
    at Object.Configstore (/Users/klaustopher/Projects/application/feed/node_modules/configstore/index.js:27:44)
    at clientId (/Users/klaustopher/Projects/application/feed/node_modules/ember-cli/lib/cli/index.js:22:21)
    at module.exports (/Users/klaustopher/Projects/application/feed/node_modules/ember-cli/lib/cli/index.js:65:19)
    at /Users/klaustopher/Projects/application/feed/node_modules/ember-cli/bin/ember:26:3
    at /Users/klaustopher/Projects/application/feed/node_modules/resolve/lib/async.js:44:21
    at ondir (/Users/klaustopher/Projects/application/feed/node_modules/resolve/lib/async.js:187:31)
          ERROR: Failed command: `/Users/klaustopher/Projects/application/feed/node_modules/ember-cli/bin/ember build --environment 'development' --output-path '/Users/klaustopher/Projects/application/tmp/ember-cli/apps/feed'`
          OUTPUT:
            /Users/klaustopher/Projects/application/feed/node_modules/configstore/index.js:53
                throw err;
                ^

Error: EACCES: permission denied, open '/Users/klaustopher/.config/configstore/ember-cli.json'
You don't have access to this file.

    at Error (native)
    at Object.fs.openSync (fs.js:584:18)
    at Object.fs.readFileSync (fs.js:431:33)
    at Object.create.all.get (/Users/klaustopher/Projects/application/feed/node_modules/configstore/index.js:34:26)
    at Object.Configstore (/Users/klaustopher/Projects/application/feed/node_modules/configstore/index.js:27:44)
    at clientId (/Users/klaustopher/Projects/application/feed/node_modules/ember-cli/lib/cli/index.js:22:21)
    at module.exports (/Users/klaustopher/Projects/application/feed/node_modules/ember-cli/lib/cli/index.js:65:19)
    at /Users/klaustopher/Projects/application/feed/node_modules/ember-cli/bin/ember:26:3
    at /Users/klaustopher/Projects/application/feed/node_modules/resolve/lib/async.js:44:21
    at ondir (/Users/klaustopher/Projects/application/feed/node_modules/resolve/lib/async.js:187:31)

          ERROR: Failed command: `/Users/klaustopher/Projects/application/feed/node_modules/ember-cli/bin/ember build --environment 'development' --output-path '/Users/klaustopher/Projects/application/tmp/ember-cli/apps/feed'`
          OUTPUT:
            /Users/klaustopher/Projects/application/feed/node_modules/configstore/index.js:53
                throw err;
                ^

Error: EACCES: permission denied, open '/Users/klaustopher/.config/configstore/ember-cli.json'
You don't have access to this file.

    at Error (native)
    at Object.fs.openSync (fs.js:584:18)
    at Object.fs.readFileSync (fs.js:431:33)
    at Object.create.all.get (/Users/klaustopher/Projects/application/feed/node_modules/configstore/index.js:34:26)
    at Object.Configstore (/Users/klaustopher/Projects/application/feed/node_modules/configstore/index.js:27:44)
    at clientId (/Users/klaustopher/Projects/application/feed/node_modules/ember-cli/lib/cli/index.js:22:21)
    at module.exports (/Users/klaustopher/Projects/application/feed/node_modules/ember-cli/lib/cli/index.js:65:19)
    at /Users/klaustopher/Projects/application/feed/node_modules/ember-cli/bin/ember:26:3
    at /Users/klaustopher/Projects/application/feed/node_modules/resolve/lib/async.js:44:21
    at ondir (/Users/klaustopher/Projects/application/feed/node_modules/resolve/lib/async.js:187:31)


$ echo $?
1

The only thing that is happening now, is that it tries to compile all three of our ember apps before failing ... But this is totally fine and I don't think it's worth to putting in any more efforts for fixing this ;)

bin/rake assets:precompile also fails as expected 👍

Thanks @seanpdoyle for all the efforts!!! I cannot thank you enough for getting this sorted out! Should we keep this branch in our Gemfile or can we expect a release soon?

@seanpdoyle
Copy link
Contributor Author

@klaustopher once CI is green, we'll merge this into master.

The only thing that is happening now, is that it tries to compile all three of our ember apps before failing ...

This is unintended behavior. I'd like to resolve this before cutting a release.

@klaustopher
Copy link

@seanpdoyle That sounds good. Let me know when you have pushed some changes so I can test. Will be available in the next 2 hours.

@seanpdoyle seanpdoyle merged commit f57ce82 into master Mar 18, 2016
@seanpdoyle seanpdoyle deleted the remove-tee branch March 18, 2016 15:48
end

def write_to_err(output)
write(out_streams + err_streams, output)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@thoughtbot/shell does it make sense to write errors to both STDERR and STDOUT?

I'm having trouble using git blame to find the commit that introduced (and justified) this behavior (this PR is only a refactoring of the behavior).

Is it unconventional to only log errors to STDERR?

Choose a reason for hiding this comment

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

Just one, please. It makes the most sense to send errors to stderr.

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.

4 participants