Adding more assets #2132

robertpfeiffer · 2020-04-04T13:07:00Z

Add an --add-asset option to p4a apk.
--add-asset assets/foo will copy the local directory ./assets/foo to assets/assets/foo in the apk. This can used multiple times, to add multiple files and directories. --add-asset /home/user/path/to/image.png:images/image.png will copy the file to assets/images/image.png.

Probably not as useful with kivy, but very useful with SDL2 and SDL2-based frameworks, as SDL2 RWops use paths relative to the assets/ directory.

inclement

Thanks, no objection in principle but some questions about the details.

inclement · 2020-04-26T00:02:18Z

pythonforandroid/bootstraps/common/build/build.py

@@ -304,6 +305,15 @@ def make_package(args):
                tar_dirs.append(python_bundle_dir)
        if get_bootstrap_name() == "webview":
            tar_dirs.append('webview_includes')
+        if hasattr(args, "assets") and args.assets is not None:


I think argparse should make this simpler, isn't it an empty list by default or something so you can just do for asset in args.assets:?

I thought I tried that and it didn't work. Maybe I misconfigured argparse.

This is when we should coverage test this part as well. Not that I'm demanding it now, but I wish we had that already covered

inclement · 2020-04-26T00:02:30Z

pythonforandroid/bootstraps/common/build/build.py

@@ -239,9 +239,10 @@ def make_package(args):
    assets_dir = "src/main/assets"

    # Delete the old assets.
-    try_unlink(join(assets_dir, 'public.mp3'))
-    try_unlink(join(assets_dir, 'private.mp3'))


Looks like try_unlink isn't used any more, could you delete it?

inclement · 2020-04-26T00:03:23Z

pythonforandroid/toolchain.py

+        parser_apk.add_argument(
+            '--add-asset', dest='assets',
+            action="append",
+            help=('Put this in the assets folder'))


Unnecessary () around the string

inclement · 2020-04-26T00:03:48Z

pythonforandroid/toolchain.py

+        parser_apk.add_argument(
+            '--add-asset', dest='assets',
+            action="append",
+            help=('Put this in the assets folder'))


Could you document the : splitting thing in the help?

inclement · 2020-04-26T00:04:59Z

pythonforandroid/toolchain.py

+                    asset_src, asset_dest = asset.split(":")
+                else:
+                    asset_src = asset_dest = asset
+                args.unknown_args += ["--asset", os.path.abspath(asset_src)+":"+asset_dest]


Isn't this just unnecessary? The unknown_args include everything that wasn't parsed by the first p4a cmdparser, so if you just don't have a --add-asset in that argumentparser it will get passed through anyway. It seems you're pulling out the arguments, then adding them again (just with --asset instead of --add-asset)?

AFAIK, the build.py is executed in a different working directory, so we need to convert the relative paths to absolute before all that. The unknown_args part is not the problem, os.path.abapath is.

Ah, got it, that's fine then, we can leave that code pattern for fixing later. Thanks for the clarification.

inclement · 2020-04-26T00:06:28Z

pythonforandroid/bootstraps/common/build/build.py

    ensure_dir(assets_dir)
+    open(os.path.join(assets_dir, ".gitkeep"), 'a').close()


Is this needed? Even if you're adding the output to git, it seems like this stuff should be handled at that later stage (although it's acceptable if there's some good reason for it).

I asked about this on Discord. As it stands now, it's fine if the assets directory is completely deleted and recreated each time, but the behaviour used to be that the assets directory was kept so I tried to be consistent with that.

Got it. I think it should be fine to ditch it and let the directory be deleted if you want, but if leaving this in could you add a comment noting this reasoning?

AndreMiras · 2020-04-26T07:14:07Z

pythonforandroid/bootstraps/common/build/build.py

+    if os.path.exists(assets_dir):
+        shutil.rmtree(assets_dir)


I'm allergic to if 😄

Suggested change

if os.path.exists(assets_dir):

shutil.rmtree(assets_dir)

shutil.rmtree(assets_dir, ignore_errors=True)

AndreMiras · 2020-04-26T07:28:00Z

pythonforandroid/toolchain.py

@@ -494,6 +494,10 @@ def add_parser(subparsers, *args, **kwargs):
        # However, it is also needed before the distribution is finally
        # assembled for locating the setup.py / other build systems, which
        # is why we also add it here:
