fix: prevent file loss when multiple files have same basename

Fixes #2: https://github.com/yarlson/lnk/issues/2

Previously, files with the same basename (e.g., a/config.json and b/config.json)
would overwrite each other in the repository, causing data loss. The second file
would completely replace the first, and removing files would fail with 'no such
file or directory' errors.

Changes:
- Store files using unique names based on full relative paths (slashes → underscores)
- Track full relative paths in .lnk file instead of just basenames
- Generate repository names from relative paths to prevent collisions
- Update symlink restoration to work with new path-based system
- Add comprehensive tests for basename collision scenarios

This ensures each file maintains its unique content and can be managed
independently, eliminating the data loss issue.
This commit is contained in:
Yar Kravtsov
2025-05-24 18:10:20 +03:00
parent 7007ec64f2
commit f96bfb6ce0
3 changed files with 397 additions and 55 deletions

View File

@@ -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) {

View File

@@ -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)
}
}

View File

@@ -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))
}