Ignore nonexistent paths in iter_modules #917

dbnicholson · 2023-07-24T15:45:53Z

I'm bundling Django and it uses pkgutil.iter_modules to find management commands provided by Django apps. The problem is that Django runs pkg_util.iter_modules([command_dir]) without bothering to check that command_dir exists. Presumably this is because the builtin python loaders return an empty list from iter_modules when the specified directory doesn't exist.

The app crashes with a FileNotFoundError PyException during startup.

07-26 19:50:01.775 18245 18245 E AndroidRuntime: FATAL EXCEPTION: main
07-26 19:50:01.775 18245 18245 E AndroidRuntime: Process: org.endlessos.testapp, PID: 18245
07-26 19:50:01.775 18245 18245 E AndroidRuntime: java.lang.RuntimeException: Unable to start activity ComponentInfo{org.endlessos.testapp/org.endlessos.testapp.MainActivity}: com.chaquo.python.PyException: FileNotFoundError: kolibri/plugins/user_profile/management/commands
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3635)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:3792)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at android.app.servertransaction.LaunchActivityItem.execute(LaunchActivityItem.java:103)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:135)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:95)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2210)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at android.os.Handler.dispatchMessage(Handler.java:106)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at android.os.Looper.loopOnce(Looper.java:201)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at android.os.Looper.loop(Looper.java:288)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at android.app.ActivityThread.main(ActivityThread.java:7839)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at java.lang.reflect.Method.invoke(Native Method)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1003)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: Caused by: com.chaquo.python.PyException: FileNotFoundError: kolibri/plugins/user_profile/management/commands
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at <python>.java.android.importer.listdir(.\java\android\importer.py:501)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at <python>.java.android.importer.iter_modules(.\java\android\importer.py:451)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at <python>.pkgutil.iter_modules(pkgutil.py:131)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at <python>.django.core.management.<listcomp>(__init__.py:30)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at <python>.django.core.management.find_commands(__init__.py:30)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at <python>.django.core.management.get_commands(__init__.py:74)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at <python>.django.core.management.call_command(__init__.py:106)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at <python>.kolibri.utils.main._migrate_databases(main.py:112)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at <python>.kolibri.utils.main.update(main.py:364)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at <python>.kolibri.utils.main.initialize(main.py:300)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at <python>.main.setup(main.py:17)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at <python>.chaquopy_java.call(chaquopy_java.pyx:379)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at <python>.chaquopy_java.Java_com_chaquo_python_PyObject_callAttrThrowsNative(chaquopy_java.pyx:351)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at com.chaquo.python.PyObject.callAttrThrowsNative(Native Method)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at com.chaquo.python.PyObject.callAttrThrows(PyObject.java:232)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at com.chaquo.python.PyObject.callAttr(PyObject.java:221)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at org.endlessos.testapp.MainActivity.onCreate(MainActivity.java:34)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at android.app.Activity.performCreate(Activity.java:8051)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at android.app.Activity.performCreate(Activity.java:8031)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1329)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3608)
07-26 19:50:01.775 18245 18245 E AndroidRuntime: 	... 12 more

dbnicholson · 2023-07-24T15:47:00Z

I actually haven't tested this since I haven't figured out how to do that, but I'm pretty sure it's correct and what the implementation in pkgutil does.

mhsmith · 2023-07-25T13:17:40Z

Thanks for the PR. You can run the tests using the demo app in the root of this repository.

First set up the "Build prerequisites" listed in product/README.md.
Build the Maven repository by running ./gradlew publish in the product directory.
Open the demo app in Android Studio, and run it on any device or emulator.
The test you added will be included in the "Python unit tests" page.

Please make sure the test passes and then let me know.

Also, can you describe the symptoms of the Django problem, and how to reproduce it? If it causes a crash, please post the stack trace here.

Related: #820.

dbnicholson · 2023-07-25T14:05:48Z

Thanks for the PR. You can run the tests using the demo app in the root of this repository.

First build the Maven repository by running ./gradlew publish in the product directory.

I'm getting there, but it's pretty complex since you need 4 python versions and 3 java versions.

Also, can you describe the symptoms of the Django problem, and how to reproduce it? If it causes a crash, please post the stack trace here.

Sorry, I should have opened an issue first. Django enumerates commands for each Django app by blindly calling pkgutil.iter_modules on the management/commands subdirectory of the app package without first checking if the directory exists. It's been doing this for years since the default iter_modules handles the directory not existing. See django and pkgutil.

mhsmith · 2023-07-25T14:16:15Z

I'm getting there, but it's pretty complex since you need 4 python versions and 3 java versions.

I agree it's a bit awkward. I recently updated product/README.md, which I hope makes it clear that to run gradlew publish you only need Java version 8.

The builtin python loaders return an empty list from `iter_modules` for a nonexistent path. Django expects this when it enumerates app management commands. Fixes: chaquo#920

dbnicholson · 2023-07-26T19:54:38Z

I was able to build everything now and run the python tests from the app demo. They pass after a fix to pass a list containing the individual path. I opened #920 to document what happens without the change.

dbnicholson · 2023-07-26T20:00:43Z

