{"thread":{"id":"31187","subject":"Bringing a bit more sanity to $GIT_DIR/objects/info/alternates?","startedAt":"2012-08-05T04:56:56Z","lastAt":"2012-09-07T17:07:01Z","messageCount":26,"participants":["Junio C Hamano","Michael Haggerty","Jeff King","Sascha Cunz","Hallvard Breien Furuseth","Oswald Buddenhagen","Dan Johnson","Jens Lehmann"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"196481","messageId":"7vmx2a3pif.fsf@alter.siamese.dyndns.org","threadId":"31187","inReplyTo":null,"subject":"Bringing a bit more sanity to $GIT_DIR/objects/info/alternates?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-05T04:56:56Z","receivedAt":"2012-08-05T04:56:56Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The \"alternates\" mechanism lets you keep a single object store (not\nnecessarily a git repository on its own, but just the objects/ part\nof it) on a machine, have multiple repositories on the same machine\nshare objects from it, to save the network transfer bandwidth when\ncloning from remote repositories and the disk space used by the\nlocal repositories. A repository created by \"clone --reference\" or\n\"clone -s\" uses this mechanism to borrow objects from the object\nstore of another repository. A user also can manually add new\nentries to $GIT_DIR/objects/info/alternates to borrow from other\nobject stores.\n\nThe UI for this mechanism however has some room for improvement, and\nwe may want to start improving it for the next release after the\nupcoming Git 1.7.12 (or even Git 2.0 if the change is a large one\nthat may be backward incompatible but gives us a vast improvement).\n\nHere are some random thoughts as a discussion starter.\n\n - By design, the borrowed object store MUST not ever lose any\n object from it, as such an object loss can corrupt the borrowing\n repositories. In theory, it is OK for the object store whose\n objects are borrowed by repositories to acquire new objects, but\n losing existing objects is an absolute no-no.\n\n But the UI of \"clone -s\" encourages users to borrow from the\n object store of a repository that the user may actively develop\n in. It is perfectly normal for users to perform operations that\n make objects that used to be reachable from tips of its branches\n unreachable (e.g. rebase, reset, \"branch -d\") in a repository\n that is used for active development, but a \"gc\" after such an\n operation will lose objects that were originally available in the\n repository. If objects lost that way were still needed by the\n repositories that borrow from it, the borrowing repository gets\n corrupt immediately.\n\n In practice, this means that users who use \"clone -s\" to make a\n new repository can *never* prune the original repository without\n risking to corrupt its borrowing repository [*1*].\n\n Some ideas:\n\n - Make \"clone --reference\" without \"-s\" not to borrow from the\n reference repository. E.g. if you have a clone of Linus\n repository at /git/linux.git/, cloning a related repository\n using it as --reference:\n\n $ git clone --reference /git/linux.git git://k.org/linux-next.git\n\n should still take advantage of /git/linux.git/{refs,objects} to\n reduce the transfer cost of fetching from k.org, but the\n resulting repository should not point /git/linux.git with its\n objects/info/alternates file.\n\n - Make the distinction between a regular repository and an object\n store that is meant to be used for object sharing stronger.\n\n Perhaps a configuration item \"core.objectstore = readonly\" can\n be introduced, and we forbid \"clone -s\" from pointing at a\n repository without such a configuration. We also forbid object\n pruning operations such as \"gc\" and \"repack\" from being run in\n a repository marked as such.\n\n It may be necessary to allow some special kind of repacking of\n such a \"readonly\" object store, in order to reduce the number\n of packfiles (and get rid of loose object files); it needs to\n be implemented carefully not to lose any object, regardless of\n local reachability.\n\n - When you have a repository and one or more repositories that\n borrow from it, you may want to dissociate the borrowing\n repositories from the borrowed one (e.g. so that you can repack\n or prune the original repository safely, or you may even want to\n remove it).\n\n I think \"git repack -a -d -f\" in the borrowing repository happens\n to be the way to do this, but it is not clear to the users why.\n\n Some ideas:\n\n - It might not be a bad idea to have a dedicated new command to\n help users manage alternates (\"git alternates\"?); obviously\n this will be one of its subcommand \"git alternates detach\" if\n we go that route.\n\n - Or just an entry in the documentation is sufficient?\n\n - When you have two or more repositories that do not share objects,\n you may want to rearrange things so that they share their objects\n from a single common object store.\n\n There is no direct UI to do this, as far as I know. You can\n obviously create a new bare repository, push there from all\n of these repositories, and then borrow from there, e.g.\n\n\tgit --bare init shared.git &&\n\tfor r in a.git b.git c.git ...\n do\n\t (\n\t\tcd \"$r\" &&\n\t git push ../shared.git \"refs/*:refs/remotes/$r/*\" &&\n\t\techo ../../../shared.git/objects >.git/objects/info/alternates\n \t )\n\tdone\n\n And then repack shared.git once.\n\n Some ideas:\n\n - (obvious: give a canned command to do the above, perhaps then\n set the core.objectstore=readonly in the resuting shared.git)\n\n - When you have one object store and a repository that does not yet\n borrow from it, you may want to make the repository borrow from\n the object store. Obviously you can run \"echo\" like the sample\n script in the previous item above, but it is not obvious how to\n perform the logical next step of shrinking $GIT_DIR/objects of\n the repository that now borrows the objects.\n\n I think \"git repack -a -d\" is the way to do this, but if you\n compare this command to \"git repack -a -d -f\" we saw previously\n in this message, it is not surprising that the users would be\n confused---it is not obvious at all.\n\n Some ideas:\n\n - (obvious: give a canned subcommand to do this)\n\n\n[Footnote]\n\n*1* Making the borrowed object store aware of all the repositories\nthat borrow from it, so that operations like \"gc\" and \"repack\" in\nthe repository with the borrowed object store can keep objects that\nare needed by borrowing repositories, is theoretically possible, but\nis not a workable approach in practice, as (1) borrowers may not\nhave a write access to the shared object store to add such a back\npointer to begin with, (2) \"gc\"/\"repack\" in the borrowed object\nstore and normal operations in the borrowing repositories can easily\nrace with each other, without any coordination between the users,\nand (3) a casual \"borrowing\" can simply be done with a simple \"echo\"\nas shown in the main text of this message, and there is no way to\nensure a backpointer from the borrowed object store to such a\nborrowing repository.\n"},{"id":"196485","messageId":"501E3F04.4050902@alum.mit.edu","threadId":"31187","inReplyTo":"7vmx2a3pif.fsf@alter.siamese.dyndns.org","subject":"Re: Bringing a bit more sanity to $GIT_DIR/objects/info/alternates?","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-08-05T09:38:12Z","receivedAt":"2012-08-05T09:38:12Z","isPatch":false,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 08/05/2012 06:56 AM, Junio C Hamano wrote:\n> The \"alternates\" mechanism [...]\n> The UI for this mechanism however has some room for improvement, and\n> we may want to start improving it for the next release after the\n> upcoming Git 1.7.12 (or even Git 2.0 if the change is a large one\n> that may be backward incompatible but gives us a vast improvement).\n >\n> Here are some random thoughts as a discussion starter. [...]\n[...]\n> - Make the distinction between a regular repository and an object\n> store that is meant to be used for object sharing stronger.\n>\n> Perhaps a configuration item \"core.objectstore = readonly\" can\n> be introduced, and we forbid \"clone -s\" from pointing at a\n> repository without such a configuration. We also forbid object\n> pruning operations such as \"gc\" and \"repack\" from being run in\n> a repository marked as such.\n\nMust the repository necessarily be \"readonly\"? It seems that it would \nbe permissible to push new objects to such a repository; just not to \ndelete existing objects. Thus maybe another term would be better to \ndescribe such a repository, like \"appendonly\" or \"noprune\" or even \nsomething more abstract like \"donor\".\n\nI have some other crazy ideas for making the concept even more powerful:\n\n* Support remote alternate repositories. Local repository obtains \nmissing objects from the remote as needed. This would probably be \ninsanely inefficient without also supporting...\n\n* Lazy copying of \"borrowed\" objects to the local repository. Any \nobject fetched from the alternate object store is copied to the local \nobject store.\n\nTogether, I think that these two features would give fully-functional \nshallow clones.\n\nSuch alternates could even be chained together: for example, keep a \nsingle local lazy clone of the upstream repository somewhere on your \nsite or on your computer, and use that as read-through cache for other \nclones.\n\n* To help manage local disk space, allow intelligent curation of the \nobjects kept in the local store when they are also available in the \nalternate. The criteria for what to keep could be things like \n\"revisions with depth <= 20 on branches X, Y/*, and Z\"; \"objects that \nhave been accessed within the last 3 months\", \"all tag objects \nrefs/tags/release-*\". It should be possible to cull objects not meeting \nthe criteria with or without actively fetching all objects meeting the \ncriteria. Probably the criteria would be stored in the configuration to \nbe reused (and perhaps run as part of \"git gc\").\n\nThis would cure a lot of \"storing big, non-deltaable files\" pain because \nbig blobs could be stored on a central server without multiplying the \nsize of every clone.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\n"},{"id":"196495","messageId":"7va9y940zr.fsf@alter.siamese.dyndns.org","threadId":"31187","inReplyTo":"501E3F04.4050902@alum.mit.edu","subject":"Re: Bringing a bit more sanity to $GIT_DIR/objects/info/alternates?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-05T19:01:12Z","receivedAt":"2012-08-05T19:01:12Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty writes:\n\n> I have some other crazy ideas for making the concept even more powerful:\n\nSorry, but the \"a bit more sanity\" topic is not interested in making\nthe concept powerful at all.\n\nThis is about making it usable with ease without the user having to\nworry about \"oh, I was about to shoot myself in the foot by running\nrepack; it is good that I remembered objects in this repository are\nborrowed by other repositories\" and things like that.\n\nFor the purpose of \"a bit more sanity\" topic, adding new things\nusers have to worry about to the mix, e.g. \"what happens if my\nnetwork goes away? I can afford not to have access to these kinds\nof objects for a while, but I must always have access to those\nobjects, so I can borrow the former but not the latter\", is going in\nthe other way.\n\nThe ideas in your messages are *not* useless. Enhancements along\nthose lines may be useful, but they do not fit in the same\ndiscussion of making the current mechanism simmpler and easier for\nusers to use the mechanism in a safe and sane way.\n"},{"id":"196570","messageId":"7vboiny9bc.fsf@alter.siamese.dyndns.org","threadId":"31187","inReplyTo":"7vmx2a3pif.fsf@alter.siamese.dyndns.org","subject":"Re: Bringing a bit more sanity to $GIT_DIR/objects/info/alternates?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-06T21:55:35Z","receivedAt":"2012-08-06T21:55:35Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> - When you have one object store and a repository that does not yet\n> borrow from it, you may want to make the repository borrow from\n> the object store. Obviously you can run \"echo\" like the sample\n> script in the previous item above, but it is not obvious how to\n> perform the logical next step of shrinking $GIT_DIR/objects of\n> the repository that now borrows the objects.\n>\n> I think \"git repack -a -d\" is the way to do this, but if you\n> compare this command to \"git repack -a -d -f\" we saw previously\n> in this message, it is not surprising that the users would be\n> confused---it is not obvious at all.\n>\n> Some ideas:\n>\n> - (obvious: give a canned subcommand to do this)\n\nThe analysis of this item is wrong, I think. \"git repack -a -d -l\"\nshould be the way to do so.\n\nThe message looks wrong when it turns out that there is no need to\nhave any object in the borrowing repository, though. We only see\n\"Nothing new to pack\" (which technically is correct), and the\ncommand exits successfully. You can peek .git/objects/ to find out\nthat all the objects the borrower used to have its own copy are now\ngone (because they are available at the alternate), but the message\ngives a false impression that we thought about doing something,\nfound nothing new to be packed, and gave up without doing anything.\n\nBut that is not what is happening. We traversed the connectivity,\nfound that all the objects necessary for our history are housed in\nour alternates, gave \"Nothing new to pack\" (because we do not have\nto have any object on our own), and then removed all the object\nfiles and packs in our repository.\n"},{"id":"196597","messageId":"20120807061616.GC13222@sigill.intra.peff.net","threadId":"31187","inReplyTo":"501E3F04.4050902@alum.mit.edu","subject":"Re: Bringing a bit more sanity to $GIT_DIR/objects/info/alternates?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-08-07T06:16:16Z","receivedAt":"2012-08-07T06:16:16Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Aug 05, 2012 at 11:38:12AM +0200, Michael Haggerty wrote:\n\n> I have some other crazy ideas for making the concept even more powerful:\n> \n> * Support remote alternate repositories. Local repository obtains\n> missing objects from the remote as needed. This would probably be\n> insanely inefficient without also supporting...\n> \n> * Lazy copying of \"borrowed\" objects to the local repository. Any\n> object fetched from the alternate object store is copied to the local\n> object store.\n> \n> Together, I think that these two features would give fully-functional\n> shallow clones.\n\nYou might be interested in looking at my rough (_very_ rough) experiment\nwith object db \"hooks\":\n\n https://github.com/peff/git/commits/jk/external-odb\n\nThe basic idea is to have helper programs that basically have two\ncommands: give a list of sha1s you can provide, and fetch a specific\nobject by sha1. That's enough for the low levels of git to fall-back to\na helper on an object lookup failure, and copy the object to a local\ncache. Managing the cache could be done externally by helper-specific\ncode.\n\nSorry, there's no documentation on the format or behavior, and most of\nthe changes are in one big patch. If you're interested and find it\nunreadable, I can try to clean it up.\n\n-Peff\n"},{"id":"196637","messageId":"5037389.HDN2QUVQO5@mephista","threadId":"31187","inReplyTo":"7vmx2a3pif.fsf@alter.siamese.dyndns.org","subject":"Re: Bringing a bit more sanity to $GIT_DIR/objects/info/alternates?","fromName":"Sascha Cunz","fromEmail":"sascha-ml@babbelbox.org","sentAt":"2012-08-08T01:42:11Z","receivedAt":"2012-08-08T01:42:11Z","isPatch":false,"sender":{"key":"sascha-ml@babbelbox.org","avatar":null},"body":"[..]\n> - By design, the borrowed object store MUST not ever lose any\n> object from it, as such an object loss can corrupt the borrowing\n> repositories. In theory, it is OK for the object store whose\n> objects are borrowed by repositories to acquire new objects, but\n> losing existing objects is an absolute no-no.\n[...]\n> In practice, this means that users who use \"clone -s\" to make a\n> new repository can *never* prune the original repository without\n> risking to corrupt its borrowing repository [*1*].\n[...]\n\nGiven your example of /git/linux.git being a clone of Linus' repository, \ncloning a related repository using it as --reference:\n\n $ cd /git\n $ git clone --reference /git/linux.git git://k.org/linux-next.git mine\n\nWouldn't it be by far a less intrusive alternative to do the following (in the \nclone step above):\n\n- create the file /git/linux.git/objects/borrowing/_git_mine (This is where we\n borrow FROM).\n This file would hold a packed-ref list of HEADs from the /git/mine clone of\n the repository.\n\n _git_mine here is slash-stripped version of the destination path. Maybe the\n packed-ref format could also be extended by a single line containing a full\n path to the foreign repository.\n\n- On every update-ref to /git/mine, update the 'borrowing' refs in\n /git/linux.git\n\n- On any maintenance on /git/linux.git (gc, prune, repack, etc.) consider refs\n in the packed-refs at objects/borrowing to be valid references.\n\n If packed-ref format was adopted like stated above, we could stat() here if\n this directory still exists and error out if it doesn't (In this case the\n user should tell us if she moved or removed the clone).\n\nAny alternatives for looking up the packed-refs list for borrowing would also \nbe doable; i.E. putting the list of valid borrowing-packed-refs-files into the \nconfig file (as opposed to lookup $GIT_DIR/objects/borrowing above).\nPutting this list into the config file would eliminate need for the packed-ref \nformat change and give the user the ability to maintain her clones with well-\nknown command 'git config'\n"},{"id":"196857","messageId":"hbf.20120811d15z@bombur.uio.no","threadId":"31187","inReplyTo":"7vmx2a3pif.fsf@alter.siamese.dyndns.org","subject":"Re: Bringing a bit more sanity to $GIT_DIR/objects/info/alternates?","fromName":"Hallvard Breien Furuseth","fromEmail":"h.b.furuseth@usit.uio.no","sentAt":"2012-08-11T09:35:53Z","receivedAt":"2012-08-11T09:35:53Z","isPatch":false,"sender":{"key":"h.b.furuseth@usit.uio.no","avatar":null},"body":"Junio C Hamano wrote:\n> Some ideas:\n> \n> - Make \"clone --reference\" without \"-s\" not to borrow from the\n> reference repository. (...)\n\nGeneralize: Introduce volatile alternate object stores. Commands like\n(remote) fetch, repack, gc will copy desired objects they see there.\n\nThat allows pruneable alternates if people want them: Make every\nborrowing repo also borrow from a companion volatile store. To prune\nsome shared objects: Move them from the alternate to the volatile.\nRepack or gc all borrowing repos. Empty the volatile alternate.\nSimilar to detach from one alternate repo while keeping others:\ngc with the to-be-dropped alternate as a volatile.\n\nAlso it gives a simple way to try to repair a repo with missing\nobjects, if you have some other repositories which might have the\nobjects: Repack with the other repositories as volatile alternates.\n\nBTW, if a wanted object disappears from the volatile alternate while\nfetch is running, fetch should get it from the remote after all.\n\n> - Make the distinction between a regular repository and an object\n> store that is meant to be used for object sharing stronger.\n> \n> Perhaps a configuration item \"core.objectstore = readonly\" can\n> be introduced, and we forbid \"clone -s\" from pointing at a\n> repository without such a configuration. We also forbid object\n> pruning operations such as \"gc\" and \"repack\" from being run in\n> a repository marked as such.\n\nI hope Michael's \"append-only\"/\"donor\" is feasible instead. In which\ncase safer gc/repack are needed, like you outline:\n\n> It may be necessary to allow some special kind of repacking of\n> such a \"readonly\" object store, in order to reduce the number\n> of packfiles (and get rid of loose object files); it needs to\n> be implemented carefully not to lose any object, regardless of\n> local reachability.\n\nAnd it needs to be default behavior in such stores, so users won't\nneed don't-shoot-myself-in-foot options.\n\n> - It might not be a bad idea to have a dedicated new command to\n> help users manage alternates (\"git alternates\"?); obviously\n> this will be one of its subcommand \"git alternates detach\" if\n> we go that route.\n\n\"git object-store -- manage alternates & object stores\"?\n\n> - Or just an entry in the documentation is sufficient?\n\nBetter doc would be useful anyway, and this command gives a place to\nput it:-) I had no idea alternates were intended to be read-only,\nbut that does explain some seeming defects I'd wondered about.\n\n> - When you have two or more repositories that do not share objects,\n> you may want to rearrange things so that they share their objects\n> from a single common object store.\n> \n> There is no direct UI to do this, as far as I know. You can\n> obviously create a new bare repository, push there from all\n> of these repositories, and then borrow from there, e.g.\n> \n> \tgit --bare init shared.git &&\n> \tfor r in a.git b.git c.git ...\n> do\n> \t (\n> \t\tcd \"$r\" &&\n> \t git push ../shared.git \"refs/*:refs/remotes/$r/*\" &&\n> \t\techo ../../../shared.git/objects >.git/objects/info/alternates\n> \t )\n> \tdone\n> \n> And then repack shared.git once.\n\n...and finally gc the other repositories.\n\nThe refs/remotes/$r/ namespace becomes misleading if the user renames\nor copies the corresponding Git repository, and then cleverly does\nsomething to the shared repo and the repo (if any) in directory $r.\n\nI suggest refs/remotes/$unique_number/ and note $unique_number\nsomewhere in the borrowing repo. If someone insists on being clever,\nthis may force them to read up on what they're doing first.\n\nOr store no refs, since the shared repo shouldn't lose objects anyway.\n\nIf we're sure objects won't be lost: Create a proper remote with the\nshared repo. That way the user can push into it once in a while, and\nhe can configure just which refs should be shared.\n\n> \n> Some ideas:\n> \n> - (obvious: give a canned command to do the above, perhaps then\n> set the core.objectstore=readonly in the resuting shared.git)\n\nThat's getting closer to 'bzr init-repository': One dir with the\nshared repo and all borrowing repositories. A simple model which Git\ncan track and the user need not think further about.\n\nThis way, git clone/init of a new repo in this dir can learn to notice\nand use the shared repo.\n\nWe can also have a command (git object-store?) to maintain the\nrepository collection, since Git knows where to find them all:\nPush from all repos into the shared repo, gc all repos, even prune\nunused objects from the shared repo - after imlementing sufficient\nparanoia.\n\n> - When you have one object store and a repository that does not yet\n> borrow from it, you may want to make the repository borrow from\n> the object store. Obviously you can run \"echo\" like the sample\n> script in the previous item above, but it is not obvious how to\n> perform the logical next step of shrinking $GIT_DIR/objects of\n> the repository that now borrows the objects.\n> \n> I think \"git repack -a -d\" is the way to do this, but if you\n> compare this command to \"git repack -a -d -f\" we saw previously\n> in this message, it is not surprising that the users would be\n> confused---it is not obvious at all.\n\nHopefully users only need to know \"git gc\".\n\n> [Footnote]\n> \n> *1* Making the borrowed object store aware of all the repositories\n> that borrow from it, so that operations like \"gc\" and \"repack\" in\n> the repository with the borrowed object store can keep objects that\n> are needed by borrowing repositories, is theoretically possible, but\n> is not a workable approach in practice, as (1) borrowers may not\n> have a write access to the shared object store to add such a back\n> pointer to begin with,\n\nThus this can only be an optional feature.\nNot via direct backrefs though, see above about refs/remotes/$r/.\n\n> (2) \"gc\"/\"repack\" in the borrowed object\n> store and normal operations in the borrowing repositories can easily\n> race with each other, without any coordination between the users,\n\nThis sounds like a bug to me, unless you refer to deleting objects\nfrom the shared store. The doc does not warn that we can't even\nmaintain shared store while using Git commands in borrowing\nrepositories.\n\n> and (3) a casual \"borrowing\" can simply be done with a simple \"echo\"\n> as shown in the main text of this message, and there is no way to\n> ensure a backpointer from the borrowed object store to such a\n> borrowing repository.\n\n-- \nHallvard\n"},{"id":"197938","messageId":"loom.20120827T233125-780@post.gmane.org","threadId":"31187","inReplyTo":"7vmx2a3pif.fsf@alter.siamese.dyndns.org","subject":"Re: Bringing a bit more sanity to $GIT_DIR/objects/info/alternates?","fromName":"Oswald Buddenhagen","fromEmail":"ossi@kde.org","sentAt":"2012-08-27T22:39:23Z","receivedAt":"2012-08-27T22:39:23Z","isPatch":false,"sender":{"key":"ossi@kde.org","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"hi,\n\nJunio C Hamano pobox.com> writes:\n> The \"alternates\" mechanism [...]\n\nsorry for the somewhat late response - i found this thread only now.\n\nat qt-project.org we have a somewhat peculiar setup: we have the qt4 repository,\nand a bunch of qt5 repositories which resulted from a split. qt5 is under active\ndevelopment, but qt4 is still maintained. that means that we need to cherry-pick\nbetween those repositories quite a lot. for an optimal cherry-picking experience\none needs three-way-merging, which means we need shared object stores. which is\nwhere the problems start:\n\nmy first approach was just a common objects/ directory with all repositories\nsymlinking into it. problems:\n- the object store can never be garbage-collected. with a lot of heavy rebasing\nand temporarily added remotes, it gets messy after a while.\n- there is a constant risk of destroying the object store by inadvertently\nrunning git gc - which is particularly likely with git-gui, as it seems to be\nretarded enough to ignore the auto-gc setting.\n\nso the second approach is the \"bare aggregator repo\" which adds all other repos\nas remotes, and the other repos link back via alternates. problems:\n- to actually share objects, one always needs to push to the aggregator\n- tags having a shared namespace doesn't actually work, because the repos have\nthe same tags on different commits (they are independent repos, after all)\n- one still cannot safely garbage-collect the aggregator, as the refs don't\ninclude the stashes and the index, so rebasing may invalidate these more\ntransient objects.\n\ni would re-propose hallvard's \"volatile\" alternates (at least i think that's\nwhat he was talking about two weeks ago): they can be used to obtain objects,\nbut every object which is in any way referenced from the current clone must be\navailable locally (or from a \"regular\" alternate). that means that diffing, etc.\nwould get objects only temporarily, while cherry-picking would actually copy\n(some of) the objects. this would make it possible to \"cross-link\" repositories,\nsafely and without any \"3rd parties\".\n\nthoughts?\n\nregards\n"},{"id":"198010","messageId":"hbf.20120828vnfp@bombur.uio.no","threadId":"31187","inReplyTo":"loom.20120827T233125-780@post.gmane.org","subject":"GC of alternate object store (was: Bringing a bit more sanity to $GIT_DIR/objects/info/alternates?)","fromName":"Hallvard Breien Furuseth","fromEmail":"h.b.furuseth@usit.uio.no","sentAt":"2012-08-28T19:19:53Z","receivedAt":"2012-08-28T19:19:53Z","isPatch":false,"sender":{"key":"h.b.furuseth@usit.uio.no","avatar":null},"body":"Oswald Buddenhagen wrote:\n> (...)so the second approach is the \"bare aggregator repo\" which adds\n> all other repos as remotes, and the other repos link back via\n> alternates. problems:\n> \n> - to actually share objects, one always needs to push to the aggregator\n\nRun a cron job which frequently does that?\n\n> - tags having a shared namespace doesn't actually work, because the\n> repos have the same tags on different commits (they are independent\n> repos, after all)\n\nJunio's proposal partially fixes that: It pushes refs/* instead of\nrefs/heads/*, to refs/remotes//. However...\n\n> - one still cannot safely garbage-collect the aggregator, as the refs\n> don't include the stashes and the index, so rebasing may invalidate\n> these more transient objects.\n\nAlso if you copy a repo (e.g. making a backup) instead of cloning it,\nand then start using both, they'll push into the same namespace -\noverwriting each other's refs. Non-fast-forward pushes can thus lose\nrefs to objects needed by the other repo.\n\nreceive.denyNonFastForwards only rejects pushes to refs/heads/ or\nsomething. (A feature, as I learned when I reported it as bug:-)\nIIRC Git has no config option to reject all non-fast-forward pushes.\n\n> i would re-propose hallvard's \"volatile\" alternates (at least i think that's\n> what he was talking about two weeks ago): they can be used to obtain\n> objects, but every object which is in any way referenced from the current\n> clone must be available locally (or from a \"regular\" alternate). that means\n> that diffing, etc. would get objects only temporarily, while cherry-picking\n> would actually copy (some of) the objects. this would make it possible to\n> \"cross-link\" repositories, safely and without any \"3rd parties\".\n\nI'm afraid that idea by itself won't work:-( Either you borrow from a\nstore or not. If Git uses an object from the volatile store, it can't\nalways know if the caller needs the object to be copied.\n\nOTOH volatile stores which you do *not* borrow from would be useful:\nLet fetch/repack/gc/whatever copy missing objects from there.\n\n\n2nd attempt for a way to gc of the alternate repo: Copy the with\nremoved objects into each borrowing repo, then gc them. Like this:\n\n1. gc, but pack all to-be-removed objects into a \"removable\" pack.\n\n2. Hardlink/copy the removable pack - with a .keep file - into\n borrowing repos when feasible: I.e. repos you can find and\n have write access to. Update their .git/objects/info/packs.\n (Is there a Git command for this?) Repeat until nothing to do,\n in case someone created a new repo during this step.\n\n3. Move the pack from the alternate repo to a backup object store\n which will keep it for a while.\n\n4. Delete the .keep files from step (2). They were needed in case\n a user gc'ed away an object from the pack and then added an\n identical object - borrowed from the to-be-removed pack.\n\n5. gc/repack the other repos at your leisure.\n\n666. Repos you could not update in step (2), can get temporarily\n broken. Their owners must link the pack from the backup store by\n hand, or use that store as a volatile store and then gc/repack.\n\nLoose objects are a problem: If a repo has longer expiry time(s)\nthan the alternate store, it will get loads of loose objects from all\nrepos which push into the alternate store. Worse, gc can *unpack*\nthose objects, consuming a lot of space. See threads \"git gc == git\ngarbage-create from removed branch\" (3 May) and \"Keeping unreachable\nobjects in a separate pack instead of loose?\" (10 Jun).\n\nPresumably the work-arounds are:\n- Use long expiry times in the alternate repo. I don't know which\n expiration config settings are relevant how.\n- Add some command which checks and warns if the repo has longer\n expiry time than the repo it borrows from.\nAlso I hope Git will be changed to instead pack such loose objects\nsomewhere, as discussed in the above threads.\n\nAll in all, this isn't something you'd want to do every day. But it\nlooks doable and can be scripted.\n"},{"id":"198033","messageId":"20120829074249.GA14408@ugly.local","threadId":"31187","inReplyTo":"hbf.20120828vnfp@bombur.uio.no","subject":"Re: GC of alternate object store (was: Bringing a bit more sanity to $GIT_DIR/objects/info/alternates?)","fromName":"Oswald Buddenhagen","fromEmail":"ossi@kde.org","sentAt":"2012-08-29T07:42:49Z","receivedAt":"2012-08-29T07:42:49Z","isPatch":false,"sender":{"key":"ossi@kde.org","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Tue, Aug 28, 2012 at 09:19:53PM +0200, Hallvard Breien Furuseth wrote:\n> Oswald Buddenhagen wrote:\n> > (...)so the second approach is the \"bare aggregator repo\" which adds\n> > all other repos as remotes, and the other repos link back via\n> > alternates. problems:\n> > \n> > - to actually share objects, one always needs to push to the aggregator\n> \n> Run a cron job which frequently does that?\n> \nnope. i also have separate repos which share the same code, so when i\ndevelop it i need to pick between them \"live\". of course it's unlikely\nto get conflicts in this case, so the missing object sharing is not that\nbad (the objects are transferred via format-patch, as i'm rewriting\npaths anyway), but when it happens it's messy to get out again.\n\n> > - tags having a shared namespace doesn't actually work, because the\n> > repos have the same tags on different commits (they are independent\n> > repos, after all)\n> \n> Junio's proposal partially fixes that: It pushes refs/* instead of\n> refs/heads/*, to refs/remotes//. However...\n> \ni did exacty that. the tags are *still* not populated - git just tries\nvery hard to treat them specially.\nand the \"stash\" file is also ignored, unfortunately.\n\n> > - one still cannot safely garbage-collect the aggregator, as the refs\n> > don't include the stashes and the index, so rebasing may invalidate\n> > these more transient objects.\n> \n> Also if you copy a repo (e.g. making a backup) instead of cloning it,\n> and then start using both, they'll push into the same namespace -\n> overwriting each other's refs.\n>\nright. it's a clear user error, though - i wouldn't *expect* it to work.\nanyway, i don't have *that* problem, as my aggregator actually pulls,\nnot the other way round.\n\nanyway, the bottom line is that using alternates as-is for anything but\nsharing refs/remotes/origin/* (which i'm assuming to be ff-only) is\na recipe for disaster.\n\nanything which is supposed to be in any way safe must make the \"donor\"\nobject store aware of the sharing, which at the very least means setting\nthe proposed append-only flag _by the borrowing_ object store. which\nmeans that the info/alternates file should be obfuscated, so people\ncan't edit it manually.\n\n> > i would re-propose hallvard's \"volatile\" alternates (at least i think that's\n> > what he was talking about two weeks ago): they can be used to obtain\n> > objects, but every object which is in any way referenced from the current\n> > clone must be available locally (or from a \"regular\" alternate). that means\n> > that diffing, etc. would get objects only temporarily, while cherry-picking\n> > would actually copy (some of) the objects. this would make it possible to\n> > \"cross-link\" repositories, safely and without any \"3rd parties\".\n> \n> I'm afraid that idea by itself won't work:-(\n\n> Either you borrow from a store or not.\n>\ncorrect. from \"regular\" alternates you \"borrow\", in \"volatile\" ones you\nonly \"peek\".\nso apparently our definitions are different after all.\n\n> If Git uses an object from the volatile store, it can't always know if\n> the caller needs the object to be copied.\n> \nit doesn't have to. the distinction comes when creating objects: if an\nobject is only in a volatile alternate, it does not already exist for the\npurpose of object creation and is thus created locally.\n\nregards\n"},{"id":"198043","messageId":"7v3935y9tw.fsf@alter.siamese.dyndns.org","threadId":"31187","inReplyTo":"20120829074249.GA14408@ugly.local","subject":"Re: GC of alternate object store","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-29T15:52:27Z","receivedAt":"2012-08-29T15:52:27Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Oswald Buddenhagen writes:\n\n> On Tue, Aug 28, 2012 at 09:19:53PM +0200, Hallvard Breien Furuseth wrote:\n> ...\n>> Junio's proposal partially fixes that: It pushes refs/* instead of\n>> refs/heads/*, to refs/remotes//. However...\n>> \n> i did exacty that. the tags are *still* not populated - git just tries\n> very hard to treat them specially.\n\nJust this part (I won't comment on the other parts in this discussion).\n\nDoesn't\n\n\tgit push $over_there 'refs/*:refs/remotes/mine/*'\n\npush your tag v1.0 to refs/remotes/mine/v1.0 over there? The\nversion of git I ship seems to do this just fine.\n\n> and the \"stash\" file is also ignored, unfortunately.\n\nThere is a work in progress to do this on 'pu'.\n"},{"id":"198090","messageId":"20120830095314.GA29038@troll08.europe.nokia.com","threadId":"31187","inReplyTo":"7v3935y9tw.fsf@alter.siamese.dyndns.org","subject":"Re: GC of alternate object store","fromName":"Oswald Buddenhagen","fromEmail":"ossi@kde.org","sentAt":"2012-08-30T09:53:14Z","receivedAt":"2012-08-30T09:53:14Z","isPatch":false,"sender":{"key":"ossi@kde.org","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Wed, Aug 29, 2012 at 08:52:27AM -0700, Junio C Hamano wrote:\n> (I won't comment on the other parts in this discussion).\n> \nwhich is kinda unfortunate. ;)\n\n> Oswald Buddenhagen writes:\n> > i did exacty that. the tags are *still* not populated - git just tries\n> > very hard to treat them specially.\n> \n> Doesn't\n> \n> \tgit push $over_there 'refs/*:refs/remotes/mine/*'\n> \n> push your tag v1.0 to refs/remotes/mine/v1.0 over there? The\n> version of git I ship seems to do this just fine.\n> \nas i wrote before, i'm pulling, not pushing, so any differences could be\nblamed on that (or git version 1.7.12.23.g948900e).\nanyway, it seems this new version does fetch the tags under the remotes'\nnamespaces, after all - but it still imports them into the global tag\nnamespace as well, which of course makes a mess with the duplicated\ntags.\n\nand for many repos i'm getting something like this (this is kinda new):\n\nFetching qtbase\nremote: Counting objects: 62375, done.\nremote: Compressing objects: 100% (28049/28049), done.\nremote: Total 55704 (delta 45280), reused 36646 (delta 27368)\nReceiving objects: 100% (55704/55704), 16.76 MiB | 4.94 MiB/s, done.\nResolving deltas: 100% (45280/45280), completed with 3017 local objects.\nfatal: bad object 90f0f499ec5953d60d616a2ff541ecaf8b0c31a2\nerror: ../qt5/qtbase did not send all necessary objects\n\nboth the aggregator and the fetched repos run cleanly through fsck.\n\nregards\n"},{"id":"198104","messageId":"7vbohstlih.fsf@alter.siamese.dyndns.org","threadId":"31187","inReplyTo":"20120830095314.GA29038@troll08.europe.nokia.com","subject":"Re: GC of alternate object store","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-30T16:03:34Z","receivedAt":"2012-08-30T16:03:34Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Oswald Buddenhagen writes:\n\n>> Doesn't\n>> \n>> \tgit push $over_there 'refs/*:refs/remotes/mine/*'\n>> \n>> push your tag v1.0 to refs/remotes/mine/v1.0 over there? The\n>> version of git I ship seems to do this just fine.\n>> \n> as i wrote before, i'm pulling, not pushing,...\n\nYou would need to decline the automatic tag following with --no-tags\n(which in hindsight is misnamed; it really means \"do not auto-follow\ntags\"), like so:\n\n\tcd $over_there &&\n git fetch --no-tags $my_repository 'refs/*:refs/remotes/mine/*'\n\nOtherwise, you will also get tags in refs/tags/.\n"},{"id":"198151","messageId":"20120831162629.GA18215@troll08.europe.nokia.com","threadId":"31187","inReplyTo":"7vbohstlih.fsf@alter.siamese.dyndns.org","subject":"Re: GC of alternate object store","fromName":"Oswald Buddenhagen","fromEmail":"ossi@kde.org","sentAt":"2012-08-31T16:26:29Z","receivedAt":"2012-08-31T16:26:29Z","isPatch":false,"sender":{"key":"ossi@kde.org","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Thu, Aug 30, 2012 at 09:03:34AM -0700, Junio C Hamano wrote:\n> Oswald Buddenhagen writes:\n> \n> >> Doesn't\n> >> \n> >> \tgit push $over_there 'refs/*:refs/remotes/mine/*'\n> >> \n> >> push your tag v1.0 to refs/remotes/mine/v1.0 over there? The\n> >> version of git I ship seems to do this just fine.\n> >> \n> > as i wrote before, i'm pulling, not pushing,...\n> \n> You would need to decline the automatic tag following with --no-tags\n> (which in hindsight is misnamed; it really means \"do not auto-follow\n> tags\"), like so:\n> \n> \tcd $over_there &&\n> git fetch --no-tags $my_repository 'refs/*:refs/remotes/mine/*'\n> \n> Otherwise, you will also get tags in refs/tags/.\n> \ngit seems to be happily ignoring that flag.\n git fetch --prune --all --no-tags\nstill re-populates the tags after i delete them manually.\n"},{"id":"198165","messageId":"CAPBPrnvrQx2SeyNM_nxnn7bB=Sakj6X=dbH2va+O-TnspY=Bpw@mail.gmail.com","threadId":"31187","inReplyTo":"20120831162629.GA18215@troll08.europe.nokia.com","subject":"Re: GC of alternate object store","fromName":"Dan Johnson","fromEmail":"computerdruid@gmail.com","sentAt":"2012-08-31T19:18:03Z","receivedAt":"2012-08-31T19:18:03Z","isPatch":false,"sender":{"key":"computerdruid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/34696?v=4"},"body":"On Fri, Aug 31, 2012 at 12:26 PM, Oswald Buddenhagen wrote:\n> On Thu, Aug 30, 2012 at 09:03:34AM -0700, Junio C Hamano wrote:\n>> Oswald Buddenhagen writes:\n>>\n>> >> Doesn't\n>> >>\n>> >> git push $over_there 'refs/*:refs/remotes/mine/*'\n>> >>\n>> >> push your tag v1.0 to refs/remotes/mine/v1.0 over there? The\n>> >> version of git I ship seems to do this just fine.\n>> >>\n>> > as i wrote before, i'm pulling, not pushing,...\n>>\n>> You would need to decline the automatic tag following with --no-tags\n>> (which in hindsight is misnamed; it really means \"do not auto-follow\n>> tags\"), like so:\n>>\n>> cd $over_there &&\n>> git fetch --no-tags $my_repository 'refs/*:refs/remotes/mine/*'\n>>\n>> Otherwise, you will also get tags in refs/tags/.\n>>\n> git seems to be happily ignoring that flag.\n> git fetch --prune --all --no-tags\n> still re-populates the tags after i delete them manually.\n\nI believe that is bad interaction with \"--all\" (probably a bug). If I\nam remembering correctly, --no-tags is internally a per-remote\nsetting, so I'm guessing it's not getting set on all remotes here.\n\nI'll look into this more a bit later tonight. Does fetch --no-tags\nwork when you specify a remote?\n\n-- \n-Dan\n"},{"id":"198166","messageId":"7vr4qmn8va.fsf@alter.siamese.dyndns.org","threadId":"31187","inReplyTo":"CAPBPrnvrQx2SeyNM_nxnn7bB=Sakj6X=dbH2va+O-TnspY=Bpw@mail.gmail.com","subject":"Re: GC of alternate object store","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-31T19:45:29Z","receivedAt":"2012-08-31T19:45:29Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dan Johnson writes:\n\n> I believe that is bad interaction with \"--all\" (probably a bug). If I\n> am remembering correctly, --no-tags is internally a per-remote\n> setting, so I'm guessing it's not getting set on all remotes here.\n>\n> I'll look into this more a bit later tonight. Does fetch --no-tags\n> work when you specify a remote?\n\nThanks.\n"},{"id":"198175","messageId":"1346473533-24175-1-git-send-email-ComputerDruid@gmail.com","threadId":"31187","inReplyTo":"7vr4qmn8va.fsf@alter.siamese.dyndns.org","subject":"[PATCH] fetch --all: pass --tags/--no-tags through to each remote","fromName":"Dan Johnson","fromEmail":"computerdruid@gmail.com","sentAt":"2012-09-01T04:25:33Z","receivedAt":"2012-09-01T04:25:33Z","isPatch":true,"sender":{"key":"computerdruid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/34696?v=4"},"body":"Reported-by: Oswald Buddenhagen \nSigned-off-by: Dan Johnson \n---\n\nJunio C Hamano writes:\n>Dan Johnson writes:\n>\n>> I believe that is bad interaction with \"--all\" (probably a bug). If I\n>> am remembering correctly, --no-tags is internally a per-remote\n>> setting, so I'm guessing it's not getting set on all remotes here.\n>>\n>> I'll look into this more a bit later tonight. Does fetch --no-tags\n>> work when you specify a remote?\n>\n>Thanks.\n\nAnd here it is. Apparently we just don't pass those options through. I didn't\nlook to see if there are any other options we should consider passing through;\nit's quite possible there are. I also have not written a test to ensure that\nthis doesn't break in the future. I will hopefully have time for these things\ntomorrow. It's getting too late for me to be able to put sentences together,\nso hopefully this mail comes out readable ;)\n\n builtin/fetch.c | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex bb9a074..c6bcbdc 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -857,6 +857,10 @@ static void add_options_to_argv(int *argc, const char **argv)\n \t\targv[(*argc)++] = \"--recurse-submodules\";\n \telse if (recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND)\n \t\targv[(*argc)++] = \"--recurse-submodules=on-demand\";\n+\tif (tags == TAGS_SET)\n+\t\targv[(*argc)++] = \"--tags\";\n+\telse if (tags == TAGS_UNSET)\n+\t\targv[(*argc)++] = \"--no-tags\";\n \tif (verbosity >= 2)\n \t\targv[(*argc)++] = \"-v\";\n \tif (verbosity >= 1)\n-- \n1.7.11.1.59.gbc9e7dd.dirty\n"},{"id":"198183","messageId":"20120901112251.GA11445@sigill.intra.peff.net","threadId":"31187","inReplyTo":"1346473533-24175-1-git-send-email-ComputerDruid@gmail.com","subject":"Re: [PATCH] fetch --all: pass --tags/--no-tags through to each remote","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-09-01T11:22:52Z","receivedAt":"2012-09-01T11:22:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Sep 01, 2012 at 12:25:33AM -0400, Dan Johnson wrote:\n\n> diff --git a/builtin/fetch.c b/builtin/fetch.c\n> index bb9a074..c6bcbdc 100644\n> --- a/builtin/fetch.c\n> +++ b/builtin/fetch.c\n> @@ -857,6 +857,10 @@ static void add_options_to_argv(int *argc, const char **argv)\n> \t\targv[(*argc)++] = \"--recurse-submodules\";\n> \telse if (recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND)\n> \t\targv[(*argc)++] = \"--recurse-submodules=on-demand\";\n> +\tif (tags == TAGS_SET)\n> +\t\targv[(*argc)++] = \"--tags\";\n> +\telse if (tags == TAGS_UNSET)\n> +\t\targv[(*argc)++] = \"--no-tags\";\n> \tif (verbosity >= 2)\n> \t\targv[(*argc)++] = \"-v\";\n> \tif (verbosity >= 1)\n\nHmm. We allocate argv in fetch_multiple like this:\n\n const char *argv[12] = { \"fetch\", \"--append\" };\n\nand then add a bunch of options to it, along with the name of the\nremote. By my count, the current code can hit exactly 12 (including the\nterminating NULL) if all options are set. Your patch would make it\npossible to overflow. Of course, I may be miscounting since it is\nextremely error-prone to figure out the right number by tracing each\npossible conditional.\n\nMaybe we should switch it to a dynamic argv_array? Like this:\n\n [1/2]: argv-array: add pop function\n [2/2]: fetch: use argv_array instead of hand-building arrays\n\n-Peff\n"},{"id":"198184","messageId":"20120901112527.GA19163@sigill.intra.peff.net","threadId":"31187","inReplyTo":"20120901112251.GA11445@sigill.intra.peff.net","subject":"[PATCH 1/2] argv-array: add pop function","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-09-01T11:25:27Z","receivedAt":"2012-09-01T11:25:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Sometimes we build a set of similar command lines, differing\nonly in the final arguments (e.g., \"fetch --multiple\"). To\nuse argv_array for this, you have to either push the same\nset of elements repeatedly, or break the abstraction by\nmanually manipulating the array's internal members.\n\nInstead, let's provide a sanctioned \"pop\" function to remove\nelements from the end.\n\nSigned-off-by: Jeff King \n---\n Documentation/technical/api-argv-array.txt | 4 ++++\n argv-array.c | 9 +++++++++\n argv-array.h | 1 +\n 3 files changed, 14 insertions(+)\n\ndiff --git a/Documentation/technical/api-argv-array.txt b/Documentation/technical/api-argv-array.txt\nindex 1b7d8f1..1a79781 100644\n--- a/Documentation/technical/api-argv-array.txt\n+++ b/Documentation/technical/api-argv-array.txt\n@@ -46,6 +46,10 @@ Functions\n \tFormat a string and push it onto the end of the array. This is a\n \tconvenience wrapper combining `strbuf_addf` and `argv_array_push`.\n \n+`argv_array_pop`::\n+\tRemove the final element from the array. If there are no\n+\telements in the array, do nothing.\n+\n `argv_array_clear`::\n \tFree all memory associated with the array and return it to the\n \tinitial, empty state.\ndiff --git a/argv-array.c b/argv-array.c\nindex 0b5f889..55e8443 100644\n--- a/argv-array.c\n+++ b/argv-array.c\n@@ -49,6 +49,15 @@ void argv_array_pushl(struct argv_array *array, ...)\n \tva_end(ap);\n }\n \n+void argv_array_pop(struct argv_array *array)\n+{\n+\tif (!array->argc)\n+\t\treturn;\n+\tfree((char *)array->argv[array->argc - 1]);\n+\tarray->argv[array->argc - 1] = NULL;\n+\tarray->argc--;\n+}\n+\n void argv_array_clear(struct argv_array *array)\n {\n \tif (array->argv != empty_argv) {\ndiff --git a/argv-array.h b/argv-array.h\nindex b93a69c..f4b9866 100644\n--- a/argv-array.h\n+++ b/argv-array.h\n@@ -16,6 +16,7 @@ void argv_array_pushl(struct argv_array *, ...);\n __attribute__((format (printf,2,3)))\n void argv_array_pushf(struct argv_array *, const char *fmt, ...);\n void argv_array_pushl(struct argv_array *, ...);\n+void argv_array_pop(struct argv_array *);\n void argv_array_clear(struct argv_array *);\n \n #endif /* ARGV_ARRAY_H */\n-- \n1.7.12.rc3.8.g89db099\n"},{"id":"198185","messageId":"20120901112735.GB19163@sigill.intra.peff.net","threadId":"31187","inReplyTo":"20120901112251.GA11445@sigill.intra.peff.net","subject":"[PATCH 2/2] fetch: use argv_array instead of hand-building arrays","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-09-01T11:27:35Z","receivedAt":"2012-09-01T11:27:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Fetch invokes itself recursively when recursing into\nsubmodules or handling \"fetch --multiple\". In both cases, it\nbuilds the child's command line by pushing options onto a\nstatically-sized array. In both cases, the array is\ncurrently just big enough to handle the largest possible\ncase. However, this technique is brittle and error-prone, so\nlet's replace it with a dynamic argv_array.\n\nSigned-off-by: Jeff King \n---\nNot very well tested by me, but hopefully it is simple enough that I\nmanaged not to screw it up.\n\nIt may be that fetch_populated_submodules would also benefit from\nconversion (here I just pass in the argc and argv separately), but I\ndidn't look.\n\n builtin/fetch.c | 47 +++++++++++++++++++++++++----------------------\n 1 file changed, 25 insertions(+), 22 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex bb9a074..b6a8be0 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -14,6 +14,7 @@\n #include \"transport.h\"\n #include \"submodule.h\"\n #include \"connected.h\"\n+#include \"argv-array.h\"\n \n static const char * const builtin_fetch_usage[] = {\n \t\"git fetch [] [ [...]]\",\n@@ -841,38 +842,35 @@ static int fetch_multiple(struct string_list *list)\n \treturn 1;\n }\n \n-static void add_options_to_argv(int *argc, const char **argv)\n+static void add_options_to_argv(struct argv_array *argv)\n {\n \tif (dry_run)\n-\t\targv[(*argc)++] = \"--dry-run\";\n+\t\targv_array_push(argv, \"--dry-run\");\n \tif (prune)\n-\t\targv[(*argc)++] = \"--prune\";\n+\t\targv_array_push(argv, \"--prune\");\n \tif (update_head_ok)\n-\t\targv[(*argc)++] = \"--update-head-ok\";\n+\t\targv_array_push(argv, \"--update-head-ok\");\n \tif (force)\n-\t\targv[(*argc)++] = \"--force\";\n+\t\targv_array_push(argv, \"--force\");\n \tif (keep)\n-\t\targv[(*argc)++] = \"--keep\";\n+\t\targv_array_push(argv, \"--keep\");\n \tif (recurse_submodules == RECURSE_SUBMODULES_ON)\n-\t\targv[(*argc)++] = \"--recurse-submodules\";\n+\t\targv_array_push(argv, \"--recurse-submodules\");\n \telse if (recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND)\n-\t\targv[(*argc)++] = \"--recurse-submodules=on-demand\";\n+\t\targv_array_push(argv, \"--recurse-submodules=on-demand\");\n \tif (verbosity >= 2)\n-\t\targv[(*argc)++] = \"-v\";\n+\t\targv_array_push(argv, \"-v\");\n \tif (verbosity >= 1)\n-\t\targv[(*argc)++] = \"-v\";\n+\t\targv_array_push(argv, \"-v\");\n \telse if (verbosity < 0)\n-\t\targv[(*argc)++] = \"-q\";\n+\t\targv_array_push(argv, \"-q\");\n \n }\n \n static int fetch_multiple(struct string_list *list)\n {\n \tint i, result = 0;\n-\tconst char *argv[12] = { \"fetch\", \"--append\" };\n-\tint argc = 2;\n-\n-\tadd_options_to_argv(&argc, argv);\n+\tstruct argv_array argv = ARGV_ARRAY_INIT;\n \n \tif (!append && !dry_run) {\n \t\tint errcode = truncate_fetch_head();\n@@ -880,18 +878,22 @@ static int fetch_multiple(struct string_list *list)\n \t\t\treturn errcode;\n \t}\n \n+\targv_array_pushl(&argv, \"fetch\", \"--append\", NULL);\n+\tadd_options_to_argv(&argv);\n+\n \tfor (i = 0; i < list->nr; i++) {\n \t\tconst char *name = list->items[i].string;\n-\t\targv[argc] = name;\n-\t\targv[argc + 1] = NULL;\n+\t\targv_array_push(&argv, name);\n \t\tif (verbosity >= 0)\n \t\t\tprintf(_(\"Fetching %s\\n\"), name);\n-\t\tif (run_command_v_opt(argv, RUN_GIT_CMD)) {\n+\t\tif (run_command_v_opt(argv.argv, RUN_GIT_CMD)) {\n \t\t\terror(_(\"Could not fetch %s\"), name);\n \t\t\tresult = 1;\n \t\t}\n+\t\targv_array_pop(&argv);\n \t}\n \n+\targv_array_clear(&argv);\n \treturn result;\n }\n \n@@ -1007,13 +1009,14 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (!result && (recurse_submodules != RECURSE_SUBMODULES_OFF)) {\n-\t\tconst char *options[10];\n-\t\tint num_options = 0;\n-\t\tadd_options_to_argv(&num_options, options);\n-\t\tresult = fetch_populated_submodules(num_options, options,\n+\t\tstruct argv_array options = ARGV_ARRAY_INIT;\n+\n+\t\tadd_options_to_argv(&options);\n+\t\tresult = fetch_populated_submodules(options.argc, options.argv,\n \t\t\t\t\t\t submodule_prefix,\n \t\t\t\t\t\t recurse_submodules,\n \t\t\t\t\t\t verbosity < 0);\n+\t\targv_array_clear(&options);\n \t}\n \n \t/* All names were strdup()ed or strndup()ed */\n-- \n1.7.12.rc3.8.g89db099\n"},{"id":"198186","messageId":"20120901113206.GB11445@sigill.intra.peff.net","threadId":"31187","inReplyTo":"20120901112251.GA11445@sigill.intra.peff.net","subject":"Re: [PATCH] fetch --all: pass --tags/--no-tags through to each remote","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-09-01T11:32:07Z","receivedAt":"2012-09-01T11:32:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Since the array struct stores a \"const char **\" argv member\n(for compatibility with most of our argv-taking functions),\nwe have to cast away the const-ness when freeing its\nelements.\n\nHowever, we used the wrong type when doing so. It doesn't\nmake a difference since free() take a void pointer anyway,\nbut it can be slightly confusing to a reader.\n\nSigned-off-by: Jeff King \n---\nNoticed this while I was adding the other free in argv_array_pop...\n\n argv-array.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/argv-array.c b/argv-array.c\nindex 55e8443..256741d 100644\n--- a/argv-array.c\n+++ b/argv-array.c\n@@ -63,7 +63,7 @@ void argv_array_clear(struct argv_array *array)\n \tif (array->argv != empty_argv) {\n \t\tint i;\n \t\tfor (i = 0; i < array->argc; i++)\n-\t\t\tfree((char **)array->argv[i]);\n+\t\t\tfree((char *)array->argv[i]);\n \t\tfree(array->argv);\n \t}\n \targv_array_init(array);\n-- \n1.7.12.rc3.8.g89db099\n"},{"id":"198187","messageId":"20120901113409.GC11445@sigill.intra.peff.net","threadId":"31187","inReplyTo":"20120901113206.GB11445@sigill.intra.peff.net","subject":"[PATCH 3/2] argv-array: fix bogus cast when freeing array","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-09-01T11:34:09Z","receivedAt":"2012-09-01T11:34:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Sep 01, 2012 at 07:32:07AM -0400, Jeff King wrote:\n\n> Since the array struct stores a \"const char **\" argv member\n> (for compatibility with most of our argv-taking functions),\n> we have to cast away the const-ness when freeing its\n> elements.\n> \n> However, we used the wrong type when doing so. It doesn't\n> make a difference since free() take a void pointer anyway,\n> but it can be slightly confusing to a reader.\n> \n> Signed-off-by: Jeff King \n> ---\n> Noticed this while I was adding the other free in argv_array_pop...\n\nArgh, managed to botch the subject line. Here it is for real.\n\n-- >8 --\nSince the array struct stores a \"const char **\" argv member\n(for compatibility with most of our argv-taking functions),\nwe have to cast away the const-ness when freeing its\nelements.\n\nHowever, we used the wrong type when doing so. It doesn't\nmake a difference since free() take a void pointer anyway,\nbut it can be slightly confusing to a reader.\n\nSigned-off-by: Jeff King \n---\n argv-array.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/argv-array.c b/argv-array.c\nindex 55e8443..256741d 100644\n--- a/argv-array.c\n+++ b/argv-array.c\n@@ -63,7 +63,7 @@ void argv_array_clear(struct argv_array *array)\n \tif (array->argv != empty_argv) {\n \t\tint i;\n \t\tfor (i = 0; i < array->argc; i++)\n-\t\t\tfree((char **)array->argv[i]);\n+\t\t\tfree((char *)array->argv[i]);\n \t\tfree(array->argv);\n \t}\n \targv_array_init(array);\n-- \n1.7.12.rc3.8.g89db099\n"},{"id":"198190","messageId":"50421CF8.60703@web.de","threadId":"31187","inReplyTo":"20120901112735.GB19163@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] fetch: use argv_array instead of hand-building arrays","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2012-09-01T14:34:32Z","receivedAt":"2012-09-01T14:34:32Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 01.09.2012 13:27, schrieb Jeff King:\n> Fetch invokes itself recursively when recursing into\n> submodules or handling \"fetch --multiple\". In both cases, it\n> builds the child's command line by pushing options onto a\n> statically-sized array. In both cases, the array is\n> currently just big enough to handle the largest possible\n> case. However, this technique is brittle and error-prone, so\n> let's replace it with a dynamic argv_array.\n> \n> Signed-off-by: Jeff King \n> ---\n> Not very well tested by me, but hopefully it is simple enough that I\n> managed not to screw it up.\n\nThis is definitely an improvement, and I can't spot any problems\neither.\n\n> It may be that fetch_populated_submodules would also benefit from\n> conversion (here I just pass in the argc and argv separately), but I\n> didn't look.\n\nYes, it does some similar brittle stuff and should be changed to use\nthe argv-array too. I'll look into that.\n"},{"id":"198192","messageId":"5042294A.7020507@web.de","threadId":"31187","inReplyTo":"50421CF8.60703@web.de","subject":"[PATCH] submodule: use argv_array instead of hand-building arrays","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2012-09-01T15:27:06Z","receivedAt":"2012-09-01T15:27:06Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"fetch_populated_submodules() allocates the full argv array it uses to\nrecurse into the submodules from the number of given options plus the six\nargv values it is going to add. It then initializes it with those values\nwhich won't change during the iteration and copies the given options into\nit. Inside the loop the two argv values different for each submodule get\nreplaced with those currently valid.\n\nHowever, this technique is brittle and error-prone (as the comment to\nexplain the magic number 6 indicates), so let's replace it with an\nargv_array. Instead of replacing the argv values, push them to the\nargv_array just before the run_command() call (including the option\nseparating them) and pop them from the argv_array right after that.\n\nSigned-off-by: Jens Lehmann \n---\n\n\nAm 01.09.2012 16:34, schrieb Jens Lehmann:\n> Am 01.09.2012 13:27, schrieb Jeff King:\n>> It may be that fetch_populated_submodules would also benefit from\n>> conversion (here I just pass in the argc and argv separately), but I\n>> didn't look.\n> \n> Yes, it does some similar brittle stuff and should be changed to use\n> the argv-array too. I'll look into that.\n\nMaybe something like this on top of your two patches?\n\nI thought about adding an argv_array_cat() function to replace the\nfor() loop copying the option values into the argv-array built inside\nfetch_populated_submodules(), but I suspect saving one line from the\ncode is not worth it. Yet I didn't check if others would benefit from\nsuch a function too.\n\n\n builtin/fetch.c | 2 +-\n submodule.c | 31 ++++++++++++++++---------------\n submodule.h | 3 ++-\n 3 files changed, 19 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex b6a8be0..aaba61e 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -1012,7 +1012,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n \t\tstruct argv_array options = ARGV_ARRAY_INIT;\n\n \t\tadd_options_to_argv(&options);\n-\t\tresult = fetch_populated_submodules(options.argc, options.argv,\n+\t\tresult = fetch_populated_submodules(&options,\n \t\t\t\t\t\t submodule_prefix,\n \t\t\t\t\t\t recurse_submodules,\n \t\t\t\t\t\t verbosity < 0);\ndiff --git a/submodule.c b/submodule.c\nindex 19dc6a6..51d48c2 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -588,13 +588,13 @@ static void calculate_changed_submodule_paths(void)\n \tinitialized_fetch_ref_tips = 0;\n }\n\n-int fetch_populated_submodules(int num_options, const char **options,\n+int fetch_populated_submodules(const struct argv_array *options,\n \t\t\t const char *prefix, int command_line_option,\n \t\t\t int quiet)\n {\n-\tint i, result = 0, argc = 0, default_argc;\n+\tint i, result = 0;\n \tstruct child_process cp;\n-\tconst char **argv;\n+\tstruct argv_array argv = ARGV_ARRAY_INIT;\n \tstruct string_list_item *name_for_path;\n \tconst char *work_tree = get_git_work_tree();\n \tif (!work_tree)\n@@ -604,17 +604,13 @@ int fetch_populated_submodules(int num_options, const char **options,\n \t\tif (read_cache() < 0)\n \t\t\tdie(\"index file corrupt\");\n\n-\t/* 6: \"fetch\" (options) --recurse-submodules-default default \"--submodule-prefix\" prefix NULL */\n-\targv = xcalloc(num_options + 6, sizeof(const char *));\n-\targv[argc++] = \"fetch\";\n-\tfor (i = 0; i < num_options; i++)\n-\t\targv[argc++] = options[i];\n-\targv[argc++] = \"--recurse-submodules-default\";\n-\tdefault_argc = argc++;\n-\targv[argc++] = \"--submodule-prefix\";\n+\targv_array_push(&argv, \"fetch\");\n+\tfor (i = 0; i < options->argc; i++)\n+\t\targv_array_push(&argv, options->argv[i]);\n+\targv_array_push(&argv, \"--recurse-submodules-default\");\n+\t/* default value, \"--submodule-prefix\" and its value are added later */\n\n \tmemset(&cp, 0, sizeof(cp));\n-\tcp.argv = argv;\n \tcp.env = local_repo_env;\n \tcp.git_cmd = 1;\n \tcp.no_stdin = 1;\n@@ -674,16 +670,21 @@ int fetch_populated_submodules(int num_options, const char **options,\n \t\t\tif (!quiet)\n \t\t\t\tprintf(\"Fetching submodule %s%s\\n\", prefix, ce->name);\n \t\t\tcp.dir = submodule_path.buf;\n-\t\t\targv[default_argc] = default_argv;\n-\t\t\targv[argc] = submodule_prefix.buf;\n+\t\t\targv_array_push(&argv, default_argv);\n+\t\t\targv_array_push(&argv, \"--submodule-prefix\");\n+\t\t\targv_array_push(&argv, submodule_prefix.buf);\n+\t\t\tcp.argv = argv.argv;\n \t\t\tif (run_command(&cp))\n \t\t\t\tresult = 1;\n+\t\t\targv_array_pop(&argv);\n+\t\t\targv_array_pop(&argv);\n+\t\t\targv_array_pop(&argv);\n \t\t}\n \t\tstrbuf_release(&submodule_path);\n \t\tstrbuf_release(&submodule_git_dir);\n \t\tstrbuf_release(&submodule_prefix);\n \t}\n-\tfree(argv);\n+\targv_array_clear(&argv);\n out:\n \tstring_list_clear(&changed_submodule_paths, 1);\n \treturn result;\ndiff --git a/submodule.h b/submodule.h\nindex e105b0e..594b50d 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -2,6 +2,7 @@\n #define SUBMODULE_H\n\n struct diff_options;\n+struct argv_array;\n\n enum {\n \tRECURSE_SUBMODULES_ON_DEMAND = -1,\n@@ -23,7 +24,7 @@ void show_submodule_summary(FILE *f, const char *path,\n \t\tconst char *del, const char *add, const char *reset);\n void set_config_fetch_recurse_submodules(int value);\n void check_for_new_submodule_commits(unsigned char new_sha1[20]);\n-int fetch_populated_submodules(int num_options, const char **options,\n+int fetch_populated_submodules(const struct argv_array *options,\n \t\t\t const char *prefix, int command_line_option,\n \t\t\t int quiet);\n unsigned is_submodule_modified(const char *path, int ignore_untracked);\n-- \n1.7.12.149.g47e61ec\n"},{"id":"198405","messageId":"1346880139-2281-1-git-send-email-ComputerDruid@gmail.com","threadId":"31187","inReplyTo":"20120901112251.GA11445@sigill.intra.peff.net","subject":"[PATCHv2] fetch --all: pass --tags/--no-tags through to each remote","fromName":"Dan Johnson","fromEmail":"computerdruid@gmail.com","sentAt":"2012-09-05T21:22:19Z","receivedAt":"2012-09-05T21:22:19Z","isPatch":false,"sender":{"key":"computerdruid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/34696?v=4"},"body":"When fetch is invoked with --all, we need to pass the tag-following\npreference to each individual fetch; without this, we will always\nauto-follow tags, preventing us from fetching the remote tags into a\nremote-specific namespace, for example.\n\nReported-by: Oswald Buddenhagen \nSigned-off-by: Dan Johnson \n---\nOn Sat, Sep 1, 2012 at 7:22 AM, Jeff King wrote:\n>Hmm. We allocate argv in fetch_multiple like this:\n>\n> const char *argv[12] = { \"fetch\", \"--append\" };\n>\n>and then add a bunch of options to it, along with the name of the\n>remote. By my count, the current code can hit exactly 12 (including the\n>terminating NULL) if all options are set. Your patch would make it\n>possible to overflow. Of course, I may be miscounting since it is\n>extremely error-prone to figure out the right number by tracing each\n>possible conditional.\n>\n>Maybe we should switch it to a dynamic argv_array? Like this:\n>\n> [1/2]: argv-array: add pop function\n> [2/2]: fetch: use argv_array instead of hand-building arrays\n\nThis version is re-rolled to be on top of jk/argv-array, avoiding the issue of\nthe fixed-size array entirely. If needed, we could of course use the old\nversion of this patch and bump the number, but I figure this is preferable.\n\nI've also added some test cases to cover this behavior, but I'm not entirely\nhappy with them. I'm not sure if/how we should be testing the pass-through\nbehavior of various arguments with fetch --all, but if so, we should probably do\nso more thouroughly than I have here, but that just seems like combining\ntogether tests of two unrelated things. It might just make more sense to ignore\nit and drop these tests, I don't know.\n\nSorry this took me a few days to send, I just kept not getting around to it.\n\n builtin/fetch.c | 4 ++++\n t/t5514-fetch-multiple.sh | 29 +++++++++++++++++++++++++++++\n 2 files changed, 33 insertions(+)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 6196e91..4494aed 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -858,6 +858,10 @@ static void add_options_to_argv(struct argv_array *argv)\n \t\targv_array_push(argv, \"--recurse-submodules\");\n \telse if (recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND)\n \t\targv_array_push(argv, \"--recurse-submodules=on-demand\");\n+\tif (tags == TAGS_SET)\n+\t\targv_array_push(argv, \"--tags\");\n+\telse if (tags == TAGS_UNSET)\n+\t\targv_array_push(argv, \"--no-tags\");\n \tif (verbosity >= 2)\n \t\targv_array_push(argv, \"-v\");\n \tif (verbosity >= 1)\ndiff --git a/t/t5514-fetch-multiple.sh b/t/t5514-fetch-multiple.sh\nindex 227dd56..cbd2460 100755\n--- a/t/t5514-fetch-multiple.sh\n+++ b/t/t5514-fetch-multiple.sh\n@@ -151,4 +151,33 @@ test_expect_success 'git fetch --multiple (ignoring skipFetchAll)' '\n \t test_cmp ../expect output)\n '\n \n+cat > expect << EOF\n+EOF\n+\n+test_expect_success 'git fetch --all --no-tags' '\n+\t(git clone one test5 &&\n+\t git clone test5 test6 &&\n+\t (cd test5 && git tag test-tag) &&\n+\t cd test6 &&\n+\t git fetch --all --no-tags &&\n+\t git tag >output &&\n+\t test_cmp ../expect output)\n+'\n+\n+cat > expect << EOF\n+test-tag\n+EOF\n+\n+test_expect_success 'git fetch --all --tags' '\n+\t(git clone one test7 &&\n+\t git clone test7 test8 &&\n+\t (cd test7 &&\n+ test_commit test-tag &&\n+ git reset --hard HEAD^) &&\n+\t cd test8 &&\n+\t git fetch --all --tags &&\n+\t git tag >output &&\n+\t test_cmp ../expect output)\n+'\n+\n test_done\n-- \n1.7.11.1\n"},{"id":"198532","messageId":"7v1uidzrre.fsf@alter.siamese.dyndns.org","threadId":"31187","inReplyTo":"1346880139-2281-1-git-send-email-ComputerDruid@gmail.com","subject":"Re: [PATCHv2] fetch --all: pass --tags/--no-tags through to each remote","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-07T17:07:01Z","receivedAt":"2012-09-07T17:07:01Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dan Johnson writes:\n\n> When fetch is invoked with --all, we need to pass the tag-following\n> preference to each individual fetch; without this, we will always\n> auto-follow tags, preventing us from fetching the remote tags into a\n> remote-specific namespace, for example.\n>\n> Reported-by: Oswald Buddenhagen \n> Signed-off-by: Dan Johnson \n> ---\n> On Sat, Sep 1, 2012 at 7:22 AM, Jeff King wrote:\n>>Hmm. We allocate argv in fetch_multiple like this:\n>>\n>> const char *argv[12] = { \"fetch\", \"--append\" };\n>>\n>>and then add a bunch of options to it, along with the name of the\n>>remote. By my count, the current code can hit exactly 12 (including the\n>>terminating NULL) if all options are set. Your patch would make it\n>>possible to overflow. Of course, I may be miscounting since it is\n>>extremely error-prone to figure out the right number by tracing each\n>>possible conditional.\n>>\n>>Maybe we should switch it to a dynamic argv_array? Like this:\n>>\n>> [1/2]: argv-array: add pop function\n>> [2/2]: fetch: use argv_array instead of hand-building arrays\n>\n> This version is re-rolled to be on top of jk/argv-array, avoiding the issue of\n> the fixed-size array entirely. If needed, we could of course use the old\n> version of this patch and bump the number, but I figure this is preferable.\n\nThanks. Queued.\n"}]}