{"thread":{"id":"64948","subject":"Re: [PATCH v2] clone: fix segfault when using --revision and v0/v1 protocol","startedAt":"2026-02-08T14:09:55Z","lastAt":"2026-02-08T14:09:55Z","messageCount":1,"participants":["Jayce Cao"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"535477","messageId":"CAGwx5_8X1O=eUycHRm1u29iJmspeF2bX0oO9E34iUt_rS1W-Hw@mail.gmail.com","threadId":"64948","inReplyTo":null,"subject":"Re: [PATCH v2] clone: fix segfault when using --revision and v0/v1 protocol","fromName":"Jayce Cao","fromEmail":"jaycecao520@gmail.com","sentAt":"2026-02-08T14:09:43Z","receivedAt":"2026-02-08T14:09:55Z","isPatch":true,"sender":{"key":"jaycecao520@gmail.com","avatar":null},"body":"Hi Junio and thanks for your time!\n\n> While your change may skip the code that segfaults, wouldn't it also\n> stop noticing a broken case where .peer_ref should have been set but\n> didn't, even when --revision=<rev> parameter is not used in the\n> command invocation?  IOW, it is better to segfault and draw attention\n> by Git developers when a valid input is given by the end user and our\n> code misbehaves (e.g., and fails to to set .peer_ref as it should).\n\nI totally agree with you.\n\n\n> Wouldn't the correct fix be more like the following?\n>\n> - split out parts from update_remote_refs() that are needed even in\n>   option_rev mode into a separate helper function, and call that\n>   from cmd_clone().\n>\n> - make the call to update_remote_refs() conditional---specifically,\n>   we shouldn't be calling it when option_rev is in effect.\n\nDo we have another fix to make a conditional call to `find_ref_by_name()`\nwhen `option_rev` is in effect? Because from the doc of `--revision`,\n\"... and detach `HEAD` to_<rev>_. ...\", that means we don't need to\nknow where the real HEAD points to?\n\n> Also, isn't this something we can specify the expected behaviour in\n> tests?  Not only we want to ensure that nothing segfaults, we would\n> want to make sure that the resulting repository has no refs and HEAD\n> is detached at the specified revision.\n\nI'll add several tests to cover the bug after we make sure how to fix it.\n"}]}