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

Fix: handle broken symlinks #1329

Merged
merged 2 commits into from
Aug 20, 2020
Merged

Fix: handle broken symlinks #1329

merged 2 commits into from
Aug 20, 2020

Conversation

lili2311
Copy link
Contributor

@lili2311 lili2311 commented Aug 13, 2020

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

What does this PR do?

Fixes an edge case when using "snyk test --all-projects" in a project that contains a broken symlink. (Currently it fails catastrophically.)

Where should the reviewer start?

Small change to src/lib/find-files.ts

How should this be manually tested?

$ ln -s /path-that-does-not-exist broken-symlink
$ node dist/cli/index.js test --all-projects

@clintonherget
Copy link
Contributor

Superseded by #1335 which should allow existing tests to pass. :)

This one can be closed.

@lili2311 lili2311 force-pushed the fix/handle-broken-symlinks branch 4 times, most recently from f51e5f4 to 55124b1 Compare August 17, 2020 11:57
gitphill
gitphill previously approved these changes Aug 17, 2020
@lili2311 lili2311 force-pushed the fix/handle-broken-symlinks branch 18 times, most recently from 0dd22bb to 422f591 Compare August 19, 2020 10:45
@lili2311 lili2311 force-pushed the fix/handle-broken-symlinks branch 3 times, most recently from 97c5acf to 5993d1e Compare August 19, 2020 11:08
@lili2311 lili2311 force-pushed the fix/handle-broken-symlinks branch 8 times, most recently from e395c7c to 9348dc1 Compare August 19, 2020 16:23
Add intentionally broken symlink to find-files test fixtures.
This will break existing find-files.ts tests)
@lili2311 lili2311 force-pushed the fix/handle-broken-symlinks branch 2 times, most recently from e5daeb4 to 0243e8e Compare August 19, 2020 16:36
@lili2311 lili2311 requested a review from gitphill August 19, 2020 17:50
@lili2311 lili2311 dismissed gitphill’s stale review August 19, 2020 17:51

the fix changed, please re-review

@@ -3,7 +3,7 @@ import * as pathLib from 'path';
import * as _ from '@snyk/lodash';
import { detectPackageManagerFromFile } from './detect';
import * as debugModule from 'debug';
const debug = debugModule('snyk');
const debug = debugModule('snyk:find-files');
Copy link
Contributor

Choose a reason for hiding this comment

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

I really like this, we should do this for more debugs

@lili2311 lili2311 changed the title Fix/handle broken symlinks Fix: handle broken symlinks Aug 20, 2020
do not fail catastrophically on broken symlinks in project folder when using --all-projects
@github-actions
Copy link
Contributor

github-actions bot commented Aug 20, 2020

Expected release notes (by @lili2311)

fixes:
skip broken symlinks in find-files (cdffdc3)
add intentionally broken symlink to find-files fixtures (811af3f)

  • I hereby acknowledge these release notes are 🥙 AWESOME 🥙

@lili2311 lili2311 merged commit a485fb2 into master Aug 20, 2020
@lili2311 lili2311 deleted the fix/handle-broken-symlinks branch August 20, 2020 11:01
@snyksec
Copy link

snyksec commented Aug 20, 2020

🎉 This PR is included in version 1.381.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

5 participants