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.

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

config-file is merged into wrong *-Info.plist when other plugin has podspec. #144

Closed
jamesyoon11 opened this issue May 13, 2020 · 5 comments
Labels
bug info-needed Further information is requested

Comments

@jamesyoon11
Copy link

jamesyoon11 commented May 13, 2020

Bug Report

The config-file in plug.xml is merged into wrong *-info.plist when it has installed Pods.

Here is the step to reproduce

npx cordova create SampleApp com.sample.app SampleApp
cd SampleApp/
npx cordova platform add ios
npx cordova plugin add cordova-plugin-facebook4
npx cordova plugin add cordova-plugin-background-mode

cordova-plugin-facebook4 installs cocoaPods Facebook SDK.

cordova-plugin-background-mode has UIBackgroundMode config-file:

<config-file target="*-Info.plist" parent="UIBackgroundModes">
            <array>
                <string>audio</string>
            </array>
</config-file>

It should have merged into SampleApp-Info.plist but it merged into Pods/Target Support Files/Pods-SampleApp/Pods-SampleApp-Info.plist.

Screen Shot 2020-05-13 at 3 10 13 PM

However, if the project name is lower than 'Pods' (ex 'MyApp'), the issue does not happen.

@EvelinJK
Copy link

EvelinJK commented Jun 5, 2020

This should theoretically have been fixed with version 4.0.0, released on March 26th, 2020.
See "GH-124" here: https://cordova.apache.org/announcements/2020/03/30/cordova-common-release-4.0.0.html

It only happens if you use CocoaPods version greater 1.6.

When the target is specified as *-Info.plist and your app name starts with a letter that comes after "P", it will find the wrong .plist file and write into that instead of the correct one. That's because there's also a "YourProjectName-Info.plist" in the "Pods" folder, which will be found first in this case.

Also see the issue where the bug was originally identified: #88

@raphinesse
Copy link
Contributor

@jamesyoon11 As @EvelinJK mentioned, this should be fixed in cordova-common@4.0.0. Now I am not sure if cordova-lib or cordova-ios performs the merging. So could you try and see if you can reproduce the bug using cordova@nightly and cordova-ios@6 (or nightly too)?

@raphinesse raphinesse added bug info-needed Further information is requested labels Jun 5, 2020
@dpa99c dpa99c changed the title config-file is merged into wrong *-Info.plist when other plugin has podsec. config-file is merged into wrong *-Info.plist when other plugin has podspec. Jun 11, 2020
@dpa99c
Copy link
Contributor

dpa99c commented Jun 12, 2020

This is still an issue with cordova-common@4.0.1 / cordova-ios@6:

If the app <name> in config.xml starts with a name which places it before Pods in the alphabet, then the issue manifests.

I've created a repo to a reproducible test case: https://github.com/dpa99c/cordova-common-issue-144

It contains 2 test plugins - both of which use a <podspec> to add an arbitrary cocoapod to the project and both of which insert values into the app .plist.

First clone the repo:

% git clone https://github.com/dpa99c/cordova-common-issue-144 && cd cordova-common-issue-144

Now checkout the branch which contains the app name "Podr" (one letter before "Pods" in the alphabet):

% git checkout Podr

Now let's add cordova-ios@6 platform:

% cordova platform add ios

And we confirm we're using the expected versions of cordova-ios and cordova-common:

% npm list cordova-common
helloworld@1.0.0 /projects/@scratch/temp/cordova-common-issue-144
└─┬ cordova-ios@6.0.0
  └── cordova-common@4.0.1 

We're expecting to see that both our plugins add their entries to the app info plist.
Because our app name is "Podr", our app list will be platforms/ios/Podr/Podr-Info.plist.

So if we search for our first plugin entry:

% grep -r "TestPluginA-present" platforms/ios | grep plist

We should see:

platforms/ios/Podr/Podr-Info.plist:	<string>TestPluginA-present</string>

Same for our second plugin:

% grep -r "TestPluginB-present" platforms/ios | grep plist		
platforms/ios/Podr/Podr-Info.plist:	<string>TestPluginB-present</string>

Now checkout the "Podt" branch (one letter after "Pods" in the alphabet)

% git checkout Podt

Remove and re-add the ios platform:

% cordova platform rm ios --nosave && cordova platform add ios --nosave

This time when we search for the first plugin plist entry, the result is unexpected:

% grep -r "TestPluginA-present" platforms/ios | grep plist
platforms/ios/Podt/Podt-Info.plist:	<string>TestPluginA-present</string>
platforms/ios/Pods/Target Support Files/Pods-Podt/Pods-Podt-Info.plist:	<string>TestPluginA-present</string>

