Skip to content

Conversation

@cr1901
Copy link
Contributor

@cr1901 cr1901 commented Sep 30, 2025

This is a follow-up to #8344. After mulling it over, I generally think borg is doing the right thing when diffing with hard links, as the changes reported match POSIX semantics for unlinking:

Upon successful completion, unlink() shall mark for update the last data modification and last file status change timestamps of the parent directory. Also, if the file's link count is not 0, the last file status change timestamp of the file shall be marked for update.

An mtime and ctime change with no content modification or permission change between the same file in two repos is probably a reasonable hint that a hard link was broken. Similarly, a ctime change with no permission change is probably a reasonable hint that the hard link count changed. This is the same info borg v1 currently reports for a hard link that was broken/hard link count that changed. Additionally, excluding one hard link but not another from a diff seems to work just fine as well, completely sidestepping the need for a fix like #8344 :) in v1.

It may be interesting to extend the diff output to be hard-link aware, but not clear to me what this should look like, short of the "primary/secondary" mechanism that required #8344 in the first place :). For now users interested in actual hard link info can supply a --format like '{hlid} {path}' to borg-2 list and sort:

william@xubuntu-dtrain:/tmp/borg-hl-test$ borg-2 list --format '{path} {hlid}{NL}' -r ./repo-2 test
.
a 72b133632359d572aeb98bebb9654a73912e922f5d714f24098dd0cdc11a4909
b 72b133632359d572aeb98bebb9654a73912e922f5d714f24098dd0cdc11a4909

As long as borg keeps track of the info, there shouldn't be much need for end users to worry about hard link diffs (and the mtime/ctime hints above are good markers).

@codecov
Copy link

codecov bot commented Sep 30, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.45%. Comparing base (d274097) to head (4951e10).
⚠️ Report is 11 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9031      +/-   ##
==========================================
+ Coverage   81.13%   81.45%   +0.32%     
==========================================
  Files          77       77              
  Lines       13515    13515              
  Branches     2004     2004              
==========================================
+ Hits        10965    11009      +44     
+ Misses       1884     1853      -31     
+ Partials      666      653      -13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ThomasWaldmann
Copy link
Member

@cr1901 can you install pre-commit, so black runs automatically?

https://borgbackup.readthedocs.io/en/latest/development.html#building-a-development-environment

@cr1901
Copy link
Contributor Author

cr1901 commented Sep 30, 2025

Okay, looks like I properly installed the pre-commits and CI is passing now.

Copy link
Member

@ThomasWaldmann ThomasWaldmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. Some minor things I found...

Copy link
Member

@ThomasWaldmann ThomasWaldmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Are you finished with this, so I can merge it?

@cr1901
Copy link
Contributor Author

cr1901 commented Oct 2, 2025

Yes, I am finished. Feel free to merge.

@ThomasWaldmann ThomasWaldmann merged commit 1c6ef7a into borgbackup:master Oct 2, 2025
17 checks passed
@ThomasWaldmann
Copy link
Member

Thanks for helping!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants