You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cordova.apache.org by GitBox <gi...@apache.org> on 2018/11/14 22:44:33 UTC

[GitHub] brodybits closed pull request #550: Fix for old plugins with non-Java sources (GH-547)

brodybits closed pull request #550: Fix for old plugins with non-Java sources (GH-547)
URL: https://github.com/apache/cordova-android/pull/550
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/bin/templates/cordova/lib/pluginHandlers.js b/bin/templates/cordova/lib/pluginHandlers.js
index f1b0dba02..794154b54 100644
--- a/bin/templates/cordova/lib/pluginHandlers.js
+++ b/bin/templates/cordova/lib/pluginHandlers.js
@@ -293,20 +293,28 @@ function generateAttributeError (attribute, element, id) {
 }
 
 function getInstallDestination (obj) {
-    return studioPathRemap(obj) ||
-        path.join(obj.targetDir, path.basename(obj.src));
-}
-
-function studioPathRemap (obj) {
-    // If any source file is using the app new directory structure,
-    // don't penalize it
-    if (!obj.targetDir.includes('app')) {
-        if (obj.src.endsWith('.java')) {
-            return path.join('app/src/main/java', obj.targetDir.substring(4), path.basename(obj.src));
+    var APP_MAIN_PREFIX = 'app/src/main';
+
+    if (obj.targetDir.includes('app')) {
+        // If any source file is using the new app directory structure,
+        // don't penalize it
+        return path.join(obj.targetDir, path.basename(obj.src));
+    } else if (obj.src.endsWith('.java')) {
+        return path.join(APP_MAIN_PREFIX, 'java', obj.targetDir.substring(4), path.basename(obj.src));
+    } else if (obj.src.endsWith('.aidl')) {
+        return path.join(APP_MAIN_PREFIX, 'aidl', obj.targetDir.substring(4), path.basename(obj.src));
+    } else if (obj.targetDir.includes('libs')) {
+        if (obj.src.endsWith('.so')) {
+            return path.join(APP_MAIN_PREFIX, 'jniLibs', obj.targetDir.substring(5), path.basename(obj.src));
         } else {
-            // For all other files, add 'app/src/main' to the targetDir if it didn't have it already
-            return path.join('app/src/main', obj.targetDir, path.basename(obj.src));
+            return path.join('app', obj.targetDir, path.basename(obj.src));
         }
+    } else if (obj.targetDir.includes('src/main')) {
+        return path.join('app', obj.targetDir, path.basename(obj.src));
+    } else {
+        // For all other source files not using the new app directory structure,
+        // add 'app/src/main' to the targetDir
+        return path.join(APP_MAIN_PREFIX, obj.targetDir, path.basename(obj.src));
     }
 
 }
diff --git a/spec/fixtures/org.test.plugins.dummyplugin/plugin.xml b/spec/fixtures/org.test.plugins.dummyplugin/plugin.xml
index b299b645a..4e2ebcc6c 100644
--- a/spec/fixtures/org.test.plugins.dummyplugin/plugin.xml
+++ b/spec/fixtures/org.test.plugins.dummyplugin/plugin.xml
@@ -76,6 +76,12 @@
                 target-dir="app/libs" />
         <source-file src="src/android/TestAar.aar"
                 target-dir="app/libs" />
+        <source-file src="src/android/mysettings.xml" target-dir="res/xml" />
+        <source-file src="src/android/other.extension" target-dir="res/values" />
+	<source-file src="src/android/myapi.aidl" target-dir="src/com/mytest" />
+	<source-file src="src/android/testaar2.aar" target-dir="libs" />
+	<source-file src="src/android/testjar2.jar" target-dir="libs" />
+	<source-file src="src/android/jniLibs/x86/libnative.so" target-dir="libs/x86" />
         <lib-file src="src/android/TestLib.jar" />
     </platform>
 </plugin>
diff --git a/spec/fixtures/org.test.plugins.dummyplugin/src/android/jniLibs/x86/libnative.so b/spec/fixtures/org.test.plugins.dummyplugin/src/android/jniLibs/x86/libnative.so
new file mode 100644
index 000000000..421376db9
--- /dev/null
+++ b/spec/fixtures/org.test.plugins.dummyplugin/src/android/jniLibs/x86/libnative.so
@@ -0,0 +1 @@
+dummy
diff --git a/spec/fixtures/org.test.plugins.dummyplugin/src/android/myapi.aidl b/spec/fixtures/org.test.plugins.dummyplugin/src/android/myapi.aidl
new file mode 100644
index 000000000..421376db9
--- /dev/null
+++ b/spec/fixtures/org.test.plugins.dummyplugin/src/android/myapi.aidl
@@ -0,0 +1 @@
+dummy
diff --git a/spec/fixtures/org.test.plugins.dummyplugin/src/android/mysettings.xml b/spec/fixtures/org.test.plugins.dummyplugin/src/android/mysettings.xml
new file mode 100644
index 000000000..421376db9
--- /dev/null
+++ b/spec/fixtures/org.test.plugins.dummyplugin/src/android/mysettings.xml
@@ -0,0 +1 @@
+dummy
diff --git a/spec/fixtures/org.test.plugins.dummyplugin/src/android/other.extension b/spec/fixtures/org.test.plugins.dummyplugin/src/android/other.extension
new file mode 100644
index 000000000..421376db9
--- /dev/null
+++ b/spec/fixtures/org.test.plugins.dummyplugin/src/android/other.extension
@@ -0,0 +1 @@
+dummy
diff --git a/spec/fixtures/org.test.plugins.dummyplugin/src/android/testaar2.aar b/spec/fixtures/org.test.plugins.dummyplugin/src/android/testaar2.aar
new file mode 100644
index 000000000..421376db9
--- /dev/null
+++ b/spec/fixtures/org.test.plugins.dummyplugin/src/android/testaar2.aar
@@ -0,0 +1 @@
+dummy
diff --git a/spec/fixtures/org.test.plugins.dummyplugin/src/android/testjar2.jar b/spec/fixtures/org.test.plugins.dummyplugin/src/android/testjar2.jar
new file mode 100644
index 000000000..421376db9
--- /dev/null
+++ b/spec/fixtures/org.test.plugins.dummyplugin/src/android/testjar2.jar
@@ -0,0 +1 @@
+dummy
diff --git a/spec/unit/pluginHandlers/handlers.spec.js b/spec/unit/pluginHandlers/handlers.spec.js
index 4c398eabb..20f5e6e93 100644
--- a/spec/unit/pluginHandlers/handlers.spec.js
+++ b/spec/unit/pluginHandlers/handlers.spec.js
@@ -103,24 +103,66 @@ describe('android project handler', function () {
                 }).toThrow(new Error('"' + target + '" already exists!'));
             });
 
