always link

Author: pablopunk

internal/component/component.go

@@ -152,7 +152,7 @@ func (m *Manager) installComponent(comp profile.ComponentInfo, forceInstall bool
 	// Check if component needs install work
 	needsInstall := forceInstall || !m.stateManager.IsComponentInstalled(comp) || m.stateManager.HasInstallChanged(comp, comp.Component.Install)
 	hasLinks := len(comp.Component.Link) > 0
-	needsLinking := hasLinks && (forceInstall || !m.stateManager.IsComponentInstalled(comp) || m.stateManager.HasChangedSince(comp, comp.Component.Link))
+	// Links always need to be processed - they're fast and ensure files are properly linked
 
 	// Check if defaults need updating (only on macOS)
 	hasDefaults := len(comp.Component.Defaults) > 0 && system.IsMacOS()
@@ -176,8 +176,8 @@ func (m *Manager) installComponent(comp profile.ComponentInfo, forceInstall bool
 		}
 	}
 
-	// If no install needed and no linking needed and no defaults needed, mark as skipped
-	if !needsInstall && !needsLinking && !needsDefaults {
+	// If no install needed and no links and no defaults needed, mark as skipped
+	if !needsInstall && !hasLinks && !needsDefaults {
 		result.Skipped = true
 		return result
 	}
@@ -213,8 +213,8 @@ func (m *Manager) installComponent(comp profile.ComponentInfo, forceInstall bool
 		}
 	}
 
