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

feat: enable cocoapods for --all-projects scanning #965

Merged
merged 1 commit into from
Jan 27, 2020

Conversation

lili2311
Copy link
Contributor

@lili2311 lili2311 commented Jan 22, 2020

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

What does this PR do?

Enable Cocoapods Podfile and Podfile.lock to be picked up when running test or monitor with --all-projects flag

@lili2311 lili2311 requested a review from a team as a code owner January 22, 2020 17:22
@ghost ghost requested review from aviadhahami and dkontorovskyy January 22, 2020 17:22
@lili2311 lili2311 force-pushed the feat/cocopoads-all-projects branch 2 times, most recently from 9a31bab to 3190f26 Compare January 22, 2020 18:06
} else if (options.docker) {
analysisType = 'docker';
} else {
packageManager = detect.detectPackageManager(path, options);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

only run this check if we are not in Docker or all-projects mode as it will fail when no supported manifests are found

@lili2311 lili2311 self-assigned this Jan 22, 2020
@lili2311 lili2311 force-pushed the feat/cocopoads-all-projects branch 4 times, most recently from a111b99 to 5432468 Compare January 23, 2020 12:47
@lili2311
Copy link
Contributor Author

Includes the PR #966

@lili2311 lili2311 force-pushed the feat/cocopoads-all-projects branch 3 times, most recently from 187b2b1 to 740236d Compare January 27, 2020 10:26
@@ -159,8 +159,10 @@ async function runTest(
res.filesystemPolicy = !!payloadPolicy;
if (!options['ignore-policy']) {
res.policy = res.policy || (payloadPolicy as string);
console.log('***** res.policy', res.policy);
Copy link
Contributor

Choose a reason for hiding this comment

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

It this a left over?

const policy = await snyk.policy.loadFromText(res.policy);
res = policy.filter(res, root);
console.log('***** ', policy);
Copy link
Contributor

Choose a reason for hiding this comment

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

It this a left over?

When running `snyk test --all-projects`
by default pick up also `Podfile` and `Podfile.lock`

Supported for `test` and `monitor`
@lili2311 lili2311 force-pushed the feat/cocopoads-all-projects branch from 740236d to 5851a7f Compare January 27, 2020 12:39
@@ -193,6 +193,12 @@ function chooseBestManifest(
)[0];
return defaultManifest.path;
}
case 'cocoapods': {
const defaultManifest = files.filter((path) =>
['Podfile'].includes(path.base),
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a blocker, just a question why not 'Podfile' === path.base?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no reason copy + 🍝, will update in the next PR

Copy link
Contributor

Choose a reason for hiding this comment

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

Because it's only relevant for npm and yarn and we bring it for all of others. We will have refactor after we release everything I think. I bet we could simplify when more ecosystems will be supported with --all-projects

Copy link
Contributor Author

@lili2311 lili2311 Jan 27, 2020

Choose a reason for hiding this comment

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

It is relevant for any ecosystem where more than 1 manifest file can be detected in the same folder and we need to choose. Example: requiremens.txt or Pipfile yarn.lock or package-lock.json.
Technically we could only look for Podfile here and remove this entry, but there are other cocopoads manifests we may auto detect later on which are currently supported via --file

Copy link
Contributor

@dkontorovskyy dkontorovskyy left a comment

Choose a reason for hiding this comment

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

LGTM

@lili2311 lili2311 merged commit 7e30ef6 into master Jan 27, 2020
@lili2311 lili2311 deleted the feat/cocopoads-all-projects branch January 27, 2020 14:03
@snyksec
Copy link

snyksec commented Jan 27, 2020

🎉 This PR is included in version 1.282.0 🎉

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
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants