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

Build anki web assets from rslib-bridge/anki submodule #217

Merged
merged 14 commits into from
Jul 19, 2022
Merged

Build anki web assets from rslib-bridge/anki submodule #217

merged 14 commits into from
Jul 19, 2022

Conversation

krmanik
Copy link
Member

@krmanik krmanik commented Jun 29, 2022

There is need to generate web assets files for showing graphs, card info, import csv etc. The file built using bazel, so it needs to add bazel build in build.gradle.

The current PR will solve the issue of generating assets for current anki version (rslib-bridge/anki) repo.

Added task in build.gradle to execute buildAnkiWebAssets. It runs script build-web-assets.(bat/sh) on windows, macos and linux. The script change directory to $SRC_DIR/rslib-bridge/anki and run bazel build on ts and pages. Then the generated web pages copied to resources of the rsdroid project. The aar files contains this pages in web dir.

@dae
Copy link
Contributor

dae commented Jun 29, 2022

While committing into a separate branch of the same repo is a common pattern for actions like doc generators, the artifacts will be multi megabytes, so it will cause the size of this repo to grow in size - I suggest the action publishes an artifact instead.

@krmanik
Copy link
Member Author

krmanik commented Jun 29, 2022

I will update it to publish the artifacts.

@krmanik
Copy link
Member Author

krmanik commented Jun 30, 2022

After changing some ts files in anki submodule for testing, the local built aar file should contains latest ts file. The current implementation in this PR will not update the new web assets in aar file. So, I came up with following implementation, if environment already configured to run bazel command, then it can be used to build the web assets. First run will take some time, but next run of bazel command will be faster. Also clean command works without any issues.

diff --git a/rsdroid/build.gradle b/rsdroid/build.gradle
index fd7bb40..12f8736 100644
--- a/rsdroid/build.gradle
+++ b/rsdroid/build.gradle
@@ -45,10 +45,23 @@ android {
     sourceSets {
         main {
             kotlin.srcDirs += "build/generated/source/fluent"
+            resources {
+                srcDirs "src/main/resources", "build/generated/anki_artifacts"
+            }
         }
     }
 }

+task buildAnkiWebAssets(type: Exec) {
+    workingDir "$rootDir"
+    String toolPath = System.getProperty('os.name').toLowerCase(Locale.ROOT).contains('windows') ? 'tools\\web\\build-web-assets.bat' : 'tools/web/build-web-assets.sh'
+    if (System.getProperty('os.name').toLowerCase(Locale.ROOT).contains('windows')) {
+        commandLine 'cmd', '/c', toolPath
+    } else {
+        commandLine 'sh', '-c', toolPath
+    }
+}
+
 // Consider upgrade to DSL: https://docs.gradle.org/current/userguide/plugins.html#sec:plugins_block
 apply plugin: "org.mozilla.rust-android-gradle.rust-android"

@@ -96,6 +109,8 @@ task generateTranslations(type: Exec) {
     }
 }

+preBuild.dependsOn "buildAnkiWebAssets"
+
 preBuild.dependsOn "generateTranslations"

 Boolean wantAllPlatforms = System.getenv("CURRENT_ONLY") != "true"
