Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not overwrite all targets with same id #545

Closed
wants to merge 5 commits into from
+58 −8
Closed

Do not overwrite all targets with same id #545

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f3a137c
* Prepare script update build properties for the main target only. Wi…
msari-ipe-ext-1 Feb 22, 2019
160542d
* Do not use the short name to retrieve the pbx target
msari-ipe-ext-1 Feb 25, 2019
0c24c4b
Adapt style
msari-ipe-ext-1 Feb 25, 2019
342396b
More linter fixes
msari-ipe-ext-1 Feb 25, 2019
86ef135
Merge branch 'master' of github.com:apache/cordova-ios
msari-ipe-ext-1 Oct 25, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions bin/templates/scripts/cordova/build
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ var buildOpts = nopt({
'codeSignIdentity': String,
'codeSignResourceRules': String,
'provisioningProfile': String,
'multipleProvisioningProfiles': Array,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should add a new property here for multipleProvisioningProfiles vs changing the existing provisioningProfile property to accept either a single string or an array.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess thats a better approach - i will change that.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to revert the idea - merging the two options do not work and require more refactoring. So I suggest to add this additional parameter which is only used for the export plist.

'automaticProvisioning': Boolean,
'developmentTeam': String,
'packageType': String,
Expand Down
13 changes: 10 additions & 3 deletions bin/templates/scripts/cordova/lib/build.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ module.exports.run = function (buildOpts) {
var buildType = buildOpts.release ? 'release' : 'debug';
var config = buildConfig.ios[buildType];
if (config) {
['codeSignIdentity', 'codeSignResourceRules', 'provisioningProfile', 'developmentTeam', 'packageType', 'buildFlag', 'iCloudContainerEnvironment', 'automaticProvisioning'].forEach(
['codeSignIdentity', 'codeSignResourceRules', 'provisioningProfile', 'multipleProvisioningProfiles', 'developmentTeam', 'packageType', 'buildFlag', 'iCloudContainerEnvironment', 'automaticProvisioning'].forEach(
function (key) {
buildOpts[key] = buildOpts[key] || config[key];
});
Expand Down Expand Up @@ -249,8 +249,15 @@ module.exports.run = function (buildOpts) {
exportOptions.teamID = buildOpts.developmentTeam;
}

if (buildOpts.provisioningProfile && bundleIdentifier) {
exportOptions.provisioningProfiles = { [ bundleIdentifier ]: String(buildOpts.provisioningProfile) };
if ((buildOpts.provisioningProfile || buildOpts.multipleProvisioningProfiles) && bundleIdentifier) {
if (buildOpts.multipleProvisioningProfiles) {
exportOptions.provisioningProfiles = {};
for (var i = 0; i < buildOpts.multipleProvisioningProfiles.length; i++) {
exportOptions.provisioningProfiles[buildOpts.multipleProvisioningProfiles[i]['key']] = String(buildOpts.multipleProvisioningProfiles[i]['value']);
}
} else {
exportOptions.provisioningProfiles = { [bundleIdentifier]: String(buildOpts.provisioningProfile) };
}
exportOptions.signingStyle = 'manual';
}

Expand Down
51 changes: 46 additions & 5 deletions bin/templates/scripts/cordova/lib/prepare.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,15 +274,56 @@ function handleOrientationSettings (platformConfig, infoPlist) {
}
}

// Make sure only update properties from our target project
function updateBuildPropertyLocal (proj, displayName, prop, value, build) {
try {
// Check if we have a valid target - during prepare we do not have it
var target = proj.pbxTargetByName(displayName);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this definitely use the display name, and not the full name? In Cordova apps those are usually the same, but it's not guaranteed (and we provide a way to change the display name).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you give me an example how to change the name - I will test that case.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In config.xml:

<name short="MyApp">My Full Application Name</name>

In that example, "MyApp" would be the short name (which is what you're putting into the displayName variable).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes you were right - I have fixed the pbx target retrieval (tried both variants)

if (target == null || target.buildConfigurationList == null) {
proj.updateBuildProperty(prop, value, build);
} else {
var targetProjectBuildReference = target.buildConfigurationList;
// Collect the uuid's from the configuration of our target
var COMMENT_KEY = /_comment$/;
var validConfigs = [];
var configList = proj.pbxXCConfigurationList();
for (var configName in configList) {
if (!COMMENT_KEY.test(configName) && targetProjectBuildReference === configName) {
var buildVariants = configList[configName].buildConfigurations;
for (var i = 0; i < buildVariants.length; i++) {
validConfigs.push(buildVariants[i].value);
}
break;
}
}
// Only update target props
var configs = proj.pbxXCBuildConfigurationSection();
for (configName in configs) {
if (!COMMENT_KEY.test(configName)) {
if (validConfigs.indexOf(configName) === -1) {
continue;
}
var config = configs[configName];
if ((build && config.name === build) || (!build)) {
config.buildSettings[prop] = value;
}
}
}
}
} catch (e) { // fallback to default behavior on error
proj.updateBuildProperty(prop, value, build);
}
}

function handleBuildSettings (platformConfig, locations, infoPlist) {
var pkg = platformConfig.getAttribute('ios-CFBundleIdentifier') || platformConfig.packageName();
var targetDevice = parseTargetDevicePreference(platformConfig.getPreference('target-device', 'ios'));
var deploymentTarget = platformConfig.getPreference('deployment-target', 'ios');
var needUpdatedBuildSettingsForLaunchStoryboard = checkIfBuildSettingsNeedUpdatedForLaunchStoryboard(platformConfig, infoPlist);
var swiftVersion = platformConfig.getPreference('SwiftVersion', 'ios');
var displayName = platformConfig.name().replace(/"/g, '');

var project;

try {
project = projectFile.parse(locations);
} catch (err) {
Expand All @@ -299,22 +340,22 @@ function handleBuildSettings (platformConfig, locations, infoPlist) {

if (origPkg !== pkg) {
events.emit('verbose', 'Set PRODUCT_BUNDLE_IDENTIFIER to ' + pkg + '.');
project.xcode.updateBuildProperty('PRODUCT_BUNDLE_IDENTIFIER', pkg);
updateBuildPropertyLocal(project.xcode, displayName, 'PRODUCT_BUNDLE_IDENTIFIER', pkg);
}

if (targetDevice) {
events.emit('verbose', 'Set TARGETED_DEVICE_FAMILY to ' + targetDevice + '.');
project.xcode.updateBuildProperty('TARGETED_DEVICE_FAMILY', targetDevice);
updateBuildPropertyLocal(project.xcode, displayName, 'TARGETED_DEVICE_FAMILY', targetDevice);
}

if (deploymentTarget) {
events.emit('verbose', 'Set IPHONEOS_DEPLOYMENT_TARGET to "' + deploymentTarget + '".');
project.xcode.updateBuildProperty('IPHONEOS_DEPLOYMENT_TARGET', deploymentTarget);
updateBuildPropertyLocal(project.xcode, displayName, 'IPHONEOS_DEPLOYMENT_TARGET', deploymentTarget);
}

if (swiftVersion) {
events.emit('verbose', 'Set SwiftVersion to "' + swiftVersion + '".');
project.xcode.updateBuildProperty('SWIFT_VERSION', swiftVersion);
updateBuildPropertyLocal(project.xcode, displayName, 'SWIFT_VERSION', swiftVersion);
}

updateBuildSettingsForLaunchStoryboard(project.xcode, platformConfig, infoPlist);
Expand Down
1 change: 1 addition & 0 deletions bin/templates/scripts/cordova/run
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ var opts = nopt({
'codeSignIdentity': String,
'codeSignResourceRules': String,
'provisioningProfile': String,
'multipleProvisioningProfiles': Array,
'automaticProvisioning': Boolean,
'buildConfig' : String,
'noSign' : Boolean
Expand Down