Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes directory order dependency for 'filenames-equal' option #11

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

chadnetzer
Copy link
Collaborator

With samename option enabled, possible matching hardlinks could be
skipped depending on the directory iteration order and existing hardlink
status of files. This was causing actual failure of
test_hardlink_tree_filenames_equal() on some systems.

This seems like a minimum-change fix to fix the problem in issue #5, and allow the "filenames-equal" option to behave a bit more like expected.

Note that there is really a policy decision required longer term, as to how the "filenames-equal" option should behave when files have existing hard-links. As it is, since not all the matching files will be linked together in the end, existing hard-links between identical files with different basenames, will not necessarily be preserved (perhaps that is desired). A longer term project might be to decide what the behavior for existing hardlinks should be, and make that fully consistent (is could require a fair amount of code restructure, though).

@chadnetzer
Copy link
Collaborator Author

I added a test that exercises this bug by setting up the initial link relationship in reverse order. Ie. it adds a test which links dir2/name1.ext to dir1/link before running the algorithm with --filenames-equal set.

This PR fixes the bug, which means both tests succeed. The bug can be demonstrated by cherry-picking only the commit with the added test, causing one of the two filenames equal tests to fail.

@chadnetzer chadnetzer changed the title Remove directory order dependency for samename Fixes directory order dependency for 'filenames-equal' option Jul 3, 2018
@chadnetzer chadnetzer closed this Jul 7, 2018
@chadnetzer chadnetzer deleted the 5_samename_tests_fix branch July 7, 2018 10:58
@chadnetzer
Copy link
Collaborator Author

Reopening this PR and rebasing fix on current master.

@chadnetzer chadnetzer reopened this Jul 15, 2018
With samename option enabled, possible matching hardlinks could be
skipped depending on the directory iteration order and existing hardlink
status of files.  This was causing actual failure of
test_hardlink_tree_filenames_equal() on some systems.
Adds a test in case dir2 is found before dir1.  With the filenames equal
option enabled, the order of iteration could can a false negative (ie.
an improperly passed test).  By changing the initial link relationship,
we can cause the test to fail if the directory iteration order is
reversed.
@chadnetzer chadnetzer force-pushed the 5_samename_tests_fix branch from 706aeec to 0989969 Compare July 15, 2018 22:24
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.

1 participant