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.

Sign up for GitHub

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

Jump to bottom

Rust sorted_tree_items() disagrees with Python version #1325

Open
danchr opened this issue Jun 3, 2024 · 5 comments
Open

Rust sorted_tree_items() disagrees with Python version #1325

danchr opened this issue Jun 3, 2024 · 5 comments

Comments

@danchr
Copy link
Contributor

danchr commented Jun 3, 2024 •
edited
Loading

Running the hg-git tests against Dulwich 0.22, I get a subtle error when using the new Rust extensions. As far as I can tell, the bug is in dulwich.objects.sorted_tree_items(), as inserting this suppresses the bug:

from dulwich import objects as dulobjs
dulobjs.sorted_tree_items = dulobjs._sorted_tree_items_py

I tried in vain to provoke the bug in test_objects.py, so I'd appreciate any help on what this might actually be.

By inserting pprint.pprint(tree.items()) in the appropriate location, I get the following difference between the Python and Rust implementations:

--- tests/test-renames.t	2024-06-03 22:34:52.000000000 +0200
+++ tests/test-renames.t.err	2024-06-03 22:35:05.000000000 +0200
@@ -443,10 +443,10 @@
    TreeEntry(path=b'gamma', mode=33188, sha=b'cf7a729ca69bfabd0995fc9b083e86a18215bd91'),
    TreeEntry(path=b'gitsubmodule2', mode=57344, sha=b'5944b31ff85b415573d1a43eb942e2dea30ab8be')]
   [TreeEntry(path=b'.gitmodules', mode=33188, sha=b'99903db4c068cf9aec3bc8911691bb920a21ab6f'),
    TreeEntry(path=b'beta', mode=33188, sha=b'08fe19ca4d2f79624f35333157d610811efc1aed'),
-   TreeEntry(path=b'gamma', mode=57344, sha=b'5944b31ff85b415573d1a43eb942e2dea30ab8be'),
    TreeEntry(path=b'gamma-new', mode=33188, sha=b'cf7a729ca69bfabd0995fc9b083e86a18215bd91'),
+   TreeEntry(path=b'gamma', mode=57344, sha=b'5944b31ff85b415573d1a43eb942e2dea30ab8be'),
    TreeEntry(path=b'gitsubmodule2', mode=57344, sha=b'5944b31ff85b415573d1a43eb942e2dea30ab8be')]
   [TreeEntry(path=b'.gitmodules', mode=33188, sha=b'434d520544ae0161f0cf5a58c41eff55a363f426'),
    TreeEntry(path=b'beta', mode=33188, sha=b'08fe19ca4d2f79624f35333157d610811efc1aed'),
    TreeEntry(path=b'gamma', mode=33188, sha=b'cf7a729ca69bfabd0995fc9b083e86a18215bd91'),
@@ -460,24 +460,25 @@
    TreeEntry(path=b'epsilon', mode=33188, sha=b'0767db5c38e2943c2ff4e2d0361672e7514fb246'),
    TreeEntry(path=b'gamma', mode=33188, sha=b'f05eb77081697006a7e3f2060cdc1576684421f5'),
    TreeEntry(path=b'gitsubmodule2', mode=57344, sha=b'5944b31ff85b415573d1a43eb942e2dea30ab8be')]
   $ git --git-dir .hg/git log master --pretty=oneline
-  8a00d0fb75377c51c9a46e92ff154c919007f0e2 delta/epsilon
-  dd7d4f1adb942a8d349dce585019f6949184bc64 gamma2
-  3f1cdaf8b603816fcda02bd29e75198ae4cb13db remove submodule and rename back
-  2a4abf1178a999e2054158ceb0c7768079665d03 rename and add submodule
+  f3f6592447685566af9447c03ae262aa5432511d delta/epsilon
+  c51ce14ec367c5ea72bf428dee3f8576f2fe1bb0 gamma2
+  df749cae534e3c7a0ad664cd0f214dd36e0ac259 remove submodule and rename back
+  8f9ec605ad0cc2532202f73cef8e35d3241797ee rename and add submodule
   88c416e8d5e0e9dd1187d45ebafaa46111764196 beta renamed back
   027d2a6e050705bf6f7e226e7e97f02ce5ae3200 beta renamed
   dc70e620634887e70ac5dd108bcc7ebd99c60ec3 move submodule
   c610256cb6959852d9e70d01902a06726317affc add submodule
   e1348449e0c3a417b086ed60fc13f068d4aa8b26 gamma
   cc83241f39927232f690d370894960b0d1943a0e beta
   938bb65bb322eb4a3558bec4cdc8a680c4d1794c alpha
   $ git --git-dir .hg/git diff-tree master~3 -r
-  2a4abf1178a999e2054158ceb0c7768079665d03
+  8f9ec605ad0cc2532202f73cef8e35d3241797ee
   :100644 100644 434d520544ae0161f0cf5a58c41eff55a363f426 99903db4c068cf9aec3bc8911691bb920a21ab6f M	.gitmodules
-  :100644 160000 cf7a729ca69bfabd0995fc9b083e86a18215bd91 5944b31ff85b415573d1a43eb942e2dea30ab8be T	gamma
+  :100644 000000 cf7a729ca69bfabd0995fc9b083e86a18215bd91 0000000000000000000000000000000000000000 D	gamma
   :000000 100644 0000000000000000000000000000000000000000 cf7a729ca69bfabd0995fc9b083e86a18215bd91 A	gamma-new
+  :000000 160000 0000000000000000000000000000000000000000 5944b31ff85b415573d1a43eb942e2dea30ab8be A	gamma
 
 Test findcopiesharder
 
   $ cd $TESTTMP
@danchr
Copy link
Contributor Author

danchr commented Jun 6, 2024 •
edited
Loading

I'll add a workaround to hg-git that disables this function: https://foss.heptapod.net/mercurial/hg-git/-/merge_requests/238

(I'm assuming it's fixed in the next release.)

@jelmer
Copy link
Owner

jelmer commented Jun 6, 2024

Nice find! Fix on the way..

danchr added a commit to danchr/hg-git that referenced this issue Jun 25, 2024
@danchr
compat: blacklist 0.22.0 and 0.22.1
4e7c6b2 
The Rust extensions has a bug in rename detection that affects hg-git.
Presumably, it'll be fixed in the next release.

See: jelmer/dulwich#1325
Closes: #437

--HG--
branch : 1.1.x
@danchr
Copy link
Contributor Author

danchr commented Aug 23, 2024

Did you make any progress on this?

@jelmer
Copy link
Owner

jelmer commented Oct 16, 2024

I just spent two hours on this and it's still a mystery to me. I can't reproduce it, and looking at both implementations I don't see how the behaviour could be different (or from that in C Git).

In the end, I did end up refactoring the rust implementation so I'd be grateful if you could try again with hg-git.

@danchr
Copy link
Contributor Author

danchr commented Oct 21, 2024

Sadly, this is still broken in 0.22.3. Maybe this helps… The test that commit that fails is created as this:

Rename a file elsewhere and replace it with a submodule:

  $ git mv gamma gamma-new
  $ git submodule add ../gitsubmodule gamma 2>&1 | python -c "$rmpwd" | sed "$clonefilt" | grep -E -v '^done\.$'
  Initialized empty Git repository in ...
  
  $ fn_git_commit -m 'rename and add submodule'

Then, on regenerating the repository from the Mercurial conversion, the results differ on the Git side of things. I'll try to do some more investigation…

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@jelmer @danchr