We see it's in both the app plist as expected, but also in the Pods plist.

And the second plugin:

% grep -r "TestPluginB-present" platforms/ios | grep plist
platforms/ios/Pods/Target Support Files/Pods-Podt/Pods-Podt-Info.plist:	<string>TestPluginB-present</string>

Is only present in the Pods plist - it's missing from the app plist.

To confirm that the <podspec> in the first plugin is responsible for this, check out the "plugin-A-no-pods" branch in which the <podspec> is commented out in the plugin.xml of the first plugin:

% git checkout plugin-A-no-pods

The app name is "Z" on this branch, which of course comes after "Pods" in the alphabet, so we'd expect to see the issue manifest (you can confirm this by checking out the "Z" branch which has the same app name but no commented out <podspec>).

Remove and re-add the ios platform:

% cordova platform rm ios --nosave && cordova platform add ios --nosave

As before, the first plugin entry is present in both plists:

% grep -r "TestPluginA-present" platforms/ios | grep plist
platforms/ios/Z/Z-Info.plist:	<string>TestPluginA-present</string>
platforms/ios/Pods/Target Support Files/Pods-Z/Pods-Z-Info.plist:	<string>TestPluginA-present</string>

But the second plugin entry is now also present in both plists:

% grep -r "TestPluginB-present" platforms/ios | grep plist
platforms/ios/Z/Z-Info.plist:	<string>TestPluginB-present</string>
platforms/ios/Pods/Target Support Files/Pods-Z/Pods-Z-Info.plist:	<string>TestPluginB-present</string>

This issue is affecting a production project of mine, so I'm keen to find a fix (currently my workaround is to manually move the missing keys from the Pods plist to the app plist).
If I'm able to track down/fix the issue, I'll submit a PR.

dpa99c added a commit to dpa99c/cordova-common that referenced this issue Jun 12, 2020
… are present (apache#144).

When multiple plist files exists in a cordova-ios project (e.g. due to a plugin containing `<podspec>`), ConfigFile was updated under CB-5989 to select the app plist as the target for changes destined for *-Info.plist.
However, the change made under CB-5989 incorrectly constructed the path to the app plist by omitting the project name subdirectory from the path, causing the fix to fail to work.

This commit fixes this by correcting the constructed path to the app plist.
dpa99c added a commit to dpa99c/cordova-ios that referenced this issue Jun 12, 2020
@dpa99c
Copy link
Contributor

dpa99c commented Jun 12, 2020

Oops, that was implicit - didn't mean to close without review

@dpa99c dpa99c reopened this Jun 12, 2020
dpa99c added a commit to dpa99c/cordova-common that referenced this issue Jun 12, 2020
… files are present (apache#144).

When multiple plist files exists in a cordova-ios project (e.g. due to a plugin containing `<podspec>`), ConfigFile was updated under CB-5989 to select the app plist as the target for changes destined for *-Info.plist.
However, the change made under CB-5989 incorrectly constructed the path to the app plist by omitting the project name subdirectory from the path, causing the fix to fail to work.

This commit fixes this by correcting the constructed path to the app plist.
@dpa99c
Copy link
Contributor

dpa99c commented Jun 12, 2020

The above PR fixes this issue. To confirm, run the following test (continuing on from my test scenarios above):

% git checkout Podt-fixed

This checks out a branch where the app name is still "Podt" but it references my fork of cordova-ios which in turn references my fork on cordova-common which contains the fix.

Re-add the platform using my fork:

% cordova platform rm ios --nosave && cordova platform add ios --nosave

Now search for the plugin plist strings:

% grep -r "TestPluginA-present" platforms/ios | grep plist
platforms/ios/Podt/Podt-Info.plist:	<string>TestPluginA-present</string>

% grep -r "TestPluginB-present" platforms/ios | grep plist
platforms/ios/Podt/Podt-Info.plist:	<string>TestPluginB-present</string>

They are now in the expected app plist location.

dpa99c added a commit to dpa99c/cordova-common that referenced this issue Jun 12, 2020
… files are present (apache#144).

When multiple plist files exists in a cordova-ios project (e.g. due to a plugin containing `<podspec>`), ConfigFile was updated under CB-5989 to select the app plist as the target for changes destined for *-Info.plist.
However, the change made under CB-5989 incorrectly constructed the path to the app plist by omitting the project name subdirectory from the path, causing the fix to fail to work.

This commit fixes this by correcting the constructed path to the app plist.
dpa99c added a commit that referenced this issue Jun 12, 2020
fix: resolve correct path to app info plist when multiple plist files are present (#144)
@dpa99c dpa99c closed this as completed Jun 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug info-needed Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants