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

Removing the plugin DELETES almost everything from the Android platform #898

Open
ragcsalo opened this issue Oct 5, 2018 · 16 comments
Open

Comments

@ragcsalo
Copy link

ragcsalo commented Oct 5, 2018

CLI command: cordova plugin remove cordova-plugin-firebase

Output:

Uninstalling cordova-plugin-firebase from android
Android Studio project detected
missing file :: app/src/main
Error during processing of action! Attempting to revert...
(node:3036) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejec
tion id: 1): Error: Uh oh!
ENOENT: no such file or directory, open 'C:\Android\Apps\2018\mentalpainter\plat
forms\android\AndroidManifest.xml'

Result: Almost EVERYTHING is deleted from platforms/android/app/src directory!!!!!!!!

PLEASE try to remove this Firebase plugin from your Android Studio project via CLI, and see what happens... it is a NIGHTMARE!!!!!!

Originally posted by @ragcsalo in #618 (comment)

@briantq
Copy link
Contributor

briantq commented Oct 5, 2018

@ragcsalo Please follow the bug template when opening an issue so we can get the necessary information to better try to assist you. What version of the plugin are you using?

@Jule-
Copy link

Jule- commented Oct 18, 2018

Describe the bug
Removing the plugin DELETES almost everything from the Android platform folder.

To Reproduce

$ cordova create plugtestapp com.test.plugtestapp PlugTestApp \
&& cd plugtestapp/ \
&& cordova plugin add cordova-plugin-firebase \
&& cordova platform add android \
&& cordova plugin remove cordova-plugin-firebase

All android platform folder is messed up, especially AndroidManifest.xml is removed, etc...

Expected behavior
Removing only plugin-cordova-firebase changes and not entire AndroidManifest.xml file as an example.

Console Logs

Uninstalling cordova-plugin-firebase from android
Android Studio project detected
missing file :: app/src/main
Error during processing of action! Attempting to revert...
Uh oh!
ENOENT: no such file or directory, open '~/plugtestapp/platforms/android/AndroidManifest.xml'

Plugin Version

$ node -v
v8.11.1
$ npm -v
6.1.0
$ cordova -v
8.1.2 (cordova-lib@8.1.1)
$ cordova plugin list
cordova-plugin-firebase 2.0.5 "Google Firebase Plugin"
cordova-plugin-whitelist 1.3.3 "Whitelist"

Desktop (please complete the following information):

  • OS: Windows (tested in WSL and PowerShell command prompt)
  • Android Studio (installed but not used to reproduce this bug)

Additional context
It should be linked to the plugin configuration and/or un/installation scripts. After running command cordova plugin remove cordova-plugin-firebase (and it happens only with this plugin) the only way to get back to a workable android platform workable project is to remove folder android/platform and restart all from the beginning, gradle configuration, project build, etc...

@Jule-
Copy link

Jule- commented Oct 23, 2018

Any insight/news on this one?

@briantq
Copy link
Contributor

briantq commented Oct 23, 2018

@Jule- I believe this is happening with the after_prepare script. When we modified it and did the same with in the install phase I believe it fixed the problem but caused other breakages. The script simply copies Google files over, it does not touch the Manifest. I'm wondering if there is a file locking problem with Cordova in this phase.

Since no one works on this plugin full time, it's completely maintained by community contributions. If you could take a look at that script and confirm that is the issue along with determining which part of the script is causing the issue, that would be a large help in resolving the issue.

@Jule-
Copy link

Jule- commented Oct 23, 2018

Indeed I looked into the (un)install plugin files and I see that you manage to write in the project build.gradle file what is a really bad practice.

The right way is to write a side build.gradle file which would be included in the Gradle build process with <framework /> of plugin.xml or just declare any dependency in <framework />.

And this is exactly what is done here:

<framework src="src/android/build.gradle" custom="true" type="gradleReference" />
<framework src="com.google.android.gms:play-services-tagmanager:+" />
<framework src="com.google.firebase:firebase-core:+" />
<framework src="com.google.firebase:firebase-messaging:+" />
<framework src="com.google.firebase:firebase-config:+" />
<framework src="com.google.firebase:firebase-perf:+" />

I did not really know why hooks are trying to write inside root project build.gradle but I guess that it is residual to old fashioned cordova+android way. I did not know if it is a backward compatibility issue to remove hooks but for recent cordova version we definitely should!

Plus I don't think we should use both build-extra.gradle file and plugin.xml framework dependencies like this <framework src="com.google.firebase:firebase-core:+" /> for the sanity of declaring all plugin dependencies in the same place.


Now the real issue is not there, in fact the issue is there:

<resource-file src="src/android/google-services.json" target="."/>

Should be written:

<resource-file src="src/android/google-services.json" target="app/google-services.json"/>
                                                              ------------------------

On uninstall it try and succeed to remove "."...
Moreover this line replace advantageously the after_prepare.js script which only copy google-services.json file.

I'll make a PR with these modifications but I need people in order to confirm that nothing is broken.

@briantq
Copy link
Contributor

briantq commented Oct 23, 2018

@Jule-, thanks for taking the time to look at the scripts! That is more than most people do.