diff --git a/tools/web/build-web-assets.sh b/tools/web/build-web-assets.sh
new file mode 100755
index 0000000..d2329d4
--- /dev/null
+++ b/tools/web/build-web-assets.sh
@@ -0,0 +1,17 @@
+# build web assets with anki submodule in rslib-bridge
+
+SRC_DIR=$(pwd)
+ANKI_SRC=$SRC_DIR/rslib-bridge/anki
+BUILD_DIR=$SRC_DIR/rsdroid/build/generated/anki_artifacts/
+
+# build for ts
+cd $ANKI_SRC/ts
+bazel build ts
+
+# build for web
+cd $ANKI_SRC/qt/aqt/data/web
+bazel build web
+
+# copy pages to build/generated
+mkdir -p $BUILD_DIR/web
+cp -r $ANKI_SRC/.bazel/bin/qt/aqt/data/web/pages/* $BUILD_DIR/web

@dae
Copy link
Contributor

dae commented Jun 30, 2022

This is convenient for testing, and is probably the best long term approach, but the problem in the short term is that it will require work to make sure the Bazel build works in all the different github actions (eg Windows/Mac build)

@dae
Copy link
Contributor

dae commented Jun 30, 2022

One other issue is that the .bazel folder will make AndroidStudio really slow, so you may want to manually specify a different location, or remove the symlink as part of the build.

@krmanik
Copy link
Member Author

krmanik commented Jul 1, 2022

This is convenient for testing, and is probably the best long term approach, but the problem in the short term is that it will require work to make sure the Bazel build works in all the different github actions (eg Windows/Mac build)

I will update the github actions workflows file for other system.

One other issue is that the .bazel folder will make AndroidStudio really slow, so you may want to manually specify a different location, or remove the symlink as part of the build.

I have used --symlink_prefix option in bazel command to symlink .bazel folder in tmp dir. The only issues is node_modules, the current size of node_modules is 450MB approx. I have tried to find the solution for getting node_modules from /tmp folder but didn't found any. May be the anki source should be copied to temp dir.

This is working solution with .bazel in /tmp dir and without considering node_modules size.

# build for ts
cd $ANKI_SRC/ts
bazel build ts --symlink_prefix=/tmp/.bazel/

# build for web
cd $ANKI_SRC/qt/aqt/data/web/pages
bazel build pages --symlink_prefix=/tmp/.bazel/

# copy pages to build/generated
mkdir -p $BUILD_DIR/web
cp -r /tmp/.bazel/bin/qt/aqt/data/web/pages/* $BUILD_DIR/web

I have also tried to symlink the anki dir (sudo ln -s $ANKI_SRC $TMP_DIR), but .bazel is still in the main dir not in temp dir.

@dae
Copy link
Contributor

dae commented Jul 1, 2022

That might be enough already? If node_modules is causing a problem, perhaps there's a way to exclude it in AndroidStudio that can be configured in a file in the repo?

@krmanik
Copy link
Member Author

krmanik commented Jul 1, 2022

Will it be considered in build.gradle?

apply plugin: 'idea'

idea.module {
    excludeDirs += file('rslib-bridge/anki/node_modules/')
}

https://stackoverflow.com/questions/47244841/how-do-i-exclude-a-directory-like-node-modules-from-android-studios-open-fil

@mikehardy
Copy link
Member

That could work for me, I don't believe gradle as a build system would ever care about node ecosystem packages. I've never seen a need to mix the two

@krmanik
Copy link
Member Author

krmanik commented Jul 1, 2022

That could work for me, I don't believe gradle as a build system would ever care about node ecosystem packages. I've never seen a need to mix the two

Then I will update the github actions with setup bazel for (Windows, mac and linux).

@krmanik krmanik deleted the branch ankidroid:main July 1, 2022 14:06
@krmanik krmanik closed this Jul 1, 2022
@krmanik krmanik deleted the main branch July 1, 2022 14:06
@krmanik
Copy link
Member Author

krmanik commented Jul 1, 2022

I will reopen it because I have renamed this branch so that when I push the github action should run. But it get closed.

Building/testing on local branch https://github.com/krmanik/Anki-Android-Backend/pull/1/

@krmanik krmanik restored the main branch July 1, 2022 14:08
@krmanik krmanik reopened this Jul 1, 2022
tools/web/build-web-assets.bat Outdated Show resolved Hide resolved
tools/web/build-web-assets.sh Outdated Show resolved Hide resolved
@dae
Copy link
Contributor

dae commented Jul 2, 2022

One remaining nice-to-have would be to generate the buildinfo.txt file, so it doesn't need to be checked into this repo/manually updated.

tools/web/build-web-assets.sh Outdated Show resolved Hide resolved
@krmanik
Copy link
Member Author

krmanik commented Jul 5, 2022

The latest buildinfo.txt file will be pushed to main when a library published.
Also added bazel and node_modules cache for faster build.

@krmanik
Copy link
Member Author

krmanik commented Jul 5, 2022

Fixing now

> Task :rsdroid:buildAnkiWebAssets
D:\a\Anki-Android-Backend\Anki-Android-Backend>bazel --version   1>nul 2>&1  && (echo "bazel command found!" )  || (
echo "bazel: command not found. Please install Bazelisk. Your distro may have it,"  
 echo "or you can fetch the binary from https://github.com/bazelbuild/bazelisk/releases"  
 echo "and add it to path."  
 exit 1 
) 
"bazel command found!"

tools/web/build-web-assets.bat Outdated Show resolved Hide resolved
@dae
Copy link
Contributor

dae commented Jul 6, 2022

Thanks for all your work on this Mani, the caching looks like a nice improvement.

@krmanik
Copy link
Member Author

krmanik commented Jul 6, 2022

For linux the build is working as expected but failing on macOS and windows.

Linux actions result
> Task :clean UP-TO-DATE
> Task :rsdroid:clean UP-TO-DATE
> Task :rsdroid-instrumented:clean UP-TO-DATE
> Task :rsdroid-testing:clean UP-TO-DATE
> Task :rsdroid:buildAnkiWebAssets
2022/07/06 13:40:55 Downloading https://releases.bazel.build/5.1.1/release/bazel-5.1.1-linux-x86_64...
Starting local Bazel server and connecting to it...
Loading: 
Loading: 0 packages loaded
Loading: 0 packages loaded
Analyzing: target //:buildinfo.txt (1 packages loaded, 0 targets configured)
Analyzing: target //:buildinfo.txt (54 packages loaded, 186 targets configured)
INFO: Analyzed target //:buildinfo.txt ([57](https://github.com/ankidroid/Anki-Android-Backend/runs/7215801459?check_suite_focus=true#step:16:58) packages loaded, 217 targets configured).
INFO: Found 1 target...
[0 / 4] [Prepa] Creating source manifest for //tools:buildinfo [for host]
Target //:buildinfo.txt up-to-date:
  /tmp/.bazel/bin/buildinfo.txt
INFO: Elapsed time: 5.464s, Critical Path: 0.22s
INFO: 5 processes: 4 internal, 1 linux-sandbox.
INFO: Build completed successfully, 5 total actions
INFO: Build completed successfully, 5 total actions
Loading: 
Loading: 0 packages loaded
INFO: Build option --compilation_mode has changed, discarding analysis cache.
Analyzing: target //qt/aqt/data/web/pages:pages (1 packages loaded, 0 targets configured)
Analyzing: target //qt/aqt/data/web/pages:pages (56 packages loaded, 545 targets configured)
Analyzing: target //qt/aqt/data/web/pages:pages (79 packages loaded, 8533 targets configured)
Analyzing: target //qt/aqt/data/web/pages:pages (166 packages loaded, 13657 targets configured)
Analyzing: target //qt/aqt/data/web/pages:pages (316 packages loaded, 18449 targets configured)
Analyzing: target //qt/aqt/data/web/pages:pages (346 packages loaded, 21065 targets configured)
INFO: Analyzed target //qt/aqt/data/web/pages:pages (361 packages loaded, 21555 targets configured).
INFO: Found 1 target...
[0 / 1] [Prepa] BazelWorkspaceStatusAction stable-status.txt
[64 / 179] checking cached actions
Target //qt/aqt/data/web/pages:pages up-to-date:
  /tmp/.bazel/bin/qt/aqt/data/web/pages/graphs-base.css
  /tmp/.bazel/bin/qt/aqt/data/web/pages/graphs.css
  /tmp/.bazel/bin/qt/aqt/data/web/pages/graphs.html
  /tmp/.bazel/bin/qt/aqt/data/web/pages/graphs.js
  /tmp/.bazel/bin/qt/aqt/data/web/pages/graphs.js.map
  /tmp/.bazel/bin/qt/aqt/data/web/pages/congrats-base.css
  /tmp/.bazel/bin/qt/aqt/data/web/pages/congrats.css
  /tmp/.bazel/bin/qt/aqt/data/web/pages/congrats.html
  /tmp/.bazel/bin/qt/aqt/data/web/pages/congrats.js
  /tmp/.bazel/bin/qt/aqt/data/web/pages/congrats.js.map
  /tmp/.bazel/bin/qt/aqt/data/web/pages/deck-options-base.css
  /tmp/.bazel/bin/qt/aqt/data/web/pages/deck-options.css
  /tmp/.bazel/bin/qt/aqt/data/web/pages/deck-options.html
  /tmp/.bazel/bin/qt/aqt/data/web/pages/deck-options.js
  /tmp/.bazel/bin/qt/aqt/data/web/pages/deck-options.js.map
  /tmp/.bazel/bin/qt/aqt/data/web/pages/change-notetype-base.css
  /tmp/.bazel/bin/qt/aqt/data/web/pages/change-notetype.css
  /tmp/.bazel/bin/qt/aqt/data/web/pages/change-notetype.html
  /tmp/.bazel/bin/qt/aqt/data/web/pages/change-notetype.js
  /tmp/.bazel/bin/qt/aqt/data/web/pages/change-notetype.js.map
  /tmp/.bazel/bin/qt/aqt/data/web/pages/card-info-base.css
  /tmp/.bazel/bin/qt/aqt/data/web/pages/card-info.css
  /tmp/.bazel/bin/qt/aqt/data/web/pages/card-info.html
  /tmp/.bazel/bin/qt/aqt/data/web/pages/card-info.js
  /tmp/.bazel/bin/qt/aqt/data/web/pages/card-info.js.map
  /tmp/.bazel/bin/qt/aqt/data/web/pages/fields-base.css
  /tmp/.bazel/bin/qt/aqt/data/web/pages/fields.css
  /tmp/.bazel/bin/qt/aqt/data/web/pages/fields.html
  /tmp/.bazel/bin/qt/aqt/data/web/pages/fields.js
  /tmp/.bazel/bin/qt/aqt/data/web/pages/fields.js.map
  /tmp/.bazel/bin/qt/aqt/data/web/pages/import-csv-base.css
  /tmp/.bazel/bin/qt/aqt/data/web/pages/import-csv.css
  /tmp/.bazel/bin/qt/aqt/data/web/pages/import-csv.html
  /tmp/.bazel/bin/qt/aqt/data/web/pages/import-csv.js
  /tmp/.bazel/bin/qt/aqt/data/web/pages/import-csv.js.map
INFO: Elapsed time: 14.442s, Critical Path: 1.34s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
INFO: Build completed successfully, 1 total action
Loading: 
Loading: 0 packages loaded
Analyzing: target //ts/reviewer:reviewer_extras_bundle (1 packages loaded, 0 targets configured)
INFO: Analyzed target //ts/reviewer:reviewer_extras_bundle (3 packages loaded, 167 targets configured).
INFO: Found 1 target...
[0 / 1] [Prepa] BazelWorkspaceStatusAction stable-status.txt
[19 / 21] Compiling TypeScript project //ts/reviewer:reviewer_ts [tsc -p ts/reviewer/tsconfig.json]; 1s linux-sandbox
[20 / 21] [Prepa] Bundling Javascript ts/reviewer/reviewer_extras.ts [esbuild]
Target //ts/reviewer:reviewer_extras_bundle up-to-date:
  /tmp/.bazel/bin/ts/reviewer/reviewer_extras_bundle.js
  /tmp/.bazel/bin/ts/reviewer/reviewer_extras_bundle.js.map
  /tmp/.bazel/bin/ts/reviewer/reviewer_extras_bundle_metadata.json
INFO: Elapsed time: 4.699s, Critical Path: 3.[60](https://github.com/ankidroid/Anki-Android-Backend/runs/7215801459?check_suite_focus=true#step:16:61)s
INFO: 21 processes: 12 internal, 9 linux-sandbox.
INFO: Build completed successfully, 21 total actions
INFO: Build completed successfully, 21 total actions
> Task :generateLinkerWrapper
> Task :rsdroid:cargoBuildArm

Trying now

./tools/web/build-web-assets.sh && ./gradlew clean assembleRelease -DtestBuildType=release

@krmanik
Copy link
Member Author

krmanik commented Jul 6, 2022

It should be fix now, I have used mustRunAfter https://docs.gradle.org/current/userguide/more_about_tasks.html#sec:ordering_tasks

I have put the cargo task in another task and configure to use mustRunAfter. I think it should fix now.

Trying now

tasks.matching { it.name != 'buildAnkiWebAssets' && it.name != 'clean' }.all { Task task ->
    task.dependsOn buildAnkiWebAssets
}

rsdroid/build.gradle Outdated Show resolved Hide resolved
@dae
Copy link
Contributor

dae commented Jul 8, 2022

That latest change looks promising. The new error might just be a flake - you could try running that bazel build line a few times in a row.

@krmanik
Copy link
Member Author

krmanik commented Jul 8, 2022

That latest change looks promising. The new error might just be a flake - you could try running that bazel build line a few times in a row.

Added in ps1

for (($i = 0); $i -lt 5; $i++)
{
    bazel build pages -k --shell_executable="C:/msys64/usr/bin/bash.exe"
}

@dae
Copy link
Contributor

dae commented Jul 8, 2022

No luck :-( Some Googling seems to indicate TypeScript can give that error when using symlinks, but I guess we're not using them there? Maybe it's some other path-related issue. That path is quite long - I wonder if you copied rslib-anki/anki to d:\a\anki and built from there, if it would help at all?

Another possible thing you could try is the -win.sh file now that you've fixed the bash issue

@dae
Copy link
Contributor

dae commented Jul 9, 2022

Great work in figuring that out Mani. Assuming you've tested that the pages can be successfully loaded on the AnkiDroid end with this build, it's a 👍 from me. Thank you for all of your hard work!

@krmanik
Copy link
Member Author

krmanik commented Jul 9, 2022

I have tested rsdroid-release.aar file by publishing from GitHub using release actions and on AnkiDroid the pages loads as expected.

On windows machine it needs Build Tools for Visual Studio 2019 (version 16.11) to build the pages using the docs windows.md (I haven't tried this).

@dae
Copy link
Contributor

dae commented Jul 11, 2022

Could we include the license files too? ankidroid/Anki-Android#11825 (comment)

@dae
Copy link
Contributor

dae commented Jul 11, 2022

Re docs, how about:

diff --git a/docs/TESTING.md b/docs/TESTING.md
index 9a95dcfc6..1213ee562 100644
--- a/docs/TESTING.md
+++ b/docs/TESTING.md
@@ -8,7 +8,11 @@ so far.

 ## Setup

-Make sure you can build AnkiDroid first.
+- Make sure you can build AnkiDroid first.
+
+- Follow the initial setup instructions in the computer version
+  (rslib-bridge/anki/docs/{windows,mac,linux}.md) to install prerequisites
+  like Bazelisk and the build tools for your platform.

 Install NDK:

@krmanik
Copy link
Member Author

krmanik commented Jul 11, 2022

Could we include the license files too? ankidroid/Anki-Android#11825 (comment)

The licenses.json from cargo and ts included as licenses-cargo.json and licenses-ts.json in web dir.

Re docs, how about:

Thanks, I have updated the docs.

Copy link
Member

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting it to the point where it is working is fantastic (and I think it is there right?) - that's the hard part
My comments are now about editing it back down to be minimal by removing any duplication that can be removed, if possible

.github/workflows/robolectric_build.yml Outdated Show resolved Hide resolved
rsdroid/build.gradle Outdated Show resolved Hide resolved
Copy link
Member

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly one question on the cache restore keys

One note on the cache paths for bazel action, and a simple comment could get this merged, or variable interpolation if that's fast for you (just a comment if not...)

.github/workflows/linux_build.yml Outdated Show resolved Hide resolved
.github/workflows/windows_build.yml Outdated Show resolved Hide resolved
@mikehardy mikehardy added the pending-merge Waiting on CI or question responses to merge, but otherwise ready label Jul 19, 2022
Copy link
Member

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a big advance - thank you!

Copy link
Member

@david-allison david-allison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing major.

I'm out of my comfort zone here and trust dae/Mani/Mike's judgment 100% on this

.github/workflows/robolectric_build.yml Outdated Show resolved Hide resolved
.github/workflows/windows_build.yml Outdated Show resolved Hide resolved
.github/workflows/windows_pure_build.yml Outdated Show resolved Hide resolved
tools/web/build-web-assets.ps1 Outdated Show resolved Hide resolved
build-web-assets.gradle Show resolved Hide resolved
@krmanik
Copy link
Member Author

krmanik commented Jul 19, 2022

Extracted the bash setup to ps1 so that user can change/revert bash changes. I have used mv.exe so that file can be rename back to bash.exe in system32.

Copy link
Member

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Mani! I'm going to merge this to unblock things in my last time available for a while, we can always do fixups as needed / if needed

@mikehardy mikehardy merged commit ba74873 into ankidroid:main Jul 19, 2022
@mikehardy mikehardy removed the pending-merge Waiting on CI or question responses to merge, but otherwise ready label Oct 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants