[FIX] JSDoc: Implement own tmp dir lifecycle

For this a new entity 'BuildContext' has been introduced in the builder. This might be the foundation for future enhancements of the (Custom-) Task API. Resolves https://github.com/SAP/ui5-cli/issues/187
SAP · May 15, 2019 · 3f85abf · 3f85abf
1 parent fe641cf
commit 3f85abf
Show file tree

Hide file tree

Showing 11 changed files with 332 additions and 71 deletions.
diff --git a/lib/builder/BuildContext.js b/lib/builder/BuildContext.js
@@ -0,0 +1,58 @@
+/**
+ * Context of a build process
+ *
+ * @private
+ * @memberof module:@ui5/builder.builder
+ */
+class BuildContext {
+	constructor() {
+		this.projectBuildContexts = [];
+	}
+
+	createProjectContext({project, resources}) {
+		const projectBuildContext = new ProjectBuildContext({
+			buildContext: this,
+			project,
+			resources
+		});
+		this.projectBuildContexts.push(projectBuildContext);
+		return projectBuildContext;
+	}
+
+	async executeCleanupTasks() {
+		await Promise.all(this.projectBuildContexts.map((ctx) => {
+			return ctx.executeCleanupTasks();
+		}));
+	}
+}
+
+
+/**
+ * Build context of a single project. Always part of an overall
+ * [Build Context]{@link module:@ui5/builder.builder.BuildContext}
+ *
+ * @private
+ * @memberof module:@ui5/builder.builder
+ */
+class ProjectBuildContext {
+	constructor({buildContext, project, resources}) {
+		// this.buildContext = buildContext;
+		// this.project = project;
+		// this.resources = resources;
+		this.queues = {
+			cleanup: []
+		};
+	}
+
+	registerCleanupTask(callback) {
+		this.queues.cleanup.push(callback);
+	}
+
+	async executeCleanupTasks() {
+		await Promise.all(this.queues.cleanup.map((callback) => {
+			return callback();
+		}));
+	}
+}
+
+module.exports = BuildContext;
diff --git a/lib/builder/builder.js b/lib/builder/builder.js
@@ -3,6 +3,7 @@ const resourceFactory = require("@ui5/fs").resourceFactory;
 const MemAdapter = require("@ui5/fs").adapters.Memory;
 const typeRepository = require("../types/typeRepository");
 const taskRepository = require("../tasks/taskRepository");
+const BuildContext = require("./BuildContext");
 
 const definedTasks = taskRepository.getAllTasks();
 
@@ -130,6 +131,61 @@ function composeTaskList({dev, selfContained, jsdoc, includedTasks, excludedTask
 	return selectedTasks;
 }
 
+async function executeCleanupTasks(buildContext) {
+	log.info("Executing cleanup tasks...");
+	await buildContext.executeCleanupTasks();
+}
+
+function registerCleanupSigHooks(buildContext) {
+	function createListener(exitCode) {
+		return function() {
+			// Asynchronously cleanup resources, then exit
+			executeCleanupTasks(buildContext).then(() => {
+				process.exit(exitCode);
+			});
+		};
+	}
+
+	const processSignals = {
+		"SIGHUP": createListener(128 + 1),
+		"SIGINT": createListener(128 + 2),
+		"SIGTERM": createListener(128 + 15),
+		"SIGBREAK": createListener(128 + 21)
+	};
+
+	for (const signal in processSignals) {
+		if (processSignals.hasOwnProperty(signal)) {
+			process.on(signal, processSignals[signal]);
+		}
+	}
+
+	// == TO BE DISCUSSED: Also cleanup for unhandled rejections and exceptions?
+	// Add additional events like signals since they are registered on the process
+	//	event emitter in a similar fashion
+	// processSignals["unhandledRejection"] = createListener(1);
+	// process.once("unhandledRejection", processSignals["unhandledRejection"]);
+	// processSignals["uncaughtException"] = function(err, origin) {
+	// 	const fs = require("fs");
+	// 	fs.writeSync(
+	// 		process.stderr.fd,
+	// 		`Caught exception: ${err}\n` +
+	// 		`Exception origin: ${origin}`
+	// 	);
+	// 	createListener(1)();
+	// };
+	// process.once("uncaughtException", processSignals["uncaughtException"]);
+
+	return processSignals;
+}
+
+function deregisterCleanupSigHooks(signals) {
+	for (const signal in signals) {
+		if (signals.hasOwnProperty(signal)) {
+			process.removeListener(signal, signals[signal]);
+		}
+	}
+}
+
 /**
  * Builder
  *
@@ -156,7 +212,7 @@ module.exports = {
 	 * @param {Array} [parameters.devExcludeProject=[]] List of projects to be excluded from development build
 	 * @returns {Promise} Promise resolving to <code>undefined</code> once build has finished
 	 */
-	build({
+	async build({
 		tree, destPath,
 		buildDependencies = false, dev = false, selfContained = false, jsdoc = false,
 		includedTasks = [], excludedTasks = [], devExcludeProject = []
@@ -173,6 +229,8 @@ module.exports = {
 			virBasePath: "/"
 		});
 
+		const buildContext = new BuildContext();
+		const cleanupSigHooks = registerCleanupSigHooks(buildContext);
 
 		const projects = {}; // Unique project index to prevent building the same project multiple times
 		const projectWriters = {}; // Collection of memory adapters of already built libraries
@@ -235,6 +293,14 @@ module.exports = {
 					name: project.metadata.name
 				});
 
+				const projectContext = buildContext.createProjectContext({
+					// project, // TODO 2.0: Add project facade object/instance here
+					resources: {
+						workspace,
+						dependencies: resourceCollections.dependencies
+					}
+				});
+
 				if (dev && devExcludeProject.indexOf(project.metadata.name) !== -1) {
 					projectTasks = composeTaskList({dev: false, selfContained, includedTasks, excludedTasks});
 				}
@@ -246,7 +312,8 @@ module.exports = {
 					},
 					tasks: projectTasks,
 					project,
-					parentLogger: log
+					parentLogger: log,
+					buildContext: projectContext
 				}).then(() => {
 					log.verbose("Finished building project %s. Writing out files...", project.metadata.name);
 					buildLogger.completeWork(1);
@@ -265,11 +332,15 @@ module.exports = {
 			});
 		}
 
-		return buildProject(tree).then(() => {
+		try {
+			await buildProject(tree);
 			log.info(`Build succeeded in ${getElapsedTime(startTime)}`);
-		}, (err) => {
+		} catch (err) {
 			log.error(`Build failed in ${getElapsedTime(startTime)}`);
 			throw err;
-		});
+		} finally {
+			deregisterCleanupSigHooks(cleanupSigHooks);
+			await executeCleanupTasks(buildContext);
+		}
 	}
 };
diff --git a/lib/tasks/jsdoc/generateJsdoc.js b/lib/tasks/jsdoc/generateJsdoc.js
@@ -1,9 +1,11 @@
 const log = require("@ui5/logger").getLogger("builder:tasks:jsdoc:generateJsdoc");
 const path = require("path");
 const makeDir = require("make-dir");
+const os = require("os");
 const fs = require("graceful-fs");
-const tmp = require("tmp");
-tmp.setGracefulCleanup();
+const {promisify} = require("util");
+const mkdtemp = promisify(fs.mkdtemp);
+const rimraf = promisify(require("rimraf"));
 const jsdocGenerator = require("../../processors/jsdoc/jsdocGenerator");
 const {resourceFactory} = require("@ui5/fs");
 
@@ -22,12 +24,15 @@ const {resourceFactory} = require("@ui5/fs");
  * @param {string} parameters.options.version Project version
  * @returns {Promise<undefined>} Promise resolving with <code>undefined</code> once data has been written
  */
