Skip to content

test(rev_store): tip negotiation serves unrelated references#13672

Open
Alizter wants to merge 1 commit intoocaml:mainfrom
Alizter:push-rrmwznnztxwx
Open

test(rev_store): tip negotiation serves unrelated references#13672
Alizter wants to merge 1 commit intoocaml:mainfrom
Alizter:push-rrmwznnztxwx

Conversation

@Alizter
Copy link
Collaborator

@Alizter Alizter commented Feb 23, 2026

We add an unrelated repository to the test added in #13598. It demonstrates that unrelated remotes' refs are being used in the tip negotiation.

Reproduction case for #13671.

Fix for this will be in a follow up PR.

Signed-off-by: Ali Caglayan <alizter@gmail.com>
Copy link
Collaborator

@Leonidas-from-XIV Leonidas-from-XIV left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice test; it's rather nice that we can get this tested despite this going through git!

I have one question and one suggestion for improvement.

let unrelated_git = Git_test_utils.git ~dir:unrelated_repo_dir in
let* () =
git_init_and_config_user unrelated_repo_dir
>>> unrelated_git [ "config"; "uploadpack.allowAnySHA1InWant"; "true" ]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a comment why this is necessary? I don't really understand why we need this config line so a quick explanation would be helpful to any readers.

[%expect {| Negotiation 'have' lines sent: 3 |}]
already has, avoiding redundant downloads. The count includes refs from
the unrelated remote. *)
[%expect {| Negotiation 'have' lines sent: 4 |}]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of counting lines, maybe it would be better to collect the expected tips when creating the repos and testing whether they have been sent? That way we can be sure that the tips we send are those that we expect (and in the case of unrelated_repo want to stop sending in the future).

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.

2 participants