[Obsolete] Add Android Export feature by venkat24 · Pull Request #545 · google/codeworld

venkat24 · 2017-09-03T04:40:35Z

exportAndroid handler which compiles a .apk and prompts a download
Modifications to build scripts to install new dependencies

- exportAndroid handler which initializes new Cordova project Add a new handler, `exportAndroid`. This takes the source as a parameter, and performs the same steps that the compile route performs, and calls `compileIfNeeded`. After compilation, the `initCordovaProject` function is invoked, which merely creates a stub cordova project, by making a subprocess call to the `cordova` executable. At this commit, it does not build the project.

- Create `android-resources` directory that holds app assets - Initialize `android-template` for using as a base for app builds - Handler copies compiled `js` files into template and invokes `cordova` A template directory is initialized with `cordova create` and assets are copied in from `android-resources`. Includes main view HTML, CSS, and App Icons. The handler clones the template directory to `data/<mode>/android/` and copies in the `js` files from the compilation step. `cordova build` is invoked and the generated `.apk` file is fetched as a binary blob and downloaded.

- Add Cordova and JDK installation to install.sh - Build Android template project building - Set android build tools paths in base.sh List of Dependencies for Android Exports - - Cordova >= 6.2.2 - Android Build Tools - Android SDK >= 19 - Gradle >= 3.4 - All executables in path and $ANROID_HOME set

- Add XML Parser TagSoup to set the App Name in `config.xml` - Add UI components to receive App Name input

cdsmith · 2017-09-04T05:17:17Z

Hi Venkat. Thanks for the pull request. I have some concerns about the performance implications of this change, and there are some critical times coming up (CodeWorld is being used for an ICFP presentation in a few hours, and at Chalmers through this coming Tuesday. Since this adds the possibility of an expensive server-side build process, it's best not to merge this until after those milestones have passed. I'll take a look soon, though, and see if there are any major concerns.

cdsmith

Sorry for the delay in reviewing this. The end of Summer of Haskell left me swamped with work to do.

cdsmith · 2017-09-10T02:57:13Z

android-resources/config.xml

+    <access origin="*" />
+    <allow-intent href="http://*/*" />
+    <allow-intent href="https://*/*" />
+    <allow-intent href="tel:*" />


What is the purpose of all these allow-intent elements?

cdsmith · 2017-09-10T02:57:48Z

android-resources/config.xml

+        CodeWorld App
+    </description>
+    <author href="https://code.world">
+        CodeWorld


Maybe set this to "CodeWorld User" so it doesn't seem like we're claiming authorship of student programs?

cdsmith · 2017-09-10T02:58:34Z

android-resources/config.xml

+<widget id="io.cordova.hellocordova" version="1.0.0" xmlns="http://www.w3.org/ns/widgets" xmlns:cdv="http://cordova.apache.org/ns/1.0">
+    <name>CodeWorld App</name>
+    <description>
+        CodeWorld App


This could be more descriptive, right? Maybe "This application is exported from CodeWorld." or something?

cdsmith · 2017-09-10T02:59:59Z

android-resources/config.xml

@@ -0,0 +1,27 @@
+<?xml version='1.0' encoding='utf-8'?>
+<widget id="io.cordova.hellocordova" version="1.0.0" xmlns="http://www.w3.org/ns/widgets" xmlns:cdv="http://cordova.apache.org/ns/1.0">


io.cordova.hellocordova ???

Would there be a problem installing two of these? Maybe the id would need to have the program hash in it to disambiguate? But I'm not sure, because I'm not sure what this is used for.

cdsmith · 2017-09-10T03:01:20Z

android-resources/www/css/index.css

@@ -0,0 +1,105 @@
+* {


Please add a source header, which you can copy from other css files in the project (e.g., in web/css).

cdsmith · 2017-09-10T03:37:42Z

codeworld-server/src/Main.hs

+    mode <- getBuildMode
+    Just source <- getParam "source"
+    maybeAppName <- getParam "appName"
+    let appName = BC.unpack $ fromMaybe (BC.pack "CodeWorld App") maybeAppName 


Is there ever a case you expect to get a request without an app name? If not, just fail the pattern match in that case.

cdsmith · 2017-09-10T03:38:19Z

codeworld-server/src/Main.hs

+        B.writeFile (buildRootDir mode </> sourceFile programId) source
+        writeDeployLink mode deployId programId
+        compileIfNeeded mode programId
+    unless success $ modifyResponse $ setResponseCode 500


If you get a 500 error, you shouldn't continue, right?

cdsmith · 2017-09-10T03:40:25Z

codeworld-server/src/Main.hs

+        writeDeployLink mode deployId programId
+        compileIfNeeded mode programId
+    unless success $ modifyResponse $ setResponseCode 500
+    let appProps = M.fromList [("appName", appName)]


This AppProps thing is entirely unnecessary. You're trying to invent a dynamically typed language in Haskell, and it's ugly. Just go ahead and commit to defining a data type with the right fields. (Which, at the moment, is just one String, right?)

cdsmith · 2017-09-10T03:42:35Z

codeworld-server/src/Util.hs

+androidBuildDir mode programId = androidRootDir mode </> sourceBase programId
+
+apkFile :: BuildMode -> ProgramId -> FilePath
+apkFile mode programId =


Once you move the build into a temporary directory (see other comment), you will want to pick a simpler path for the apk file.

Since you're letting people choose different app names for the same code, you cannot uniquely identify apk files by the hash of their code. You need a different scheme.

cdsmith · 2017-09-10T03:47:44Z

codeworld-server/src/AndroidExport.hs

+          nameId = (~/= ("<name>"::String))
+
+buildApk :: BuildMode -> ProgramId -> IO ()
+buildApk mode programId = do


You need to set a time limit, and kill the process if it exceeds the time limit. There's a utility for that at https://github.com/google/codeworld/blob/master/codeworld-compiler/src/Compile.hs#L203, but unfortunately it was moved from codeworld-server to codeworld-compiler, as part of another Summer of Haskell project. For now, feel free to copy it back into codeworld-server/src/Util.hs

venkat24 · 2017-09-18T14:41:26Z

Apologies for the delay! I've been having tests this week, I'll get on this immediately.

venkat24 force-pushed the master branch 2 times, most recently from 1ed764a to 0ad0fd7 Compare September 3, 2017 05:42

venkat24 added 3 commits September 3, 2017 11:18

Allow user to set App Name

59ee7ab

- Add XML Parser TagSoup to set the App Name in `config.xml` - Add UI components to receive App Name input

venkat24 force-pushed the master branch from 0ad0fd7 to 59ee7ab Compare September 3, 2017 05:48

cdsmith reviewed Sep 10, 2017

View reviewed changes

cdsmith changed the title ~~Add Android Export feature~~ [Obsolete] Add Android Export feature Aug 23, 2020

cdsmith mentioned this pull request Aug 18, 2021

Option to export projects to Android and/or iOS #22

Open

		@@ -0,0 +1,27 @@
		<?xml version='1.0' encoding='utf-8'?>
		<widget id="io.cordova.hellocordova" version="1.0.0" xmlns="http://www.w3.org/ns/widgets" xmlns:cdv="http://cordova.apache.org/ns/1.0">

Conversation

venkat24 commented Sep 3, 2017

Uh oh!

cdsmith commented Sep 4, 2017

Uh oh!

cdsmith left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

venkat24 commented Sep 18, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants