{"thread":{"id":"24845","subject":"reducing object store size with remote alternates or shallow clone?","startedAt":"2010-08-24T06:59:11Z","lastAt":"2010-09-09T18:56:37Z","messageCount":13,"participants":["Kumar Gala","Junio C Hamano","Brandon Casey","Jonathan Nieder"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"148825","messageId":"14526ED4-F65C-4DF2-ABDD-BF1E76DAC2B0@kernel.crashing.org","threadId":"24845","inReplyTo":null,"subject":"reducing object store size with remote alternates or shallow clone?","fromName":"Kumar Gala","fromEmail":"galak@kernel.crashing.org","sentAt":"2010-08-24T06:59:11Z","receivedAt":"2010-08-24T06:59:11Z","isPatch":false,"sender":{"key":"galak@kernel.crashing.org","avatar":null},"body":"I'm trying to package a linux kernel source tree and would like to just tar up a tree with .git/. However this is a bit problematic as the size of tgz is about 500M which exceeds the limit of the image I'm trying to produce.\n\nI was wondering if anyone had a means to reduce the size drastically. I'm ok if a post processing step is required to get full git support. One idea I had was doing a shallow clone and if there was some means to \"reconnect\" it to a full work state after the fact.\n\nthanks\n\n- k"},{"id":"148846","messageId":"7vhbikx8lu.fsf@alter.siamese.dyndns.org","threadId":"24845","inReplyTo":"14526ED4-F65C-4DF2-ABDD-BF1E76DAC2B0@kernel.crashing.org","subject":"Re: reducing object store size with remote alternates or shallow clone?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-24T16:45:33Z","receivedAt":"2010-08-24T16:45:33Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kumar Gala writes:\n\n> I'm trying to package a linux kernel source tree and would like to just\n> tar up a tree with .git/. However this is a bit problematic as the size\n> of tgz is about 500M which exceeds the limit of the image I'm trying to\n> produce.\n>\n> I was wondering if anyone had a means to reduce the size drastically.\n> I'm ok if a post processing step is required to get full git support.\n> One idea I had was doing a shallow clone and if there was some means to\n> \"reconnect\" it to a full work state after the fact.\n\nAlthough your message body does not state your design constraints clearly,\nyour subject line hints that you are fine with a solution that involves\nuse of remote alternates by the recipient of your tarball.\n\nI am _guessing_ that you have a fork from some well known tree and want to\nsneakernet it to your recipients (iow, they do not \"git pull\" from your\nrepository).\n\nHow about doing\n\n $ LINUS=git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git\n $ git fetch $LINUS\n $ git bundle create myfork.bundle HEAD..master\n\nand sending the thing over? The recipient would then do something like:\n\n $ LINUS=git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git\n $ git clone $LINUS linux\n $ cd linux\n $ git pull /tmp/myfork.bundle master\n"},{"id":"148855","messageId":"2dePxmEZeCbs4IkjGd-Ig2cma8lknd0XEwVdaypeAxtQDY3EZeSEPA@cipher.nrlssc.navy.mil","threadId":"24845","inReplyTo":"7vhbikx8lu.fsf@alter.siamese.dyndns.org","subject":"Re: reducing object store size with remote alternates or shallow clone?","fromName":"Brandon Casey","fromEmail":"brandon.casey.ctr@nrlssc.navy.mil","sentAt":"2010-08-24T18:15:58Z","receivedAt":"2010-08-24T18:15:58Z","isPatch":false,"sender":{"key":"brandon.casey.ctr@nrlssc.navy.mil","avatar":null},"body":"On 08/24/2010 11:45 AM, Junio C Hamano wrote:\n\n> How about doing\n> \n> $ LINUS=git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git\n\n> $ git fetch $LINUS\n> $ git bundle create myfork.bundle HEAD..master\n\nI think you mean\n\n $ git fetch $LINUS master\n $ git bundle create myfork.bundle FETCH_HEAD..master\n\n-Brandon\n"},{"id":"148858","messageId":"7vfwy3vnti.fsf@alter.siamese.dyndns.org","threadId":"24845","inReplyTo":"2dePxmEZeCbs4IkjGd-Ig2cma8lknd0XEwVdaypeAxtQDY3EZeSEPA@cipher.nrlssc.navy.mil","subject":"Re: reducing object store size with remote alternates or shallow clone?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-24T18:59:53Z","receivedAt":"2010-08-24T18:59:53Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Casey writes:\n\n> On 08/24/2010 11:45 AM, Junio C Hamano wrote:\n>\n>> How about doing\n>> \n>> $ LINUS=git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git\n>\n>> $ git fetch $LINUS\n>> $ git bundle create myfork.bundle HEAD..master\n>\n> I think you mean\n>\n> $ git fetch $LINUS master\n> $ git bundle create myfork.bundle FETCH_HEAD..master\n\nThanks, of course you are right.\n\nStrictly speaking, as I know there is only one branch in the repository of\nLinus, there is no need to say \"master\" when fetching, but it would be a\ngood discipline to explicitly specify what you mean on the command line.\n"},{"id":"148907","messageId":"pzml8liT3RErVlMrdxbSkHmhBs1RMvwYma9UXgvG6WY@cipher.nrlssc.navy.mil","threadId":"24845","inReplyTo":"7vfwy3vnti.fsf@alter.siamese.dyndns.org","subject":"Re: reducing object store size with remote alternates or shallow clone?","fromName":"Brandon Casey","fromEmail":"brandon.casey.ctr@nrlssc.navy.mil","sentAt":"2010-08-24T23:29:42Z","receivedAt":"2010-08-24T23:29:42Z","isPatch":false,"sender":{"key":"brandon.casey.ctr@nrlssc.navy.mil","avatar":null},"body":"On 08/24/2010 01:59 PM, Junio C Hamano wrote:\n> Brandon Casey writes:\n> \n>> On 08/24/2010 11:45 AM, Junio C Hamano wrote:\n>>\n>>> How about doing\n>>>\n>>> $ LINUS=git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git\n>>\n>>> $ git fetch $LINUS\n>>> $ git bundle create myfork.bundle HEAD..master\n>>\n>> I think you mean\n>>\n>> $ git fetch $LINUS master\n>> $ git bundle create myfork.bundle FETCH_HEAD..master\n> \n> Thanks, of course you are right.\n> \n> Strictly speaking, as I know there is only one branch in the repository of\n> Linus, there is no need to say \"master\" when fetching\n\nHmm. It appears that if the current checked-out branch has a configured\nmerge ref, then a fetch that supplies a repository url (not a remote name)\nand no fetch refspec, will not fall back to fetch HEAD from the remote\nrepository.\n\ni.e. the following fetch does not retrieve any objects nor update FETCH_HEAD\n\n $ git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git linux\n $ cd linux\n $ git fetch git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git\n\nbut, if you create a new branch, that has no merge ref configuration, then\ngit behaves as expected:\n\n $ git branch foo\n $ git checkout foo\n $ git fetch git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git\n\nNamely we retrieve new objects and update FETCH_HEAD:\n\n From git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6\n * branch HEAD -> FETCH_HEAD\n\n\nI think the problem is in builtin/fetch.c: get_ref_map().\n\nWhen fetch is called as above, with a repository url but no refspec,\nwe get this call sequence:\n\n cmd_fetch -> fetch_one\n fetch_one -> do_fetch(argc = 0)\n do_fetch -> get_ref_map(ref_count = 0)\n line 148: has_merge is assigned 1 since the current checked out\n branch has a merge ref configured\n The 'if' branch is entered, the 'for' loop is not entered,\n ref_map retains its NULL initialization value and\n get_ref_map() returns NULL\n do_fetch -> fetch_refs(ref_map = NULL)\n and the transports do nothing since no refs have been requested\n\nPerhaps the fix should look something like this (warning copy/paste):\n\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex fab3fce..218e71d 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -146,7 +146,8 @@ static struct ref *get_ref_map(struct transport *transport,\n struct remote *remote = transport->remote;\n struct branch *branch = branch_get(NULL);\n int has_merge = branch_has_merge_config(branch);\n- if (remote && (remote->fetch_refspec_nr || has_merge)) {\n+ if (remote && (remote->fetch_refspec_nr || (has_merge &&\n+ !strcmp(branch->remote_name, remote->name)))) {\n for (i = 0; i < remote->fetch_refspec_nr; i++) {\n get_fetch_map(remote_refs, &remote->fetch[i], &tail, 0);\n if (remote->fetch[i].dst &&\n\n\n-Brandon\n"},{"id":"148977","messageId":"O7UxM6KEqdDAhjJAF7ODSonSLShoyHHhaZNp8vb1mx2_JFqmMUj1Op5xiv_-bSd8xW04rLMul2k@cipher.nrlssc.navy.mil","threadId":"24845","inReplyTo":"pzml8liT3RErVlMrdxbSkHmhBs1RMvwYma9UXgvG6WY@cipher.nrlssc.navy.mil","subject":"[PATCH 1/2] t/t5510: demonstrate failure to fetch when current branch has merge ref","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-08-25T17:52:55Z","receivedAt":"2010-08-25T17:52:55Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey \n\nWhen 'git fetch' is supplied just a repository URL (not a remote name),\nand without a fetch refspec, it should fetch from the remote HEAD branch\nand update FETCH_HEAD with the fetched ref. Currently, when 'git fetch'\nis called like this, it fails to retrieve anything, and does not update\nFETCH_HEAD, if the current checked-out branch has a configured merge ref.\n\ni.e. this fetch fails to retrieve anything nor update FETCH_HEAD:\n\n git checkout master\n git config branch.master.merge refs/heads/master\n git fetch git://git.kernel.org/pub/scm/git/git.git\n\nbut this one does:\n\n git config --unset branch.master.merge\n git fetch git://git.kernel.org/pub/scm/git/git.git\n\nAdd a test to demonstrate this flaw.\n\nSigned-off-by: Brandon Casey \n---\n t/t5510-fetch.sh | 6 ++++++\n 1 files changed, 6 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex 4eb10f6..3c7569c 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -240,6 +240,12 @@ test_expect_success 'fetch with a non-applying branch..merge' '\n \tgit fetch blub\n '\n \n+test_expect_failure 'fetch from GIT URL with a non-applying branch..merge' '\n+\tgit update-ref -d FETCH_HEAD &&\n+\tgit fetch one &&\n+\tgit rev-parse --verify FETCH_HEAD\n+'\n+\n # the strange name is: a\\!'b\n test_expect_success 'quoting of a strangely named repo' '\n \ttest_must_fail git fetch \"a\\\\!'\\''b\" > result 2>&1 &&\n-- \n1.7.2.1\n"},{"id":"148976","messageId":"O7UxM6KEqdDAhjJAF7ODSlo_kZavb8gBCJ6laH3QPOlG9a1q29koMQOkS7wDMj0BpyrLYfAcEh4@cipher.nrlssc.navy.mil","threadId":"24845","inReplyTo":"pzml8liT3RErVlMrdxbSkHmhBs1RMvwYma9UXgvG6WY@cipher.nrlssc.navy.mil","subject":"[PATCH 2/2] builtin/fetch.c: ignore merge config when not fetching from branch's remote","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-08-25T17:52:56Z","receivedAt":"2010-08-25T17:52:56Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey \n\nWhen 'git fetch' is supplied a single argument, it tries to match it\nagainst a configured remote and then fetch the refs specified by the\nnamed remote's fetchspec. Additionally, or alternatively, if the current\nbranch has a merge ref configured, and if the name of the remote supplied\nto fetch matches the one in the branch's configuration, then git also adds\nthe merge ref to the list of refs to update.\n\nIf the argument to fetch does not specify a named remote, or if the name\nsupplied does not match the remote configured for the current branch, then\nthe current branch's merge configuration should not be considered.\n\ngit currently mishandles the case when the argument to fetch specifies a\nGIT URL(i.e. not a named remote) and the current branch has a configured\nmerge ref. In this case, fetch should ignore the branch's merge ref and\nattempt to fetch from the remote repository's HEAD branch. But, since\nfetch only checks _whether_ the current branch has a merge ref configured,\nand does _not_ check whether the branch's configured remote matches the\ncommand line argument (until later), it will mistakenly enter the wrong\nbranch of an 'if' statement and will not fall back to fetch the HEAD branch.\nThe fetch ends up doing nothing and returns with a successful zero status.\n\nFix this by comparing the remote repository's name to the branch's remote\nname, in addition to whether it has a configured merge ref, sooner, so that\nfetch can correctly decide whether the branch's configuration is interesting\nor not, and fall back to fetching from the remote's HEAD branch when\nappropriate.\n\nThis fixes the test in t5510.\n\nSigned-off-by: Brandon Casey \n---\n builtin/fetch.c | 3 ++-\n t/t5510-fetch.sh | 2 +-\n 2 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex ea14d5d..be6c27a 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -146,7 +146,8 @@ static struct ref *get_ref_map(struct transport *transport,\n \t\tstruct remote *remote = transport->remote;\n \t\tstruct branch *branch = branch_get(NULL);\n \t\tint has_merge = branch_has_merge_config(branch);\n-\t\tif (remote && (remote->fetch_refspec_nr || has_merge)) {\n+\t\tif (remote && (remote->fetch_refspec_nr || (has_merge &&\n+\t\t\t\t!strcmp(branch->remote_name, remote->name)))) {\n \t\t\tfor (i = 0; i < remote->fetch_refspec_nr; i++) {\n \t\t\t\tget_fetch_map(remote_refs, &remote->fetch[i], &tail, 0);\n \t\t\t\tif (remote->fetch[i].dst &&\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex 3c7569c..8fbd894 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -240,7 +240,7 @@ test_expect_success 'fetch with a non-applying branch..merge' '\n \tgit fetch blub\n '\n \n-test_expect_failure 'fetch from GIT URL with a non-applying branch..merge' '\n+test_expect_success 'fetch from GIT URL with a non-applying branch..merge' '\n \tgit update-ref -d FETCH_HEAD &&\n \tgit fetch one &&\n \tgit rev-parse --verify FETCH_HEAD\n-- \n1.7.2.1\n"},{"id":"149000","messageId":"20100825211641.GC2319@burratino","threadId":"24845","inReplyTo":"O7UxM6KEqdDAhjJAF7ODSlo_kZavb8gBCJ6laH3QPOlG9a1q29koMQOkS7wDMj0BpyrLYfAcEh4@cipher.nrlssc.navy.mil","subject":"Re: [PATCH 2/2] builtin/fetch.c: ignore merge config when not fetching from branch's remote","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-25T21:16:41Z","receivedAt":"2010-08-25T21:16:41Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Brandon Casey wrote:\n\n> If the argument to fetch does not specify a named remote, or if the name\n> supplied does not match the remote configured for the current branch, then\n> the current branch's merge configuration should not be considered.\n\nThanks for a fix.\n\n> +++ b/builtin/fetch.c\n> @@ -146,7 +146,8 @@ static struct ref *get_ref_map(struct transport *transport,\n> \t\tstruct remote *remote = transport->remote;\n> \t\tstruct branch *branch = branch_get(NULL);\n> \t\tint has_merge = branch_has_merge_config(branch);\n> -\t\tif (remote && (remote->fetch_refspec_nr || has_merge)) {\n> +\t\tif (remote && (remote->fetch_refspec_nr || (has_merge &&\n> +\t\t\t\t!strcmp(branch->remote_name, remote->name)))) {\n\nWhat will happen with this (invalid) branch?\n\n\t[branch \"tmp\"]\n\t\tmerge = refs/heads/tmp\n"},{"id":"149001","messageId":"7vy6bupeko.fsf@alter.siamese.dyndns.org","threadId":"24845","inReplyTo":"O7UxM6KEqdDAhjJAF7ODSonSLShoyHHhaZNp8vb1mx2_JFqmMUj1Op5xiv_-bSd8xW04rLMul2k@cipher.nrlssc.navy.mil","subject":"Re: [PATCH 1/2] t/t5510: demonstrate failure to fetch when current branch has merge ref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-25T21:28:23Z","receivedAt":"2010-08-25T21:28:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Casey writes:\n\n> From: Brandon Casey \n>\n> When 'git fetch' is supplied just a repository URL (not a remote name),\n> and without a fetch refspec, it should fetch from the remote HEAD branch\n> and update FETCH_HEAD with the fetched ref. Currently, when 'git fetch'\n> is called like this, it fails to retrieve anything, and does not update\n> FETCH_HEAD, if the current checked-out branch has a configured merge ref.\n>\n> i.e. this fetch fails to retrieve anything nor update FETCH_HEAD:\n>\n> git checkout master\n> git config branch.master.merge refs/heads/master\n> git fetch git://git.kernel.org/pub/scm/git/git.git\n\nHmph, we can call it a regression, since we certainly won't see this\nfailure with versions of git that is unaware of branch.*.merge.\n\nBut what should we be expecting?\n\nJust as a datapoint, an ancient git (e.g. v1.4.0), the above command line\nwould have fetched the HEAD from the remote side, no matter what that\nsymref is pointing at. Your [2/2] patch replicates this behaviour, which\nis fine by me [*1*].\n\nYour test only checks if we leave _anything_ in FETCH_HEAD, and does not\ncheck if we only fetch one, if we fetch all the refs, or if we fetch what\nthe configuration branch.*.merge asks for (but without the corresponding\nbranch.*.remote configuration, doing so is pointless).\n\nI think it would be better to have two tests. One arranges the current\nbranch to track the same branch the HEAD at the remote points at, and the\nother arranges the current branch to track a branch different from the\nHEAD at the remote points at. In both cases, as \"fetch\" should ignore the\nconfiguration, we should get the object pointed by the HEAD on the remote\nside.\n\nThanks.\n\n\n[Footnote]\n\n*1* A plausible alternative is to match the given URL against list of\nexisting remote..url (make sure there is only one), and behave as if\nthat the remote name is given. I can be persuaded either way, but not\nlooking at the configuration feels a lot simpler to explain and understand\n(i.e. \"with name, we use the set of configuration variable given to that\nname; without name, there is no configuration for us to look up\").\n"},{"id":"149004","messageId":"TDHEkC5Y5b2p-yjw_5mixlQb49a7TMinj2d5CgfEHHH9Dq6lNFV7dQ@cipher.nrlssc.navy.mil","threadId":"24845","inReplyTo":"20100825211641.GC2319@burratino","subject":"Re: [PATCH 2/2] builtin/fetch.c: ignore merge config when not fetching from branch's remote","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-08-25T21:41:02Z","receivedAt":"2010-08-25T21:41:02Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"On 08/25/2010 04:16 PM, Jonathan Nieder wrote:\n> Brandon Casey wrote:\n> \n>> If the argument to fetch does not specify a named remote, or if the name\n>> supplied does not match the remote configured for the current branch, then\n>> the current branch's merge configuration should not be considered.\n> \n> Thanks for a fix.\n> \n>> +++ b/builtin/fetch.c\n>> @@ -146,7 +146,8 @@ static struct ref *get_ref_map(struct transport *transport,\n>> \t\tstruct remote *remote = transport->remote;\n>> \t\tstruct branch *branch = branch_get(NULL);\n>> \t\tint has_merge = branch_has_merge_config(branch);\n>> -\t\tif (remote && (remote->fetch_refspec_nr || has_merge)) {\n>> +\t\tif (remote && (remote->fetch_refspec_nr || (has_merge &&\n>> +\t\t\t\t!strcmp(branch->remote_name, remote->name)))) {\n> \n> What will happen with this (invalid) branch?\n> \n> \t[branch \"tmp\"]\n> \t\tmerge = refs/heads/tmp\n\nThe same thing that would have happened before, since a few lines\nfurther down there is this:\n\n if (has_merge &&\n !strcmp(branch->remote_name, remote->name))\n add_merge_config(&ref_map, remote_refs, branch, &tail);\n\nI didn't trace branch_get() to check whether it returns an object\nwith remote_name initialized in all cases. I relied on the form\nof the existing code. Perhaps it's worth investigating. If something\nneeds to be fixed, then it was already broken and deserves a separate\npatch anyway.\n\n-Brandon\n"},{"id":"149005","messageId":"7vtymipddv.fsf@alter.siamese.dyndns.org","threadId":"24845","inReplyTo":"O7UxM6KEqdDAhjJAF7ODSlo_kZavb8gBCJ6laH3QPOlG9a1q29koMQOkS7wDMj0BpyrLYfAcEh4@cipher.nrlssc.navy.mil","subject":"Re: [PATCH 2/2] builtin/fetch.c: ignore merge config when not fetching from branch's remote","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-25T21:54:04Z","receivedAt":"2010-08-25T21:54:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Casey writes:\n\n> diff --git a/builtin/fetch.c b/builtin/fetch.c\n> index ea14d5d..be6c27a 100644\n> --- a/builtin/fetch.c\n> +++ b/builtin/fetch.c\n> @@ -146,7 +146,8 @@ static struct ref *get_ref_map(struct transport *transport,\n> \t\tstruct remote *remote = transport->remote;\n> \t\tstruct branch *branch = branch_get(NULL);\n> \t\tint has_merge = branch_has_merge_config(branch);\n> -\t\tif (remote && (remote->fetch_refspec_nr || has_merge)) {\n> +\t\tif (remote && (remote->fetch_refspec_nr || (has_merge &&\n> +\t\t\t\t!strcmp(branch->remote_name, remote->name)))) {\n\nCouldn't branch->remote_name or remote->name be NULL here?\n\nI think remote->name would be the same as the URL given on the command\nline (i.e. no risk of being NULL), and has_merge would be false even when\nbranch..merge is specified if branch..remote is missing\n(i.e. no risk of running this strcmp() to begin with), but the latter\nsafety guarantee is a bit too subtle for my taste to go without a in-code\ncomment.\n\nThanks.\n"},{"id":"150386","messageId":"NYNWO-6R8khWRYvZHfeUgclo0qTrx32yM6iROg9eTvmBQn8zP_PDNNLiYY_Cx1TwXuGliHPxYzQ@cipher.nrlssc.navy.mil","threadId":"24845","inReplyTo":"7vtymipddv.fsf@alter.siamese.dyndns.org","subject":"[PATCH 1/2] builtin/fetch.c: comment that branch->remote_name is usable when has_merge","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-09-09T18:56:36Z","receivedAt":"2010-09-09T18:56:36Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey \n\nSave future readers the trouble of tracing code to determine that the two\nuses of branch->remote_name are safe when has_merge is set, by adding a\ncomment explaining that it is so.\n\nSigned-off-by: Brandon Casey \n---\n builtin/fetch.c | 3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex fccc9cb..6fc5047 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -148,6 +148,7 @@ static struct ref *get_ref_map(struct transport *transport,\n \t\tint has_merge = branch_has_merge_config(branch);\n \t\tif (remote &&\n \t\t (remote->fetch_refspec_nr ||\n+\t\t /* Note: has_merge implies non-NULL branch->remote_name */\n \t\t (has_merge && !strcmp(branch->remote_name, remote->name)))) {\n \t\t\tfor (i = 0; i < remote->fetch_refspec_nr; i++) {\n \t\t\t\tget_fetch_map(remote_refs, &remote->fetch[i], &tail, 0);\n@@ -162,6 +163,8 @@ static struct ref *get_ref_map(struct transport *transport,\n \t\t\t * if the remote we're fetching from is the same\n \t\t\t * as given in branch..remote, we add the\n \t\t\t * ref given in branch..merge, too.\n+\t\t\t *\n+\t\t\t * Note: has_merge implies non-NULL branch->remote_name\n \t\t\t */\n \t\t\tif (has_merge &&\n \t\t\t !strcmp(branch->remote_name, remote->name))\n-- \n1.7.2.1\n"},{"id":"150385","messageId":"NYNWO-6R8khWRYvZHfeUgZWw3DJpqSnrL38KkBJYm9iti69nWEX9zBwCvwiTEGTR8e9aY62V_L8@cipher.nrlssc.navy.mil","threadId":"24845","inReplyTo":"7vtymipddv.fsf@alter.siamese.dyndns.org","subject":"[PATCH 2/2] t/t5510-fetch.sh: improve testing with explicit URL and merge spec","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-09-09T18:56:37Z","receivedAt":"2010-09-09T18:56:37Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey \n\nCommit 6106ce46 introduced a test to demonstrate fetch's failure to\nretrieve any objects or update FETCH_HEAD when it was supplied a repository\nURL and the current branch had a configured merge spec. This commit\nexpands the original test based on comments from Junio Hamano. In addition\nto actually verifying that the fetch updates FETCH_HEAD correctly, and does\nnot update the current branch, two more tests are added to ensure that the\nmerge configuration is ignored even when the supplied URL matches the URL\nof the remote configured for the branch.\n\nSigned-off-by: Brandon Casey \n---\n t/t5510-fetch.sh | 30 ++++++++++++++++++++++++++++--\n 1 files changed, 28 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex 8fbd894..efb42d1 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -240,10 +240,36 @@ test_expect_success 'fetch with a non-applying branch..merge' '\n \tgit fetch blub\n '\n \n-test_expect_success 'fetch from GIT URL with a non-applying branch..merge' '\n+# URL supplied to fetch does not match the url of the configured branch's remote\n+test_expect_success 'fetch from GIT URL with a non-applying branch..merge [1]' '\n+\tone_head=$(cd one && git rev-parse HEAD) &&\n+\tthis_head=$(git rev-parse HEAD) &&\n \tgit update-ref -d FETCH_HEAD &&\n \tgit fetch one &&\n-\tgit rev-parse --verify FETCH_HEAD\n+\ttest $one_head = \"$(git rev-parse --verify FETCH_HEAD)\" &&\n+\ttest $this_head = \"$(git rev-parse --verify HEAD)\"\n+'\n+\n+# URL supplied to fetch matches the url of the configured branch's remote and\n+# the merge spec matches the branch the remote HEAD points to\n+test_expect_success 'fetch from GIT URL with a non-applying branch..merge [2]' '\n+\tone_ref=$(cd one && git symbolic-ref HEAD) &&\n+\tgit config branch.master.remote blub &&\n+\tgit config branch.master.merge \"$one_ref\" &&\n+\tgit update-ref -d FETCH_HEAD &&\n+\tgit fetch one &&\n+\ttest $one_head = \"$(git rev-parse --verify FETCH_HEAD)\" &&\n+\ttest $this_head = \"$(git rev-parse --verify HEAD)\"\n+'\n+\n+# URL supplied to fetch matches the url of the configured branch's remote, but\n+# the merge spec does not match the branch the remote HEAD points to\n+test_expect_success 'fetch from GIT URL with a non-applying branch..merge [3]' '\n+\tgit config branch.master.merge \"${one_ref}_not\" &&\n+\tgit update-ref -d FETCH_HEAD &&\n+\tgit fetch one &&\n+\ttest $one_head = \"$(git rev-parse --verify FETCH_HEAD)\" &&\n+\ttest $this_head = \"$(git rev-parse --verify HEAD)\"\n '\n \n # the strange name is: a\\!'b\n-- \n1.7.2.1\n"}]}