From 3ac5b5c58dfbecea88bf532ef35f829ee0c0fc16 Mon Sep 17 00:00:00 2001 From: kyegupov Date: Mon, 10 Jun 2019 12:36:32 +0100 Subject: [PATCH] fix: support both Gradle 2.x and configuration attributes in Gradle 3+ (#80) --- .travis.yml | 3 + lib/index.ts | 55 ++++++++++--------- lib/init.gradle | 45 ++++++++++++--- test/fixtures/api-configuration/build.gradle | 4 +- .../multi-config-attributes/build.gradle | 40 ++++++++++---- test/fixtures/multi-config/build.gradle | 12 ++-- .../multi-project/subproj/build.gradle | 10 ++-- test/system/failure-states.test.ts | 2 +- test/system/plugin.test.ts | 41 ++++++++------ 9 files changed, 138 insertions(+), 74 deletions(-) diff --git a/.travis.yml b/.travis.yml index d8d7ab8..b9966ac 100644 --- a/.travis.yml +++ b/.travis.yml @@ -12,6 +12,9 @@ node_js: - "8" - "6" env: + - GRADLE_VERSION=2.14 JAVA_VERSION=openjdk8 + # java 9 and gradle 2 not supported + - GRADLE_VERSION=3.5.1 JAVA_VERSION=openjdk8 # java 9 and gradle 3 not supported diff --git a/lib/index.ts b/lib/index.ts index 58a7a30..a778637 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -286,45 +286,59 @@ function leftPad(s: string, n: number) { return ' '.repeat(Math.max(n - s.length, 0)) + s; } -async function getAllDeps(root, targetFile, options: SingleRootInspectOptions | MultiRootsInspectOptions): - Promise { - - let initGradlePath: string | null = null; +async function getInjectedScriptPath(): Promise<{injectedScripPath: string, cleanupCallback?: () => void}> { + let initGradleAsset: string | null = null; if (/index.js$/.test(__filename)) { // running from ./dist - initGradlePath = path.join(__dirname, '../lib/init.gradle'); + // path.join call has to be exactly in this format, needed by "pkg" to build a standalone Snyk CLI binary: + // https://www.npmjs.com/package/pkg#detecting-assets-in-source-code + initGradleAsset = path.join(__dirname, '../lib/init.gradle'); } else if (/index.ts$/.test(__filename)) { // running from ./lib - initGradlePath = path.join(__dirname, 'init.gradle'); + initGradleAsset = path.join(__dirname, 'init.gradle'); } else { throw new Error('Cannot locate Snyk init.gradle script'); } - const args = buildArgs( - root, targetFile, initGradlePath, - options); - // We could be running from a bundled CLI generated by `pkg`. // The Node filesystem in that case is not real: https://github.com/zeit/pkg#snapshot-filesystem // Copying the injectable script into a temp file. - let tmpInitGradle: tmp.SynchrounousResult | null = null; try { - tmpInitGradle = tmp.fileSync({postfix: '-init.gradle'}); - await fs.createReadStream(initGradlePath).pipe(fs.createWriteStream('', {fd: tmpInitGradle!.fd})); - initGradlePath = tmpInitGradle.name; + const tmpInitGradle = tmp.fileSync({postfix: '-init.gradle'}); + fs.createReadStream(initGradleAsset).pipe(fs.createWriteStream('', {fd: tmpInitGradle!.fd})); + return { injectedScripPath: tmpInitGradle.name, cleanupCallback: tmpInitGradle.removeCallback }; } catch (error) { error.message = error.message + '\n\n' + 'Failed to create a temporary file to host Snyk init script for Gradle build analysis.'; throw error; } +} + +async function getAllDeps(root, targetFile, options: SingleRootInspectOptions | MultiRootsInspectOptions): + Promise { const command = getCommand(root, targetFile); + + let gradleVersionOutput = '[COULD NOT RUN gradle -v] '; + try { + gradleVersionOutput = await subProcess.execute(command, ['-v'], {cwd: root}); + } catch (_) { + // intentionally empty + } + + if (gradleVersionOutput.match(/Gradle 1/)) { + throw new Error('Gradle 1.x is not supported'); + } + + const { injectedScripPath, cleanupCallback } = await getInjectedScriptPath(); + const args = buildArgs(root, targetFile, injectedScripPath, options); + const fullCommandText = 'gradle command: ' + command + ' ' + args.join(' '); debugLog('Executing ' + fullCommandText); try { const stdoutText = await subProcess.execute(command, args, {cwd: root}, printIfEcho); - if (tmpInitGradle !== null) { - tmpInitGradle.removeCallback(); + if (cleanupCallback) { + cleanupCallback(); } return extractJsonFromScriptOutput(stdoutText); } catch (error0) { @@ -332,15 +346,6 @@ async function getAllDeps(root, targetFile, options: SingleRootInspectOptions | const gradleErrorMarkers = /^\s*>\s.*$/; const gradleErrorEssence = error.message.split('\n').filter((l) => gradleErrorMarkers.test(l)).join('\n'); - // It'd be nice to set it in the inner catch{} block below. - // However, it's not safe: the inner catch{} will be executed even it inner try{} - // succeeds. Seems like an async/await implementation problem. - let gradleVersionOutput = '[COULD NOT RUN gradle -v] '; - try { - gradleVersionOutput = await subProcess.execute(command, ['-v'], {cwd: root}); - } catch (_) { - // intentionally empty - } const orange = chalk.rgb(255, 128, 0); const blackOnYellow = chalk.bgYellowBright.black; gradleVersionOutput = orange(gradleVersionOutput); diff --git a/lib/init.gradle b/lib/init.gradle index 1f5ec51..3764b2f 100644 --- a/lib/init.gradle +++ b/lib/init.gradle @@ -2,6 +2,9 @@ import groovy.json.JsonOutput import java.util.regex.Pattern import java.util.regex.Matcher +// Snyk dependency resolution script for Gradle. +// Tested on Gradle versions from 2.14 to 5.4.1 + // This script does the following: for all the projects in the build file, // generate a merged configuration of all the available configurations, // and then list the dependencies as a tree. @@ -16,7 +19,9 @@ import java.util.regex.Matcher // (-q to have clean output, -P supplies args as per https://stackoverflow.com/a/48370451) -// confAttr is a comma-separated list of key:value pairs. The "key" is a case-insensitive substring +// confAttr parameter (supported only in Gradle 3+) is used to perform attribute-based dependency variant matching +// (important for Android: https://developer.android.com/studio/build/dependencies#variant_aware) +// Its value is a comma-separated list of key:value pairs. The "key" is a case-insensitive substring // of the class name of the attribute (e.g. "buildtype" would match com.android.build.api.attributes.BuildTypeAttr), // the value should be a case-insensitive stringified value of the attribute @@ -72,16 +77,27 @@ allprojects { everyProj -> depsToDict = { Iterable deps, Set currentChain -> def res = [:] deps.each { d -> - def depName = "$d.moduleGroup:$d.moduleName" + def depName = d.moduleGroup + ':' + d.moduleName if (!currentChain.contains(depName)) { - def row = ['name': depName, 'version': d.moduleVersion] + def row = ['name': depName, 'version': d.moduleVersion, 'conf': d.configuration] currentChain.add(depName) def subDeps = depsToDict(d.children, currentChain) currentChain.remove(depName) if (subDeps.size() > 0) { row['dependencies'] = subDeps } - res[row['name']] = row + // In Gradle 2, there can be several instances of the same dependency present at each level, + // each for a different configuration. In this case, we need to merge the dependencies. + if (res.containsKey(depName)) { + if (subDeps.size() > 0) { + if (!res[depName].containsKey('dependencies')) { + res[depName]['dependencies'] = [:] + } + res[depName]['dependencies'].putAll(subDeps) + } + } else { + res[depName] = row + } } } return res @@ -89,6 +105,10 @@ allprojects { everyProj -> def matchesAttributeFilter matchesAttributeFilter = { conf -> + if (!conf.hasProperty('attributes')) { + // Gradle before version 3 does not support attributes + return true + } def matches = true def attrs = conf.attributes attrs.keySet().each({ attr -> @@ -136,6 +156,10 @@ allprojects { everyProj -> def attributesAsStrings = [:] // Map> rootProject.allprojects.findAll(shouldScanProject).each { proj -> proj.configurations.findAll({ it.name != 'snykMergedDepsConf' && it.name =~ confNameFilter && matchesAttributeFilter(it) }).each { conf -> + if (!conf.hasProperty('attributes')) { + // Gradle before version 3 does not support attributes + return + } def attrs = conf.attributes attrs.keySet().each({ attr -> def value = attrs.getAttribute(attr) @@ -184,11 +208,14 @@ allprojects { everyProj -> mergeableConfs.each { snykConf.extendsFrom(it) } // Copy all the unambiguous build attributes into the merged configuration - allConfigurationAttributes.each({ attr, valueSet -> - if (valueSet.size() == 1) { - snykConf.attributes.attribute(attr, valueSet.head()) - } - }) + // Gradle before version 3 does not support attributes + if (snykConf.hasProperty('attributes')) { + allConfigurationAttributes.each({ attr, valueSet -> + if (valueSet.size() == 1) { + snykConf.attributes.attribute(attr, valueSet.head()) + } + }) + } } if (snykConf != null) { println('SNYKECHO resolving configuration ' + snykConf.name) diff --git a/test/fixtures/api-configuration/build.gradle b/test/fixtures/api-configuration/build.gradle index 91751fa..4d1938a 100644 --- a/test/fixtures/api-configuration/build.gradle +++ b/test/fixtures/api-configuration/build.gradle @@ -1,4 +1,6 @@ -apply plugin: 'java-library' +configurations { + api +} repositories { mavenCentral() diff --git a/test/fixtures/multi-config-attributes/build.gradle b/test/fixtures/multi-config-attributes/build.gradle index f8fec83..255931e 100644 --- a/test/fixtures/multi-config-attributes/build.gradle +++ b/test/fixtures/multi-config-attributes/build.gradle @@ -11,19 +11,37 @@ repositories { } // See https://docs.gradle.org/current/userguide/dependency_management_attribute_based_matching.html -def usageAttr = Attribute.of("org.gradle.usage", Usage) -configurations { - apiConf { - attributes { - attribute(usageAttr, project.objects.named(Usage, "java-api")) - } - } - runtimeConf { - attributes { - attribute(usageAttr, project.objects.named(Usage, "java-runtime")) - } +if (project.hasProperty('objects')) { + def usageAttr = Attribute.of("org.gradle.usage", Usage) + // Gradle 4+ + configurations { + apiConf { + attributes { + attribute(usageAttr, project.objects.named(Usage, "java-api")) + } + } + runtimeConf { + attributes { + attribute(usageAttr, project.objects.named(Usage, "java-runtime")) + } + } } +} else { + // Gradle 3 + def usageAttr = Attribute.of('usage', String) + configurations { + apiConf { + attributes { + attribute(usageAttr, "java-api") + } + } + runtimeConf { + attributes { + attribute(usageAttr, "java-runtime") + } + } + } } dependencies { diff --git a/test/fixtures/multi-config/build.gradle b/test/fixtures/multi-config/build.gradle index 9896955..97b96ca 100644 --- a/test/fixtures/multi-config/build.gradle +++ b/test/fixtures/multi-config/build.gradle @@ -11,11 +11,13 @@ repositories { } dependencies { - compile 'com.google.guava:guava:18.0' - compile 'batik:batik-dom:1.6' - compile 'commons-discovery:commons-discovery:0.2' - compile 'axis:axis:1.3' - compile 'com.android.tools.build:builder:2.3.0' + // Gradle 3+ will not pick up "compile" dependencies for "compileOnly" + // Gradle 2 will, so for configuration-matching tests we use "runtime" + runtime 'com.google.guava:guava:18.0' + runtime 'batik:batik-dom:1.6' + runtime 'commons-discovery:commons-discovery:0.2' + runtime 'axis:axis:1.3' + runtime 'com.android.tools.build:builder:2.3.0' compileOnly 'javax.servlet:servlet-api:2.5' } diff --git a/test/fixtures/multi-project/subproj/build.gradle b/test/fixtures/multi-project/subproj/build.gradle index ff0ee43..301e7be 100644 --- a/test/fixtures/multi-project/subproj/build.gradle +++ b/test/fixtures/multi-project/subproj/build.gradle @@ -11,11 +11,13 @@ repositories { } dependencies { - compile 'com.google.guava:guava:18.0' - compile 'batik:batik-dom:1.6' - compile 'commons-discovery:commons-discovery:0.2' + // Gradle 3+ will not pick up "compile" dependencies for "compileOnly" + // Gradle 2 will, so for configuration-matching tests we use "runtime" + runtime 'com.google.guava:guava:18.0' + runtime 'batik:batik-dom:1.6' + runtime 'commons-discovery:commons-discovery:0.2' compileOnly 'axis:axis:1.3' - compile 'com.android.tools.build:builder:2.3.0' + runtime 'com.android.tools.build:builder:2.3.0' } task sourcesJar(type: Jar, dependsOn: classes) { diff --git a/test/system/failure-states.test.ts b/test/system/failure-states.test.ts index f2ac677..f7b4750 100644 --- a/test/system/failure-states.test.ts +++ b/test/system/failure-states.test.ts @@ -21,7 +21,7 @@ test('failing inspect()', async (t) => { } catch (error) { t.match(error.message, 'Please ensure you are calling the `snyk` command with correct arguments', 'proper error message'); - t.match(error.message, /Gradle \d+\.\d+\.\d+/, + t.match(error.message, /Gradle \d+\.\d+(\.\d+)?/, 'the error message has Gradle version'); } }); diff --git a/test/system/plugin.test.ts b/test/system/plugin.test.ts index 3167f6d..1587dbe 100644 --- a/test/system/plugin.test.ts +++ b/test/system/plugin.test.ts @@ -1,8 +1,8 @@ import * as path from 'path'; import {fixtureDir} from '../common'; import {test} from 'tap'; - import {inspect} from '../../lib'; +import * as subProcess from '../../lib/sub-process'; const rootNoWrapper = fixtureDir('no wrapper'); @@ -87,20 +87,25 @@ test('multi-confg: only deps for specified conf are picked up (using legacy CLI t.equal(Object.keys(result.package.dependencies!).length, 1, 'top level deps: 1'); }); -if (!process.env.GRADLE_VERSION || !process.env.GRADLE_VERSION!.startsWith('3.')) { - test('multi-confg: only deps for specified conf are picked up (attribute match)', async (t) => { - const result = await inspect('.', - path.join(fixtureDir('multi-config-attributes'), 'build.gradle'), - {'configuration-attributes': 'usage:java-api'}); - t.match(result.package.name, '.', - 'returned project name is not sub-project'); - t.notOk(result.package - .dependencies!['org.apache.commons:commons-lang3'], - 'no runtime dep found'); - t.equal(result.package - .dependencies!['commons-httpclient:commons-httpclient'].version, - '3.1', - 'correct version of api dep found'); - t.equal(Object.keys(result.package.dependencies!).length, 2, 'top level deps: 2'); // 1 with good attr, 1 with no attr - }); -} +test('tests for Gradle 3+', async (t0) => { + const gradleVersionOutput = await subProcess.execute('gradle', ['-v'], {}); + const isGradle3Plus = parseInt(gradleVersionOutput.match(/Gradle (\d+)\.\d+(\.\d+)?/)![1]) >= 3; + if (isGradle3Plus) { + t0.test('multi-confg: only deps for specified conf are picked up (attribute match)', async (t) => { + + const result = await inspect('.', + path.join(fixtureDir('multi-config-attributes'), 'build.gradle'), + {'configuration-attributes': 'usage:java-api'}); + t.match(result.package.name, '.', + 'returned project name is not sub-project'); + t.notOk(result.package + .dependencies!['org.apache.commons:commons-lang3'], + 'no runtime dep found'); + t.equal(result.package + .dependencies!['commons-httpclient:commons-httpclient'].version, + '3.1', + 'correct version of api dep found'); + t.equal(Object.keys(result.package.dependencies!).length, 2, 'top level deps: 2'); // 1 with good attr, 1 with no attr + }); + } +})