Skip to content

fix: ensure patch utility is gnu version#258

Closed
curbengh wants to merge 1 commit intosnyk:masterfrom
curbengh:check-gnu-patch
Closed

fix: ensure patch utility is gnu version#258
curbengh wants to merge 1 commit intosnyk:masterfrom
curbengh:check-gnu-patch

Conversation

@curbengh
Copy link

@curbengh curbengh commented Oct 31, 2018

  • Ready for review
  • Follows CONTRIBUTING rules
  • Reviewed by Snyk internal team

What does this PR do?

This pull adds a check to test whether patch binary is GNU version and outputs an error message if it's not.

How should this be manually tested?

  1. Use alpine as CI build environment
  2. Install a patch-able node package (e.g. renovate).
  3. snyk protect
  4. There should be error message Snyk couldn't patch the specified vulnerabilities because GNU's patch is not available. Please install 'patch' and try again..
  5. Repeat 1-4 again, but this time with GNU patch installed apk add patch.

Current behavior

With this pull

Any background context you want to provide?

Alpine and *BSD ships with BusyBox and BSD versions of patch respectively, which don't support the arguments (i.e. --backup in BusyBox and --verbose in BSD) used by Synk.

This pull helps those OS users to locate the issue, even without --debug.

What are the relevant tickets?

Closes #108 @welwood08

Additional questions

  1. This comment mention Snyk ships with patch binary. Please clarify.
  2. hasbin() can be replaced by if(error) in exec(). Shall I remove it?
  3. Is test case required? not sure what the following line does currently,
    https://github.com/snyk/snyk/blob/c98b8b39779410f70027eab31a97dd910994f332/test/patch-ensure-patch.js#L33

@CLAassistant
Copy link

CLAassistant commented Oct 31, 2018

CLA assistant check
All committers have signed the CLA.

@adrukh
Copy link
Contributor

adrukh commented Feb 18, 2019

@weyusi please check a release newer than 1.131.0.
This should not be an issue now that we've moved away from using the binary patch to apply patches.

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