Open the demo app in Android Studio, and run it on any device or emulator.

The test you added will be included in the "Python unit tests" page.

I assume you've looked into this, but have you considered running the python tests as instrumented tests instead of running them from the demo app? I.e., product/runtime/src/androidTest? Since the demo app already uses gradle 8, you could even run them with gradle managed devices and do it in github actions as part of the CI.

dbnicholson · 2023-07-26T20:10:22Z

I'm getting there, but it's pretty complex since you need 4 python versions and 3 java versions.

I agree it's a bit awkward. I recently updated product/README.md, which I hope makes it clear that to run gradlew publish you only need Java version 8.

True, but you need Java 17 to build the demo app. It's doable, but not straightforward. target/Dockerfile looks like it gets you pretty close, but it would be nice if there was a recipe that covered everything. I ended up kinda using pyenv and kinda running commands from ci.yml. I'm really curious how you run the tests since you obviously don't push garbage commits. Ideally you'd run all the tests from github actions so even if you couldn't figure out how to setup the local environment the CI could be trusted to catch regressions.

mhsmith · 2023-07-29T22:56:13Z

I assume you've looked into this, but have you considered running the python tests as instrumented tests instead of running them from the demo app? I.e., product/runtime/src/androidTest? Since the demo app already uses gradle 8, you could even run them with gradle managed devices and do it in github actions as part of the CI.

Yes, I'd like to run them in CI, but havent got round to it yet. It was only last week I added CI for the Gradle plugin tests, which are more important to automate because they're so much slower.

I'm really curious how you run the tests since you obviously don't push garbage commits.

At the moment I just run them manually on my own devices or emulators. For each individual commit I use my judgement for which devices are necessary to test on, and then before each release I run them on a full set of devices as listed in release/README.md.

mhsmith · 2023-08-19T22:22:48Z

product/runtime/src/test/python/chaquopy/test/android/test_import.py

        for path in ("somemissingpackage", "some/missing/package"):
-            mod_infos = list(pkgutil.iter_modules([path]))
+            mod_infos = list(pkgutil.iter_modules([f"{murmurhash.__path__[0]}/{path}"]))
            self.assertEqual([], mod_infos)


Similarly to #924, the test didn't reproduce the original problem, because it wasn't passing a path that actually led to an AssetFinder. I've fixed this by making it pass an absolute path.

mhsmith · 2023-08-19T22:23:17Z

Thanks!

mhsmith mentioned this pull request Jul 25, 2023

Adding AssetFinder subdirectories to sys.path causes extract_packages error #820

Closed

Ignore nonexistent paths in iter_modules

deb7b85

The builtin python loaders return an empty list from `iter_modules` for a nonexistent path. Django expects this when it enumerates app management commands. Fixes: chaquo#920

dbnicholson force-pushed the iter_modules branch from e713260 to deb7b85 Compare July 26, 2023 19:52

mhsmith mentioned this pull request Jul 29, 2023

pkgutil.iter_modules fails on missing directory #920

Closed

mhsmith modified the milestones: 15.1, 15.0 Jul 29, 2023

mhsmith added 2 commits August 19, 2023 23:09

Merge branch 'master' into iter_modules

6421a39

Make iter_modules test reproduce the original problem

43e441d

mhsmith reviewed Aug 19, 2023

View reviewed changes

mhsmith merged commit 9f30387 into chaquo:master Aug 19, 2023

dbnicholson mentioned this pull request Sep 1, 2023

zipfile LZMA support fails #952

Closed

dbnicholson deleted the iter_modules branch September 10, 2023 20:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ignore nonexistent paths in iter_modules #917

Ignore nonexistent paths in iter_modules #917

dbnicholson commented Jul 24, 2023 •

edited by mhsmith

Loading

dbnicholson commented Jul 24, 2023

mhsmith commented Jul 25, 2023 •

edited

Loading

dbnicholson commented Jul 25, 2023

mhsmith commented Jul 25, 2023

dbnicholson commented Jul 26, 2023

dbnicholson commented Jul 26, 2023

dbnicholson commented Jul 26, 2023

mhsmith commented Jul 29, 2023

mhsmith Aug 19, 2023 •

edited

Loading

mhsmith commented Aug 19, 2023

Ignore nonexistent paths in iter_modules #917

Ignore nonexistent paths in iter_modules #917

Conversation

dbnicholson commented Jul 24, 2023 • edited by mhsmith Loading

dbnicholson commented Jul 24, 2023

mhsmith commented Jul 25, 2023 • edited Loading

dbnicholson commented Jul 25, 2023

mhsmith commented Jul 25, 2023

dbnicholson commented Jul 26, 2023

dbnicholson commented Jul 26, 2023

dbnicholson commented Jul 26, 2023

mhsmith commented Jul 29, 2023

mhsmith Aug 19, 2023 • edited Loading

Choose a reason for hiding this comment

mhsmith commented Aug 19, 2023

dbnicholson commented Jul 24, 2023 •

edited by mhsmith

Loading

mhsmith commented Jul 25, 2023 •

edited

Loading

mhsmith Aug 19, 2023 •

edited

Loading