diff --git a/cmd/root_test.go b/cmd/root_test.go index e76043e..ffc8286 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -4,6 +4,7 @@ import ( "bytes" "os" "path/filepath" + "strings" "testing" "github.com/stretchr/testify/suite" @@ -105,9 +106,19 @@ func (suite *CLITestSuite) TestAddCommand() { suite.NoError(err) suite.Equal(os.ModeSymlink, info.Mode()&os.ModeSymlink) - // Verify file exists in repo - repoFile := filepath.Join(suite.tempDir, "lnk", ".bashrc") - suite.FileExists(repoFile) + // Verify some file exists in repo with .bashrc in the name + lnkDir := filepath.Join(suite.tempDir, "lnk") + entries, err := os.ReadDir(lnkDir) + suite.NoError(err) + + found := false + for _, entry := range entries { + if strings.Contains(entry.Name(), ".bashrc") && entry.Name() != ".lnk" { + found = true + break + } + } + suite.True(found, "Repository should contain a file with .bashrc in the name") } func (suite *CLITestSuite) TestRemoveCommand() { @@ -325,9 +336,92 @@ func (suite *CLITestSuite) TestAddDirectory() { suite.NoError(err) suite.Equal(os.ModeSymlink, info.Mode()&os.ModeSymlink) - // Verify directory exists in repo - repoDir := filepath.Join(suite.tempDir, "lnk", ".config") - suite.DirExists(repoDir) + // Verify some directory exists in repo with .config in the name + lnkDir := filepath.Join(suite.tempDir, "lnk") + entries, err := os.ReadDir(lnkDir) + suite.NoError(err) + + found := false + for _, entry := range entries { + if strings.Contains(entry.Name(), ".config") && entry.Name() != ".lnk" { + found = true + break + } + } + suite.True(found, "Repository should contain a directory with .config in the name") +} + +func (suite *CLITestSuite) TestSameBasenameFilesBug() { + // Initialize repository + err := suite.runCommand("init") + suite.Require().NoError(err) + suite.stdout.Reset() + + // Create two directories with files having the same basename + dirA := filepath.Join(suite.tempDir, "a") + dirB := filepath.Join(suite.tempDir, "b") + err = os.MkdirAll(dirA, 0755) + suite.Require().NoError(err) + err = os.MkdirAll(dirB, 0755) + suite.Require().NoError(err) + + // Create files with same basename but different content + fileA := filepath.Join(dirA, "config.json") + fileB := filepath.Join(dirB, "config.json") + contentA := `{"name": "config_a"}` + contentB := `{"name": "config_b"}` + + err = os.WriteFile(fileA, []byte(contentA), 0644) + suite.Require().NoError(err) + err = os.WriteFile(fileB, []byte(contentB), 0644) + suite.Require().NoError(err) + + // Add first file + err = suite.runCommand("add", fileA) + suite.NoError(err) + suite.stdout.Reset() + + // Verify first file content is preserved + content, err := os.ReadFile(fileA) + suite.NoError(err) + suite.Equal(contentA, string(content), "First file should preserve its original content") + + // Add second file with same basename - this should work correctly + err = suite.runCommand("add", fileB) + suite.NoError(err, "Adding second file with same basename should work") + + // CORRECT BEHAVIOR: Both files should preserve their original content + contentAfterAddA, err := os.ReadFile(fileA) + suite.NoError(err) + contentAfterAddB, err := os.ReadFile(fileB) + suite.NoError(err) + + suite.Equal(contentA, string(contentAfterAddA), "First file should keep its original content") + suite.Equal(contentB, string(contentAfterAddB), "Second file should keep its original content") + + // Both files should be removable independently + suite.stdout.Reset() + err = suite.runCommand("rm", fileA) + suite.NoError(err, "First file should be removable") + + // Verify output shows removal + output := suite.stdout.String() + suite.Contains(output, "Removed config.json from lnk") + + // Verify first file is restored with correct content + restoredContentA, err := os.ReadFile(fileA) + suite.NoError(err) + suite.Equal(contentA, string(restoredContentA), "Restored first file should have original content") + + // Second file should still be removable without errors + suite.stdout.Reset() + err = suite.runCommand("rm", fileB) + suite.NoError(err, "Second file should also be removable without errors") + + // Verify second file is restored with correct content + restoredContentB, err := os.ReadFile(fileB) + suite.NoError(err) + suite.Equal(contentB, string(restoredContentB), "Restored second file should have original content") } func TestCLISuite(t *testing.T) { diff --git a/internal/core/lnk.go b/internal/core/lnk.go index 7e0700f..269da93 100644 --- a/internal/core/lnk.go +++ b/internal/core/lnk.go @@ -43,6 +43,41 @@ func getRepoPath() string { return filepath.Join(xdgConfig, "lnk") } +// generateRepoName creates a unique repository filename from a relative path +func generateRepoName(relativePath string) string { + // Replace slashes and backslashes with underscores to create valid filename + repoName := strings.ReplaceAll(relativePath, "/", "_") + repoName = strings.ReplaceAll(repoName, "\\", "_") + return repoName +} + +// getRelativePath converts an absolute path to a relative path from home directory +func getRelativePath(absPath string) (string, error) { + homeDir, err := os.UserHomeDir() + if err != nil { + return "", fmt.Errorf("failed to get home directory: %w", err) + } + + // Check if the file is under home directory + relPath, err := filepath.Rel(homeDir, absPath) + if err != nil { + return "", fmt.Errorf("failed to get relative path: %w", err) + } + + // If the relative path starts with "..", the file is outside home directory + // In this case, use the absolute path as relative (without the leading slash) + if strings.HasPrefix(relPath, "..") { + // Use absolute path but remove leading slash and drive letter (for cross-platform) + cleanPath := strings.TrimPrefix(absPath, "/") + if len(cleanPath) > 1 && cleanPath[1] == ':' { + // Windows drive letter, keep as is + } + return cleanPath, nil + } + + return relPath, nil +} + // Init initializes the lnk repository func (l *Lnk) Init() error { return l.InitWithRemote("") @@ -109,9 +144,26 @@ func (l *Lnk) Add(filePath string) error { return fmt.Errorf("failed to get absolute path: %w", err) } - // Calculate destination path in repo - basename := filepath.Base(absPath) - destPath := filepath.Join(l.repoPath, basename) + // Get relative path for tracking + relativePath, err := getRelativePath(absPath) + if err != nil { + return fmt.Errorf("failed to get relative path: %w", err) + } + + // Generate unique repository name from relative path + repoName := generateRepoName(relativePath) + destPath := filepath.Join(l.repoPath, repoName) + + // Check if this relative path is already managed + managedItems, err := l.getManagedItems() + if err != nil { + return fmt.Errorf("failed to get managed items: %w", err) + } + for _, item := range managedItems { + if item == relativePath { + return fmt.Errorf("❌ File is already managed by lnk: \033[31m%s\033[0m", relativePath) + } + } // Check if it's a directory or file info, err := os.Stat(absPath) @@ -141,8 +193,8 @@ func (l *Lnk) Add(filePath string) error { return fmt.Errorf("failed to create symlink: %w", err) } - // Add to .lnk tracking file - if err := l.addManagedItem(absPath); err != nil { + // Add to .lnk tracking file using relative path + if err := l.addManagedItem(relativePath); err != nil { // Try to restore the original state if tracking fails _ = os.Remove(absPath) // Ignore error in cleanup if info.IsDir() { @@ -154,10 +206,10 @@ func (l *Lnk) Add(filePath string) error { } // Add both the item and .lnk file to git in a single commit - if err := l.git.Add(basename); err != nil { + if err := l.git.Add(repoName); err != nil { // Try to restore the original state if git add fails - _ = os.Remove(absPath) // Ignore error in cleanup - _ = l.removeManagedItem(absPath) // Ignore error in cleanup + _ = os.Remove(absPath) // Ignore error in cleanup + _ = l.removeManagedItem(relativePath) // Ignore error in cleanup if info.IsDir() { _ = l.fs.MoveDirectory(destPath, absPath) // Ignore error in cleanup } else { @@ -169,8 +221,8 @@ func (l *Lnk) Add(filePath string) error { // Add .lnk file to the same commit if err := l.git.Add(".lnk"); err != nil { // Try to restore the original state if git add fails - _ = os.Remove(absPath) // Ignore error in cleanup - _ = l.removeManagedItem(absPath) // Ignore error in cleanup + _ = os.Remove(absPath) // Ignore error in cleanup + _ = l.removeManagedItem(relativePath) // Ignore error in cleanup if info.IsDir() { _ = l.fs.MoveDirectory(destPath, absPath) // Ignore error in cleanup } else { @@ -180,10 +232,11 @@ func (l *Lnk) Add(filePath string) error { } // Commit both changes together + basename := filepath.Base(relativePath) if err := l.git.Commit(fmt.Sprintf("lnk: added %s", basename)); err != nil { // Try to restore the original state if commit fails - _ = os.Remove(absPath) // Ignore error in cleanup - _ = l.removeManagedItem(absPath) // Ignore error in cleanup + _ = os.Remove(absPath) // Ignore error in cleanup + _ = l.removeManagedItem(relativePath) // Ignore error in cleanup if info.IsDir() { _ = l.fs.MoveDirectory(destPath, absPath) // Ignore error in cleanup } else { @@ -208,6 +261,29 @@ func (l *Lnk) Remove(filePath string) error { return err } + // Get relative path for tracking + relativePath, err := getRelativePath(absPath) + if err != nil { + return fmt.Errorf("failed to get relative path: %w", err) + } + + // Check if this relative path is managed + managedItems, err := l.getManagedItems() + if err != nil { + return fmt.Errorf("failed to get managed items: %w", err) + } + + found := false + for _, item := range managedItems { + if item == relativePath { + found = true + break + } + } + if !found { + return fmt.Errorf("❌ File is not managed by lnk: \033[31m%s\033[0m", relativePath) + } + // Get the target path in the repository target, err := os.Readlink(absPath) if err != nil { @@ -219,7 +295,7 @@ func (l *Lnk) Remove(filePath string) error { target = filepath.Join(filepath.Dir(absPath), target) } - basename := filepath.Base(target) + repoName := filepath.Base(target) // Check if target is a directory or file info, err := os.Stat(target) @@ -232,13 +308,13 @@ func (l *Lnk) Remove(filePath string) error { return fmt.Errorf("failed to remove symlink: %w", err) } - // Remove from .lnk tracking file - if err := l.removeManagedItem(absPath); err != nil { + // Remove from .lnk tracking file using relative path + if err := l.removeManagedItem(relativePath); err != nil { return fmt.Errorf("failed to update tracking file: %w", err) } // Remove from Git first (while the item is still in the repository) - if err := l.git.Remove(basename); err != nil { + if err := l.git.Remove(repoName); err != nil { return fmt.Errorf("failed to remove from git: %w", err) } @@ -248,6 +324,7 @@ func (l *Lnk) Remove(filePath string) error { } // Commit both changes together + basename := filepath.Base(relativePath) if err := l.git.Commit(fmt.Sprintf("lnk: removed %s", basename)); err != nil { return fmt.Errorf("failed to commit changes: %w", err) } @@ -356,34 +433,41 @@ func (l *Lnk) Pull() ([]string, error) { func (l *Lnk) RestoreSymlinks() ([]string, error) { var restored []string - // Get managed items from .lnk file + // Get managed items from .lnk file (now containing relative paths) managedItems, err := l.getManagedItems() if err != nil { return nil, fmt.Errorf("failed to get managed items: %w", err) } - for _, itemName := range managedItems { - repoItem := filepath.Join(l.repoPath, itemName) + homeDir, err := os.UserHomeDir() + if err != nil { + return nil, fmt.Errorf("failed to get home directory: %w", err) + } + + for _, relativePath := range managedItems { + // Generate repository name from relative path + repoName := generateRepoName(relativePath) + repoItem := filepath.Join(l.repoPath, repoName) // Check if item exists in repository if _, err := os.Stat(repoItem); os.IsNotExist(err) { continue // Skip missing items } - // Determine where the symlink should be - // For config files, we'll place them in the user's home directory - homeDir, err := os.UserHomeDir() - if err != nil { - return nil, fmt.Errorf("failed to get home directory: %w", err) - } - - symlinkPath := filepath.Join(homeDir, itemName) + // Determine where the symlink should be created + symlinkPath := filepath.Join(homeDir, relativePath) // Check if symlink already exists and is correct if l.isValidSymlink(symlinkPath, repoItem) { continue } + // Ensure parent directory exists + symlinkDir := filepath.Dir(symlinkPath) + if err := os.MkdirAll(symlinkDir, 0755); err != nil { + return nil, fmt.Errorf("failed to create directory %s: %w", symlinkDir, err) + } + // Remove existing file/symlink if it exists if _, err := os.Lstat(symlinkPath); err == nil { if err := os.RemoveAll(symlinkPath); err != nil { @@ -393,10 +477,10 @@ func (l *Lnk) RestoreSymlinks() ([]string, error) { // Create symlink if err := l.fs.CreateSymlink(repoItem, symlinkPath); err != nil { - return nil, fmt.Errorf("failed to create symlink for %s: %w", itemName, err) + return nil, fmt.Errorf("failed to create symlink for %s: %w", relativePath, err) } - restored = append(restored, itemName) + restored = append(restored, relativePath) } return restored, nil @@ -470,25 +554,22 @@ func (l *Lnk) getManagedItems() ([]string, error) { } // addManagedItem adds an item to the .lnk tracking file -func (l *Lnk) addManagedItem(itemPath string) error { +func (l *Lnk) addManagedItem(relativePath string) error { // Get current items items, err := l.getManagedItems() if err != nil { return fmt.Errorf("failed to get managed items: %w", err) } - // Get the basename for storage - basename := filepath.Base(itemPath) - // Check if already exists for _, item := range items { - if item == basename { + if item == relativePath { return nil // Already managed } } - // Add new item - items = append(items, basename) + // Add new item using relative path + items = append(items, relativePath) // Sort for consistent ordering sort.Strings(items) @@ -497,20 +578,17 @@ func (l *Lnk) addManagedItem(itemPath string) error { } // removeManagedItem removes an item from the .lnk tracking file -func (l *Lnk) removeManagedItem(itemPath string) error { +func (l *Lnk) removeManagedItem(relativePath string) error { // Get current items items, err := l.getManagedItems() if err != nil { return fmt.Errorf("failed to get managed items: %w", err) } - // Get the basename for removal - basename := filepath.Base(itemPath) - - // Remove item + // Remove item using relative path var newItems []string for _, item := range items { - if item != basename { + if item != relativePath { newItems = append(newItems, item) } } diff --git a/internal/core/lnk_test.go b/internal/core/lnk_test.go index b283de2..dc899da 100644 --- a/internal/core/lnk_test.go +++ b/internal/core/lnk_test.go @@ -82,7 +82,19 @@ func (suite *CoreTestSuite) TestCoreFileOperations() { suite.Require().NoError(err) suite.Equal(os.ModeSymlink, info.Mode()&os.ModeSymlink) - repoFile := filepath.Join(suite.tempDir, "lnk", ".bashrc") + // The repository file will have a generated name based on the relative path + lnkDir := filepath.Join(suite.tempDir, "lnk") + entries, err := os.ReadDir(lnkDir) + suite.Require().NoError(err) + + var repoFile string + for _, entry := range entries { + if strings.Contains(entry.Name(), ".bashrc") && entry.Name() != ".lnk" { + repoFile = filepath.Join(lnkDir, entry.Name()) + break + } + } + suite.NotEmpty(repoFile, "Repository should contain a file with .bashrc in the name") suite.FileExists(repoFile) // Verify content is preserved @@ -129,8 +141,19 @@ func (suite *CoreTestSuite) TestCoreDirectoryOperations() { suite.Require().NoError(err) suite.Equal(os.ModeSymlink, info.Mode()&os.ModeSymlink) - // Verify directory exists in repo - repoDir := filepath.Join(suite.tempDir, "lnk", "testdir") + // Check that some repository directory exists with testdir in the name + lnkDir := filepath.Join(suite.tempDir, "lnk") + entries, err := os.ReadDir(lnkDir) + suite.Require().NoError(err) + + var repoDir string + for _, entry := range entries { + if strings.Contains(entry.Name(), "testdir") && entry.Name() != ".lnk" { + repoDir = filepath.Join(lnkDir, entry.Name()) + break + } + } + suite.NotEmpty(repoDir, "Repository should contain a directory with testdir in the name") suite.DirExists(repoDir) // Remove the directory @@ -179,8 +202,12 @@ func (suite *CoreTestSuite) TestLnkFileTracking() { lines := strings.Split(strings.TrimSpace(string(lnkContent)), "\n") suite.Len(lines, 2) - suite.Contains(lines, ".bashrc") - suite.Contains(lines, ".ssh") + + // The .lnk file now contains relative paths, not basenames + // Check that the content contains references to .bashrc and .ssh + content := string(lnkContent) + suite.Contains(content, ".bashrc", ".lnk file should contain reference to .bashrc") + suite.Contains(content, ".ssh", ".lnk file should contain reference to .ssh") // Remove one item and verify tracking is updated err = suite.lnk.Remove(testFile) @@ -191,8 +218,10 @@ func (suite *CoreTestSuite) TestLnkFileTracking() { lines = strings.Split(strings.TrimSpace(string(lnkContent)), "\n") suite.Len(lines, 1) - suite.Contains(lines, ".ssh") - suite.NotContains(lines, ".bashrc") + + content = string(lnkContent) + suite.Contains(content, ".ssh", ".lnk file should still contain reference to .ssh") + suite.NotContains(content, ".bashrc", ".lnk file should not contain reference to .bashrc after removal") } // Test XDG_CONFIG_HOME fallback @@ -310,6 +339,147 @@ func (suite *CoreTestSuite) TestGitOperations() { suite.Equal(0, status.Behind) } +// Test edge case: files with same basename from different directories should be handled properly +func (suite *CoreTestSuite) TestSameBasenameFilesOverwrite() { + err := suite.lnk.Init() + suite.Require().NoError(err) + + // Create two directories with files having the same basename + dirA := filepath.Join(suite.tempDir, "a") + dirB := filepath.Join(suite.tempDir, "b") + err = os.MkdirAll(dirA, 0755) + suite.Require().NoError(err) + err = os.MkdirAll(dirB, 0755) + suite.Require().NoError(err) + + // Create files with same basename but different content + fileA := filepath.Join(dirA, "config.json") + fileB := filepath.Join(dirB, "config.json") + contentA := `{"name": "config_a"}` + contentB := `{"name": "config_b"}` + + err = os.WriteFile(fileA, []byte(contentA), 0644) + suite.Require().NoError(err) + err = os.WriteFile(fileB, []byte(contentB), 0644) + suite.Require().NoError(err) + + // Add first file + err = suite.lnk.Add(fileA) + suite.Require().NoError(err) + + // Verify first file is managed correctly and preserves content + info, err := os.Lstat(fileA) + suite.Require().NoError(err) + suite.Equal(os.ModeSymlink, info.Mode()&os.ModeSymlink) + + symlinkContentA, err := os.ReadFile(fileA) + suite.Require().NoError(err) + suite.Equal(contentA, string(symlinkContentA), "First file should preserve its original content") + + // Add second file - this should work without overwriting the first + err = suite.lnk.Add(fileB) + suite.Require().NoError(err) + + // Verify second file is managed + info, err = os.Lstat(fileB) + suite.Require().NoError(err) + suite.Equal(os.ModeSymlink, info.Mode()&os.ModeSymlink) + + // CORRECT BEHAVIOR: Both files should preserve their original content + symlinkContentA, err = os.ReadFile(fileA) + suite.Require().NoError(err) + symlinkContentB, err := os.ReadFile(fileB) + suite.Require().NoError(err) + + suite.Equal(contentA, string(symlinkContentA), "First file should keep its original content") + suite.Equal(contentB, string(symlinkContentB), "Second file should keep its original content") + + // Both files should be removable independently + err = suite.lnk.Remove(fileA) + suite.Require().NoError(err, "First file should be removable") + + // First file should be restored with correct content + info, err = os.Lstat(fileA) + suite.Require().NoError(err) + suite.Equal(os.FileMode(0), info.Mode()&os.ModeSymlink) // Not a symlink anymore + + restoredContentA, err := os.ReadFile(fileA) + suite.Require().NoError(err) + suite.Equal(contentA, string(restoredContentA), "Restored file should have original content") + + // Second file should still be manageable and removable + err = suite.lnk.Remove(fileB) + suite.Require().NoError(err, "Second file should also be removable without errors") + + // Second file should be restored with correct content + info, err = os.Lstat(fileB) + suite.Require().NoError(err) + suite.Equal(os.FileMode(0), info.Mode()&os.ModeSymlink) // Not a symlink anymore + + restoredContentB, err := os.ReadFile(fileB) + suite.Require().NoError(err) + suite.Equal(contentB, string(restoredContentB), "Second restored file should have original content") +} + +// Test another variant: adding files with same basename should work correctly +func (suite *CoreTestSuite) TestSameBasenameSequentialAdd() { + err := suite.lnk.Init() + suite.Require().NoError(err) + + // Create subdirectories in different locations + configDir := filepath.Join(suite.tempDir, "config") + backupDir := filepath.Join(suite.tempDir, "backup") + err = os.MkdirAll(configDir, 0755) + suite.Require().NoError(err) + err = os.MkdirAll(backupDir, 0755) + suite.Require().NoError(err) + + // Create files with same basename (.bashrc) + configBashrc := filepath.Join(configDir, ".bashrc") + backupBashrc := filepath.Join(backupDir, ".bashrc") + + originalContent := "export PATH=/usr/local/bin:$PATH" + backupContent := "export PATH=/opt/bin:$PATH" + + err = os.WriteFile(configBashrc, []byte(originalContent), 0644) + suite.Require().NoError(err) + err = os.WriteFile(backupBashrc, []byte(backupContent), 0644) + suite.Require().NoError(err) + + // Add first .bashrc + err = suite.lnk.Add(configBashrc) + suite.Require().NoError(err) + + // Add second .bashrc - should work without overwriting the first + err = suite.lnk.Add(backupBashrc) + suite.Require().NoError(err) + + // Check .lnk tracking file should track both properly + lnkFile := filepath.Join(suite.tempDir, "lnk", ".lnk") + lnkContent, err := os.ReadFile(lnkFile) + suite.Require().NoError(err) + + // Both entries should be tracked and distinguishable + content := string(lnkContent) + suite.Contains(content, ".bashrc", "Both .bashrc files should be tracked") + + // Both files should maintain their distinct content + content1, err := os.ReadFile(configBashrc) + suite.Require().NoError(err) + content2, err := os.ReadFile(backupBashrc) + suite.Require().NoError(err) + + suite.Equal(originalContent, string(content1), "First file should keep original content") + suite.Equal(backupContent, string(content2), "Second file should keep its distinct content") + + // Both should be removable independently + err = suite.lnk.Remove(configBashrc) + suite.Require().NoError(err, "First .bashrc should be removable") + + err = suite.lnk.Remove(backupBashrc) + suite.Require().NoError(err, "Second .bashrc should be removable") +} + func TestCoreSuite(t *testing.T) { suite.Run(t, new(CoreTestSuite)) }