- Fix follow_symlinks parameter being ignored in is_dir(), is_file()
- Change from realpath() to abspath() to preserve symlinks
- Add separate caches for stat() and lstat() results
- Remove incorrect follow_symlinks param from is_symlink()
- Add comprehensive docstring explaining purpose and design
When follow_symlinks=False, methods now correctly return False for
symlinks instead of following them. Previously all symlinks were
resolved, breaking symlink-aware backup operations.
Fixes#8
Initialize file descriptors to None before opening to prevent
UnboundLocalError in the finally block if file opening fails.
Changes:
- Initialize fin = None and fout = None at function start
- Check if descriptors are not None before closing
- Remove UnboundLocalError from exception handling (no longer needed)
This improves code quality by eliminating the need to catch an error
that indicates a logic bug. The existing tests verify correct behavior.
Fixes#6
This commit addresses 8 high-priority issues identified in code analysis.
Fixes#3Fixes#4Fixes#5Fixes#7Fixes#10Fixes#19Fixes#20Fixes#21
## Critical Bug Fixes
1. **Race condition in lock file creation (#3)**
- Changed to atomic file creation using os.O_CREAT | os.O_EXCL
- Prevents two processes from both acquiring the lock
- Location: curateipsum/backup.py:110-115
2. **Invalid lock file error handling (#4)**
- Added try/except for corrupted/empty lock files
- Gracefully removes corrupted locks and retries
- Location: curateipsum/backup.py:121-133
3. **SIGKILL vs SIGTERM issue (#5)**
- Now sends SIGTERM first for graceful shutdown
- Waits 5 seconds before escalating to SIGKILL
- Allows previous process to clean up resources
- Location: curateipsum/backup.py:146-156
4. **Wrong stat object for permissions (#7)**
- Fixed bug where dst_stat was used instead of src_stat
- Permissions are now correctly updated during rsync
- Location: curateipsum/fs.py:371
5. **os.chown() fails for non-root users (#10)**
- Wrapped all os.chown() calls in try/except blocks
- Logs debug message instead of crashing
- Allows backups to succeed for non-root users
- Locations: curateipsum/fs.py:217-221, 228-231, 383-387, 469-472
## Comprehensive Test Coverage
6. **Lock file tests (#19)**
- Added TestBackupLock class with 7 test cases
- Tests: creation, concurrent prevention, stale locks, corruption
- Location: tests/test_backups.py:228-330
7. **Filesystem operation tests (#20)**
- Added tests/test_fs_extended.py with 6 test classes
- Tests: copy_file, copy_direntry, rsync, hardlink_dir, scantree, rm_direntry
- 20+ test cases covering normal and edge cases
- Location: tests/test_fs_extended.py
8. **Integration tests (#21)**
- Added tests/test_integration.py with 2 test classes
- Tests full backup workflow end-to-end
- Tests: incremental backups, hardlinks, delta dirs, cleanup, recovery
- 14 test cases covering complete backup lifecycle
- Location: tests/test_integration.py
## Test Results
All 68 tests pass successfully:
- 11 original backup cleanup tests
- 7 new lock file tests
- 16 original fs tests
- 20 new fs extended tests
- 14 new integration tests
## Impact
These fixes address critical bugs that could cause:
- Data corruption from concurrent backups
- Incomplete cleanup from forced process termination
- Permission sync failures
- Tool unusability for non-root users
The comprehensive test coverage ensures these bugs are caught early
and provides confidence for future refactoring.
