Skip to content

fix: Fix build script bugs #137

Merged
chipkent merged 4 commits into
mainfrom
fix_install_2
Jan 8, 2026
Merged

fix: Fix build script bugs #137
chipkent merged 4 commits into
mainfrom
fix_install_2

Conversation

@chipkent

@chipkent chipkent commented Jan 8, 2026

Copy link
Copy Markdown
Collaborator

Add convenience script for quick setup and improve installation process (#137)

Add build_and_run.sh convenience script for one-command environment setup and server startup, update README with quick start instructions, fix virtual environment path naming to use hyphens instead of equals signs, and reorder package installation to install deephaven-server before ibapi to improve dependency resolution.

… process (#137)

Add `build_and_run.sh` convenience script for one-command environment setup and server startup, update README with quick start instructions, fix virtual environment path naming to use hyphens instead of equals signs, and reorder package installation to install deephaven-server before ibapi to improve dependency resolution.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes several build script issues to improve the installation process. The changes include fixing virtual environment path naming to use hyphens instead of equals signs for better shell compatibility, reordering package installation to install deephaven-server before ibapi for improved dependency resolution, adding a convenience script for quick one-command setup, and correcting a pip command in the README documentation.

Key changes:

  • Fixed venv path naming from venv-release-dhib={version} to venv-release-dhib-{version} for better shell compatibility
  • Reordered package installation to install deephaven-server before building/installing ibapi
  • Added build_and_run.sh convenience script for automated environment setup and server startup

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
dhib_env.py Fixed virtual environment path naming to use hyphens instead of equals signs, and reordered package installation to install deephaven-server before ibapi in both dev() and release() functions
build_and_run.sh New convenience script that automates the process of creating a temporary installer venv, building the release environment, and starting the Deephaven server
README.md Added quick start documentation for the new convenience script, and fixed incorrect pip upgrade command from pip --upgrade pip to python -m pip install --upgrade pip

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread build_and_run.sh Outdated
Comment thread build_and_run.sh Outdated
Comment thread build_and_run.sh

# Create temporary installer virtual environment
# This small venv is only used to run the dhib_env.py script
python3.12 -m venv .venv-installer

Copilot AI Jan 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The script hardcodes python3.12 but doesn't verify that this version is available on the system. Consider adding a check to verify python3.12 exists, or use a more flexible approach like python3 with a version check, consistent with the README documentation which mentions python3.

Copilot uses AI. Check for mistakes.
Comment thread build_and_run.sh Outdated
Comment on lines +1 to +30
#!/bin/bash

# Convenience script to build a release virtual environment and start the Deephaven server.
# This script automates the process of:
# 1. Creating a temporary installer virtual environment
# 2. Building the release virtual environment with deephaven-ib, deephaven-server, and ibapi
# 3. Starting the Deephaven server
#
# Press Ctrl-C to stop the server when done.

# Display Java home (required for Deephaven)
echo $JAVA_HOME

# Clean up any existing virtual environments
deactivate 2>/dev/null || true # Deactivate if already in a venv
rm -rf .venv-installer
rm -rf venv-release-dhib*

# Create temporary installer virtual environment
# This small venv is only used to run the dhib_env.py script
python3.12 -m venv .venv-installer
source .venv-installer/bin/activate

# Install dependencies needed to run dhib_env.py
python -m pip install --upgrade pip
pip install -r requirements_dhib_env.txt

# Build the release virtual environment
# This creates venv-release-dhib-<version> with all required packages
python ./dhib_env.py release

Copilot AI Jan 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The script lacks error handling and will continue executing even if critical steps fail (e.g., if creating the virtual environment fails, or if the dhib_env.py script fails). Consider adding set -e at the beginning of the script to exit on any command failure, or add explicit error checking after critical commands.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI commented Jan 8, 2026

Copy link
Copy Markdown
Contributor

@chipkent I've opened a new pull request, #138, to work on those changes. Once the pull request is ready, I'll request review from you.

- [x] Add `set -e` to the script to exit on any command failure
- [x] Add `set -u` to catch undefined variables
- [x] Add `set -o pipefail` to catch errors in piped commands
- [ ] Run code review
- [ ] Reply to the comment with the commit hash

<!-- START COPILOT CODING AGENT TIPS -->
---

💡 You can make Copilot smarter by setting up custom instructions,
customizing its development environment and configuring Model Context
Protocol (MCP) servers. Learn more [Copilot coding agent
tips](https://gh.io/copilot-coding-agent-tips) in the docs.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: chipkent <5250374+chipkent@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread build_and_run.sh Outdated
Comment thread build_and_run.sh
rm -rf .venv-installer

# Activate the release virtual environment and start Deephaven server
source ./venv-release-dhib*/bin/activate

Copilot AI Jan 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a wildcard pattern in the source command could fail if multiple matching directories exist or if no matching directory exists. Consider capturing the venv path from the dhib_env.py output or failing explicitly if the expected venv directory doesn't exist. For example, you could check for the existence of the directory first or use a more robust path resolution.

Suggested change
source ./venv-release-dhib*/bin/activate
venv_activate_candidates=( ./venv-release-dhib*/bin/activate )
# Handle cases where the glob does not match or matches multiple environments
if [ "${#venv_activate_candidates[@]}" -eq 1 ] && [ "${venv_activate_candidates[0]}" = "./venv-release-dhib*/bin/activate" ]; then
echo "Error: No release virtual environment found matching ./venv-release-dhib*/bin/activate" >&2
exit 1
fi
if [ "${#venv_activate_candidates[@]}" -eq 0 ]; then
echo "Error: No release virtual environment found matching ./venv-release-dhib*/bin/activate" >&2
exit 1
elif [ "${#venv_activate_candidates[@]}" -gt 1 ]; then
echo "Error: Multiple release virtual environments found. Please remove duplicates:" >&2
printf ' %s\n' "${venv_activate_candidates[@]}" >&2
exit 1
fi
if [ ! -f "${venv_activate_candidates[0]}" ]; then
echo "Error: Expected activate script not found at ${venv_activate_candidates[0]}" >&2
exit 1
fi
source "${venv_activate_candidates[0]}"

Copilot uses AI. Check for mistakes.
Comment thread build_and_run.sh
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@chipkent chipkent merged commit cea0763 into main Jan 8, 2026
9 checks passed
@chipkent chipkent deleted the fix_install_2 branch January 8, 2026 06:00
@github-actions github-actions Bot locked and limited conversation to collaborators Jan 8, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants