Skip to content

Conversation

@KraHsu
Copy link

@KraHsu KraHsu commented Nov 14, 2025

Description

This PR fixes an issue in the Windows batch script responsible for detecting the torch version.
Inside :ensure_cuda_torch, the for /f ('...') construct wraps the inner command with an additional layer, resulting in something like:

for /f "tokens=2" %%V in ('"!python_exe!" -m pip show torch ^| findstr /B /C:"Version:"') do set "TORCH_CUR=%%V"
==>
cmd /c ""root_path\_isaac_sim\python.bat" -m pip show zipp | find "Version:""

This triggers the quote-handling behavior of cmd /c, causing the outer pair of quotes to be stripped. As a result, the actual inner command becomes something like:

root_path\_isaac_sim\python.bat" -m pip show zipp | find "Version:"

This leads to the error: The filename, directory name, or volume label syntax is incorrect.

Considering that this fix is extremely small and straightforward, I did not open a separate issue for it (please let me know if this aligns with the contribution guidelines). Also, because the change is minimal and the bug itself is not severe, I did not add my name to CONTRIBUTOR.md.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Screenshots

Before the fix (my Windows system is in Chinese, so the error message is localized):
Debug - findstr with utf-8_1
After the fix:
Debug - findstr with utf-8_2

Checklist

  • I have read and understood the contribution guidelines
  • I have run the pre-commit checks with ./isaaclab.sh --format
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the changelog and the corresponding version in the extension's config/extension.toml file
  • I have added my name to the CONTRIBUTORS.md or my name already exists there

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Nov 14, 2025

Greptile Overview

Greptile Summary

Fixed CMD quote-handling bug in Windows batch script that prevented PyTorch version detection from working correctly.

  • Removed unnecessary quotes around !python_exe! variable in for /f loop on line 56
  • When for /f executes a command in parentheses with quotes around variables, CMD's /c subprocess strips the outer quotes incorrectly, breaking the command syntax
  • The fix aligns line 56 with the consistent pattern used elsewhere in the file (lines 25, 76, 81)
  • This is a minimal, surgical fix that resolves a Windows-specific error without affecting functionality

Confidence Score: 5/5

  • This PR is safe to merge with no risk
  • The fix is a single-character change (removing quotes) that corrects a well-documented CMD.exe quote-handling bug, aligns with existing patterns in the codebase, and has been tested by the author with before/after screenshots
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
isaaclab.bat 5/5 Removed unnecessary quotes around !python_exe! variable in for /f loop to fix CMD quote-handling issue that caused syntax errors

Sequence Diagram

sequenceDiagram
    participant User
    participant isaaclab.bat
    participant ensure_cuda_torch
    participant CMD
    participant pip
    
    User->>isaaclab.bat: Run with -i flag
    isaaclab.bat->>ensure_cuda_torch: Call :ensure_cuda_torch
    ensure_cuda_torch->>pip: Check if torch installed
    alt torch already installed
        ensure_cuda_torch->>CMD: Execute for /f loop to get version
        Note over CMD: Before fix: quotes around !python_exe!<br/>caused CMD to strip outer quotes<br/>breaking the command
        CMD->>pip: Run pip show torch | findstr
        pip-->>CMD: Return version string
        CMD-->>ensure_cuda_torch: Set TORCH_CUR variable
        ensure_cuda_torch->>ensure_cuda_torch: Compare versions
        alt version mismatch
            ensure_cuda_torch->>pip: Uninstall old torch
            ensure_cuda_torch->>pip: Install correct torch version
        end
    else torch not installed
        ensure_cuda_torch->>pip: Install torch with CUDA
    end
    ensure_cuda_torch-->>isaaclab.bat: Return success
    isaaclab.bat->>User: Continue installation
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

@KraHsu
Copy link
Author

KraHsu commented Nov 15, 2025

Apologies, I just realized that the original use of double quotes "" might have been to avoid errors caused by spaces in the path. Based on my tests, after modifying my PR, it does indeed cause errors when there are spaces in the ISAACLAB_PATH.

However, other for /f statements in the isaaclab.bat script seem to use the version without quotes, so the issue might actually be in line with the “consistent behavior” across the script?

I suspect a better solution might be to clarify in the documentation not to place IsaacLab in a path with spaces (or make more significant changes to ensure there are no errors in such cases).

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

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

Labels

bug Something isn't working infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant