{"thread":{"id":"41586","subject":"[PATCH] bundle doc: 'verify' is not about verifying the bundle","startedAt":"2016-03-01T21:36:05Z","lastAt":"2016-03-02T18:18:40Z","messageCount":4,"participants":["Junio C Hamano","Philip Oakley","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"280006","messageId":"xmqqd1rd7uey.fsf@gitster.mtv.corp.google.com","threadId":"41586","inReplyTo":null,"subject":"[PATCH] bundle doc: 'verify' is not about verifying the bundle","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-01T21:36:05Z","receivedAt":"2016-03-01T21:36:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Even though the command does read the bundle header and checks to\nsee if it looks reasonable, the thin-pack data stream that follows\nthe header in the bundle file is not checked. More importantly,\nbecause the thin-pack data does not have a trailing checksum like\non-disk packfiles do, there isn't much \"verification\" the command\ncan do without unpacking the objects from the stream even if it\nwanted to.\n\nThe documentation gives an incorrect impression that the thin-pack\ndata contained in the bundle is validated, but the command is to\nvalidate that the receiving repository is ready to accept the\nbundle, not to check the validity of a bundle file. Rephrase the\nparagraph to clarify this.\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/git-bundle.txt | 9 ++++-----\n 1 file changed, 4 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-bundle.txt b/Documentation/git-bundle.txt\nindex 3a8120c..c0113a8 100644\n--- a/Documentation/git-bundle.txt\n+++ b/Documentation/git-bundle.txt\n@@ -38,11 +38,10 @@ create ::\n \t'git-rev-list-args' arguments to define the bundle contents.\n \n verify ::\n-\tUsed to check that a bundle file is valid and will apply\n-\tcleanly to the current repository. This includes checks on the\n-\tbundle format itself as well as checking that the prerequisite\n-\tcommits exist and are fully linked in the current repository.\n-\t'git bundle' prints a list of missing commits, if any, and exits\n+\tVerifies that the given 'file' has a valid-looking bundle\n+\theader, and that your repository has all prerequisite\n+\tobjects necessary to unpack the file as a bundle. The\n+\tcommand prints a list of missing commits, if any, and exits\n \twith a non-zero status.\n \n list-heads ::\n-- \n2.8.0-rc0-114-g0b3e5e5\n"},{"id":"280015","messageId":"499EFADFE03B4EDFA48F7E0B99A0E36D@PhilipOakley","threadId":"41586","inReplyTo":"xmqqd1rd7uey.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] bundle doc: 'verify' is not about verifying the bundle","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":null,"receivedAt":"2016-03-01T22:49:58Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Junio C Hamano\" \n> Even though the command does read the bundle header and checks to\n> see if it looks reasonable, the thin-pack data stream that follows\n> the header in the bundle file is not checked. More importantly,\n> because the thin-pack data does not have a trailing checksum like\n> on-disk packfiles do, there isn't much \"verification\" the command\n> can do without unpacking the objects from the stream even if it\n> wanted to.\n> \n> The documentation gives an incorrect impression that the thin-pack\n> data contained in the bundle is validated, but the command is to\n> validate that the receiving repository is ready to accept the\n> bundle, not to check the validity of a bundle file. Rephrase the\n> paragraph to clarify this.\n> \n\nThis looks good to me.\n\nI was actually looking at this over the weekend with respect to a back-\nburner issue about indicating which ref is HEAD within the bundle.\n(thread variously $gmane/265001 and it's prior links, though now I'm minded\nto add \\0HEAD after the correct ref; e.g. ' refs/heads/\\0HEAD')\n\nPhilip\n\n> Signed-off-by: Junio C Hamano \n> ---\n> Documentation/git-bundle.txt | 9 ++++-----\n> 1 file changed, 4 insertions(+), 5 deletions(-)\n> \n> diff --git a/Documentation/git-bundle.txt b/Documentation/git-bundle.txt\n> index 3a8120c..c0113a8 100644\n> --- a/Documentation/git-bundle.txt\n> +++ b/Documentation/git-bundle.txt\n> @@ -38,11 +38,10 @@ create ::\n> 'git-rev-list-args' arguments to define the bundle contents.\n> \n> verify ::\n> - Used to check that a bundle file is valid and will apply\n> - cleanly to the current repository. This includes checks on the\n> - bundle format itself as well as checking that the prerequisite\n> - commits exist and are fully linked in the current repository.\n> - 'git bundle' prints a list of missing commits, if any, and exits\n> + Verifies that the given 'file' has a valid-looking bundle\n> + header, and that your repository has all prerequisite\n> + objects necessary to unpack the file as a bundle. The\n> + command prints a list of missing commits, if any, and exits\n> with a non-zero status.\n> \n> list-heads ::\n> -- \n> 2.8.0-rc0-114-g0b3e5e5\n> \n> --\n"},{"id":"280054","messageId":"20160302092236.GB30065@sigill.intra.peff.net","threadId":"41586","inReplyTo":"xmqqd1rd7uey.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] bundle doc: 'verify' is not about verifying the bundle","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-03-02T09:22:36Z","receivedAt":"2016-03-02T09:22:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 01, 2016 at 01:36:05PM -0800, Junio C Hamano wrote:\n\n> Even though the command does read the bundle header and checks to\n> see if it looks reasonable, the thin-pack data stream that follows\n> the header in the bundle file is not checked. More importantly,\n> because the thin-pack data does not have a trailing checksum like\n> on-disk packfiles do, there isn't much \"verification\" the command\n> can do without unpacking the objects from the stream even if it\n> wanted to.\n\nAre you sure about that trailing checksum thing?\n\n $ git tag foo $(echo foo | git hash-object -w --stdin)\n $ git bundle create foo.bundle foo\n $ perl -ne '\n BEGIN { binmode STDIN; binmode STDOUT }\n $print = 1 if /^PACK/;\n print if $print;\n ' foo.pack\n\n $ tail -c 20 foo.pack | xxd -p -c 20\n aa4e77be3695e8003bf4fbd2139b3fe0a9a22c8f\n\n $ git index-pack --stdin The documentation gives an incorrect impression that the thin-pack\n> data contained in the bundle is validated, but the command is to\n> validate that the receiving repository is ready to accept the\n> bundle, not to check the validity of a bundle file. Rephrase the\n> paragraph to clarify this.\n\nI still think this is a good change, though. \"verify\" is doing a\nseparate thing here than asking \"did I get bit errors\". We might _also_\nwant that check, but it is probably less interesting than \"does the\nbundle have all of the objects it claims to\". And you can only find that\nby expanding the objects, which would _also_ notice bit errors.\n\nAnd the simplest way to do that is \"git bundle apply\". :)\n\n-Peff\n"},{"id":"280079","messageId":"xmqqbn6w4ubj.fsf@gitster.mtv.corp.google.com","threadId":"41586","inReplyTo":"20160302092236.GB30065@sigill.intra.peff.net","subject":"Re: [PATCH] bundle doc: 'verify' is not about verifying the bundle","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-02T18:18:40Z","receivedAt":"2016-03-02T18:18:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Tue, Mar 01, 2016 at 01:36:05PM -0800, Junio C Hamano wrote:\n>\n>> Even though the command does read the bundle header and checks to\n>> see if it looks reasonable, the thin-pack data stream that follows\n>> the header in the bundle file is not checked. More importantly,\n>> because the thin-pack data does not have a trailing checksum like\n>> on-disk packfiles do, there isn't much \"verification\" the command\n>> can do without unpacking the objects from the stream even if it\n>> wanted to.\n>\n> Are you sure about that trailing checksum thing?\n\nNo. I misread the fact that we do not say the final csum on the\ncommand output from pack-objects in the --stdout case. We do call\nthe sha1close() to append the csum at the end of the pack stream;\nwe just do not make it available to the caller who is driving the\npack-object procedure.\n"}]}