apache / cordova-ios

Apache Cordova iOS
https://cordova.apache.org/
Apache License 2.0
2.15k stars 986 forks source link

cordova-ios@7 breaks cordova-node-xcode addBuildPhase #1357

Closed dpolivy closed 9 months ago

dpolivy commented 10 months ago

Bug Report

Problem

I have an after_prepare hook that adds a new build phase to the Xcode project file. This worked fine on cordova-ios up to v6.3.0 but now fails on 7.0.0. While this may be a bug in cordova-node-xcode, it is exposed by changes to the project file in cordova-ios. I'm not sure if the project template structure can be adjusted to resolve this? It seems that some elements (e.g. Assets.xcassets) are now missing a path variable which is causing an exception in the addBuildPhase function.

What is expected to happen?

After adding the hook and running cordova prepare ios, the build phase should get added to the project file.

What does actually happen?

$ cordova platform add ios
Using cordova-fetch for cordova-ios
Adding ios project...
Creating Cordova project for the iOS platform:
    Path: platforms/ios
    Package: com.hook.test
    Name: Test
iOS project created with cordova-ios@7.0.0
Platform Root: /Users/dan/Projects/hooktest/platforms/ios
Project Name: Test
Project location: /Users/dan/Projects/hooktest/platforms/ios/Test.xcodeproj/project.pbxproj
TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received undefined
    at new NodeError (node:internal/errors:399:5)
    at validateString (node:internal/validators:163:11)
    at Object.basename (node:path:1309:5)
    at new pbxFile (/Users/dan/Projects/hooktest/node_modules/xcode/lib/pbxFile.js:189:26)
    at pbxProject.addBuildPhase (/Users/dan/Projects/hooktest/node_modules/xcode/lib/pbxProject.js:921:26)
    at pbxProject.<anonymous> (/Users/dan/Projects/hooktest/script/addbuildphase.js:86:26)
    at pbxProject.emit (node:events:513:28)
    at pbxProject.<anonymous> (/Users/dan/Projects/hooktest/node_modules/xcode/lib/pbxProject.js:48:18)
    at ChildProcess.emit (node:events:513:28)
    at emit (node:internal/child_process:937:14) {
  code: 'ERR_INVALID_ARG_TYPE'
}

Information

The issue is related to two assets in the project file: Assets.xcassets and CDVLaunchScreen.storyboard are missing a path parameter in cordova-ios@7:

For example, this line: https://github.com/apache/cordova-ios/blob/master/templates/project/__PROJECT_NAME__.xcodeproj/project.pbxproj#L61

Should be modified to include a path variable like so: 0207DA571B56EA530066E2B4 / Assets.xcassets / = {isa = PBXFileReference; lastKnownFileType = folder.assetcatalog; name = Assets.xcassets; path = Project/Assets.xcassets; sourceTree = ""; };

Command or Code

const fs = require('fs');
const path = require('path');
const xcode = require('xcode');
const deferral = require('q').defer();

const BUILD_PHASE_NAME = 'Settings Bundle Version Update';

/**
 * Finds the Xcode project for the app
 */
function findXCodeProjectNameIn(projectPath) {
    var files = fs.readdirSync(projectPath).filter(function(elm) {
        return elm.match(/.*\.xcodeproj/ig);
    });

    if (files.length > 1) {
        console.log('WARNING: Found multiple .xcodeproj directories!');
    }
    else if (files.length === 0) {
        console.log('ERROR: No Xcode project found.');
        return;
    }

    return files[0].replace('.xcodeproj', '');
}

module.exports = function(ctx) {
    if (ctx.opts.platforms.indexOf('ios') < 0) {
        return;
    }

    var platforms = ctx.cordova.platforms['ios'].parse;

    var platformRoot = path.join(ctx.opts.projectRoot, 'platforms/ios');
    var projName = findXCodeProjectNameIn(platformRoot);

    if (!projName) return;

    console.log('Platform Root:', platformRoot);
    console.log('Project Name:', projName);

    var projFileLocation = path.join(platformRoot, projName + '.xcodeproj', 'project.pbxproj');
    var myProj = xcode.project(projFileLocation);

    console.log('Project location:', projFileLocation);

    // Parse the project file
    myProj.parse(function (err) {
        var scriptBuildPhases = myProj.hash.project.objects['PBXShellScriptBuildPhase'];

        // See if the build phase already exists
        for (var phase in scriptBuildPhases) {
            if (scriptBuildPhases[phase].name === '"' + BUILD_PHASE_NAME + '"') {
                deferral.resolve();
                console.log('Settings: Build phase already exists in ' + projFileLocation)
                return;
            }
        }

        var options = {
            shellPath: '/bin/sh',
            shellScript: 'app_version=`/usr/libexec/PlistBuddy -c \"Print CFBundleShortVersionString\" $SRCROOT/' + projName + '/' + projName + '-Info.plist`\\n\\napp_bundle_version=`/usr/libexec/PlistBuddy -c \"Print CFBundleVersion\" $SRCROOT/' + projName + '/' + projName + '-Info.plist`\\n\\n/usr/libexec/PlistBuddy \"$SRCROOT/' + projName + '/Resources/Settings.bundle/Root.plist\" -c \"set PreferenceSpecifiers:0:DefaultValue $app_version ($app_bundle_version)\"'
        };

        try {
            var newPhase = myProj.addBuildPhase([], 'PBXShellScriptBuildPhase', BUILD_PHASE_NAME, myProj.getFirstTarget().uuid, options);
        } catch (e) { console.log(e) }

        // Move the new phase to the top so it runs before the build
        var buildPhases = myProj.pbxNativeTargetSection()[myProj.getFirstTarget().uuid].buildPhases;
        for (var phase in buildPhases) {
            if (buildPhases[phase].value === newPhase.uuid) {
                buildPhases.splice(0, 0, buildPhases.splice(phase, 1)[0]);
            }
        }

        fs.writeFileSync(projFileLocation, myProj.writeSync());
        console.log('Settings: Added build phase to project ' + projFileLocation);
        deferral.resolve();
    });

    return deferral.promise;
};

Environment, Platform, Device

cordova-ios@7

Version information

Cordova CLI 12 cordova-ios 7

Checklist

dpogue commented 10 months ago

Ugh, this is what happens when you let Xcode touch project files 😩

I see most things with path, a few with name, and even some with both path and name. I think it makes sense to convert those few top-level files (by hand!) to use path for consistency.

dpolivy commented 10 months ago

Awesome, thanks for the prompt response to this! On the surface your fix looks good to me. What's the path to release for this? Is there a point release of cordova-ios scheduled anytime soon?