Skip to content
This repository has been archived by the owner on Jan 10, 2023. It is now read-only.

Switch from execSync to execFileSync #180

Merged
merged 15 commits into from
Jul 17, 2020
Merged

Switch from execSync to execFileSync #180

merged 15 commits into from
Jul 17, 2020

Conversation

drazisil
Copy link
Contributor

No description provided.

@drazisil drazisil marked this pull request as ready for review June 30, 2020 14:17
@drazisil drazisil requested a review from hootener June 30, 2020 14:17
@drazisil drazisil marked this pull request as draft July 10, 2020 15:29
@lgtm-com
Copy link

lgtm-com bot commented Jul 16, 2020

This pull request fixes 5 alerts when merging a4cc565 into 0c4d7f3 - view on LGTM.com

fixed alerts:

  • 3 for Unsafe shell command constructed from library input
  • 2 for Shell command built from environment values

@lgtm-com
Copy link

lgtm-com bot commented Jul 16, 2020

This pull request fixes 5 alerts when merging 2956ed4 into 0c4d7f3 - view on LGTM.com

fixed alerts:

  • 3 for Unsafe shell command constructed from library input
  • 2 for Shell command built from environment values

@lgtm-com
Copy link

lgtm-com bot commented Jul 16, 2020

This pull request fixes 5 alerts when merging 27792cc into 0c4d7f3 - view on LGTM.com

fixed alerts:

  • 3 for Unsafe shell command constructed from library input
  • 2 for Shell command built from environment values

@lgtm-com
Copy link

lgtm-com bot commented Jul 16, 2020

This pull request fixes 5 alerts when merging 46aed7b into 0c4d7f3 - view on LGTM.com

fixed alerts:

  • 3 for Unsafe shell command constructed from library input
  • 2 for Shell command built from environment values

@lgtm-com
Copy link

lgtm-com bot commented Jul 16, 2020

This pull request fixes 5 alerts when merging de77ff3 into 0c4d7f3 - view on LGTM.com

fixed alerts:

  • 3 for Unsafe shell command constructed from library input
  • 2 for Shell command built from environment values

@lgtm-com
Copy link

lgtm-com bot commented Jul 16, 2020

This pull request fixes 5 alerts when merging 7148c18 into 0c4d7f3 - view on LGTM.com

fixed alerts:

  • 3 for Unsafe shell command constructed from library input
  • 2 for Shell command built from environment values

@drazisil drazisil marked this pull request as ready for review July 16, 2020 16:07
@drazisil
Copy link
Contributor Author

This is fixed on Linux/MacOS. I have no ideas how to make Windows work.

@eddiemoore , @edward-codecov , @hootener , @thomasrockhu , etc. Suggestions very welcome.

@lgtm-com
Copy link

lgtm-com bot commented Jul 16, 2020

This pull request fixes 5 alerts when merging e9bd861 into 0c4d7f3 - view on LGTM.com

fixed alerts:

  • 3 for Unsafe shell command constructed from library input
  • 2 for Shell command built from environment values

@lgtm-com
Copy link

lgtm-com bot commented Jul 16, 2020

This pull request fixes 5 alerts when merging caca56c into 0c4d7f3 - view on LGTM.com

fixed alerts:

  • 3 for Unsafe shell command constructed from library input
  • 2 for Shell command built from environment values

@lgtm-com
Copy link

lgtm-com bot commented Jul 16, 2020

This pull request fixes 5 alerts when merging 40caa82 into 0c4d7f3 - view on LGTM.com

fixed alerts:

  • 3 for Unsafe shell command constructed from library input
  • 2 for Shell command built from environment values

lib/codecov.js Outdated Show resolved Hide resolved
Co-authored-by: Guillaume St-Pierre <guillaume@codecov.io>
@lgtm-com
Copy link

lgtm-com bot commented Jul 17, 2020

This pull request fixes 5 alerts when merging aa09765 into 5f6cc62 - view on LGTM.com

fixed alerts:

  • 3 for Unsafe shell command constructed from library input
  • 2 for Shell command built from environment values

lib/codecov.js Outdated Show resolved Hide resolved
drazisil and others added 2 commits July 17, 2020 11:59
Co-authored-by: Guillaume St-Pierre <guillaume@codecov.io>
@lgtm-com
Copy link

lgtm-com bot commented Jul 17, 2020

This pull request fixes 5 alerts when merging 1dd9c25 into 5f6cc62 - view on LGTM.com

fixed alerts:

  • 3 for Unsafe shell command constructed from library input
  • 2 for Shell command built from environment values

@lgtm-com
Copy link

lgtm-com bot commented Jul 17, 2020

This pull request fixes 5 alerts when merging 9f0c2c9 into 5f6cc62 - view on LGTM.com

fixed alerts:

  • 3 for Unsafe shell command constructed from library input
  • 2 for Shell command built from environment values

@drazisil drazisil merged commit c0711c6 into master Jul 17, 2020
@drazisil drazisil deleted the CE-1846 branch July 17, 2020 16:20
yhatt added a commit to yhatt/codecov-node that referenced this pull request Jul 21, 2020
ref: codecov#184
continuous from: ada583b

codecov#180 makes `find` command change to run through
`execFileSync()`. However, it always returns empty result because of
the quote has over-included as the filename of search condition. So
removed quotes from generated conditions.

In addition, removing brackets from find arguments (a4cc565) has
affected to the result of finding coverage files. This commit reverts
brackets to get back the original search condition too.

Previously quotes and escaped-brackets are required to run command on
the shell environment, but no longer need them because of using
`execFileSync()` that is disabled shell environment by default.
Copy link
Collaborator

@eddiemoore eddiemoore left a comment

Choose a reason for hiding this comment

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

I know it's already merged, but thought I should comment on it.

// It's not straightforward due to the nature of the find command
_findArgs = [root].concat(patterns)
if (more_patterns) {
_findArgs.concat(more_patterns)
Copy link
Collaborator

Choose a reason for hiding this comment

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

concat doesn't mutate, it returns a new instance. So in this case you'd need to do _findArgs = _findArgs.concat(more_patterns)

Comment on lines +500 to +504
_findArgs = [root].concat(winPatterns)
if (more_patterns) {
_findArgs.concat(more_patterns)
}
_files = execSync('dir ' + winPatterns.join(' ') + more_patterns)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like nothing is happening with _findArgs in here.

_files = execSync('dir ' + patterns + more_patterns)
_findArgs = [root].concat(winPatterns)
if (more_patterns) {
_findArgs.concat(more_patterns)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same thing here with concat doesn't mutate.

@drazisil
Copy link
Contributor Author

Thanks, @eddiemoore. Can you throw together a PR and we'll get it fixed? (It might have already been, this version was broken and needed to be fixed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants