From 2b037ee146dcfe6ed40f5848ecd825e76c58fa3d Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 24 Sep 2018 17:12:42 -0600 Subject: [PATCH] Prevent races by blocking on SDK builds If we don't block on SDK builds, then the riot-web build fails due to half-built dependencies. This needs to be done at two levels: the js-sdk because it is used by both the react-sdk and riot-web, and at the react-sdk because riot-web needs it. This means our build process is synchronous for js -> react -> riot, at least for the initial build. This does increase the startup time, particularly because the file watch timer is at 5 seconds. The timer is used to detect a storm of file changes in the underlying SDKs and give the build process some room to compile larger files if needed. The file watcher is accompanied by a "canary signal file" to prevent the build-blocking script from unblocking too early. Both the js and react SDKs build when `npm install` is run, so we ensure that we only listen for the `npm start` build for each SDK. This is all done at the riot level instead of at the individual SDK levels (where we could use a canary file to signal up the stack) because: * babel (used by the js-sdk) doesn't really provide an "end up build" signal * webpack is a bit of a nightmare to get it to behave at times * this blocking approach is really only applicable to riot-web, although may be useful to some other projects. Hopefully that all makes sense. --- .gitignore | 1 + package-lock.json | 6 ++++ package.json | 17 ++++++----- scripts/block-on-sdk-build.js | 56 +++++++++++++++++++++++++++++++++++ scripts/build-watch-sdk.js | 25 ++++++++++++++++ 5 files changed, 97 insertions(+), 8 deletions(-) create mode 100644 scripts/block-on-sdk-build.js diff --git a/.gitignore b/.gitignore index 14030632ca..9c2c394cf0 100644 --- a/.gitignore +++ b/.gitignore @@ -19,3 +19,4 @@ electron/pub /config.json.* /config.local*.json /src/component-index.js +/.tmp diff --git a/package-lock.json b/package-lock.json index 90d41371fe..e7da86c95e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -384,6 +384,12 @@ "integrity": "sha1-GdOGodntxufByF04iu28xW0zYC0=", "dev": true }, + "async-lock": { + "version": "1.1.3", + "resolved": "https://registry.npmjs.org/async-lock/-/async-lock-1.1.3.tgz", + "integrity": "sha512-nxlfFLGfCJ1r7p9zhR5OuL6jYkDd9P7FqSitfLji+C1NdyhCz4+rWW3kiPiyPASHhN7VlsKEvRWWbnME9lYngw==", + "dev": true + }, "asynckit": { "version": "0.4.0", "resolved": "https://registry.npmjs.org/asynckit/-/asynckit-0.4.0.tgz", diff --git a/package.json b/package.json index 5503a72abe..333966ce30 100644 --- a/package.json +++ b/package.json @@ -44,14 +44,14 @@ "install:electron": "install-app-deps", "electron": "npm run install:electron && electron .", "start:res": "node scripts/copy-res.js -w", - "start:js": "webpack-dev-server --output-filename=bundles/_dev_/[name].js --output-chunk-file=bundles/_dev_/[name].js -w --progress", - "start:js:prod": "cross-env NODE_ENV=production webpack-dev-server -w --progress", - "start:js-sdk": "node scripts//build-watch-sdk.js watch js", - "start:js-sdk:prod": "cross-env NODE_ENV=production node scripts//build-watch-sdk.js watch js", - "start:react-sdk": "node scripts//build-watch-sdk.js watch react", - "start:react-sdk:prod": "cross-env NODE_ENV=production node scripts//build-watch-sdk.js watch react", - "start": "concurrently --kill-others -n js-sdk,react-sdk,reskindex,res,js \"npm run start:js-sdk\" \"npm run start:react-sdk\" \"npm run reskindex:watch\" \"npm run start:res\" \"npm run start:js\"", - "start:prod": "concurrently --kill-others -n js-sdk,react-sdk,reskindex,res,js \"npm run start:js-sdk:prod\" \"npm run start:react-sdk:prod\" \"npm run reskindex:watch\" \"npm run start:res\" \"npm run start:js:prod\"", + "start:js": "node scripts/block-on-sdk-build.js js && node scripts/block-on-sdk-build.js react && webpack-dev-server --output-filename=bundles/_dev_/[name].js --output-chunk-file=bundles/_dev_/[name].js -w --progress", + "start:js:prod": "cross-env NODE_ENV=production node scripts/block-on-sdk-build.js js && node scripts/block-on-sdk-build.js react && webpack-dev-server -w --progress", + "start:js-sdk": "node scripts/build-watch-sdk.js watch js", + "start:js-sdk:prod": "cross-env NODE_ENV=production node && scripts/build-watch-sdk.js watch js", + "start:react-sdk": "node scripts/block-on-sdk-build.js js && node scripts/build-watch-sdk.js watch react", + "start:react-sdk:prod": "cross-env NODE_ENV=production node && scripts/block-on-sdk-build.js js && node scripts//build-watch-sdk.js watch react", + "start": "concurrently --kill-others-on-fail --prefix \"{time} [{name}]\" -n js-sdk,react-sdk,reskindex,res,riot-js \"npm run start:js-sdk\" \"npm run start:react-sdk\" \"npm run reskindex:watch\" \"npm run start:res\" \"npm run start:js\"", + "start:prod": "concurrently --kill-others-on-fail --prefix \"{time} [{name}]\" -n js-sdk,react-sdk,reskindex,res,riot-js \"npm run start:js-sdk:prod\" \"npm run start:react-sdk:prod\" \"npm run reskindex:watch\" \"npm run start:res\" \"npm run start:js:prod\"", "lint": "eslint src/", "lintall": "eslint src/ test/", "clean": "rimraf lib webapp electron_app/dist", @@ -81,6 +81,7 @@ "url": "^0.11.0" }, "devDependencies": { + "async-lock": "^1.1.3", "autoprefixer": "^6.6.0", "babel-cli": "^6.5.2", "babel-core": "^6.14.0", diff --git a/scripts/block-on-sdk-build.js b/scripts/block-on-sdk-build.js new file mode 100644 index 0000000000..cc29275a77 --- /dev/null +++ b/scripts/block-on-sdk-build.js @@ -0,0 +1,56 @@ +const path = require('path'); +const chokidar = require('chokidar'); +const AsyncLock = require('async-lock'); + + +const WAIT_TIME = 5000; // ms + +function waitForCanary(canaryName) { + return new Promise((resolve, reject) => { + const filename = path.resolve(path.join(".tmp", canaryName)); + + // See triggerCanarySignal in build-watch-sdk.js for why we watch for `unlink` + const watcher = chokidar.watch(filename).on('unlink', (path) => { + console.log("[block-on-build] Received signal to start watching for builds"); + watcher.close(); + resolve(); + }); + }); +} + +function waitOnSdkBuild(sdkName) { + // First we wait for a local canary file to be changed + return waitForCanary(sdkName).then(() => new Promise((resolve, reject) => { + const buildDirectory = path.dirname(require.resolve(`matrix-${sdkName}-sdk`)); + const lock = new AsyncLock(); + let timerId = null; + + const watcher = chokidar.watch(buildDirectory).on('all', (event, path) => { + lock.acquire("timer", (done) => { + if (timerId !== null) { + //console.log("Resetting countdown"); + clearTimeout(timerId); + } + //console.log(`Waiting ${WAIT_TIME}ms for another file update...`); + timerId = setTimeout(() => { + console.log("[block-on-build] No updates - unblocking"); + watcher.close(); + resolve(); + }, WAIT_TIME); + done(); + }, null, null); + }); + })); +} + +const sdkName = process.argv[2]; +if (!sdkName) { + console.error("[block-on-build] No SDK name provided"); + process.exit(1); +} + +console.log("[block-on-build] Waiting for SDK: " + sdkName); +waitOnSdkBuild(sdkName).then(() => { + console.log("[block-on-build] Unblocked"); + process.exit(0); +}); \ No newline at end of file diff --git a/scripts/build-watch-sdk.js b/scripts/build-watch-sdk.js index 9a9edd444e..775ea4a502 100644 --- a/scripts/build-watch-sdk.js +++ b/scripts/build-watch-sdk.js @@ -36,9 +36,34 @@ if (fs.existsSync(path.join(sdkPath, '.git'))) { }); } + console.log("Sending signal that other processes may unblock"); + triggerCanarySignal(sdkName); + console.log("Performing task: " + task); child_process.execSync(`npm ${task === "build" ? "run build" : "start"}`, { env: process.env, cwd: sdkPath, }); } + +function triggerCanarySignal(sdkName) { + const tmpPath = path.resolve(".tmp"); + + try { + fs.mkdirSync(tmpPath); + } catch (e) { + if (e.code !== 'EEXIST') { + console.error(e); + throw "Failed to create temporary directory"; + } + } + + // Note: we intentionally create then delete the file to work around + // a file watching problem where if the file exists on startup it may + // fire a "created" event for the file. By having the behaviour be "do + // something on delete" we avoid accidentally firing the signal too + // early. + const canaryPath = path.join(tmpPath, sdkName); + fs.closeSync(fs.openSync(canaryPath, 'w')); + fs.unlinkSync(canaryPath); +} \ No newline at end of file