I would agree with you that modifying build.gradle is not optimal. That being said, I don't think you understand the difference between the two different build.gradle files. The one at the root (modified by the install process) is actually defines the dependencies and sources for the gradle instance that builds/runs the build.gradle in the app directory. Firebase requires a plugin to be run as part of the build process, please see the Firebase Install Guide. Hence, the plugin and it's dependencies are required to be added to the root build.gradle file so they are available during the build process. Due to this requirement, both files HAVE to be modified. Since Cordova does not provide a mechanism to modify the root build.gradle, this is the only option (definitely open to other options if you know of other ways). Since the two files serve difference purposes, there is no way to consolidate dependencies.

As for the main issue, I have found that resource file entry to be misleading. The google-services.json specifies details about your Firebase account. If you look at the file in the plugin, you will find it contains dummy information (at least I hope no one has project number 123). Best I can guess is that the file was copied over to prevent build errors.

The meaningful copy comes in the after_prepare when the google-services.json is copied from the project directory, not the plugin directory. This allows consumers of this plugin to simply save their goggle-services.json in their project instead of requiring everyone to maintain their own fork. If you remove the after_prepare without some how copying the google-services.json file, you will be able to build and I believe run your project but nothing will be reported back to your firebase account, nor will you be able to use push notifications. Hence, this step is an important one.

If you have suggestions on how we might approach the problem differently, I would love to hear them. I truly believe there is an obscure bug in Cordova as we never touch the Manifest file via custom scripts but some how it is removed.

@Jule-
Copy link

Jule- commented Oct 24, 2018

Ho well! I should have look more carefully.

1/ Ok for the build.gradle modification, I see the problem. I've tried to look a little into it but it seems not possible to extend classpath from another file like I thought. I've found some stuff for the google service plugin which allow to do the include from non root gradle file but cannot find the same for io.fabric:
https://github.com/fechanique/cordova-plugin-fcm/blob/161d02dcebd0d798e017a2d16bc016b930da97b2/src/android/FCMPlugin.gradle#L11-L13

// apply plugin: 'com.google.gms.google-services'
// class must be used instead of id(string) to be able to apply plugin from non-root gradle file
apply plugin: com.google.gms.googleservices.GoogleServicesPlugin

If someone have some clue on this Class apply in gradle file maybe we could do the same for io.fabric.

2/ Ok I didn't see that coming! Of course the plugin can't add my google-services.json in plugin.xml... 🤦‍♂️
If I understand well it is for error purpose but at the same time it hides the fact that developers have to add their own so it is not really a good practice I think. Plus it does not avoid error since the gradle sync tells us that it cannot find client for our package, not mentioning google-services.json. While with no file at all the error is way clearer:

File google-services.json is missing. The Google Services Plugin cannot function without it. 
 Searched Location:
[list of searched locations]

3/ The good news is that unless I missed some stuff on my side thoughts... 🙃 I have really spotted the issue! Which is definitely this line that should be safely removed!

<resource-file src="src/android/google-services.json" target="."/>

It really causes what I said: on uninstall cordova try to remove all inside . which resolves in cordova to app/src/main/ where AndroidManifest.xml resides. Plus whatever the cause is, the target attribute should not point to a folder. In cordova when you can point safely to a folder you have target-dir attribute.

I am making a PR.

Jule- added a commit to Jule-/cordova-plugin-firebase that referenced this issue Oct 24, 2018
@Jule-
Copy link

Jule- commented Oct 24, 2018

Ok I've finished my PR which not passes tests due to global jcenter() issue. There is a workaround switching manually repositories in CordovaLib/build.gradle but it seems overkill since it should be patched soon in the cordova-android@7.1.2 release.

Please @briantq, can you take a look at this?

@BumbleeLin
Copy link

like WTH? I can't uninstall this plugin. Is there any manual fix that I can apply?

@ruano84
Copy link

ruano84 commented Apr 26, 2019

@BumbleeLin Remove the platform, remove the plugin, and install the platform again

@Jule-
Copy link

Jule- commented Apr 26, 2019

As I have suggested here on the 3rd point: #898 (comment)
I think you can manually remove the line mentioned in the plugin before trying to remove the plugin.

Or follow @ruano84 suggestion since this will obviously work as intended.

@BumbleeLin
Copy link

@ruano84 @Jule- thanks for the suggestions. I don't think removing the platform is a viable solution at this point. I have other plugins and other workarounds that I have implemented in the platforms. I will try Jule suggestion and let you guys know.

@Jule-
Copy link

Jule- commented Apr 29, 2019

And maybe for a permanent fix you could upvote my pull request which is still in pending state... 😞

@ragcsalo
Copy link
Author

As I have suggested here on the 3rd point: #898 (comment)
I think you can manually remove the line mentioned in the plugin before trying to remove the plugin.

THANKS, this is the real solution!!! :-)

@Jule-
Copy link

Jule- commented May 22, 2019

@ragcsalo I am happy that worked for you too! Hope my PR will be merged some day...

@ragcsalo
Copy link
Author

PR still not merged after 2 years.... :-(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants