Conversation
Co-authored-by: volpatto <5941226+volpatto@users.noreply.github.com>
Co-authored-by: volpatto <5941226+volpatto@users.noreply.github.com>
|
Oh, it was a good experiment. So this is how good Copilot can be! Fair enough. |
Co-authored-by: volpatto <5941226+volpatto@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates the OS provisioning scripts to avoid reinstalling Miniconda/Miniforge when an existing conda installation is already present, while still applying conda configuration/init steps.
Changes:
- Add “existing conda” detection on Ubuntu/macOS/Windows to skip the download/install step when possible.
- Add installation of additional developer tools (e.g., flex/bison) and introduce pixi installation steps.
- Add installation of a broad scientific/HPC library set on Ubuntu and macOS.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 11 comments.
| File | Description |
|---|---|
scripts/provision-ubuntu.sh |
Detect existing conda before installing Miniconda; adds HPC libraries and user-level pixi install + verification/summary updates. |
scripts/provision-macos.sh |
Detect existing conda (including PATH) before installing Miniforge; adds pixi + HPC libraries and updates final tool summary. |
scripts/provision-win.ps1 |
Detect existing conda before installing Miniconda; adds winflexbison and installs pixi via remote script. |
Comments suppressed due to low confidence (1)
scripts/provision-win.ps1:230
- The existing-conda short-circuit ignores
-ForceReinstallMiniconda: if any conda is found on disk or PATH, the script will skip installation even when the user explicitly requested a reinstall at$MinicondaInstallDir. Consider bypassing/limiting the existing-conde check whenForceReinstallMinicondais true, and update the surrounding status messages so the output doesn’t say “Installing/installed Miniconda” when an existing conda was reused.
if ($existingConda) {
Write-Host "Conda already found at $existingConda — skipping Miniconda download/install."
} elseif ($MinicondaUseDirectInstaller) {
if ($MinicondaInstallDir -match '\s') {
throw "MinicondaInstallDir contains spaces. NSIS '/D=' cannot be quoted reliably. Use a path without spaces (e.g., C:\tools\miniconda3)."
}
Install-Miniconda-Direct -InstallDir $MinicondaInstallDir -ForceReinstall:$ForceReinstallMiniconda
} else {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| brew_install gcc # includes gfortran | ||
| brew_install openblas | ||
| brew_install open-mpi | ||
| brew_install fftw # includes MPI support when open-mpi is present | ||
| brew_install hwloc | ||
| brew_install hdf5 | ||
| brew_install mumps | ||
| brew_install metis | ||
| brew_install netcdf | ||
| brew_install parallel-netcdf | ||
| brew_install scotch # includes ptscotch | ||
| brew_install scalapack | ||
| brew_install suite-sparse | ||
| brew_install superlu | ||
| brew_install superlu_dist |
There was a problem hiding this comment.
Installing the full scientific/HPC stack unconditionally (gcc, MPI, HDF5, mumps, scotch, etc.) will substantially increase runtime/disk usage and can introduce long build times (e.g., gcc/mumps). Consider making this optional (flag/profile) or grouping into a separate opt-in section to reduce operational impact for typical dev setups.
| log "Installing scientific/HPC libraries..." | ||
| DEBIAN_FRONTEND=noninteractive apt-get install -y \ | ||
| libopenblas-dev libopenmpi-dev \ | ||
| libfftw3-dev libfftw3-mpi-dev \ | ||
| libhwloc-dev libhdf5-mpi-dev \ | ||
| libmumps-ptscotch-dev libmetis-dev \ | ||
| libnetcdf-dev libpnetcdf-dev \ | ||
| libptscotch-dev libscalapack-openmpi-dev \ | ||
| libsuitesparse-dev libsuperlu-dev libsuperlu-dist-dev |
There was a problem hiding this comment.
Installing the full scientific/HPC dependency set by default can significantly increase provisioning time and may pull in large transitive dependencies. Consider making this optional (e.g., --install-hpc-libs) or only enabling it for a dedicated profile to reduce operational impact.
| log "Installing scientific/HPC libraries..." | |
| DEBIAN_FRONTEND=noninteractive apt-get install -y \ | |
| libopenblas-dev libopenmpi-dev \ | |
| libfftw3-dev libfftw3-mpi-dev \ | |
| libhwloc-dev libhdf5-mpi-dev \ | |
| libmumps-ptscotch-dev libmetis-dev \ | |
| libnetcdf-dev libpnetcdf-dev \ | |
| libptscotch-dev libscalapack-openmpi-dev \ | |
| libsuitesparse-dev libsuperlu-dev libsuperlu-dist-dev | |
| if [ "${INSTALL_HPC_LIBS:-true}" = "true" ]; then | |
| log "Installing scientific/HPC libraries (INSTALL_HPC_LIBS=${INSTALL_HPC_LIBS:-true})..." | |
| DEBIAN_FRONTEND=noninteractive apt-get install -y \ | |
| libopenblas-dev libopenmpi-dev \ | |
| libfftw3-dev libfftw3-mpi-dev \ | |
| libhwloc-dev libhdf5-mpi-dev \ | |
| libmumps-ptscotch-dev libmetis-dev \ | |
| libnetcdf-dev libpnetcdf-dev \ | |
| libptscotch-dev libscalapack-openmpi-dev \ | |
| libsuitesparse-dev libsuperlu-dev libsuperlu-dist-dev | |
| else | |
| log "Skipping scientific/HPC libraries (INSTALL_HPC_LIBS=${INSTALL_HPC_LIBS:-false})." | |
| fi |
| # ---------- 3a) Scientific / HPC libraries ---------- | ||
| log "Installing scientific/HPC libraries..." | ||
| brew_install gcc # includes gfortran | ||
| brew_install openblas | ||
| brew_install open-mpi | ||
| brew_install fftw # includes MPI support when open-mpi is present | ||
| brew_install hwloc | ||
| brew_install hdf5 |
There was a problem hiding this comment.
This PR adds installation of a large set of scientific/HPC packages on macOS (OpenBLAS/OpenMPI/HDF5/MUMPS/etc.) unconditionally. This is a significant behavior change and isn’t mentioned in the PR description/title; if intentional, please document it there and/or gate it behind a flag/profile so the default provisioning remains lightweight.
| # -------- Scientific / HPC libraries -------- | ||
| log "Installing scientific/HPC libraries..." | ||
| DEBIAN_FRONTEND=noninteractive apt-get install -y \ | ||
| libopenblas-dev libopenmpi-dev \ | ||
| libfftw3-dev libfftw3-mpi-dev \ |
There was a problem hiding this comment.
This PR introduces installation of many scientific/HPC libraries (OpenBLAS/OpenMPI/HDF5/MUMPS/etc.) on Ubuntu unconditionally, which is a substantial behavior change beyond the stated goal of skipping conda installs. If this is intended, please update the PR description and consider gating it behind a flag/profile to avoid unexpectedly long apt installs for minimal setups.
| echo "• Miniconda for $TARGET_USER: $TARGET_HOME/miniconda3" | ||
| echo "• conda-forge enabled (strict), conda init done for bash, zsh." | ||
| echo "• pixi installed for $TARGET_USER: $TARGET_HOME/.pixi (open a NEW shell or add ~/.pixi/bin to PATH)." |
There was a problem hiding this comment.
The summary always prints Miniconda for $TARGET_USER: $TARGET_HOME/miniconda3, but the script may now reuse an existing conda from ~/miniforge3, ~/anaconda3, or /opt/conda. Consider tracking the actual conda_bin/prefix used and reporting that to avoid misleading output.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
… download Co-authored-by: volpatto <5941226+volpatto@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. 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:
|
…release download Co-authored-by: volpatto <5941226+volpatto@users.noreply.github.com>
Improve provisioning scripts by skipping conda/Miniconda/Miniforge download and install when an existing conda installation is already detected, and add
pixito all three platform provisioning scripts with supply-chain-safe installation.Changes Made
Conda detection (all platforms)
provision-ubuntu.sh):install_miniconda_user()scans~/miniconda3,~/miniforge3,~/anaconda3, and/opt/condafor an existingcondabinary before downloading; resolvesconda_binvia${existing_conda:-$prefix/bin/conda}.provision-macos.sh): Miniforge block scans five common macOS paths plustype -P conda; falls back to$MINIFORGE_PREFIX/bin/condaonly when no existing installation is found.provision-win.ps1): Checks eight commoncondabin\conda.batcandidate paths plusGet-Command condabefore invoking the installer; reuses the discovered path as$condaBatfor all subsequent config/init calls.In all cases,
conda config(channel priority) andconda init(shell integration) still run against the discovered binary.pixi installation (all platforms)
All pixi installers download the binary directly from GitHub Releases and verify its SHA-256 checksum against the published
SHA256SUMSfile before installation, eliminating the supply-chain risk of executing a remote script.provision-ubuntu.sh):install_pixi_user()function downloads the versionedpixi-{arch}.tar.gz(supports x86_64 and aarch64), verifies SHA-256, and installs to~/.pixi/bin. Accepts a--pixi-version=X.Y.Zflag; defaults to the latest GitHub release when omitted.provision-macos.sh): Installs pixi via Homebrew (brew_install pixi).provision-win.ps1):Install-Pixifunction downloadspixi-x86_64-pc-windows-msvc.zip, verifies SHA-256, and extracts to~\.pixi\bin. Accepts a-PixiVersionparameter for pinning (e.g.-PixiVersion 0.44.0); defaults to the latest GitHub release when omitted.🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.