Skip to content

Fix for rxd reaction compilation in windows. - #3735

Open
adamjhn wants to merge 1 commit into
masterfrom
rxd_windows_compile
Open

Fix for rxd reaction compilation in windows.#3735
adamjhn wants to merge 1 commit into
masterfrom
rxd_windows_compile

Conversation

@adamjhn

@adamjhn adamjhn commented Mar 26, 2026

Copy link
Copy Markdown
Member

I think anaconda modifies PATH so the compiler tried to use the wrong version of a library when building rxd reactions in windows.

I think anaconda modifies PATH so the compiler tried to use the wrong version of a library when building rxd reactions in windows.
@adamjhn
adamjhn requested a review from ceciliaromaro March 26, 2026 21:01
@adamjhn adamjhn added bug rxd reaction-diffusion labels Mar 26, 2026
@sonarqubecloud

Copy link
Copy Markdown

@codecov

codecov Bot commented Mar 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.31%. Comparing base (cf1fde0) to head (09557a7).

Files with missing lines Patch % Lines
share/lib/python/neuron/rxd/rxd.py 50.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3735      +/-   ##
==========================================
- Coverage   68.31%   68.31%   -0.01%     
==========================================
  Files         689      689              
  Lines      111034   111036       +2     
==========================================
+ Hits        75848    75849       +1     
- Misses      35186    35187       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

Copy link
Copy Markdown
Contributor

✔️ 09557a7 -> artifacts URL

@@ -536,12 +536,20 @@ def _cxx_compile(formula):
my_path = os.getenv("PATH")
os.putenv(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Gemini makes a reasonably compelling argument that we shouldn't change globals to achieve a temporary effect, and suggests this pattern instead:

import os
import subprocess
import copy

# 1. Create a copy of the current environment so you don't pollute the global one
new_env = os.environ.copy()

# 2. Update the copy
new_paths = [
    os.path.join(h.neuronhome(), "mingw", "mingw64", "bin"),
    os.path.join(h.neuronhome(), "bin"),
    new_env.get("PATH", "")
]
new_env["PATH"] = os.pathsep.join(new_paths)

# 3. Run the command using this specific environment
# This leaves os.environ untouched, so there is no need to "restore" anything
result = subprocess.run(["your_command"], env=new_env)
rflag = result.returncode

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug rxd reaction-diffusion

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants