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.

Sign up for GitHub

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

Jump to bottom

(v6.x backport) process: add --redirect-warnings command line argument #14480

Closed
wants to merge 2 commits into from
Closed

(v6.x backport) process: add --redirect-warnings command line argument #14480

wants to merge 2 commits into from

Conversation

sam-github
Copy link
Contributor

The --redirect-warnings command line argument allows process warnings
to be written to a specified file rather than printed to stderr.

Also adds an equivalent NODE_REDIRECT_WARNINGS environment variable.

If the specified file cannot be opened or written to for any reason,
the argument is ignored and the warning is printed to stderr.

If the file already exists, it will be appended to.

PR-URL: #10116
Reviewed-By: Michael Dawson michael_dawson@ca.ibm.com
Reviewed-By: Michal Zasso targos@protonmail.com
Reviewed-By: Fedor Indutny fedor.indutny@gmail.com

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. process Issues and PRs related to the process subsystem. v6.x labels Jul 25, 2017
@sam-github
Copy link
Contributor Author

Parking this until the next semver minor release of 6.x, see #14418 (comment)

@nodejs/lts

@sam-github sam-github added lts-watch-v6.x semver-minor PRs that contain new features and should be released in the next minor version. labels Jul 25, 2017
@MylesBorins MylesBorins force-pushed the v6.x-staging branch 6 times, most recently from f9419c2 to 403c465 Compare August 16, 2017 18:43
@mhdawson mhdawson mentioned this pull request Aug 28, 2017
Closed
@MylesBorins MylesBorins force-pushed the v6.x-staging branch 2 times, most recently from aaf4e13 to 31f572c Compare September 5, 2017 16:50
@gibfahn gibfahn mentioned this pull request Sep 16, 2017
Closed
@MylesBorins
Copy link
Contributor

Needs a rebase. Good to land

@sam-github sam-github force-pushed the v6.x-backport-redirect-warnings branch 2 times, most recently from f6063fc to 33c45ce Compare September 20, 2017 18:21
@sam-github
Copy link
Contributor Author

Landed in adf6d16

@sam-github sam-github closed this Sep 20, 2017
@MylesBorins MylesBorins reopened this Sep 20, 2017
@MylesBorins
Copy link
Contributor

backing this out of v6.x-staging it is not ready to land yet. We will land the minors next month when we are prepping the minor release

@sam-github sam-github force-pushed the v6.x-backport-redirect-warnings branch from 33c45ce to a76b3b8 Compare September 25, 2017 19:44
@sam-github sam-github mentioned this pull request Sep 25, 2017
Closed
4 tasks
@MylesBorins
Copy link
Contributor

landed in 158906b707

@MylesBorins MylesBorins reopened this Oct 10, 2017
@MylesBorins
Copy link
Contributor

@sam-github I had to back this out as it was breaking the build. Can you please rebase and confirm that this compiles

@sam-github sam-github force-pushed the v6.x-backport-redirect-warnings branch 2 times, most recently from b879e03 to 2604584 Compare October 10, 2017 21:46
@sam-github
Copy link
Contributor Author

rebased again, and it compiles for me, I'm running tests locally and on ci: https://ci.nodejs.org/job/node-test-pull-request/10590/

@sam-github
Copy link
Contributor Author

OK, I think this is because secure_getenv() doesn't exist on all linux platforms. I think this should have been fixed by #11051, but perhaps #11051 was landed before this semver-minor, so the part applying to this PR is missing. I'll patch.

jasnell and others added 2 commits October 10, 2017 16:26
@jasnell @sam-github
process: add --redirect-warnings command line argument
bc89407 
The --redirect-warnings command line argument allows process warnings
to be written to a specified file rather than printed to stderr.

Also adds an equivalent NODE_REDIRECT_WARNINGS environment variable.

If the specified file cannot be opened or written to for any reason,
the argument is ignored and the warning is printed to stderr.

If the file already exists, it will be appended to.

PR-URL: nodejs#10116
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Michal Zasso <targos@protonmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
@sam-github
src: make copies of startup environment variables
63c1ce4 
Mutations of the environment can invalidate pointers to environment
variables, so make `secure_getenv()` copy them out instead of returning
pointers.

This is the part of nodejs#11051 that
applies to
nodejs@03e89b3
@sam-github sam-github force-pushed the v6.x-backport-redirect-warnings branch from 2604584 to 63c1ce4 Compare October 10, 2017 23:27
@sam-github
Copy link
Contributor Author

replaced by #12677

@sam-github sam-github closed this Oct 10, 2017
@sam-github sam-github deleted the v6.x-backport-redirect-warnings branch October 16, 2018 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. process Issues and PRs related to the process subsystem. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sam-github @MylesBorins @nodejs-github-bot @jasnell