#559 incorrect change when moving file back to previous name
Plain pijul commands have the same issue.
pijul init
echo 'Hello Pijul!' > readme
pijul record --all --message readme
pijul mv readme READ_ME
pijul record --all --message READ_ME
pijul mv READ_ME readme
pijul record --all --message 'readme again'
Looking at the logs it’s correctly determining that there’s a file move via parent but also that it’s resurrecting the file though a different parent.
This loop in collect_moved_edges needs something to prioritise the move
for parent in iter_adjacent(
txn,
channel,
current_pos.inode_vertex(),
EdgeFlags::FOLDER | EdgeFlags::PARENT,
EdgeFlags::all(),
)
Log with both move and resurrect
libpijul::record] record_moved_file RecordItem { v_papa: Position { change: Some(ChangeId(AAAAAAAAAAAAA)), pos: ChangePosition(L64(0)) }, papa: Inode(AAAAAAAAAAAAA), inode: Inode(AT4G5WYZ2U5JA), basename: "readme", full_path: "readme", metadata: InodeMetadata(0) }
libpijul::record] collect_moved_edges Position { change: ChangeId(MVNNDZX6BPVP4), pos: ChangePosition(L64(1)) }
libpijul::record] parent = E(BLOCK | FOLDER | PARENT, T2LKWRJ7XXHDI[31], T2LKWRJ7XXHDI)
libpijul::pristine::sanakirja] find_block_end, loop, k = V(ChangeId(MVNNDZX6BPVP4)[1:1]), p = Position { change: ChangeId(T2LKWRJ7XXHDI), pos: ChangePosition(L64(31)) }
libpijul::pristine::sanakirja] find_block_end, loop, k = V(ChangeId(T2LKWRJ7XXHDI)[0:31]), p = Position { change: ChangeId(T2LKWRJ7XXHDI), pos: ChangePosition(L64(31)) }
libpijul::pristine::sanakirja] find_block_end, V(ChangeId(T2LKWRJ7XXHDI)[0:31]) Position { change: ChangeId(T2LKWRJ7XXHDI), pos: ChangePosition(L64(31)) }
libpijul::pristine::sanakirja] find_block_end, V(ChangeId(T2LKWRJ7XXHDI)[0:31]) Position { change: ChangeId(T2LKWRJ7XXHDI), pos: ChangePosition(L64(31)) }
libpijul::changestore::filesystem] get_contents V(ChangeId(T2LKWRJ7XXHDI)[0:31])
libpijul::changestore::filesystem] get_contents 31
libpijul::record] parent_dest V(ChangeId(T2LKWRJ7XXHDI)[0:31]) InodeMetadata(0) "READ_ME"
libpijul::record] new_meta = InodeMetadata(0), parent_meta = InodeMetadata(0)
libpijul::record] grandparent: E(BLOCK | FOLDER | PARENT, AAAAAAAAAAAAA[0], T2LKWRJ7XXHDI)
libpijul::record] grandparent_dest V(ChangeId(AAAAAAAAAAAAA)[0:0]) Ok("\u{7}\u{0}\u{0}\u{0}\u{0}\u{0}\u{0}\u{0}READ_ME\u{1}\u{5}\u{0}\u{0}\u{0}\u{0}\u{0}\u{0}\u{0}UTF-8")
libpijul::record] change = false true false <========= move detected
libpijul::record] parent = E(BLOCK | FOLDER | PARENT, MVNNDZX6BPVP4[46], MVNNDZX6BPVP4)
libpijul::pristine::sanakirja] find_block_end, loop, k = V(ChangeId(MVNNDZX6BPVP4)[16:46]), p = Position { change: ChangeId(MVNNDZX6BPVP4), pos: ChangePosition(L64(46)) }
libpijul::pristine::sanakirja] find_block_end, V(ChangeId(MVNNDZX6BPVP4)[16:46]) Position { change: ChangeId(MVNNDZX6BPVP4), pos: ChangePosition(L64(46)) }
libpijul::pristine::sanakirja] find_block_end, V(ChangeId(MVNNDZX6BPVP4)[16:46]) Position { change: ChangeId(MVNNDZX6BPVP4), pos: ChangePosition(L64(46)) }
libpijul::changestore::filesystem] get_contents V(ChangeId(MVNNDZX6BPVP4)[16:46])
libpijul::changestore::filesystem] get_contents 30
libpijul::record] parent_dest V(ChangeId(MVNNDZX6BPVP4)[16:46]) InodeMetadata(0) "readme"
libpijul::record] new_meta = InodeMetadata(0), parent_meta = InodeMetadata(0)
libpijul::record] grandparent: E(BLOCK | FOLDER | PARENT | DELETED, AAAAAAAAAAAAA[0], T2LKWRJ7XXHDI)
libpijul::record] grandparent_dest V(ChangeId(AAAAAAAAAAAAA)[0:0]) Ok("\u{6}\u{0}\u{0}\u{0}\u{0}\u{0}\u{0}\u{0}readme\u{1}\u{5}\u{0}\u{0}\u{0}\u{0}\u{0}\u{0}\u{0}UTF-8")
libpijul::record] change = false false false <======= move not detected
PR32ALRGHSEWKZWV4GAYOTJZNGBKYMHKH7X6RWLLBFRQUHYFQWWAC Hi! thanks for the patch. However, due to a bug in the Nest, it seems I’ve lost it. Would you mind pushing it again to this discussion? Sorry about that.
Sure. I’ve hit a related issue where some moves were not applied resulting in a “Path deletion conflict”. I’ve not yet checked whether that’s due to this change.
Confirmed that there is an issue with Pijul. Without this change there is no deletion conflict but the same files are now in both the correct and original place and the original ones are no longer part of the recorded repo. I’ll try and create a minimal reproduction.
VXFXV55PVPRLLMTV5LR3WUVFSMC3KK3HD5GCU25DYSBGPZBWV5PQC TELBT3CINIPFLPR2NAW75U3T76CU5LEC2NHTXTDAM64ZKNHRVSKQC A path deletion conflict should be triggered by a file move or edit, done in parallel with a deletion, either in that file or in a directory above.
What do you think of the patch I just pushed, avoiding two loops? Is there any reason why we shouldn’t refactor these loops?
(also, thanks again for re-pushing)
The directory the file was moved from was deleted but I wouldn’t have expected a conflict as it was done in the same change as the move.
The reason I did it in two loops was I couldn’t tell from the iterator code whether the rename would always be found before the un-deletion. That was the ordering in my single file moved tests but I haven’t tried more complex scenarios.
Haven’t been able to replicate the directory deletion + file move self-conflict but I have found that deleting multiple directory levels displays incorrectly in the diff even if the change appears to work correctly. Given a directory hierarchy a/b/c when removing that entirely the recorded change shows that the top level a was deleted multiple times and the others aren’t mentioned.
SMMRGKCXLDFNI7RAACOTGRGUPZLWY5MKVBL6WB2ZPEUCKDQ4PLIAC The little descriptions in the patch format are just metadata allowing users to make sense of the patches. The actual work is done by the actual graph changes. I’ll close this since it is fixed on our test cases (I tried as well).
When a git repo has a file which was moved then moved back again in a later commit then Pijul is recording that as an undeletion, even though the loggings shows it as a move.
Initial move changes:
and
but then
The sample git repo is https://drive.google.com/file/d/1o8DwtryDeWuPwGV8y-QJsVkgIBj38aEj/view?usp=sharing