HomeSoftware Heritage

Fix handling of 'done' in graph walker and implement the 'no-done' capability.

Description

Fix handling of 'done' in graph walker and implement the 'no-done' capability. (#88)

The gist of this gnarly problem is that it looks like the implementation over HTTP behaves as-if the no-done capability was enabled but unspecified so the reference client would make a second round trip back with more haves but then it doesn't know what to do with the PACK that was already sent. Alternatively there are cases where the side-band was used when it was expecting ACK/NAK. There were multiple problems with the code base that made this issue rather hard to stomp out, and here are the fixes (some verbatim from commit messages).

  • MultiAckDetailedGraphWalkerImpl rebuilt to ensure output generated matches the reference implementation, like the git-http-backend. (Even though this is not strictly necessary, repeatedly calling all_wants_specified doesn't really change much given that there's only one place where the commits can be added in, which is by the object store through the ack method.
  • Correction to the communication after all have lines have been processed. As a freebie I got no-done capability implemented since that's what it looked like before.
  • There is another issue where attempts to pull an unrelated repo, the various combination of the previous flaws made it difficult to send the correct number of NAKs. Removing the walker.send_ack call from the walker implementation and just ensure the ack is the only method that will call send_ack simplifies the process of correcting this.
  • Added various state variables to the handler to track whether the done token is expected.
  • Added a couple methods to deal with the handling of the above state variables, which the walker implementations themselves have access to through a method provided by the generic protocol walker class
  • Added a method to the graph walker that takes in the state variables provided by the handler to delegate the dealing of the final ACK/NAK line that ends the section to permit the PACK section to begin, which then gets delegated to the Impl instances (which also have been normalized to address this).
  • Finally, cleaned up the test cases and added further tests where relevant. Naturally

one of the earlier commits introduce some test examples that demonstrates this problem.

Details

Provenance
Tommy Yu <tommy.yu@auckland.ac.nz>Authored on May 24 2015, 4:29 PM
jelmerCommitted on May 24 2015, 4:30 PM
ardumontPushed on Sep 27 2021, 5:34 PM
Parents
rPPDWe7fe2c362dc1: Add TestCase.assertObjectstoreEqual.
Branches
Unknown
Tags
Unknown

Event Timeline