+        parser_apk.add_argument(


Could you cover test your change in tests/test_toolchain.py?
There're already some examples in this file and as I understand what you're doing in toolchain.py unit testing the behaviour should be fairly easy
Edit: forgot to mention I'd be glad to help if you're stuck

AndreMiras

Thanks for updating ❤️ this is looking good to me.
Let's wait for inclement approval otherwise I'll merge before this conflict further.
I wish we had (non blocking):

unit tests for toolchain as suggested here Adding more assets #2132 (comment)
squashed commits (I hope we think about it upon merging)

* command line option to add to the assets/ directory * allow custom destination paths * code review suggestions * fix bugs sleepliy introduced in commit "code review suggestions" * flake8 compliance

🔖 v2020.06.02 * Adds missing requests sub dependencies (kivy#2221) * Bumps to Gradle 6.4.1 (kivy#2222) * Bumps to Cython==0.29.19 (kivy#2220) * Updates install and troubleshooting docs (kivy#2219) * Bumps to Ubuntu 20.04 (kivy#2218) * Attempt to improve the issue template (kivy#2217) * Add `opencv_extras` recipe (kivy#2209) * Split logic for build modes & debug symbols (kivy#2213) * Troubleshoot SSL error (kivy#2205) * Remove superfluous recipes fixes (kivy#2202) * Add tests for hostpython3 recipe (kivy#2196) * Fix for 'cannot find setuptools module' (kivy#2195) * Rename `Hostpython3Recipe` class to camel case (kivy#2194) * Fix `test_should_build` (kivy#2193) * Add initial tests for python3 recipe (kivy#2192) * PythonActivityUtil helper for unpacking data (kivy#2189) * Fixes flake8 errors post update (kivy#2191) * Share PythonUtil.java between bootstraps (kivy#2188) * Java code linting using PMD 6.23.0 (kivy#2187) * Deletes deprecated renpy Python{Activity,Service}.java (kivy#2186) * Removes java concurrency/ folder (kivy#2185) * Reuse common AssetExtract.java (kivy#2182) * Use common Hardware.java (kivy#2183) * Moves kamranzafar/ java directory to common/ (kivy#2184) * Updates release documentation (kivy#2177) * Fixes service only unittest loading (kivy#2181) * Narrows some context manager scopes (kivy#2179) * Downgrades to SDL2 2.0.9 (kivy#2180) * Bump to SDL2 2.0.10 & extract .java from SDL2 tarball (kivy#2113) * Adds pygame recipe (kivy#2164) * Adds macOS install instructions (2165) * Removed python2 support mention from README (kivy#2162) * Adding more assets (kivy#2132) * Get --add-source working for dirs in Gradle builds (kivy#2156) * Fixes python build with macOS venv (kivy#2159)

Robert Pfeiffer added 2 commits April 4, 2020 13:54

command line option to add to the assets/ directory

9686a98

allow custom destination paths

975162e

inclement reviewed Apr 26, 2020

View reviewed changes

AndreMiras reviewed Apr 26, 2020

View reviewed changes

AndreMiras mentioned this pull request Apr 26, 2020

pygame recipe #2019

Closed

Robert Pfeiffer added 4 commits April 26, 2020 18:18

code review suggestions

bd68502

Merge remote-tracking branch 'kivy/develop' into adding-more-assets

bcee016

fix bugs sleepliy introduced in commit "code review suggestions"

e5aff0c

flake8 compliance

e97872e

AndreMiras approved these changes Apr 27, 2020

View reviewed changes

inclement merged commit 4b6e095 into kivy:develop Apr 29, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding more assets #2132

Adding more assets #2132

robertpfeiffer commented Apr 4, 2020

inclement left a comment

inclement Apr 26, 2020

robertpfeiffer Apr 26, 2020

AndreMiras Apr 26, 2020

inclement Apr 26, 2020

inclement Apr 26, 2020

inclement Apr 26, 2020

inclement Apr 26, 2020

robertpfeiffer Apr 26, 2020

inclement Apr 26, 2020

inclement Apr 26, 2020

robertpfeiffer Apr 26, 2020

inclement Apr 26, 2020

AndreMiras Apr 26, 2020

AndreMiras Apr 26, 2020 •

edited

Loading

AndreMiras left a comment •

edited

Loading

		ensure_dir(assets_dir)
		open(os.path.join(assets_dir, ".gitkeep"), 'a').close()

	if os.path.exists(assets_dir):
	shutil.rmtree(assets_dir)
	shutil.rmtree(assets_dir, ignore_errors=True)

Adding more assets #2132

Adding more assets #2132

Conversation

robertpfeiffer commented Apr 4, 2020

inclement left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndreMiras Apr 26, 2020 • edited Loading

Choose a reason for hiding this comment

AndreMiras left a comment • edited Loading

Choose a reason for hiding this comment

AndreMiras Apr 26, 2020 •

edited

Loading

AndreMiras left a comment •

edited

Loading