From 6a94fc467c0cdd28648dd5ebe244fc42fdebb93c Mon Sep 17 00:00:00 2001 From: Lars Kissel Date: Fri, 9 Jul 2021 15:18:23 +0200 Subject: [PATCH 1/9] [FEATURE] Support to only build certain dependencies Projects can be built with a selection of dependencies that have to be included into the build result. The following CLI parameters can be used flexibly to configure the selection of the dependencies to be built: --include-dependency, --include-dependency-regexp, --include-dependency-tree, --exclude-dependency, --exclude-dependency-regexp, --exclude-dependency-tree JIRA: CPOUI5FOUNDATION-208 --- lib/cli/commands/build.js | 55 +++++++++- lib/utils/buildHelper.js | 104 ++++++++++++++++++ test/lib/cli/commands/build.js | 192 +++++++++++++++++++++++++++++++-- test/lib/utils/buildHelper.js | 131 ++++++++++++++++++++++ 4 files changed, 475 insertions(+), 7 deletions(-) create mode 100644 lib/utils/buildHelper.js create mode 100644 test/lib/utils/buildHelper.js diff --git a/lib/cli/commands/build.js b/lib/cli/commands/build.js index 9c5bf6c7..cb4b8f80 100644 --- a/lib/cli/commands/build.js +++ b/lib/cli/commands/build.js @@ -1,6 +1,7 @@ // Build const baseMiddleware = require("../middlewares/base.js"); +const buildHelper = require("../../utils/buildHelper"); const build = { command: "build", @@ -37,6 +38,30 @@ build.builder = function(cli) { default: false, type: "boolean" }) + .option("include-dependency", { + describe: "A list of dependencies to be included into build process", + type: "array" + }) + .option("include-dependency-regexp", { + describe: "A list of regular expressions defining dependencies to be included into build process", + type: "array" + }) + .option("include-dependency-tree", { + describe: "A list of dependencies to be included into build process including their sub-dependencies", + type: "array" + }) + .option("exclude-dependency", { + describe: "A list of dependencies to be excluded from build process", + type: "array" + }) + .option("exclude-dependency-regexp", { + describe: "A list of regular expressions defining dependencies to be excluded from build process", + type: "array" + }) + .option("exclude-dependency-tree", { + describe: "A list of dependencies to be excluded from build process including their sub-dependencies", + type: "array" + }) .option("dest", { describe: "Path of build destination", default: "./dist", @@ -98,11 +123,39 @@ async function handleBuild(argv) { } const tree = await normalizer.generateProjectTree(normalizerOptions); + const buildSettings = (tree.builder && tree.builder.settings) || {}; + + const includedDependencies = buildHelper.createDependencyList({ + tree: tree, dependencies: argv["include-dependency"], dependenciesRegExp: argv["include-dependency-regexp"]}); + const includedDependencyTrees = buildHelper.createDependencyList( + {tree: tree, dependencies: argv["include-dependency-tree"], handleSubtree: true}); + const excludedDependencies = buildHelper.createDependencyList({ + tree: tree, dependencies: argv["exclude-dependency"], dependenciesRegExp: argv["exclude-dependency-regexp"]}); + const excludedDependencyTrees = buildHelper.createDependencyList( + {tree: tree, dependencies: argv["exclude-dependency-tree"], handleSubtree: true}); + const defaultIncludedDependencies = buildHelper.createDependencyList({ + tree: tree, + dependencies: buildSettings.includeDependency, + dependenciesRegExp: buildSettings.includeDependencyRegExp + }); + const defaultIncludedDependencyTrees = buildHelper.createDependencyList( + {tree: tree, dependencies: buildSettings.includeDependencyTree, handleSubtree: true}); + + buildHelper.mergeDependencyLists(includedDependencies, includedDependencyTrees, excludedDependencies); + buildHelper.mergeDependencyLists(excludedDependencies, excludedDependencyTrees, includedDependencies); + + Array.prototype.push.apply(defaultIncludedDependencies, defaultIncludedDependencyTrees); + buildHelper.mergeDependencyLists(includedDependencies, defaultIncludedDependencies, excludedDependencies); + + const buildAll = buildHelper.alignWithBuilderAPI(argv.all, includedDependencies, excludedDependencies); + await builder.build({ tree: tree, destPath: argv.dest, cleanDest: argv["clean-dest"], - buildDependencies: argv.all, + buildDependencies: buildAll, + includedDependencies: includedDependencies, + excludedDependencies: excludedDependencies, dev: command === "dev", selfContained: command === "self-contained", jsdoc: command === "jsdoc", diff --git a/lib/utils/buildHelper.js b/lib/utils/buildHelper.js new file mode 100644 index 00000000..58051309 --- /dev/null +++ b/lib/utils/buildHelper.js @@ -0,0 +1,104 @@ +const log = require("@ui5/logger").getLogger("cli:commands:build"); + +/** + * Creates a list of dependencies as String or RegExp by processing the given dependencies and + * dependenciesRegExp parameters. Dependencies are only added to the list if they exist in the + * project tree. Optionally, also the sub-dependencies of given dependencies are + * added to the list. + * + * @param {object} parameters Parameters + * @param {object} parameters.tree Project tree as generated by the + * [@ui5/project.normalizer]{@link module:@ui5/project.normalizer} + * @param {Array.} parameters.dependencies The dependencies to be considered + * @param {Array.} [parameters.dependenciesRegExp] The dependencies to be considered as new + * RegExp instances + * @param {boolean} [parameters.handleSubtree] Decides whether the sub-dependencies of the given + * dependencies are included into the returned dependency list + * @returns {Array.} Array of the computed dependencies + */ +function createDependencyList({tree, dependencies = [], dependenciesRegExp = [], handleSubtree}) { + const dependencyList = []; + + function _addUnique(depName) { + if (!dependencyList.includes(depName)) { + dependencyList.push(depName); + } + } + function _processSubtree(project) { + project.dependencies.forEach((project0) => { + _addUnique(project0.metadata.name); + _processSubtree(project0); + }); + } + + dependencies.forEach((depName) => { + if (depName === "*") { + return _addUnique(depName); + } + const project = tree.dependencies.find((dep) => dep.metadata.name === depName); + if (project) { + _addUnique(depName); + if (handleSubtree) { + _processSubtree(project); + } + } else { + log.warn(`Could not find dependency '${depName}' for '${tree.metadata.name}'.`); + } + }); + + return dependencyList.concat(dependenciesRegExp.map((exp) => new RegExp(exp))); +} + +/** + * Merges a dependencyList into a targetDependencyList without duplicates. Values + * given in excludeList are skipped from this operation. + * + * @param {Array.} targetDependencyList The target list + * @param {Array.} dependencyList The source list + * @param {Array.} excludeList The list of values to be excluded from the merge operation + */ +function mergeDependencyLists(targetDependencyList, dependencyList, excludeList) { + function isDepIncluded(depList, depName) { + return depList.some((dep) => dep instanceof RegExp ? dep.test(depName) : dep === depName); + } + dependencyList.forEach((dep) => { + if (!isDepIncluded(targetDependencyList, dep) && !isDepIncluded(excludeList, dep)) { + targetDependencyList.push(dep); + } + }); +} + +/** + * Returns whether project dependencies have to be built influenced by includedDependencies and + * excludedDependencies. + * In case not all but selected dependencies (via includedDependencies) have to be built, the + * "*" sign is added to the excludedDependencies to make sure that all other dependencies are + * excluded. + * In case a "*" sign is included in includedDependencies, the "*" is removed and the + * buildAll flag is set to true because it behaves as an alias. + * + * @param {boolean} buildAll The value of the all command line parameter to decide if project + * dependencies have to be built + * @param {Array.} includedDependencies The list of included dependencies + * @param {Array.} excludedDependencies The list of excluded dependencies + * @returns {boolean} Whether it is required to build project dependencies + */ +function alignWithBuilderAPI(buildAll, includedDependencies, excludedDependencies) { + if ((!buildAll && !includedDependencies.includes("*")) && includedDependencies.length) { + excludedDependencies.push("*"); + } + if (includedDependencies.includes("*")) { + buildAll = true; + includedDependencies.splice(includedDependencies.indexOf("*"), 1); + } + if (!buildAll && includedDependencies.length) { + buildAll = true; + } + return buildAll; +} + +module.exports = { + createDependencyList, + mergeDependencyLists, + alignWithBuilderAPI +}; diff --git a/test/lib/cli/commands/build.js b/test/lib/cli/commands/build.js index 2b4fffb9..4a8c2cce 100644 --- a/test/lib/cli/commands/build.js +++ b/test/lib/cli/commands/build.js @@ -1,6 +1,6 @@ const test = require("ava"); const sinon = require("sinon"); -const build = require("../../../../lib/cli/commands/build"); +const mock = require("mock-require"); const normalizer = require("@ui5/project").normalizer; const builder = require("@ui5/builder").builder; const logger = require("@ui5/logger"); @@ -22,6 +22,8 @@ const defaultBuilderArgs = { }, destPath: "./dist", buildDependencies: undefined, + includedDependencies: [], + excludedDependencies: [], cleanDest: undefined, dev: false, selfContained: false, @@ -35,10 +37,17 @@ test.beforeEach((t) => { normalizerStub = sinon.stub(normalizer, "generateProjectTree"); builderStub = sinon.stub(builder, "build").returns(Promise.resolve()); sinon.stub(logger, "setShowProgress"); + t.context.log = { + warn: sinon.stub() + }; + sinon.stub(logger, "getLogger").withArgs("cli:commands:build").returns(t.context.log); + mock.reRequire("../../../../lib/utils/buildHelper"); // rerequire buildHelper to ensure usage of stubbed logger + t.context.build = mock.reRequire("../../../../lib/cli/commands/build"); }); test.afterEach.always((t) => { sinon.restore(); + mock.stopAll(); }); test.serial("ui5 build (default) ", async (t) => { @@ -49,7 +58,7 @@ test.serial("ui5 build (default) ", async (t) => { } }); args._ = ["build"]; - await build.handler(args); + await t.context.build.handler(args); t.deepEqual(builderStub.getCall(0).args[0], defaultBuilderArgs, "default build triggered with expected arguments"); }); @@ -61,7 +70,7 @@ test.serial("ui5 build dev", async (t) => { } }); args._ = ["build", "dev"]; - await build.handler(args); + await t.context.build.handler(args); t.deepEqual( builderStub.getCall(0).args[0], Object.assign({}, defaultBuilderArgs, {dev: true}), @@ -77,7 +86,7 @@ test.serial("ui5 build self-contained", async (t) => { } }); args._ = ["build", "self-contained"]; - await build.handler(args); + await t.context.build.handler(args); t.deepEqual( builderStub.getCall(0).args[0], Object.assign({}, defaultBuilderArgs, {selfContained: true}), @@ -93,7 +102,7 @@ test.serial("ui5 build jsdoc", async (t) => { } }); args._ = ["build", "jsdoc"]; - await build.handler(args); + await t.context.build.handler(args); t.deepEqual( builderStub.getCall(0).args[0], Object.assign({}, defaultBuilderArgs, {jsdoc: true}), @@ -111,7 +120,7 @@ test.serial("ui5 build --framework-version 1.99", async (t) => { args._ = ["build"]; args.frameworkVersion = "1.99.0"; - await build.handler(args); + await t.context.build.handler(args); t.deepEqual( normalizerStub.getCall(0).args[0], { @@ -123,3 +132,174 @@ test.serial("ui5 build --framework-version 1.99", async (t) => { }, "generateProjectTree got called with expected arguments" ); }); + +async function assertIncludeDependency(t, { + treeDeps, includeDeps, includeDepsRegExp, includeDepsTree, excludeDeps, expectedBuilderArgs +}) { + const tree = Object.assign({metadata: {name: "Sample"}}, treeDeps); + const _args = Object.assign({}, args); // copy default args to ensure it is not modified + normalizerStub.resolves(tree); + _args._ = ["build"]; + _args["include-dependency"] = includeDeps; + _args["include-dependency-regexp"] = includeDepsRegExp; + _args["include-dependency-tree"] = includeDepsTree; + _args["exclude-dependency"] = excludeDeps; + await t.context.build.handler(_args); + t.deepEqual(builderStub.getCall(0).args[0], + Object.assign({}, defaultBuilderArgs, {tree: tree}, expectedBuilderArgs), + "default build triggered with expected arguments"); +} + +test.serial("ui5 build --include-dependency", async (t) => { + await assertIncludeDependency(t, { + treeDeps: { + dependencies: [{ + metadata: { + name: "sap.ui.core" + } + }] + }, + includeDeps: ["sap.ui.core"], + expectedBuilderArgs: { + buildDependencies: true, + includedDependencies: ["sap.ui.core"], + excludedDependencies: ["*"] + } + }); +}); + +test.serial("ui5 build (dependency included via ui5.yaml)", async (t) => { + await assertIncludeDependency(t, { + treeDeps: { + dependencies: [{ + metadata: { + name: "sap.ui.core" + } + }], + builder: { + settings: { + includeDependency: ["sap.ui.core"] + } + } + }, + expectedBuilderArgs: { + buildDependencies: true, + includedDependencies: ["sap.ui.core"], + excludedDependencies: ["*"] + } + }); +}); + +test.serial("ui5 build --include-dependency-regexp", async (t) => { + await assertIncludeDependency(t, { + treeDeps: { + dependencies: [{ + metadata: { + name: "sap.ui.core" + } + }] + }, + includeDepsRegExp: ["^sap.[mf]$"], + expectedBuilderArgs: { + buildDependencies: true, + includedDependencies: [/^sap.[mf]$/], + excludedDependencies: ["*"] + } + }); +}); + +test.serial("ui5 build --include-dependency-tree", async (t) => { + await assertIncludeDependency(t, { + treeDeps: { + dependencies: [{ + metadata: { + name: "a" + }, + dependencies: [{ + metadata: { + name: "b0" + }, + dependencies: [] + }, { + metadata: { + name: "b1" + }, + dependencies: [{ + metadata: { + name: "c" + }, + dependencies: [] + }] + }] + }] + }, + includeDepsTree: ["a"], + expectedBuilderArgs: { + buildDependencies: true, + includedDependencies: ["a", "b0", "b1", "c"], + excludedDependencies: ["*"] + } + }); +}); + +test.serial("ui5 build --include-dependency=* --exclude-dependency=sap.ui.core", async (t) => { + await assertIncludeDependency(t, { + treeDeps: { + dependencies: [{ + metadata: { + name: "sap.ui.core" + } + }] + }, + includeDeps: ["*"], + excludeDeps: ["sap.ui.core"], + expectedBuilderArgs: { + buildDependencies: true, + includedDependencies: [], + excludedDependencies: ["sap.ui.core"] + } + }); +}); + +test.serial("ui5 build --include-dependency-tree=a --exclude-dependency=a", async (t) => { + await assertIncludeDependency(t, { + treeDeps: { + dependencies: [{ + metadata: { + name: "a" + }, + dependencies: [{ + metadata: { + name: "b0" + }, + dependencies: [] + }, { + metadata: { + name: "b1" + }, + dependencies: [] + }] + }] + }, + includeDepsTree: ["a"], + excludeDeps: ["a"], + expectedBuilderArgs: { + buildDependencies: true, + includedDependencies: ["b0", "b1"], + excludedDependencies: ["a", "*"] + } + }); +}); + +test.serial("ui5 build --include-dependency (dependency not found)", async (t) => { + const {log} = t.context; + await assertIncludeDependency(t, { + treeDeps: { + dependencies: [] + }, + includeDeps: ["sap.ui.core"] + }); + t.is(log.warn.callCount, 1, "log.warn should be called once"); + t.deepEqual(log.warn.getCall(0).args, ["Could not find dependency 'sap.ui.core' for 'Sample'."], + "logger.warn should be called with expected string"); +}); diff --git a/test/lib/utils/buildHelper.js b/test/lib/utils/buildHelper.js new file mode 100644 index 00000000..6dc99feb --- /dev/null +++ b/test/lib/utils/buildHelper.js @@ -0,0 +1,131 @@ +const test = require("ava"); +const sinon = require("sinon"); +const mock = require("mock-require"); +const logger = require("@ui5/logger"); + +test.beforeEach((t) => { + t.context.log = { + warn: sinon.stub() + }; + sinon.stub(logger, "getLogger").withArgs("cli:commands:build").returns(t.context.log); + t.context.buildHelper = mock.reRequire("../../../lib/utils/buildHelper"); +}); + +test.afterEach.always((t) => { + sinon.restore(); + mock.stopAll(); +}); + +test.serial("createDependencyList", (t) => { + const {createDependencyList} = t.context.buildHelper; + const tree = { + metadata: { + name: "Sample" + }, + dependencies: [{ + metadata: { + name: "a" + } + }, { + metadata: { + name: "b" + } + }] + }; + const deps = ["*", "a", "b", "b", "c"]; + const depsRegExp = ["^abc$"]; + + t.deepEqual(createDependencyList({tree: tree, dependencies: deps, dependenciesRegExp: depsRegExp}), + ["*", "a", "b", /^abc$/]); + t.is(t.context.log.warn.callCount, 1, "log.warn should be called once"); + t.deepEqual(t.context.log.warn.getCall(0).args, ["Could not find dependency 'c' for 'Sample'."], + "logger.warn should be called with expected string"); +}); + +test.serial("createDependencyList: handleSubtree", (t) => { + const {createDependencyList} = t.context.buildHelper; + const tree = { + metadata: { + name: "Sample" + }, + dependencies: [{ + metadata: { + name: "a" + }, + dependencies: [{ + metadata: { + name: "b" + }, + dependencies: [{ + metadata: { + name: "c" + }, + dependencies: [{ + metadata: { + name: "d0" + }, + dependencies: [] + }, { + metadata: { + name: "d1" + }, + dependencies: [] + }] + }] + }] + }] + }; + const deps = ["a", "x"]; + + t.deepEqual(createDependencyList({tree: tree, dependencies: deps, handleSubtree: true}), + ["a", "b", "c", "d0", "d1"]); + t.is(t.context.log.warn.callCount, 1, "log.warn should be called once"); + t.deepEqual(t.context.log.warn.getCall(0).args, ["Could not find dependency 'x' for 'Sample'."], + "logger.warn should be called with expected string"); +}); + +test.serial("mergeDependencyLists", (t) => { + const {mergeDependencyLists} = t.context.buildHelper; + const targetDependencyList = ["a"]; + const dependencyList = ["a", "b", "c", "d"]; + const excludeList = ["c"]; + + mergeDependencyLists(targetDependencyList, dependencyList, excludeList); + t.deepEqual(targetDependencyList, ["a", "b", "d"]); +}); + +test.serial("mergeDependencyLists: RegExp", (t) => { + const {mergeDependencyLists} = t.context.buildHelper; + const targetDependencyList = [/a/]; + const dependencyList = ["a", "b", "c", "d", /e/]; + const excludeList = [/c/, /e/]; + + mergeDependencyLists(targetDependencyList, dependencyList, excludeList); + t.deepEqual(targetDependencyList, [/a/, "b", "d"]); +}); + +test.serial("alignWithBuilderAPI: * is added to excludedDependencies", (t) => { + const {alignWithBuilderAPI} = t.context.buildHelper; + const args = { + includedDependencies: ["a"], + excludedDependencies: [] + }; + t.is(alignWithBuilderAPI(false, args.includedDependencies, args.excludedDependencies), true); + t.deepEqual(args, { + includedDependencies: ["a"], + excludedDependencies: ["*"] + }); +}); + +test.serial("alignWithBuilderAPI: includedDependencies=* is is an alias for buildAll", (t) => { + const {alignWithBuilderAPI} = t.context.buildHelper; + const args = { + includedDependencies: ["*"], + excludedDependencies: [] + }; + t.is(alignWithBuilderAPI(false, args.includedDependencies, args.excludedDependencies), true); + t.deepEqual(args, { + includedDependencies: [], + excludedDependencies: [] + }); +}); From 63f30e52b1f24c7b7a1f7360b81013de25653319 Mon Sep 17 00:00:00 2001 From: Lars Kissel Date: Tue, 13 Jul 2021 17:51:30 +0200 Subject: [PATCH 2/9] [INTERNAL] Apply suggestions from code review --- lib/cli/commands/build.js | 50 +++++--- lib/utils/buildHelper.js | 60 +++++++--- test/lib/cli/commands/build.js | 207 ++++++++++++++++++++++++++++++--- test/lib/utils/buildHelper.js | 107 ++++++++++++++--- 4 files changed, 355 insertions(+), 69 deletions(-) diff --git a/lib/cli/commands/build.js b/lib/cli/commands/build.js index cb4b8f80..c57199ce 100644 --- a/lib/cli/commands/build.js +++ b/lib/cli/commands/build.js @@ -39,27 +39,29 @@ build.builder = function(cli) { type: "boolean" }) .option("include-dependency", { - describe: "A list of dependencies to be included into build process", + describe: "A list of dependencies to be included into the build process", type: "array" }) .option("include-dependency-regexp", { - describe: "A list of regular expressions defining dependencies to be included into build process", + describe: "A list of regular expressions defining dependencies to be included into the build process", type: "array" }) .option("include-dependency-tree", { - describe: "A list of dependencies to be included into build process including their sub-dependencies", + describe: "A list of dependencies to be included into the build process; sub-dependencies are" + + " implicitly included and do not need to be part of this list", type: "array" }) .option("exclude-dependency", { - describe: "A list of dependencies to be excluded from build process", + describe: "A list of dependencies to be excluded from the build process", type: "array" }) .option("exclude-dependency-regexp", { - describe: "A list of regular expressions defining dependencies to be excluded from build process", + describe: "A list of regular expressions defining dependencies to be excluded from the build process", type: "array" }) .option("exclude-dependency-tree", { - describe: "A list of dependencies to be excluded from build process including their sub-dependencies", + describe: "A list of dependencies to be excluded from the build process; sub-dependencies are" + + " implicitly included and do not need to be part of this list", type: "array" }) .option("dest", { @@ -126,28 +128,46 @@ async function handleBuild(argv) { const buildSettings = (tree.builder && tree.builder.settings) || {}; const includedDependencies = buildHelper.createDependencyList({ - tree: tree, dependencies: argv["include-dependency"], dependenciesRegExp: argv["include-dependency-regexp"]}); - const includedDependencyTrees = buildHelper.createDependencyList( - {tree: tree, dependencies: argv["include-dependency-tree"], handleSubtree: true}); + tree: tree, + dependencies: argv["include-dependency"], + dependenciesRegExp: argv["include-dependency-regexp"] + }); + const includedDependencyTrees = buildHelper.createDependencyList({ + tree: tree, + dependencies: argv["include-dependency-tree"], + handleSubtree: true + }); const excludedDependencies = buildHelper.createDependencyList({ - tree: tree, dependencies: argv["exclude-dependency"], dependenciesRegExp: argv["exclude-dependency-regexp"]}); - const excludedDependencyTrees = buildHelper.createDependencyList( - {tree: tree, dependencies: argv["exclude-dependency-tree"], handleSubtree: true}); + tree: tree, + dependencies: argv["exclude-dependency"], + dependenciesRegExp: argv["exclude-dependency-regexp"] + }); + const excludedDependencyTrees = buildHelper.createDependencyList({ + tree: tree, + dependencies: argv["exclude-dependency-tree"], + handleSubtree: true + }); const defaultIncludedDependencies = buildHelper.createDependencyList({ tree: tree, dependencies: buildSettings.includeDependency, dependenciesRegExp: buildSettings.includeDependencyRegExp }); - const defaultIncludedDependencyTrees = buildHelper.createDependencyList( - {tree: tree, dependencies: buildSettings.includeDependencyTree, handleSubtree: true}); + const defaultIncludedDependencyTrees = buildHelper.createDependencyList({ + tree: tree, + dependencies: buildSettings.includeDependencyTree, + handleSubtree: true + }); + // Append resolved include-trees to includedDependencies, ignoring excluded projects buildHelper.mergeDependencyLists(includedDependencies, includedDependencyTrees, excludedDependencies); + // Append resolved exclude-trees to excludedDependencies, ignoring explicitly included projects buildHelper.mergeDependencyLists(excludedDependencies, excludedDependencyTrees, includedDependencies); + // Combine dependencies set in build settings and append them to includedDependencies, ignoring excluded projects Array.prototype.push.apply(defaultIncludedDependencies, defaultIncludedDependencyTrees); buildHelper.mergeDependencyLists(includedDependencies, defaultIncludedDependencies, excludedDependencies); - const buildAll = buildHelper.alignWithBuilderAPI(argv.all, includedDependencies, excludedDependencies); + const buildAll = buildHelper.alignWithBuilderApi(argv.all, includedDependencies, excludedDependencies); await builder.build({ tree: tree, diff --git a/lib/utils/buildHelper.js b/lib/utils/buildHelper.js index 58051309..618e34b1 100644 --- a/lib/utils/buildHelper.js +++ b/lib/utils/buildHelper.js @@ -1,4 +1,4 @@ -const log = require("@ui5/logger").getLogger("cli:commands:build"); +const log = require("@ui5/logger").getLogger("cli:utils:buildHelper"); /** * Creates a list of dependencies as String or RegExp by processing the given dependencies and @@ -12,18 +12,36 @@ const log = require("@ui5/logger").getLogger("cli:commands:build"); * @param {Array.} parameters.dependencies The dependencies to be considered * @param {Array.} [parameters.dependenciesRegExp] The dependencies to be considered as new * RegExp instances - * @param {boolean} [parameters.handleSubtree] Decides whether the sub-dependencies of the given + * @param {boolean} [parameters.handleSubtree=false] Decides whether the sub-dependencies of the given * dependencies are included into the returned dependency list - * @returns {Array.} Array of the computed dependencies + * @returns {Array.} Array of the computed dependencies */ function createDependencyList({tree, dependencies = [], dependenciesRegExp = [], handleSubtree}) { const dependencyList = []; + if (handleSubtree && dependenciesRegExp.length) { + throw new Error("RegExp's should not be appended to list of sub-dependencies"); + } function _addUnique(depName) { if (!dependencyList.includes(depName)) { dependencyList.push(depName); } } + function _findDependency(searchProject, depName) { + let project = searchProject.dependencies.find((dep) => dep.metadata.name === depName); + + if (!project) { + for (const project0 of searchProject.dependencies) { + const result = _findDependency(project0, depName); + if (result) { + project = result; + break; + } + } + } + + return project; + } function _processSubtree(project) { project.dependencies.forEach((project0) => { _addUnique(project0.metadata.name); @@ -35,7 +53,7 @@ function createDependencyList({tree, dependencies = [], dependenciesRegExp = [], if (depName === "*") { return _addUnique(depName); } - const project = tree.dependencies.find((dep) => dep.metadata.name === depName); + const project = _findDependency(tree, depName); if (project) { _addUnique(depName); if (handleSubtree) { @@ -51,18 +69,24 @@ function createDependencyList({tree, dependencies = [], dependenciesRegExp = [], /** * Merges a dependencyList into a targetDependencyList without duplicates. Values - * given in excludeList are skipped from this operation. + * given in excludeList are skipped. * - * @param {Array.} targetDependencyList The target list - * @param {Array.} dependencyList The source list - * @param {Array.} excludeList The list of values to be excluded from the merge operation + * @param {Array.} targetDependencyList The target list + * @param {Array.} dependencyList The source list + * @param {Array.} excludeList The list of values to be excluded from the merge operation */ function mergeDependencyLists(targetDependencyList, dependencyList, excludeList) { - function isDepIncluded(depList, depName) { + function isInList(depList, depName) { return depList.some((dep) => dep instanceof RegExp ? dep.test(depName) : dep === depName); } + function isRegExpInList(depList, depRegExp) { + return depList.some((dep) => dep instanceof RegExp && dep.toString() === depRegExp.toString()); + } dependencyList.forEach((dep) => { - if (!isDepIncluded(targetDependencyList, dep) && !isDepIncluded(excludeList, dep)) { + if ( + dep instanceof RegExp && !isRegExpInList(targetDependencyList, dep) && !isRegExpInList(excludeList, dep) || + !(dep instanceof RegExp) && !targetDependencyList.includes(dep) && !isInList(excludeList, dep) + ) { targetDependencyList.push(dep); } }); @@ -71,19 +95,19 @@ function mergeDependencyLists(targetDependencyList, dependencyList, excludeList) /** * Returns whether project dependencies have to be built influenced by includedDependencies and * excludedDependencies. - * In case not all but selected dependencies (via includedDependencies) have to be built, the - * "*" sign is added to the excludedDependencies to make sure that all other dependencies are + * If only selected dependencies (via includedDependencies) have to be built, the "*" character + * is added to the excludedDependencies to make sure that all other dependencies are * excluded. - * In case a "*" sign is included in includedDependencies, the "*" is removed and the - * buildAll flag is set to true because it behaves as an alias. + * In case a "*" character is included in includedDependencies, it is removed and the + * buildAll flag is set to true as it behaves as an alias. * * @param {boolean} buildAll The value of the all command line parameter to decide if project * dependencies have to be built - * @param {Array.} includedDependencies The list of included dependencies - * @param {Array.} excludedDependencies The list of excluded dependencies + * @param {Array.} includedDependencies The list of included dependencies + * @param {Array.} excludedDependencies The list of excluded dependencies * @returns {boolean} Whether it is required to build project dependencies */ -function alignWithBuilderAPI(buildAll, includedDependencies, excludedDependencies) { +function alignWithBuilderApi(buildAll, includedDependencies, excludedDependencies) { if ((!buildAll && !includedDependencies.includes("*")) && includedDependencies.length) { excludedDependencies.push("*"); } @@ -100,5 +124,5 @@ function alignWithBuilderAPI(buildAll, includedDependencies, excludedDependencie module.exports = { createDependencyList, mergeDependencyLists, - alignWithBuilderAPI + alignWithBuilderApi }; diff --git a/test/lib/cli/commands/build.js b/test/lib/cli/commands/build.js index 4a8c2cce..e2819c22 100644 --- a/test/lib/cli/commands/build.js +++ b/test/lib/cli/commands/build.js @@ -40,7 +40,7 @@ test.beforeEach((t) => { t.context.log = { warn: sinon.stub() }; - sinon.stub(logger, "getLogger").withArgs("cli:commands:build").returns(t.context.log); + sinon.stub(logger, "getLogger").withArgs("cli:utils:buildHelper").returns(t.context.log); mock.reRequire("../../../../lib/utils/buildHelper"); // rerequire buildHelper to ensure usage of stubbed logger t.context.build = mock.reRequire("../../../../lib/cli/commands/build"); }); @@ -134,7 +134,8 @@ test.serial("ui5 build --framework-version 1.99", async (t) => { }); async function assertIncludeDependency(t, { - treeDeps, includeDeps, includeDepsRegExp, includeDepsTree, excludeDeps, expectedBuilderArgs + treeDeps, includeDeps, includeDepsRegExp, includeDepsTree, excludeDeps, excludeDepsRegExp, excludeDepsTree, + expectedBuilderArgs }) { const tree = Object.assign({metadata: {name: "Sample"}}, treeDeps); const _args = Object.assign({}, args); // copy default args to ensure it is not modified @@ -144,6 +145,8 @@ async function assertIncludeDependency(t, { _args["include-dependency-regexp"] = includeDepsRegExp; _args["include-dependency-tree"] = includeDepsTree; _args["exclude-dependency"] = excludeDeps; + _args["exclude-dependency-regexp"] = excludeDepsRegExp; + _args["exclude-dependency-tree"] = excludeDepsTree; await t.context.build.handler(_args); t.deepEqual(builderStub.getCall(0).args[0], Object.assign({}, defaultBuilderArgs, {tree: tree}, expectedBuilderArgs), @@ -168,52 +171,142 @@ test.serial("ui5 build --include-dependency", async (t) => { }); }); -test.serial("ui5 build (dependency included via ui5.yaml)", async (t) => { +[{ + title: "no excludes", + excludeDepsRegExp: [], + excludeDepsTree: [], + expectedBuilderArgs: { + buildDependencies: true, + includedDependencies: ["a", /regexp/, "b1", "c"], + excludedDependencies: ["*"] + } +}, { + title: "overridden by excludes", + excludeDepsRegExp: ["regexp"], + excludeDepsTree: ["b1"], + expectedBuilderArgs: { + buildDependencies: true, + includedDependencies: ["a"], + excludedDependencies: [/regexp/, "b1", "c", "*"] + } +}].forEach((fixture) => { + test.serial(`ui5 build (dependency included via ui5.yaml); ${fixture.title}`, async (t) => { + await assertIncludeDependency(t, { + treeDeps: { + dependencies: [{ + metadata: { + name: "a" + }, + dependencies: [{ + metadata: { + name: "b0" + }, + dependencies: [] + }, { + metadata: { + name: "b1" + }, + dependencies: [{ + metadata: { + name: "c" + }, + dependencies: [] + }] + }] + }], + builder: { + settings: { + includeDependency: ["a"], + includeDependencyRegExp: ["regexp"], + includeDependencyTree: ["b1"], + } + } + }, + excludeDepsRegExp: fixture.excludeDepsRegExp, + excludeDepsTree: fixture.excludeDepsTree, + expectedBuilderArgs: fixture.expectedBuilderArgs + }); + }); +}); + +test.serial("ui5 build --include-dependency-regexp", async (t) => { await assertIncludeDependency(t, { treeDeps: { dependencies: [{ metadata: { name: "sap.ui.core" } - }], - builder: { - settings: { - includeDependency: ["sap.ui.core"] - } - } + }] }, + includeDepsRegExp: ["^sap.[mf]$"], expectedBuilderArgs: { buildDependencies: true, - includedDependencies: ["sap.ui.core"], + includedDependencies: [/^sap.[mf]$/], excludedDependencies: ["*"] } }); }); -test.serial("ui5 build --include-dependency-regexp", async (t) => { +test.serial("ui5 build --include-dependency-tree", async (t) => { await assertIncludeDependency(t, { treeDeps: { dependencies: [{ metadata: { - name: "sap.ui.core" - } + name: "a" + }, + dependencies: [{ + metadata: { + name: "b0" + }, + dependencies: [] + }, { + metadata: { + name: "b1" + }, + dependencies: [{ + metadata: { + name: "c" + }, + dependencies: [] + }] + }] }] }, - includeDepsRegExp: ["^sap.[mf]$"], + includeDepsTree: ["a"], expectedBuilderArgs: { buildDependencies: true, - includedDependencies: [/^sap.[mf]$/], + includedDependencies: ["a", "b0", "b1", "c"], excludedDependencies: ["*"] } }); }); -test.serial("ui5 build --include-dependency-tree", async (t) => { +test.serial("ui5 build include/exclude tree (two subtrees, sharing a transitive dependency)", async (t) => { await assertIncludeDependency(t, { treeDeps: { dependencies: [{ metadata: { - name: "a" + name: "a0" + }, + dependencies: [{ + metadata: { + name: "b0" + }, + dependencies: [] + }, { + metadata: { + name: "b1" + }, + dependencies: [{ + metadata: { + name: "c" + }, + dependencies: [] + }] + }] + }, { + metadata: { + name: "a1" }, dependencies: [{ metadata: { @@ -233,10 +326,51 @@ test.serial("ui5 build --include-dependency-tree", async (t) => { }] }] }, - includeDepsTree: ["a"], + includeDepsTree: ["a0"], + excludeDepsTree: ["a1"], expectedBuilderArgs: { buildDependencies: true, - includedDependencies: ["a", "b0", "b1", "c"], + includedDependencies: ["a0", "b0", "b1", "c"], + excludedDependencies: ["a1", "*"] + } + }); +}); + +test.serial("ui5 build --include-dependency --include-dependency-tree (select a transitive dependency)", async (t) => { + await assertIncludeDependency(t, { + treeDeps: { + dependencies: [{ + metadata: { + name: "a" + }, + dependencies: [{ + metadata: { + name: "b0" + }, + dependencies: [{ + metadata: { + name: "b0.c" + }, + dependencies: [] + }] + }, { + metadata: { + name: "b1" + }, + dependencies: [{ + metadata: { + name: "b1.c" + }, + dependencies: [] + }] + }] + }] + }, + includeDeps: ["b0"], + includeDepsTree: ["b1"], + expectedBuilderArgs: { + buildDependencies: true, + includedDependencies: ["b0", "b1", "b1.c"], excludedDependencies: ["*"] } }); @@ -291,6 +425,41 @@ test.serial("ui5 build --include-dependency-tree=a --exclude-dependency=a", asyn }); }); +test.serial("ui5 build --include-dependency-tree include/exclude tree has a lower priority", async (t) => { + await assertIncludeDependency(t, { + treeDeps: { + dependencies: [{ + metadata: { + name: "a" + }, + dependencies: [{ + metadata: { + name: "b0" + }, + dependencies: [] + }, { + metadata: { + name: "b1" + }, + dependencies: [{ + metadata: { + name: "c" + }, + dependencies: [] + }] + }] + }] + }, + includeDepsTree: ["a"], + excludeDepsRegExp: ["^b.$"], + expectedBuilderArgs: { + buildDependencies: true, + includedDependencies: ["a", "c"], + excludedDependencies: [/^b.$/, "*"] + } + }); +}); + test.serial("ui5 build --include-dependency (dependency not found)", async (t) => { const {log} = t.context; await assertIncludeDependency(t, { diff --git a/test/lib/utils/buildHelper.js b/test/lib/utils/buildHelper.js index 6dc99feb..cd2fc184 100644 --- a/test/lib/utils/buildHelper.js +++ b/test/lib/utils/buildHelper.js @@ -7,7 +7,7 @@ test.beforeEach((t) => { t.context.log = { warn: sinon.stub() }; - sinon.stub(logger, "getLogger").withArgs("cli:commands:build").returns(t.context.log); + sinon.stub(logger, "getLogger").withArgs("cli:utils:buildHelper").returns(t.context.log); t.context.buildHelper = mock.reRequire("../../../lib/utils/buildHelper"); }); @@ -24,21 +24,43 @@ test.serial("createDependencyList", (t) => { }, dependencies: [{ metadata: { - name: "a" - } + name: "a0" + }, + dependencies: [{ + metadata: { + name: "b" + }, + dependencies: [{ + metadata: { + name: "c" + }, + dependencies: [{ + metadata: { + name: "d0" + }, + dependencies: [] + }, { + metadata: { + name: "d1" + }, + dependencies: [] + }] + }] + }] }, { metadata: { - name: "b" - } + name: "a1" + }, + dependencies: [] }] }; - const deps = ["*", "a", "b", "b", "c"]; + const deps = ["*", "a0", "a1", "b", "b", "d1", "unknown"]; const depsRegExp = ["^abc$"]; t.deepEqual(createDependencyList({tree: tree, dependencies: deps, dependenciesRegExp: depsRegExp}), - ["*", "a", "b", /^abc$/]); + ["*", "a0", "a1", "b", "d1", /^abc$/]); t.is(t.context.log.warn.callCount, 1, "log.warn should be called once"); - t.deepEqual(t.context.log.warn.getCall(0).args, ["Could not find dependency 'c' for 'Sample'."], + t.deepEqual(t.context.log.warn.getCall(0).args, ["Could not find dependency 'unknown' for 'Sample'."], "logger.warn should be called with expected string"); }); @@ -84,6 +106,55 @@ test.serial("createDependencyList: handleSubtree", (t) => { "logger.warn should be called with expected string"); }); +test.serial("createDependencyList: handleSubtree (select dependency within the subtree)", (t) => { + const {createDependencyList} = t.context.buildHelper; + const tree = { + metadata: { + name: "Sample" + }, + dependencies: [{ + metadata: { + name: "a" + }, + dependencies: [{ + metadata: { + name: "b" + }, + dependencies: [{ + metadata: { + name: "c" + }, + dependencies: [{ + metadata: { + name: "d0" + }, + dependencies: [] + }, { + metadata: { + name: "d1" + }, + dependencies: [] + }] + }] + }] + }] + }; + const deps = ["b"]; + + t.deepEqual(createDependencyList({tree: tree, dependencies: deps, handleSubtree: true}), + ["b", "c", "d0", "d1"]); +}); + +test.serial("createDependencyList: illegal call", (t) => { + const {createDependencyList} = t.context.buildHelper; + const error = t.throws(() => { + createDependencyList({tree: {}, dependencies: ["a"], dependenciesRegExp: ["b"], handleSubtree: true}) + }); + + t.is(error.message, `RegExp's should not be appended to list of sub-dependencies`, + "Threw with expected error message"); +}); + test.serial("mergeDependencyLists", (t) => { const {mergeDependencyLists} = t.context.buildHelper; const targetDependencyList = ["a"]; @@ -97,33 +168,35 @@ test.serial("mergeDependencyLists", (t) => { test.serial("mergeDependencyLists: RegExp", (t) => { const {mergeDependencyLists} = t.context.buildHelper; const targetDependencyList = [/a/]; - const dependencyList = ["a", "b", "c", "d", /e/]; - const excludeList = [/c/, /e/]; + const dependencyList = [/a/, "a", "b", "c", "d", /e+$/]; + const excludeList = [/c/, /e+$/]; mergeDependencyLists(targetDependencyList, dependencyList, excludeList); - t.deepEqual(targetDependencyList, [/a/, "b", "d"]); + t.deepEqual(targetDependencyList, [/a/, "a", "b", "d"]); }); -test.serial("alignWithBuilderAPI: * is added to excludedDependencies", (t) => { - const {alignWithBuilderAPI} = t.context.buildHelper; +test.serial("alignWithBuilderApi: * is added to excludedDependencies", (t) => { + const {alignWithBuilderApi} = t.context.buildHelper; const args = { includedDependencies: ["a"], excludedDependencies: [] }; - t.is(alignWithBuilderAPI(false, args.includedDependencies, args.excludedDependencies), true); + t.is(alignWithBuilderApi(false, args.includedDependencies, args.excludedDependencies), true, + "Should return true if includedDependencies are given"); t.deepEqual(args, { includedDependencies: ["a"], excludedDependencies: ["*"] }); }); -test.serial("alignWithBuilderAPI: includedDependencies=* is is an alias for buildAll", (t) => { - const {alignWithBuilderAPI} = t.context.buildHelper; +test.serial("alignWithBuilderApi: includedDependencies=* is is an alias for buildAll", (t) => { + const {alignWithBuilderApi} = t.context.buildHelper; const args = { includedDependencies: ["*"], excludedDependencies: [] }; - t.is(alignWithBuilderAPI(false, args.includedDependencies, args.excludedDependencies), true); + t.is(alignWithBuilderApi(false, args.includedDependencies, args.excludedDependencies), true, + "Should return true if includedDependencies are given"); t.deepEqual(args, { includedDependencies: [], excludedDependencies: [] From 119fd55f08fc7dd357a3c94ab696c7841c0d7a56 Mon Sep 17 00:00:00 2001 From: Lars Kissel Date: Tue, 13 Jul 2021 18:11:50 +0200 Subject: [PATCH 3/9] [INTERNAL] Fix ESlint issue --- test/lib/utils/buildHelper.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/lib/utils/buildHelper.js b/test/lib/utils/buildHelper.js index cd2fc184..add5c606 100644 --- a/test/lib/utils/buildHelper.js +++ b/test/lib/utils/buildHelper.js @@ -148,7 +148,7 @@ test.serial("createDependencyList: handleSubtree (select dependency within the s test.serial("createDependencyList: illegal call", (t) => { const {createDependencyList} = t.context.buildHelper; const error = t.throws(() => { - createDependencyList({tree: {}, dependencies: ["a"], dependenciesRegExp: ["b"], handleSubtree: true}) + createDependencyList({tree: {}, dependencies: ["a"], dependenciesRegExp: ["b"], handleSubtree: true}); }); t.is(error.message, `RegExp's should not be appended to list of sub-dependencies`, From 6add4eac58cafbdb4d483d9ae8e7bd00cb69ae78 Mon Sep 17 00:00:00 2001 From: Lars Kissel Date: Fri, 16 Jul 2021 09:44:16 +0200 Subject: [PATCH 4/9] [INTERNAL] Improve creation of dependency lists, enhance documentation --- lib/cli/commands/build.js | 68 ++++------ lib/utils/buildHelper.js | 227 ++++++++++++++++++++++----------- test/lib/cli/commands/build.js | 64 ++++++---- test/lib/utils/buildHelper.js | 217 +++++++++++++++++++------------ 4 files changed, 352 insertions(+), 224 deletions(-) diff --git a/lib/cli/commands/build.js b/lib/cli/commands/build.js index c57199ce..3108b796 100644 --- a/lib/cli/commands/build.js +++ b/lib/cli/commands/build.js @@ -39,28 +39,34 @@ build.builder = function(cli) { type: "boolean" }) .option("include-dependency", { - describe: "A list of dependencies to be included into the build process", + describe: "A list of dependencies to be included into the build process; using the '*' as value is an" + + " alias for including all dependencies into the build process; the listed dependencies cannot be" + + " overruled by dependencies defined in 'exclude-dependency'", type: "array" }) .option("include-dependency-regexp", { - describe: "A list of regular expressions defining dependencies to be included into the build process", + describe: "A list of regular expressions defining dependencies to be included into the build process;" + + " this list is prioritized like 'include-dependency'", type: "array" }) .option("include-dependency-tree", { - describe: "A list of dependencies to be included into the build process; sub-dependencies are" + - " implicitly included and do not need to be part of this list", + describe: "A list of dependencies to be included into the build process; transitive dependencies are" + + " implicitly included and do not need to be part of this list; these dependencies overrule" + + " the selection of 'exclude-dependency-tree' but can be overruled by 'exclude-dependency'", type: "array" }) .option("exclude-dependency", { - describe: "A list of dependencies to be excluded from the build process", + describe: "A list of dependencies to be excluded from the build process; the listed dependencies can" + + " be overruled by dependencies defined in 'include-dependency'", type: "array" }) .option("exclude-dependency-regexp", { - describe: "A list of regular expressions defining dependencies to be excluded from the build process", + describe: "A list of regular expressions defining dependencies to be excluded from the build process;" + + " this list is prioritized like 'exclude-dependency'", type: "array" }) .option("exclude-dependency-tree", { - describe: "A list of dependencies to be excluded from the build process; sub-dependencies are" + + describe: "A list of dependencies to be excluded from the build process; transitive dependencies are" + " implicitly included and do not need to be part of this list", type: "array" }) @@ -127,46 +133,18 @@ async function handleBuild(argv) { const tree = await normalizer.generateProjectTree(normalizerOptions); const buildSettings = (tree.builder && tree.builder.settings) || {}; - const includedDependencies = buildHelper.createDependencyList({ - tree: tree, - dependencies: argv["include-dependency"], - dependenciesRegExp: argv["include-dependency-regexp"] - }); - const includedDependencyTrees = buildHelper.createDependencyList({ - tree: tree, - dependencies: argv["include-dependency-tree"], - handleSubtree: true - }); - const excludedDependencies = buildHelper.createDependencyList({ - tree: tree, - dependencies: argv["exclude-dependency"], - dependenciesRegExp: argv["exclude-dependency-regexp"] - }); - const excludedDependencyTrees = buildHelper.createDependencyList({ + const {includedDependencies, excludedDependencies} = buildHelper.createDependencyLists({ tree: tree, - dependencies: argv["exclude-dependency-tree"], - handleSubtree: true + includeDependency: argv["include-dependency"], + includeDependencyRegExp: argv["include-dependency-regexp"], + includeDependencyTree: argv["include-dependency-tree"], + excludeDependency: argv["exclude-dependency"], + excludeDependencyRegExp: argv["exclude-dependency-regexp"], + excludeDependencyTree: argv["exclude-dependency-tree"], + defaultIncludeDependency: buildSettings.includeDependency, + defaultIncludeDependencyRegExp: buildSettings.includeDependencyRegExp, + defaultIncludeDependencyTree: buildSettings.includeDependencyTree }); - const defaultIncludedDependencies = buildHelper.createDependencyList({ - tree: tree, - dependencies: buildSettings.includeDependency, - dependenciesRegExp: buildSettings.includeDependencyRegExp - }); - const defaultIncludedDependencyTrees = buildHelper.createDependencyList({ - tree: tree, - dependencies: buildSettings.includeDependencyTree, - handleSubtree: true - }); - - // Append resolved include-trees to includedDependencies, ignoring excluded projects - buildHelper.mergeDependencyLists(includedDependencies, includedDependencyTrees, excludedDependencies); - // Append resolved exclude-trees to excludedDependencies, ignoring explicitly included projects - buildHelper.mergeDependencyLists(excludedDependencies, excludedDependencyTrees, includedDependencies); - - // Combine dependencies set in build settings and append them to includedDependencies, ignoring excluded projects - Array.prototype.push.apply(defaultIncludedDependencies, defaultIncludedDependencyTrees); - buildHelper.mergeDependencyLists(includedDependencies, defaultIncludedDependencies, excludedDependencies); - const buildAll = buildHelper.alignWithBuilderApi(argv.all, includedDependencies, excludedDependencies); await builder.build({ diff --git a/lib/utils/buildHelper.js b/lib/utils/buildHelper.js index 618e34b1..e4470983 100644 --- a/lib/utils/buildHelper.js +++ b/lib/utils/buildHelper.js @@ -1,95 +1,172 @@ const log = require("@ui5/logger").getLogger("cli:utils:buildHelper"); /** - * Creates a list of dependencies as String or RegExp by processing the given dependencies and - * dependenciesRegExp parameters. Dependencies are only added to the list if they exist in the - * project tree. Optionally, also the sub-dependencies of given dependencies are - * added to the list. + * Creates an object containing the flattened project dependency tree. Each dependency is defined as an object key while + * its value is an array of all of its transitive dependencies. * - * @param {object} parameters Parameters - * @param {object} parameters.tree Project tree as generated by the - * [@ui5/project.normalizer]{@link module:@ui5/project.normalizer} - * @param {Array.} parameters.dependencies The dependencies to be considered - * @param {Array.} [parameters.dependenciesRegExp] The dependencies to be considered as new - * RegExp instances - * @param {boolean} [parameters.handleSubtree=false] Decides whether the sub-dependencies of the given - * dependencies are included into the returned dependency list - * @returns {Array.} Array of the computed dependencies + * @param {object} tree Project tree as generated by the [@ui5/project.normalizer]{@link module:@ui5/project.normalizer} + * @returns {object} An object with dependency names as key and each with an array of its transitive + * dependencies as value */ -function createDependencyList({tree, dependencies = [], dependenciesRegExp = [], handleSubtree}) { - const dependencyList = []; - if (handleSubtree && dependenciesRegExp.length) { - throw new Error("RegExp's should not be appended to list of sub-dependencies"); - } - - function _addUnique(depName) { - if (!dependencyList.includes(depName)) { - dependencyList.push(depName); - } - } - function _findDependency(searchProject, depName) { - let project = searchProject.dependencies.find((dep) => dep.metadata.name === depName); +function getFlattenedDependencyTree(tree) { + const dependencyInfo = {}; - if (!project) { - for (const project0 of searchProject.dependencies) { - const result = _findDependency(project0, depName); - if (result) { - project = result; - break; - } + function _getTransitiveDependencies(project, dependencies) { + project.dependencies.forEach((dep) => { + if (!dependencies.includes(dep.metadata.name)) { + dependencies.push(dep.metadata.name); + _getTransitiveDependencies(dep, dependencies); } - } - - return project; - } - function _processSubtree(project) { - project.dependencies.forEach((project0) => { - _addUnique(project0.metadata.name); - _processSubtree(project0); }); + return dependencies; } - - dependencies.forEach((depName) => { - if (depName === "*") { - return _addUnique(depName); - } - const project = _findDependency(tree, depName); - if (project) { - _addUnique(depName); - if (handleSubtree) { - _processSubtree(project); + function _processDependencies(project) { + project.dependencies.forEach((dep) => { + if (!dependencyInfo[dep.metadata.name]) { + dependencyInfo[dep.metadata.name] = _getTransitiveDependencies(dep, []); + _processDependencies(dep); } - } else { - log.warn(`Could not find dependency '${depName}' for '${tree.metadata.name}'.`); - } - }); + }); + } - return dependencyList.concat(dependenciesRegExp.map((exp) => new RegExp(exp))); + _processDependencies(tree); + return dependencyInfo; } /** - * Merges a dependencyList into a targetDependencyList without duplicates. Values - * given in excludeList are skipped. + * Creates dependency lists for 'includedDependencies' and 'excludedDependencies'. Regular expressions are directly + * applied to a list of all project dependencies so that they don't need to be evaluated in later processing steps. + * Generally, includes are handled with a higher priority than excludes. Additionally, operations for processing + * transitive dependencies are handled with a lower priority than explicitly mentioned dependencies. The default + * dependencies set in the build settings are appended in the end. * - * @param {Array.} targetDependencyList The target list - * @param {Array.} dependencyList The source list - * @param {Array.} excludeList The list of values to be excluded from the merge operation + * The priority of the various dependency lists is applied in the following order: + *
    + *
  1. includeDependency, includeDependencyRegExp
    2. + *
  2. excludeDependency, excludeDependencyRegExp
    3. + *
  3. includeDependencyTree
    4. + *
  4. excludeDependencyTree
    5. + *
  5. defaultIncludeDependency, defaultIncludeDependencyRegExp, defaultIncludeDependencyTree
    6. + *
+ * + * @param {object} parameters Parameters + * @param {object} parameters.tree Project tree as generated by the + * [@ui5/project.normalizer]{@link module:@ui5/project.normalizer} + * @param {Array.} parameters.includeDependency The dependencies to be considered in 'includedDependencies'; the + * "*" character can be used as wildcard for all dependencies and is an alias for the CLI option "--all" + * @param {Array.} parameters.includeDependencyRegExp Strings which are interpreted as regular expressions + * to describe the selection of dependencies to be considered in 'includedDependencies' + * @param {Array.} parameters.includeDependencyTree The dependencies to be considered in 'includedDependencies'; + * transitive dependencies are also appended + * @param {Array.} parameters.excludeDependency The dependencies to be considered in 'excludedDependencies' + * @param {Array.} parameters.excludeDependencyRegExp Strings which are interpreted as regular expressions + * to describe the selection of dependencies to be considered in 'excludedDependencies' + * @param {Array.} parameters.excludeDependencyTree The dependencies to be considered in 'excludedDependencies'; + * transitive dependencies are also appended + * @param {Array.} parameters.defaultIncludeDependency Same as 'includeDependency' parameter; used for build + * settings + * @param {Array.} parameters.defaultIncludeDependencyRegExp Same as 'includeDependencyRegExp' parameter; used + * for build settings + * @param {Array.} parameters.defaultIncludeDependencyTree Same as 'includeDependencyTree' parameter; used for + * build settings + * @returns {object} An object containing the 'includedDependencies' and 'excludedDependencies' */ -function mergeDependencyLists(targetDependencyList, dependencyList, excludeList) { - function isInList(depList, depName) { - return depList.some((dep) => dep instanceof RegExp ? dep.test(depName) : dep === depName); - } - function isRegExpInList(depList, depRegExp) { - return depList.some((dep) => dep instanceof RegExp && dep.toString() === depRegExp.toString()); +function createDependencyLists({ + tree, + includeDependency = [], includeDependencyRegExp = [], includeDependencyTree = [], + excludeDependency = [], excludeDependencyRegExp = [], excludeDependencyTree = [], + defaultIncludeDependency = [], defaultIncludeDependencyRegExp = [], defaultIncludeDependencyTree = [] +}) { + const flattenedDependencyTree = getFlattenedDependencyTree(tree); + + function isExcluded(excludeList, depName) { + return excludeList && excludeList.has(depName); } - dependencyList.forEach((dep) => { - if ( - dep instanceof RegExp && !isRegExpInList(targetDependencyList, dep) && !isRegExpInList(excludeList, dep) || - !(dep instanceof RegExp) && !targetDependencyList.includes(dep) && !isInList(excludeList, dep) - ) { - targetDependencyList.push(dep); + function processDependencies({targetList, dependencies, dependenciesRegExp = [], excludeList, handleSubtree}) { + if (handleSubtree && dependenciesRegExp.length) { + throw new Error("dependenciesRegExp can't be combined with handleSubtree:true option"); } + dependencies.forEach((depName) => { + if (depName === "*") { + targetList.add(depName); + } else if (Object.keys(flattenedDependencyTree).includes(depName)) { + if (!isExcluded(excludeList, depName)) { + targetList.add(depName); + } + if (handleSubtree) { + flattenedDependencyTree[depName].forEach((dep) => { + if (!isExcluded(excludeList, dep)) { + targetList.add(dep); + } + }); + } + } else { + log.warn( + `Could not find dependency '${depName}' in '${tree.metadata.name}' - dependency filter is ignored` + ); + } + }); + dependenciesRegExp.map((exp) => new RegExp(exp)).forEach((regExp) => { + Object.keys(flattenedDependencyTree).forEach((depName) => { + if (regExp.test(depName) && !isExcluded(excludeList, depName)) { + targetList.add(depName); + } + }); + }); + } + + const includedDependencies = new Set(); + const excludedDependencies = new Set(); + + // add dependencies defined in includeDependency and includeDependencyRegExp to the list of includedDependencies + processDependencies({ + targetList: includedDependencies, + dependencies: includeDependency, + dependenciesRegExp: includeDependencyRegExp + }); + // add dependencies defined in excludeDependency and excludeDependencyRegExp to the list of excludedDependencies + processDependencies({ + targetList: excludedDependencies, + dependencies: excludeDependency, + dependenciesRegExp: excludeDependencyRegExp + }); + // add dependencies defined in includeDependencyTree with their transitive dependencies to the list of + // includedDependencies; due to prioritization only those dependencies are added which are not excluded + // by excludedDependencies + processDependencies({ + targetList: includedDependencies, + dependencies: includeDependencyTree, + excludeList: excludedDependencies, + handleSubtree: true }); + // add dependencies defined in excludeDependencyTree with their transitive dependencies to the list of + // excludedDependencies; due to prioritization only those dependencies are added which are not excluded + // by includedDependencies + processDependencies({ + targetList: excludedDependencies, + dependencies: excludeDependencyTree, + excludeList: includedDependencies, + handleSubtree: true + }); + // due to the lowest priority only add the dependencies defined in build settings if they are not excluded + // by any other dependency defined in excludedDependencies + processDependencies({ + targetList: includedDependencies, + dependencies: defaultIncludeDependency, + dependenciesRegExp: defaultIncludeDependencyRegExp, + excludeList: excludedDependencies + }); + processDependencies({ + targetList: includedDependencies, + dependencies: defaultIncludeDependencyTree, + excludeList: excludedDependencies, + handleSubtree: true + }); + + return { + includedDependencies: Array.from(includedDependencies), + excludedDependencies: Array.from(excludedDependencies) + }; } /** @@ -122,7 +199,7 @@ function alignWithBuilderApi(buildAll, includedDependencies, excludedDependencie } module.exports = { - createDependencyList, - mergeDependencyLists, + getFlattenedDependencyTree, + createDependencyLists, alignWithBuilderApi }; diff --git a/test/lib/cli/commands/build.js b/test/lib/cli/commands/build.js index e2819c22..3e63458a 100644 --- a/test/lib/cli/commands/build.js +++ b/test/lib/cli/commands/build.js @@ -18,7 +18,8 @@ const defaultBuilderArgs = { tree: { metadata: { name: "Sample" - } + }, + dependencies: [] }, destPath: "./dist", buildDependencies: undefined, @@ -52,10 +53,10 @@ test.afterEach.always((t) => { test.serial("ui5 build (default) ", async (t) => { normalizerStub.resolves({ - metadata: - { + metadata: { name: "Sample" - } + }, + dependencies: [] }); args._ = ["build"]; await t.context.build.handler(args); @@ -64,10 +65,10 @@ test.serial("ui5 build (default) ", async (t) => { test.serial("ui5 build dev", async (t) => { normalizerStub.resolves({ - metadata: - { + metadata: { name: "Sample" - } + }, + dependencies: [] }); args._ = ["build", "dev"]; await t.context.build.handler(args); @@ -80,10 +81,10 @@ test.serial("ui5 build dev", async (t) => { test.serial("ui5 build self-contained", async (t) => { normalizerStub.resolves({ - metadata: - { + metadata: { name: "Sample" - } + }, + dependencies: [] }); args._ = ["build", "self-contained"]; await t.context.build.handler(args); @@ -96,10 +97,10 @@ test.serial("ui5 build self-contained", async (t) => { test.serial("ui5 build jsdoc", async (t) => { normalizerStub.resolves({ - metadata: - { + metadata: { name: "Sample" - } + }, + dependencies: [] }); args._ = ["build", "jsdoc"]; await t.context.build.handler(args); @@ -112,10 +113,10 @@ test.serial("ui5 build jsdoc", async (t) => { test.serial("ui5 build --framework-version 1.99", async (t) => { normalizerStub.resolves({ - metadata: - { + metadata: { name: "Sample" - } + }, + dependencies: [] }); args._ = ["build"]; @@ -159,7 +160,8 @@ test.serial("ui5 build --include-dependency", async (t) => { dependencies: [{ metadata: { name: "sap.ui.core" - } + }, + dependencies: [] }] }, includeDeps: ["sap.ui.core"], @@ -177,17 +179,17 @@ test.serial("ui5 build --include-dependency", async (t) => { excludeDepsTree: [], expectedBuilderArgs: { buildDependencies: true, - includedDependencies: ["a", /regexp/, "b1", "c"], + includedDependencies: ["a", "b0", "b1", "c"], excludedDependencies: ["*"] } }, { title: "overridden by excludes", - excludeDepsRegExp: ["regexp"], + excludeDepsRegExp: ["^b0$"], excludeDepsTree: ["b1"], expectedBuilderArgs: { buildDependencies: true, includedDependencies: ["a"], - excludedDependencies: [/regexp/, "b1", "c", "*"] + excludedDependencies: ["b0", "b1", "c", "*"] } }].forEach((fixture) => { test.serial(`ui5 build (dependency included via ui5.yaml); ${fixture.title}`, async (t) => { @@ -217,7 +219,7 @@ test.serial("ui5 build --include-dependency", async (t) => { builder: { settings: { includeDependency: ["a"], - includeDependencyRegExp: ["regexp"], + includeDependencyRegExp: ["^b0$"], includeDependencyTree: ["b1"], } } @@ -235,13 +237,24 @@ test.serial("ui5 build --include-dependency-regexp", async (t) => { dependencies: [{ metadata: { name: "sap.ui.core" - } + }, + dependencies: [] + }, { + metadata: { + name: "sap.m" + }, + dependencies: [] + }, { + metadata: { + name: "sap.f" + }, + dependencies: [] }] }, includeDepsRegExp: ["^sap.[mf]$"], expectedBuilderArgs: { buildDependencies: true, - includedDependencies: [/^sap.[mf]$/], + includedDependencies: ["sap.m", "sap.f"], excludedDependencies: ["*"] } }); @@ -382,7 +395,8 @@ test.serial("ui5 build --include-dependency=* --exclude-dependency=sap.ui.core", dependencies: [{ metadata: { name: "sap.ui.core" - } + }, + dependencies: [] }] }, includeDeps: ["*"], @@ -455,7 +469,7 @@ test.serial("ui5 build --include-dependency-tree include/exclude tree has a lowe expectedBuilderArgs: { buildDependencies: true, includedDependencies: ["a", "c"], - excludedDependencies: [/^b.$/, "*"] + excludedDependencies: ["b0", "b1", "*"] } }); }); diff --git a/test/lib/utils/buildHelper.js b/test/lib/utils/buildHelper.js index add5c606..b7308925 100644 --- a/test/lib/utils/buildHelper.js +++ b/test/lib/utils/buildHelper.js @@ -16,8 +16,8 @@ test.afterEach.always((t) => { mock.stopAll(); }); -test.serial("createDependencyList", (t) => { - const {createDependencyList} = t.context.buildHelper; +test.serial("getFlattenedDependencyTree", (t) => { + const {getFlattenedDependencyTree} = t.context.buildHelper; const tree = { metadata: { name: "Sample" @@ -51,128 +51,187 @@ test.serial("createDependencyList", (t) => { metadata: { name: "a1" }, - dependencies: [] + dependencies: [{ + metadata: { + name: "c" + }, + dependencies: [{ + metadata: { + name: "d0" + }, + dependencies: [] + }, { + metadata: { + name: "d1" + }, + dependencies: [] + }] + }] }] }; - const deps = ["*", "a0", "a1", "b", "b", "d1", "unknown"]; - const depsRegExp = ["^abc$"]; - t.deepEqual(createDependencyList({tree: tree, dependencies: deps, dependenciesRegExp: depsRegExp}), - ["*", "a0", "a1", "b", "d1", /^abc$/]); - t.is(t.context.log.warn.callCount, 1, "log.warn should be called once"); - t.deepEqual(t.context.log.warn.getCall(0).args, ["Could not find dependency 'unknown' for 'Sample'."], - "logger.warn should be called with expected string"); + t.deepEqual(getFlattenedDependencyTree(tree), { + a0: ["b", "c", "d0", "d1"], + a1: ["c", "d0", "d1"], + b: ["c", "d0", "d1"], + c: ["d0", "d1"], + d0: [], + d1: [] + }); }); -test.serial("createDependencyList: handleSubtree", (t) => { - const {createDependencyList} = t.context.buildHelper; +function assertCreateDependencyLists(t, { + includeDependency, includeDependencyRegExp, includeDependencyTree, + excludeDependency, excludeDependencyRegExp, excludeDependencyTree, + defaultIncludeDependency, defaultIncludeDependencyRegExp, defaultIncludeDependencyTree, + expectedIncludedDependencies, expectedExcludedDependencies +}) { + const {createDependencyLists} = t.context.buildHelper; const tree = { metadata: { name: "Sample" }, dependencies: [{ metadata: { - name: "a" + name: "a0" }, dependencies: [{ metadata: { - name: "b" + name: "b0" + }, + dependencies: [] + }, { + metadata: { + name: "b1" }, dependencies: [{ metadata: { name: "c" }, - dependencies: [{ - metadata: { - name: "d0" - }, - dependencies: [] - }, { - metadata: { - name: "d1" - }, - dependencies: [] - }] + dependencies: [] }] }] - }] - }; - const deps = ["a", "x"]; - - t.deepEqual(createDependencyList({tree: tree, dependencies: deps, handleSubtree: true}), - ["a", "b", "c", "d0", "d1"]); - t.is(t.context.log.warn.callCount, 1, "log.warn should be called once"); - t.deepEqual(t.context.log.warn.getCall(0).args, ["Could not find dependency 'x' for 'Sample'."], - "logger.warn should be called with expected string"); -}); - -test.serial("createDependencyList: handleSubtree (select dependency within the subtree)", (t) => { - const {createDependencyList} = t.context.buildHelper; - const tree = { - metadata: { - name: "Sample" - }, - dependencies: [{ + }, { metadata: { - name: "a" + name: "a1" }, dependencies: [{ metadata: { - name: "b" + name: "b0" + }, + dependencies: [] + }, { + metadata: { + name: "b1" }, dependencies: [{ metadata: { name: "c" }, - dependencies: [{ - metadata: { - name: "d0" - }, - dependencies: [] - }, { - metadata: { - name: "d1" - }, - dependencies: [] - }] + dependencies: [] }] + }, { + metadata: { + name: "b2" + }, + dependencies: [] + }] + }, { + metadata: { + name: "a2" + }, + dependencies: [{ + metadata: { + name: "b3" + }, + dependencies: [] }] }] }; - const deps = ["b"]; - t.deepEqual(createDependencyList({tree: tree, dependencies: deps, handleSubtree: true}), - ["b", "c", "d0", "d1"]); + const {includedDependencies, excludedDependencies} = createDependencyLists({ + tree: tree, + includeDependency: includeDependency, + includeDependencyRegExp: includeDependencyRegExp, + includeDependencyTree: includeDependencyTree, + excludeDependency: excludeDependency, + excludeDependencyRegExp: excludeDependencyRegExp, + excludeDependencyTree: excludeDependencyTree, + defaultIncludeDependency: defaultIncludeDependency, + defaultIncludeDependencyRegExp: defaultIncludeDependencyRegExp, + defaultIncludeDependencyTree: defaultIncludeDependencyTree + }); + t.deepEqual(includedDependencies, expectedIncludedDependencies); + t.deepEqual(excludedDependencies, expectedExcludedDependencies); +} + +test.serial("createDependencyLists: only includes", (t) => { + assertCreateDependencyLists(t, { + includeDependency: ["a1", "b2"], + includeDependencyRegExp: ["^b0$"], + includeDependencyTree: ["a2"], + expectedIncludedDependencies: ["a1", "b2", "b0", "a2", "b3"], + expectedExcludedDependencies: [] + }); }); -test.serial("createDependencyList: illegal call", (t) => { - const {createDependencyList} = t.context.buildHelper; - const error = t.throws(() => { - createDependencyList({tree: {}, dependencies: ["a"], dependenciesRegExp: ["b"], handleSubtree: true}); +test.serial("createDependencyLists: only excludes", (t) => { + assertCreateDependencyLists(t, { + excludeDependency: ["a1", "b2"], + excludeDependencyRegExp: ["^b0$"], + excludeDependencyTree: ["a2"], + expectedIncludedDependencies: [], + expectedExcludedDependencies: ["a1", "b2", "b0", "a2", "b3"] }); +}); - t.is(error.message, `RegExp's should not be appended to list of sub-dependencies`, - "Threw with expected error message"); +test.serial("createDependencyLists: includeDependencyTree has lower priority than excludes", (t) => { + assertCreateDependencyLists(t, { + includeDependencyTree: ["a1"], + excludeDependency: ["a1"], + excludeDependencyRegExp: ["^b[0-2]$"], + expectedIncludedDependencies: ["c"], + expectedExcludedDependencies: ["a1", "b0", "b1", "b2"] + }); }); -test.serial("mergeDependencyLists", (t) => { - const {mergeDependencyLists} = t.context.buildHelper; - const targetDependencyList = ["a"]; - const dependencyList = ["a", "b", "c", "d"]; - const excludeList = ["c"]; +test.serial("createDependencyLists: excludeDependencyTree has lower priority than includes", (t) => { + assertCreateDependencyLists(t, { + includeDependency: ["a1"], + includeDependencyRegExp: ["^b[0-2]$"], + excludeDependencyTree: ["a1"], + expectedIncludedDependencies: ["a1", "b0", "b1", "b2"], + expectedExcludedDependencies: ["c"] + }); +}); - mergeDependencyLists(targetDependencyList, dependencyList, excludeList); - t.deepEqual(targetDependencyList, ["a", "b", "d"]); +test.serial("createDependencyLists: includeDependencyTree has higher priority than excludeDependencyTree", (t) => { + assertCreateDependencyLists(t, { + includeDependencyTree: ["a1"], + excludeDependencyTree: ["a1"], + expectedIncludedDependencies: ["a1", "b0", "b1", "c", "b2"], + expectedExcludedDependencies: [] + }); }); -test.serial("mergeDependencyLists: RegExp", (t) => { - const {mergeDependencyLists} = t.context.buildHelper; - const targetDependencyList = [/a/]; - const dependencyList = [/a/, "a", "b", "c", "d", /e+$/]; - const excludeList = [/c/, /e+$/]; +test.serial("createDependencyLists: defaultIncludeDependency/RegExp has lower priority than excludes", (t) => { + assertCreateDependencyLists(t, { + defaultIncludeDependency: ["a1", "b2", "c"], + defaultIncludeDependencyRegExp: ["^b0$"], + excludeDependency: ["a1"], + excludeDependencyRegExp: ["^b.$"], + expectedIncludedDependencies: ["c"], + expectedExcludedDependencies: ["a1", "b0", "b1", "b2", "b3"] + }); +}); - mergeDependencyLists(targetDependencyList, dependencyList, excludeList); - t.deepEqual(targetDependencyList, [/a/, "a", "b", "d"]); +test.serial("createDependencyLists: defaultIncludeDependencyTree has lower priority than excludes", (t) => { + assertCreateDependencyLists(t, { + defaultIncludeDependencyTree: ["a1"], + excludeDependencyTree: ["b1"], + expectedIncludedDependencies: ["a1", "b0", "b2"], + expectedExcludedDependencies: ["b1", "c"] + }); }); test.serial("alignWithBuilderApi: * is added to excludedDependencies", (t) => { From a1875841229fa1ac6ddecd61911472dcab70bb03 Mon Sep 17 00:00:00 2001 From: Lars Kissel Date: Fri, 16 Jul 2021 09:48:54 +0200 Subject: [PATCH 5/9] [INTERNAL] Fix broken test --- test/lib/cli/commands/build.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/lib/cli/commands/build.js b/test/lib/cli/commands/build.js index 3e63458a..32da92fb 100644 --- a/test/lib/cli/commands/build.js +++ b/test/lib/cli/commands/build.js @@ -483,6 +483,7 @@ test.serial("ui5 build --include-dependency (dependency not found)", async (t) = includeDeps: ["sap.ui.core"] }); t.is(log.warn.callCount, 1, "log.warn should be called once"); - t.deepEqual(log.warn.getCall(0).args, ["Could not find dependency 'sap.ui.core' for 'Sample'."], + t.deepEqual(log.warn.getCall(0).args, + ["Could not find dependency 'sap.ui.core' in 'Sample' - dependency filter is ignored"], "logger.warn should be called with expected string"); }); From 7ac5b84e3528dd39500bf23235102c3151360227 Mon Sep 17 00:00:00 2001 From: Lars Kissel Date: Mon, 19 Jul 2021 11:28:43 +0200 Subject: [PATCH 6/9] [INTERNAL] Apply suggestions from code review --- lib/cli/commands/build.js | 28 ++++++++++++++-------------- lib/utils/buildHelper.js | 19 ++++++++++++++----- test/lib/cli/commands/build.js | 2 +- 3 files changed, 29 insertions(+), 20 deletions(-) diff --git a/lib/cli/commands/build.js b/lib/cli/commands/build.js index 3108b796..5f614277 100644 --- a/lib/cli/commands/build.js +++ b/lib/cli/commands/build.js @@ -39,35 +39,35 @@ build.builder = function(cli) { type: "boolean" }) .option("include-dependency", { - describe: "A list of dependencies to be included into the build process; using the '*' as value is an" + - " alias for including all dependencies into the build process; the listed dependencies cannot be" + - " overruled by dependencies defined in 'exclude-dependency'", + describe: "A list of dependencies to be included into the build process. Using the '*' as value is an" + + " alias for including all dependencies into the build process. The listed dependencies cannot be" + + " overruled by dependencies defined in 'exclude-dependency'.", type: "array" }) .option("include-dependency-regexp", { - describe: "A list of regular expressions defining dependencies to be included into the build process;" + - " this list is prioritized like 'include-dependency'", + describe: "A list of regular expressions defining dependencies to be included into the build process." + + " This list is prioritized like 'include-dependency'.", type: "array" }) .option("include-dependency-tree", { - describe: "A list of dependencies to be included into the build process; transitive dependencies are" + - " implicitly included and do not need to be part of this list; these dependencies overrule" + - " the selection of 'exclude-dependency-tree' but can be overruled by 'exclude-dependency'", + describe: "A list of dependencies to be included into the build process. Transitive dependencies are" + + " implicitly included and do not need to be part of this list. These dependencies overrule" + + " the selection of 'exclude-dependency-tree' but can be overruled by 'exclude-dependency'.", type: "array" }) .option("exclude-dependency", { - describe: "A list of dependencies to be excluded from the build process; the listed dependencies can" + - " be overruled by dependencies defined in 'include-dependency'", + describe: "A list of dependencies to be excluded from the build process. The listed dependencies can" + + " be overruled by dependencies defined in 'include-dependency'.", type: "array" }) .option("exclude-dependency-regexp", { - describe: "A list of regular expressions defining dependencies to be excluded from the build process;" + - " this list is prioritized like 'exclude-dependency'", + describe: "A list of regular expressions defining dependencies to be excluded from the build process." + + " This list is prioritized like 'exclude-dependency'.", type: "array" }) .option("exclude-dependency-tree", { - describe: "A list of dependencies to be excluded from the build process; transitive dependencies are" + - " implicitly included and do not need to be part of this list", + describe: "A list of dependencies to be excluded from the build process. Transitive dependencies are" + + " implicitly included and do not need to be part of this list.", type: "array" }) .option("dest", { diff --git a/lib/utils/buildHelper.js b/lib/utils/buildHelper.js index e4470983..443098bf 100644 --- a/lib/utils/buildHelper.js +++ b/lib/utils/buildHelper.js @@ -77,6 +77,15 @@ function createDependencyLists({ excludeDependency = [], excludeDependencyRegExp = [], excludeDependencyTree = [], defaultIncludeDependency = [], defaultIncludeDependencyRegExp = [], defaultIncludeDependencyTree = [] }) { + if ( + !includeDependency.length && !includeDependencyRegExp.length && !includeDependencyTree.length && + !excludeDependency.length && !excludeDependencyRegExp.length && !excludeDependencyTree.length && + !defaultIncludeDependency.length && !defaultIncludeDependencyRegExp.length && + !defaultIncludeDependencyTree.length + ) { + return {includedDependencies: [], excludedDependencies: []}; + } + const flattenedDependencyTree = getFlattenedDependencyTree(tree); function isExcluded(excludeList, depName) { @@ -89,7 +98,7 @@ function createDependencyLists({ dependencies.forEach((depName) => { if (depName === "*") { targetList.add(depName); - } else if (Object.keys(flattenedDependencyTree).includes(depName)) { + } else if (flattenedDependencyTree[depName]) { if (!isExcluded(excludeList, depName)) { targetList.add(depName); } @@ -102,16 +111,16 @@ function createDependencyLists({ } } else { log.warn( - `Could not find dependency '${depName}' in '${tree.metadata.name}' - dependency filter is ignored` - ); + `Could not find dependency "${depName}" for project ${tree.metadata.name}. Dependency filter is ` + + `ignored`); } }); dependenciesRegExp.map((exp) => new RegExp(exp)).forEach((regExp) => { - Object.keys(flattenedDependencyTree).forEach((depName) => { + for (const depName in flattenedDependencyTree) { if (regExp.test(depName) && !isExcluded(excludeList, depName)) { targetList.add(depName); } - }); + } }); } diff --git a/test/lib/cli/commands/build.js b/test/lib/cli/commands/build.js index 32da92fb..d64cf07a 100644 --- a/test/lib/cli/commands/build.js +++ b/test/lib/cli/commands/build.js @@ -484,6 +484,6 @@ test.serial("ui5 build --include-dependency (dependency not found)", async (t) = }); t.is(log.warn.callCount, 1, "log.warn should be called once"); t.deepEqual(log.warn.getCall(0).args, - ["Could not find dependency 'sap.ui.core' in 'Sample' - dependency filter is ignored"], + ["Could not find dependency \"sap.ui.core\" for project Sample. Dependency filter is ignored"], "logger.warn should be called with expected string"); }); From 7fe851fe86268adce4e7f6740ed9e2284d0153db Mon Sep 17 00:00:00 2001 From: Lars Kissel Date: Tue, 20 Jul 2021 16:27:56 +0200 Subject: [PATCH 7/9] [INTERNAL] Improve build command description for option "include-dependency" --- lib/cli/commands/build.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cli/commands/build.js b/lib/cli/commands/build.js index 5f614277..9c63cae1 100644 --- a/lib/cli/commands/build.js +++ b/lib/cli/commands/build.js @@ -39,8 +39,8 @@ build.builder = function(cli) { type: "boolean" }) .option("include-dependency", { - describe: "A list of dependencies to be included into the build process. Using the '*' as value is an" + - " alias for including all dependencies into the build process. The listed dependencies cannot be" + + describe: "A list of dependencies to be included into the build process. You can use the asterisk '*' as" + + " an alias for including all dependencies into the build process. The listed dependencies cannot be" + " overruled by dependencies defined in 'exclude-dependency'.", type: "array" }) From 692d93b4dc6ed19b13f9cb0922136f6b998ef335 Mon Sep 17 00:00:00 2001 From: Lars Kissel Date: Fri, 23 Jul 2021 12:01:51 +0200 Subject: [PATCH 8/9] [INTERNAL] Improve JSDoc param types --- lib/utils/buildHelper.js | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/lib/utils/buildHelper.js b/lib/utils/buildHelper.js index 443098bf..b3200ebb 100644 --- a/lib/utils/buildHelper.js +++ b/lib/utils/buildHelper.js @@ -5,7 +5,7 @@ const log = require("@ui5/logger").getLogger("cli:utils:buildHelper"); * its value is an array of all of its transitive dependencies. * * @param {object} tree Project tree as generated by the [@ui5/project.normalizer]{@link module:@ui5/project.normalizer} - * @returns {object} An object with dependency names as key and each with an array of its transitive + * @returns {Object} An object with dependency names as key and each with an array of its transitive * dependencies as value */ function getFlattenedDependencyTree(tree) { @@ -40,7 +40,8 @@ function getFlattenedDependencyTree(tree) { * transitive dependencies are handled with a lower priority than explicitly mentioned dependencies. The default * dependencies set in the build settings are appended in the end. * - * The priority of the various dependency lists is applied in the following order: + * The priority of the various dependency lists is applied in the following order, but note that a later list can't + * overrule earlier ones: *
    *
  1. includeDependency, includeDependencyRegExp
    2. *
  2. excludeDependency, excludeDependencyRegExp
    3. @@ -52,24 +53,24 @@ function getFlattenedDependencyTree(tree) { * @param {object} parameters Parameters * @param {object} parameters.tree Project tree as generated by the * [@ui5/project.normalizer]{@link module:@ui5/project.normalizer} - * @param {Array.} parameters.includeDependency The dependencies to be considered in 'includedDependencies'; the + * @param {string[]} parameters.includeDependency The dependencies to be considered in 'includedDependencies'; the * "*" character can be used as wildcard for all dependencies and is an alias for the CLI option "--all" - * @param {Array.} parameters.includeDependencyRegExp Strings which are interpreted as regular expressions + * @param {string[]} parameters.includeDependencyRegExp Strings which are interpreted as regular expressions * to describe the selection of dependencies to be considered in 'includedDependencies' - * @param {Array.} parameters.includeDependencyTree The dependencies to be considered in 'includedDependencies'; + * @param {string[]} parameters.includeDependencyTree The dependencies to be considered in 'includedDependencies'; * transitive dependencies are also appended - * @param {Array.} parameters.excludeDependency The dependencies to be considered in 'excludedDependencies' - * @param {Array.} parameters.excludeDependencyRegExp Strings which are interpreted as regular expressions + * @param {string[]} parameters.excludeDependency The dependencies to be considered in 'excludedDependencies' + * @param {string[]} parameters.excludeDependencyRegExp Strings which are interpreted as regular expressions * to describe the selection of dependencies to be considered in 'excludedDependencies' - * @param {Array.} parameters.excludeDependencyTree The dependencies to be considered in 'excludedDependencies'; + * @param {string[]} parameters.excludeDependencyTree The dependencies to be considered in 'excludedDependencies'; * transitive dependencies are also appended - * @param {Array.} parameters.defaultIncludeDependency Same as 'includeDependency' parameter; used for build + * @param {string[]} parameters.defaultIncludeDependency Same as 'includeDependency' parameter; used for build * settings - * @param {Array.} parameters.defaultIncludeDependencyRegExp Same as 'includeDependencyRegExp' parameter; used + * @param {string[]} parameters.defaultIncludeDependencyRegExp Same as 'includeDependencyRegExp' parameter; used * for build settings - * @param {Array.} parameters.defaultIncludeDependencyTree Same as 'includeDependencyTree' parameter; used for + * @param {string[]} parameters.defaultIncludeDependencyTree Same as 'includeDependencyTree' parameter; used for * build settings - * @returns {object} An object containing the 'includedDependencies' and 'excludedDependencies' + * @returns {Object} An object containing the 'includedDependencies' and 'excludedDependencies' */ function createDependencyLists({ tree, @@ -189,8 +190,8 @@ function createDependencyLists({ * * @param {boolean} buildAll The value of the all command line parameter to decide if project * dependencies have to be built - * @param {Array.} includedDependencies The list of included dependencies - * @param {Array.} excludedDependencies The list of excluded dependencies + * @param {string[]} includedDependencies The list of included dependencies + * @param {string[]} excludedDependencies The list of excluded dependencies * @returns {boolean} Whether it is required to build project dependencies */ function alignWithBuilderApi(buildAll, includedDependencies, excludedDependencies) { From e69b951631d4e6f7a018a88dd5226a4c4acb7d0e Mon Sep 17 00:00:00 2001 From: Lars Kissel Date: Fri, 23 Jul 2021 12:24:54 +0200 Subject: [PATCH 9/9] [INTERNAL] Fix JSDoc return types --- lib/utils/buildHelper.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/utils/buildHelper.js b/lib/utils/buildHelper.js index b3200ebb..546e2e01 100644 --- a/lib/utils/buildHelper.js +++ b/lib/utils/buildHelper.js @@ -5,7 +5,7 @@ const log = require("@ui5/logger").getLogger("cli:utils:buildHelper"); * its value is an array of all of its transitive dependencies. * * @param {object} tree Project tree as generated by the [@ui5/project.normalizer]{@link module:@ui5/project.normalizer} - * @returns {Object} An object with dependency names as key and each with an array of its transitive + * @returns {object} An object with dependency names as key and each with an array of its transitive * dependencies as value */ function getFlattenedDependencyTree(tree) { @@ -70,7 +70,8 @@ function getFlattenedDependencyTree(tree) { * for build settings * @param {string[]} parameters.defaultIncludeDependencyTree Same as 'includeDependencyTree' parameter; used for * build settings - * @returns {Object} An object containing the 'includedDependencies' and 'excludedDependencies' + * @returns {{includedDependencies:string[],excludedDependencies:string[]}} An object containing the + * 'includedDependencies' and 'excludedDependencies' */ function createDependencyLists({ tree,