dpa99c / cordova-plugin-firebasex

Cordova plugin for Google Firebase
MIT License
572 stars 462 forks source link

Plugin doesn't install if android resource file colors.xml already exists #132

Closed ir2pid closed 4 years ago

ir2pid commented 4 years ago

I get this error as another plugin has already created the colors.xml file and the plugin doesn't append but tries to copy it first

Failed to install 'cordova-plugin-firebasex': CordovaError: Uh oh!
"/Users/xx/Workspace/xx/xx-xx-app/platforms/android/app/src/main/res/values/colors.xml" already exists!

Bug report

Current behavior:

use any plugin which adds colors to the res/values/colors.xml simplest way is to create this file already. Then try to install the plugin

Expected behavior: line 59 of plugin.xml tries to copy the colors file and installation fails if it's already there. <source-file src="src/android/colors.xml" target-dir="res/values" />

Steps to reproduce:

use any plugin which adds colors to the res/values/colors.xml the simplest way is to create this file already. Then try to install the plugin

Environment information

Android build issue:

Console output

console output ``` Failed to install 'cordova-plugin-firebasex': CordovaError: Uh oh! "/Users/xx/Workspace/xx/xx-xx-app/platforms/android/app/src/main/res/values/colors.xml" already exists! ```


dpa99c commented 4 years ago

The Cordova <source-file> element doesn't handle the case if the file already exists at the target location - there's no option to merge or overwrite it.

I guess the ideal behaviour would be to check if the file exists already and if so, add the keys to it from this plugin's colors.xml (if they don't already exist).

Since Cordova doesn't provide a mechanism to do that, it would have to be done with a custom hook script. This something which could be done, but it's not high on my priority list.

ir2pid commented 4 years ago

This is what I use, checks if colors.xml file exists else make one before the plugin is installed and appends values.

removed below line from plugin's config.xml <!--<source-file src="src/android/colors.xml" target-dir="res/values" />-->

add before_plugin_install hook before_plugin_install.js

var fs = require("fs");
var path = require("path");
var rootdir = process.env.PWD;
var valuesPath = "/platforms/android/app/src/main/res/values/";
var colorsFile = "colors.xml";
var destFile = path.join(rootdir, valuesPath, colorsFile);
var destDir = path.dirname(destFile);

module.exports = {

    createAndroidColorXml: function (context) {
        console.log("*************************************");
        console.log(`before_plugin_install running`);
        console.log("*************************************");

        if (fs.existsSync(destDir)) {
            if (!fs.existsSync(destFile)) {
                console.log("*************************************");
                console.log(`creating new  ${destFile}`);
                console.log("*************************************");
                fs.writeFile(
                    destFile,
                    '<?xml version="1.0" encoding="utf-8"?>\n<resources>\n <color name="colorPrimary">#3F51B5</color>\n <color name="colorPrimaryDark">#303F9F</color>\n <color name="colorAccent">#FF4081</color>\n</resources>',
                    function (err) {
                        if (err) {
                            console.error("writeFile error" + err);
                        }
                        console.log(`${colorsFile} created!`);
                    }
                );
            } else {
                console.log(`skipping, file ${colorsFile}  exists`);
            }
        } else {
            console.log("skipping, android platform not found");
        }
    }

};
tom-hartz commented 4 years ago

Changing it from <source-file target-dir= to <resource-file target= is a much simpler fix for this. I've had this change for a while in my fork of the arnesson plugin: https://github.com/tom-hartz/cordova-plugin-firebase/commit/d5b7a4338d6daeca2bb7f4729a64b1124cc53a2d

tom-hartz commented 4 years ago

Oops. My suggestion only works if you already have a colors.xml with "accent" defined in it, and with my changes the plugin variable ANDROID_ICON_ACCENT is just ignored. If your project colors.xml does not define an "accent" color, you then end up with this build error:

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':app:processDebugResources'.
> Android resource linking failed
  ../platforms/android/app/build/intermediates/merged_manifests/debug/AndroidManifest.xml:73: AAPT: error: resource color/accent (aka <package>:color/accent) not found.

Ideally there should be a build hook or something that can correctly merge the colors.xml, probably like what @ir2pid suggested.

captainerd commented 4 years ago

Script at: /plugins/cordova-plugin-firebasex/scripts/after_prepare.js it keeps replacing my own defined colors.xml in config.xml so i was getting errors and couln't compile.

Cause At line 193:

Utilities.writeJsonToXmlFile($colorsXml, PLATFORM.ANDROID.colorsXml.target); Utilities.log('Updated colors.xml with accent color');

It overwrites the original colors, i worked around this with including #FF00FFFF to my colors.xml and commenting out // that line of script

kapilSoni101 commented 4 years ago

@captainerd :Are u fixed above issue?