Skip to content

Commit 73c5fef

Browse files
Copilotaooohan
andauthored
fix: storage.sdkPath configuration being ignored (#601)
* Initial plan * Fix storage.sdkPath configuration not being applied - Add ApplyStoragePath method to PathMeta to update Installs path from config - Update NewSdkManager to apply storage.sdkPath after loading config - Add unit tests for ApplyStoragePath functionality - Fixes issue where storage.sdkPath in config.yaml was ignored Co-authored-by: aooohan <40265686+aooohan@users.noreply.github.com> * Address code review: improve test to use dynamic path suffix - Import filepath in path_meta_test.go - Update test to derive expected suffix from NewPathMeta instead of hardcoding - Makes test more robust to changes in installedDirPrefix constant Co-authored-by: aooohan <40265686+aooohan@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: aooohan <40265686+aooohan@users.noreply.github.com>
1 parent 0d9a4cb commit 73c5fef

3 files changed

Lines changed: 71 additions & 0 deletions

File tree

internal/manager.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -781,6 +781,13 @@ func NewSdkManager() (*Manager, error) {
781781
return nil, fmt.Errorf("load config failed: %w", err)
782782
}
783783

784+
// Apply storage.sdkPath from config to PathMeta
785+
if c.Storage != nil && c.Storage.SdkPath != "" {
786+
if err := meta.ApplyStoragePath(c.Storage.SdkPath); err != nil {
787+
return nil, fmt.Errorf("failed to apply storage path: %w", err)
788+
}
789+
}
790+
784791
return &Manager{
785792
RuntimeEnvContext: &env.RuntimeEnvContext{
786793
UserConfig: c,

internal/pathmeta/path_meta.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,3 +171,21 @@ func getExecutablePath() string {
171171
}
172172
return exePath
173173
}
174+
175+
// ApplyStoragePath applies the storage path from config to update the Installs path
176+
// If storagePath is empty, the current Installs path is unchanged
177+
func (p *PathMeta) ApplyStoragePath(storagePath string) error {
178+
if storagePath == "" {
179+
return nil
180+
}
181+
182+
// Update the Installs path to use the configured storage path
183+
p.Shared.Installs = filepath.Join(storagePath, installedDirPrefix)
184+
185+
// Ensure the directory exists
186+
if err := os.MkdirAll(p.Shared.Installs, ReadWriteAuth); err != nil {
187+
return fmt.Errorf("failed to create storage directory: %w", err)
188+
}
189+
190+
return nil
191+
}

internal/pathmeta/path_meta_test.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
package pathmeta
2020

2121
import (
22+
"path/filepath"
2223
"runtime"
2324
"testing"
2425
)
@@ -155,3 +156,48 @@ func TestContainsPathSegment(t *testing.T) {
155156
})
156157
}
157158
}
159+
160+
func TestApplyStoragePath(t *testing.T) {
161+
// Create a temporary directory for testing
162+
tmpDir := t.TempDir()
163+
164+
// Create a PathMeta instance with default paths
165+
meta := &PathMeta{
166+
Shared: SharedPaths{
167+
Root: tmpDir,
168+
Installs: tmpDir + "/default/cache",
169+
},
170+
}
171+
172+
t.Run("Empty storage path does not change Installs", func(t *testing.T) {
173+
originalInstalls := meta.Shared.Installs
174+
err := meta.ApplyStoragePath("")
175+
if err != nil {
176+
t.Errorf("ApplyStoragePath with empty path should not error: %v", err)
177+
}
178+
if meta.Shared.Installs != originalInstalls {
179+
t.Errorf("Installs path changed when it shouldn't: got %q, want %q", meta.Shared.Installs, originalInstalls)
180+
}
181+
})
182+
183+
t.Run("Valid storage path updates Installs", func(t *testing.T) {
184+
customPath := tmpDir + "/custom"
185+
186+
// Create a reference PathMeta to get the expected suffix
187+
refMeta, err := NewPathMeta(tmpDir, customPath, tmpDir, 1)
188+
if err != nil {
189+
t.Fatalf("Failed to create reference PathMeta: %v", err)
190+
}
191+
expectedSuffix := filepath.Base(refMeta.Shared.Installs)
192+
193+
// Now test ApplyStoragePath
194+
err = meta.ApplyStoragePath(customPath)
195+
if err != nil {
196+
t.Errorf("ApplyStoragePath with valid path should not error: %v", err)
197+
}
198+
expectedPath := filepath.Join(customPath, expectedSuffix)
199+
if meta.Shared.Installs != expectedPath {
200+
t.Errorf("Installs path not updated correctly: got %q, want %q", meta.Shared.Installs, expectedPath)
201+
}
202+
})
203+
}

0 commit comments

Comments
 (0)