fix(shell): add OpenSSL env vars to bash profile and zsh env - #632
Conversation
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
Summary of ChangesHello @shunkakinoki, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the shell environment configuration to ensure that OpenSSL is correctly discoverable for Rust/Cargo builds, particularly on Linux systems. It achieves this by strategically placing OpenSSL-related environment variables in appropriate bash and zsh configuration files, specifically targeting login shells for bash and all instances for zsh. Additionally, it standardizes the sourcing of Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
- Add OpenSSL environment variables to bash profileExtra for login shells - Move zsh Linux-specific env vars from initContent to envExtra (.zshenv) - Ensures cargo builds work in login shells and non-interactive zsh contexts - Source .bashrc from .bash_profile for consistent PATH setup
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThis PR modifies bash and zsh initialization scripts in home-manager to add Linux-specific OpenSSL environment variables for Rust/Cargo builds and refactors the zsh configuration structure. Bash now sources user's .bashrc and sets OpenSSL-related paths, while zsh introduces a new envExtra attribute to separate environment variables from initialization content. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
✨ 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 |
There was a problem hiding this comment.
Code Review
This pull request correctly sets up OpenSSL environment variables for bash and zsh, which is great for ensuring tools like Cargo can find necessary libraries. The logic for zsh is sound, moving environment settings to .zshenv for broader availability. However, I've found some code duplication in the bash configuration that should be addressed to improve maintainability.
| # OpenSSL for cargo builds on Linux (available in login shells) | ||
| if [ "$(uname)" = "Linux" ]; then | ||
| export XDG_RUNTIME_DIR="/run/user/$(id -u)" | ||
| export PKG_CONFIG_PATH="${pkgs.openssl.dev}/lib/pkgconfig''${PKG_CONFIG_PATH:+:$PKG_CONFIG_PATH}" | ||
| export OPENSSL_DIR="${pkgs.openssl.dev}" | ||
| export OPENSSL_LIB_DIR="${pkgs.openssl.out}/lib" | ||
| export OPENSSL_INCLUDE_DIR="${pkgs.openssl.dev}/include" | ||
| fi |
There was a problem hiding this comment.
This block defining XDG_RUNTIME_DIR and the OpenSSL environment variables is redundant. The bashrcExtra configuration (lines 36-44) already sets these same variables. Since you are now sourcing .bashrc from .bash_profile (lines 82-84), these variables will be available in login shells, making this block unnecessary. Removing this duplication will make the configuration cleaner and easier to maintain.
Mesa DescriptionTL;DRAdded OpenSSL environment variables to bash profile and zsh environment. What changed?
Description generated by Mesa. Update settings |
Mesa DescriptionTL;DREnsure OpenSSL environment variables are correctly loaded in bash login shells and all zsh contexts to facilitate Cargo builds, and standardize PATH setup in bash login shells by sourcing What changed?
Description generated by Mesa. Update settings |
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="home-manager/programs/bash/default.nix">
<violation number="1" location="home-manager/programs/bash/default.nix:72">
P2: OpenSSL export block duplicated in profileExtra and bashrcExtra; login shells source .bashrc and run the block twice, duplicating PKG_CONFIG_PATH entries and bloating env</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| . '/nix/var/nix/profiles/default/etc/profile.d/nix-daemon.sh' | ||
| fi | ||
|
|
||
| # OpenSSL for cargo builds on Linux (available in login shells) |
There was a problem hiding this comment.
P2: OpenSSL export block duplicated in profileExtra and bashrcExtra; login shells source .bashrc and run the block twice, duplicating PKG_CONFIG_PATH entries and bloating env
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/programs/bash/default.nix, line 72:
<comment>OpenSSL export block duplicated in profileExtra and bashrcExtra; login shells source .bashrc and run the block twice, duplicating PKG_CONFIG_PATH entries and bloating env</comment>
<file context>
@@ -68,6 +68,20 @@
. '/nix/var/nix/profiles/default/etc/profile.d/nix-daemon.sh'
fi
+
+ # OpenSSL for cargo builds on Linux (available in login shells)
+ if [ "$(uname)" = "Linux" ]; then
+ export XDG_RUNTIME_DIR="/run/user/$(id -u)"
</file context>
There was a problem hiding this comment.
Pull request overview
This pull request enhances shell configuration to ensure OpenSSL environment variables are available across different shell contexts for cargo/rust builds. The changes also improve bash login shell behavior by ensuring .bashrc is sourced.
Changes:
- Move zsh Linux-specific environment variables from
initContenttoenvExtra(.zshenv) for broader availability - Add OpenSSL environment variables to bash
profileExtrafor login shell contexts - Source
.bashrcfrom.bash_profileto ensure PATH and other settings are available in login shells
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| home-manager/programs/zsh/default.nix | Moves XDG_RUNTIME_DIR and OpenSSL environment variables from initContent to envExtra to ensure they are available in all zsh instances including non-interactive scripts |
| home-manager/programs/bash/default.nix | Adds OpenSSL and XDG_RUNTIME_DIR environment variables to profileExtra and sources .bashrc from .bash_profile for login shells |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # OpenSSL for cargo builds on Linux (available in login shells) | ||
| if [ "$(uname)" = "Linux" ]; then | ||
| export XDG_RUNTIME_DIR="/run/user/$(id -u)" | ||
| export PKG_CONFIG_PATH="${pkgs.openssl.dev}/lib/pkgconfig''${PKG_CONFIG_PATH:+:$PKG_CONFIG_PATH}" | ||
| export OPENSSL_DIR="${pkgs.openssl.dev}" | ||
| export OPENSSL_LIB_DIR="${pkgs.openssl.out}/lib" | ||
| export OPENSSL_INCLUDE_DIR="${pkgs.openssl.dev}/include" | ||
| fi | ||
|
|
There was a problem hiding this comment.
The OpenSSL and XDG_RUNTIME_DIR environment variables are being set in both bashrcExtra (lines 35-44) and profileExtra (lines 72-79). This creates unnecessary duplication. Since bashrcExtra is sourced by .bashrc and line 82-84 sources .bashrc from .bash_profile, these variables will already be available in login shells through the existing bashrcExtra configuration. The duplication in profileExtra should be removed.
| # OpenSSL for cargo builds on Linux (available in login shells) | |
| if [ "$(uname)" = "Linux" ]; then | |
| export XDG_RUNTIME_DIR="/run/user/$(id -u)" | |
| export PKG_CONFIG_PATH="${pkgs.openssl.dev}/lib/pkgconfig''${PKG_CONFIG_PATH:+:$PKG_CONFIG_PATH}" | |
| export OPENSSL_DIR="${pkgs.openssl.dev}" | |
| export OPENSSL_LIB_DIR="${pkgs.openssl.out}/lib" | |
| export OPENSSL_INCLUDE_DIR="${pkgs.openssl.dev}/include" | |
| fi |
| }; | ||
|
|
||
| initContent = '' | ||
| # envExtra goes to .zshenv (always sourced) |
There was a problem hiding this comment.
The comment states "always sourced" but should be more specific. The .zshenv file is sourced by all zsh instances including non-interactive shells and scripts, but it's not sourced by other shells like bash. Consider updating the comment to be more precise about the sourcing behavior.
| # envExtra goes to .zshenv (always sourced) | |
| # envExtra goes to .zshenv (sourced by all zsh instances, including non-interactive shells) |
Changes
profileExtrafor login shellsinitContenttoenvExtra(.zshenv).bashrcfrom.bash_profilefor consistent PATH setupTechnical Details
profileExtrain bash maps to.bash_profilewhich is sourced by login shellsenvExtrain zsh maps to.zshenvwhich is sourced by all zsh instances including non-interactive scriptsTesting
make buildGenerated with Claude Code by claude-opus-4-5-20251101
Summary by cubic
Ensure OpenSSL env vars are loaded in bash login shells and all zsh contexts so cargo builds find OpenSSL reliably. Also source .bashrc from .bash_profile for consistent PATH.
Written for commit 657c316. Summary will update on new commits.