server: uploadpack, implement multi_ack capability #1204
Conversation
9f04bee to
cc22bde
Compare
|
@manland as part of And remove:
This becomes an invalid state, its func needs updating, so that it can encode |
|
Side note: Once this is merged, the caveats called out in #64 for Azure DevOps are no longer required for users using the The Azure DevOps example becomes redundant, as the other examples will just work: |
|
@pjbgf Thank you for review. I fixed what you found. |
|
@pjbgf Here we are! An implem of single ack and multi_ack compatible with current API! I'm happy because I have put lot of efforts, try, fail and rework before all is ok (I have read c git code to understand what "acceptable commit" is 😄 ) I had added some tests too. In my project (a simple git server) this is how I test it:
Future tests could be to make same scenario but with 300 branches of 300 commits and to cut half commits of all branches, I think it's the worst case possible, but depend a lot of the client strategy to traverse the tree. Please review but not merge, all is in place to add multi_ack_detailed... soon in a next commit! Just tell me if you are OK with this implem, I'll squash bad commit and push multi_ack_detailed! |
|
@manland nice work so far, I will be taking a proper look over the weekend and provide some additional feedback. |
|
@pjbgf Just to let you know I'm on it! I successfully implemented multi_ack_detailed but when adding more complex scenario (in my project) found a bug in the nak/ack algo when encoding response. Unfortunately it's too late for me, I'm pretty sure after a big sleep all will become more simple 😅 |
|
@pjbgf All is good now! I have squash previous commits and added a new one with multi_ack_detailed. To test this dev I had:
RESET=$(shuf -i 0-$NB_COMMIT -n 1)
real_quiet_git reset --hard HEAD~$RESET
count=$((NB_COMMIT-RESET))
for (( j=0; j<=$count; j++ ))
do
echo "${i}_${j}_2" >> issues/bigCommits.txt
git add .
git commit -m "my ${i}_${j}_2 commit"
done
I have succefully make:
The client part should be done here with a reader for the server response. This is out of the scope of this PR which focus on server side. For now I prefer to try to implement |
@manland that sounds great. Feel free to create an issue with some links/additional information, so that whoever is keen on picking this up can ping you on it. 🙇 |
Since the multi_ack implementation (go-git#1204), Azure DevOps works out of the box, no longer requiring code changes. Therefore, the previous devops specific example is no longer needed. Signed-off-by: Paulo Gomes <pjbgf@linux.com>
…ities This adds support for multi-ack and multi-ack-detailed capabilities in the transport package. The support includes both the client and server. Related: go-git@2ebd65f Related: go-git#1204
…ities This adds support for multi-ack and multi-ack-detailed capabilities in the transport package. The support includes both the client and server. Related: go-git@2ebd65f Related: go-git#1204
…ities This adds support for multi-ack and multi-ack-detailed capabilities in the transport package. The support includes both the client and server. Related: go-git@2ebd65f Related: go-git#1204
…ities This adds support for multi-ack and multi-ack-detailed capabilities in the transport package. The support includes both the client and server. Related: go-git@2ebd65f Related: go-git#1204
I push to v6 (because I use it on my project) but I think it should work on v5.
Should close #647 (at least on my project).
I'm not sure why, but when I add
ar.Capabilities.Add("multi_ack")in my project I can delete theCapabilities.Add("no-thin")capability.