fix: provision lua runtime and repair fish activation - #1858
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe activation scripts now select per-user XDG runtime directories, export session defaults in fish, resolve Lua and LuaRocks through ordered candidates, and report missing runtime components. The core package list adds Lua as a dependency. ChangesLua runtime activation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Activation
participant check_lua_runtime
participant LuaResolvers
participant stderr
Activation->>check_lua_runtime: validate Lua runtime
check_lua_runtime->>LuaResolvers: resolve lua and luarocks
LuaResolvers-->>check_lua_runtime: return paths or missing components
check_lua_runtime->>stderr: print detailed missing list
check_lua_runtime-->>Activation: return success or failure
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Configs/.local/lib/hyde/shell/activate.fish`:
- Around line 13-17: Update the XDG_RUNTIME_DIR fallback in the activation setup
to avoid assigning the shared /tmp directory. Use a validated per-user runtime
directory instead, or fail with an actionable diagnostic when none is available;
leave the other XDG_* defaults unchanged.
- Around line 13-17: The XDG variable initialization in the activation script
does not export values that already exist in Fish scopes. Update each affected
XDG assignment, including the session-variable block noted in the review, to
select the existing value or default first, then explicitly set the final value
with set -gx so child processes receive it.
In `@Scripts/install_pre.sh`:
- Around line 45-55: Use the same Lua executable-resolution contract everywhere:
update Scripts/install_pre.sh lines 45-55 to honor the LUA and LUAROCKS
overrides and versioned executables, or delegate preflight validation to
lua_env.py; update Configs/.local/lib/hyde/shell/activate lines 73-90 and
Configs/.local/lib/hyde/shell/activate.fish lines 64-77 to apply the identical
resolution logic during activation, preserving consistent behavior across
installation and both shell types.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 402e8398-84d3-47c6-8461-5e7813f8d70d
📒 Files selected for processing (5)
Configs/.local/lib/hyde/shell/activateConfigs/.local/lib/hyde/shell/activate.fishScripts/dots/deps.tomlScripts/install_pre.shScripts/pkg_core.lst
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Configs/.local/lib/hyde/shell/activate`:
- Around line 22-35: The runtime-directory fallbacks must fail closed instead of
exporting a predictable or improperly secured path. In
Configs/.local/lib/hyde/shell/activate lines 22-35, update
hyde_runtime_dir/setup_xdg to create a unique owner-private directory atomically
or validate its owner and mode, return failure when setup or hardening fails,
and avoid assigning XDG_RUNTIME_DIR in that case. Apply the equivalent secure
creation/validation and failure guard in
Configs/.local/lib/hyde/shell/activate.fish lines 21-35, preventing set -gx
XDG_RUNTIME_DIR after failure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6c2dd7ef-5058-4892-84ad-b6055bf0f682
📒 Files selected for processing (3)
Configs/.local/lib/hyde/shell/activateConfigs/.local/lib/hyde/shell/activate.fishScripts/install_pre.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- Scripts/install_pre.sh
| _dir="${TMPDIR:-/tmp}/hyde-${_uid:-$$}" | ||
| mkdir -p "$_dir" 2>/dev/null && chmod 700 "$_dir" 2>/dev/null | ||
| printf '%s\n' "$_dir" | ||
| unset _uid _dir | ||
| } | ||
|
|
||
| setup_xdg() { | ||
| : "${XDG_CONFIG_HOME:=$HOME/.config}" | ||
| : "${XDG_CACHE_HOME:=$HOME/.cache}" | ||
| : "${XDG_DATA_HOME:=$HOME/.local/share}" | ||
| : "${XDG_STATE_HOME:=$HOME/.local/state}" | ||
|
|
||
| # fallback only | ||
| : "${XDG_RUNTIME_DIR:=/tmp}" | ||
| [ -n "${XDG_RUNTIME_DIR:-}" ] || XDG_RUNTIME_DIR=$(hyde_runtime_dir) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Secure runtime-directory creation and fail closed in both activation scripts. Both fallbacks accept a predictable path under $TMPDIR and export it even if permission hardening fails, allowing a pre-created directory to become the session runtime namespace.
Configs/.local/lib/hyde/shell/activate#L22-L35: create a unique owner-private directory atomically (or validate owner/mode) and return failure without exporting it when setup fails.Configs/.local/lib/hyde/shell/activate.fish#L21-L35: apply the equivalent secure creation/validation and preventset -gx XDG_RUNTIME_DIRafter failure.
📍 Affects 2 files
Configs/.local/lib/hyde/shell/activate#L22-L35(this comment)Configs/.local/lib/hyde/shell/activate.fish#L21-L35
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Configs/.local/lib/hyde/shell/activate` around lines 22 - 35, The
runtime-directory fallbacks must fail closed instead of exporting a predictable
or improperly secured path. In Configs/.local/lib/hyde/shell/activate lines
22-35, update hyde_runtime_dir/setup_xdg to create a unique owner-private
directory atomically or validate its owner and mode, return failure when setup
or hardening fails, and avoid assigning XDG_RUNTIME_DIR in that case. Apply the
equivalent secure creation/validation and failure guard in
Configs/.local/lib/hyde/shell/activate.fish lines 21-35, preventing set -gx
XDG_RUNTIME_DIR after failure.
There was a problem hiding this comment.
Yes, that line is the fix rather than the old code. The /tmp default is gone:
[ -n "${XDG_RUNTIME_DIR:-}" ] || XDG_RUNTIME_DIR=$(hyde_runtime_dir)hyde_runtime_dir prefers /run/user/$(id -u) when it exists and is writable, and only otherwise falls back to ${TMPDIR:-/tmp}/hyde-$(id -u) created with mode 700. Same change in activate.fish.
Checked both paths: with /run/user present both shells report /run/user/1000; with a stubbed id returning an unused uid both create the private directory as drwx------.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
|
All three findings are addressed in e29d241. Shared runtime directory. Pre-existing Fish variables. Resolution contract. All three shell layers now follow the same order as Verified against a stub
|
|
@RAprogramm I somehow tweak a bit the install.sh to use |
|
@RAprogramm Maybe we can solve this first? I recently push deez-dots for core dependency installation. so lua and luraocks should exist first. |
|
Rebased onto dev, conflict resolved. Part of this was overtaken by upstream in the meantime, so the branch is now smaller:
What is left is the part upstream did not cover:
Both files parse clean and |
|
@kRHYME7 agreed, and that part is already out — I dropped the dependency declarations and the What is left is only the guard, and one case where it is not just belt and braces: check_lua_runtime() {
hyde_has lua || return 1
hyde_has luarocks || return 1
}
The other two are independent of dependency installation: the Happy to cut it down further if you would rather keep just the resolution part. |
| _dir="${TMPDIR:-/tmp}/hyde-${_uid:-$$}" | ||
| mkdir -p "$_dir" 2>/dev/null && chmod 700 "$_dir" 2>/dev/null | ||
| printf '%s\n' "$_dir" | ||
| unset _uid _dir | ||
| } | ||
|
|
||
| setup_xdg() { | ||
| : "${XDG_CONFIG_HOME:=$HOME/.config}" | ||
| : "${XDG_CACHE_HOME:=$HOME/.cache}" | ||
| : "${XDG_DATA_HOME:=$HOME/.local/share}" | ||
| : "${XDG_STATE_HOME:=$HOME/.local/state}" | ||
|
|
||
| # fallback only | ||
| : "${XDG_RUNTIME_DIR:=/tmp}" | ||
| [ -n "${XDG_RUNTIME_DIR:-}" ] || XDG_RUNTIME_DIR=$(hyde_runtime_dir) |
2 participants
Pull Request
Description
Closes #1852.
Rebased onto dev after
fd70502fand9db7cf1dlanded. Everything those twoalready cover is gone from this branch, so what follows describes the diff as
it stands now, not the original scope.
What upstream now covers
install_pre.shinstall.shluaandluarocksindeps.tomlluarocksinpkg_core.lstWhat is left
Activation no longer rejects a runtime the environment manager accepts.
check_lua_runtimelooks forluaandluarocksby those exact names:pyutils/lua_env.pyresolves versioned binaries too. On a distribution thatships
lua5.4andluarocks-5.4without unversioned symlinks, theenvironment provisions correctly and activation then refuses it, leaving
HYPRLAND_CONFIGunset with onlyNo valid HyDE configuration foundto goon. Both shell variants now use the same order as
lua_env.py:$LUA, thenlua,lua5.5,lua5.4,lua5.3, and$LUAROCKS, thenluarocks,luarocks-5.5,luarocks-5.4,luarocks-5.3. When something really ismissing, the message names it.
A private runtime directory instead of a shared one. Both variants fell
back to
XDG_RUNTIME_DIR=/tmp, which puts sockets from every session into onenamespace. The fallback now prefers
/run/user/$(id -u)when it exists and iswritable, and otherwise creates
${TMPDIR:-/tmp}/hyde-$(id -u)with mode 700.pkg_core.lstgainslua. Onlyluarockswas listed there; theinterpreter itself was missing.
Type of change
Checklist
Testing
Syntax:
sh -n activate,fish -n activate.fish— both clean.activateisclean under shellcheck at error severity.
Behaviour, in an isolated environment with a stubbed
PATH:lua5.4andluarocks-5.4present$LUAand$LUAROCKSsetLua runtime incomplete, missing: lua luarocksRuntime directory: with
/run/userpresent both shells report/run/user/1000; with a stubbedidreturning an unused uid both fall back tothe private directory, created
drwx------.Verified against Hyprland 0.56.1 on Arch Linux.
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Chores