Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[FIX] ProjectBuilder now can be executed in parallel #669

Merged
merged 2 commits into from
Oct 20, 2023
Merged

[FIX] ProjectBuilder now can be executed in parallel #669

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5c533d5
[FIX] Create a new BuildLogger for every ProjectBuilder instance
d3xter666 Oct 19, 2023
3076e97
Add tests
d3xter666 Oct 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 27 additions & 26 deletions lib/build/ProjectBuilder.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import {rimraf} from "rimraf";
import * as resourceFactory from "@ui5/fs/resourceFactory";
import BuildLogger from "@ui5/logger/internal/loggers/Build";
const log = new BuildLogger("ProjectBuilder");
import composeProjectList from "./helpers/composeProjectList.js";
import BuildContext from "./helpers/BuildContext.js";
import prettyHrtime from "pretty-hrtime";
Expand All @@ -12,6 +11,7 @@ import prettyHrtime from "pretty-hrtime";
* @alias @ui5/project/build/ProjectBuilder
*/
class ProjectBuilder {
#log;
/**
* Build Configuration
*
Expand Down Expand Up @@ -112,6 +112,7 @@ class ProjectBuilder {

this._graph = graph;
this._buildContext = new BuildContext(graph, taskRepository, buildConfig);
this.#log = new BuildLogger("ProjectBuilder");
}

/**
Expand Down Expand Up @@ -148,8 +149,8 @@ class ProjectBuilder {
}
}
const rootProjectName = this._graph.getRoot().getName();
log.info(`Preparing build for project ${rootProjectName}`);
log.info(` Target directory: ${destPath}`);
this.#log.info(`Preparing build for project ${rootProjectName}`);
this.#log.info(` Target directory: ${destPath}`);

// Get project filter function based on include/exclude params
// (also logs some info to console)
Expand Down Expand Up @@ -195,19 +196,19 @@ class ProjectBuilder {
}
});

log.setProjects(queue.map((projectBuildContext) => {
this.#log.setProjects(queue.map((projectBuildContext) => {
return projectBuildContext.getProject().getName();
}));
if (queue.length > 1) { // Do not log if only the root project is being built
log.info(`Processing ${queue.length} projects`);
this.#log.info(`Processing ${queue.length} projects`);
if (alreadyBuilt.length) {
log.info(` Reusing build results of ${alreadyBuilt.length} projects`);
log.info(` Building ${queue.length - alreadyBuilt.length} projects`);
this.#log.info(` Reusing build results of ${alreadyBuilt.length} projects`);
this.#log.info(` Building ${queue.length - alreadyBuilt.length} projects`);
}

if (log.isLevelEnabled("verbose")) {
log.verbose(` Required projects:`);
log.verbose(` ${queue
if (this.#log.isLevelEnabled("verbose")) {
this.#log.verbose(` Required projects:`);
this.#log.verbose(` ${queue
.map((projectBuildContext) => {
const projectName = projectBuildContext.getProject().getName();
let msg;
Expand All @@ -225,7 +226,7 @@ class ProjectBuilder {
}

if (cleanDest) {
log.info(`Cleaning target directory...`);
this.#log.info(`Cleaning target directory...`);
await rimraf(destPath);
}
const startTime = process.hrtime();
Expand All @@ -234,29 +235,29 @@ class ProjectBuilder {
for (const projectBuildContext of queue) {
const projectName = projectBuildContext.getProject().getName();
const projectType = projectBuildContext.getProject().getType();
log.verbose(`Processing project ${projectName}...`);
this.#log.verbose(`Processing project ${projectName}...`);

// Only build projects that are not already build (i.e. provide a matching build manifest)
if (alreadyBuilt.includes(projectName)) {
log.skipProjectBuild(projectName, projectType);
this.#log.skipProjectBuild(projectName, projectType);
} else {
log.startProjectBuild(projectName, projectType);
this.#log.startProjectBuild(projectName, projectType);
await projectBuildContext.getTaskRunner().runTasks();
log.endProjectBuild(projectName, projectType);
this.#log.endProjectBuild(projectName, projectType);
}
if (!requestedProjects.includes(projectName)) {
// Project has not been requested
// => Its resources shall not be part of the build result
continue;
}

log.verbose(`Writing out files...`);
this.#log.verbose(`Writing out files...`);
pWrites.push(this._writeResults(projectBuildContext, fsTarget));
}
await Promise.all(pWrites);
log.info(`Build succeeded in ${this._getElapsedTime(startTime)}`);
this.#log.info(`Build succeeded in ${this._getElapsedTime(startTime)}`);
} catch (err) {
log.error(`Build failed in ${this._getElapsedTime(startTime)}`);
this.#log.error(`Build failed in ${this._getElapsedTime(startTime)}`);
throw err;
} finally {
this._deregisterCleanupSigHooks(cleanupSigHooks);
Expand All @@ -272,7 +273,7 @@ class ProjectBuilder {
const projectBuildContexts = new Map();

for (const projectName of requiredProjects) {
log.verbose(`Creating build context for project ${projectName}...`);
this.#log.verbose(`Creating build context for project ${projectName}...`);
const projectBuildContext = this._buildContext.createProjectContext({
project: this._graph.getProject(projectName)
});
Expand Down Expand Up @@ -320,15 +321,15 @@ class ProjectBuilder {

if (includedDependencies.length) {
if (includedDependencies.length === this._graph.getSize() - 1) {
log.info(` Including all dependencies`);
this.#log.info(` Including all dependencies`);
} else {
log.info(` Requested dependencies:`);
log.info(` + ${includedDependencies.join("\n + ")}`);
this.#log.info(` Requested dependencies:`);
this.#log.info(` + ${includedDependencies.join("\n + ")}`);
}
}
if (excludedDependencies.length) {
log.info(` Excluded dependencies:`);
log.info(` - ${excludedDependencies.join("\n + ")}`);
this.#log.info(` Excluded dependencies:`);
this.#log.info(` - ${excludedDependencies.join("\n + ")}`);
}

const rootProjectName = this._graph.getRoot().getName();
Expand Down Expand Up @@ -379,7 +380,7 @@ class ProjectBuilder {

await Promise.all(resources.map((resource) => {
if (taskUtil.getTag(resource, taskUtil.STANDARD_TAGS.OmitFromBuildResult)) {
log.silly(`Skipping write of resource tagged as "OmitFromBuildResult": ` +
this.#log.silly(`Skipping write of resource tagged as "OmitFromBuildResult": ` +
resource.getPath());
return; // Skip target write for this resource
}
Expand All @@ -388,7 +389,7 @@ class ProjectBuilder {
}

async _executeCleanupTasks() {
log.info("Executing cleanup tasks...");
this.#log.info("Executing cleanup tasks...");
await this._buildContext.executeCleanupTasks();
}

Expand Down
18 changes: 18 additions & 0 deletions test/lib/build/ProjectBuilder.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -731,6 +731,24 @@ test("_executeCleanupTasks", async (t) => {
"BuildContext#executeCleanupTasks got called with no arguments");
});

test("instantiate new logger for every ProjectBuilder", async (t) => {
function CreateBuildLoggerMock(moduleName) {
t.is(moduleName, "ProjectBuilder", "BuildLogger created with expected moduleName");
return {};
}

const {graph, taskRepository, sinon} = t.context;
const createBuildLoggerMockSpy = sinon.spy(CreateBuildLoggerMock);
const ProjectBuilder = await esmock("../../../lib/build/ProjectBuilder.js", {
"@ui5/logger/internal/loggers/Build": createBuildLoggerMockSpy
});

new ProjectBuilder({graph, taskRepository});
new ProjectBuilder({graph, taskRepository});

t.is(createBuildLoggerMockSpy.callCount, 2, "BuildLogger is instantiated for every ProjectBuilder instance");
});


function getProcessListenerCount() {
return ["SIGHUP", "SIGINT", "SIGTERM", "SIGBREAK"].map((eventName) => {
Expand Down
Loading