Enable kernel overlay installs #7
Conversation
…tecture. this will break python support briefly.
…l, non-mobile, conda envs. conda environments not relocated with conda-pack do not always have activate/deactivate files addressible with source. R library location determiantion done by routing directly to $ENV/bin/Rscript (so, for now, it better be there) Cannot use 'conda activate', as this requires conda init and a bash refresh.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
…dpriest-ui/icrn_manager into enable-kernel-overlay-installs
| "R") | ||
| unpack_r_kernel "$target_unpacked" "$language" "$targetname" "$version" | ||
| echo "registering R library" | ||
| register_r_library_in_user_catalog "$target_location" "$language" "$targetname" "$version" |
There was a problem hiding this comment.
Bug: Missing error handling for R library registration
The call to register_r_library_in_user_catalog doesn't check its return value. If this function fails (returns 1), for example when the conda-meta directory doesn't exist, the script continues to register the overlay anyway. This creates an inconsistent catalog state where overlay_path may be set but absolute_path is missing or incorrect, causing subsequent kernels use commands to fail.
| user_catalog_tmp=$(mktemp) | ||
| jq -r ".\"$language\".\"$targetname\".\"$version\"={\"absolute_path\":\"$target_kernel_path\"} " "$user_catalog" > "$user_catalog_tmp" && mv "$user_catalog_tmp" "$user_catalog" | ||
|
|
||
| jq -r ".\"$language\".\"$targetname\".\"$version\"={\"absolute_path\":\"$target_kernel_path\"}" "$user_catalog" > "$user_catalog_tmp" && mv "$user_catalog_tmp" "$user_catalog" |
There was a problem hiding this comment.
Bug: Catalog entry replacement loses existing data
The jq command uses = to replace the entire version entry with a new object containing only absolute_path. If a user re-runs kernels get on an already-checked-out kernel, any existing fields in the catalog entry (like overlay_path from a previous checkout) are deleted before being re-added. This creates unnecessary data churn and fragility, though the overlay path gets re-added immediately after by register_user_overlay_in_user_catalog.
| ;; | ||
| "Python") | ||
| unpack_python_kernel "$target_unpacked" "$language" "$targetname" "$version" | ||
| register_python_library_in_user_catalog "$target_location" "$language" "$targetname" "$version" |
There was a problem hiding this comment.
Bug: Missing error handling for Python library registration
The call to register_python_library_in_user_catalog doesn't check its return value. If this function fails (returns 1), for example when the bin/activate script doesn't exist, the script exits the case statement without reporting the error properly. This could leave the user catalog in an inconsistent state or give the impression that the checkout succeeded when it actually failed.
| chmod +x "$TEST_REPO/r_kernels/ggplot2/3.4.0/bin/deactivate" | ||
| echo "#!/bin/bash" > "$TEST_REPO/r_kernels/ggplot2/3.4.0/bin/Rscript" | ||
| echo "echo '/mock/r/lib'" >> "$TEST_REPO/r_kernels/ggplot2/3.4.0/bin/Rscript" | ||
| chmod +x "$TEST_REPO/r_kernels/ggplot2/3.4.0/bin/Rscript" |
There was a problem hiding this comment.
Bug: Test setup missing conda-meta directories for R kernels
The test setup creates mock R kernel environments with bin/ directories but doesn't create the conda-meta/ directories that register_r_library_in_user_catalog checks for on line 506 of icrn_manager. This causes R kernel registration to fail silently in tests, masking the missing error handling bug and preventing proper validation of the R kernel checkout flow.
Note
Replaces tar-based kernel relocation with in-place environments, adds per-user R overlay registration, updates activation/cleanup flows, and aligns tests and update_r_libs.sh with the new model.
icrn_manager):environment_location.overlay_pathin user catalog.register_r_library_in_user_catalog,register_python_library_in_user_catalog,register_user_overlay_in_user_catalog.kernels getto validate inputs, create overlay (R), and register paths; aliasget→get_in_place.kernels use:overlay_path, symlink core lib, callupdate_r_libs.shwith kernel and overlay paths.use <lang> noneremoves only catalog-installed kernels.kernels removecommand; keepcleanfor catalog entries only.target_kernel_pathandtarget_overlay_path; writeR_LIBSasoverlay:kernel:${R_LIBS}and support unsetting.environment_locationand mock conda envs; drop tar/conda-pack artifacts.noneuninstall behavior.Written by Cursor Bugbot for commit 8bf30fb. This will update automatically on new commits. Configure here.