-            it('Test#007 : should allow installing sources using proper path', function () {
+            // TODO: renumber these tests and other tests below
+            it('Test#00a6 : should allow installing sources with new app target-dir scheme', function () {
                 android['source-file'].install(valid_source[1], dummyPluginInfo, dummyProject, {android_studio: true});
                 expect(copyFileSpy)
                     .toHaveBeenCalledWith(dummyplugin, 'src/android/DummyPlugin2.java', temp, path.join('app/src/main/src/com/phonegap/plugins/dummyplugin/DummyPlugin2.java'), false);
             });
 
-            // TODO: renumber these tests and other tests below
-            it('Test#007a : should allow installing lib file from sources using proper path', function () {
+            it('Test#006b : should allow installing jar lib file from sources with new app target-dir scheme', function () {
                 android['source-file'].install(valid_source[2], dummyPluginInfo, dummyProject, {android_studio: true});
                 expect(copyFileSpy)
                     .toHaveBeenCalledWith(dummyplugin, 'src/android/TestLib.jar', temp, path.join('app/libs/TestLib.jar'), false);
             });
 
-            it('Test#007b : should allow installing aar file from sources using proper path', function () {
+            it('Test#006c : should allow installing aar lib file from sources with new app target-dir scheme', function () {
                 android['source-file'].install(valid_source[3], dummyPluginInfo, dummyProject, {android_studio: true});
                 expect(copyFileSpy)
                     .toHaveBeenCalledWith(dummyplugin, 'src/android/TestAar.aar', temp, path.join('app/libs/TestAar.aar'), false);
             });
+
+            it('Test#006d : should allow installing xml file from sources with old target-dir scheme', function () {
+                android['source-file'].install(valid_source[4], dummyPluginInfo, dummyProject, {android_studio: true});
+                expect(copyFileSpy).toHaveBeenCalledWith(dummyplugin,
+                    'src/android/mysettings.xml', temp,
+                    path.join('app/src/main/res/xml/mysettings.xml'), false);
+            });
+
+            it('Test#006e : should allow installing file with other extension from sources with old target-dir scheme', function () {
+                android['source-file'].install(valid_source[5], dummyPluginInfo, dummyProject, {android_studio: true});
+                expect(copyFileSpy).toHaveBeenCalledWith(dummyplugin,
+                    'src/android/other.extension', temp,
+                    path.join('app/src/main/res/values/other.extension'), false);
+            });
+
+            it('Test#006f : should allow installing aidl file from sources with old target-dir scheme (GH-547)', function () {
+                android['source-file'].install(valid_source[6], dummyPluginInfo, dummyProject, {android_studio: true});
+                expect(copyFileSpy).toHaveBeenCalledWith(dummyplugin,
+                    'src/android/myapi.aidl', temp,
+                    path.join('app/src/main/aidl/com/mytest/myapi.aidl'), false);
+            });
+
+            it('Test#006g : should allow installing aar lib file from sources with old target-dir scheme (GH-547)', function () {
+                android['source-file'].install(valid_source[7], dummyPluginInfo, dummyProject, {android_studio: true});
+                expect(copyFileSpy).toHaveBeenCalledWith(dummyplugin,
+                    'src/android/testaar2.aar', temp,
+                    path.join('app/libs/testaar2.aar'), false);
+            });
+
+            it('Test#006h : should allow installing jar lib file from sources with old target-dir scheme (GH-547)', function () {
+                android['source-file'].install(valid_source[8], dummyPluginInfo, dummyProject, {android_studio: true});
+                expect(copyFileSpy).toHaveBeenCalledWith(dummyplugin,
+                    'src/android/testjar2.jar', temp,
+                    path.join('app/libs/testjar2.jar'), false);
+            });
+
+            it('Test#006i : should allow installing .so lib file from sources with old target-dir scheme (GH-547)', function () {
+                android['source-file'].install(valid_source[9], dummyPluginInfo, dummyProject, {android_studio: true});
+                expect(copyFileSpy).toHaveBeenCalledWith(dummyplugin,
+                    'src/android/jniLibs/x86/libnative.so', temp,
+                    path.join('app/src/main/jniLibs/x86/libnative.so'), false);
+            });
         });
 
         describe('of <framework> elements', function () {
@@ -270,40 +312,66 @@ describe('android project handler', function () {
             });
         });
 
-        // TODO:
-        // - merge tests of <source-file> elements into single describe block
-        //   (with proper beforeEach/afterEach)
-        // - renumber the tests after Test#019
         describe('of <source-file> elements', function () {
             it('Test#019 : should remove stuff by calling common.deleteJava for Android Studio projects', function () {
                 android['source-file'].install(valid_source[0], dummyPluginInfo, dummyProject);
                 android['source-file'].uninstall(valid_source[0], dummyPluginInfo, dummyProject);
                 expect(deleteJavaSpy).toHaveBeenCalledWith(temp, path.join('app/src/main/java/com/phonegap/plugins/dummyplugin/DummyPlugin.java'));
             });
-        });
 
-        describe('of <source-file> element, with specific app target-dir', function () {
             it('Test#019a : should remove stuff by calling common.deleteJava for Android Studio projects, with specific app target-dir', function () {
                 android['source-file'].install(valid_source[1], dummyPluginInfo, dummyProject, {android_studio: true});
                 android['source-file'].uninstall(valid_source[1], dummyPluginInfo, dummyProject, {android_studio: true});
                 expect(deleteJavaSpy).toHaveBeenCalledWith(temp, path.join('app/src/main/src/com/phonegap/plugins/dummyplugin/DummyPlugin2.java'));
             });
-        });
 
-        describe('of <source-file> element, with JAR in specific app target-dir', function () {
-            it('Test#019b : should remove stuff by calling common.deleteJava for Android Studio projects, with JAR into specific app target-dir', function () {
+            it('Test#019b : should remove stuff by calling common.removeFile for Android Studio projects, of jar with new app target-dir scheme', function () {
                 android['source-file'].install(valid_source[2], dummyPluginInfo, dummyProject, {android_studio: true});
                 android['source-file'].uninstall(valid_source[2], dummyPluginInfo, dummyProject, {android_studio: true});
                 expect(removeFileSpy).toHaveBeenCalledWith(temp, path.join('app/libs/TestLib.jar'));
             });
-        });
 
-        describe('of <source-file> element, with AAR in specific app target-dir', function () {
-            it('Test#019c : should remove stuff by calling common.deleteJava for Android Studio projects, with JAR into specific app target-dir', function () {
+            it('Test#019c : should remove stuff by calling common.removeFile for Android Studio projects, of aar with new app target-dir scheme', function () {
                 android['source-file'].install(valid_source[3], dummyPluginInfo, dummyProject, {android_studio: true});
                 android['source-file'].uninstall(valid_source[3], dummyPluginInfo, dummyProject, {android_studio: true});
                 expect(removeFileSpy).toHaveBeenCalledWith(temp, path.join('app/libs/TestAar.aar'));
             });
+
+            it('Test#019d : should remove stuff by calling common.removeFile for Android Studio projects, of xml with old target-dir scheme', function () {
+                android['source-file'].install(valid_source[4], dummyPluginInfo, dummyProject, {android_studio: true});
+                android['source-file'].uninstall(valid_source[4], dummyPluginInfo, dummyProject, {android_studio: true});
+                expect(removeFileSpy).toHaveBeenCalledWith(temp, path.join('app/src/main/res/xml/mysettings.xml'));
+            });
+
+            it('Test#019e : should remove stuff by calling common.removeFile for Android Studio projects, of file with other extension with old target-dir scheme', function () {
+                android['source-file'].install(valid_source[5], dummyPluginInfo, dummyProject, {android_studio: true});
+                android['source-file'].uninstall(valid_source[5], dummyPluginInfo, dummyProject, {android_studio: true});
+                expect(removeFileSpy).toHaveBeenCalledWith(temp, path.join('app/src/main/res/values/other.extension'));
+            });
+
+            it('Test#019f : should remove stuff by calling common.removeFile for Android Studio projects, of aidl with old target-dir scheme (GH-547)', function () {
+                android['source-file'].install(valid_source[6], dummyPluginInfo, dummyProject, {android_studio: true});
+                android['source-file'].uninstall(valid_source[6], dummyPluginInfo, dummyProject, {android_studio: true});
+                expect(removeFileSpy).toHaveBeenCalledWith(temp, path.join('app/src/main/aidl/com/mytest/myapi.aidl'));
+            });
+
+            it('Test#019g : should remove stuff by calling common.removeFile for Android Studio projects, of aar with old target-dir scheme (GH-547)', function () {
+                android['source-file'].install(valid_source[7], dummyPluginInfo, dummyProject, {android_studio: true});
+                android['source-file'].uninstall(valid_source[7], dummyPluginInfo, dummyProject, {android_studio: true});
+                expect(removeFileSpy).toHaveBeenCalledWith(temp, path.join('app/libs/testaar2.aar'));
+            });
+
+            it('Test#019h : should remove stuff by calling common.removeFile for Android Studio projects, of jar with old target-dir scheme (GH-547)', function () {
+                android['source-file'].install(valid_source[8], dummyPluginInfo, dummyProject, {android_studio: true});
+                android['source-file'].uninstall(valid_source[8], dummyPluginInfo, dummyProject, {android_studio: true});
+                expect(removeFileSpy).toHaveBeenCalledWith(temp, path.join('app/libs/testjar2.jar'));
+            });
+
+            it('Test#019i : should remove stuff by calling common.removeFile for Android Studio projects, of .so lib file with old target-dir scheme (GH-547)', function () {
+                android['source-file'].install(valid_source[9], dummyPluginInfo, dummyProject, {android_studio: true});
+                android['source-file'].uninstall(valid_source[9], dummyPluginInfo, dummyProject, {android_studio: true});
+                expect(removeFileSpy).toHaveBeenCalledWith(temp, path.join('app/src/main/jniLibs/x86/libnative.so'));
+            });
         });
 
         describe('of <framework> elements', function () {


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org