Skip to content

Commit 34d6675

Browse files
aknyshaknysh
authored
fix: Windows toolchain installation issues (#2012)
* windows fixes * windows fixes * updates for test * updates for test * windows fixes * windows fixes * windows fixes * updates for test * windows fixes * updates for test * windows fixes * windows fixes * windows fixes, address comments, add tests * address comments, add tests * address comments, add tests * address comments, add tests * address comments, add tests
1 parent 8e2cc18 commit 34d6675

File tree

22 files changed

+1526
-92
lines changed

22 files changed

+1526
-92
lines changed

‎docs/fixes/windows-atmos-d-and-toolchain-issues.md‎

Lines changed: 284 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,284 @@
1+
# Windows Toolchain Fixes
2+
3+
## Summary
4+
5+
This document describes Windows-specific issues reported by users and the fixes applied.
6+
7+
| Issue | Status | Description |
8+
|------------------------------------|--------------------|----------------------------------------------------------|
9+
| `.atmos.d` auto-import | ✅ Verified Working | Configuration loading works correctly on Windows |
10+
| Toolchain binary `.exe` extension | ✅ Fixed | Centralized function ensures `.exe` extension on Windows |
11+
| Download URL `.exe` handling | ✅ Fixed | GitHub release URLs get `.exe` for raw binaries |
12+
| Archive extraction `.exe` handling | ✅ Fixed | Files extracted correctly from archives |
13+
| PowerShell hint message | ✅ Fixed | Shows correct `Invoke-Expression` syntax |
14+
15+
---
16+
17+
## Issue #1: `.atmos.d` Auto-Import
18+
19+
### Status: ✅ Verified Working
20+
21+
After testing on Windows, the `.atmos.d` auto-import functionality works correctly. No code changes required.
22+
23+
### Improvements Made
24+
25+
Enhanced debug logging in `pkg/config/load.go`:
26+
27+
- Logs at **Debug** level when directories are found.
28+
- Users can diagnose issues with `ATMOS_LOGS_LEVEL=Debug`.
29+
30+
---
31+
32+
## Issue #2: Toolchain Installation Failures
33+
34+
### Reported Problems
35+
36+
1. Binary installed without `.exe` extension - causes `terraform --version` to hang.
37+
2. Download URL missing `.exe` - tools like jq fail with 404 on Windows (e.g., `jq-windows-amd64` vs `jq-windows-amd64.exe`).
38+
3. Archive extraction fails for tools like helm - looking for `windows-amd64/helm` instead of `windows-amd64/helm.exe`.
39+
4. Hint message shows Unix `eval` syntax instead of PowerShell `Invoke-Expression`.
40+
41+
### Architecture: Centralized Windows Extension Handling
42+
43+
Following [Aqua's Windows support approach](https://aquaproj.github.io/docs/reference/windows-support/), Windows executables need the `.exe` extension to be found by `os/exec.LookPath`. We use a centralized function for consistent handling.
44+
45+
**Key design decisions:**
46+
47+
- **Single utility function**: `EnsureWindowsExeExtension()` handles all Windows extension logic in one place.
48+
- **Tool type determines URL handling**:
49+
- `github_release` type: Automatically adds `.exe` to download URLs for raw binaries (non-archive assets) on Windows. This matches Aqua's behavior.
50+
- `http` type: No automatic `.exe` handling - the asset template must specify the complete URL including `.exe` if needed.
51+
- **Archive extraction**: Only attempts `.exe` fallback on Windows (not on Unix).
52+
53+
### Download URL Handling by Tool Type
54+
55+
| Tool Type | Download URL `.exe` Handling |
56+
|------------------|------------------------------------------------------------------------|
57+
| `github_release` | Automatic: adds `.exe` on Windows for raw binaries (no archive ext) |
58+
| `http` | Manual: asset template must include `.exe` in URL if needed |
59+
60+
**Example - `github_release` type (jq):**
61+
```text
62+
Asset template: jq-{{.OS}}-{{.Arch}}
63+
On Windows: https://github.com/jqlang/jq/releases/download/jq-1.7.1/jq-windows-amd64.exe ✅
64+
On Linux: https://github.com/jqlang/jq/releases/download/jq-1.7.1/jq-linux-amd64 ✅
65+
```
66+
67+
**Example - `http` type (must specify `.exe` in template):**
68+
```text
69+
Asset template: https://example.com/tool-{{.OS}}-{{.Arch}}{{if eq .OS "windows"}}.exe{{end}}
70+
```
71+
72+
### Fixes Applied
73+
74+
| File | Fix |
75+
|------------------------------------|--------------------------------------------------------------|
76+
| `toolchain/installer/installer.go` | Added `EnsureWindowsExeExtension()` centralized function |
77+
| `toolchain/installer/installer.go` | Uses centralized function for installed binary naming |
78+
| `toolchain/installer/asset.go` | Adds `.exe` to GitHub release URLs for raw binaries on Win |
79+
| `toolchain/installer/extract.go` | Uses centralized function; `.exe` fallback only on Windows |
80+
| `toolchain/install_helpers.go` | Platform-aware hint message for PowerShell |
81+
82+
### Centralized Function
83+
84+
```go
85+
// EnsureWindowsExeExtension appends .exe to the binary name on Windows if not present.
86+
// This follows Aqua's behavior where executables need the .exe extension on Windows
87+
// to be found by os/exec.LookPath.
88+
func EnsureWindowsExeExtension(binaryName string) string {
89+
defer perf.Track(nil, "installer.EnsureWindowsExeExtension")()
90+
91+
if runtime.GOOS == "windows" && !strings.HasSuffix(strings.ToLower(binaryName), ".exe") {
92+
return binaryName + ".exe"
93+
}
94+
return binaryName
95+
}
96+
```
97+
98+
### GitHub Release URL Builder (for raw binaries)
99+
100+
```go
101+
// In buildGitHubReleaseURL():
102+
// On Windows, add .exe to raw binary asset names that don't have an archive extension.
103+
// This follows Aqua's behavior where Windows binaries need .exe extension in the download URL.
104+
if !hasArchiveExtension(assetName) {
105+
assetName = EnsureWindowsExeExtension(assetName)
106+
}
107+
```
108+
109+
---
110+
111+
## Tests
112+
113+
### Unit Tests
114+
115+
Run toolchain installer tests:
116+
117+
```bash
118+
go test ./toolchain/installer/... -v
119+
```
120+
121+
### Integration Tests
122+
123+
Test file: `tests/toolchain_custom_commands_test.go`
124+
125+
Uses `testhelpers.NewAtmosRunner` for building and running atmos binary (shared infrastructure).
126+
127+
| Test | Description |
128+
|-------------------------------------------------------|----------------------------------------------------------------|
129+
| `TestToolchainCustomCommands_InstallAllTools` | Installs gum, k9s, helm, jq, tofu and verifies `.exe` binaries |
130+
| `TestToolchainCustomCommands_ToolsExecutable` | Verifies tools execute `--version` correctly |
131+
| `TestToolchainCustomCommands_PathEnvOutput` | Tests bash and PowerShell format output |
132+
| `TestToolchainCustomCommands_WindowsExeExtension` | Verifies `.exe` extension on Windows |
133+
| `TestToolchainCustomCommands_CustomCommandsLoaded` | Verifies custom commands appear in help |
134+
| `TestToolchainCustomCommands_ExecuteWithDependencies` | Tests custom commands with `dependencies.tools` |
135+
136+
**Run on macOS/Linux:**
137+
138+
```bash
139+
go test -v -run "TestToolchainCustomCommands" ./tests/... -timeout 10m
140+
```
141+
142+
**Run on Windows:**
143+
144+
```powershell
145+
go test -v -run "TestToolchainCustomCommands" .\tests\... -timeout 10m
146+
```
147+
148+
### Manual CLI Testing
149+
150+
**Test Fixture:** `tests/fixtures/scenarios/toolchain-custom-commands`
151+
152+
#### Windows (PowerShell)
153+
154+
```powershell
155+
# Navigate to test fixture
156+
cd tests\fixtures\scenarios\toolchain-custom-commands
157+
158+
# Build atmos
159+
cd ..\..\..\..
160+
go build -o atmos.exe .
161+
cd tests\fixtures\scenarios\toolchain-custom-commands
162+
163+
# Install tools
164+
..\..\..\..\atmos.exe toolchain install charmbracelet/gum@0.17.0
165+
166+
# Verify .exe extension in output
167+
# Expected: ✓ Installed charmbracelet/gum@0.17.0 to .tools\bin\...\gum.exe
168+
169+
# Verify PowerShell hint
170+
# Expected: 💡 Export the PATH ... using Invoke-Expression (atmos ... --format powershell)
171+
172+
# Set PATH and test
173+
Invoke-Expression (..\..\..\..\atmos.exe toolchain env --format powershell)
174+
gum --version
175+
176+
# Test custom command with dependencies
177+
..\..\..\..\atmos.exe test-gum
178+
```
179+
180+
#### macOS/Linux
181+
182+
```bash
183+
# Navigate to test fixture
184+
cd tests/fixtures/scenarios/toolchain-custom-commands
185+
186+
# Build atmos
187+
cd ../../../..
188+
go build -o atmos .
189+
cd tests/fixtures/scenarios/toolchain-custom-commands
190+
191+
# Install tools
192+
../../../../atmos toolchain install charmbracelet/gum@0.17.0
193+
194+
# Set PATH and test
195+
eval "$(../../../../atmos toolchain env)"
196+
gum --version
197+
198+
# Test custom command with dependencies
199+
../../../../atmos test-gum
200+
```
201+
202+
---
203+
204+
## Test Results (Windows)
205+
206+
All tests pass on Windows.
207+
208+
### Installation Output
209+
210+
Tools are installed with `.exe` extension and display the PowerShell hint:
211+
212+
```text
213+
✓ Installed charmbracelet/gum@0.17.0 to .tools\bin\charmbracelet\gum\0.17.0\gum.exe (13mb)
214+
💡 Export the PATH environment variable for your toolchain tools using Invoke-Expression (atmos --chdir /path/to/project toolchain env --format powershell)
215+
216+
✓ Installed derailed/k9s@0.32.7 to .tools\bin\derailed\k9s\0.32.7\k9s.exe (97mb)
217+
218+
✓ Installed helm/helm@3.16.3 to .tools\bin\helm\helm\3.16.3\helm.exe (55mb)
219+
220+
✓ Installed jqlang/jq@1.7.1 to .tools\bin\jqlang\jq\1.7.1\jq.exe (962kb)
221+
222+
✓ Installed opentofu/opentofu@1.9.0 to .tools\bin\opentofu\opentofu\1.9.0\tofu.exe (83mb)
223+
```
224+
225+
### Tool Version Verification
226+
227+
After setting PATH with `Invoke-Expression (atmos toolchain env --format powershell)`:
228+
229+
```text
230+
> gum --version
231+
gum version v0.17.0 (6045525)
232+
233+
> jq --version
234+
jq-1.7.1
235+
236+
> helm version --short
237+
v3.16.3+gcfd0749
238+
239+
> tofu version
240+
OpenTofu v1.9.0
241+
on windows_amd64
242+
```
243+
244+
### Custom Command Execution with Dependencies
245+
246+
Running `atmos test-jq` automatically installs all dependencies and executes:
247+
248+
```text
249+
✓ Installed charmbracelet/gum@0.17.0
250+
✓ Installed derailed/k9s@0.32.7
251+
✓ Installed helm/helm@3.16.3
252+
✓ Installed jqlang/jq@1.7.1
253+
✓ Installed opentofu/opentofu@1.9.0
254+
255+
✓ Installed 5 tools
256+
```
257+
258+
### Integration Test Summary
259+
260+
```text
261+
--- PASS: TestToolchainCustomCommands_InstallAllTools (14.04s)
262+
--- PASS: Install_gum (0.89s) - .tools\bin\charmbracelet\gum\0.17.0\gum.exe
263+
--- PASS: Install_k9s (1.36s) - .tools\bin\derailed\k9s\0.32.7\k9s.exe
264+
--- PASS: Install_helm (1.42s) - .tools\bin\helm\helm\3.16.3\helm.exe
265+
--- PASS: Install_jq (1.06s) - .tools\bin\jqlang\jq\1.7.1\jq.exe
266+
--- PASS: Install_tofu (1.09s) - .tools\bin\opentofu\opentofu\1.9.0\tofu.exe
267+
--- PASS: TestToolchainCustomCommands_ToolsExecutable (12.33s)
268+
--- PASS: Execute_gum (0.96s)
269+
--- PASS: Execute_jq (0.87s)
270+
--- PASS: Execute_helm (1.54s)
271+
--- PASS: Execute_tofu (1.25s)
272+
--- PASS: TestToolchainCustomCommands_PathEnvOutput (10.09s)
273+
--- PASS: BashFormat (0.63s)
274+
--- PASS: PowershellFormat (0.63s)
275+
--- PASS: TestToolchainCustomCommands_WindowsExeExtension (8.91s)
276+
--- PASS: TestToolchainCustomCommands_CustomCommandsLoaded (8.31s)
277+
--- PASS: TestToolchainCustomCommands_ExecuteWithDependencies (14.50s)
278+
--- PASS: test-jq (4.40s)
279+
--- PASS: test-gum (0.59s)
280+
--- PASS: test-helm (0.78s)
281+
--- PASS: test-tofu (0.76s)
282+
PASS
283+
ok github.com/cloudposse/atmos/tests 68.332s
284+
```

‎pkg/config/load.go‎

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1039,19 +1039,31 @@ func loadAtmosDFromGitRoot(dirPath string, dst *viper.Viper) {
10391039
// and loads their configurations into the destination viper instance.
10401040
func loadAtmosDFromDirectory(dirPath string, dst *viper.Viper) {
10411041
// Search for `atmos.d/` configurations.
1042-
searchPattern := filepath.Join(filepath.FromSlash(dirPath), filepath.Join("atmos.d", "**", "*"))
1043-
if err := loadAtmosConfigsFromDirectory(searchPattern, dst, "atmos.d"); err != nil {
1044-
log.Trace("Failed to load atmos.d configs", "error", err, "path", dirPath)
1045-
// Don't return error - just log and continue.
1046-
// This maintains existing behavior where .atmos.d loading is optional.
1042+
atmosDPath := filepath.Join(filepath.FromSlash(dirPath), "atmos.d")
1043+
if stat, err := os.Stat(atmosDPath); err == nil && stat.IsDir() {
1044+
log.Debug("Found atmos.d directory, loading configurations", "path", atmosDPath)
1045+
searchPattern := filepath.Join(atmosDPath, "**", "*")
1046+
if err := loadAtmosConfigsFromDirectory(searchPattern, dst, "atmos.d"); err != nil {
1047+
log.Debug("Failed to load atmos.d configs", "error", err, "path", atmosDPath)
1048+
}
1049+
} else if err != nil && !os.IsNotExist(err) {
1050+
log.Debug("Failed to stat atmos.d directory", "path", atmosDPath, "error", err)
1051+
} else {
1052+
log.Trace("No atmos.d directory found", "path", atmosDPath)
10471053
}
10481054

10491055
// Search for `.atmos.d` configurations.
1050-
searchPattern = filepath.Join(filepath.FromSlash(dirPath), filepath.Join(".atmos.d", "**", "*"))
1051-
if err := loadAtmosConfigsFromDirectory(searchPattern, dst, ".atmos.d"); err != nil {
1052-
log.Trace("Failed to load .atmos.d configs", "error", err, "path", dirPath)
1053-
// Don't return error - just log and continue.
1054-
// This maintains existing behavior where .atmos.d loading is optional.
1056+
dotAtmosDPath := filepath.Join(filepath.FromSlash(dirPath), ".atmos.d")
1057+
if stat, err := os.Stat(dotAtmosDPath); err == nil && stat.IsDir() {
1058+
log.Debug("Found .atmos.d directory, loading configurations", "path", dotAtmosDPath)
1059+
searchPattern := filepath.Join(dotAtmosDPath, "**", "*")
1060+
if err := loadAtmosConfigsFromDirectory(searchPattern, dst, ".atmos.d"); err != nil {
1061+
log.Debug("Failed to load .atmos.d configs", "error", err, "path", dotAtmosDPath)
1062+
}
1063+
} else if err != nil && !os.IsNotExist(err) {
1064+
log.Debug("Failed to stat .atmos.d directory", "path", dotAtmosDPath, "error", err)
1065+
} else {
1066+
log.Trace("No .atmos.d directory found", "path", dotAtmosDPath)
10551067
}
10561068
}
10571069

0 commit comments

Comments
 (0)