Improve run_multi_process_host.py helper script [chromium/src : main]
0 views
Skip to first unread message
Yuwei Huang (Gerrit)
May 20, 2026, 8:53:47 PM (21 hours ago) May 20
to Joe Downing, chromium...@chromium.org, chromotin...@chromium.org, android-bu...@system.gserviceaccount.com
Attention needed from Joe Downing
Yuwei Huang voted and added 1 comment![]( "Open in Gerrit")
Votes added by Yuwei Huang
| Auto-Submit | +1 |
| Commit-Queue | +1 |
1 comment
Patchset-level comments
Related details
Attention is currently required from:
- Joe Downing
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joe Downing (Gerrit)
May 20, 2026, 9:00:59 PM (21 hours ago) May 20
to Yuwei Huang, Chromium LUCI CQ, chromium...@chromium.org, chromotin...@chromium.org, android-bu...@system.gserviceaccount.com
Attention needed from Yuwei Huang
Joe Downing voted and added 1 comment![]( "Open in Gerrit")
Votes added by Joe Downing
| Code-Review | +1 |
1 comment
File remoting/tools/run_multi_process_host.py
Line 228, Patchset 4 (Latest):
ensure_permissions(abs_out_dir, user_home, force=set_permissions)Joe Downing . unresolved
If this script is run before the binaries are built, `check_permissions()` will fail because the binaries don't exist yet. This will cause the script to unnecessarily traverse up the parent directories running `setfacl`, and then print a confusing "ACLs updated but permissions still failing." warning.
Consider moving this `ensure_permissions` call down into the `if host_state is not None and core_state is not None:` block where `start_daemon()` is called, as you already do in the monitor loop. This will prevent it from running when the binaries are absent.
Related details
Attention is currently required from:
- Yuwei Huang
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yuwei Huang (Gerrit)
May 20, 2026, 9:32:11 PM (21 hours ago) May 20
to Joe Downing, Chromium LUCI CQ, chromium...@chromium.org, chromotin...@chromium.org, android-bu...@system.gserviceaccount.com
Yuwei Huang voted and added 2 comments![]( "Open in Gerrit")
Votes added by Yuwei Huang
| Auto-Submit | +1 |
| Commit-Queue | +2 |
2 comments
Patchset-level comments
File remoting/tools/run_multi_process_host.py
Line 228, Patchset 4:
ensure_permissions(abs_out_dir, user_home, force=set_permissions)Joe Downing . resolved
If this script is run before the binaries are built, `check_permissions()` will fail because the binaries don't exist yet. This will cause the script to unnecessarily traverse up the parent directories running `setfacl`, and then print a confusing "ACLs updated but permissions still failing." warning.
Consider moving this `ensure_permissions` call down into the `if host_state is not None and core_state is not None:` block where `start_daemon()` is called, as you already do in the monitor loop. This will prevent it from running when the binaries are absent.
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yuwei Huang (Gerrit)
May 20, 2026, 9:38:15 PM (21 hours ago) May 20
to Joe Downing, Chromium LUCI CQ, chromium...@chromium.org, chromotin...@chromium.org, android-bu...@system.gserviceaccount.com
Yuwei Huang voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
May 20, 2026, 11:51:30 PM (18 hours ago) May 20
to Yuwei Huang, Joe Downing, chromium...@chromium.org, chromotin...@chromium.org, android-bu...@system.gserviceaccount.com
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
4 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: remoting/tools/run_multi_process_host.py
Insertions: 1, Deletions: 2.
@@ -225,8 +225,6 @@
remoting_host_path = os.path.join(abs_out_dir, "remoting_me2me_host")
remoting_core_path = os.path.join(abs_out_dir, "libremoting_core.so")
- ensure_permissions(abs_out_dir, user_home, force=set_permissions)
-
daemon_command = [remoting_host_path, "--type=daemon"]
# Initial state for monitoring
@@ -262,6 +260,7 @@
atexit.register(terminate_remote_sessions)
if host_state is not None and core_state is not None:
+ ensure_permissions(abs_out_dir, user_home, force=set_permissions)
start_daemon()
else:
print("Binaries are missing. Waiting for them to be built...")
```
Change information
Commit message:
Improve run_multi_process_host.py helper script
* Add --set-permissions option to forcibly run setfacl. The permissions
check only look at libremoting_core.so and remoting_me2me_host, so
it will miss any changes you make outside of remoting/.
* Monitor libremoting_core.so and remoting_me2me_host and
automatically restart daemon on change.
TAG=agy
CONV=58f33be9-8a77-42c7-8e29-00ab51811d1c
Change-Id: Ia55ab0986065b9865913d94dce0d8d8489e10dbc
Auto-Submit: Yuwei Huang <yuw...@chromium.org>
Commit-Queue: Yuwei Huang <yuw...@chromium.org>
Reviewed-by: Joe Downing <joe...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1634025}
Files:
- M remoting/tools/run_multi_process_host.py
Change size: M
Delta: 1 file changed, 150 insertions(+), 64 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Joe Downing
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages