From 32c0d228cc1f55eb80ced30d118db0d31608625b Mon Sep 17 00:00:00 2001 From: Tom Najdek Date: Wed, 25 May 2022 15:10:30 +0200 Subject: [PATCH 1/4] Fix all scss files rebuild on every non-js change multimatch test was incorrect triggering scss rebuild on every change that reached that logic. Also fixed weird (legacy?) use of path.join() --- scripts/sass.js | 4 ++-- scripts/watch.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/sass.js b/scripts/sass.js index 6e3e457607..81061a8341 100644 --- a/scripts/sass.js +++ b/scripts/sass.js @@ -33,13 +33,13 @@ async function getSass(source, options, signatures={}) { let newFileSignature = await getFileSignature(f); let destFile = getPathRelativeTo(f, 'scss'); destFile = path.join(path.dirname(destFile), path.basename(destFile, '.scss') + '.css'); - let dest = path.join.apply(this, ['build', 'chrome', 'skin', 'default', 'zotero', destFile]); + let dest = path.join('build', 'chrome', 'skin', 'default', 'zotero', destFile); if (['win', 'mac', 'unix'].some(platform => f.endsWith(`-${platform}.scss`))) { let platform = f.slice(f.lastIndexOf('-') + 1, f.lastIndexOf('.')); destFile = destFile.slice(0, destFile.lastIndexOf('-')) + destFile.slice(destFile.lastIndexOf('-') + 1 + platform.length); - dest = path.join.apply(this, ['build', 'chrome', 'content', 'zotero-platform', platform, destFile]); + dest = path.join('build', 'chrome', 'content', 'zotero-platform', platform, destFile); } try { diff --git a/scripts/watch.js b/scripts/watch.js index a341de2dda..c15926224b 100644 --- a/scripts/watch.js +++ b/scripts/watch.js @@ -56,7 +56,7 @@ function getWatch() { return; } for (var i = 0; i < scssFiles.length; i++) { - if (multimatch(path, scssFiles[i])) { + if (multimatch(path, scssFiles[i]).length) { onSuccess(await getSass(scssFiles[i], { ignore: ignoreMask })); onSuccess(await cleanUp(signatures)); return; From eb5f2978ccd9e8b5930748beadd4bc4b8a14dfa3 Mon Sep 17 00:00:00 2001 From: Tom Najdek Date: Wed, 25 May 2022 16:21:27 +0200 Subject: [PATCH 2/4] Fix a problem with build when switching branch When switching between master and fx102 bogus files were created outside of the build/ directory. That's because previously we had a symlinked file itembox.css and now we compile a file itemBox.css into the same directory. However when running `npm start`, after switching branch, the symlink still existed and thus when writing a new file, symlink was followed and output has been written inside a source file instead. Build system will now run cleanup step first (where it checks if files frim `.signatures.json` still exist in src), then proceed with build, so old symlinks will be removed thus producing a valid build. This problem probably only happened on case-insensitive file-systems (like default config on macOS HFS+). --- scripts/build.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/scripts/build.js b/scripts/build.js index 089f6b86dd..5e920db6d8 100644 --- a/scripts/build.js +++ b/scripts/build.js @@ -24,6 +24,11 @@ if (require.main === module) { .concat([`!${formatDirsForMatcher(copyDirs)}/**`]); const signatures = await getSignatures(); + + // Check if all files in signatures are still present in src; Needed to avoid a problem + // where what was a symlink before, now is compiled, resulting in polluting source files + onSuccess(await cleanUp(signatures)); + const results = await Promise.all([ getBrowserify(signatures), getCopy(copyDirs.map(d => `${d}/**`), { ignore: ignoreMask }, signatures), @@ -31,7 +36,6 @@ if (require.main === module) { ...scssFiles.map(scf => getSass(scf, { ignore: ignoreMask }, signatures)), getSymlinks(symlinks, { nodir: true, ignore: ignoreMask }, signatures), getSymlinks(symlinkDirs, { ignore: ignoreMask }, signatures), - cleanUp(signatures), getPDFReader(signatures), getPDFWorker(signatures), getZoteroNoteEditor(signatures) From 7aa3bde170eeba4482450ef67d0c072feea13d13 Mon Sep 17 00:00:00 2001 From: Tom Najdek Date: Wed, 25 May 2022 19:37:20 +0200 Subject: [PATCH 3/4] On error, ring the bell Should help with noticing syntax errors etc. --- scripts/utils.js | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/utils.js b/scripts/utils.js index 969711b1c1..e9c88bd73b 100644 --- a/scripts/utils.js +++ b/scripts/utils.js @@ -11,6 +11,7 @@ const NODE_ENV = process.env.NODE_ENV; function onError(err) { + console.log('\u0007'); //🔔 console.log(colors.red('Error:'), err); } From 8b76bf6e650756e654754c144276c533d159b333 Mon Sep 17 00:00:00 2001 From: Tom Najdek Date: Wed, 25 May 2022 19:56:23 +0200 Subject: [PATCH 4/4] Run add_omni_file script when file change detected --- scripts/browserify.js | 7 ++-- scripts/copy.js | 7 ++-- scripts/js.js | 10 +++--- scripts/sass.js | 10 +++--- scripts/symlinks.js | 1 + scripts/utils.js | 3 ++ scripts/watch.js | 82 ++++++++++++++++++++++++++++++++----------- 7 files changed, 86 insertions(+), 34 deletions(-) diff --git a/scripts/browserify.js b/scripts/browserify.js index 3cd1a278b7..fa9c9b1670 100644 --- a/scripts/browserify.js +++ b/scripts/browserify.js @@ -11,7 +11,7 @@ const ROOT = path.resolve(__dirname, '..'); async function getBrowserify(signatures) { const t1 = Date.now(); - var count = 0; + const outFiles = []; var config, f, totalCount; while ((config = browserifyConfigs.pop()) != null) { @@ -48,7 +48,7 @@ async function getBrowserify(signatures) { onProgress(f, dest, 'browserify'); signatures[f] = newFileSignature; - count++; + outFiles.push(dest); } catch (err) { throw new Error(`Failed on ${f}: ${err}`); } @@ -58,7 +58,8 @@ async function getBrowserify(signatures) { const t2 = Date.now(); return { action: 'browserify', - count, + count: outFiles.length, + outFiles, totalCount, processingTime: t2 - t1 }; diff --git a/scripts/copy.js b/scripts/copy.js index 329a2288e6..b16766992b 100644 --- a/scripts/copy.js +++ b/scripts/copy.js @@ -12,7 +12,7 @@ async function getCopy(source, options, signatures) { const t1 = Date.now(); const files = await globby(source, Object.assign({ cwd: ROOT }, options )); const totalCount = files.length; - var count = 0; + const outFiles = []; var f; while ((f = files.pop()) != null) { @@ -34,7 +34,7 @@ async function getCopy(source, options, signatures) { await fs.copy(f, dest); onProgress(f, dest, 'cp'); signatures[f] = newFileSignature; - count++; + outFiles.push(dest); } catch (err) { throw new Error(`Failed on ${f}: ${err}`); } @@ -43,7 +43,8 @@ async function getCopy(source, options, signatures) { const t2 = Date.now(); return { action: 'copy', - count, + count: outFiles.length, + outFiles, totalCount, processingTime: t2 - t1 }; diff --git a/scripts/js.js b/scripts/js.js index d3f4b35725..aed8324b29 100644 --- a/scripts/js.js +++ b/scripts/js.js @@ -14,7 +14,6 @@ async function getJS(source, options, signatures) { const matchingJSFiles = await globby(source, Object.assign({ cwd: ROOT }, options)); const cpuCount = os.cpus().length; const totalCount = matchingJSFiles.length; - var count = 0; var isError = false; cluster.setupMaster({ @@ -48,7 +47,8 @@ async function getJS(source, options, signatures) { const t2 = Date.now(); return Promise.resolve({ action: 'js', - count, + count: 0, + outFiles: [], totalCount, processingTime: t2 - t1 }); @@ -56,6 +56,7 @@ async function getJS(source, options, signatures) { // distribute processing among workers const workerCount = Math.min(cpuCount, filesForProcessing.length); + const outFiles = []; var workersActive = workerCount; NODE_ENV == 'debug' && console.log(`Will process ${filesForProcessing.length} files using ${workerCount} processes`); return new Promise((resolve, reject) => { @@ -75,7 +76,7 @@ async function getJS(source, options, signatures) { } else { NODE_ENV == 'debug' && console.log(`process ${this.id} took ${ev.processingTime} ms to process ${ev.sourcefile} into ${ev.outfile}`); NODE_ENV != 'debug' && onProgress(ev.sourcefile, ev.outfile, 'js'); - count++; + outFiles.push(ev.outfile); } } @@ -95,7 +96,8 @@ async function getJS(source, options, signatures) { const t2 = Date.now(); resolve({ action: 'js', - count, + count: outFiles.length, + outFiles, totalCount, processingTime: t2 - t1 }); diff --git a/scripts/sass.js b/scripts/sass.js index 81061a8341..fd9c6ef90a 100644 --- a/scripts/sass.js +++ b/scripts/sass.js @@ -11,11 +11,12 @@ const sassRender = universalify.fromCallback(sass.render); const ROOT = path.resolve(__dirname, '..'); -async function getSass(source, options, signatures={}) { +async function getSass(source, options, signatures = {}) { const t1 = Date.now(); const files = await globby(source, Object.assign({ cwd: ROOT }, options)); const totalCount = files.length; - var count = 0, shouldRebuild = false; + const outFiles = []; + var shouldRebuild = false; for (const f of files) { // if any file changed, rebuild all onSuccess @@ -54,7 +55,7 @@ async function getSass(source, options, signatures={}) { await fs.outputFile(`${dest}.map`, sass.map); onProgress(f, dest, 'sass'); signatures[f] = newFileSignature; - count++; + outFiles.push(dest); } catch (err) { throw new Error(`Failed on ${f}: ${err}`); @@ -65,7 +66,8 @@ async function getSass(source, options, signatures={}) { const t2 = Date.now(); return { action: 'sass', - count, + count: outFiles.length, + outFiles, totalCount, processingTime: t2 - t1 }; diff --git a/scripts/symlinks.js b/scripts/symlinks.js index a43301bc1d..ff5f5c8d6e 100644 --- a/scripts/symlinks.js +++ b/scripts/symlinks.js @@ -56,6 +56,7 @@ async function getSymlinks(source, options, signatures) { return { action: 'symlink', count: filesProcessedCount, + outFiles: filesToProcess, totalCount: files.length, processingTime: t2 - t1 }; diff --git a/scripts/utils.js b/scripts/utils.js index e9c88bd73b..3cb1480d6b 100644 --- a/scripts/utils.js +++ b/scripts/utils.js @@ -117,10 +117,13 @@ function comparePaths(actualPath, testedPath) { return path.normalize(actualPath) === path.normalize(testedPath); } +const envCheckTrue = env => !!(env && (parseInt(env) || env === true || env === "true")); + module.exports = { cleanUp, comparePaths, compareSignatures, + envCheckTrue, formatDirsForMatcher, getFileSignature, getPathRelativeTo, diff --git a/scripts/watch.js b/scripts/watch.js index c15926224b..e8645b14a4 100644 --- a/scripts/watch.js +++ b/scripts/watch.js @@ -1,8 +1,10 @@ const path = require('path'); +const fs = require('fs-extra'); const chokidar = require('chokidar'); const multimatch = require('multimatch'); +const { exec } = require('child_process'); const { dirs, jsFiles, scssFiles, ignoreMask, copyDirs, symlinkFiles } = require('./config'); -const { onSuccess, onError, getSignatures, writeSignatures, cleanUp, formatDirsForMatcher } = require('./utils'); +const { envCheckTrue, onSuccess, onError, getSignatures, writeSignatures, cleanUp, formatDirsForMatcher } = require('./utils'); const getJS = require('./js'); const getSass = require('./sass'); const getCopy = require('./copy'); @@ -10,6 +12,9 @@ const getSymlinks = require('./symlinks'); const ROOT = path.resolve(__dirname, '..'); +const addOmniExecPath = path.join(ROOT, '..', 'zotero-standalone-build', 'scripts', 'add_omni_file'); +let shouldAddOmni = false; + const source = [ 'chrome', 'components', @@ -45,32 +50,69 @@ process.on('SIGINT', () => { process.exit(); }); -function getWatch() { +async function addOmniFiles(relPaths) { + const t1 = Date.now(); + const buildDirPath = path.join(ROOT, 'build'); + const wrappedPaths = relPaths.map(relPath => `"${path.relative(buildDirPath, relPath)}"`); + + await new Promise((resolve, reject) => { + const cmd = `"${addOmniExecPath}" ${wrappedPaths.join(' ')}`; + exec(cmd, { cwd: buildDirPath }, (error, output) => { + if (error) { + reject(error); + } + else { + process.env.NODE_ENV === 'debug' && console.log(`Executed:\n${cmd};\nOutput:\n${output}\n`); + resolve(output); + } + }); + }); + + const t2 = Date.now(); + + return { + action: 'add-omni-files', + count: relPaths.length, + totalCount: relPaths.length, + processingTime: t2 - t1 + }; +} + +async function getWatch() { + try { + await fs.access(addOmniExecPath, fs.constants.F_OK); + shouldAddOmni = !envCheckTrue(process.env.SKIP_OMNI); + } + catch (_) {} + let watcher = chokidar.watch(source, { cwd: ROOT }) .on('change', async (path) => { try { - var matched = false; + var result = false; if (multimatch(path, jsFiles).length && !multimatch(path, ignoreMask).length) { - onSuccess(await getJS(path, { ignore: ignoreMask }, signatures)); + result = await getJS(path, { ignore: ignoreMask }, signatures); onSuccess(await cleanUp(signatures)); - return; } - for (var i = 0; i < scssFiles.length; i++) { - if (multimatch(path, scssFiles[i]).length) { - onSuccess(await getSass(scssFiles[i], { ignore: ignoreMask })); - onSuccess(await cleanUp(signatures)); - return; + if (!result) { + for (var i = 0; i < scssFiles.length; i++) { + if (multimatch(path, scssFiles[i]).length) { + result = await getSass(scssFiles[i], { ignore: ignoreMask }); // eslint-disable-line no-await-in-loop + break; + } } } - if (multimatch(path, copyDirs.map(d => `${d}/**`)).length) { - onSuccess(await getCopy(path, {}, signatures)); - onSuccess(await cleanUp(signatures)); - return; + if (!result && multimatch(path, copyDirs.map(d => `${d}/**`)).length) { + result = await getCopy(path, {}, signatures); } - if (multimatch(path, symlinks).length) { - onSuccess(await getSymlinks(path, { nodir: true }, signatures)); - onSuccess(await cleanUp(signatures)); - return; + if (!result && multimatch(path, symlinks).length) { + result = await getSymlinks(path, { nodir: true }, signatures); + } + + onSuccess(result); + onSuccess(await cleanUp(signatures)); + + if (shouldAddOmni && result.outFiles?.length) { + onSuccess(await addOmniFiles(result.outFiles)); } } catch (err) { @@ -83,7 +125,7 @@ function getWatch() { }); watcher.add(source); - console.log('Watching files for changes...'); + console.log(`Watching files for changes (omni updates ${shouldAddOmni ? 'enabled' : 'disabled'})...`); } module.exports = getWatch; @@ -93,4 +135,4 @@ if (require.main === module) { signatures = await getSignatures(); getWatch(); })(); -} \ No newline at end of file +}