Junio C Hamano <gitster@xxxxxxxxx> writes: > Karthik Nayak <karthik.188@xxxxxxxxx> writes: > >> The 'git-send-pack(1)' allows users to push objects to a remote >> repository and explicitly list the references to be pushed. The status >> of each reference pushed is captured into a list mapped by refname. >> >> If a reference fails to be updated, its error message is captured in the >> `ref->remote_status` field. While the command allows duplicate ref >> inputs, the list of doesn't accommodate this behavior as a particular > > -ECANNOTPARSE around "the list of doesn't accommodate". > Indeed, a s/of// should make this readable. >> refname is linked to a single `struct ref*` element. So if the user >> inputs a reference twice like: >> >> git send-pack remote.git A:foo B:foo >> >> where the user is trying to update the same reference 'foo' twice and >> the reference fails to be updated, we first fill `ref->remote_status` >> with error message for the input 'A:foo' then we override the same field >> with the error message for 'B:foo'. This override happens without first >> free'ing the previous value. Fix this leak. > > OK. A natural question is what happens when A successfully updates > and B fails, or A fails but B successfully updates (failing both is > much less interesting). What should happen in such cases is unclear > but my gut feeling is that the last-one wins (which you implemented) > is probably just as OK as the first-one gets retained (which might > be less work at runtime), and perhaps keeping-the-more-severe-one > might be more useful than either of these two, but I didn't think > it through. > I think the whole thing reeks a little bit. We shouldn't allow such duplicates in the first place. This is something we tackle in the next patch, where any such duplicates fails all updates. This is a lot more deterministic and also the error reporting to the user is inline with what is actually done. As of this patch, if there are duplicates, one success one failure, means that the ref is updated once. But we report to the user that both the refs failed to be updated as: $ git send-pack remote.git A:foo B:foo Enumerating objects: 3, done. Counting objects: 100% (3/3), done. Delta compression using up to 20 threads Compressing objects: 100% (2/2), done. Writing objects: 100% (3/3), 226 bytes | 226.00 KiB/s, done. Total 3 (delta 1), reused 0 (delta 0), pack-reused 0 (from 0) remote: error: cannot lock ref 'refs/heads/foo': reference already exists To remote.git ! [remote rejected] A -> foo (failed to update ref) ! [remote failure] B -> foo (remote failed to report status) This is totally wrong, since we actually do update one of the refs. > THanks.
Attachment:
signature.asc
Description: PGP signature
