Run executables directly - #645
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe change adds separate APIs for starting registered apps and executing apps from explicit locations. Shared startup code now handles arguments, streams, parent-child results, and manifest-less instances. ESP32 and POSIX loaders validate executable files before loading and expose executable checks. Applications, services, tests, and the file manager use the new APIs. SDK integration paths and tooling versions are updated. Merge Risk: 🔵 Low · up to This change enables direct execution from Files, but Apple builds may expose Run for invalid shared-object content. The impact is bounded to an incorrect launch affordance, so the change is mergeable with owner awareness and follow-up. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 31.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 135 functions across 61 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Team
Run ID: f06b09dc-27e9-4aba-b418-4b3c86152a95
📒 Files selected for processing (17)
Modules/app-esp32-module/source/app_esp32_loader_service.cppModules/app-module/include/app/elf_check.hModules/app-module/include/app/exec.hModules/app-module/include/app/loader.hModules/app-module/private/app/private/exec_internal.hModules/app-module/source/elf_check.cppModules/app-module/source/exec.cppModules/app-module/source/internal_loader.cppModules/app-module/source/module.cppModules/app-module/tests/source/exec_test.cppModules/app-posix-module/source/app_posix_loader_service.cppModules/app-posix-module/tests/CMakeLists.txtModules/app-posix-module/tests/source/loader_test.cppTactility/Private/Tactility/app/files/View.hTactility/Source/app/files/View.cppTests/SdkIntegration/main/CMakeLists.txtTests/SdkIntegration/tactility.py
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Modules/app-esp32-module/source/app_esp32_loader_service.cpp (1)
163-164: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winReject null
AppLocation::locationvalues forAPP_LOCATION_PATH.
AppLocationdoes not define a non-null contract. Both loaders passlocation.locationto astd::stringparameter without validation. A null value can terminate or crash during string construction. Enforce this precondition in the public path entry points, includingapi_load()andapi_is_executable().
🧹 Nitpick comments (1)
Modules/app-module/tests/source/execute_test.cpp (1)
136-138: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd direct
app_execute()path coverage.The POSIX suite executes
FIXTURE_APP_PATHthroughapp_start_for_result(), but no test callsapp_execute()withAPP_LOCATION_PATH. Add this case to the POSIX integration suite and verify the fixture lifecycle and result.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Team
Run ID: dd00113f-0317-491c-a115-1c5a3a537b6f
📒 Files selected for processing (49)
Modules/app-esp32-module/source/app_esp32_loader_service.cppModules/app-module/include/app/execute.hModules/app-module/include/app/manager.hModules/app-module/include/app/start.hModules/app-module/include/app/stream.hModules/app-module/private/app/private/ledger.hModules/app-module/private/app/private/manager_internal.hModules/app-module/source/execute.cppModules/app-module/source/manager.cppModules/app-module/source/module.cppModules/app-module/source/scheduler.cppModules/app-module/source/start.cppModules/app-module/tests/source/execute_test.cppModules/app-module/tests/source/io_test.cppModules/app-module/tests/source/manager_test.cppModules/app-module/tests/source/stream_test.cppModules/app-posix-module/source/app_posix_loader_service.cppModules/app-posix-module/tests/source/loader_test.cppTactility/Include/Tactility/app/fileselection/FileSelection.hTactility/Include/Tactility/app/wifimanage/WifiManage.hTactility/Source/Tactility.cppTactility/Source/app/alertdialog/AlertDialog.cppTactility/Source/app/appdetails/AppDetails.cppTactility/Source/app/apphubdetails/AppHubDetailsApp.cppTactility/Source/app/applist/AppList.cppTactility/Source/app/boot/Boot.cppTactility/Source/app/btmanage/BtManage.cppTactility/Source/app/btpeersettings/BtPeerSettings.cppTactility/Source/app/crashdiagnostics/CrashDiagnostics.cppTactility/Source/app/development/Development.cppTactility/Source/app/files/View.cppTactility/Source/app/fileselection/FileSelection.cppTactility/Source/app/gpssettings/GpsSettings.cppTactility/Source/app/i2cscanner/I2cScanner.cppTactility/Source/app/imageviewer/ImageViewer.cppTactility/Source/app/inputdialog/InputDialog.cppTactility/Source/app/launcher/Launcher.cppTactility/Source/app/notes/Notes.cppTactility/Source/app/selectiondialog/SelectionDialog.cppTactility/Source/app/settings/Settings.cppTactility/Source/app/setup/Setup.cppTactility/Source/app/timedatesettings/TimeDateSettings.cppTactility/Source/app/timezone/TimeZone.cppTactility/Source/app/touchcalibration/TouchCalibration.cppTactility/Source/app/wifiapsettings/WifiApSettings.cppTactility/Source/app/wificonnect/WifiConnect.cppTactility/Source/app/wifimanage/WifiManage.cppTactility/Source/service/development/DevelopmentService.cppTactility/Source/service/webserver/WebServerService.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
- Modules/app-module/private/app/private/ledger.h
Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Team
Run ID: cf985051-4ff7-4192-8db9-99dfb50fdb96
📒 Files selected for processing (6)
Modules/app-module/include/app/execute.hModules/app-module/private/app/private/arguments.hModules/app-module/private/app/private/ledger.hModules/app-module/source/internal_loader.cppModules/app-module/source/manager.cppModules/app-module/source/scheduler.cpp
💤 Files with no reviewable changes (2)
- Modules/app-module/private/app/private/ledger.h
- Modules/app-module/source/internal_loader.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
- Modules/app-module/include/app/execute.h
Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review.
Uh oh!
There was an error while loading. Please reload this page.