Skip to content

statically link vcruntime140.dll in windows builds#495

Closed
acunniffe wants to merge 4 commits intomainfrom
fix/windows-static-link-cpp-runtime
Closed

statically link vcruntime140.dll in windows builds#495
acunniffe wants to merge 4 commits intomainfrom
fix/windows-static-link-cpp-runtime

Conversation

@acunniffe
Copy link
Copy Markdown
Collaborator

@acunniffe acunniffe commented Feb 9, 2026

Installs would silently fail for fresh windows machines without vcruntime

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

View 4 additional findings in Devin Review.

Open in Devin Review

Comment thread install.ps1 Outdated
Comment on lines +407 to +417
# Verify the binary actually runs (catches missing vcredist / DLL issues)
Write-Host 'Verifying binary...'
try {
$verOutput = & $finalExe version 2>&1
if ($LASTEXITCODE -ne 0) {
Write-ErrorAndExit "git-ai binary failed to run (exit code $LASTEXITCODE). Output: $verOutput`nThis may indicate missing system libraries. Please report this at https://github.com/git-ai-project/git-ai/issues."
}
Write-Success "Binary verified: $verOutput"
} catch {
Write-ErrorAndExit "git-ai binary failed to execute: $($_.Exception.Message)`nThis may indicate missing system libraries (e.g. Visual C++ Redistributable). Please report this at https://github.com/git-ai-project/git-ai/issues."
}
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.

🟡 Binary verification runs after exchange-nonce already executes the binary

The new binary verification step (lines 407-417) is designed to catch missing DLL/vcredist issues and provide a clear, actionable error message. However, it is placed after the exchange-nonce call at install.ps1:396-405, which already executes $finalExe. When $env:INSTALL_NONCE and $env:API_BASE are set, a DLL failure will first be caught by the exchange-nonce try/catch, silently setting $needLogin = $true and masking the real cause.

Detailed Explanation

When the binary has a missing DLL issue and the nonce env vars are set:

  1. Line 398 (& $finalExe exchange-nonce) fails with a DLL load error
  2. The catch block at line 402-404 silently sets $needLogin = $true, treating it as a login issue
  3. The verification at line 410 then also fails and calls Write-ErrorAndExit with the correct message

The verification step still catches the problem, so the user does get the right error eventually. However, the intent of the verification is to be the first execution of the binary to provide immediate, clear diagnostics. Moving the verification block before the exchange-nonce block would ensure the user always gets the clear DLL-related error message first, without any confusing intermediate behavior.

Impact: When INSTALL_NONCE and API_BASE are set and the binary has DLL issues, the user sees the correct error but only after an unnecessary failed nonce exchange attempt.

Prompt for agents
Move the binary verification block (lines 407-417 in install.ps1) to before the exchange-nonce block (currently lines 394-405). The verification should be the first thing that executes the binary after installation, so that missing DLL issues are caught with a clear error message before any other binary invocations. Specifically, move the block starting with '# Verify the binary actually runs' to just after line 393 (after the git-og shim creation), and before the '# Login user with install token if provided' comment.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@acunniffe acunniffe closed this Feb 9, 2026
@svarlamov svarlamov deleted the fix/windows-static-link-cpp-runtime branch April 4, 2026 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant