Conversation
…ript Co-authored-by: streed <805140+streed@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the installation script to automatically install additional dependencies (pdftotext and lil-rag service) when no configuration file is detected (first install). This addresses the need for a more complete installation experience for new users.
- Added first-install detection based on configuration file presence
- Added automatic installation of
pdftotextvia system package managers - Added automatic installation of
lil-ragservice from GitHub repository
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| local temp_dir=$(mktemp -d) | ||
| cleanup_temp() { | ||
| rm -rf "$temp_dir" | ||
| } | ||
| trap cleanup_temp EXIT |
There was a problem hiding this comment.
The trap is set inside the function scope and will only execute when the function exits, not when the script exits. If the script is interrupted or fails elsewhere after calling this function, the temporary directory may not be cleaned up. Move the trap outside the function or use a global cleanup mechanism.
install.sh
Outdated
| cd "$temp_dir/lil-rag" | ||
| if ! go build -o lil-rag .; then |
There was a problem hiding this comment.
Using cd without saving the previous directory can leave the script in an unexpected location if subsequent operations fail. Consider using pushd/popd or executing the build command with the -C flag: go build -C \"$temp_dir/lil-rag\" -o lil-rag .
| cd "$temp_dir/lil-rag" | |
| if ! go build -o lil-rag .; then | |
| if ! go build -C "$temp_dir/lil-rag" -o lil-rag .; then |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: streed <805140+streed@users.noreply.github.com>
|
@copilot the lil-rag installation should use the |
Co-authored-by: streed <805140+streed@users.noreply.github.com>
lil-rag service from https://github.com/streed/lil-rag as well as install the pdftotext command.Co-authored-by: streed <805140+streed@users.noreply.github.com>
Updated the lil-rag installation to use the repository's own Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Addresses the requirement to automatically install additional dependencies when no configuration file is present (first install).
Changes Made
The installation script now detects first-time installations and automatically sets up the complete ML Notes ecosystem:
🔍 First Install Detection
is_first_install()function that checks for configuration file at~/.config/ml-notes/config.jsonml-notes config pathif the binary is already available📦 Automatic Dependency Installation
lil-rag Service Installation:
https://github.com/streed/lil-raginstall.shscript for proper installation/usr/local/binalongside ml-notespdftotext Command Installation:
poppler-utilsvia apt/yum/dnf/pacman package managerspopplervia Homebrew🛡️ Robust Error Handling
🔧 Enhanced Requirements
gitto required tools since it's needed for cloning lil-ragExample Output
On first install, users will see:
If dependencies fail to install automatically, users get clear guidance:
On subsequent installs where configuration already exists, the script behaves exactly as before with no additional dependency installation.
This enhancement ensures users get a complete, ready-to-use ML Notes setup on first install while maintaining the reliability and simplicity of the existing installation process.
Warning
https://api.github.com/repos/streed/ml-notes/releases/latestcurl -s REDACTED(http block)If you need me to access, download, or install something from one of these locations, you can either:
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.