{"thread":{"id":"7158","subject":"[PATCH 2/3] git-bundle: die if a given ref is not included in bundle","startedAt":"2007-03-09T02:48:46Z","lastAt":"2007-03-13T03:16:16Z","messageCount":21,"participants":["Johannes Schindelin","Mark Levedahl","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"36660","messageId":"Pine.LNX.4.63.0703090348300.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"7158","inReplyTo":null,"subject":"[PATCH 2/3] git-bundle: die if a given ref is not included in bundle","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-03-09T02:48:46Z","receivedAt":"2007-03-09T02:48:46Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nThe earlier patch tried to be nice by just warning, but it seems\nmore likely that the user wants to adjust the parameters.\n\nAlso, it prevents a bundle containing _all_ revisions in the case\nwhen the user only gave one ref, but also rev-list options which\nexcluded the ref.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n\n\tIt really was a dumb idea not to error out there. Sorry.\n\n builtin-bundle.c |    6 ++----\n 1 files changed, 2 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin-bundle.c b/builtin-bundle.c\nindex 33b533f..ca3de60 100644\n--- a/builtin-bundle.c\n+++ b/builtin-bundle.c\n@@ -328,11 +328,9 @@ static int create_bundle(struct bundle_header *header, const char *path,\n \t\t * other limiting options could have prevented all the tips\n \t\t * from getting output.\n \t\t */\n-\t\tif (!(e->item->flags & SHOWN)) {\n-\t\t\twarn(\"ref '%s' is excluded by the rev-list options\",\n+\t\tif (!(e->item->flags & SHOWN))\n+\t\t\tdie(\"ref '%s' is excluded by the rev-list options\",\n \t\t\t\te->name);\n-\t\t\tcontinue;\n-\t\t}\n \t\twrite_or_die(bundle_fd, sha1_to_hex(e->item->sha1), 40);\n \t\twrite_or_die(bundle_fd, \" \", 1);\n \t\twrite_or_die(bundle_fd, ref, strlen(ref));\n-- \n1.5.0.3.2601.gc1e5-dirty\n"},{"id":"36664","messageId":"45F0D1B3.9020204@gmail.com","threadId":"7158","inReplyTo":"Pine.LNX.4.63.0703090348300.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2007-03-09T03:17:07Z","receivedAt":"2007-03-09T03:17:07Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Johannes Schindelin wrote:\n> The earlier patch tried to be nice by just warning, but it seems\n> more likely that the user wants to adjust the parameters.\n>\n> Also, it prevents a bundle containing _all_ revisions in the case\n> when the user only gave one ref, but also rev-list options which\n> excluded the ref.\n>\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n>\n>   \nThis patch makes git-bundle greatly complicates one of my primary uses: \na nightly update generated and emailed to other users. With this, I need \nto write code to separately explore each branch in the repository to \nfind what changed, then include only those. Without, I can just do git \nbundle ... <list of refs> and those which have been updated get included.\n\nSo, I would really like an option to error out only if the bundle would \nbe empty.\n\nMark\n"},{"id":"36665","messageId":"11734123161927-git-send-email-mdl123@verizon.net","threadId":"7158","inReplyTo":"Pine.LNX.4.63.0703090348300.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"[PATCH] git-bundle: die if the bundle is empty.","fromName":"Mark Levedahl","fromEmail":"mdl123@verizon.net","sentAt":"2007-03-09T03:51:56Z","receivedAt":"2007-03-09T03:51:56Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"A common use of bundle is to schedule nightly emailed updates to a remote\nsystem containing all updates made over a given period. We want bundle\nto complain only if the resulting bundle would be empty, but not if at\nleast one of the given refs has an update resulting in a non-empty bundle.\n\nSigned-off-by: Mark Levedahl <mdl123@verizon.net>\n---\n builtin-bundle.c |   17 +++++++++--------\n 1 files changed, 9 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin-bundle.c b/builtin-bundle.c\nindex c85f996..b888a9f 100644\n--- a/builtin-bundle.c\n+++ b/builtin-bundle.c\n@@ -261,6 +261,7 @@ static int create_bundle(struct bundle_header *header, const char *path,\n \t\tint argc, const char **argv)\n {\n \tint bundle_fd = -1;\n+\tint isempty = 1;\n \tconst char **argv_boundary = xmalloc((argc + 4) * sizeof(const char *));\n \tconst char **argv_pack = xmalloc(5 * sizeof(const char *));\n \tint pid, in, out, i, status;\n@@ -328,17 +329,17 @@ static int create_bundle(struct bundle_header *header, const char *path,\n \t\t * other limiting options could have prevented all the tips\n \t\t * from getting output.\n \t\t */\n-\t\tif (!(e->item->flags & SHOWN)) {\n-\t\t\twarn(\"ref '%s' is excluded by the rev-list options\",\n-\t\t\t\te->name);\n-\t\t\tcontinue;\n+\t\tif ((e->item->flags & SHOWN)) {\n+\t\t\tisempty = 0;\n+\t\t\twrite_or_die(bundle_fd, sha1_to_hex(e->item->sha1), 40);\n+\t\t\twrite_or_die(bundle_fd, \" \", 1);\n+\t\t\twrite_or_die(bundle_fd, ref, strlen(ref));\n+\t\t\twrite_or_die(bundle_fd, \"\\n\", 1);\n \t\t}\n-\t\twrite_or_die(bundle_fd, sha1_to_hex(e->item->sha1), 40);\n-\t\twrite_or_die(bundle_fd, \" \", 1);\n-\t\twrite_or_die(bundle_fd, ref, strlen(ref));\n-\t\twrite_or_die(bundle_fd, \"\\n\", 1);\n \t\tfree(ref);\n \t}\n+\tif (isempty)\n+\t\tdie(\"Bundle is empty\");\n \n \t/* end header */\n \twrite_or_die(bundle_fd, \"\\n\", 1);\n-- \n1.5.0.3.927.g2432c-dirty\n"},{"id":"36667","messageId":"7vejny7umx.fsf@assigned-by-dhcp.cox.net","threadId":"7158","inReplyTo":"45F0D1B3.9020204@gmail.com","subject":"Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-09T07:17:58Z","receivedAt":"2007-03-09T07:17:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mark Levedahl <mlevedahl@gmail.com> writes:\n\n> ... Without, I\n> can just do git bundle ... <list of refs> and those which have been\n> updated get included.\n\nI am not sure if the above is true.  How are you preventing the\ncommand from bundling everything?  You must have some limiter at\nthe bottom, something like --since=25.hours (to account for cron\nschedule skew), not just <list of refs>.\n\nI do not think the max-age limiter works as you are expecting,\nespecially when merges and clock skew among committing machines\nare involved (just as you are distributing from central repo, I\nam assuming you are getting work back from worker-bee machines\nto the central repo at some point, and doing sneakernet implies\nto me that they are disconnected machines, not running ntp, but\nthis is just me guessing).\n\nIn any case, the semantics of --since=25.hours limiter is not\n\"show everything newer than 25.hours that are reachable from any\nof these refs\"; it is \"start digging from these tips, and stop\nexploring the path as soon as you hit something that is newer\nthan 25.hours\".\n\nIt appears that for the past few days we have been spending\nsignificant effort to make --max-count and --max-age work with\nbundles, but my honest impression is they do not play well in\nthe real world, especially when clock skew is involved.  On the\nother hand, revision ranges (\"include these, exclude those\") are\nalways precise, and that is what you would want to be using,\nespecially from an automated script.\n\nIf I were doing a nightly script, I would probably be doing\nsomething like this:\n\n\t#!/bin/sh\n\tyesterday=$(git bundle list-heads yesterday.bdl | sed -e 's/ .*//')\n\tgit bundle create today.bdl --all --not $yesterday\n\t# mail it out\n\nAfter sending today's bundle out, you will rotate it out to\nyesterday.bdl in order to prepare for the next round.  It is\nlikely that you would want to keep a few day's worth of bundles\nfor other reasons _anyway_ (say, some project members might be\nout of office, or mail gets dropped, or whatever), I think the\nabove is a reasonably clean and easy thing to arrange.\n"},{"id":"36674","messageId":"45F163B4.2000905@gmail.com","threadId":"7158","inReplyTo":"7vejny7umx.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2007-03-09T13:40:04Z","receivedAt":"2007-03-09T13:40:04Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Junio C Hamano wrote:\n> Mark Levedahl <mlevedahl@gmail.com> writes:\n> I am not sure if the above is true.  How are you preventing the\n> command from bundling everything?  You must have some limiter at\n> the bottom, something like --since=25.hours (to account for cron\n> schedule skew), not just <list of refs>.\n>   \nSorry, I trimmed too much. My typical command usage is something like\n    git bundle create foo --since=10.days.ago --all\n\nI include a very generous date range so that folks don't have to update \ndaily, can cover a vacation with a single bundle, etc. I think this \nusage renders moot all practical issues with clock skew. I am being \nloose, if the bundle won't apply then get one from the previous week, \napply, and go forward.\n\nBTW, shouldn't git check for clock skew when creating a commit to assure \nthe parents predate the child? Clock skew could allow this circumstance \nwhich would look suspicious when exploring a history.\n> In any case, the semantics of --since=25.hours limiter is not\n> \"show everything newer than 25.hours that are reachable from any\n> of these refs\"; it is \"start digging from these tips, and stop\n> exploring the path as soon as you hit something that is newer\n> than 25.hours\".\n>   \nI presume you mean \"older than 2.5\" hours in the above.\n\n> If I were doing a nightly script, I would probably be doing\n> something like this:\n>\n> \t#!/bin/sh\n> \tyesterday=$(git bundle list-heads yesterday.bdl | sed -e 's/ .*//')\n> \tgit bundle create today.bdl --all --not $yesterday\n> \t# mail it out\n>\n> After sending today's bundle out, you will rotate it out to\n> yesterday.bdl in order to prepare for the next round.  It is\n> likely that you would want to keep a few day's worth of bundles\n> for other reasons _anyway_ (say, some project members might be\n> out of office, or mail gets dropped, or whatever), I think the\n> above is a reasonably clean and easy thing to arrange.\n>\n>   \nThis is certainly a reliable method, but has the difficulty of how to \nget started and of course requires that history be kept for the entire \nrange covered by each bundle. The first bundle is either a) the entire \nrepository, or b) crafted by trying each and every ref to find which are \nlegal to define. While all of this can be done, I think this cure is \nworse than the disease (seconds to minutes of clock skew).\n\nI would prefer to just add a warning to the manual that when limiting by \ndate, be generous to allow for all possible clock skew across the \ndistributed set of computers.\n\nMark\n"},{"id":"36677","messageId":"45F17EF0.5060008@gmail.com","threadId":"7158","inReplyTo":"7vejny7umx.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2007-03-09T15:36:16Z","receivedAt":"2007-03-09T15:36:16Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Junio C Hamano wrote:\n> If I were doing a nightly script, I would probably be doing\n> something like this:\n>\n> \t#!/bin/sh\n> \tyesterday=$(git bundle list-heads yesterday.bdl | sed -e 's/ .*//')\n> \tgit bundle create today.bdl --all --not $yesterday\n> \t# mail it out\nThinking about this further, the above has a problem (or should, but see \nbelow). Consider a case where master is not updated since yesterday. \nEffectively, the above becomes\n\ngit bundle create today.bdl  master <other-refs> --not master <other-refs>\n\nAs ref master is excluded, the bundle creation should die because master \ncannot be included. Experimenting with next (299fcfbdcb5afd85) however, \nI get:\n\ngit>git bundle create t.bdl master --not master\nGenerating pack...\nDone counting 0 objects.\nWriting 0 objects.\nTotal 0 (delta 0), reused 0 (delta 0)\ngit>git ls-remote t.bdl\ngit>\n\ne.g, an empty bundle is created without any error or warning. This is \nthe one case I believe an error should result: there is no use to \nsending (or even creating) an empty bundle.\n\nAs a date limited bundle containing all updated refs is my basic use, I \nreally want this case to not be hard, and it definitely should not \nrequire externally maintained history or scripting to create. Absent the \n\"die if any ref wasn't updated in the given date range\" logic, and \nadding always die if the resulting bundle is empty, git bundle in next \naccomplishes what I want.\n\nMark\n"},{"id":"36681","messageId":"Pine.LNX.4.63.0703091726530.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"7158","inReplyTo":"45F17EF0.5060008@gmail.com","subject":"[PATCH] git-bundle: only die if pack would be empty, warn if ref is skipped","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-03-09T16:30:15Z","receivedAt":"2007-03-09T16:30:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nA use case for git-bundle expected to be quite common is this:\n\n\t$ git bundle create daily.bundle --since=10.days.ago --all\n\nThe expected outcome is _not_ to error out if only a couple of the\nrefs were not changed during the last 10 days.\n\nThis patch complains loudly about refs which are skipped due to the\npack not containing the corresponding objects, but dies only if\nno objects would be in the pack _at all_.\n\nSigned-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\n---\n\n\tOn Fri, 9 Mar 2007, Mark Levedahl wrote:\n\n\t> Junio C Hamano wrote:\n\t> > If I were doing a nightly script, I would probably be doing\n\t> > something like this:\n\t> > \n\t> > \t#!/bin/sh\n\t> > \tyesterday=$(git bundle list-heads yesterday.bdl | sed -e 's/ .*//')\n\t> > \tgit bundle create today.bdl --all --not $yesterday\n\t> > \t# mail it out\n\t>\n\t> Thinking about this further, the above has a problem (or should, \n\t> but see below). [...]\n\n\tI see another problem, too: if at least one ref was not updated \n\tsince yesterday, \"create\" would fail with the latest patches.\n\n\tThis fixes it.\n\n\tBTW I had a little laugh when seeing what git-describe made of my \n\tcurrent version :-)\n\n builtin-bundle.c |   11 ++++++++---\n 1 files changed, 8 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-bundle.c b/builtin-bundle.c\nindex 55f6d0a..7868080 100644\n--- a/builtin-bundle.c\n+++ b/builtin-bundle.c\n@@ -263,7 +263,7 @@ static int create_bundle(struct bundle_header *header, const char *path,\n \tint bundle_fd = -1;\n \tconst char **argv_boundary = xmalloc((argc + 4) * sizeof(const char *));\n \tconst char **argv_pack = xmalloc(5 * sizeof(const char *));\n-\tint pid, in, out, i, status;\n+\tint pid, in, out, i, status, ref_count = 0;\n \tchar buffer[1024];\n \tstruct rev_info revs;\n \n@@ -328,15 +328,20 @@ static int create_bundle(struct bundle_header *header, const char *path,\n \t\t * other limiting options could have prevented all the tips\n \t\t * from getting output.\n \t\t */\n-\t\tif (!(e->item->flags & SHOWN))\n-\t\t\tdie(\"ref '%s' is excluded by the rev-list options\",\n+\t\tif (!(e->item->flags & SHOWN)) {\n+\t\t\twarn(\"ref '%s' is excluded by the rev-list options\",\n \t\t\t\te->name);\n+\t\t\tcontinue;\n+\t\t}\n+\t\tref_count++;\n \t\twrite_or_die(bundle_fd, sha1_to_hex(e->item->sha1), 40);\n \t\twrite_or_die(bundle_fd, \" \", 1);\n \t\twrite_or_die(bundle_fd, ref, strlen(ref));\n \t\twrite_or_die(bundle_fd, \"\\n\", 1);\n \t\tfree(ref);\n \t}\n+\tif (!ref_count)\n+\t\tdie (\"Refusing to create empty bundle.\");\n \n \t/* end header */\n \twrite_or_die(bundle_fd, \"\\n\", 1);\n-- \n1.5.0.3.2621.gaaaa-dirty\n"},{"id":"36701","messageId":"7v1wjy56qf.fsf@assigned-by-dhcp.cox.net","threadId":"7158","inReplyTo":"45F17EF0.5060008@gmail.com","subject":"Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-09T23:37:12Z","receivedAt":"2007-03-09T23:37:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mark Levedahl <mlevedahl@gmail.com> writes:\n\n> git>git bundle create t.bdl master --not master\n> Generating pack...\n> Done counting 0 objects.\n> Writing 0 objects.\n> Total 0 (delta 0), reused 0 (delta 0)\n> git>git ls-remote t.bdl\n> git>\n>\n> e.g, an empty bundle is created without any error or warning. This is\n> the one case I believe an error should result: there is no use to\n> sending (or even creating) an empty bundle.\n\nI agree that erroring on an empty output is a sensible _option_\njust like pack-objects has --no-empty option.\n\nThe above is actually an interesting example in a different\nsense.  When somebody did the following, what should be output?\n\n\t$ edit; git commit -a ;# on master\n        $ git checkout -b side\n        $ edit; git commit -a ;# on side\n        $ git bundle create foo.bdl master side ^master\n\t$ git bundle verify foo.bdl\n\nMy answer is that it should list master and side as the\navailable heads and master itself as also a prerequisite (which\nis not what the current code does).  \n\nI think unbundling foo.bdl should be the moral equivalent of\nfetching from the originating repository by somebody who has its\nprerequisites as tips of some branches.  So I think foo.bdl\nshould list refs/heads/master as one of the available heads to\nfetch/pull from, while requiring the same commit as prerequisite\nof the bundle.  It is as if you tried \"git fetch\" and found out\nthat you are up to date.  Listing where the 'master' tip is,\neven though you did not have to include any commits from that\nbranch, gives you a useful bit of information (\"I am up to date\nwith respect to that branch\").\n\nSo I think if you did this instead in the above sequence: \n\n\t$ git bundle create foo.bdl master ^master\n\nit would be sensible to have an option to error out because of\nempty pack, but at the same time it would equally be sensible to\nhave an option to still create a bundle with an empty pack\ncontents.  In either case, the head and prerequisite section\nshould include the tip of the master.  Earlier, I said we should\nerror out if we do not find 'master' in the list of shown\nobjects, but I think it is more sensible to add it to both the\nlist of head _and_ prerequisites.\n"},{"id":"36727","messageId":"45F245D5.4030103@gmail.com","threadId":"7158","inReplyTo":"Pine.LNX.4.63.0703091726530.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] git-bundle: only die if pack would be empty, warn if ref is skipped","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2007-03-10T05:44:53Z","receivedAt":"2007-03-10T05:44:53Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Johannes Schindelin wrote:\n> A use case for git-bundle expected to be quite common is this:\n>\n> \t$ git bundle create daily.bundle --since=10.days.ago --all\n>\n>   \nThis patch works for me... Thanks.\n\nMark\n"},{"id":"36728","messageId":"45F246A4.2040304@gmail.com","threadId":"7158","inReplyTo":"7v1wjy56qf.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2007-03-10T05:48:20Z","receivedAt":"2007-03-10T05:48:20Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Junio C Hamano wrote:\n> My answer is that it should list master and side as the\n> available heads and master itself as also a prerequisite (which\n> is not what the current code does).\n>   \nSo, practically speaking the proposal is that for each ref where \ngit-bundle currently issues a warning, instead the ref should be added \nto the prerequisites list, and that all refs given on the command line \nare in the bundle's defined refs.\n\nSounds reasonable to me.\n\nMark\n"},{"id":"36745","messageId":"Pine.LNX.4.63.0703101637300.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"7158","inReplyTo":"7v1wjy56qf.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-03-10T15:39:16Z","receivedAt":"2007-03-10T15:39:16Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 9 Mar 2007, Junio C Hamano wrote:\n\n> Mark Levedahl <mlevedahl@gmail.com> writes:\n> \n> > git>git bundle create t.bdl master --not master\n> > Generating pack...\n> > Done counting 0 objects.\n> > Writing 0 objects.\n> > Total 0 (delta 0), reused 0 (delta 0)\n> > git>git ls-remote t.bdl\n> > git>\n> >\n> > e.g, an empty bundle is created without any error or warning. This is\n> > the one case I believe an error should result: there is no use to\n> > sending (or even creating) an empty bundle.\n> \n> I agree that erroring on an empty output is a sensible _option_\n> just like pack-objects has --no-empty option.\n> \n> The above is actually an interesting example in a different\n> sense.  When somebody did the following, what should be output?\n> \n> \t$ edit; git commit -a ;# on master\n>         $ git checkout -b side\n>         $ edit; git commit -a ;# on side\n>         $ git bundle create foo.bdl master side ^master\n\nIMHO saying \"master ^master\" should blow into the user's face. If she says \n\"I want it\" _and_ \"I don't want it\", she should sorta expect it not to \nwork.\n\nCiao,\nDscho\n"},{"id":"36746","messageId":"45F2D955.8050800@gmail.com","threadId":"7158","inReplyTo":"Pine.LNX.4.63.0703101637300.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2007-03-10T16:14:13Z","receivedAt":"2007-03-10T16:14:13Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Johannes Schindelin wrote:\n> Hi,\n>   \n> IMHO saying \"master ^master\" should blow into the user's face. If she says \n> \"I want it\" _and_ \"I don't want it\", she should sorta expect it not to \n> work.\n>\n> Ciao,\n> Dscho\n>\n>   \nThe command\n    git-bundle create foo next ^master\n\nis legitimate, even if next points to the same commit as master. The \ncurrent logic would reject this, and should not as we might want to push \nout the base of a new development branch in this manner. Consider that \ngit-fetch <url> would happily update next in this case, git bundle / \ngit-fetch should as well.\n\nI think we should think of the git-bundle command as accepting two lists \nof rev-args\n1 - the list of heads to define in the bundle (possibly --all, should \nalso accept refs/heads/*)\n2 - the list of commits to require as prerequisites for applying the \nbundle (possibly defined as --since=, possibly defined as a list of \ncommits, etc).\n\nAs long as the lists are syntactically acceptable (all exist), we should \njust create the bundle with the given refs and prerequisites. The \nresulting bundle will apply cleanly and conforms to general git \nsemantics. So, I think git-bundle should error out only if a ref does \nnot exist or if no refs are defined. An empty pack file is legitimate.\n\nMark\n"},{"id":"36747","messageId":"Pine.LNX.4.63.0703101749270.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"7158","inReplyTo":"45F2D955.8050800@gmail.com","subject":"Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-03-10T16:53:30Z","receivedAt":"2007-03-10T16:53:30Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 10 Mar 2007, Mark Levedahl wrote:\n\n> Johannes Schindelin wrote:\n>\n> > IMHO saying \"master ^master\" should blow into the user's face. If she \n> > says \"I want it\" _and_ \"I don't want it\", she should sorta expect it \n> > not to work.\n>\n> [...]\n>\n> As long as the lists are syntactically acceptable (all exist), we should \n> just create the bundle with the given refs and prerequisites.\n\nSo, what do you do if some of your users do, and some others do not, have \nthe \"blue-sky\" branch? If you say \"git bundle create new.bundle --all \n-10\", your bundle will list \"blue-sky\" as a prerequisite.\n\nBoom.\n\nSome of your users -- those without \"blue-sky\" -- will _not_ be able \nto fetch _anything_ from the bundle. They are lacking the prerequisites.\n\nThe semantics of git-bundle used to be so clear and sensible, since they \nexactly reflected what git-pack-objects would do.\n\nNow they are no longer?\n\nCiao,\nDscho\n"},{"id":"36754","messageId":"45F2F934.8060407@gmail.com","threadId":"7158","inReplyTo":"Pine.LNX.4.63.0703101749270.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2007-03-10T18:30:12Z","receivedAt":"2007-03-10T18:30:12Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Johannes Schindelin wrote:\n> Hi,\n>   \n> So, what do you do if some of your users do, and some others do not, have \n> the \"blue-sky\" branch? If you say \"git bundle create new.bundle --all \n> -10\", your bundle will list \"blue-sky\" as a prerequisite.\n>\n> Boom.\n>\n> Some of your users -- those without \"blue-sky\" -- will _not_ be able \n> to fetch _anything_ from the bundle. They are lacking the prerequisites.\n>   \nThose who have the prerequisites can apply the bundle. Those who do not, \ncannot. This is unchanged, and completely unrelated to whether the \nbundle defines 0 objects or 10,000. If you do not have the \nprerequisites, you need a different bundle.\n\nMark\n"},{"id":"36767","messageId":"Pine.LNX.4.63.0703110201450.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"7158","inReplyTo":"45F2F934.8060407@gmail.com","subject":"Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-03-11T01:08:04Z","receivedAt":"2007-03-11T01:08:04Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 10 Mar 2007, Mark Levedahl wrote:\n\n> Johannes Schindelin wrote:\n>\n> > So, what do you do if some of your users do, and some others do not, \n> > have the \"blue-sky\" branch? If you say \"git bundle create new.bundle \n> > --all -10\", your bundle will list \"blue-sky\" as a prerequisite.\n> > \n> > Boom.\n> > \n> > Some of your users -- those without \"blue-sky\" -- will _not_ be able \n> > to fetch _anything_ from the bundle. They are lacking the \n> > prerequisites.\n>\n> Those who have the prerequisites can apply the bundle. Those who do not, \n> cannot. This is unchanged, and completely unrelated to whether the \n> bundle defines 0 objects or 10,000. If you do not have the \n> prerequisites, you need a different bundle.\n\nOnly that I suspect that you want to stick more than one ref into the \nbundle. And at some point you -- or any other user -- will expect the \nbundle to work _with several refs_, even if different recipients will pick \n_only one_ of them.\n\nBasically, I am saying that this whole bundle concept is not thought \nthrough, that it is too loosely defined, and that it will result in unmet \nexpectations sooner or later. (Which usually means sooner.)\n\nSo, either we have to rethink how to handle prerequisites (so that only \nthose are checked which are strictly necessary for _the one_ ref you are \nupdating), or we have to make it _very_ obvious to (human) users of \ngit-bundle that you should _not_ bundle two unrelated -- or only remotely \nrelated -- refs into one bundle.\n\nCiao,\nDscho\n"},{"id":"36768","messageId":"7vy7m4y3cn.fsf@assigned-by-dhcp.cox.net","threadId":"7158","inReplyTo":"Pine.LNX.4.63.0703110201450.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-11T01:29:44Z","receivedAt":"2007-03-11T01:29:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Basically, I am saying that this whole bundle concept is not thought \n> through, that it is too loosely defined, and that it will result in unmet \n> expectations sooner or later. (Which usually means sooner.)\n\nEarlier I thought you said that bundle had a clearly defined\nsemantics, which I did not quite understand, but now you are\nagreeing with me...\n\n> So, either we have to rethink how to handle prerequisites (so that only \n> those are checked which are strictly necessary for _the one_ ref you are \n> updating), or we have to make it _very_ obvious to (human) users of \n> git-bundle that you should _not_ bundle two unrelated -- or only remotely \n> related -- refs into one bundle.\n\nI've been wondering if we can define prereqs per listed head.\n"},{"id":"36773","messageId":"Pine.LNX.4.63.0703110244130.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"7158","inReplyTo":"7vy7m4y3cn.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-03-11T01:54:22Z","receivedAt":"2007-03-11T01:54:22Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 10 Mar 2007, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> > Basically, I am saying that this whole bundle concept is not thought \n> > through, that it is too loosely defined, and that it will result in \n> > unmet expectations sooner or later. (Which usually means sooner.)\n> \n> Earlier I thought you said that bundle had a clearly defined semantics, \n> which I did not quite understand, but now you are agreeing with me...\n\ngit-bundle (as is in \"next\") has clearly defined semantics.\n\nBut the bundle concept is not thought through. Obviously, the clearly \ndefined semantics of git-bundle do not match what people want to use \nbundles for.\n\n> > So, either we have to rethink how to handle prerequisites (so that \n> > only those are checked which are strictly necessary for _the one_ ref \n> > you are updating), or we have to make it _very_ obvious to (human) \n> > users of git-bundle that you should _not_ bundle two unrelated -- or \n> > only remotely related -- refs into one bundle.\n> \n> I've been wondering if we can define prereqs per listed head.\n\nThat is a substantial change, and it makes creating a bundle potentially \nvery expensive. We would have to find out reachability for every boundary \ncommit from each ref.\n\nGranted, we could do that while finding the boundary objects, but we only \nhave 28-16 bits left to store from which ref the commit is reachable, \nwhich means only 12 refs, and if you have more (which is likely when you \nsay \"--all\" and have a reasonable number of tags) you need to allocate a \nbitfield for the remaining refs _for each_ traversed commit's parents.\n\nSo I wonder if there is not a better way.\n\nCiao,\nDscho\n"},{"id":"36808","messageId":"45F41787.4080506@gmail.com","threadId":"7158","inReplyTo":"Pine.LNX.4.63.0703110244130.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2007-03-11T14:51:51Z","receivedAt":"2007-03-11T14:51:51Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Johannes Schindelin wrote:\n> git-bundle (as is in \"next\") has clearly defined semantics.\n>   \ngit-bundle on next with the patch in \n<Pine.LNX.4.63.0703091726530.22628@wbgn013.biozentrum.uni-wuerzburg.de> \nworks well enough for me, but absent that latter patch is too punishing.\n> But the bundle concept is not thought through. Obviously, the clearly \n> defined semantics of git-bundle do not match what people want to use \n> bundles for.\n>\n>   \n>> I've been wondering if we can define prereqs per listed head.\n>>     \nCurrently, a bundle has a single pack. The bundle's prerequisites are \nthat pack's dependencies. Splitting prerequisites per head requires \neither creating one pack per head or unpacking at the receiving and and \nextracting only the objects needed for the selected head. I'm not sure \nthat either is warranted, and my uses of bundle do not require this \nregardless.\n\nI think we need to restate purpose here. git-bundle is an alternate \ntransport mechanism: git-bundle + git-fetch over sneakernet allows doing \nwhat git-push or git-fetch can do when directly connected. However, \nthere are limitations due to the lack of direct connect, specifically \nthe user of git-bundle needs to specify the prerequisites as the \nprotocol cannot negotiate these. The exchange needs to be robust in that \ngit-bundle+git-fetch must never result in leaving a repository in a \ncorrupted state: the current prerequisites list + use of git-fetch seem \nto satisfy this.\n\n From a given connected repo, I can do:\n\n    git fetch -f <source url> master:master\n\nand nothing complains, even if no update occurs (remote master is up to \ndate). I can also do\n        git fetch -f <source url> master:next\n\nand the new ref is created without complaint even if no new objects need \nto be defined or if the new definition is completely unrelated to the old.\n\nWith appropriate remote settings in .git/config, I can have git-fetch \nget all branches, or all branches and tags, and never complain when no \nupdate is required for something.\n\nWhat I desire is similar functionality across sneakernet, and this is \nwhere git-bundle steps in. I cannot know what is on the destination \nrepository exactly, so I need an imprecise way to specify prerequisites \n(e.g., --since=10.days.ago): I *know* that this is not exactly correct \nand *must* be conservative so that the bundle likely can be used. As the \nsystem is distributed and I don't control the recipients of the bundle, \nthere is *no* way to know exactly what exists, the previous bundle is \nnot definitive, it might not have been applied, they might have received \ndata by a side-channel communication, etc.\n\nSo, a date range is the best method I have found to specify a bundle's \nprerequisites. However, I should not have to know which refs have been \nupdated within the date range: this is too punishing. Directly connected \ngit-fetch does not abort when the refspec says \"get everything\" but some \nref in \"everything\" has not changed, why should git-bundle complain? \nAbsent the latest patch (i.e., what is now on next), git-bundle will \nerror out which is extremely unfriendly and unhelpful.\n\nThe single disconnect for the above with latest git-bundle + patch is \nthat we cannot package a ref whose commit object is directly a bundle \nprerequisite. (I cannot do the equivalent of \"git fetch remote \nmaster:next\", where I already have all the objects for master). These \ninstead result in a string of warning messages with the latest patch: I \ncan live with this limitation (though I don't think this should even be \na warning, git-fetch/git-push do not warn here). Absent the latest \npatch, git-bundle errors out: this is too punishing to the user.\n\nWhile it is possible to fetch a particular ref from the bundle rather \nthan taking all, the monolithic pack structure and protocol dictates \nthat you will get all objects regardless. I do not see this as a \nproblem: the bundle came from a single repository, everything in the \nbundle is therefore related, excess is easily trimmed by git-repack. \nThis is really just a limitation of the disconnected protocol that \ncannot optimize the pack for the exact transfer required.\n\nAt some point, we have to make a clear distinction between what rules \nthe protocol should enforce for \"correctness\" vs what an \"intelligent\" \nuse of bundle is, and not try to enforce the latter in the software. \nWhat practices are useful or good vary considerably from business to \nbusiness (I have many times been told that things I find essential to my \nwork are \"bad practice,\" usually stated by people who didn't have to \nsolve a problem given constraints I actually face). The only requests \ngit-bundle/git-fetch should refuse are things that will corrupt a \ngit-repository, and the pair should endeavor to enable any information \ntransfer that can be done with git-push or git-fetch given direct \nconnections.\n\nBottom line, I strongly advocate Dscho's last patch + what is on next be \npromoted to master. We can revisit how well it is working and refine it \nafter it gets some usage from others defining additional use-cases.\n\nMark\n"},{"id":"36844","messageId":"7vps7fwo09.fsf@assigned-by-dhcp.cox.net","threadId":"7158","inReplyTo":"45F41787.4080506@gmail.com","subject":"Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-11T19:58:46Z","receivedAt":"2007-03-11T19:58:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mark Levedahl <mlevedahl@gmail.com> writes:\n\n> Bottom line, I strongly advocate Dscho's last patch + what is on next\n> be promoted to master.\n\nI agree that is a very sensible thing to do.  Let's do that.\n"},{"id":"36878","messageId":"Pine.LNX.4.63.0703112302140.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"7158","inReplyTo":"45F41787.4080506@gmail.com","subject":"Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-03-11T22:28:33Z","receivedAt":"2007-03-11T22:28:33Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 11 Mar 2007, Mark Levedahl wrote:\n\n> Johannes Schindelin wrote:\n> > git-bundle (as is in \"next\") has clearly defined semantics.\n>\n> git-bundle on next with the patch in \n> <Pine.LNX.4.63.0703091726530.22628@wbgn013.biozentrum.uni-wuerzburg.de> \n> works well enough for me, but absent that latter patch is too punishing.\n\nI don't want to punish you! But I want to prevent \"Huh?\" moments early on.\n\n> > I've been wondering if we can define prereqs per listed head.\n>\n> Currently, a bundle has a single pack. The bundle's prerequisites \n> are that pack's dependencies. Splitting prerequisites per head requires \n> either creating one pack per head or unpacking at the receiving and and \n> extracting only the objects needed for the selected head. I'm not sure \n> that either is warranted, and my uses of bundle do not require this \n> regardless.\n\nYou can just store the pack and update the ref (at least as long as the \npack is not thin): we have a long-standing tradition of guaranteeing the \nintegrity of commit chains _only_ when the commits are reachable by at \nleast one ref.\n\n> I think we need to restate purpose here. git-bundle is an alternate \n> transport mechanism: git-bundle + git-fetch over sneakernet allows doing \n> what git-push or git-fetch can do when directly connected.\n\nBut would that not mean that we expect the user of the bundle to update \n_all_ refs contained in the bundle?\n\n> However, there are limitations due to the lack of direct connect, \n> specifically the user of git-bundle needs to specify the prerequisites \n> as the protocol cannot negotiate these. The exchange needs to be robust \n> in that git-bundle+git-fetch must never result in leaving a repository \n> in a corrupted state: the current prerequisites list + use of git-fetch \n> seem to satisfy this.\n\nYes. I think we definitely need to ensure the integrity _before_ updating \nthe refs (and not rely on the prereqs in the bundle). And yes, that means \nI changed my mind from when I argued against your use of fsck, and _for_ \nprereqs.\n\n> With appropriate remote settings in .git/config, I can have git-fetch \n> get all branches, or all branches and tags, and never complain when no \n> update is required for something.\n\nMakes sense.\n\n> While it is possible to fetch a particular ref from the bundle rather \n> than taking all, the monolithic pack structure and protocol dictates \n> that you will get all objects regardless.\n\n... but a subsequent git-gc will strip them away.\n\n> At some point, we have to make a clear distinction between what rules \n> the protocol should enforce for \"correctness\" vs what an \"intelligent\" \n> use of bundle is, and not try to enforce the latter in the software.\n\nConcur. But I think that we have to make sure that the \"correctness\" \nenforcing means that you do not end up with something that people want to \nuse for what it cannot be used for.\n\n> Bottom line, I strongly advocate Dscho's last patch + what is on next be \n> promoted to master. We can revisit how well it is working and refine it \n> after it gets some usage from others defining additional use-cases.\n\nFWIW my plans are to make the pack thin _only_ when there is only one \nprereq and/or ref in the bundle (this prevents a _wanted_ object being \ndeltified against a not-wanted object).\n\nAlso, as mentioned above, I think that we have to check that \"git rev-list \n--objects <new-refs> --not --all\" does not result in missing objects.\n\nCiao,\nDscho\n"},{"id":"36985","messageId":"45F61780.2090001@gmail.com","threadId":"7158","inReplyTo":"Pine.LNX.4.63.0703112302140.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2007-03-13T03:16:16Z","receivedAt":"2007-03-13T03:16:16Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Johannes Schindelin wrote:\n> FWIW my plans are to make the pack thin _only_ when there is only one \n> prereq and/or ref in the bundle (this prevents a _wanted_ object being \n> deltified against a not-wanted object).\n>\n>   \nI am not sure that this is really necessary or accomplishes additional \nsafety. The prerequisites must exist and be well connected in the target \nrepo before the pack file is indexed: presumably, the reference objects \nall exist if the checks hold, or there is a logic flaw in the thin-pack \ngeneration.\n\nIf the prereq test is removed, then avoiding a thin pack might allow the \npack file to be applied to a repo that held only the prereqs for a \nsingle head out of many in the bundle, but there is no info for the user \nto understand how or when to do this and I don't really think that is a \ngood practice to encourage. I suggest waiting for a well defined \nuse-case that really demands being able to apply only part of a pack \nfile before implementing.\n\n> Also, as mentioned above, I think that we have to check that \"git rev-list \n> --objects <new-refs> --not --all\" does not result in missing objects.\n>   \nThis is certainly a good safety check: even though the prereqs are \nsatisfied and all *should* be ok, some error might still exist and it is \nbetter to be safe.\n> Ciao,\n> Dscho\n>   \n"}]}