-const generateJsdoc = async function({workspace, dependencies, options} = {}) {
+const generateJsdoc = async function({buildContext, workspace, dependencies, options} = {}) {
 	if (!options || !options.projectName || !options.namespace || !options.version || !options.pattern) {
 		throw new Error("[generateJsdoc]: One or more mandatory options not provided");
 	}
 
-	const {sourcePath: resourcePath, targetPath, tmpPath} = await generateJsdoc._createTmpDirs(options.projectName);
+	const {sourcePath: resourcePath, targetPath, tmpPath, cleanup} =
+		await generateJsdoc._createTmpDirs(options.projectName);
+
+	buildContext.registerCleanupTask(cleanup);
 
 	const [writtenResourcesCount] = await Promise.all([
 		generateJsdoc._writeResourcesToDir({
@@ -64,7 +69,6 @@ const generateJsdoc = async function({workspace, dependencies, options} = {}) {
 	}));
 };
 
-
 /**
  * Create temporary directories for JSDoc generation processor
  *
@@ -73,7 +77,7 @@ const generateJsdoc = async function({workspace, dependencies, options} = {}) {
  * @returns {Promise<Object>} Promise resolving with sourcePath, targetPath and tmpPath strings
  */
 async function createTmpDirs(projectName) {
-	const {path: tmpDirPath} = await createTmpDir(projectName);
+	const tmpDirPath = await generateJsdoc._createTmpDir(projectName);
 
 	const sourcePath = path.join(tmpDirPath, "src"); // dir will be created by writing project resources below
 	await makeDir(sourcePath, {fs});
@@ -86,7 +90,10 @@ async function createTmpDirs(projectName) {
 	return {
 		sourcePath,
 		targetPath,
-		tmpPath
+		tmpPath,
+		cleanup: async () => {
+			return rimraf(tmpDirPath);
+		}
 	};
 }
 
@@ -95,28 +102,16 @@ async function createTmpDirs(projectName) {
  *
  * @private
  * @param {string} projectName Project name used for naming the temporary directory
- * @param {boolean} [keep=false] Whether to keep the temporary directory
- * @returns {Promise<Object>} Promise resolving with path of the temporary directory
+ * @returns {Promise<string>} Promise resolving with path of the temporary directory
  */
-function createTmpDir(projectName, keep = false) {
-	// Remove all non alpha-num characters from project name
+async function createTmpDir(projectName) {
 	const sanitizedProjectName = projectName.replace(/[^A-Za-z0-9]/g, "");
-	return new Promise((resolve, reject) => {
-		tmp.dir({
-			prefix: `ui5-tooling-tmp-jsdoc-${sanitizedProjectName}-`,
-			keep,
-			unsafeCleanup: true
-		}, (err, path) => {
-			if (err) {
-				reject(err);
-				return;
-			}
-
-			resolve({
-				path
-			});
-		});
-	});
+
+	const tmpRootPath = path.join(os.tmpdir(), "ui5-tooling");
+	await makeDir(tmpRootPath, {fs});
+
+	// Appending minus sign also because node docs advise to "avoid trailing X characters in prefix"
+	return mkdtemp(path.join(tmpRootPath, `jsdoc-${sanitizedProjectName}-`));
 }
 
 /**

diff --git a/lib/types/AbstractBuilder.js b/lib/types/AbstractBuilder.js
@@ -14,7 +14,7 @@ class AbstractBuilder {
 	 * @param {Object} parameters.project Project configuration
 	 * @param {GroupLogger} parameters.parentLogger Logger to use
 	 */
-	constructor({resourceCollections, project, parentLogger}) {
+	constructor({resourceCollections, project, parentLogger, buildContext}) {
 		if (new.target === AbstractBuilder) {
 			throw new TypeError("Class 'AbstractBuilder' is abstract");
 		}
@@ -26,8 +26,8 @@ class AbstractBuilder {
 
 		this.tasks = {};
 		this.taskExecutionOrder = [];
-		this.addStandardTasks({resourceCollections, project, log: this.log});
-		this.addCustomTasks({resourceCollections, project});
+		this.addStandardTasks({resourceCollections, project, log: this.log, buildContext});
+		this.addCustomTasks({resourceCollections, project, buildContext});
 	}
 
 	/**
@@ -42,7 +42,7 @@ class AbstractBuilder {
 	 * @param {Object} parameters.project Project configuration
 	 * @param {Object} parameters.log <code>@ui5/logger</code> logger instance
 	 */
-	addStandardTasks({resourceCollections, project, log}) {
+	addStandardTasks({resourceCollections, project, log, buildContext}) {
 		throw new Error("Function 'addStandardTasks' is not implemented");
 	}
 
@@ -56,7 +56,7 @@ class AbstractBuilder {
 	 * @param {ReaderCollection} parameters.resourceCollections.dependencies Workspace Resource
 	 * @param {Object} parameters.project Project configuration
 	 */
-	addCustomTasks({resourceCollections, project}) {
+	addCustomTasks({resourceCollections, project, buildContext}) {
 		const projectCustomTasks = project.builder && project.builder.customTasks;
 		if (!projectCustomTasks || projectCustomTasks.length === 0) {
 			return; // No custom tasks defined

diff --git a/lib/types/application/applicationType.js b/lib/types/application/applicationType.js
@@ -5,8 +5,8 @@ module.exports = {
 	format: function(project) {
 		return new ApplicationFormatter().format(project);
 	},
-	build: function({resourceCollections, tasks, project, parentLogger}) {
-		return new ApplicationBuilder({resourceCollections, project, parentLogger}).build(tasks);
+	build: function({resourceCollections, tasks, project, parentLogger, buildContext}) {
+		return new ApplicationBuilder({resourceCollections, project, parentLogger, buildContext}).build(tasks);
 	},
 
 	// Export type classes for extensibility

diff --git a/lib/types/library/LibraryBuilder.js b/lib/types/library/LibraryBuilder.js
@@ -18,7 +18,7 @@ const tasks = { // can't require index.js due to circular dependency
 };
 
 class LibraryBuilder extends AbstractBuilder {
-	addStandardTasks({resourceCollections, project, parentLogger}) {
+	addStandardTasks({resourceCollections, project, parentLogger, buildContext}) {
 		const namespace = project.metadata.name.replace(/\./g, "/");
 
 		this.addTask("replaceCopyright", () => {
@@ -57,6 +57,7 @@ class LibraryBuilder extends AbstractBuilder {
 			}
 
 			return generateJsdoc({
+				buildContext,
 				workspace: resourceCollections.workspace,
 				dependencies: resourceCollections.dependencies,
 				options: {

diff --git a/lib/types/library/libraryType.js b/lib/types/library/libraryType.js
@@ -5,8 +5,8 @@ module.exports = {
 	format: function(project) {
 		return new LibraryFormatter().format(project);
 	},
-	build: function({resourceCollections, tasks, project, parentLogger}) {
-		return new LibraryBuilder({resourceCollections, project, parentLogger}).build(tasks);
+	build: function({resourceCollections, tasks, project, parentLogger, buildContext}) {
+		return new LibraryBuilder({resourceCollections, project, parentLogger, buildContext}).build(tasks);
 	},
 
 	// Export type classes for extensibility

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -111,9 +111,9 @@
 		"pretty-data": "^0.40.0",
 		"pretty-hrtime": "^1.0.3",
 		"replacestream": "^4.0.3",
+		"rimraf": "^2.6.3",
 		"semver": "^6.0.0",
 		"tap-xunit": "^2.3.0",
-		"tmp": "0.1.0",
 		"uglify-es": "^3.2.2",
 		"xml2js": "^0.4.17",
 		"yazl": "^2.5.1"
@@ -133,7 +133,6 @@
 		"nyc": "^14.1.1",
 		"opn-cli": "^4.1.0",
 		"recursive-readdir": "^2.1.1",
-		"rimraf": "^2.6.3",
 		"sinon": "^7.3.2",
 		"tap-nyan": "^1.1.0"
 	}