feat(shell): add OpenSSL env vars for cargo builds on Linux - #630
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 development experience for Rust projects on Linux by streamlining the process of locating OpenSSL dependencies during Cargo builds. It addresses a common issue where 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
|
Set PKG_CONFIG_PATH, OPENSSL_DIR, OPENSSL_LIB_DIR, OPENSSL_INCLUDE_DIR in both bash and fish so cargo can find OpenSSL without nix-shell. Fixes: cargo build failing with "openssl-sys: system library not found" 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughEnvironment variables for OpenSSL are added to both Bash and Fish shell initialization scripts in Linux. The changes configure PKG_CONFIG_PATH, OPENSSL_DIR, OPENSSL_LIB_DIR, and OPENSSL_INCLUDE_DIR to support cargo builds with OpenSSL dependencies. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Mesa DescriptionTL;DRAdded OpenSSL environment variables to bash and fish shells on Linux to enable What changed?File summaries were undefined. Description generated by Mesa. Update settings |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds necessary OpenSSL environment variables for Cargo builds on Linux, which is a great improvement. The changes for both bash and fish shells are mostly correct. However, there is a small but critical syntax error in the Nix configuration for bash related to string escaping, which will likely cause the build to fail. I've provided a specific comment with a suggested fix.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@home-manager/programs/bash/default.nix`:
- Around line 38-43: The PKG_CONFIG_PATH export line is over 100 chars; split it
into multiple concatenated strings to keep lines under the limit while
preserving the same value. Update the export statement that sets PKG_CONFIG_PATH
(the line referencing pkgs.openssl.dev and
${PKG_CONFIG_PATH:+:$PKG_CONFIG_PATH}) by breaking the string interpolation into
two or more quoted pieces (e.g., separate the
"${pkgs.openssl.dev}/lib/pkgconfig" segment from the
"${PKG_CONFIG_PATH:+:$PKG_CONFIG_PATH}" segment) so the resulting export
PKG_CONFIG_PATH assignment still produces the same final string but each source
line stays <=100 chars.
🧹 Nitpick comments (1)
home-manager/programs/fish/default.nix (1)
14-19: OpenSSL exports look correct, minor inconsistency with bash version.The fish implementation is functionally correct. However, there's a subtle difference from the bash version:
- Bash (line 40): Uses
${PKG_CONFIG_PATH:+:$PKG_CONFIG_PATH}which only appends if the variable is non-empty- Fish (line 16): Always appends
$PKG_CONFIG_PATH, which may be empty/unsetIn practice, fish handles empty variables gracefully in
set -gx, so this works fine. But for strict consistency, you could use:Optional: Match bash behavior for empty PKG_CONFIG_PATH
- set -gx PKG_CONFIG_PATH "${pkgs.openssl.dev}/lib/pkgconfig" $PKG_CONFIG_PATH + if set -q PKG_CONFIG_PATH + set -gx PKG_CONFIG_PATH "${pkgs.openssl.dev}/lib/pkgconfig" $PKG_CONFIG_PATH + else + set -gx PKG_CONFIG_PATH "${pkgs.openssl.dev}/lib/pkgconfig" + end
|
|
||
| # OpenSSL for cargo builds (rust crates like openssl-sys) | ||
| 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" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the file to see the full context
head -n 50 home-manager/programs/bash/default.nixRepository: shunkakinoki/dotfiles
Length of output: 1563
🏁 Script executed:
# Also check if the file exists and its size
wc -l home-manager/programs/bash/default.nixRepository: shunkakinoki/dotfiles
Length of output: 106
🏁 Script executed:
# Let's examine the exact lines mentioned (38-43) with more context
sed -n '30,50p' home-manager/programs/bash/default.nixRepository: shunkakinoki/dotfiles
Length of output: 858
🏁 Script executed:
# Check the module structure - look at the beginning of the file
head -n 10 home-manager/programs/bash/default.nixRepository: shunkakinoki/dotfiles
Length of output: 245
🏁 Script executed:
# Check the exact length of line 40
sed -n '40p' home-manager/programs/bash/default.nix | wc -cRepository: shunkakinoki/dotfiles
Length of output: 68
🏁 Script executed:
# Check if openssl is declared as a dependency in this file
grep -n "openssl" home-manager/programs/bash/default.nixRepository: shunkakinoki/dotfiles
Length of output: 432
🏁 Script executed:
# Check if there are home.packages or similar dependencies declared
grep -n "home\." home-manager/programs/bash/default.nix | head -20Repository: shunkakinoki/dotfiles
Length of output: 47
🏁 Script executed:
# Let's look at the entire file to see if there are any package dependencies
cat home-manager/programs/bash/default.nixRepository: shunkakinoki/dotfiles
Length of output: 2302
🏁 Script executed:
# Check if there's a flake.nix that defines nixpkgs inputs
if [ -f "flake.nix" ]; then
head -30 flake.nix | grep -A5 "nixpkgs"
fiRepository: shunkakinoki/dotfiles
Length of output: 756
Line 40 exceeds the 100-character limit—refactor the PKG_CONFIG_PATH export.
The OpenSSL environment variable setup is technically sound: Nix string interpolation is correct, bash variable escaping with ''$ is proper, and the .dev/.out package split follows Nix conventions. The comment adequately documents the purpose. However, line 40 is 108 characters, violating the guideline of keeping lines under 100 characters in Nix files. Split this line for compliance with the style requirements.
🤖 Prompt for AI Agents
In `@home-manager/programs/bash/default.nix` around lines 38 - 43, The
PKG_CONFIG_PATH export line is over 100 chars; split it into multiple
concatenated strings to keep lines under the limit while preserving the same
value. Update the export statement that sets PKG_CONFIG_PATH (the line
referencing pkgs.openssl.dev and ${PKG_CONFIG_PATH:+:$PKG_CONFIG_PATH}) by
breaking the string interpolation into two or more quoted pieces (e.g., separate
the "${pkgs.openssl.dev}/lib/pkgconfig" segment from the
"${PKG_CONFIG_PATH:+:$PKG_CONFIG_PATH}" segment) so the resulting export
PKG_CONFIG_PATH assignment still produces the same final string but each source
line stays <=100 chars.
There was a problem hiding this comment.
Pull request overview
This PR adds OpenSSL environment variables to bash and fish shell configurations on Linux systems to enable cargo builds for Rust crates with OpenSSL dependencies without requiring explicit nix-shell invocations.
Changes:
- Added four OpenSSL-related environment variables (PKG_CONFIG_PATH, OPENSSL_DIR, OPENSSL_LIB_DIR, OPENSSL_INCLUDE_DIR) to bash and fish shells
- Variables are conditionally set only on Linux systems where OpenSSL packages are available
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| home-manager/programs/bash/default.nix | Added OpenSSL environment variables within the existing Linux-specific conditional block in bashrcExtra |
| home-manager/programs/fish/default.nix | Added OpenSSL environment variables within the existing Linux-specific conditional block in shellInit |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # OpenSSL for cargo builds (rust crates like openssl-sys) | ||
| 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" |
There was a problem hiding this comment.
The zsh configuration (home-manager/programs/zsh/default.nix) also has the same Linux-specific XDG_RUNTIME_DIR setup pattern but is missing the OpenSSL environment variables that are being added to bash and fish. For consistency, zsh should also receive the same OpenSSL environment variable configuration since it's an active shell in this repository.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Summary
Set OpenSSL environment variables in bash and fish shells on Linux so cargo can find OpenSSL without needing
nix-shell -p gcc pkg-config openssl.Environment variables added:
PKG_CONFIG_PATH- points to openssl.dev pkgconfigOPENSSL_DIR- openssl dev directoryOPENSSL_LIB_DIR- openssl library directoryOPENSSL_INCLUDE_DIR- openssl headersProblem
Cargo builds for crates depending on
openssl-sysfail with:Test plan
make buildpassescargo build --releaseworks for projects with openssl-sys dependency🤖 Generated with Claude Code
Summary by cubic
Set OpenSSL env vars in bash and fish on Linux so cargo can find OpenSSL during builds. Fixes openssl-sys build failures and removes the need to enter nix-shell.
Written for commit 94ed1ec. Summary will update on new commits.