-	// Create links only if linking is needed
-	if needsLinking {
+	// Create links if component has any links (always run - it's fast and ensures proper linking)
+	if hasLinks {
 		linkResults, err := m.linkManager.CreateLinks(comp.Component.Link)
 		result.LinkResults = linkResults
 
@@ -292,7 +292,7 @@ func (m *Manager) installComponentWithProgress(comp profile.ComponentInfo, force
 	// Check if component needs install work
 	needsInstall := forceInstall || !m.stateManager.IsComponentInstalled(comp) || m.stateManager.HasInstallChanged(comp, comp.Component.Install)
 	hasLinks := len(comp.Component.Link) > 0
-	needsLinking := hasLinks && (forceInstall || !m.stateManager.IsComponentInstalled(comp) || m.stateManager.HasChangedSince(comp, comp.Component.Link))
+	// Links always need to be processed - they're fast and ensure files are properly linked
 
 	// Check if defaults need updating (only on macOS)
 	hasDefaults := len(comp.Component.Defaults) > 0 && system.IsMacOS()
@@ -316,8 +316,8 @@ func (m *Manager) installComponentWithProgress(comp profile.ComponentInfo, force
 		}
 	}
 
-	// If no install needed and no linking needed and no defaults needed, mark as skipped
-	if !needsInstall && !needsLinking && !needsDefaults {
+	// If no install needed and no links and no defaults needed, mark as skipped
+	if !needsInstall && !hasLinks && !needsDefaults {
 		result.Skipped = true
 		progress.CompleteSuccess()
 		return result
@@ -354,8 +354,8 @@ func (m *Manager) installComponentWithProgress(comp profile.ComponentInfo, force
 		}
 	}
 
-	// Create links only if linking is needed
-	if needsLinking {
+	// Create links if component has any links (always run - it's fast and ensures proper linking)
+	if hasLinks {
 		progress.StartLinking()
 		linkResults, err := m.linkManager.CreateLinks(comp.Component.Link)
 		result.LinkResults = linkResults

internal/component/component_test.go

@@ -215,10 +215,18 @@ func TestInstallComponentsForceInstall(t *testing.T) {
 		t.Fatalf("Second install result count = %v, want 1", len(results2))
 	}
 
-	// The component should be skipped because no changes detected
-	if !results2[0].Skipped {
-		t.Errorf("Second install should be skipped when no changes detected. Error: %v, LinkResults: %d", 
-			results2[0].Error, len(results2[0].LinkResults))
+	// The component should not be skipped because links always run (they're fast and ensure proper linking)
+	if results2[0].Skipped {
+		t.Errorf("Second install should not be skipped because links always need to run. Error: %v", results2[0].Error)
+	}
+	
+	// But it should have succeeded and run links
+	if results2[0].Error != nil {
+		t.Errorf("Second install should succeed but had error: %v", results2[0].Error)
+	}
+	
+	if len(results2[0].LinkResults) == 0 {
+		t.Errorf("Second install should have run links but LinkResults was empty")
 	}
 
 	// Install again with force - should not be skipped
@@ -263,6 +271,11 @@ func TestUninstallRemovedComponents(t *testing.T) {
 	testComponent := profile.ComponentInfo{
 		ProfileName:   "work",
 		ComponentName: "removed",
+		Component: config.Component{
+			Install: map[string]string{
+				"test": "echo 'this is a unique install command for removed component'",
+			},
+		},
 	}
 	manager.stateManager.MarkComponentInstalled(testComponent, "echo", "echo test", map[string]string{})
 
@@ -385,10 +398,18 @@ func TestInstallComponentWithProgress(t *testing.T) {
 		t.Errorf("Symlink should exist at %s: %v", targetPath, err)
 	}
 
-	// Test skipping with progress (install again without force)
+	// Test second install with progress (links should always run)
 	result2 := manager.installComponentWithProgress(comp, false, progressManager)
-	if !result2.Skipped {
-		t.Error("Second install should be skipped")
+	if result2.Skipped {
+		t.Error("Second install should not be skipped because links always run")
+	}
+	
+	if result2.Error != nil {
+		t.Errorf("Second install should succeed but had error: %v", result2.Error)
+	}
+	
+	if len(result2.LinkResults) == 0 {
+		t.Error("Second install should have run links but LinkResults was empty")
 	}
 
 	// Test force install with progress

internal/config/config.go

@@ -184,8 +184,9 @@ func (c *Component) MatchesOS(currentOS string) bool {
 	return false
 }
 
-// ContentHash generates a hash of the component's actual content (install, link, etc.)
-// excluding the component name/path. This allows detecting renames/moves.
+// ContentHash generates a hash of the component's core functionality
+// This focuses on install/uninstall commands and hooks, but excludes links and paths
+// to better detect renames where the component functionality is the same
 func (c *Component) ContentHash() string {
 	h := sha256.New()
 
@@ -213,18 +214,6 @@ func (c *Component) ContentHash() string {
 		}
 	}
 
-	// Sort and hash link commands
-	if len(c.Link) > 0 {
-		var linkKeys []string
-		for k := range c.Link {
-			linkKeys = append(linkKeys, k)
-		}
-		sort.Strings(linkKeys)
-		for _, k := range linkKeys {
-			h.Write([]byte(fmt.Sprintf("link:%s:%s;", k, c.Link[k])))
-		}
-	}
-	
 	// Hash post-install and post-link hooks
 	if c.PostInstall != "" {
 		h.Write([]byte(fmt.Sprintf("postinstall:%s;", c.PostInstall)))
@@ -255,5 +244,9 @@ func (c *Component) ContentHash() string {
 		}
 	}
 
+	// NOTE: We intentionally exclude Link mapping from the hash because
+	// component moves/renames often involve path changes in links, but
+	// the core functionality (install commands, hooks) remains the same
+	
 	return fmt.Sprintf("%x", h.Sum(nil))
 }

internal/state/state.go

@@ -269,21 +269,22 @@ func (m *Manager) GetLockFilePath() string {
 }
 
 // migrateRenamedComponent moves a component from old name to new name in the state
-// This handles the case where a component was renamed/moved but has identical content
+// This handles the case where a component was renamed/moved but has identical core functionality
 func (m *Manager) migrateRenamedComponent(oldKey string, newComponent profile.ComponentInfo, oldState ComponentState) {
 	newKey := newComponent.FullName()
 
-	// Create new state with updated names but preserve everything else
+	// Create new state preserving installation state but resetting link state
+	// This ensures that links will be re-created for the new component location
 	newState := ComponentState{
 		ProfileName:    newComponent.ProfileName,
 		ComponentName:  newComponent.ComponentName,
 		InstalledAt:    oldState.InstalledAt,
 		PackageManager: oldState.PackageManager,
 		InstallCommand: oldState.InstallCommand,
-		Links:          oldState.Links,
+		Links:          map[string]string{}, // Reset links so they get re-created
 		PostInstallRan: oldState.PostInstallRan,
-		PostLinkRan:    oldState.PostLinkRan,
-		ContentHash:    oldState.ContentHash, // Keep the same content hash
+		PostLinkRan:    false, // Reset post-link state since links will be re-created
+		ContentHash:    newComponent.Component.ContentHash(),
 	}
 
 	// Remove old entry and add new one