From 86324b7b1fe5fd2e40eeeb37cbdfe1d2d4c54183 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Wed, 5 Jul 2023 19:09:39 -0700 Subject: [PATCH 1/2] fix: determine parser based on target filename --- lib/util/files.js | 2 +- lib/util/parser.js | 46 ++++++++++++++++++++++++++++++---------------- package.json | 1 + 3 files changed, 32 insertions(+), 17 deletions(-) diff --git a/lib/util/files.js b/lib/util/files.js index c18979bf..e0abb5e7 100644 --- a/lib/util/files.js +++ b/lib/util/files.js @@ -35,7 +35,7 @@ const getParsers = (dir, files, options) => { return new (parser(Parser.Parsers))(target, file, options, { clean }) } - return new (Parser(file))(target, file, options, { clean }) + return new (Parser(target))(target, file, options, { clean }) }) return parsers.filter(Boolean) diff --git a/lib/util/parser.js b/lib/util/parser.js index e209dfe7..48ba7377 100644 --- a/lib/util/parser.js +++ b/lib/util/parser.js @@ -1,11 +1,12 @@ const fs = require('fs/promises') -const { basename, extname, dirname } = require('path') +const { dirname } = require('path') const yaml = require('yaml') const NpmPackageJson = require('@npmcli/package-json') const jsonParse = require('json-parse-even-better-errors') const Diff = require('diff') const { unset } = require('lodash') const ini = require('ini') +const { minimatch } = require('minimatch') const template = require('./template.js') const jsonDiff = require('./json-diff') const merge = require('./merge.js') @@ -167,17 +168,17 @@ class Base { } class Gitignore extends Base { - static types = ['codeowners', 'gitignore'] + static types = ['codeowners', '.gitignore'] comment = (c) => `# ${c}` } class Js extends Base { - static types = ['js'] + static types = ['*.js'] comment = (c) => `/* ${c} */` } class Ini extends Base { - static types = ['ini'] + static types = ['*.ini'] comment = (c) => `; ${c}` toString (s) { @@ -202,17 +203,17 @@ class Ini extends Base { } class IniMerge extends Ini { - static types = ['npmrc'] + static types = ['.npmrc'] merge = (t, s) => merge(t, s) } class Markdown extends Base { - static types = ['md'] + static types = ['*.md'] comment = (c) => `` } class Yml extends Base { - static types = ['yml'] + static types = ['*.yml'] comment = (c) => ` ${c}` toString (s) { @@ -274,7 +275,7 @@ class YmlMerge extends Yml { } class Json extends Base { - static types = ['json'] + static types = ['*.json'] // its a json comment! not really but we do add a special key // to json objects comment = (c) => ({ [`//${this.options.config.__NAME__}`]: c }) @@ -306,7 +307,7 @@ class JsonMerge extends Json { } class PackageJson extends JsonMerge { - static types = ['pkg.json'] + static types = ['package.json'] async prepare (s, t) { // merge new source with current pkg content @@ -348,15 +349,28 @@ const Parsers = { PackageJson, } -const parserLookup = Object.values(Parsers) +// Create an order to lookup parsers based on filename the only important part +// of ordering is that we want to match types by exact match first, then globs, +// so we always sort globs to the bottom +const parserLookup = [] +for (const parser of Object.values(Parsers)) { + for (const type of parser.types) { + const parserEntry = [type, parser] + if (type.includes('*')) { + parserLookup.push(parserEntry) + } else { + parserLookup.unshift(parserEntry) + } + } +} const getParser = (file) => { - const base = basename(file).toLowerCase() - const ext = extname(file).slice(1).toLowerCase() - - return parserLookup.find((p) => p.types.includes(base)) - || parserLookup.find((p) => p.types.includes(ext)) - || Parsers.Base + for (const [type, parser] of parserLookup) { + if (minimatch(file, type, { nocase: true, dot: true, matchBase: true })) { + return parser + } + } + return Parsers.Base } module.exports = getParser diff --git a/package.json b/package.json index ba4c320a..dac5871d 100644 --- a/package.json +++ b/package.json @@ -50,6 +50,7 @@ "just-deep-map-values": "^1.1.1", "just-diff": "^6.0.0", "lodash": "^4.17.21", + "minimatch": "^9.0.2", "npm-package-arg": "^10.0.0", "proc-log": "^3.0.0", "release-please": "npm:@npmcli/release-please@^14.2.6", From 912f78168c2b45b3038ff667189c487255556329 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Wed, 5 Jul 2023 22:45:39 -0700 Subject: [PATCH 2/2] feat: add overwrite false property to added files By default template-oss allows for written files to be configured on a per-repo basis. This is helpful for different repos to create their own templated files and apply those. With this change, a repo can now set overwrite: false to a templated file and have those updates be applied after the default template-oss changes are made. --- lib/apply/apply-files.js | 2 +- lib/check/check-apply.js | 3 +- lib/config.js | 8 ++-- lib/util/files.js | 72 ++++++++++++++++++++++++--------- lib/util/merge.js | 75 +++++++++++++++++++++++++++++------ lib/util/parser.js | 2 +- test/apply/overwrite-false.js | 40 +++++++++++++++++++ 7 files changed, 165 insertions(+), 37 deletions(-) create mode 100644 test/apply/overwrite-false.js diff --git a/lib/apply/apply-files.js b/lib/apply/apply-files.js index af8034a2..b852b0a3 100644 --- a/lib/apply/apply-files.js +++ b/lib/apply/apply-files.js @@ -9,7 +9,7 @@ const run = async (dir, files, options) => { await rmEach(dir, rm, options, (f) => fs.rm(f)) log.verbose('apply-files', 'add', add) - await parseEach(dir, add, options, (p) => p.applyWrite()) + await parseEach(dir, add, options, {}, (p) => p.applyWrite()) } module.exports = [{ diff --git a/lib/check/check-apply.js b/lib/check/check-apply.js index c76399bb..13b882e9 100644 --- a/lib/check/check-apply.js +++ b/lib/check/check-apply.js @@ -12,7 +12,8 @@ const run = async (type, dir, files, options) => { const { add: addFiles, rm: rmFiles } = files const rm = await rmEach(dir, rmFiles, options, (f) => rel(f)) - const [add, update] = partition(await parseEach(dir, addFiles, options, async (p) => { + const parseOpts = { allowMultipleSources: false } + const [add, update] = partition(await parseEach(dir, addFiles, options, parseOpts, async (p) => { const diff = await p.applyDiff() const target = rel(p.target) if (diff === null) { diff --git a/lib/config.js b/lib/config.js index 0b2cb1b6..d5acc35a 100644 --- a/lib/config.js +++ b/lib/config.js @@ -4,8 +4,8 @@ const semver = require('semver') const parseCIVersions = require('./util/parse-ci-versions.js') const getGitUrl = require('./util/get-git-url.js') const gitignore = require('./util/gitignore.js') -const { withArrays } = require('./util/merge.js') -const { FILE_KEYS, parseConfig: parseFiles, getAddedFiles } = require('./util/files.js') +const { mergeWithArrays } = require('./util/merge.js') +const { FILE_KEYS, parseConfig: parseFiles, getAddedFiles, mergeFiles } = require('./util/files.js') const CONFIG_KEY = 'templateOSS' const getPkgConfig = (pkg) => pkg[CONFIG_KEY] || {} @@ -14,7 +14,7 @@ const { name: NAME, version: LATEST_VERSION } = require('../package.json') const MERGE_KEYS = [...FILE_KEYS, 'defaultContent', 'content'] const DEFAULT_CONTENT = require.resolve(NAME) -const merge = withArrays('branches', 'distPaths', 'allowPaths', 'ignorePaths') +const merge = mergeWithArrays('branches', 'distPaths', 'allowPaths', 'ignorePaths') const makePosix = (v) => v.split(win32.sep).join(posix.sep) const deglob = (v) => makePosix(v).replace(/[/*]+$/, '') @@ -120,7 +120,7 @@ const getFullConfig = async ({ // Files get merged in from the default content (that template-oss provides) as well // as any content paths provided from the root or the workspace const fileDirs = uniq([useDefault && defaultDir, rootDir, pkgDir].filter(Boolean)) - const files = merge(useDefault && defaultFiles, rootFiles, pkgFiles) + const files = mergeFiles(useDefault && defaultFiles, rootFiles, pkgFiles) const repoFiles = isRoot ? files.rootRepo : files.workspaceRepo const moduleFiles = isRoot ? files.rootModule : files.workspaceModule diff --git a/lib/util/files.js b/lib/util/files.js index e0abb5e7..3b2b5723 100644 --- a/lib/util/files.js +++ b/lib/util/files.js @@ -1,27 +1,62 @@ const { join } = require('path') -const { defaultsDeep } = require('lodash') -const merge = require('./merge.js') +const { defaultsDeep, omit } = require('lodash') const deepMapValues = require('just-deep-map-values') const { glob } = require('glob') +const { mergeWithCustomizers, customizers } = require('./merge.js') const Parser = require('./parser.js') const template = require('./template.js') +const ADD_KEY = 'add' +const RM_KEY = 'rm' const FILE_KEYS = ['rootRepo', 'rootModule', 'workspaceRepo', 'workspaceModule'] const globify = pattern => pattern.split('\\').join('/') -const fileEntries = (dir, files, options) => Object.entries(files) - // remove any false values - .filter(([_, v]) => v !== false) - // target paths need to be joinsed with dir and templated - .map(([k, source]) => { - const target = join(dir, template(k, options)) - return [target, source] - }) +const mergeFiles = mergeWithCustomizers((value, srcValue, key, target, source, stack) => { + // This will merge all files except if the src file has overwrite:false. Then + // the files will be turned into an array so they can be applied on top of + // each other in the parser. + if ( + stack[0] === ADD_KEY && + FILE_KEYS.includes(stack[1]) && + value?.file && + srcValue?.overwrite === false + ) { + return [value, omit(srcValue, 'overwrite')] + } +}, customizers.overwriteArrays) + +const fileEntries = (dir, files, options, { allowMultipleSources = true } = {}) => { + const results = [] + + for (const [key, source] of Object.entries(files)) { + // remove any false values first since that means those targets are skipped + if (source === false) { + continue + } + + // target paths need to be joinsed with dir and templated + const target = join(dir, template(key, options)) + + if (Array.isArray(source)) { + // When turning an object of files into all its entries, we allow + // multiples when applying changes, but not when checking for changes + // since earlier files would always return as needing an update. So we + // either allow multiples and return the array or only return the last + // source file in the array. + const sources = allowMultipleSources ? source : source.slice(-1) + results.push(...sources.map(s => [target, s])) + } else { + results.push([target, source]) + } + } + + return results +} // given an obj of files, return the full target/source paths and associated parser -const getParsers = (dir, files, options) => { - const parsers = fileEntries(dir, files, options).map(([target, source]) => { +const getParsers = (dir, files, options, parseOptions) => { + const parsers = fileEntries(dir, files, options, parseOptions).map(([target, source]) => { const { file, parser, filter, clean: shouldClean } = source if (typeof filter === 'function' && !filter(options)) { @@ -62,9 +97,9 @@ const rmEach = async (dir, files, options, fn) => { return res.filter(Boolean) } -const parseEach = async (dir, files, options, fn) => { +const parseEach = async (dir, files, options, parseOptions, fn) => { const res = [] - for (const parser of getParsers(dir, files, options)) { + for (const parser of getParsers(dir, files, options, parseOptions)) { res.push(await fn(parser)) } return res.filter(Boolean) @@ -72,7 +107,7 @@ const parseEach = async (dir, files, options, fn) => { const parseConfig = (files, dir, overrides) => { const normalizeFiles = (v) => deepMapValues(v, (value, key) => { - if (key === 'rm' && Array.isArray(value)) { + if (key === RM_KEY && Array.isArray(value)) { return value.reduce((acc, k) => { acc[k] = true return acc @@ -88,16 +123,16 @@ const parseConfig = (files, dir, overrides) => { return value }) - const merged = merge(normalizeFiles(files), normalizeFiles(overrides)) + const merged = mergeFiles(normalizeFiles(files), normalizeFiles(overrides)) const withDefaults = defaultsDeep(merged, FILE_KEYS.reduce((acc, k) => { - acc[k] = { add: {}, rm: {} } + acc[k] = { [ADD_KEY]: {}, [RM_KEY]: {} } return acc }, {})) return withDefaults } -const getAddedFiles = (files) => files ? Object.keys(files.add || {}) : [] +const getAddedFiles = (files) => files ? Object.keys(files[ADD_KEY] || {}) : [] module.exports = { rmEach, @@ -105,4 +140,5 @@ module.exports = { FILE_KEYS, parseConfig, getAddedFiles, + mergeFiles, } diff --git a/lib/util/merge.js b/lib/util/merge.js index 90646b82..6395b925 100644 --- a/lib/util/merge.js +++ b/lib/util/merge.js @@ -1,21 +1,72 @@ -const { mergeWith } = require('lodash') +const { mergeWith: _mergeWith } = require('lodash') -const merge = (...objects) => mergeWith({}, ...objects, (value, srcValue, key) => { - if (Array.isArray(srcValue)) { - // Dont merge arrays, last array wins - return srcValue - } -}) +// Adapted from https://github.com/lodash/lodash/issues/3901#issuecomment-517983996 +// Allows us to keep track of the current key during each merge so a customizer +// can make different merges based on the parent keys. +const mergeWith = (...args) => { + const customizer = args.pop() + const objects = args + const sourceStack = [] + const keyStack = [] + return _mergeWith({}, ...objects, (value, srcValue, key, target, source) => { + let currentKeys + while (true) { + if (!sourceStack.length) { + sourceStack.push(source) + keyStack.push([]) + } + if (source === sourceStack[sourceStack.length - 1]) { + currentKeys = keyStack[keyStack.length - 1].concat(key) + sourceStack.push(srcValue) + keyStack.push(currentKeys) + break + } + sourceStack.pop() + keyStack.pop() + } + // Remove the last key since that is the current one and reverse the whole + // array so that the first entry is the parent, 2nd grandparent, etc + return customizer(value, srcValue, key, target, source, currentKeys.slice(0, -1).reverse()) + }) +} + +// Create a merge function that will run a set of customizer functions +const mergeWithCustomizers = (...customizers) => { + return (...objects) => mergeWith({}, ...objects, (...args) => { + for (const customizer of customizers) { + const result = customizer(...args) + // undefined means the customizer will defer to the next one + // the default behavior of undefined in lodash is to merge + if (result !== undefined) { + return result + } + } + }) +} -const mergeWithArrays = (...keys) => - (...objects) => mergeWith({}, ...objects, (value, srcValue, key) => { +const customizers = { + // Dont merge arrays, last array wins + overwriteArrays: (value, srcValue) => { + if (Array.isArray(srcValue)) { + return srcValue + } + }, + // Merge arrays if their key matches one of the passed in keys + mergeArrays: (...keys) => (value, srcValue, key) => { if (Array.isArray(srcValue)) { if (keys.includes(key)) { return (Array.isArray(value) ? value : []).concat(srcValue) } return srcValue } - }) + }, +} -module.exports = merge -module.exports.withArrays = mergeWithArrays +module.exports = { + // default merge is to overwrite arrays + merge: mergeWithCustomizers(customizers.overwriteArrays), + mergeWithArrays: (...keys) => mergeWithCustomizers(customizers.mergeArrays(...keys)), + mergeWithCustomizers, + mergeWith, + customizers, +} diff --git a/lib/util/parser.js b/lib/util/parser.js index 48ba7377..3ca63e9e 100644 --- a/lib/util/parser.js +++ b/lib/util/parser.js @@ -9,7 +9,7 @@ const ini = require('ini') const { minimatch } = require('minimatch') const template = require('./template.js') const jsonDiff = require('./json-diff') -const merge = require('./merge.js') +const { merge } = require('./merge.js') const setFirst = (first, rest) => ({ ...first, ...rest }) diff --git a/test/apply/overwrite-false.js b/test/apply/overwrite-false.js new file mode 100644 index 00000000..ca0bedbd --- /dev/null +++ b/test/apply/overwrite-false.js @@ -0,0 +1,40 @@ +const t = require('tap') +const setup = require('../setup.js') + +t.test('json merge', async (t) => { + const s = await setup(t, { + ok: true, + package: { + templateOSS: { + content: 'content', + }, + }, + testdir: { + content: { + 'index.js': `module.exports=${JSON.stringify({ + rootModule: { + add: { + 'package.json': { + file: 'more-package.json', + overwrite: false, + }, + }, + }, + })}`, + 'more-package.json': JSON.stringify({ + scripts: { + test: 'tap test/', + }, + }), + }, + }, + }) + + await s.apply() + + const pkg = await s.readJson('package.json') + t.equal(pkg.scripts.test, 'tap test/') + t.equal(pkg.scripts.snap, 'tap') + + t.strictSame(await s.check(), []) +})