From 615db8e0c2b633768503a163c9bbc80f1632ac75 Mon Sep 17 00:00:00 2001 From: Darryl Pogue Date: Fri, 23 Aug 2024 00:59:30 -0700 Subject: [PATCH 1/4] feat: Support finding an unprefixed Info.plist file --- spec/ConfigChanges/ConfigFile.spec.js | 24 +++++++++++++++-- src/ConfigChanges/ConfigFile.js | 38 ++++++++++++++++++++------- 2 files changed, 51 insertions(+), 11 deletions(-) diff --git a/spec/ConfigChanges/ConfigFile.spec.js b/spec/ConfigChanges/ConfigFile.spec.js index c5e7958..c95e601 100644 --- a/spec/ConfigChanges/ConfigFile.spec.js +++ b/spec/ConfigChanges/ConfigFile.spec.js @@ -78,14 +78,19 @@ describe('ConfigFile tests', function () { expect(ConfigFile.resolveConfigFilePath('project_dir', 'android', 'strings.xml')).toBe(stringsPath); }); + it('should return android values xml file path', function () { + const resPath = path.join(projectDir, 'res', 'values', 'colors.xml'); + expect(ConfigFile.resolveConfigFilePath('project_dir', 'android', path.join('res', 'values', 'colors.xml'))).toBe(resPath); + }); + it('should return ios config.xml file path', function () { - spyOn(ConfigFile, 'getIOSProjectname').and.returnValue('iospath'); + ConfigFile.__set__('getIOSProjectname', () => 'iospath'); const configPath = path.join('project_dir', 'iospath', 'config.xml'); expect(ConfigFile.resolveConfigFilePath('project_dir', 'ios', 'config.xml')).toBe(configPath); }); it('should return osx config.xml file path', function () { - spyOn(ConfigFile, 'getIOSProjectname').and.returnValue('osxpath'); + ConfigFile.__set__('getIOSProjectname', () => 'osxpath'); const configPath = path.join('project_dir', 'osxpath', 'config.xml'); expect(ConfigFile.resolveConfigFilePath('project_dir', 'osx', 'config.xml')).toBe(configPath); }); @@ -116,6 +121,21 @@ describe('ConfigFile tests', function () { expect(ConfigFile.resolveConfigFilePath('', 'ios', '*-Info.plist')).toBe(expectedPlistPath); }); + + it('should return Info.plist file', function () { + const projName = 'XXX'; + const expectedPlistPath = path.join(projName, 'Info.plist'); + + ConfigFile.__set__('getIOSProjectname', () => projName); + spyOn(require('fast-glob'), 'sync').and.returnValue([ + 'AAA/Info.plist', + `Pods/Target Support Files/Pods-${projName}/Info.plist`, + `Pods/Target Support Files/Pods-${projName}/Pods-${projName}-Info.plist`, + expectedPlistPath + ]); + + expect(ConfigFile.resolveConfigFilePath('', 'ios', '*-Info.plist')).toBe(expectedPlistPath); + }); }); }); }); diff --git a/src/ConfigChanges/ConfigFile.js b/src/ConfigChanges/ConfigFile.js index bc2673c..80c2765 100644 --- a/src/ConfigChanges/ConfigFile.js +++ b/src/ConfigChanges/ConfigFile.js @@ -158,7 +158,6 @@ class ConfigFile { // Some config-file target attributes are not qualified with a full leading directory, or contain wildcards. // Resolve to a real path in this function. -// TODO: getIOSProjectname is slow because of glob, try to avoid calling it several times per project. function resolveConfigFilePath (project_dir, platform, file) { let filepath = path.join(project_dir, file); let matches; @@ -173,14 +172,26 @@ function resolveConfigFilePath (project_dir, platform, file) { absolute: true }).map(path.normalize); - if (matches.length) filepath = matches[0]; + if (matches.length) { + filepath = matches[0]; + } - // [CB-5989] multiple Info.plist files may exist. default to $PROJECT_NAME-Info.plist + // [CB-5989] multiple Info.plist files may exist. default to Info.plist then $PROJECT_NAME-Info.plist if (matches.length > 1 && file.includes('-Info.plist')) { const projName = getIOSProjectname(project_dir); + + // Try to find a unprefix Info.plist file first + let plistPath = path.join(project_dir, projName, 'Info.plist'); + if (matches.includes(plistPath)) { + return plistPath; + } + + // Then try to find one prefixed with the project name const plistName = `${projName}-Info.plist`; - const plistPath = path.join(project_dir, projName, plistName); - if (matches.includes(plistPath)) return plistPath; + plistPath = path.join(project_dir, projName, plistName); + if (matches.includes(plistPath)) { + return plistPath; + } } return filepath; } @@ -216,7 +227,7 @@ function resolveConfigFilePath (project_dir, platform, file) { if (platform === 'ios' || platform === 'osx') { filepath = path.join( project_dir, - module.exports.getIOSProjectname(project_dir), + getIOSProjectname(project_dir), 'config.xml' ); } else { @@ -235,9 +246,16 @@ function resolveConfigFilePath (project_dir, platform, file) { return filepath; } -// Find out the real name of an iOS or OSX project -// TODO: glob is slow, need a better way or caching, or avoid using more than once. +const xcodeprojMap = new Map(); +// Find out the real name of an iOS project +// +// This caches the project name for a given directory path, assuming that it +// won't change over the course of a single Cordova command invocation function getIOSProjectname (project_dir) { + if (xcodeprojMap.has(project_dir)) { + return xcodeprojMap.get(project_dir); + } + const matches = modules.glob.sync('*.xcodeproj', { cwd: project_dir, onlyDirectories: true }); if (matches.length !== 1) { @@ -248,7 +266,9 @@ function getIOSProjectname (project_dir) { throw new Error(`${msg} in ${project_dir}`); } - return path.basename(matches[0], '.xcodeproj'); + const projectName = path.basename(matches[0], '.xcodeproj'); + xcodeprojMap.set(project_dir, projectName); + return projectName; } // determine if a plist file is binary From 1e7d18ecb67a7838294ed5b95bb0c76bb0ca9c9c Mon Sep 17 00:00:00 2001 From: Darryl Pogue Date: Fri, 23 Aug 2024 09:26:03 -0700 Subject: [PATCH 2/4] fix: Avoid reading plist files twice This will also allow us to drop the read-chunk dependency. --- spec/ConfigChanges/ConfigChanges.spec.js | 2 +- src/ConfigChanges/ConfigFile.js | 18 ++++++++++-------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/spec/ConfigChanges/ConfigChanges.spec.js b/spec/ConfigChanges/ConfigChanges.spec.js index 6085591..ccaee9f 100644 --- a/spec/ConfigChanges/ConfigChanges.spec.js +++ b/spec/ConfigChanges/ConfigChanges.spec.js @@ -508,7 +508,7 @@ describe('config-changes module', function () { const munger = new configChanges.PlatformMunger('ios', temp, platformJson, pluginInfoProvider); munger.process(plugins_dir); - expect(spy).toHaveBeenCalledWith(path.join(temp, 'SampleApp', 'SampleApp-Info.plist'), 'utf8'); + expect(spy).toHaveBeenCalledWith(path.join(temp, 'SampleApp', 'SampleApp-Info.plist')); }); it('Test 026 : should move successfully installed plugins from queue to installed plugins section, and include/retain vars if applicable', function () { diff --git a/src/ConfigChanges/ConfigFile.js b/src/ConfigChanges/ConfigFile.js index 80c2765..6e7de09 100644 --- a/src/ConfigChanges/ConfigFile.js +++ b/src/ConfigChanges/ConfigFile.js @@ -16,6 +16,7 @@ const fs = require('node:fs'); const path = require('node:path'); +const util = require('node:util'); const readChunk = require('read-chunk'); // Use delay loading to ensure plist and other node modules to not get loaded @@ -73,13 +74,13 @@ class ConfigFile { } else { // plist file this.type = 'plist'; - // TODO: isBinaryPlist() reads the file and then parse re-reads it again. - // We always write out text plist, not binary. - // Do we still need to support binary plist? - // If yes, use plist.parseStringSync() and read the file once. - this.data = isBinaryPlist(filepath) - ? modules.bplist.parseBuffer(fs.readFileSync(filepath))[0] - : modules.plist.parse(fs.readFileSync(filepath, 'utf8')); + + const fileData = fs.readFileSync(filepath); + if (fileData.toString('utf8', 0, 6) === 'bplist') { + this.data = modules.bplist.parseBuffer(fileData)[0]; + } else { + this.data = modules.plist.parse(fileData.toString('utf8')); + } } } @@ -273,11 +274,12 @@ function getIOSProjectname (project_dir) { // determine if a plist file is binary // i.e. they start with the magic header "bplist" +// TODO: Remove in next major function isBinaryPlist (filename) { return readChunk.sync(filename, 0, 6).toString() === 'bplist'; } module.exports = ConfigFile; -module.exports.isBinaryPlist = isBinaryPlist; +module.exports.isBinaryPlist = util.deprecate(isBinaryPlist, 'isBinaryPlist is deprecated'); module.exports.getIOSProjectname = getIOSProjectname; module.exports.resolveConfigFilePath = resolveConfigFilePath; From 4bea6de5e316359f71d822474153c97c5a10589c Mon Sep 17 00:00:00 2001 From: Darryl Pogue Date: Fri, 23 Aug 2024 21:54:51 -0700 Subject: [PATCH 3/4] fix: Address feedback issues around testing Co-Authored-By: Norman Breau --- spec/ConfigChanges/ConfigFile.spec.js | 8 ++-- src/ConfigChanges/ConfigFile.js | 57 ++++++++++++++------------- 2 files changed, 33 insertions(+), 32 deletions(-) diff --git a/spec/ConfigChanges/ConfigFile.spec.js b/spec/ConfigChanges/ConfigFile.spec.js index c95e601..55f40fa 100644 --- a/spec/ConfigChanges/ConfigFile.spec.js +++ b/spec/ConfigChanges/ConfigFile.spec.js @@ -84,13 +84,13 @@ describe('ConfigFile tests', function () { }); it('should return ios config.xml file path', function () { - ConfigFile.__set__('getIOSProjectname', () => 'iospath'); + spyOn(ConfigFile, 'getIOSProjectname').and.returnValue('iospath'); const configPath = path.join('project_dir', 'iospath', 'config.xml'); expect(ConfigFile.resolveConfigFilePath('project_dir', 'ios', 'config.xml')).toBe(configPath); }); it('should return osx config.xml file path', function () { - ConfigFile.__set__('getIOSProjectname', () => 'osxpath'); + spyOn(ConfigFile, 'getIOSProjectname').and.returnValue('osxpath'); const configPath = path.join('project_dir', 'osxpath', 'config.xml'); expect(ConfigFile.resolveConfigFilePath('project_dir', 'osx', 'config.xml')).toBe(configPath); }); @@ -111,7 +111,7 @@ describe('ConfigFile tests', function () { const projName = 'XXX'; const expectedPlistPath = `${projName}${path.sep}${projName}-Info.plist`; - ConfigFile.__set__('getIOSProjectname', () => projName); + spyOn(ConfigFile, 'getIOSProjectname').and.returnValue(projName); spyOn(require('fast-glob'), 'sync').and.returnValue([ `AAA/${projName}-Info.plist`, `Pods/Target Support Files/Pods-${projName}/Info.plist`, @@ -126,7 +126,7 @@ describe('ConfigFile tests', function () { const projName = 'XXX'; const expectedPlistPath = path.join(projName, 'Info.plist'); - ConfigFile.__set__('getIOSProjectname', () => projName); + spyOn(ConfigFile, 'getIOSProjectname').and.returnValue(projName); spyOn(require('fast-glob'), 'sync').and.returnValue([ 'AAA/Info.plist', `Pods/Target Support Files/Pods-${projName}/Info.plist`, diff --git a/src/ConfigChanges/ConfigFile.js b/src/ConfigChanges/ConfigFile.js index 6e7de09..bec8adc 100644 --- a/src/ConfigChanges/ConfigFile.js +++ b/src/ConfigChanges/ConfigFile.js @@ -30,6 +30,9 @@ const modules = { get xml_helpers () { return require('../util/xml-helpers'); } }; +// Cache of project folder paths to Xcode project names +const xcodeprojMap = new Map(); + /****************************************************************************** * ConfigFile class * @@ -155,6 +158,30 @@ class ConfigFile { this.is_changed = true; } + + // Find out the real name of an iOS project + // + // This caches the project name for a given directory path, assuming that + // it won't change over the course of a single Cordova command invocation + static getIOSProjectname (project_dir) { + if (xcodeprojMap.has(project_dir)) { + return xcodeprojMap.get(project_dir); + } + + const matches = modules.glob.sync('*.xcodeproj', { cwd: project_dir, onlyDirectories: true }); + + if (matches.length !== 1) { + const msg = matches.length === 0 + ? 'Does not appear to be an xcode project, no xcode project file' + : 'There are multiple *.xcodeproj dirs'; + + throw new Error(`${msg} in ${project_dir}`); + } + + const projectName = path.basename(matches[0], '.xcodeproj'); + xcodeprojMap.set(project_dir, projectName); + return projectName; + } } // Some config-file target attributes are not qualified with a full leading directory, or contain wildcards. @@ -179,7 +206,7 @@ function resolveConfigFilePath (project_dir, platform, file) { // [CB-5989] multiple Info.plist files may exist. default to Info.plist then $PROJECT_NAME-Info.plist if (matches.length > 1 && file.includes('-Info.plist')) { - const projName = getIOSProjectname(project_dir); + const projName = ConfigFile.getIOSProjectname(project_dir); // Try to find a unprefix Info.plist file first let plistPath = path.join(project_dir, projName, 'Info.plist'); @@ -228,7 +255,7 @@ function resolveConfigFilePath (project_dir, platform, file) { if (platform === 'ios' || platform === 'osx') { filepath = path.join( project_dir, - getIOSProjectname(project_dir), + ConfigFile.getIOSProjectname(project_dir), 'config.xml' ); } else { @@ -247,31 +274,6 @@ function resolveConfigFilePath (project_dir, platform, file) { return filepath; } -const xcodeprojMap = new Map(); -// Find out the real name of an iOS project -// -// This caches the project name for a given directory path, assuming that it -// won't change over the course of a single Cordova command invocation -function getIOSProjectname (project_dir) { - if (xcodeprojMap.has(project_dir)) { - return xcodeprojMap.get(project_dir); - } - - const matches = modules.glob.sync('*.xcodeproj', { cwd: project_dir, onlyDirectories: true }); - - if (matches.length !== 1) { - const msg = matches.length === 0 - ? 'Does not appear to be an xcode project, no xcode project file' - : 'There are multiple *.xcodeproj dirs'; - - throw new Error(`${msg} in ${project_dir}`); - } - - const projectName = path.basename(matches[0], '.xcodeproj'); - xcodeprojMap.set(project_dir, projectName); - return projectName; -} - // determine if a plist file is binary // i.e. they start with the magic header "bplist" // TODO: Remove in next major @@ -281,5 +283,4 @@ function isBinaryPlist (filename) { module.exports = ConfigFile; module.exports.isBinaryPlist = util.deprecate(isBinaryPlist, 'isBinaryPlist is deprecated'); -module.exports.getIOSProjectname = getIOSProjectname; module.exports.resolveConfigFilePath = resolveConfigFilePath; From 49e3f4c9a7aa2e041dca6afa84a5d22dc32df301 Mon Sep 17 00:00:00 2001 From: Darryl Pogue Date: Thu, 29 Aug 2024 02:47:24 -0700 Subject: [PATCH 4/4] fix: Glob for *Info.plist rather than *-Info.plist --- src/ConfigChanges/ConfigFile.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ConfigChanges/ConfigFile.js b/src/ConfigChanges/ConfigFile.js index bec8adc..202b8a2 100644 --- a/src/ConfigChanges/ConfigFile.js +++ b/src/ConfigChanges/ConfigFile.js @@ -194,7 +194,7 @@ function resolveConfigFilePath (project_dir, platform, file) { if (file.includes('*')) { // handle wildcards in targets using glob. - matches = modules.glob.sync(`**/${file}`, { + matches = modules.glob.sync(`**/${file.replace('*-Info.plist', '*Info.plist')}`, { fs, cwd: project_dir, absolute: true