{"thread":{"id":"30670","subject":"git version statistics","startedAt":"2012-05-31T11:48:02Z","lastAt":"2012-07-09T14:59:57Z","messageCount":84,"participants":["Jeff King","Stephen Bash","Junio C Hamano","Tomas Carnecky","Thomas Rast","Jonathan Nieder","Stefano Lattarini"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"192559","messageId":"20120531114801.GA21367@sigill.intra.peff.net","threadId":"30670","inReplyTo":null,"subject":"git version statistics","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-31T11:48:02Z","receivedAt":"2012-05-31T11:48:02Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Just for fun, I've assembled a few statistics on git client versions\nthat hit github.com. These are collected from the http user-agent\nstrings provided by smart-http requests (we don't allow dumb http at all\nthese days, and the git protocol does not provide any version\ninformation). It includes only the git/* entries (which are the vast\nmajority of clients hitting \"info/refs?service=.*\"; the second biggest\nis JGit/*). And I counted each version only a single time from each IP\n(so this would skew if you had a lot of clients behind a single IP;\ntheir version would be counted only once).\n\nHere are the results from the last few days:\n\n git/1.7.11.x | 0.1% (0%) | \n git/1.7.10.x | 21.1% (21%) | **********************************\n git/1.7.9.x | 11.4% (32%) | ******************\n git/1.7.8.x | 24.1% (56%) | ****************************************\n git/1.7.7.x | 12.2% (68%) | ********************\n git/1.7.6.x | 1.8% (70%) | **\n git/1.7.5.x | 8.2% (78%) | *************\n git/1.7.4.x | 7.2% (86%) | ***********\n git/1.7.3.x | 1.9% (87%) | ***\n git/1.7.2.x | 3.0% (90%) | ****\n git/1.7.1.x | 2.1% (93%) | ***\n git/1.7.0.x | 7.0% (99%) | ***********\n git/1.6.6.x | 0.0% (100%) | \n\nIn this table, I've collapsed all of the x.y.z.* versions into a single\nstring to get an overview (a more detailed table is below). The first\npercentage is the portion of the total requests; the second is a\ncumulative portion (so, for example, 90% of clients are running v1.7.2.x\nor higher, though only 3% are running v1.7.2.x itself).\n\nNote that these numbers are skewed away from git versions lower than\nv1.6.6, because that is when we added smart http support. Requests over\ngit:// or ssh are not included at all in these results.\n\nFor comparison, here's the same table from mid-September of 2011 (v1.7.7\nwas in -rc1 at the time):\n\n git/1.7.7.x | 0.2% (0%) | \n git/1.7.6.x | 19.3% (19%) | *****************\n git/1.7.5.x | 8.0% (27%) | *******\n git/1.7.4.x | 43.8% (71%) | ****************************************\n git/1.7.3.x | 10.7% (82%) | *********\n git/1.7.2.x | 4.9% (86%) | ****\n git/1.7.1.x | 4.7% (91%) | ****\n git/1.7.0.x | 8.2% (99%) | *******\n git/1.6.6.x | 0.1% (100%) | \n\nSo it seems that a large proportion of git users (or at least github\nusers) stay within a few versions of the most current. In both cases,\n70% are within 4 major releases of the most recent version.\n\nHere's a more detailed table (from recent data), showing individual\nw.x.y.z versions:\n\n git/1.7.10.3 | 3.6% (3%) | *******\n git/1.7.10.2 | 4.4% (8%) | *********\n git/1.7.10.1 | 1.5% (9%) | ***\n git/1.7.10 | 2.4% (11%) | *****\n git/1.7.9.6 | 0.2% (12%) | \n git/1.7.9.5 | 9.2% (21%) | *******************\n git/1.7.9.4 | 1.2% (22%) | **\n git/1.7.9.3 | 0.3% (22%) | \n git/1.7.9.2 | 0.4% (23%) | \n git/1.7.9.1 | 0.4% (23%) | \n git/1.7.9 | 1.1% (24%) | **\n git/1.7.8.6 | 0.3% (25%) | \n git/1.7.8.5 | 0.0% (25%) | \n git/1.7.8.4 | 0.7% (25%) | *\n git/1.7.8.3 | 12.2% (38%) | *************************\n git/1.7.8.2 | 18.8% (56%) | ****************************************\n git/1.7.8.1 | 0.1% (56%) | \n git/1.7.8 | 0.3% (57%) | \n git/1.7.7.6 | 0.5% (57%) | *\n git/1.7.7.4 | 0.2% (57%) | \n git/1.7.7.3 | 0.4% (58%) | \n git/1.7.7.2 | 0.1% (58%) | \n git/1.7.7.1 | 0.0% (58%) | \n git/1.7.7 | 0.7% (59%) | *\n git/1.7.6.5 | 0.1% (59%) | \n git/1.7.6.4 | 0.2% (59%) | \n git/1.7.6.3 | 0.0% (59%) | \n git/1.7.6.1 | 0.5% (59%) | *\n git/1.7.6 | 0.7% (60%) | *\n git/1.7.5.4 | 10.9% (71%) | ***********************\n git/1.7.5.3 | 0.1% (71%) | \n git/1.7.5.2 | 0.1% (71%) | \n git/1.7.5.1 | 0.2% (71%) | \n git/1.7.5 | 0.1% (71%) | \n git/1.7.4.5 | 0.4% (72%) | \n git/1.7.4.4 | 3.3% (75%) | *******\n git/1.7.4.3 | 0.0% (75%) | \n git/1.7.4.2 | 0.0% (75%) | \n git/1.7.4.1 | 5.3% (81%) | ***********\n git/1.7.4 | 0.2% (81%) | \n git/1.7.3.5 | 0.2% (81%) | \n git/1.7.3.4 | 1.2% (82%) | **\n git/1.7.3.3 | 0.1% (82%) | \n git/1.7.3.2 | 0.4% (83%) | \n git/1.7.3.1 | 0.1% (83%) | \n git/1.7.3 | 0.1% (83%) | \n git/1.7.2.5 | 3.6% (86%) | *******\n git/1.7.2.3 | 0.3% (87%) | \n git/1.7.2.2 | 0.1% (87%) | \n git/1.7.2.1 | 0.0% (87%) | \n git/1.7.2 | 0.1% (87%) | \n git/1.7.1.1 | 0.1% (87%) | \n git/1.7.1 | 2.8% (90%) | ******\n git/1.7.0.6 | 0.0% (90%) | \n git/1.7.0.5 | 0.0% (90%) | \n git/1.7.0.4 | 7.0% (97%) | **************\n git/1.7.0 | 2.6% (99%) | *****\n git/1.6.6 | 0.0% (100%) | \n\nThe interesting thing to me is how spiky it is, and where the spikes\nfall. I would expect to see a spike around the highest maint release of\neach major version (so v1.7.8.6, for example, with many fewer installs\nof v1.7.8.5, v1.7.8.4, and so forth). But that's not what happens. The\nmost popular v1.7.8.x versions are .3 and .2, and hardly anybody\nbothered to move to v1.7.8.6.\n\nI can only assume these are skewed by some widely-used binary\ndistribution being locked onto particular versions (e.g., the spike at\nv1.7.2.5 represents Debian stable).\n\nIf anybody has suggestions for other interesting analyses to perform,\nlet me know.\n\n-Peff\n"},{"id":"192560","messageId":"20120531120018.GA24986@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120531114801.GA21367@sigill.intra.peff.net","subject":"Re: git version statistics","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-31T12:00:19Z","receivedAt":"2012-05-31T12:00:19Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 31, 2012 at 07:48:01AM -0400, Jeff King wrote:\n\n> Just for fun, I've assembled a few statistics on git client versions\n> that hit github.com. These are collected from the http user-agent\n> strings provided by smart-http requests (we don't allow dumb http at all\n> these days, and the git protocol does not provide any version\n> information).\n\nWe have capabilities, and I looked into trying to fingerprint client\nversions based on the capabilities. Unfortunately, it's extremely\ncoarse-grained, because we just don't add capabilities very often.\n\nI came up with this list of fetch-pack capabilities, and when they were\nintroduced:\n\n - multi_ack v0.99.9\n - thin-pack v1.3.0\n - side-band v1.4.1\n - side-band-64k v1.4.3\n - ofs-delta v1.4.4\n - shallow v1.5.0\n - no-progress v1.5.1\n - include-tag v1.5.5\n - multi_ack_detailed v1.6.6\n - no-done v1.7.5\n\nBut note that \"shallow\" is not requested explicitly by the client; you\ncan detect it by the client sending \"shallow\" commands, but then only if\nit is a shallow repository. Similarly, \"no-progress\" is only requested\nif the client is run with \"--no-progress\" or without a tty.\n\nThe \"no-done\" capability is only requested in stateless-rpc mode (i.e.,\nover smart-http). So we can use it only for smart-http, in which case we\nhave more accurate numbers already (because git puts the version in the\nhttp user-agent string).\n\nSo realistically you are looking at identifying whether a client version\nis >= v1.6.6 (if it has multi_ack_detailed) or >= v1.5.5 (if it has\ninclude-tag). Beyond that it gets uselessly old, and that's really not\nvery fine-grained at all.\n\nI'm really tempted to do something like the patch below, which adds an\nagent field to the capability string. It wouldn't help with identifying\nolder versions, but eventually all versions of git would send it (and\nthose that didn't could be stuck in the \"wow, that's old\" bin).\n\n-Peff\n\n-- >8 --\nSubject: include agent identifier in capability string\n\nInstead of having the client advertise a particular version\nnumber in the git protocol, we have managed extensions and\nbackwards compatibility by having clients and servers\nadvertise capabilities that they support. This is far more\nrobust than having each side consult a table of\nknown versions, and provides sufficient information for the\nprotocol interaction to complete.\n\nHowever, it does not allow servers to keep statistics on\nwhich client versions are being used. This information is\nnot necessary to complete the network request (the\ncapabilities provide enough information for that), but it\nmay be helpful to conduct a general survey of client\nversions in use.\n\nWe already send the client version in the user-agent header\nfor http requests; adding it here would allow us to gather\nsimilar statistics for non-http requests.\n\nSigned-off-by: Jeff King \n---\n builtin/fetch-pack.c | 1 +\n builtin/send-pack.c | 6 ++++--\n 2 files changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex 149db88..f3b8422 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -327,6 +327,7 @@ static int find_common(int fd[2], unsigned char *result_sha1,\n \t\t\tif (args.no_progress) strbuf_addstr(&c, \" no-progress\");\n \t\t\tif (args.include_tag) strbuf_addstr(&c, \" include-tag\");\n \t\t\tif (prefer_ofs_delta) strbuf_addstr(&c, \" ofs-delta\");\n+\t\t\tstrbuf_addf(&c, \" agent=git/%s\", git_version_string);\n \t\t\tpacket_buf_write(&req_buf, \"want %s%s\\n\", remote_hex, c.buf);\n \t\t\tstrbuf_release(&c);\n \t\t} else\ndiff --git a/builtin/send-pack.c b/builtin/send-pack.c\nindex d5d7105..3d87c71 100644\n--- a/builtin/send-pack.c\n+++ b/builtin/send-pack.c\n@@ -306,11 +306,13 @@ int send_pack(struct send_pack_args *args,\n \t\t\tint quiet = quiet_supported && (args->quiet || !args->progress);\n \n \t\t\tif (!cmds_sent && (status_report || use_sideband || args->quiet)) {\n-\t\t\t\tpacket_buf_write(&req_buf, \"%s %s %s%c%s%s%s\",\n+\t\t\t\tpacket_buf_write(&req_buf,\n+\t\t\t\t\t\t \"%s %s %s%c%s%s%s agent=git/%s\",\n \t\t\t\t\t\t old_hex, new_hex, ref->name, 0,\n \t\t\t\t\t\t status_report ? \" report-status\" : \"\",\n \t\t\t\t\t\t use_sideband ? \" side-band-64k\" : \"\",\n-\t\t\t\t\t\t quiet ? \" quiet\" : \"\");\n+\t\t\t\t\t\t quiet ? \" quiet\" : \"\",\n+\t\t\t\t\t\t git_version_string);\n \t\t\t}\n \t\t\telse\n \t\t\t\tpacket_buf_write(&req_buf, \"%s %s %s\",\n-- \n1.7.11.rc0.35.ga99aee0\n"},{"id":"192596","messageId":"981b755b-ca86-4320-a4fc-8aa28caa099d@mail","threadId":"30670","inReplyTo":"20120531114801.GA21367@sigill.intra.peff.net","subject":"Re: git version statistics","fromName":"Stephen Bash","fromEmail":"bash@genarts.com","sentAt":"2012-05-31T15:20:46Z","receivedAt":"2012-05-31T15:20:46Z","isPatch":false,"sender":{"key":"bash@genarts.com","avatar":null},"body":"----- Original Message -----\n> From: \"Jeff King\" \n> Sent: Thursday, May 31, 2012 7:48:02 AM\n> Subject: git version statistics\n> \n> Just for fun, I've assembled a few statistics on git client versions\n> that hit github.com.\n\nCertainly an interesting read... Thanks!\n\n> The interesting thing to me is how spiky it is, and where the spikes\n> fall. I would expect to see a spike around the highest maint release\n> of each major version (so v1.7.8.6, for example, with many fewer\n> installs of v1.7.8.5, v1.7.8.4, and so forth). But that's not what\n> happens. The most popular v1.7.8.x versions are .3 and .2, and hardly\n> anybody bothered to move to v1.7.8.6.\n\nI wonder if the spikes correlate with time between releases? For open source projects I tend to download the most recent when setting up a new machine (or after encountering a nasty bug), and then not upgrade for a while. So in that mode of operation, releases that are \"the newest\" for the longest would get more users... (though at least on my Mac the homebrew project is helping me break the habit and stay more up-to-date)\n \n> I can only assume these are skewed by some widely-used binary\n> distribution being locked onto particular versions (e.g., the spike at\n> v1.7.2.5 represents Debian stable).\n\n... but then again, that's probably a better reason for large swaths of users to have a fixed version...\n\nThanks again for the interesting read.\n\nStephen\n"},{"id":"192616","messageId":"7v396g6t53.fsf@alter.siamese.dyndns.org","threadId":"30670","inReplyTo":"20120531120018.GA24986@sigill.intra.peff.net","subject":"Re: git version statistics","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-31T19:35:20Z","receivedAt":"2012-05-31T19:35:20Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\n> index 149db88..f3b8422 100644\n> --- a/builtin/fetch-pack.c\n> +++ b/builtin/fetch-pack.c\n> @@ -327,6 +327,7 @@ static int find_common(int fd[2], unsigned char *result_sha1,\n> \t\t\tif (args.no_progress) strbuf_addstr(&c, \" no-progress\");\n> \t\t\tif (args.include_tag) strbuf_addstr(&c, \" include-tag\");\n> \t\t\tif (prefer_ofs_delta) strbuf_addstr(&c, \" ofs-delta\");\n> +\t\t\tstrbuf_addf(&c, \" agent=git/%s\", git_version_string);\n> \t\t\tpacket_buf_write(&req_buf, \"want %s%s\\n\", remote_hex, c.buf);\n> \t\t\tstrbuf_release(&c);\n\nEven though the version string GIT-VERSION-GEN script deduces from\nthe repository version is designed to be safe, in general \"version\"\nfile can contain a string with whitespaces. You may want to be\ncareful about that in the above.\n\nDo we want a similar identifier string on the other side of the\nconnection?\n"},{"id":"192633","messageId":"20120601085211.GB32340@sigill.intra.peff.net","threadId":"30670","inReplyTo":"981b755b-ca86-4320-a4fc-8aa28caa099d@mail","subject":"Re: git version statistics","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-01T08:52:11Z","receivedAt":"2012-06-01T08:52:11Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 31, 2012 at 11:20:46AM -0400, Stephen Bash wrote:\n\n> > The interesting thing to me is how spiky it is, and where the spikes\n> > fall. I would expect to see a spike around the highest maint release\n> > of each major version (so v1.7.8.6, for example, with many fewer\n> > installs of v1.7.8.5, v1.7.8.4, and so forth). But that's not what\n> > happens. The most popular v1.7.8.x versions are .3 and .2, and hardly\n> > anybody bothered to move to v1.7.8.6.\n> \n> I wonder if the spikes correlate with time between releases? For open\n> source projects I tend to download the most recent when setting up a\n> new machine (or after encountering a nasty bug), and then not upgrade\n> for a while. So in that mode of operation, releases that are \"the\n> newest\" for the longest would get more users... (though at least on my\n> Mac the homebrew project is helping me break the habit and stay more\n> up-to-date)\n\nGood point. If you assume that users pick a random day to upgrade or\ninstall and choose the latest version, then you will get a non-uniform\ndistribution. Because the release dates are non-uniform, their time\nspent as the latest is not even. There may also be natural variations in\ninstallations over time (e.g., over holidays).\n\nHere are the release dates for the v1.7.8.x series, as well as the\nadjacent master releases:\n\n v1.7.8 2011-12-02\n v1.7.8.1 2011-12-21\n v1.7.8.2 2011-12-28\n v1.7.8.3 2012-01-06\n v1.7.8.4 2012-01-18\n v1.7.9 2012-01-27\n v1.7.8.5 2012-02-26\n v1.7.10 2012-04-06\n v1.7.8.6 2012-04-26\n\nSo .2 and .3 were latest for 9 and 12 days, respectively. However, .4\nwas also the latest for 9 days (until v1.7.9 came out), but does not\nhave as many users. So why did nobody bother upgrading to v1.7.8.4?\nAnd why wouldn't v1.7.8 have a spike, since it was at the top for 19\ndays?\n\nI can see why v1.7.8.5 and v1.7.8.6 are the way they are (they were\nnever latest, and most people would just install v1.7.9 or v1.7.10\ninstead).\n\nSo I think your theory probably explains some of the data, but not all\n(and it seems that most people don't really seem to care about old maint\nreleases once a new master release is out).\n\n-Peff\n"},{"id":"192637","messageId":"20120601090337.GC32340@sigill.intra.peff.net","threadId":"30670","inReplyTo":"7v396g6t53.fsf@alter.siamese.dyndns.org","subject":"Re: git version statistics","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-01T09:03:38Z","receivedAt":"2012-06-01T09:03:38Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 31, 2012 at 12:35:20PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\n> > index 149db88..f3b8422 100644\n> > --- a/builtin/fetch-pack.c\n> > +++ b/builtin/fetch-pack.c\n> > @@ -327,6 +327,7 @@ static int find_common(int fd[2], unsigned char *result_sha1,\n> > \t\t\tif (args.no_progress) strbuf_addstr(&c, \" no-progress\");\n> > \t\t\tif (args.include_tag) strbuf_addstr(&c, \" include-tag\");\n> > \t\t\tif (prefer_ofs_delta) strbuf_addstr(&c, \" ofs-delta\");\n> > +\t\t\tstrbuf_addf(&c, \" agent=git/%s\", git_version_string);\n> > \t\t\tpacket_buf_write(&req_buf, \"want %s%s\\n\", remote_hex, c.buf);\n> > \t\t\tstrbuf_release(&c);\n> \n> Even though the version string GIT-VERSION-GEN script deduces from\n> the repository version is designed to be safe, in general \"version\"\n> file can contain a string with whitespaces. You may want to be\n> careful about that in the above.\n\nYeah, I agree. I should have been more clear that this patch was an RFC\nabout the idea, not the implementation.\n\nWe could also strip off junk like \".dirty\" which is unlikely to be\ninteresting for statistical reporting. On the other hand, it could be\nuseful for somebody debugging, and it can always be stripped later.\n\nI don't know if anybody cares about the security or privacy implications\nof advertising your client version. Maybe it should be configurable?\n\n> Do we want a similar identifier string on the other side of the\n> connection?\n\nWe could. I don't see much point, unless you were going to conduct a\nsimilar survey by hitting random IPs looking for git ports (but even\nthen, you're not likely to turn up much, because you have to know a repo\nname before you can convince git to show a capability string). I suppose\nit could also help with debugging if your client is having trouble\ntalking to a server that is not under your control.\n\nSome traditional security advice I have heard is that servers should not\nadvertise their versions, as it makes it more obvious what holes they\nhave. Personally, I find that argument to be mostly security through\nobscurity. If I have an exploit for version X, it's generally just as\neasy to try it as it is to check the version (unless the exploit\nrequires a lot of effort, like guessing a value that might take\nthousands of tries).\n\n-Peff\n"},{"id":"192666","messageId":"7vzk8n3x5e.fsf@alter.siamese.dyndns.org","threadId":"30670","inReplyTo":"20120601090337.GC32340@sigill.intra.peff.net","subject":"Re: git version statistics","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-01T14:49:17Z","receivedAt":"2012-06-01T14:49:17Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I don't know if anybody cares about the security or privacy implications\n> of advertising your client version. Maybe it should be configurable?\n\nI do not think it is worth it.\n\nMy initial reaction to the patch was a bit of trouble with the word\n\"agent\", as we do not call Git acting on behalf of the end user \"an\nagent\" in general. But it could be used as an excuse for not giving\nan extra knob to tweak, as you generally do not muck with User-Agent\nstrings, either ;-).\n\n>> Do we want a similar identifier string on the other side of the\n>> connection?\n>\n> We could. I don't see much point, unless you were going to conduct a\n> similar survey by hitting random IPs looking for git ports (but even\n> then, you're not likely to turn up much, because you have to know a repo\n> name before you can convince git to show a capability string). I suppose\n> it could also help with debugging if your client is having trouble\n> talking to a server that is not under your control.\n\nThe latter use case was exactly what I had in mind.\n\n> Some traditional security advice I have heard is that servers should not\n> advertise their versions, as it makes it more obvious what holes they\n> have. Personally, I find that argument to be mostly security through\n> obscurity.\n\nI do, too, but shipping with a configuration knob to optionally turn\nit off would be sufficient.\n"},{"id":"192718","messageId":"20120602163248.GD15017@sigill.intra.peff.net","threadId":"30670","inReplyTo":"7vzk8n3x5e.fsf@alter.siamese.dyndns.org","subject":"Re: git version statistics","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-02T16:32:48Z","receivedAt":"2012-06-02T16:32:48Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 01, 2012 at 07:49:17AM -0700, Junio C Hamano wrote:\n\n> My initial reaction to the patch was a bit of trouble with the word\n> \"agent\", as we do not call Git acting on behalf of the end user \"an\n> agent\" in general.\n\nYeah, I don't especially like the term \"agent\". I had initially called\nit \"version\", but rejected that for two reasons:\n\n 1. It is not just a version, but also telling what software is in use\n (so I would expect git to write git/v1.7.10, and other\n implementations to write write dulwich/1.2.3 or whatever).\n\n 2. I didn't want it to be confused as a protocol version.\n\nBut maybe those are non-issues. It should be fairly obvious what it is\nwhen you see even one example of the value.\n\n> > Some traditional security advice I have heard is that servers should not\n> > advertise their versions, as it makes it more obvious what holes they\n> > have. Personally, I find that argument to be mostly security through\n> > obscurity.\n> \n> I do, too, but shipping with a configuration knob to optionally turn\n> it off would be sufficient.\n\nI think the most sensible thing is to just add a Makefile variable\nthat defaults to $(GIT_VERSION), and let people override it if they want\nprivacy. The http user-agent variable actually respects an environment\nvariable, but I don't see much point in going that far.\n\nI'll cook up a new version of the patch.\n\n-Peff\n"},{"id":"192721","messageId":"1338656374-ner-2980@calvin","threadId":"30670","inReplyTo":"20120602163248.GD15017@sigill.intra.peff.net","subject":"Re: git version statistics","fromName":"Tomas Carnecky","fromEmail":"tomas.carnecky@gmail.com","sentAt":"2012-06-02T16:59:34Z","receivedAt":"2012-06-02T16:59:34Z","isPatch":false,"sender":{"key":"tomas.carnecky@gmail.com","avatar":null},"body":"On Sat, 02 Jun 2012 12:32:48 -0400, Jeff King wrote:\n> On Fri, Jun 01, 2012 at 07:49:17AM -0700, Junio C Hamano wrote:\n> \n> > My initial reaction to the patch was a bit of trouble with the word\n> > \"agent\", as we do not call Git acting on behalf of the end user \"an\n> > agent\" in general.\n\nWikipedia defines User agent as 'a software [...] that is acting on behalf of\na user'. That fits this situation quite well, don't you think?\n"},{"id":"192725","messageId":"20120602184948.GA14269@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120602163248.GD15017@sigill.intra.peff.net","subject":"Re: git version statistics","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-02T18:49:48Z","receivedAt":"2012-06-02T18:49:48Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jun 02, 2012 at 12:32:48PM -0400, Jeff King wrote:\n\n> I'll cook up a new version of the patch.\n\nThe refactoring ended up expanding this into a few patches:\n\n [1/4]: move git_version_string into version.c\n [2/4]: version: add git_user_agent function\n [3/4]: http: get default user-agent from git_user_agent\n [4/4]: include agent identifier in capability string\n\n-Peff\n"},{"id":"192726","messageId":"20120602185142.GA14369@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120602184948.GA14269@sigill.intra.peff.net","subject":"[PATCH 1/4] move git_version_string into version.c","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-02T18:51:42Z","receivedAt":"2012-06-02T18:51:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The global git_version_string currently lives in git.c, but\ndoesn't have anything to do with the git wrapper. Let's move\nit into its own file, where it will be more appropriate to\nbuild more version-related functions.\n\nSigned-off-by: Jeff King \n---\n Makefile | 8 ++++++--\n builtin.h | 1 -\n builtin/log.c | 1 +\n git.c | 2 --\n help.c | 1 +\n version.c | 4 ++++\n version.h | 6 ++++++\n 7 files changed, 18 insertions(+), 5 deletions(-)\n create mode 100644 version.c\n create mode 100644 version.h\n\ndiff --git a/Makefile b/Makefile\nindex 4592f1f..b394f85 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -799,6 +799,7 @@ LIB_OBJS += usage.o\n LIB_OBJS += userdiff.o\n LIB_OBJS += utf8.o\n LIB_OBJS += varint.o\n+LIB_OBJS += version.o\n LIB_OBJS += walker.o\n LIB_OBJS += wrapper.o\n LIB_OBJS += write_or_die.o\n@@ -1962,7 +1963,7 @@ strip: $(PROGRAMS) git$X\n \t$(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X\n \n git.o: common-cmds.h\n-git.sp git.s git.o: EXTRA_CPPFLAGS = -DGIT_VERSION='\"$(GIT_VERSION)\"' \\\n+git.sp git.s git.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_HTML_PATH=\"$(htmldir_SQ)\"' \\\n \t'-DGIT_MAN_PATH=\"$(mandir_SQ)\"' \\\n \t'-DGIT_INFO_PATH=\"$(infodir_SQ)\"'\n@@ -1979,6 +1980,9 @@ builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_MAN_PATH=\"$(mandir_SQ)\"' \\\n \t'-DGIT_INFO_PATH=\"$(infodir_SQ)\"'\n \n+version.sp version.s version.o: EXTRA_CPPFLAGS = \\\n+\t'-DGIT_VERSION=\"$(GIT_VERSION)\"'\n+\n $(BUILT_INS): git$X\n \t$(QUIET_BUILT_IN)$(RM) $@ && \\\n \tln git$X $@ 2>/dev/null || \\\n@@ -2089,7 +2093,7 @@ configure: configure.ac\n \t$(RM) $<+\n \n # These can record GIT_VERSION\n-git.o git.spec http.o \\\n+version.o git.spec http.o \\\n \t$(patsubst %.sh,%,$(SCRIPT_SH)) \\\n \t$(patsubst %.perl,%,$(SCRIPT_PERL)) \\\n \t: GIT-VERSION-FILE\ndiff --git a/builtin.h b/builtin.h\nindex 338f540..dea1643 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -9,7 +9,6 @@\n \n #define DEFAULT_MERGE_LOG_LEN 20\n \n-extern const char git_version_string[];\n extern const char git_usage_string[];\n extern const char git_more_info_string[];\n \ndiff --git a/builtin/log.c b/builtin/log.c\nindex 906dca4..4f1b42a 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -21,6 +21,7 @@\n #include \"parse-options.h\"\n #include \"branch.h\"\n #include \"streaming.h\"\n+#include \"version.h\"\n \n /* Set a default date-time format for git log (\"log.date\" config variable) */\n static const char *default_date_mode = NULL;\ndiff --git a/git.c b/git.c\nindex d232de9..4da3db5 100644\n--- a/git.c\n+++ b/git.c\n@@ -256,8 +256,6 @@ static int handle_alias(int *argcp, const char ***argv)\n \treturn ret;\n }\n \n-const char git_version_string[] = GIT_VERSION;\n-\n #define RUN_SETUP\t\t(1<<0)\n #define RUN_SETUP_GENTLY\t(1<<1)\n #define USE_PAGER\t\t(1<<2)\ndiff --git a/help.c b/help.c\nindex 6012c07..662349d 100644\n--- a/help.c\n+++ b/help.c\n@@ -6,6 +6,7 @@\n #include \"common-cmds.h\"\n #include \"string-list.h\"\n #include \"column.h\"\n+#include \"version.h\"\n \n void add_cmdname(struct cmdnames *cmds, const char *name, int len)\n {\ndiff --git a/version.c b/version.c\nnew file mode 100644\nindex 0000000..ca68653\n--- /dev/null\n+++ b/version.c\n@@ -0,0 +1,4 @@\n+#include \"git-compat-util.h\"\n+#include \"version.h\"\n+\n+const char git_version_string[] = GIT_VERSION;\ndiff --git a/version.h b/version.h\nnew file mode 100644\nindex 0000000..8d6c413\n--- /dev/null\n+++ b/version.h\n@@ -0,0 +1,6 @@\n+#ifndef VERSION_H\n+#define VERSION_H\n+\n+extern const char git_version_string[];\n+\n+#endif /* VERSION_H */\n-- \n1.7.7.7.32.g4b73117\n"},{"id":"192727","messageId":"20120602190112.GB14369@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120602184948.GA14269@sigill.intra.peff.net","subject":"[PATCH 2/4] version: add git_user_agent function","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-02T19:01:12Z","receivedAt":"2012-06-02T19:01:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This is basically a fancy way of saying \"git/$GIT_VERSION\",\nexcept that it is overridable at build-time and through the\nenvironment. Which means that people who don't want to\nadvertise their git version (for privacy or security\nreasons) can tweak it.\n\nSigned-off-by: Jeff King \n---\nThe next patch adapts http.c to use this, and of course the one after\nthat adds support for the git protocol itself. There are a few other\nplaces where the git version leaks publicly, including at least:\n\n 1. in the x-mailer header of send-email\n\n 2. at the bottom of format-patch emails\n\n 3. in the mime boundaries (!) of format-patch emails\n\nSince the default here is functionally identical, and the only people\nwho would care are those interested in masking their version, and nobody\nhas actually come forward and said they want to do that, I am inclined\nnot to worry about it. I'd consider the masking to be more important (if\nit is important at all) on the server side. Perhaps this series will\ncatch the attention of people who do care, and they can decide if they\nwould like to take it farther.\n\n Makefile | 11 +++++++++++\n version.c | 13 +++++++++++++\n version.h | 2 ++\n 3 files changed, 26 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex b394f85..e6e65ca 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -296,6 +296,9 @@ all::\n # the diff algorithm. It gives a nice speedup if your processor has\n # fast unaligned word loads. Does NOT work on big-endian systems!\n # Enabled by default on x86_64.\n+#\n+# Define GIT_USER_AGENT if you want to change how git identifies itself during\n+# network interactions. The default is \"git/$(GIT_VERSION)\".\n \n GIT-VERSION-FILE: FORCE\n \t@$(SHELL_PATH) ./GIT-VERSION-GEN\n@@ -905,6 +908,8 @@ BUILTIN_OBJS += builtin/write-tree.o\n GITLIBS = $(LIB_FILE) $(XDIFF_LIB)\n EXTLIBS =\n \n+GIT_USER_AGENT = git/$(GIT_VERSION)\n+\n #\n # Platform specific tweaks\n #\n@@ -1916,6 +1921,11 @@ SHELL_PATH_CQ_SQ = $(subst ','\\'',$(SHELL_PATH_CQ))\n BASIC_CFLAGS += -DSHELL_PATH='$(SHELL_PATH_CQ_SQ)'\n endif\n \n+GIT_USER_AGENT_SQ = $(subst ','\\'',$(GIT_USER_AGENT))\n+GIT_USER_AGENT_CQ = \"$(subst \",\\\",$(subst \\,\\\\,$(GIT_USER_AGENT)))\"\n+GIT_USER_AGENT_CQ_SQ = $(subst ','\\'',$(GIT_USER_AGENT_CQ))\n+BASIC_CFLAGS += -DGIT_USER_AGENT='$(GIT_USER_AGENT_CQ_SQ)'\n+\n ALL_CFLAGS += $(BASIC_CFLAGS)\n ALL_LDFLAGS += $(BASIC_LDFLAGS)\n \n@@ -2000,6 +2010,7 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \\\n -e 's|@@DIFF@@|$(DIFF_SQ)|' \\\n -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \\\n+ -e 's|@@GIT_USER_AGENT@@|$(GIT_USER_AGENT_SQ)|g' \\\n -e 's|@@LOCALEDIR@@|$(localedir_SQ)|g' \\\n -e 's/@@NO_CURL@@/$(NO_CURL)/g' \\\n -e 's/@@USE_GETTEXT_SCHEME@@/$(USE_GETTEXT_SCHEME)/g' \\\ndiff --git a/version.c b/version.c\nindex ca68653..f98d5a6 100644\n--- a/version.c\n+++ b/version.c\n@@ -2,3 +2,16 @@\n #include \"version.h\"\n \n const char git_version_string[] = GIT_VERSION;\n+\n+const char *git_user_agent(void)\n+{\n+\tstatic const char *agent = NULL;\n+\n+\tif (!agent) {\n+\t\tagent = getenv(\"GIT_USER_AGENT\");\n+\t\tif (!agent)\n+\t\t\tagent = GIT_USER_AGENT;\n+\t}\n+\n+\treturn agent;\n+}\ndiff --git a/version.h b/version.h\nindex 8d6c413..fd9cdd6 100644\n--- a/version.h\n+++ b/version.h\n@@ -3,4 +3,6 @@\n \n extern const char git_version_string[];\n \n+const char *git_user_agent(void);\n+\n #endif /* VERSION_H */\n-- \n1.7.7.7.32.g4b73117\n"},{"id":"192728","messageId":"20120602190308.GC14369@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120602184948.GA14269@sigill.intra.peff.net","subject":"[PATCH 3/4] http: get default user-agent from git_user_agent","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-02T19:03:08Z","receivedAt":"2012-06-02T19:03:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This means we will respect the GIT_USER_AGENT build-time\nconfiguration and run-time environment variable.\n\nSigned-off-by: Jeff King \n---\nNote that this needs to be applied on the recent \"http.o depends\non GIT-VERSION-FILE\" fix by Erik, which just went into master. It\nactually undoes that commit, but that is because the dependency no\nlonger exists.\n\n Makefile | 5 +----\n http.c | 3 ++-\n 2 files changed, 3 insertions(+), 5 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex e6e65ca..62de0b4 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2104,7 +2104,7 @@ configure: configure.ac\n \t$(RM) $<+\n \n # These can record GIT_VERSION\n-version.o git.spec http.o \\\n+version.o git.spec \\\n \t$(patsubst %.sh,%,$(SCRIPT_SH)) \\\n \t$(patsubst %.perl,%,$(SCRIPT_PERL)) \\\n \t: GIT-VERSION-FILE\n@@ -2274,9 +2274,6 @@ attr.sp attr.s attr.o: EXTRA_CPPFLAGS = \\\n gettext.sp gettext.s gettext.o: EXTRA_CPPFLAGS = \\\n \t-DGIT_LOCALE_PATH='\"$(localedir_SQ)\"'\n \n-http.sp http.s http.o: EXTRA_CPPFLAGS = \\\n-\t-DGIT_HTTP_USER_AGENT='\"git/$(GIT_VERSION)\"'\n-\n ifdef NO_EXPAT\n http-walker.sp http-walker.s http-walker.o: EXTRA_CPPFLAGS = -DNO_EXPAT\n endif\ndiff --git a/http.c b/http.c\nindex 5cb87f1..b61ac85 100644\n--- a/http.c\n+++ b/http.c\n@@ -4,6 +4,7 @@\n #include \"run-command.h\"\n #include \"url.h\"\n #include \"credential.h\"\n+#include \"version.h\"\n \n int active_requests;\n int http_is_verbose;\n@@ -299,7 +300,7 @@ static CURL *get_curl_handle(void)\n \t\tcurl_easy_setopt(result, CURLOPT_VERBOSE, 1);\n \n \tcurl_easy_setopt(result, CURLOPT_USERAGENT,\n-\t\tuser_agent ? user_agent : GIT_HTTP_USER_AGENT);\n+\t\tuser_agent ? user_agent : git_user_agent());\n \n \tif (curl_ftp_no_epsv)\n \t\tcurl_easy_setopt(result, CURLOPT_FTP_USE_EPSV, 0);\n-- \n1.7.7.7.32.g4b73117\n"},{"id":"192729","messageId":"20120602190503.GD14369@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120602184948.GA14269@sigill.intra.peff.net","subject":"[PATCH 4/4] include agent identifier in capability string","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-02T19:05:03Z","receivedAt":"2012-06-02T19:05:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Instead of having the client advertise a particular version\nnumber in the git protocol, we have managed extensions and\nbackwards compatibility by having clients and servers\nadvertise capabilities that they support. This is far more\nrobust than having each side consult a table of\nknown versions, and provides sufficient information for the\nprotocol interaction to complete.\n\nHowever, it does not allow servers to keep statistics on\nwhich client versions are being used. This information is\nnot necessary to complete the network request (the\ncapabilities provide enough information for that), but it\nmay be helpful to conduct a general survey of client\nversions in use.\n\nWe already send the client version in the user-agent header\nfor http requests; adding it here allows us to gather\nsimilar statistics for non-http requests.\n\nSigned-off-by: Jeff King \n---\nTwo important changes from the previous round:\n\n 1. We sanitize the agent string to remove non-printable characters and\n whitespace (they are replaced with '.'). This generally shouldn't\n happen, but is a defensive measure against breaking the protocol.\n\n 2. The server half of the connection will advertise their versions in\n the capability strings, too.\n\n builtin/fetch-pack.c | 2 ++\n builtin/receive-pack.c | 6 ++++--\n builtin/send-pack.c | 7 +++++--\n upload-pack.c | 7 +++++--\n version.c | 21 +++++++++++++++++++++\n version.h | 1 +\n 6 files changed, 38 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex 149db88..fe56596 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -10,6 +10,7 @@\n #include \"remote.h\"\n #include \"run-command.h\"\n #include \"transport.h\"\n+#include \"version.h\"\n \n static int transfer_unpack_limit = -1;\n static int fetch_unpack_limit = -1;\n@@ -327,6 +328,7 @@ static int find_common(int fd[2], unsigned char *result_sha1,\n \t\t\tif (args.no_progress) strbuf_addstr(&c, \" no-progress\");\n \t\t\tif (args.include_tag) strbuf_addstr(&c, \" include-tag\");\n \t\t\tif (prefer_ofs_delta) strbuf_addstr(&c, \" ofs-delta\");\n+\t\t\tstrbuf_addf(&c, \" agent=%s\", git_user_agent_sanitized());\n \t\t\tpacket_buf_write(&req_buf, \"want %s%s\\n\", remote_hex, c.buf);\n \t\t\tstrbuf_release(&c);\n \t\t} else\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 0afb8b2..fbfa128 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -12,6 +12,7 @@\n #include \"string-list.h\"\n #include \"sha1-array.h\"\n #include \"connected.h\"\n+#include \"version.h\"\n \n static const char receive_pack_usage[] = \"git receive-pack \";\n \n@@ -121,10 +122,11 @@ static void show_ref(const char *path, const unsigned char *sha1)\n \tif (sent_capabilities)\n \t\tpacket_write(1, \"%s %s\\n\", sha1_to_hex(sha1), path);\n \telse\n-\t\tpacket_write(1, \"%s %s%c%s%s\\n\",\n+\t\tpacket_write(1, \"%s %s%c%s%s agent=%s\\n\",\n \t\t\t sha1_to_hex(sha1), path, 0,\n \t\t\t \" report-status delete-refs side-band-64k quiet\",\n-\t\t\t prefer_ofs_delta ? \" ofs-delta\" : \"\");\n+\t\t\t prefer_ofs_delta ? \" ofs-delta\" : \"\",\n+\t\t\t git_user_agent_sanitized());\n \tsent_capabilities = 1;\n }\n \ndiff --git a/builtin/send-pack.c b/builtin/send-pack.c\nindex d5d7105..c4d4211 100644\n--- a/builtin/send-pack.c\n+++ b/builtin/send-pack.c\n@@ -8,6 +8,7 @@\n #include \"send-pack.h\"\n #include \"quote.h\"\n #include \"transport.h\"\n+#include \"version.h\"\n \n static const char send_pack_usage[] =\n \"git send-pack [--all | --mirror] [--dry-run] [--force] [--receive-pack=] [--verbose] [--thin] [:] [...]\\n\"\n@@ -306,11 +307,13 @@ int send_pack(struct send_pack_args *args,\n \t\t\tint quiet = quiet_supported && (args->quiet || !args->progress);\n \n \t\t\tif (!cmds_sent && (status_report || use_sideband || args->quiet)) {\n-\t\t\t\tpacket_buf_write(&req_buf, \"%s %s %s%c%s%s%s\",\n+\t\t\t\tpacket_buf_write(&req_buf,\n+\t\t\t\t\t\t \"%s %s %s%c%s%s%s agent=%s\",\n \t\t\t\t\t\t old_hex, new_hex, ref->name, 0,\n \t\t\t\t\t\t status_report ? \" report-status\" : \"\",\n \t\t\t\t\t\t use_sideband ? \" side-band-64k\" : \"\",\n-\t\t\t\t\t\t quiet ? \" quiet\" : \"\");\n+\t\t\t\t\t\t quiet ? \" quiet\" : \"\",\n+\t\t\t\t\t\t git_user_agent_sanitized());\n \t\t\t}\n \t\t\telse\n \t\t\t\tpacket_buf_write(&req_buf, \"%s %s %s\",\ndiff --git a/upload-pack.c b/upload-pack.c\nindex bb08e2e..2e90ccb 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -11,6 +11,7 @@\n #include \"list-objects.h\"\n #include \"run-command.h\"\n #include \"sigchain.h\"\n+#include \"version.h\"\n \n static const char upload_pack_usage[] = \"git upload-pack [--strict] [--timeout=] \";\n \n@@ -734,9 +735,11 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo\n \t}\n \n \tif (capabilities)\n-\t\tpacket_write(1, \"%s %s%c%s%s\\n\", sha1_to_hex(sha1), refname_nons,\n+\t\tpacket_write(1, \"%s %s%c%s%s agent=%s\\n\",\n+\t\t\t sha1_to_hex(sha1), refname_nons,\n \t\t\t 0, capabilities,\n-\t\t\t stateless_rpc ? \" no-done\" : \"\");\n+\t\t\t stateless_rpc ? \" no-done\" : \"\",\n+\t\t\t git_user_agent_sanitized());\n \telse\n \t\tpacket_write(1, \"%s %s\\n\", sha1_to_hex(sha1), refname_nons);\n \tcapabilities = NULL;\ndiff --git a/version.c b/version.c\nindex f98d5a6..6106a80 100644\n--- a/version.c\n+++ b/version.c\n@@ -1,5 +1,6 @@\n #include \"git-compat-util.h\"\n #include \"version.h\"\n+#include \"strbuf.h\"\n \n const char git_version_string[] = GIT_VERSION;\n \n@@ -15,3 +16,23 @@ const char *git_user_agent(void)\n \n \treturn agent;\n }\n+\n+const char *git_user_agent_sanitized(void)\n+{\n+\tstatic const char *agent = NULL;\n+\n+\tif (!agent) {\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tint i;\n+\n+\t\tstrbuf_addstr(&buf, git_user_agent());\n+\t\tstrbuf_trim(&buf);\n+\t\tfor (i = 0; i < buf.len; i++) {\n+\t\t\tif (buf.buf[i] <= 32 || buf.buf[i] >= 127)\n+\t\t\t\tbuf.buf[i] = '.';\n+\t\t}\n+\t\tagent = buf.buf;\n+\t}\n+\n+\treturn agent;\n+}\ndiff --git a/version.h b/version.h\nindex fd9cdd6..6911a4f 100644\n--- a/version.h\n+++ b/version.h\n@@ -4,5 +4,6 @@\n extern const char git_version_string[];\n \n const char *git_user_agent(void);\n+const char *git_user_agent_sanitized(void);\n \n #endif /* VERSION_H */\n-- \n1.7.7.7.32.g4b73117\n"},{"id":"193862","messageId":"87pq8vrvqz.fsf@thomas.inf.ethz.ch","threadId":"30670","inReplyTo":"20120602190112.GB14369@sigill.intra.peff.net","subject":"Re: [PATCH 2/4] version: add git_user_agent function","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2012-06-19T18:40:04Z","receivedAt":"2012-06-19T18:40:04Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"I'm terribly late to the party, seeing as this is already in next, but:\n\nJeff King writes:\n\n> This is basically a fancy way of saying \"git/$GIT_VERSION\",\n> except that it is overridable at build-time and through the\n> environment. Which means that people who don't want to\n> advertise their git version (for privacy or security\n> reasons) can tweak it.\n...\n> +GIT_USER_AGENT = git/$(GIT_VERSION)\n...\n> +GIT_USER_AGENT_SQ = $(subst ','\\'',$(GIT_USER_AGENT))\n> +GIT_USER_AGENT_CQ = \"$(subst \",\\\",$(subst \\,\\\\,$(GIT_USER_AGENT)))\"\n> +GIT_USER_AGENT_CQ_SQ = $(subst ','\\'',$(GIT_USER_AGENT_CQ))\n> +BASIC_CFLAGS += -DGIT_USER_AGENT='$(GIT_USER_AGENT_CQ_SQ)'\n\nUnless the user manually sets GIT_USER_AGENT, This forces a full rebuild\ndue to changed CFLAGS whenever the version changes. Can you make it so\nthat only version.o needs to be rebuilt, as with the normal git version\nstring?\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"193863","messageId":"20120619185916.GA16429@sigill.intra.peff.net","threadId":"30670","inReplyTo":"87pq8vrvqz.fsf@thomas.inf.ethz.ch","subject":"Re: [PATCH 2/4] version: add git_user_agent function","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-19T18:59:16Z","receivedAt":"2012-06-19T18:59:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 19, 2012 at 08:40:04PM +0200, Thomas Rast wrote:\n\n> I'm terribly late to the party, seeing as this is already in next, but:\n\nIt's never too late.\n\n> > +GIT_USER_AGENT = git/$(GIT_VERSION)\n> ...\n> > +GIT_USER_AGENT_SQ = $(subst ','\\'',$(GIT_USER_AGENT))\n> > +GIT_USER_AGENT_CQ = \"$(subst \",\\\",$(subst \\,\\\\,$(GIT_USER_AGENT)))\"\n> > +GIT_USER_AGENT_CQ_SQ = $(subst ','\\'',$(GIT_USER_AGENT_CQ))\n> > +BASIC_CFLAGS += -DGIT_USER_AGENT='$(GIT_USER_AGENT_CQ_SQ)'\n> \n> Unless the user manually sets GIT_USER_AGENT, This forces a full rebuild\n> due to changed CFLAGS whenever the version changes. Can you make it so\n> that only version.o needs to be rebuilt, as with the normal git version\n> string?\n\nIck, yeah. I had though this was already the case, but it turns out that\nit was a peculiarity of my personal config.mak (I set a custom $prefix\nbased on the branch, and it has a similar problem). I'll see if I can\nfix both.\n\n-Peff\n"},{"id":"193868","messageId":"20120619195229.GA14692@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120619185916.GA16429@sigill.intra.peff.net","subject":"Re: [PATCH 2/4] version: add git_user_agent function","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-19T19:52:29Z","receivedAt":"2012-06-19T19:52:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 19, 2012 at 02:59:16PM -0400, Jeff King wrote:\n\n> > Unless the user manually sets GIT_USER_AGENT, This forces a full rebuild\n> > due to changed CFLAGS whenever the version changes. Can you make it so\n> > that only version.o needs to be rebuilt, as with the normal git version\n> > string?\n> \n> Ick, yeah. I had though this was already the case, but it turns out that\n> it was a peculiarity of my personal config.mak (I set a custom $prefix\n> based on the branch, and it has a similar problem). I'll see if I can\n> fix both.\n\nHere it is.\n\n [1/3]: Makefile: apply dependencies consistently to sparse/asm targets\n [2/3]: Makefile: split GIT_USER_AGENT from GIT-CFLAGS\n [3/3]: Makefile: split prefix flags from GIT-CFLAGS\n\nPatches go on top of jk/version-string.\n\n-Peff\n"},{"id":"193869","messageId":"20120619195255.GA14714@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120619195229.GA14692@sigill.intra.peff.net","subject":"[PATCH 1/3] Makefile: apply dependencies consistently to sparse/asm targets","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-19T19:52:55Z","receivedAt":"2012-06-19T19:52:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When a C file includes a header file or depends on a\ncommand-line \"-D\" macro, we note it in the Makefile like:\n\n git.o: common-cmds.h\n\nHowever, other targets built from the C file should also\nknow about this dependency (in particular, .sp and .s files\nthat are not part of the usual build process). We sometimes\nnoted these and sometimes not; let's make sure they are\nalways included.\n\nSigned-off-by: Jeff King \n---\n Makefile | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 62de0b4..537d2ea 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1972,7 +1972,7 @@ shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell\n strip: $(PROGRAMS) git$X\n \t$(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X\n \n-git.o: common-cmds.h\n+git.sp git.s git.o: common-cmds.h\n git.sp git.s git.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_HTML_PATH=\"$(htmldir_SQ)\"' \\\n \t'-DGIT_MAN_PATH=\"$(mandir_SQ)\"' \\\n@@ -1982,9 +1982,9 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ git.o \\\n \t\t$(BUILTIN_OBJS) $(ALL_LDFLAGS) $(LIBS)\n \n-help.sp help.o: common-cmds.h\n+help.sp help.s help.o: common-cmds.h\n \n-builtin/help.sp builtin/help.o: common-cmds.h\n+builtin/help.sp builtin/help.s builtin/help.o: common-cmds.h\n builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_HTML_PATH=\"$(htmldir_SQ)\"' \\\n \t'-DGIT_MAN_PATH=\"$(mandir_SQ)\"' \\\n-- \n1.7.11.rc3.5.g201460b\n"},{"id":"193870","messageId":"20120619200147.GB14714@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120619195229.GA14692@sigill.intra.peff.net","subject":"[PATCH 2/3] Makefile: split GIT_USER_AGENT from GIT-CFLAGS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-19T20:01:47Z","receivedAt":"2012-06-19T20:01:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The default user-agent depends on the GIT_VERSION, which\nmeans that anytime you switch versions, it causes a full\nrebuild. Instead, let's split it out into its own file and\nrestrict the dependency to version.o.\n\nSigned-off-by: Jeff King \n---\nThis can't just depend on GIT-VERSION-FILE, since the user may have set\nGIT_USER_AGENT independently, and we would want to trigger a rebuild in\nthat case.\n\nI'm not happy about adding an extra built file and 4 lines of ugly shell\nscript per variable, but I don't see a good way around it. It would be\nslightly nicer if we actually just built auto/user-agent.h with the\nvalue, #included it from version.c, and then let the automatic\ndependency checker pick it up. But we'd still have to do the \"only\nupdate it if the value is actually changed\" dance.\n\nI've worked on systems in the past which use a file per variable, which\nlets make handle the dependencies normally. Something like:\n\n $ echo my-value >config/foo\n $ cat Makefile\n auto/foo.o: config/foo\n ./build-cstr $<\n\nbut that is a radical departure from the current config procedure. It\nalso doesn't play all that well with version control (are config/* files\nversioned? If not, where do the defaults come from? If so, how do you\noverride without the vcs wanting to commit changes?).\n\nWe are pretty well tied to GNU make. So there might be some GNU-specific\nway of templating GIT-CFLAGS, GIT-LDFLAGS, GIT-USER-AGENT etc.\n\n .gitignore | 1 +\n Makefile | 10 ++++++++--\n 2 files changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/.gitignore b/.gitignore\nindex bf66648..7329cfe 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -2,6 +2,7 @@\n /GIT-CFLAGS\n /GIT-LDFLAGS\n /GIT-GUI-VARS\n+/GIT-USER-AGENT\n /GIT-VERSION-FILE\n /bin-wrappers/\n /git\ndiff --git a/Makefile b/Makefile\nindex 537d2ea..42ce2dd 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1924,7 +1924,11 @@ endif\n GIT_USER_AGENT_SQ = $(subst ','\\'',$(GIT_USER_AGENT))\n GIT_USER_AGENT_CQ = \"$(subst \",\\\",$(subst \\,\\\\,$(GIT_USER_AGENT)))\"\n GIT_USER_AGENT_CQ_SQ = $(subst ','\\'',$(GIT_USER_AGENT_CQ))\n-BASIC_CFLAGS += -DGIT_USER_AGENT='$(GIT_USER_AGENT_CQ_SQ)'\n+GIT-USER-AGENT: FORCE\n+\t@if test x'$(GIT_USER_AGENT_SQ)' != x\"`cat GIT-USER-AGENT 2>/dev/null`\"; then \\\n+\t\techo >&2 \" * new user-agent flag\"; \\\n+\t\techo '$(GIT_USER_AGENT_SQ)' >GIT-USER-AGENT; \\\n+\tfi\n \n ALL_CFLAGS += $(BASIC_CFLAGS)\n ALL_LDFLAGS += $(BASIC_LDFLAGS)\n@@ -1990,8 +1994,10 @@ builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_MAN_PATH=\"$(mandir_SQ)\"' \\\n \t'-DGIT_INFO_PATH=\"$(infodir_SQ)\"'\n \n+version.sp version.s version.o: GIT-USER-AGENT\n version.sp version.s version.o: EXTRA_CPPFLAGS = \\\n-\t'-DGIT_VERSION=\"$(GIT_VERSION)\"'\n+\t'-DGIT_VERSION=\"$(GIT_VERSION)\"' \\\n+\t'-DGIT_USER_AGENT=$(GIT_USER_AGENT_CQ_SQ)'\n \n $(BUILT_INS): git$X\n \t$(QUIET_BUILT_IN)$(RM) $@ && \\\n-- \n1.7.11.rc3.5.g201460b\n"},{"id":"193871","messageId":"20120619200354.GC14714@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120619195229.GA14692@sigill.intra.peff.net","subject":"[PATCH 3/3] Makefile: split prefix flags from GIT-CFLAGS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-19T20:03:54Z","receivedAt":"2012-06-19T20:03:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Most of the build targets do not care about the setting of\n$prefix (or its derivative variables), but will be rebuilt\nif the prefix changes. For most setups this doesn't matter\n(they set prefix once and never change it), but for a setup\nwhich puts each branch or version in its own prefix, this\nunnecessarily causes a full rebuild whenever the branc is\nchanged.\n\nSigned-off-by: Jeff King \n---\nWe could break this down even further and save a few compilations (e.g.,\nconfig.o does not care about bindir, only sysconfdir). But given that\nmost people just set prefix anyway, I don't think it's worth the extra\nlines of Makefile (and this patch will never generate a _wrong_ answer;\nit is too conservative in doing extra compiles).\n\n .gitignore | 1 +\n Makefile | 29 +++++++++++++++++++++--------\n 2 files changed, 22 insertions(+), 8 deletions(-)\n\ndiff --git a/.gitignore b/.gitignore\nindex 7329cfe..c60c5a3 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -2,6 +2,7 @@\n /GIT-CFLAGS\n /GIT-LDFLAGS\n /GIT-GUI-VARS\n+/GIT-PREFIX\n /GIT-USER-AGENT\n /GIT-VERSION-FILE\n /bin-wrappers/\ndiff --git a/Makefile b/Makefile\nindex 42ce2dd..9761ae8 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1976,7 +1976,7 @@ shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell\n strip: $(PROGRAMS) git$X\n \t$(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X\n \n-git.sp git.s git.o: common-cmds.h\n+git.sp git.s git.o: common-cmds.h GIT-PREFIX\n git.sp git.s git.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_HTML_PATH=\"$(htmldir_SQ)\"' \\\n \t'-DGIT_MAN_PATH=\"$(mandir_SQ)\"' \\\n@@ -1988,7 +1988,7 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n \n help.sp help.s help.o: common-cmds.h\n \n-builtin/help.sp builtin/help.s builtin/help.o: common-cmds.h\n+builtin/help.sp builtin/help.s builtin/help.o: common-cmds.h GIT-PREFIX\n builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_HTML_PATH=\"$(htmldir_SQ)\"' \\\n \t'-DGIT_MAN_PATH=\"$(mandir_SQ)\"' \\\n@@ -2036,7 +2036,7 @@ $(SCRIPT_LIB) : % : %.sh\n ifndef NO_PERL\n $(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak\n \n-perl/perl.mak: GIT-CFLAGS perl/Makefile perl/Makefile.PL\n+perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL\n \t$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' $(@F)\n \n $(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl\n@@ -2080,7 +2080,7 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)) git-instaweb: % : unimplemented.sh\n endif # NO_PERL\n \n ifndef NO_PYTHON\n-$(patsubst %.py,%,$(SCRIPT_PYTHON)): GIT-CFLAGS\n+$(patsubst %.py,%,$(SCRIPT_PYTHON)): GIT-CFLAGS GIT-PREFIX\n $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : %.py\n \t$(QUIET_GEN)$(RM) $@ $@+ && \\\n \tINSTLIBDIR=`MAKEFLAGS= $(MAKE) -C git_remote_helpers -s \\\n@@ -2263,20 +2263,25 @@ xdiff-interface.o $(XDIFF_OBJS): $(XDIFF_H)\n $(VCSSVN_OBJS) $(VCSSVN_TEST_OBJS): $(LIB_H) $(VCSSVN_H)\n endif\n \n+exec_cmd.sp exec_cmd.s exec_cmd.o: GIT-PREFIX\n exec_cmd.sp exec_cmd.s exec_cmd.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_EXEC_PATH=\"$(gitexecdir_SQ)\"' \\\n \t'-DBINDIR=\"$(bindir_relative_SQ)\"' \\\n \t'-DPREFIX=\"$(prefix_SQ)\"'\n \n+builtin/init-db.sp builtin/init-db.s builtin/init-db.o: GIT-PREFIX\n builtin/init-db.sp builtin/init-db.s builtin/init-db.o: EXTRA_CPPFLAGS = \\\n \t-DDEFAULT_GIT_TEMPLATE_DIR='\"$(template_dir_SQ)\"'\n \n+config.sp config.s config.o: GIT-PREFIX\n config.sp config.s config.o: EXTRA_CPPFLAGS = \\\n \t-DETC_GITCONFIG='\"$(ETC_GITCONFIG_SQ)\"'\n \n+attr.sp attr.s attr.o: GIT-PREFIX\n attr.sp attr.s attr.o: EXTRA_CPPFLAGS = \\\n \t-DETC_GITATTRIBUTES='\"$(ETC_GITATTRIBUTES_SQ)\"'\n \n+gettext.sp gettext.s gettext.o: GIT-PREFIX\n gettext.sp gettext.s gettext.o: EXTRA_CPPFLAGS = \\\n \t-DGIT_LOCALE_PATH='\"$(localedir_SQ)\"'\n \n@@ -2400,14 +2405,22 @@ cscope:\n \t$(FIND_SOURCE_FILES) | xargs cscope -b\n \n ### Detect prefix changes\n-TRACK_CFLAGS = $(CC):$(subst ','\\'',$(ALL_CFLAGS)):\\\n- $(bindir_SQ):$(gitexecdir_SQ):$(template_dir_SQ):$(prefix_SQ):\\\n- $(localedir_SQ):$(USE_GETTEXT_SCHEME)\n+TRACK_PREFIX = $(bindir_SQ):$(gitexecdir_SQ):$(template_dir_SQ):$(prefix_SQ):\\\n+\t\t$(localedir_SQ)\n+\n+GIT-PREFIX: FORCE\n+\t@FLAGS='$(TRACK_PREFIX)'; \\\n+\tif test x\"$$FLAGS\" != x\"`cat GIT-PREFIX 2>/dev/null`\" ; then \\\n+\t\techo 1>&2 \" * new prefix flags\"; \\\n+\t\techo \"$$FLAGS\" >GIT-PREFIX; \\\n+\tfi\n+\n+TRACK_CFLAGS = $(CC):$(subst ','\\'',$(ALL_CFLAGS)):$(USE_GETTEXT_SCHEME)\n \n GIT-CFLAGS: FORCE\n \t@FLAGS='$(TRACK_CFLAGS)'; \\\n \t if test x\"$$FLAGS\" != x\"`cat GIT-CFLAGS 2>/dev/null`\" ; then \\\n-\t\techo 1>&2 \" * new build flags or prefix\"; \\\n+\t\techo 1>&2 \" * new build flags\"; \\\n \t\techo \"$$FLAGS\" >GIT-CFLAGS; \\\n fi\n \n-- \n1.7.11.rc3.5.g201460b\n"},{"id":"193876","messageId":"7vvcinyr3o.fsf@alter.siamese.dyndns.org","threadId":"30670","inReplyTo":"20120619195255.GA14714@sigill.intra.peff.net","subject":"Re: [PATCH 1/3] Makefile: apply dependencies consistently to sparse/asm targets","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-19T20:38:35Z","receivedAt":"2012-06-19T20:38:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Makes sense. Thanks.\n"},{"id":"193877","messageId":"7vr4tbyr33.fsf@alter.siamese.dyndns.org","threadId":"30670","inReplyTo":"20120619200147.GB14714@sigill.intra.peff.net","subject":"Re: [PATCH 2/3] Makefile: split GIT_USER_AGENT from GIT-CFLAGS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-19T20:38:56Z","receivedAt":"2012-06-19T20:38:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> The default user-agent depends on the GIT_VERSION, which\n> means that anytime you switch versions, it causes a full\n> rebuild. Instead, let's split it out into its own file and\n> restrict the dependency to version.o.\n>\n> Signed-off-by: Jeff King \n> ---\n> This can't just depend on GIT-VERSION-FILE, since the user may have set\n> GIT_USER_AGENT independently, and we would want to trigger a rebuild in\n> that case.\n\nOK. Will queue.\n"},{"id":"193879","messageId":"7vmx3zyqil.fsf@alter.siamese.dyndns.org","threadId":"30670","inReplyTo":"20120619200354.GC14714@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] Makefile: split prefix flags from GIT-CFLAGS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-19T20:51:14Z","receivedAt":"2012-06-19T20:51:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Most of the build targets do not care about the setting of $prefix\n> (or its derivative variables), but will be rebuilt if the prefix\n> changes. For most setups this doesn't matter (they set prefix once\n> and never change it), but for a setup which puts each branch or\n> version in its own prefix, this unnecessarily causes a full\n> rebuild whenever the branc is changed.\n\ns/branc /branch /.\n\nI have to wonder if is this something we care about that much.\n\nThe damage is not too bad from the point of view of linecount, but\nthis embeds the implicit knowledge of dependencies from $prefix to\nvarious path variables to selected object files that embed these\npaths variables by scattering dependencies on GIT-PREFIX in the\nMakefile, which does not seem to scale very well. I wonder if it\nmakes sense to have a single default-paths.o file that holds these\nstrings and recompile only that file when any of the paths change,\nto localize the damage.\n\nOf course, the current users of GIT_HTML_PATH that expect they can\ndo sizeof(GIT_HTML_PATH)-1 in place of strlen(GIT_HTML_PATH) may\nneed to be adjusted if we go that route.\n\nWill queue, but we might want to rethink this a bit more.\n\nThanks.\n"},{"id":"193880","messageId":"20120619210426.GA21838@sigill.intra.peff.net","threadId":"30670","inReplyTo":"7vmx3zyqil.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/3] Makefile: split prefix flags from GIT-CFLAGS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-19T21:04:26Z","receivedAt":"2012-06-19T21:04:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 19, 2012 at 01:51:14PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > Most of the build targets do not care about the setting of $prefix\n> > (or its derivative variables), but will be rebuilt if the prefix\n> > changes. For most setups this doesn't matter (they set prefix once\n> > and never change it), but for a setup which puts each branch or\n> > version in its own prefix, this unnecessarily causes a full\n> > rebuild whenever the branc is changed.\n> [...]\n> \n> I have to wonder if is this something we care about that much.\n\nIt does speed up my build a fair bit, but I admit I have a somewhat\nuncommon setup.\n\n> The damage is not too bad from the point of view of linecount, but\n> this embeds the implicit knowledge of dependencies from $prefix to\n> various path variables to selected object files that embed these\n> paths variables by scattering dependencies on GIT-PREFIX in the\n> Makefile, which does not seem to scale very well. I wonder if it\n> makes sense to have a single default-paths.o file that holds these\n> strings and recompile only that file when any of the paths change,\n> to localize the damage.\n> \n> Of course, the current users of GIT_HTML_PATH that expect they can\n> do sizeof(GIT_HTML_PATH)-1 in place of strlen(GIT_HTML_PATH) may\n> need to be adjusted if we go that route.\n\nYeah, I think that would be nicer overall. If we move to a link-time\ndependency, then we can even put _all_ of the Makefile-based strings in\nthere without ever having to care about who uses it. Of course, it won't\nwork for things that truly need to be preprocessor macros (for\nconditional compilation), so we'd still be stuck with those (most of\nthem just end up in CFLAGS and trigger a full rebuild, which is probably\nOK).\n\n-Peff\n"},{"id":"193885","messageId":"7vzk7zx9pz.fsf@alter.siamese.dyndns.org","threadId":"30670","inReplyTo":"20120619210426.GA21838@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] Makefile: split prefix flags from GIT-CFLAGS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-19T21:39:20Z","receivedAt":"2012-06-19T21:39:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Tue, Jun 19, 2012 at 01:51:14PM -0700, Junio C Hamano wrote:\n>\n>> Jeff King writes:\n>> \n>> > Most of the build targets do not care about the setting of $prefix\n>> > (or its derivative variables), but will be rebuilt if the prefix\n>> > changes. For most setups this doesn't matter (they set prefix once\n>> > and never change it), but for a setup which puts each branch or\n>> > version in its own prefix, this unnecessarily causes a full\n>> > rebuild whenever the branc is changed.\n>> [...]\n>> \n>> I have to wonder if is this something we care about that much.\n>\n> It does speed up my build a fair bit, but I admit I have a somewhat\n> uncommon setup.\n\nOh, I wouldn't question the \"speeding up\" part. I simply expected\nthat people who use separate prefix depending on branch would have\nseparate checkout for these branches they build and install\nregularly to their own prefixes, so recompilation due to prefix\nchange would not be an issue, because by definition in such a set-up\nyou won't change branch in a checkout.\n\n> Yeah, I think that would be nicer overall. If we move to a link-time\n> dependency, then we can even put _all_ of the Makefile-based strings in\n> there without ever having to care about who uses it. Of course, it won't\n> work for things that truly need to be preprocessor macros (for\n> conditional compilation), so we'd still be stuck with those (most of\n> them just end up in CFLAGS and trigger a full rebuild, which is probably\n> OK).\n\nYeah, I didn't go through the list of Make variables to see how much\ndamage I was talking about ;-).\n"},{"id":"193889","messageId":"20120619214308.GA22166@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120619210426.GA21838@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] Makefile: split prefix flags from GIT-CFLAGS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-19T21:43:08Z","receivedAt":"2012-06-19T21:43:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 19, 2012 at 05:04:26PM -0400, Jeff King wrote:\n\n> > The damage is not too bad from the point of view of linecount, but\n> > this embeds the implicit knowledge of dependencies from $prefix to\n> > various path variables to selected object files that embed these\n> > paths variables by scattering dependencies on GIT-PREFIX in the\n> > Makefile, which does not seem to scale very well. I wonder if it\n> > makes sense to have a single default-paths.o file that holds these\n> > strings and recompile only that file when any of the paths change,\n> > to localize the damage.\n> > \n> > Of course, the current users of GIT_HTML_PATH that expect they can\n> > do sizeof(GIT_HTML_PATH)-1 in place of strlen(GIT_HTML_PATH) may\n> > need to be adjusted if we go that route.\n> \n> Yeah, I think that would be nicer overall. If we move to a link-time\n> dependency, then we can even put _all_ of the Makefile-based strings in\n> there without ever having to care about who uses it. Of course, it won't\n> work for things that truly need to be preprocessor macros (for\n> conditional compilation), so we'd still be stuck with those (most of\n> them just end up in CFLAGS and trigger a full rebuild, which is probably\n> OK).\n\nI started on this, but it is a little bit trickier than that. We can\ncover C compilation with one strategy, but make variables end up going\nlots of other places, too, like shell scripts, the perl Makefile\ngeneration, etc. Those places all need to individually respect a\nseparate sentinel-file dependency. So I don't think there is an easy way\nout of the sprinkling of dependencies.\n\nAt the very least, I tried to keep the dependency close to the\npoint-of-use, like:\n\n foo.o: GIT-PREFIX\n foo.o: EXTRA_CPPFLAGS = \\\n '-DPREFIX=$(prefix_SQ)'\n\neven if the actual build rules for foo.o are found elsewhere (and\ntypically they are, as they are part of a big pattern-based rule).\n\nSome of the existing locations did not do a great job of that, and\ninstead it like this:\n\n version.o git.spec \\\n $(patsubst %.sh,%,$(SCRIPT_SH)) \\\n $(patsubst %.perl,%,$(SCRIPT_PERL)) \\\n : GIT-VERSION-FILE\n\n [... 500 lines later ...]\n\n git.spec: git.spec.in\n sed -e 's/@@VERSION@@/$(GIT_VERSION)/g' <$< >$@+\n\nIf you read the latter hunk and wanted to emulate it, there is nothing\nto indicate that you must also modify the earlier hunk. I think putting\nthem together makes more sense (even though it technically takes more\nlines).\n\n-Peff\n"},{"id":"193900","messageId":"20120619232231.GA6328@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120619214308.GA22166@sigill.intra.peff.net","subject":"[PATCHv2 0/8] makefile cleanups","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-19T23:22:31Z","receivedAt":"2012-06-19T23:22:31Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 19, 2012 at 05:43:08PM -0400, Jeff King wrote:\n\n> Some of the existing locations did not do a great job of that, and\n> instead it like this:\n> \n> version.o git.spec \\\n> $(patsubst %.sh,%,$(SCRIPT_SH)) \\\n> $(patsubst %.perl,%,$(SCRIPT_PERL)) \\\n> : GIT-VERSION-FILE\n> \n> [... 500 lines later ...]\n> \n> git.spec: git.spec.in\n> sed -e 's/@@VERSION@@/$(GIT_VERSION)/g' <$< >$@+\n> \n> If you read the latter hunk and wanted to emulate it, there is nothing\n> to indicate that you must also modify the earlier hunk. I think putting\n> them together makes more sense (even though it technically takes more\n> lines).\n\nHmm. The deeper I dig into the Makefile, the more problems I find. I\nthink I'm ready to come out of the rabbit hole for today, and I ended up\nwith the series below. Some of it is cleanup and some of it fixes real\nproblems.\n\nMy philosophy in doing the series was:\n\n 1. Dependencies should be inclusive. That is, re-running \"make\" in an\n already-built tree with different parameters should produce the\n same result as running in a clean tree.\n\n 2. Dependencies do not have to be precise. That is, it's OK to re-run\n a build step even if it will produce the same output. Of course,\n it's nice to be more precise because it saves time, but for the\n sake of simplicity, we compartmentalize groups of changes (e.g.,\n GIT-CFLAGS depends on SHELL_PATH even though only run-command.c\n needs it; that's OK because SHELL_PATH doesn't get changed often).\n\n 3. Dependencies on variables that change regularly _should_ be\n precise. GIT_VERSION (and anything that depends on it) is a good\n example, because you don't want to rebuild every time you check out\n a new version.\n\n [1/8]: Makefile: apply dependencies consistently to sparse/asm targets\n [2/8]: Makefile: do not replace @@GIT_USER_AGENT@@ in scripts\n [3/8]: Makefile: split GIT_USER_AGENT from GIT-CFLAGS\n [4/8]: Makefile: split prefix flags from GIT-CFLAGS\n [5/8]: Makefile: do not replace @@GIT_VERSION@@ in shell scripts\n [6/8]: Makefile: update scripts when build-time parameters change\n [7/8]: Makefile: build instaweb similar to other scripts\n [8/8]: Makefile: move GIT-VERSION-FILE dependencies closer to use\n\nThis replaces the series I sent earlier today, and goes on top of\njk/version-string. A lot of it could be a separate topic, but it's\ntextually pretty tangled. If jk/version-string is soon to graduate, then\nI'd just as soon leave it as-is rather than go to the work of\ndisentangling it. Even though there are fixes here, none of them is so\ncritical that they can't be held hostage for a few days or a week.\n\n-Peff\n"},{"id":"193901","messageId":"20120619232310.GA6496@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120619232231.GA6328@sigill.intra.peff.net","subject":"[PATCHv2 1/8] Makefile: apply dependencies consistently to sparse/asm targets","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-19T23:23:11Z","receivedAt":"2012-06-19T23:23:11Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When a C file includes a header file or depends on a\ncommand-line \"-D\" macro, we note it in the Makefile like:\n\n git.o: common-cmds.h\n\nHowever, other targets built from the C file should also\nknow about this dependency (in particular, .sp and .s files\nthat are not part of the usual build process). We sometimes\nnoted these and sometimes not; let's make sure they are\nalways included.\n\nSigned-off-by: Jeff King \n---\nSame as v1.\n\n Makefile | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 62de0b4..537d2ea 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1972,7 +1972,7 @@ shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell\n strip: $(PROGRAMS) git$X\n \t$(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X\n \n-git.o: common-cmds.h\n+git.sp git.s git.o: common-cmds.h\n git.sp git.s git.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_HTML_PATH=\"$(htmldir_SQ)\"' \\\n \t'-DGIT_MAN_PATH=\"$(mandir_SQ)\"' \\\n@@ -1982,9 +1982,9 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ git.o \\\n \t\t$(BUILTIN_OBJS) $(ALL_LDFLAGS) $(LIBS)\n \n-help.sp help.o: common-cmds.h\n+help.sp help.s help.o: common-cmds.h\n \n-builtin/help.sp builtin/help.o: common-cmds.h\n+builtin/help.sp builtin/help.s builtin/help.o: common-cmds.h\n builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_HTML_PATH=\"$(htmldir_SQ)\"' \\\n \t'-DGIT_MAN_PATH=\"$(mandir_SQ)\"' \\\n-- \n1.7.11.rc3.5.g201460b\n"},{"id":"193902","messageId":"20120619232453.GB6496@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120619232231.GA6328@sigill.intra.peff.net","subject":"[PATCHv2 2/8] Makefile: do not replace @@GIT_USER_AGENT@@ in scripts","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-19T23:24:53Z","receivedAt":"2012-06-19T23:24:53Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"No scripts actually care about this replacement. This was\nerroneously added by d937411.\n\nSigned-off-by: Jeff King \n---\nI think I started that series by just replacing GIT_USER_AGENT\neverywhere that GIT_VERSION was before realizing that I didn't need to\ndo that at all, and this was leftover cruft.\n\nTechnically I could re-roll jk/version-string with this squashed in,\nsince you are probably about to do the post-release rewind of next, but\nI don't know that it's worth the trouble unless we are re-rolling the\nwhole thing (to put the Makefile fixups first, and then build\nversion-string stuff on top).\n\n Makefile | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex 537d2ea..23c289d 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2010,7 +2010,6 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \\\n -e 's|@@DIFF@@|$(DIFF_SQ)|' \\\n -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \\\n- -e 's|@@GIT_USER_AGENT@@|$(GIT_USER_AGENT_SQ)|g' \\\n -e 's|@@LOCALEDIR@@|$(localedir_SQ)|g' \\\n -e 's/@@NO_CURL@@/$(NO_CURL)/g' \\\n -e 's/@@USE_GETTEXT_SCHEME@@/$(USE_GETTEXT_SCHEME)/g' \\\n-- \n1.7.11.rc3.5.g201460b\n"},{"id":"193903","messageId":"20120619232529.GC6496@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120619232231.GA6328@sigill.intra.peff.net","subject":"[PATCHv2 3/8] Makefile: split GIT_USER_AGENT from GIT-CFLAGS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-19T23:25:29Z","receivedAt":"2012-06-19T23:25:29Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The default user-agent depends on the GIT_VERSION, which\nmeans that anytime you switch versions, it causes a full\nrebuild. Instead, let's split it out into its own file and\nrestrict the dependency to version.o.\n\nSigned-off-by: Jeff King \n---\nv1 forgot to add a line to \"make clean\" to remove the meta-file.\n\n .gitignore | 1 +\n Makefile | 11 +++++++++--\n 2 files changed, 10 insertions(+), 2 deletions(-)\n\ndiff --git a/.gitignore b/.gitignore\nindex bf66648..7329cfe 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -2,6 +2,7 @@\n /GIT-CFLAGS\n /GIT-LDFLAGS\n /GIT-GUI-VARS\n+/GIT-USER-AGENT\n /GIT-VERSION-FILE\n /bin-wrappers/\n /git\ndiff --git a/Makefile b/Makefile\nindex 23c289d..8ff61c5 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1924,7 +1924,11 @@ endif\n GIT_USER_AGENT_SQ = $(subst ','\\'',$(GIT_USER_AGENT))\n GIT_USER_AGENT_CQ = \"$(subst \",\\\",$(subst \\,\\\\,$(GIT_USER_AGENT)))\"\n GIT_USER_AGENT_CQ_SQ = $(subst ','\\'',$(GIT_USER_AGENT_CQ))\n-BASIC_CFLAGS += -DGIT_USER_AGENT='$(GIT_USER_AGENT_CQ_SQ)'\n+GIT-USER-AGENT: FORCE\n+\t@if test x'$(GIT_USER_AGENT_SQ)' != x\"`cat GIT-USER-AGENT 2>/dev/null`\"; then \\\n+\t\techo >&2 \" * new user-agent flag\"; \\\n+\t\techo '$(GIT_USER_AGENT_SQ)' >GIT-USER-AGENT; \\\n+\tfi\n \n ALL_CFLAGS += $(BASIC_CFLAGS)\n ALL_LDFLAGS += $(BASIC_LDFLAGS)\n@@ -1990,8 +1994,10 @@ builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_MAN_PATH=\"$(mandir_SQ)\"' \\\n \t'-DGIT_INFO_PATH=\"$(infodir_SQ)\"'\n \n+version.sp version.s version.o: GIT-USER-AGENT\n version.sp version.s version.o: EXTRA_CPPFLAGS = \\\n-\t'-DGIT_VERSION=\"$(GIT_VERSION)\"'\n+\t'-DGIT_VERSION=\"$(GIT_VERSION)\"' \\\n+\t'-DGIT_USER_AGENT=$(GIT_USER_AGENT_CQ_SQ)'\n \n $(BUILT_INS): git$X\n \t$(QUIET_BUILT_IN)$(RM) $@ && \\\n@@ -2736,6 +2742,7 @@ ifndef NO_TCLTK\n \t$(MAKE) -C git-gui clean\n endif\n \t$(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-GUI-VARS GIT-BUILD-OPTIONS\n+\t$(RM) GIT-USER-AGENT\n \n .PHONY: all install profile-clean clean strip\n .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell\n-- \n1.7.11.rc3.5.g201460b\n"},{"id":"193904","messageId":"20120619232554.GD6496@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120619232231.GA6328@sigill.intra.peff.net","subject":"[PATCHv2 4/8] Makefile: split prefix flags from GIT-CFLAGS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-19T23:25:54Z","receivedAt":"2012-06-19T23:25:54Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Most of the build targets do not care about the setting of\n$prefix (or its derivative variables), but will be rebuilt\nif the prefix changes. For most setups this doesn't matter\n(they set prefix once and never change it), but for a setup\nwhich puts each branch or version in its own prefix, this\nunnecessarily causes a full rebuild whenever the branc is\nchanged.\n\nSigned-off-by: Jeff King \n---\nv1 forgot to remove the file during \"make clean\".\n\n .gitignore | 1 +\n Makefile | 31 ++++++++++++++++++++++---------\n 2 files changed, 23 insertions(+), 9 deletions(-)\n\ndiff --git a/.gitignore b/.gitignore\nindex 7329cfe..c60c5a3 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -2,6 +2,7 @@\n /GIT-CFLAGS\n /GIT-LDFLAGS\n /GIT-GUI-VARS\n+/GIT-PREFIX\n /GIT-USER-AGENT\n /GIT-VERSION-FILE\n /bin-wrappers/\ndiff --git a/Makefile b/Makefile\nindex 8ff61c5..c95a70c 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1976,7 +1976,7 @@ shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell\n strip: $(PROGRAMS) git$X\n \t$(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X\n \n-git.sp git.s git.o: common-cmds.h\n+git.sp git.s git.o: common-cmds.h GIT-PREFIX\n git.sp git.s git.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_HTML_PATH=\"$(htmldir_SQ)\"' \\\n \t'-DGIT_MAN_PATH=\"$(mandir_SQ)\"' \\\n@@ -1988,7 +1988,7 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n \n help.sp help.s help.o: common-cmds.h\n \n-builtin/help.sp builtin/help.s builtin/help.o: common-cmds.h\n+builtin/help.sp builtin/help.s builtin/help.o: common-cmds.h GIT-PREFIX\n builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_HTML_PATH=\"$(htmldir_SQ)\"' \\\n \t'-DGIT_MAN_PATH=\"$(mandir_SQ)\"' \\\n@@ -2035,7 +2035,7 @@ $(SCRIPT_LIB) : % : %.sh\n ifndef NO_PERL\n $(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak\n \n-perl/perl.mak: GIT-CFLAGS perl/Makefile perl/Makefile.PL\n+perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL\n \t$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' $(@F)\n \n $(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl\n@@ -2079,7 +2079,7 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)) git-instaweb: % : unimplemented.sh\n endif # NO_PERL\n \n ifndef NO_PYTHON\n-$(patsubst %.py,%,$(SCRIPT_PYTHON)): GIT-CFLAGS\n+$(patsubst %.py,%,$(SCRIPT_PYTHON)): GIT-CFLAGS GIT-PREFIX\n $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : %.py\n \t$(QUIET_GEN)$(RM) $@ $@+ && \\\n \tINSTLIBDIR=`MAKEFLAGS= $(MAKE) -C git_remote_helpers -s \\\n@@ -2262,20 +2262,25 @@ xdiff-interface.o $(XDIFF_OBJS): $(XDIFF_H)\n $(VCSSVN_OBJS) $(VCSSVN_TEST_OBJS): $(LIB_H) $(VCSSVN_H)\n endif\n \n+exec_cmd.sp exec_cmd.s exec_cmd.o: GIT-PREFIX\n exec_cmd.sp exec_cmd.s exec_cmd.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_EXEC_PATH=\"$(gitexecdir_SQ)\"' \\\n \t'-DBINDIR=\"$(bindir_relative_SQ)\"' \\\n \t'-DPREFIX=\"$(prefix_SQ)\"'\n \n+builtin/init-db.sp builtin/init-db.s builtin/init-db.o: GIT-PREFIX\n builtin/init-db.sp builtin/init-db.s builtin/init-db.o: EXTRA_CPPFLAGS = \\\n \t-DDEFAULT_GIT_TEMPLATE_DIR='\"$(template_dir_SQ)\"'\n \n+config.sp config.s config.o: GIT-PREFIX\n config.sp config.s config.o: EXTRA_CPPFLAGS = \\\n \t-DETC_GITCONFIG='\"$(ETC_GITCONFIG_SQ)\"'\n \n+attr.sp attr.s attr.o: GIT-PREFIX\n attr.sp attr.s attr.o: EXTRA_CPPFLAGS = \\\n \t-DETC_GITATTRIBUTES='\"$(ETC_GITATTRIBUTES_SQ)\"'\n \n+gettext.sp gettext.s gettext.o: GIT-PREFIX\n gettext.sp gettext.s gettext.o: EXTRA_CPPFLAGS = \\\n \t-DGIT_LOCALE_PATH='\"$(localedir_SQ)\"'\n \n@@ -2399,14 +2404,22 @@ cscope:\n \t$(FIND_SOURCE_FILES) | xargs cscope -b\n \n ### Detect prefix changes\n-TRACK_CFLAGS = $(CC):$(subst ','\\'',$(ALL_CFLAGS)):\\\n- $(bindir_SQ):$(gitexecdir_SQ):$(template_dir_SQ):$(prefix_SQ):\\\n- $(localedir_SQ):$(USE_GETTEXT_SCHEME)\n+TRACK_PREFIX = $(bindir_SQ):$(gitexecdir_SQ):$(template_dir_SQ):$(prefix_SQ):\\\n+\t\t$(localedir_SQ)\n+\n+GIT-PREFIX: FORCE\n+\t@FLAGS='$(TRACK_PREFIX)'; \\\n+\tif test x\"$$FLAGS\" != x\"`cat GIT-PREFIX 2>/dev/null`\" ; then \\\n+\t\techo 1>&2 \" * new prefix flags\"; \\\n+\t\techo \"$$FLAGS\" >GIT-PREFIX; \\\n+\tfi\n+\n+TRACK_CFLAGS = $(CC):$(subst ','\\'',$(ALL_CFLAGS)):$(USE_GETTEXT_SCHEME)\n \n GIT-CFLAGS: FORCE\n \t@FLAGS='$(TRACK_CFLAGS)'; \\\n \t if test x\"$$FLAGS\" != x\"`cat GIT-CFLAGS 2>/dev/null`\" ; then \\\n-\t\techo 1>&2 \" * new build flags or prefix\"; \\\n+\t\techo 1>&2 \" * new build flags\"; \\\n \t\techo \"$$FLAGS\" >GIT-CFLAGS; \\\n fi\n \n@@ -2742,7 +2755,7 @@ ifndef NO_TCLTK\n \t$(MAKE) -C git-gui clean\n endif\n \t$(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-GUI-VARS GIT-BUILD-OPTIONS\n-\t$(RM) GIT-USER-AGENT\n+\t$(RM) GIT-USER-AGENT GIT-PREFIX\n \n .PHONY: all install profile-clean clean strip\n .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell\n-- \n1.7.11.rc3.5.g201460b\n"},{"id":"193905","messageId":"20120619232728.GE6496@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120619232231.GA6328@sigill.intra.peff.net","subject":"[PATCHv2 5/8] Makefile: do not replace @@GIT_VERSION@@ in shell scripts","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-19T23:27:28Z","receivedAt":"2012-06-19T23:27:28Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"No shell script actually uses the replacement (it is used in\nsome perl scripts, but cmd_munge_script only handles shell\nscripts). We can also therefore drop the dependency on\nGIT-VERSION-FILE.\n\nSigned-off-by: Jeff King \n---\nProbably not a big deal, since building scripts is relatively fast, but\nit should make the output much shorter. :)\n\n Makefile | 3 ---\n 1 file changed, 3 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex c95a70c..c0bf3ad 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2015,7 +2015,6 @@ $(RM) $@ $@+ && \\\n sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \\\n -e 's|@@DIFF@@|$(DIFF_SQ)|' \\\n- -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \\\n -e 's|@@LOCALEDIR@@|$(localedir_SQ)|g' \\\n -e 's/@@NO_CURL@@/$(NO_CURL)/g' \\\n -e 's/@@USE_GETTEXT_SCHEME@@/$(USE_GETTEXT_SCHEME)/g' \\\n@@ -2061,7 +2060,6 @@ gitweb:\n git-instaweb: git-instaweb.sh gitweb\n \t$(QUIET_GEN)$(RM) $@ $@+ && \\\n \tsed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n-\t -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \\\n \t -e 's/@@NO_CURL@@/$(NO_CURL)/g' \\\n \t -e 's|@@GITWEBDIR@@|$(gitwebdir_SQ)|g' \\\n \t -e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \\\n@@ -2110,7 +2108,6 @@ configure: configure.ac\n \n # These can record GIT_VERSION\n version.o git.spec \\\n-\t$(patsubst %.sh,%,$(SCRIPT_SH)) \\\n \t$(patsubst %.perl,%,$(SCRIPT_PERL)) \\\n \t: GIT-VERSION-FILE\n \n-- \n1.7.11.rc3.5.g201460b\n"},{"id":"193906","messageId":"20120619232845.GF6496@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120619232231.GA6328@sigill.intra.peff.net","subject":"[PATCHv2 6/8] Makefile: update scripts when build-time parameters change","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-19T23:28:45Z","receivedAt":"2012-06-19T23:28:45Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Currently, running:\n\n make SHELL_PATH=/bin/bash &&\n make SHELL_PATH=/bin/sh\n\nwill not rebuild any shell scripts in the second command,\nleading to incorrect results when building from an unclean\nworking directory.\n\nThis patch introduces a new dependency meta-file to notice\nthe change.\n\nSigned-off-by: Jeff King \n---\nI suspect nobody complained because the parameters replace in shell\nfiles tend not to change. I think perl scripts have a similar problem,\nbut I decided not to tackle them today.\n\n .gitignore | 1 +\n Makefile | 16 +++++++++++++---\n 2 files changed, 14 insertions(+), 3 deletions(-)\n\ndiff --git a/.gitignore b/.gitignore\nindex c60c5a3..6535cd7 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -3,6 +3,7 @@\n /GIT-LDFLAGS\n /GIT-GUI-VARS\n /GIT-PREFIX\n+/GIT-SCRIPT-DEFINES\n /GIT-USER-AGENT\n /GIT-VERSION-FILE\n /bin-wrappers/\ndiff --git a/Makefile b/Makefile\nindex c0bf3ad..979ba31 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2010,6 +2010,8 @@ common-cmds.h: ./generate-cmdlist.sh command-list.txt\n common-cmds.h: $(wildcard Documentation/git-*.txt)\n \t$(QUIET_GEN)./generate-cmdlist.sh > $@+ && mv $@+ $@\n \n+SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\\\n+\t$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ)\n define cmd_munge_script\n $(RM) $@ $@+ && \\\n sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n@@ -2022,12 +2024,20 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n $@.sh >$@+\n endef\n \n-$(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh\n+GIT-SCRIPT-DEFINES: FORCE\n+\t@FLAGS='$(SCRIPT_DEFINES)'; \\\n+\t if test x\"$$FLAGS\" != x\"`cat $@ 2>/dev/null`\" ; then \\\n+\t\techo 1>&2 \" * new script parameters\"; \\\n+\t\techo \"$$FLAGS\" >$@; \\\n+ fi\n+\n+\n+$(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh GIT-SCRIPT-DEFINES\n \t$(QUIET_GEN)$(cmd_munge_script) && \\\n \tchmod +x $@+ && \\\n \tmv $@+ $@\n \n-$(SCRIPT_LIB) : % : %.sh\n+$(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES\n \t$(QUIET_GEN)$(cmd_munge_script) && \\\n \tmv $@+ $@\n \n@@ -2752,7 +2762,7 @@ ifndef NO_TCLTK\n \t$(MAKE) -C git-gui clean\n endif\n \t$(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-GUI-VARS GIT-BUILD-OPTIONS\n-\t$(RM) GIT-USER-AGENT GIT-PREFIX\n+\t$(RM) GIT-USER-AGENT GIT-PREFIX GIT-SCRIPT-DEFINES\n \n .PHONY: all install profile-clean clean strip\n .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell\n-- \n1.7.11.rc3.5.g201460b\n"},{"id":"193907","messageId":"20120619232909.GG6496@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120619232231.GA6328@sigill.intra.peff.net","subject":"[PATCHv2 7/8] Makefile: build instaweb similar to other scripts","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-19T23:29:09Z","receivedAt":"2012-06-19T23:29:09Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Instaweb would not properly rebuild if the build-time\nparameters changed. Fix this by depending on the\nGIT-SCRIPT-DEFINES meta-file and using $(cmd_munge_script)\nlike all the other shell scripts. This requires adding a few\nnew parametres to cmd_munge_script, but that doesn't hurt\nexisting scripts.\n\nSigned-off-by: Jeff King \n---\n Makefile | 14 ++++++--------\n 1 file changed, 6 insertions(+), 8 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 979ba31..ad183d9 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2011,7 +2011,8 @@ common-cmds.h: $(wildcard Documentation/git-*.txt)\n \t$(QUIET_GEN)./generate-cmdlist.sh > $@+ && mv $@+ $@\n \n SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\\\n-\t$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ)\n+\t$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\\\n+\t$(gitwebdir_SQ):$(PERL_PATH_SQ)\n define cmd_munge_script\n $(RM) $@ $@+ && \\\n sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n@@ -2021,6 +2022,8 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n -e 's/@@NO_CURL@@/$(NO_CURL)/g' \\\n -e 's/@@USE_GETTEXT_SCHEME@@/$(USE_GETTEXT_SCHEME)/g' \\\n -e $(BROKEN_PATH_FIX) \\\n+ -e 's|@@GITWEBDIR@@|$(gitwebdir_SQ)|g' \\\n+ -e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \\\n $@.sh >$@+\n endef\n \n@@ -2067,13 +2070,8 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl\n gitweb:\n \t$(QUIET_SUBDIR0)gitweb $(QUIET_SUBDIR1) all\n \n-git-instaweb: git-instaweb.sh gitweb\n-\t$(QUIET_GEN)$(RM) $@ $@+ && \\\n-\tsed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n-\t -e 's/@@NO_CURL@@/$(NO_CURL)/g' \\\n-\t -e 's|@@GITWEBDIR@@|$(gitwebdir_SQ)|g' \\\n-\t -e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \\\n-\t $@.sh > $@+ && \\\n+git-instaweb: git-instaweb.sh gitweb GIT-SCRIPT-DEFINES\n+\t$(QUIET_GEN)$(cmd_munge_script) && \\\n \tchmod +x $@+ && \\\n \tmv $@+ $@\n else # NO_PERL\n-- \n1.7.11.rc3.5.g201460b\n"},{"id":"193908","messageId":"20120619233032.GH6496@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120619232231.GA6328@sigill.intra.peff.net","subject":"[PATCHv2 8/8] Makefile: move GIT-VERSION-FILE dependencies closer to use","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-19T23:30:32Z","receivedAt":"2012-06-19T23:30:32Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"There is a list of all of the targets which depend on\nGIT-VERSION-FILE, but it can be quite far from the actual\npoint where the targets actually use $(GIT_VERSION). This\ncan make it hard to verify that each use of $(GIT_VERSION)\nhas a matching dependency.\n\nThis patch moves the dependency closer to the actual build\ninstructions, which makes verification easier. This also\nfixes the generation of \"configure\", which did not properly\nmark the dependency.\n\nSigned-off-by: Jeff King \n---\nThis is the patch I was trying to get to when I started. :)\n\nI do think this pattern will make patches easier to review; you should\ntypically see the dependency in the same hunk as the point-of-use.\n\n Makefile | 13 ++++---------\n 1 file changed, 4 insertions(+), 9 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex ad183d9..d21702c 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1994,7 +1994,7 @@ builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_MAN_PATH=\"$(mandir_SQ)\"' \\\n \t'-DGIT_INFO_PATH=\"$(infodir_SQ)\"'\n \n-version.sp version.s version.o: GIT-USER-AGENT\n+version.sp version.s version.o: GIT-VERSION-FILE GIT-USER-AGENT\n version.sp version.s version.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_VERSION=\"$(GIT_VERSION)\"' \\\n \t'-DGIT_USER_AGENT=$(GIT_USER_AGENT_CQ_SQ)'\n@@ -2050,7 +2050,7 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak\n perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL\n \t$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' $(@F)\n \n-$(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl\n+$(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl GIT-VERSION-FILE\n \t$(QUIET_GEN)$(RM) $@ $@+ && \\\n \tINSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir` && \\\n \tsed -e '1{' \\\n@@ -2107,18 +2107,13 @@ $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : unimplemented.sh\n \tmv $@+ $@\n endif # NO_PYTHON\n \n-configure: configure.ac\n+configure: configure.ac GIT-VERSION-FILE\n \t$(QUIET_GEN)$(RM) $@ $<+ && \\\n \tsed -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \\\n \t $< > $<+ && \\\n \tautoconf -o $@ $<+ && \\\n \t$(RM) $<+\n \n-# These can record GIT_VERSION\n-version.o git.spec \\\n-\t$(patsubst %.perl,%,$(SCRIPT_PERL)) \\\n-\t: GIT-VERSION-FILE\n-\n TEST_OBJS := $(patsubst test-%$X,test-%.o,$(TEST_PROGRAMS))\n GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \\\n \tgit.o\n@@ -2676,7 +2671,7 @@ quick-install-html:\n \n ### Maintainer's dist rules\n \n-git.spec: git.spec.in\n+git.spec: git.spec.in GIT-VERSION-FILE\n \tsed -e 's/@@VERSION@@/$(GIT_VERSION)/g' < $< > $@+\n \tmv $@+ $@\n \n-- \n1.7.11.rc3.5.g201460b\n"},{"id":"193909","messageId":"20120619233659.GA6621@sigill.intra.peff.net","threadId":"30670","inReplyTo":"7vzk7zx9pz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/3] Makefile: split prefix flags from GIT-CFLAGS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-19T23:36:59Z","receivedAt":"2012-06-19T23:36:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 19, 2012 at 02:39:20PM -0700, Junio C Hamano wrote:\n\n> >> I have to wonder if is this something we care about that much.\n> >\n> > It does speed up my build a fair bit, but I admit I have a somewhat\n> > uncommon setup.\n> \n> Oh, I wouldn't question the \"speeding up\" part. I simply expected\n> that people who use separate prefix depending on branch would have\n> separate checkout for these branches they build and install\n> regularly to their own prefixes, so recompilation due to prefix\n> change would not be an issue, because by definition in such a set-up\n> you won't change branch in a checkout.\n\nI dunno if other people do that. For me, separate checkouts would be\noverkill. I use a single working tree, and I don't mind switching\nbetween topic branches, but I want to keep installed versions from\npolluting each other (e.g., via cruft left in the exec-path or in\ntemplates), which has caused oddness with testing in the past.\n\nIt's also nice that I can do:\n\n $ git checkout v1.7.10.5\n $ make install\n\nand now I need only point my PATH to $HOME/local/git/v1.7.10.5/bin\nto test that version forever. It saves lots of compile time when I'm\ninvestigating bug reports.\n\nOf course, I am probably one of the few people in the world who actually\nwants to have 50 built versions of git on hand.\n\n-Peff\n"},{"id":"193913","messageId":"7vy5nix3ao.fsf@alter.siamese.dyndns.org","threadId":"30670","inReplyTo":"20120619233659.GA6621@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] Makefile: split prefix flags from GIT-CFLAGS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-19T23:58:07Z","receivedAt":"2012-06-19T23:58:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Of course, I am probably one of the few people in the world who actually\n> wants to have 50 built versions of git on hand.\n\nI have this script as \"rungit\" on my PATH and \"rungit -l\" shows 110+\nvariants. So at least there are two ;-)\n\n-- >8 --\n#!/bin/sh\n# Run various vintage of git\n\nvariant=\"${0##*/}\" &&\n: ${RUNGIT_BASE=$HOME/g} &&\ncase \"$variant\" in\nrungit)\n\tcase $# in \n\t0)\n\t\techo >&2 \"which version?\"\n\t\texit 1\n\t\t;;\n\tesac\n\tvariant=$1\n\tshift\n\t;;\nesac &&\ncase \"$variant\" in\n-l)\n\tfor d in \"$RUNGIT_BASE/\"git-*/bin/git\n\tdo\n\t\td=$(basename ${d%/bin/git})\n\t\td=${d#git-}\n\t\td=${d#snap-}\n\t\techo \"$d\"\n\tdone\n\texit\n\t;;\ngit-*)\n\tvariant=${variant#git-} ;;\nv[0-9]*)\n\tvariant=snap-$variant ;;\nesac &&\nd=\"$RUNGIT_BASE/git-$variant\" &&\nif test -f \"$d/bin/git\"\nthen\n\texec \"$d/bin/git\" \"$@\"\nelse\n\techo >&2 \"$variant: No such variant for $a\"\n\texit 1\nfi\n"},{"id":"193919","messageId":"20120620035015.GA4213@burratino","threadId":"30670","inReplyTo":"20120619232310.GA6496@sigill.intra.peff.net","subject":"Re: [PATCHv2 1/8] Makefile: apply dependencies consistently to sparse/asm targets","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-06-20T03:50:15Z","receivedAt":"2012-06-20T03:50:15Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nJeff King wrote:\n\n> When a C file includes a header file or depends on a\n> command-line \"-D\" macro, we note it in the Makefile like:\n>\n> git.o: common-cmds.h\n>\n> However, other targets built from the C file should also\n> know about this dependency (in particular, .sp and .s files\n> that are not part of the usual build process). We sometimes\n> noted these and sometimes not; let's make sure they are\n> always included.\n\nFirst reactions:\n\nThis improves consistency. Yay!\n\nMaking .sp and .s targets depend on generated .h files like\ncommon-cmds.h is very important. Otherwise, I would not be able to\ngenerate my git.s assembler listing or sparse-check git.c unless\ncommon-cmds.h has already been generated as a side-effect of some\nearlier build process.\n\nOn the other hand, making .sp and .s targets depend on preexisting .h\nfiles and files like GIT-CFLAGS would not have any effect at all,\nbecause:\n\n - .sp targets are phony --- there is no stamp file that certifies\n a given file has been checked by a \"make sparse\" run. Maybe that\n will change some day.\n\n - .s targets are rebuilt every time. Maybe I am just weird, but I\n find myself upgrading my compiler pretty often, so when I manually\n ask for an assembler listing I am happy to see it regenerated\n unconditionally using the new code generation rules.\n\nIt turns out that this patch is only about common-cmds.h, which was\nthe straightforward case. Why not say so and save the reader from\nhaving to think so hard? ;)\n\nHope that helps,\nJonathan\n"},{"id":"193920","messageId":"20120620042607.GA10414@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120620035015.GA4213@burratino","subject":"Re: [PATCHv2 1/8] Makefile: apply dependencies consistently to sparse/asm targets","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-20T04:26:07Z","receivedAt":"2012-06-20T04:26:07Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 19, 2012 at 10:50:15PM -0500, Jonathan Nieder wrote:\n\n> Making .sp and .s targets depend on generated .h files like\n> common-cmds.h is very important. Otherwise, I would not be able to\n> generate my git.s assembler listing or sparse-check git.c unless\n> common-cmds.h has already been generated as a side-effect of some\n> earlier build process.\n\nI suspect in most cases that the earlier build process has happened, and\nthat's why nobody really complained.\n\n> On the other hand, making .sp and .s targets depend on preexisting .h\n> files and files like GIT-CFLAGS would not have any effect at all,\n> because:\n> \n> - .sp targets are phony --- there is no stamp file that certifies\n> a given file has been checked by a \"make sparse\" run. Maybe that\n> will change some day.\n> \n> - .s targets are rebuilt every time. Maybe I am just weird, but I\n> find myself upgrading my compiler pretty often, so when I manually\n> ask for an assembler listing I am happy to see it regenerated\n> unconditionally using the new code generation rules.\n\nI don't have a strong opinion, as I don't use either feature. I noticed\nthe generated header file was a problem, and didn't realize that we\nforce .s builds.\n\nMy counters to the above points (and again, I don't really care\nthat much) would be:\n\n 1. The .sp and .s targets _do_ need the same -D macros that the .o\n targets get. So it ends up being very obvious that you are omitting\n them in something like:\n\n foo.o: GIT-VERSION-FILE\n foo.o foo.sp foo.s: EXTRA_CPPFLAGS = \\\n -DGIT_VERSION='$(GIT_VERSION)'\n\n I tend to think it is more readable to simply specify the\n dependencies fully (even if they end up being irrelevant because we\n force-build) than to confuse a reader who is not aware of our\n force-build '.s' rule that is 500 lines away (I was not aware of it\n until you mentioned it).\n\n 2. You describe the behavior now, and I certainly have no plans to\n change it. But there also plausible reasons for both cases to stop\n force-building, in which case these dependencies would become\n relevant.\n\nIn other words, I think relying on the force-build is a bit of a\nlayering violation. Of course, it is a Makefile, which is not exactly\nstructured programming, but I like to try.\n\n> It turns out that this patch is only about common-cmds.h, which was\n> the straightforward case. Why not say so and save the reader from\n> having to think so hard? ;)\n\nBecause I didn't realize it was the case at all. :) My intent was\nactually to clean up these lines so that they would be correct when I\nadded GIT-VERSION-FILES and the like to them later.\n\nSo I think my preference would be to tack on a note to the commit\nmessage saying \"yeah, this doesn't do anything for meta-dependencies,\nbut it doesn't hurt either\". OK?\n\n-Peff\n"},{"id":"193926","messageId":"20120620102750.GB4579@burratino","threadId":"30670","inReplyTo":"20120620042607.GA10414@sigill.intra.peff.net","subject":"Re: [PATCHv2 1/8] Makefile: apply dependencies consistently to sparse/asm targets","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-06-20T10:27:50Z","receivedAt":"2012-06-20T10:27:50Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n> 1. The .sp and .s targets _do_ need the same -D macros that the .o\n> targets get.\n\nAh, you mean EXTRA_CPPFLAGS. Yeah, that's also important, though the\npatch doesn't have anything to do with it.\n\nSome circuit in my mind missed that you meant EXTRA_CPPFLAGS and not a\nfile like GIT-CFLAGS.\n\n[...]\n>> It turns out that this patch is only about common-cmds.h, which was\n>> the straightforward case. Why not say so and save the reader from\n>> having to think so hard? ;)\n>\n> Because I didn't realize it was the case at all. :) My intent was\n> actually to clean up these lines so that they would be correct when I\n> added GIT-VERSION-FILES and the like to them later.\n>\n> So I think my preference would be to tack on a note to the commit\n> message saying \"yeah, this doesn't do anything for meta-dependencies,\n> but it doesn't hurt either\". OK?\n\nWhat is a meta-dependency? I would find that even more confusing.\n\nThis change could be motivated more simply by saying that it prevents\n\"make git.sp\", \"make git.s\", \"make help.s\", and \"make builtin/help.s\"\nfrom failing when common-cmds.h doesn't exist yet, no?\n\nThe commit message could also say that it is improving consistency,\nwhich is certainly valuable.\n\nAnd a mention of EXTRA_CPPFLAGS and generated header files vs.\npre-existing header files could help explain that consistency.\n\nBut suggesting that we are supposed to ignore the FORCE just leaves\nthe reader wondering why the same patch does not also urgently need\nto make additional changes such as the following, no?\n\n\tbuiltin/branch.o builtin/checkout.o builtin/clone.o \\\n\tbuiltin/reset.o branch.o transport.o: branch.h\n\nto\n\n\tbuiltin/branch.sp builtin/branch.o builtin/branch.s \\\n\tbuiltin/checkout.sp builtin/checkout.o builtin/checkout.s \\\n\tbuiltin/clone.sp builtin/clone.o builtin/clone.s \\\n\tbuiltin/reset.sp builtin/reset.o builtin/reset.s \\\n\tbranch.sp branch.o branch.s \\\n\ttransport.sp transport.o transport.s: branch.h\n\nHoping that clarifies,\nJonathan\n"},{"id":"193937","messageId":"20120620163714.GB12856@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120620102750.GB4579@burratino","subject":"Re: [PATCHv2 1/8] Makefile: apply dependencies consistently to sparse/asm targets","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-20T16:37:14Z","receivedAt":"2012-06-20T16:37:14Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 20, 2012 at 05:27:50AM -0500, Jonathan Nieder wrote:\n\n> Jeff King wrote:\n> \n> > 1. The .sp and .s targets _do_ need the same -D macros that the .o\n> > targets get.\n> \n> Ah, you mean EXTRA_CPPFLAGS. Yeah, that's also important, though the\n> patch doesn't have anything to do with it.\n> \n> Some circuit in my mind missed that you meant EXTRA_CPPFLAGS and not a\n> file like GIT-CFLAGS.\n\nNo, I meant GIT-CFLAGS. But the point of my series is that the two are\nintimately paired: if you are setting EXTRA_CPPFLAGS to mention a make\nvariable, then you should have a dependency on a file that changes if\nthat make variable changes.\n\n> > So I think my preference would be to tack on a note to the commit\n> > message saying \"yeah, this doesn't do anything for meta-dependencies,\n> > but it doesn't hurt either\". OK?\n> \n> What is a meta-dependency? I would find that even more confusing.\n\nIt is my term for things like GIT-CFLAGS; they are not really\ndependencies in the sense that the build process even looks at them, but\nthey are a marker whose timestamp changes when things which we _do_\nactually depend on change. Better name suggestions are welcome.\n\n> This change could be motivated more simply by saying that it prevents\n> \"make git.sp\", \"make git.s\", \"make help.s\", and \"make builtin/help.s\"\n> from failing when common-cmds.h doesn't exist yet, no?\n\nMore simply, perhaps, but that was not the entire motivation when\nwriting the patch. It is connected with patches later in the series\nwhich update those lines.\n\n> But suggesting that we are supposed to ignore the FORCE just leaves\n> the reader wondering why the same patch does not also urgently need\n> to make additional changes such as the following, no?\n> \n> \tbuiltin/branch.o builtin/checkout.o builtin/clone.o \\\n> \tbuiltin/reset.o branch.o transport.o: branch.h\n> \n> to\n> \n> \tbuiltin/branch.sp builtin/branch.o builtin/branch.s \\\n> [...]\n\nThose lines were not updated because I did not notice them, as I was\nkeeping the scope of the updates to generated headers and files like\nGIT-CFLAGS. IOW, my patch is a step in what I think is the right\ndirection, but it does not remove all issues, only one class of them.\n\nAs a side note, I have to wonder if those lines are really worthwhile.\nEverything already depends on LIB_H (when computed header dependencies\nare not used). Headers like \"branch.h\" seem to be split out of LIB_H to\navoid causing a full rebuild when uncommon headers are updated. But it\nis a half-hearted attempt; LIB_H has plenty of infrequently used\nheaders, and a solution which requires manually updating the target\nlists seems doomed to staleness. These days COMPUTE_HEADER_DEPENDENCIES\nis on by default, and I expect most developers use it.\n\nCan we just fold these few headers into LIB_H, let people without \"gcc\n-MMD\" deal with the extra compilation, and drop MISC_H and these extra\nmanual dependencies entirely? And note that \"extra compilation\" there\nonly happens when you are trying to rebuild a new version of git in a\nworking tree containing an older version (so probably bisection would be\nthe only place people would see it, and even then, only when jumping\nbetween versions that update one of the header files listed in MISC_H,\nbut _not_ any of the ones listed in LIB_H).\n\n-Peff\n"},{"id":"193951","messageId":"20120620182855.GA26948@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120620163714.GB12856@sigill.intra.peff.net","subject":"Re: [PATCHv2 1/8] Makefile: apply dependencies consistently to sparse/asm targets","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-20T18:28:55Z","receivedAt":"2012-06-20T18:28:55Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 20, 2012 at 12:37:14PM -0400, Jeff King wrote:\n\n> > But suggesting that we are supposed to ignore the FORCE just leaves\n> > the reader wondering why the same patch does not also urgently need\n> > to make additional changes such as the following, no?\n> > \n> > \tbuiltin/branch.o builtin/checkout.o builtin/clone.o \\\n> > \tbuiltin/reset.o branch.o transport.o: branch.h\n> > \n> > to\n> > \n> > \tbuiltin/branch.sp builtin/branch.o builtin/branch.s \\\n> > [...]\n> \n> Those lines were not updated because I did not notice them, as I was\n> keeping the scope of the updates to generated headers and files like\n> GIT-CFLAGS. IOW, my patch is a step in what I think is the right\n> direction, but it does not remove all issues, only one class of them.\n> \n> As a side note, I have to wonder if those lines are really worthwhile.\n> [...]\n\nHere's an updated series that drops these lines and I hope will address\nthe commit message issues you brought up:\n\n [01/11]: Makefile: sort LIB_H list\n [02/11]: Makefile: fold MISC_H into LIB_H\n\nNew in this iteration to get rid of these largely pointless manual\ndependencies.\n\n [03/11]: Makefile: do not have git.o depend on common-cmds.h\n\nNew in this iteration. I noticed while double-checking that this\ndependency is pointless.\n\n [04/11]: Makefile: apply dependencies consistently to sparse/asm targets\n\nUpdated based on earlier patches, and with a new commit message\nexplaining a little more of what's going on.\n\n [05/11]: Makefile: do not replace @@GIT_USER_AGENT@@ in scripts\n [06/11]: Makefile: split GIT_USER_AGENT from GIT-CFLAGS\n [07/11]: Makefile: split prefix flags from GIT-CFLAGS\n [08/11]: Makefile: do not replace @@GIT_VERSION@@ in shell scripts\n [09/11]: Makefile: update scripts when build-time parameters change\n [10/11]: Makefile: build instaweb similar to other scripts\n [11/11]: Makefile: move GIT-VERSION-FILE dependencies closer to use\n\nThe rest are largely the same, but with a few minor textual updates to\naccomodate the earlier changes.\n\n-Peff\n"},{"id":"193952","messageId":"20120620183008.GA30995@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120620182855.GA26948@sigill.intra.peff.net","subject":"[PATCHv3 01/11] Makefile: sort LIB_H list","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-20T18:30:08Z","receivedAt":"2012-06-20T18:30:08Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This was mostly sorted already, but put things like\n\"cache-tree.h\" after \"cache.h\", even though \"-\" comes before\n\".\" (at least in the C locale). This will make it easier to\nkeep the list sorted later by piping it through \"sort\".\n\nSigned-off-by: Jeff King \n---\n Makefile | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 62de0b4..72cdb56 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -595,8 +595,8 @@ LIB_H += attr.h\n LIB_H += blob.h\n LIB_H += builtin.h\n LIB_H += bulk-checkin.h\n-LIB_H += cache.h\n LIB_H += cache-tree.h\n+LIB_H += cache.h\n LIB_H += color.h\n LIB_H += commit.h\n LIB_H += compat/bswap.h\n@@ -636,13 +636,13 @@ LIB_H += mailmap.h\n LIB_H += merge-file.h\n LIB_H += merge-recursive.h\n LIB_H += mergesort.h\n-LIB_H += notes.h\n LIB_H += notes-cache.h\n LIB_H += notes-merge.h\n+LIB_H += notes.h\n LIB_H += object.h\n-LIB_H += pack.h\n LIB_H += pack-refs.h\n LIB_H += pack-revindex.h\n+LIB_H += pack.h\n LIB_H += parse-options.h\n LIB_H += patch-ids.h\n LIB_H += pkt-line.h\n@@ -668,8 +668,8 @@ LIB_H += submodule.h\n LIB_H += tag.h\n LIB_H += thread-utils.h\n LIB_H += transport.h\n-LIB_H += tree.h\n LIB_H += tree-walk.h\n+LIB_H += tree.h\n LIB_H += unpack-trees.h\n LIB_H += userdiff.h\n LIB_H += utf8.h\n-- \n1.7.11.5.gc0eeaa8\n"},{"id":"193953","messageId":"20120620183055.GB30995@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120620182855.GA26948@sigill.intra.peff.net","subject":"[PATCHv3 02/11] Makefile: fold MISC_H into LIB_H","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-20T18:30:56Z","receivedAt":"2012-06-20T18:30:56Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We keep a list of most of the header files in LIB_H, but\nsome are split out into MISC_H. The original point\nof LIB_H was that it would force recompilation of C files\nwhen any of the library headers changed. It was\nover-encompassing, since not all C files included all of the\nlibrary headers; this made it simple to maintain, but meant\nthat we sometimes recompiled when it was not necessary.\n\nOver time, some new headers were omitted from LIB_H, and\nrules were added to the Makefile for a few specific targets\nto explicitly depend on them. This avoided some unnecessary\nrecompilation at the cost of having to maintain the\ndependency list of those targets manually (e.g., d349a03).\n\nLater, we needed a complete list of headers from which we\nshould extract strings to localized. Thus 1b8b2e4 introduced\nMISC_H to mention all header files not included in LIB_H,\nand the concatenation of the two lists is fed to xgettext.\nHeaders mentioned as dependencies must also be manually\nadded to MISC_H to receive the benefits of localization.\n\nHaving to update multiple locations manually is a pain and\nhas led to errors. For example, see \"git log -Swt-status.h\nMakefile\" for some back-and-forth between the two locations.\nOr the fact that column.h was never added to MISC_H, and\ntherefore was not localized (which is fixed by this patch).\nMoreover, the benefits of keeping these few headers out of\nLIB_H is not that great, for two reasons:\n\n 1. The better way to do this is by auto-computing the\n dependencies, which is more accurate and less work to\n maintain. If your compiler supports it, we turn on\n computed header dependencies by default these days. So\n these manual dependencies are used only for people who\n do not have gcc at all (which increases the chance of\n them becoming stale, as many developers will never even\n use them).\n\n 2. Even if you do not have gcc, the manual header\n dependencies do not help all that much. They obviously\n cannot help with an initial compilation (since their\n purpose is to avoid unnecessary recompilation when a\n header changes), which means they are only useful when\n building a new version of git in the working tree that\n held an existing build (e.g., after checkout or during a\n bisection). But since a change of a header in LIB_H\n will force recompilation, and given that the vast\n majority of headers are in LIB_H, most version changes\n will result in a full rebuild anyway.\n\nLet's just fold MISC_H into LIB_H and get rid of these\nmanual rules. The worst case is some extra compilation, but\neven that is unlikely to matter due to the reasons above.\n\nThe one exception is that we should keep common-cmds.h\nseparate. Because it is generated, the computed dependencies\ndo not handle it properly, and we must keep separate\nindividual dependencies on it. Let's therefore rename MISC_H\nto GENERATED_H to make it more clear what should go in it.\n\nSigned-off-by: Jeff King \n---\n Makefile | 45 ++++++++++++++++-----------------------------\n 1 file changed, 16 insertions(+), 29 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 72cdb56..500966b 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -397,7 +397,7 @@ XDIFF_OBJS =\n VCSSVN_H =\n VCSSVN_OBJS =\n VCSSVN_TEST_OBJS =\n-MISC_H =\n+GENERATED_H =\n EXTRA_CPPFLAGS =\n LIB_H =\n LIB_OBJS =\n@@ -574,30 +574,22 @@ VCSSVN_H += vcs-svn/fast_export.h\n VCSSVN_H += vcs-svn/svndiff.h\n VCSSVN_H += vcs-svn/svndump.h\n \n-MISC_H += bisect.h\n-MISC_H += branch.h\n-MISC_H += bundle.h\n-MISC_H += common-cmds.h\n-MISC_H += fetch-pack.h\n-MISC_H += reachable.h\n-MISC_H += send-pack.h\n-MISC_H += shortlog.h\n-MISC_H += tar.h\n-MISC_H += thread-utils.h\n-MISC_H += url.h\n-MISC_H += walker.h\n-MISC_H += wt-status.h\n+GENERATED_H += common-cmds.h\n \n LIB_H += advice.h\n LIB_H += archive.h\n LIB_H += argv-array.h\n LIB_H += attr.h\n+LIB_H += bisect.h\n LIB_H += blob.h\n+LIB_H += branch.h\n LIB_H += builtin.h\n LIB_H += bulk-checkin.h\n+LIB_H += bundle.h\n LIB_H += cache-tree.h\n LIB_H += cache.h\n LIB_H += color.h\n+LIB_H += column.h\n LIB_H += commit.h\n LIB_H += compat/bswap.h\n LIB_H += compat/cygwin.h\n@@ -618,6 +610,7 @@ LIB_H += diff.h\n LIB_H += diffcore.h\n LIB_H += dir.h\n LIB_H += exec_cmd.h\n+LIB_H += fetch-pack.h\n LIB_H += fmt-merge-msg.h\n LIB_H += fsck.h\n LIB_H += gettext.h\n@@ -627,6 +620,7 @@ LIB_H += graph.h\n LIB_H += grep.h\n LIB_H += hash.h\n LIB_H += help.h\n+LIB_H += http.h\n LIB_H += kwset.h\n LIB_H += levenshtein.h\n LIB_H += list-objects.h\n@@ -649,6 +643,7 @@ LIB_H += pkt-line.h\n LIB_H += progress.h\n LIB_H += prompt.h\n LIB_H += quote.h\n+LIB_H += reachable.h\n LIB_H += reflog-walk.h\n LIB_H += refs.h\n LIB_H += remote.h\n@@ -656,9 +651,11 @@ LIB_H += rerere.h\n LIB_H += resolve-undo.h\n LIB_H += revision.h\n LIB_H += run-command.h\n+LIB_H += send-pack.h\n LIB_H += sequencer.h\n LIB_H += sha1-array.h\n LIB_H += sha1-lookup.h\n+LIB_H += shortlog.h\n LIB_H += sideband.h\n LIB_H += sigchain.h\n LIB_H += strbuf.h\n@@ -666,14 +663,18 @@ LIB_H += streaming.h\n LIB_H += string-list.h\n LIB_H += submodule.h\n LIB_H += tag.h\n+LIB_H += tar.h\n LIB_H += thread-utils.h\n LIB_H += transport.h\n LIB_H += tree-walk.h\n LIB_H += tree.h\n LIB_H += unpack-trees.h\n+LIB_H += url.h\n LIB_H += userdiff.h\n LIB_H += utf8.h\n LIB_H += varint.h\n+LIB_H += walker.h\n+LIB_H += wt-status.h\n LIB_H += xdiff-interface.h\n LIB_H += xdiff/xdiff.h\n \n@@ -2237,20 +2238,6 @@ else\n # gcc detects!\n \n $(GIT_OBJS): $(LIB_H)\n-builtin/branch.o builtin/checkout.o builtin/clone.o builtin/reset.o branch.o transport.o: branch.h\n-builtin/bundle.o bundle.o transport.o: bundle.h\n-builtin/bisect--helper.o builtin/rev-list.o bisect.o: bisect.h\n-builtin/clone.o builtin/fetch-pack.o transport.o: fetch-pack.h\n-builtin/index-pack.o builtin/grep.o builtin/pack-objects.o transport-helper.o thread-utils.o: thread-utils.h\n-builtin/send-pack.o transport.o: send-pack.h\n-builtin/log.o builtin/shortlog.o: shortlog.h\n-builtin/prune.o builtin/reflog.o reachable.o: reachable.h\n-builtin/commit.o builtin/revert.o wt-status.o: wt-status.h\n-builtin/tar-tree.o archive-tar.o: tar.h\n-connect.o transport.o url.o http-backend.o: url.h\n-builtin/branch.o builtin/commit.o builtin/tag.o column.o help.o pager.o: column.h\n-http-fetch.o http-walker.o remote-curl.o transport.o walker.o: walker.h\n-http.o http-walker.o http-push.o http-fetch.o remote-curl.o: http.h url.h\n \n xdiff-interface.o $(XDIFF_OBJS): $(XDIFF_H)\n \n@@ -2347,7 +2334,7 @@ XGETTEXT_FLAGS_C = $(XGETTEXT_FLAGS) --language=C \\\n \t--keyword=_ --keyword=N_ --keyword=\"Q_:1,2\"\n XGETTEXT_FLAGS_SH = $(XGETTEXT_FLAGS) --language=Shell\n XGETTEXT_FLAGS_PERL = $(XGETTEXT_FLAGS) --keyword=__ --language=Perl\n-LOCALIZED_C := $(C_OBJ:o=c) $(LIB_H) $(XDIFF_H) $(VCSSVN_H) $(MISC_H)\n+LOCALIZED_C := $(C_OBJ:o=c) $(LIB_H) $(XDIFF_H) $(VCSSVN_H) $(GENERATED_H)\n LOCALIZED_SH := $(SCRIPT_SH)\n LOCALIZED_PERL := $(SCRIPT_PERL)\n \n-- \n1.7.11.5.gc0eeaa8\n"},{"id":"193954","messageId":"20120620183118.GC30995@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120620182855.GA26948@sigill.intra.peff.net","subject":"[PATCHv3 03/11] Makefile: do not have git.o depend on common-cmds.h","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-20T18:31:18Z","receivedAt":"2012-06-20T18:31:18Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This dependency has been stale since 70827b1 (Split up\nbuiltin commands into separate files from git.c, 2006-04-21).\n\nSigned-off-by: Jeff King \n---\n Makefile | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex 500966b..58e2099 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1973,7 +1973,6 @@ shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell\n strip: $(PROGRAMS) git$X\n \t$(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X\n \n-git.o: common-cmds.h\n git.sp git.s git.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_HTML_PATH=\"$(htmldir_SQ)\"' \\\n \t'-DGIT_MAN_PATH=\"$(mandir_SQ)\"' \\\n-- \n1.7.11.5.gc0eeaa8\n"},{"id":"193955","messageId":"20120620183133.GD30995@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120620182855.GA26948@sigill.intra.peff.net","subject":"[PATCHv3 04/11] Makefile: apply dependencies consistently to sparse/asm targets","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-20T18:31:33Z","receivedAt":"2012-06-20T18:31:33Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When a C file \"foo.c\" depends on a generated header file, we\nnote the dependency for the \"foo.o\" target. However, we\nshould also note it for other targets that are built from\nfoo.c, like \"foo.sp\" and \"foo.s\". These tend to be missed\nbecause the latter two are not part of the default build,\nand are typically built after a regular build which will\ngenerate the header. Let's be consistent about including\nthem in dependencies.\n\nThis also makes us more consistent with nearby lines which\ntack on EXTRA_CPPFLAGS when building certain files. These\nflags may sometimes require extra dependencies to be added\n(e.g., like GIT-VERSION-FILE; this is not the case for any\nof the updated lines in this patch, but it is establishing a\nstyle that will be used in later patches). Technically the\n\".sp\" and \".s\" targets do not care about these dependencies,\nbecause they are force-built (\".sp\" because it is a phony\ntarget, and \".s\" because we explicitly force a rebuild).\n\nSince the blocks in question are about communicating \"things\nbuilt from foo.c depend on these flags\", it frees the reader\nfrom having to know or care more about how those targets are\nimplemented, and why it is OK for only \"foo.o\" to depend on\nGIT-VERSION-FILE while \"foo.sp\" and \"foo.s\" both are\nimpacted by $(GIT_VERSION). And it helps future-proof us if\nthose force-build details should ever change.\n\nThis patch explicitly does not update the static header\ndependencies used when COMPUTED_HEADER_DEPENDENCIES is off.\nThey are similar to the GIT-VERSION-FILE case above, in that\ntechnically \"foo.s\" would depend on its included headers,\nbut it is irrelevant because we force-build it anyway. So it\nwould be tempting to update them in the same way (for\nreadability and future-proofing). However, those rules are\nmeant as a fallback to the computed header dependencies,\nwhich do not handle \".s\" and \".sp\" at all (and are a much\nharder problem to solve, as gcc is the one generating those\ndependency lists).\n\nSo let's leave that harder problem until (and if) somebody\nwants to change the \".sp\" and \".s\" rules, and keep the\nstatic header dependencies consistent with the computed\nones.\n\nSigned-off-by: Jeff King \n---\n Makefile | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 58e2099..d2112f8 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1982,9 +1982,9 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ git.o \\\n \t\t$(BUILTIN_OBJS) $(ALL_LDFLAGS) $(LIBS)\n \n-help.sp help.o: common-cmds.h\n+help.sp help.s help.o: common-cmds.h\n \n-builtin/help.sp builtin/help.o: common-cmds.h\n+builtin/help.sp builtin/help.s builtin/help.o: common-cmds.h\n builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_HTML_PATH=\"$(htmldir_SQ)\"' \\\n \t'-DGIT_MAN_PATH=\"$(mandir_SQ)\"' \\\n-- \n1.7.11.5.gc0eeaa8\n"},{"id":"193956","messageId":"20120620183142.GE30995@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120620182855.GA26948@sigill.intra.peff.net","subject":"[PATCHv3 05/11] Makefile: do not replace @@GIT_USER_AGENT@@ in scripts","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-20T18:31:42Z","receivedAt":"2012-06-20T18:31:42Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"No scripts actually care about this replacement. This was\nerroneously added by d937411.\n\nSigned-off-by: Jeff King \n---\n Makefile | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex d2112f8..336cfc4 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2010,7 +2010,6 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \\\n -e 's|@@DIFF@@|$(DIFF_SQ)|' \\\n -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \\\n- -e 's|@@GIT_USER_AGENT@@|$(GIT_USER_AGENT_SQ)|g' \\\n -e 's|@@LOCALEDIR@@|$(localedir_SQ)|g' \\\n -e 's/@@NO_CURL@@/$(NO_CURL)/g' \\\n -e 's/@@USE_GETTEXT_SCHEME@@/$(USE_GETTEXT_SCHEME)/g' \\\n-- \n1.7.11.5.gc0eeaa8\n"},{"id":"193957","messageId":"20120620183151.GF30995@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120620182855.GA26948@sigill.intra.peff.net","subject":"[PATCHv3 06/11] Makefile: split GIT_USER_AGENT from GIT-CFLAGS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-20T18:31:51Z","receivedAt":"2012-06-20T18:31:51Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The default user-agent depends on the GIT_VERSION, which\nmeans that anytime you switch versions, it causes a full\nrebuild. Instead, let's split it out into its own file and\nrestrict the dependency to version.o.\n\nSigned-off-by: Jeff King \n---\n .gitignore | 1 +\n Makefile | 11 +++++++++--\n 2 files changed, 10 insertions(+), 2 deletions(-)\n\ndiff --git a/.gitignore b/.gitignore\nindex bf66648..7329cfe 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -2,6 +2,7 @@\n /GIT-CFLAGS\n /GIT-LDFLAGS\n /GIT-GUI-VARS\n+/GIT-USER-AGENT\n /GIT-VERSION-FILE\n /bin-wrappers/\n /git\ndiff --git a/Makefile b/Makefile\nindex 336cfc4..7518ad7 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1925,7 +1925,11 @@ endif\n GIT_USER_AGENT_SQ = $(subst ','\\'',$(GIT_USER_AGENT))\n GIT_USER_AGENT_CQ = \"$(subst \",\\\",$(subst \\,\\\\,$(GIT_USER_AGENT)))\"\n GIT_USER_AGENT_CQ_SQ = $(subst ','\\'',$(GIT_USER_AGENT_CQ))\n-BASIC_CFLAGS += -DGIT_USER_AGENT='$(GIT_USER_AGENT_CQ_SQ)'\n+GIT-USER-AGENT: FORCE\n+\t@if test x'$(GIT_USER_AGENT_SQ)' != x\"`cat GIT-USER-AGENT 2>/dev/null`\"; then \\\n+\t\techo >&2 \" * new user-agent flag\"; \\\n+\t\techo '$(GIT_USER_AGENT_SQ)' >GIT-USER-AGENT; \\\n+\tfi\n \n ALL_CFLAGS += $(BASIC_CFLAGS)\n ALL_LDFLAGS += $(BASIC_LDFLAGS)\n@@ -1990,8 +1994,10 @@ builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_MAN_PATH=\"$(mandir_SQ)\"' \\\n \t'-DGIT_INFO_PATH=\"$(infodir_SQ)\"'\n \n+version.sp version.s version.o: GIT-USER-AGENT\n version.sp version.s version.o: EXTRA_CPPFLAGS = \\\n-\t'-DGIT_VERSION=\"$(GIT_VERSION)\"'\n+\t'-DGIT_VERSION=\"$(GIT_VERSION)\"' \\\n+\t'-DGIT_USER_AGENT=$(GIT_USER_AGENT_CQ_SQ)'\n \n $(BUILT_INS): git$X\n \t$(QUIET_BUILT_IN)$(RM) $@ && \\\n@@ -2722,6 +2728,7 @@ ifndef NO_TCLTK\n \t$(MAKE) -C git-gui clean\n endif\n \t$(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-GUI-VARS GIT-BUILD-OPTIONS\n+\t$(RM) GIT-USER-AGENT\n \n .PHONY: all install profile-clean clean strip\n .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell\n-- \n1.7.11.5.gc0eeaa8\n"},{"id":"193958","messageId":"20120620183155.GG30995@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120620182855.GA26948@sigill.intra.peff.net","subject":"[PATCHv3 07/11] Makefile: split prefix flags from GIT-CFLAGS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-20T18:31:55Z","receivedAt":"2012-06-20T18:31:55Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Most of the build targets do not care about the setting of\n$prefix (or its derivative variables), but will be rebuilt\nif the prefix changes. For most setups this doesn't matter\n(they set prefix once and never change it), but for a setup\nwhich puts each branch or version in its own prefix, this\nunnecessarily causes a full rebuild whenever the branc is\nchanged.\n\nSigned-off-by: Jeff King \n---\n .gitignore | 1 +\n Makefile | 30 ++++++++++++++++++++++--------\n 2 files changed, 23 insertions(+), 8 deletions(-)\n\ndiff --git a/.gitignore b/.gitignore\nindex 7329cfe..c60c5a3 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -2,6 +2,7 @@\n /GIT-CFLAGS\n /GIT-LDFLAGS\n /GIT-GUI-VARS\n+/GIT-PREFIX\n /GIT-USER-AGENT\n /GIT-VERSION-FILE\n /bin-wrappers/\ndiff --git a/Makefile b/Makefile\nindex 7518ad7..957b6a6 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1977,6 +1977,7 @@ shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell\n strip: $(PROGRAMS) git$X\n \t$(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X\n \n+git.sp git.s git.o: GIT-PREFIX\n git.sp git.s git.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_HTML_PATH=\"$(htmldir_SQ)\"' \\\n \t'-DGIT_MAN_PATH=\"$(mandir_SQ)\"' \\\n@@ -1988,7 +1989,7 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n \n help.sp help.s help.o: common-cmds.h\n \n-builtin/help.sp builtin/help.s builtin/help.o: common-cmds.h\n+builtin/help.sp builtin/help.s builtin/help.o: common-cmds.h GIT-PREFIX\n builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_HTML_PATH=\"$(htmldir_SQ)\"' \\\n \t'-DGIT_MAN_PATH=\"$(mandir_SQ)\"' \\\n@@ -2035,7 +2036,7 @@ $(SCRIPT_LIB) : % : %.sh\n ifndef NO_PERL\n $(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak\n \n-perl/perl.mak: GIT-CFLAGS perl/Makefile perl/Makefile.PL\n+perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL\n \t$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' $(@F)\n \n $(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl\n@@ -2079,7 +2080,7 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)) git-instaweb: % : unimplemented.sh\n endif # NO_PERL\n \n ifndef NO_PYTHON\n-$(patsubst %.py,%,$(SCRIPT_PYTHON)): GIT-CFLAGS\n+$(patsubst %.py,%,$(SCRIPT_PYTHON)): GIT-CFLAGS GIT-PREFIX\n $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : %.py\n \t$(QUIET_GEN)$(RM) $@ $@+ && \\\n \tINSTLIBDIR=`MAKEFLAGS= $(MAKE) -C git_remote_helpers -s \\\n@@ -2248,20 +2249,25 @@ xdiff-interface.o $(XDIFF_OBJS): $(XDIFF_H)\n $(VCSSVN_OBJS) $(VCSSVN_TEST_OBJS): $(LIB_H) $(VCSSVN_H)\n endif\n \n+exec_cmd.sp exec_cmd.s exec_cmd.o: GIT-PREFIX\n exec_cmd.sp exec_cmd.s exec_cmd.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_EXEC_PATH=\"$(gitexecdir_SQ)\"' \\\n \t'-DBINDIR=\"$(bindir_relative_SQ)\"' \\\n \t'-DPREFIX=\"$(prefix_SQ)\"'\n \n+builtin/init-db.sp builtin/init-db.s builtin/init-db.o: GIT-PREFIX\n builtin/init-db.sp builtin/init-db.s builtin/init-db.o: EXTRA_CPPFLAGS = \\\n \t-DDEFAULT_GIT_TEMPLATE_DIR='\"$(template_dir_SQ)\"'\n \n+config.sp config.s config.o: GIT-PREFIX\n config.sp config.s config.o: EXTRA_CPPFLAGS = \\\n \t-DETC_GITCONFIG='\"$(ETC_GITCONFIG_SQ)\"'\n \n+attr.sp attr.s attr.o: GIT-PREFIX\n attr.sp attr.s attr.o: EXTRA_CPPFLAGS = \\\n \t-DETC_GITATTRIBUTES='\"$(ETC_GITATTRIBUTES_SQ)\"'\n \n+gettext.sp gettext.s gettext.o: GIT-PREFIX\n gettext.sp gettext.s gettext.o: EXTRA_CPPFLAGS = \\\n \t-DGIT_LOCALE_PATH='\"$(localedir_SQ)\"'\n \n@@ -2385,14 +2391,22 @@ cscope:\n \t$(FIND_SOURCE_FILES) | xargs cscope -b\n \n ### Detect prefix changes\n-TRACK_CFLAGS = $(CC):$(subst ','\\'',$(ALL_CFLAGS)):\\\n- $(bindir_SQ):$(gitexecdir_SQ):$(template_dir_SQ):$(prefix_SQ):\\\n- $(localedir_SQ):$(USE_GETTEXT_SCHEME)\n+TRACK_PREFIX = $(bindir_SQ):$(gitexecdir_SQ):$(template_dir_SQ):$(prefix_SQ):\\\n+\t\t$(localedir_SQ)\n+\n+GIT-PREFIX: FORCE\n+\t@FLAGS='$(TRACK_PREFIX)'; \\\n+\tif test x\"$$FLAGS\" != x\"`cat GIT-PREFIX 2>/dev/null`\" ; then \\\n+\t\techo 1>&2 \" * new prefix flags\"; \\\n+\t\techo \"$$FLAGS\" >GIT-PREFIX; \\\n+\tfi\n+\n+TRACK_CFLAGS = $(CC):$(subst ','\\'',$(ALL_CFLAGS)):$(USE_GETTEXT_SCHEME)\n \n GIT-CFLAGS: FORCE\n \t@FLAGS='$(TRACK_CFLAGS)'; \\\n \t if test x\"$$FLAGS\" != x\"`cat GIT-CFLAGS 2>/dev/null`\" ; then \\\n-\t\techo 1>&2 \" * new build flags or prefix\"; \\\n+\t\techo 1>&2 \" * new build flags\"; \\\n \t\techo \"$$FLAGS\" >GIT-CFLAGS; \\\n fi\n \n@@ -2728,7 +2742,7 @@ ifndef NO_TCLTK\n \t$(MAKE) -C git-gui clean\n endif\n \t$(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-GUI-VARS GIT-BUILD-OPTIONS\n-\t$(RM) GIT-USER-AGENT\n+\t$(RM) GIT-USER-AGENT GIT-PREFIX\n \n .PHONY: all install profile-clean clean strip\n .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell\n-- \n1.7.11.5.gc0eeaa8\n"},{"id":"193962","messageId":"20120620183209.GH30995@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120620182855.GA26948@sigill.intra.peff.net","subject":"[PATCHv3 08/11] Makefile: do not replace @@GIT_VERSION@@ in shell scripts","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-20T18:32:10Z","receivedAt":"2012-06-20T18:32:10Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"No shell script actually uses the replacement (it is used in\nsome perl scripts, but cmd_munge_script only handles shell\nscripts). We can also therefore drop the dependency on\nGIT-VERSION-FILE.\n\nSigned-off-by: Jeff King \n---\n Makefile | 3 ---\n 1 file changed, 3 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 957b6a6..b977903 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2016,7 +2016,6 @@ $(RM) $@ $@+ && \\\n sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \\\n -e 's|@@DIFF@@|$(DIFF_SQ)|' \\\n- -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \\\n -e 's|@@LOCALEDIR@@|$(localedir_SQ)|g' \\\n -e 's/@@NO_CURL@@/$(NO_CURL)/g' \\\n -e 's/@@USE_GETTEXT_SCHEME@@/$(USE_GETTEXT_SCHEME)/g' \\\n@@ -2062,7 +2061,6 @@ gitweb:\n git-instaweb: git-instaweb.sh gitweb\n \t$(QUIET_GEN)$(RM) $@ $@+ && \\\n \tsed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n-\t -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \\\n \t -e 's/@@NO_CURL@@/$(NO_CURL)/g' \\\n \t -e 's|@@GITWEBDIR@@|$(gitwebdir_SQ)|g' \\\n \t -e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \\\n@@ -2111,7 +2109,6 @@ configure: configure.ac\n \n # These can record GIT_VERSION\n version.o git.spec \\\n-\t$(patsubst %.sh,%,$(SCRIPT_SH)) \\\n \t$(patsubst %.perl,%,$(SCRIPT_PERL)) \\\n \t: GIT-VERSION-FILE\n \n-- \n1.7.11.5.gc0eeaa8\n"},{"id":"193961","messageId":"20120620183216.GI30995@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120620182855.GA26948@sigill.intra.peff.net","subject":"[PATCHv3 09/11] Makefile: update scripts when build-time parameters change","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-20T18:32:16Z","receivedAt":"2012-06-20T18:32:16Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Currently, running:\n\n make SHELL_PATH=/bin/bash &&\n make SHELL_PATH=/bin/sh\n\nwill not rebuild any shell scripts in the second command,\nleading to incorrect results when building from an unclean\nworking directory.\n\nThis patch introduces a new dependency meta-file to notice\nthe change.\n\nSigned-off-by: Jeff King \n---\n .gitignore | 1 +\n Makefile | 16 +++++++++++++---\n 2 files changed, 14 insertions(+), 3 deletions(-)\n\ndiff --git a/.gitignore b/.gitignore\nindex c60c5a3..6535cd7 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -3,6 +3,7 @@\n /GIT-LDFLAGS\n /GIT-GUI-VARS\n /GIT-PREFIX\n+/GIT-SCRIPT-DEFINES\n /GIT-USER-AGENT\n /GIT-VERSION-FILE\n /bin-wrappers/\ndiff --git a/Makefile b/Makefile\nindex b977903..ea55179 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2011,6 +2011,8 @@ common-cmds.h: ./generate-cmdlist.sh command-list.txt\n common-cmds.h: $(wildcard Documentation/git-*.txt)\n \t$(QUIET_GEN)./generate-cmdlist.sh > $@+ && mv $@+ $@\n \n+SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\\\n+\t$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ)\n define cmd_munge_script\n $(RM) $@ $@+ && \\\n sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n@@ -2023,12 +2025,20 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n $@.sh >$@+\n endef\n \n-$(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh\n+GIT-SCRIPT-DEFINES: FORCE\n+\t@FLAGS='$(SCRIPT_DEFINES)'; \\\n+\t if test x\"$$FLAGS\" != x\"`cat $@ 2>/dev/null`\" ; then \\\n+\t\techo 1>&2 \" * new script parameters\"; \\\n+\t\techo \"$$FLAGS\" >$@; \\\n+ fi\n+\n+\n+$(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh GIT-SCRIPT-DEFINES\n \t$(QUIET_GEN)$(cmd_munge_script) && \\\n \tchmod +x $@+ && \\\n \tmv $@+ $@\n \n-$(SCRIPT_LIB) : % : %.sh\n+$(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES\n \t$(QUIET_GEN)$(cmd_munge_script) && \\\n \tmv $@+ $@\n \n@@ -2739,7 +2749,7 @@ ifndef NO_TCLTK\n \t$(MAKE) -C git-gui clean\n endif\n \t$(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-GUI-VARS GIT-BUILD-OPTIONS\n-\t$(RM) GIT-USER-AGENT GIT-PREFIX\n+\t$(RM) GIT-USER-AGENT GIT-PREFIX GIT-SCRIPT-DEFINES\n \n .PHONY: all install profile-clean clean strip\n .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell\n-- \n1.7.11.5.gc0eeaa8\n"},{"id":"193960","messageId":"20120620183219.GJ30995@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120620182855.GA26948@sigill.intra.peff.net","subject":"[PATCHv3 10/11] Makefile: build instaweb similar to other scripts","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-20T18:32:19Z","receivedAt":"2012-06-20T18:32:19Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Instaweb would not properly rebuild if the build-time\nparameters changed. Fix this by depending on the\nGIT-SCRIPT-DEFINES meta-file and using $(cmd_munge_script)\nlike all the other shell scripts. This requires adding a few\nnew parametres to cmd_munge_script, but that doesn't hurt\nexisting scripts.\n\nSigned-off-by: Jeff King \n---\n Makefile | 14 ++++++--------\n 1 file changed, 6 insertions(+), 8 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex ea55179..592e6cb 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2012,7 +2012,8 @@ common-cmds.h: $(wildcard Documentation/git-*.txt)\n \t$(QUIET_GEN)./generate-cmdlist.sh > $@+ && mv $@+ $@\n \n SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\\\n-\t$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ)\n+\t$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\\\n+\t$(gitwebdir_SQ):$(PERL_PATH_SQ)\n define cmd_munge_script\n $(RM) $@ $@+ && \\\n sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n@@ -2022,6 +2023,8 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n -e 's/@@NO_CURL@@/$(NO_CURL)/g' \\\n -e 's/@@USE_GETTEXT_SCHEME@@/$(USE_GETTEXT_SCHEME)/g' \\\n -e $(BROKEN_PATH_FIX) \\\n+ -e 's|@@GITWEBDIR@@|$(gitwebdir_SQ)|g' \\\n+ -e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \\\n $@.sh >$@+\n endef\n \n@@ -2068,13 +2071,8 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl\n gitweb:\n \t$(QUIET_SUBDIR0)gitweb $(QUIET_SUBDIR1) all\n \n-git-instaweb: git-instaweb.sh gitweb\n-\t$(QUIET_GEN)$(RM) $@ $@+ && \\\n-\tsed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n-\t -e 's/@@NO_CURL@@/$(NO_CURL)/g' \\\n-\t -e 's|@@GITWEBDIR@@|$(gitwebdir_SQ)|g' \\\n-\t -e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \\\n-\t $@.sh > $@+ && \\\n+git-instaweb: git-instaweb.sh gitweb GIT-SCRIPT-DEFINES\n+\t$(QUIET_GEN)$(cmd_munge_script) && \\\n \tchmod +x $@+ && \\\n \tmv $@+ $@\n else # NO_PERL\n-- \n1.7.11.5.gc0eeaa8\n"},{"id":"193959","messageId":"20120620183222.GK30995@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120620182855.GA26948@sigill.intra.peff.net","subject":"[PATCHv3 11/11] Makefile: move GIT-VERSION-FILE dependencies closer to use","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-20T18:32:22Z","receivedAt":"2012-06-20T18:32:22Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"There is a list of all of the targets which depend on\nGIT-VERSION-FILE, but it can be quite far from the actual\npoint where the targets actually use $(GIT_VERSION). This\ncan make it hard to verify that each use of $(GIT_VERSION)\nhas a matching dependency.\n\nThis patch moves the dependency closer to the actual build\ninstructions, which makes verification easier. This also\nfixes the generation of \"configure\", which did not properly\nmark the dependency.\n\nSigned-off-by: Jeff King \n---\n Makefile | 13 ++++---------\n 1 file changed, 4 insertions(+), 9 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 592e6cb..6b8bfa4 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1995,7 +1995,7 @@ builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_MAN_PATH=\"$(mandir_SQ)\"' \\\n \t'-DGIT_INFO_PATH=\"$(infodir_SQ)\"'\n \n-version.sp version.s version.o: GIT-USER-AGENT\n+version.sp version.s version.o: GIT-VERSION-FILE GIT-USER-AGENT\n version.sp version.s version.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_VERSION=\"$(GIT_VERSION)\"' \\\n \t'-DGIT_USER_AGENT=$(GIT_USER_AGENT_CQ_SQ)'\n@@ -2051,7 +2051,7 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak\n perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL\n \t$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' $(@F)\n \n-$(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl\n+$(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl GIT-VERSION-FILE\n \t$(QUIET_GEN)$(RM) $@ $@+ && \\\n \tINSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir` && \\\n \tsed -e '1{' \\\n@@ -2108,18 +2108,13 @@ $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : unimplemented.sh\n \tmv $@+ $@\n endif # NO_PYTHON\n \n-configure: configure.ac\n+configure: configure.ac GIT-VERSION-FILE\n \t$(QUIET_GEN)$(RM) $@ $<+ && \\\n \tsed -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \\\n \t $< > $<+ && \\\n \tautoconf -o $@ $<+ && \\\n \t$(RM) $<+\n \n-# These can record GIT_VERSION\n-version.o git.spec \\\n-\t$(patsubst %.perl,%,$(SCRIPT_PERL)) \\\n-\t: GIT-VERSION-FILE\n-\n TEST_OBJS := $(patsubst test-%$X,test-%.o,$(TEST_PROGRAMS))\n GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \\\n \tgit.o\n@@ -2663,7 +2658,7 @@ quick-install-html:\n \n ### Maintainer's dist rules\n \n-git.spec: git.spec.in\n+git.spec: git.spec.in GIT-VERSION-FILE\n \tsed -e 's/@@VERSION@@/$(GIT_VERSION)/g' < $< > $@+\n \tmv $@+ $@\n \n-- \n1.7.11.5.gc0eeaa8\n"},{"id":"193972","messageId":"CAFzf2Xw3TdvZCFLvbqKY5F9b+0hTzTQEEfmqjL9u=uvyc7mZ5w@mail.gmail.com","threadId":"30670","inReplyTo":"20120620182855.GA26948@sigill.intra.peff.net","subject":"Re: [PATCHv2 1/8] Makefile: apply dependencies consistently to sparse/asm targets","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-06-20T19:30:40Z","receivedAt":"2012-06-20T19:30:40Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n> Here's an updated series that drops these lines and I hope will address\n> the commit message issues you brought up:\n>\n>  [01/11]: Makefile: sort LIB_H list\n>  [02/11]: Makefile: fold MISC_H into LIB_H\n\nPlease no. Progress would be teaching the remaining compilers to\nautocompute header dependencies so there would be no need to maintain\na master header list at all. I would understand if the headers had\nbeen falling out of date and this were a way of saying \"we give up\",\nbut given that that is not happening, why would this change be a good\nthing?\n\nMy comments were only about explaining what I found hard to understand\nin the commit message. The patch was good. Please don't retaliate this\nway. :)\n\nHope that helps,\nJonathan\n"},{"id":"193977","messageId":"20120620193638.GA32418@sigill.intra.peff.net","threadId":"30670","inReplyTo":"CAFzf2Xw3TdvZCFLvbqKY5F9b+0hTzTQEEfmqjL9u=uvyc7mZ5w@mail.gmail.com","subject":"Re: [PATCHv2 1/8] Makefile: apply dependencies consistently to sparse/asm targets","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-20T19:36:39Z","receivedAt":"2012-06-20T19:36:39Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 20, 2012 at 02:30:40PM -0500, Jonathan Nieder wrote:\n\n> Jeff King wrote:\n> \n> > Here's an updated series that drops these lines and I hope will address\n> > the commit message issues you brought up:\n> >\n> >  [01/11]: Makefile: sort LIB_H list\n> >  [02/11]: Makefile: fold MISC_H into LIB_H\n> \n> Please no. Progress would be teaching the remaining compilers to\n> autocompute header dependencies so there would be no need to maintain\n> a master header list at all. I would understand if the headers had\n> been falling out of date and this were a way of saying \"we give up\",\n> but given that that is not happening, why would this change be a good\n> thing?\n\nDid you read the argument in patch 2? They are almost certainly not\nhelping anyone, anyway. Forward progress would be to split all of LIB_H\nout into specific targets. But keeping that up to date manually is\ninsanity, so we have this silly half-hearted attempt.\n\nI would much rather \"include header-deps.mk\", and then periodically\nrebuild and commit it using \"gcc -M\" (and cleaning up and canonicalizing\nthe result) for the benefit of those without access to a compiler that\ncan do header dependencies.\n\nIn an ideal world, all compilers could do it, but I am not volunteering\nto patch MSVC. ;)\n\n> My comments were only about explaining what I found hard to understand\n> in the commit message. The patch was good. Please don't retaliate this\n> way. :)\n\n:) The revised commit message in patch 4 should make sense with or\nwithout these patches, so it really is a separate issue.\n\n-Peff\n"},{"id":"193980","messageId":"CAFzf2XwmZo4ErG_9w0m66k4OPtDr_4_xvATL_6sOo8QVg0DhJA@mail.gmail.com","threadId":"30670","inReplyTo":"20120620193638.GA32418@sigill.intra.peff.net","subject":"Re: [PATCHv2 1/8] Makefile: apply dependencies consistently to sparse/asm targets","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-06-20T19:45:31Z","receivedAt":"2012-06-20T19:45:31Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n> Did you read the argument in patch 2? They are almost certainly not\n> helping anyone, anyway.\n\nYes, I read patch 2. I hacked on git from time to time in the days\nbefore COMPUTE_HEADER_DEPENDENCIES, and it sometimes involved changing\nheader files. When they were not in LIB_H, the experience was much\nnicer.\n\nIs that called \"not helping\"? I'm afraid I don't follow this line of\nargument at all.\n\nOn the other hand, if someone were proposing adding a simple awk\nscript to implement a \"make dep\" fallback, I would understand that.\n"},{"id":"193985","messageId":"20120620195709.GB32228@sigill.intra.peff.net","threadId":"30670","inReplyTo":"CAFzf2XwmZo4ErG_9w0m66k4OPtDr_4_xvATL_6sOo8QVg0DhJA@mail.gmail.com","subject":"Re: [PATCHv2 1/8] Makefile: apply dependencies consistently to sparse/asm targets","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-20T19:57:09Z","receivedAt":"2012-06-20T19:57:09Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 20, 2012 at 02:45:31PM -0500, Jonathan Nieder wrote:\n\n> Jeff King wrote:\n> \n> > Did you read the argument in patch 2? They are almost certainly not\n> > helping anyone, anyway.\n> \n> Yes, I read patch 2. I hacked on git from time to time in the days\n> before COMPUTE_HEADER_DEPENDENCIES, and it sometimes involved changing\n> header files. When they were not in LIB_H, the experience was much\n> nicer.\n> \n> Is that called \"not helping\"? I'm afraid I don't follow this line of\n> argument at all.\n\nI just assumed that people who are actively hacking on individual header\nfiles in git actually have a compiler that can do COMPUTE_HEADER_DEPENDENCIES.\nMaybe that is not the case. If it were such a big deal, then why is\neverything in LIB_H? Why don't people use these manual rules, or convert\nexisting LIB_H entries to use them?\n\nFor people who are not actively hacking on header files in git, the\narguments from that patch apply (namely that LIB_H is so gigantic that\nyou are unlikely to hit a specific change where one of the few manual\nrules is triggered, but LIB_H is not).\n\n> On the other hand, if someone were proposing adding a simple awk\n> script to implement a \"make dep\" fallback, I would understand that.\n\nI'd be OK with that. Do you have one in mind, or do we need to write it\nfrom scratch? Surely somebody else has solved this problem before.\n\n-Peff\n"},{"id":"193986","messageId":"7vwr31u532.fsf@alter.siamese.dyndns.org","threadId":"30670","inReplyTo":"20120620183008.GA30995@sigill.intra.peff.net","subject":"Re: [PATCHv3 01/11] Makefile: sort LIB_H list","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-20T20:00:01Z","receivedAt":"2012-06-20T20:00:01Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> This was mostly sorted already, but put things like\n> \"cache-tree.h\" after \"cache.h\", even though \"-\" comes before\n> \".\" (at least in the C locale). This will make it easier to\n> keep the list sorted later by piping it through \"sort\".\n\nI agree this would make it easier to blindly run \"sort\", but I think\nthe result hurts scannability. Is it a good change?\n\n> Signed-off-by: Jeff King \n> ---\n> Makefile | 8 ++++----\n> 1 file changed, 4 insertions(+), 4 deletions(-)\n>\n> diff --git a/Makefile b/Makefile\n> index 62de0b4..72cdb56 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -595,8 +595,8 @@ LIB_H += attr.h\n> LIB_H += blob.h\n> LIB_H += builtin.h\n> LIB_H += bulk-checkin.h\n> -LIB_H += cache.h\n> LIB_H += cache-tree.h\n> +LIB_H += cache.h\n> LIB_H += color.h\n> LIB_H += commit.h\n> LIB_H += compat/bswap.h\n> @@ -636,13 +636,13 @@ LIB_H += mailmap.h\n> LIB_H += merge-file.h\n> LIB_H += merge-recursive.h\n> LIB_H += mergesort.h\n> -LIB_H += notes.h\n> LIB_H += notes-cache.h\n> LIB_H += notes-merge.h\n> +LIB_H += notes.h\n> LIB_H += object.h\n> -LIB_H += pack.h\n> LIB_H += pack-refs.h\n> LIB_H += pack-revindex.h\n> +LIB_H += pack.h\n> LIB_H += parse-options.h\n> LIB_H += patch-ids.h\n> LIB_H += pkt-line.h\n> @@ -668,8 +668,8 @@ LIB_H += submodule.h\n> LIB_H += tag.h\n> LIB_H += thread-utils.h\n> LIB_H += transport.h\n> -LIB_H += tree.h\n> LIB_H += tree-walk.h\n> +LIB_H += tree.h\n> LIB_H += unpack-trees.h\n> LIB_H += userdiff.h\n> LIB_H += utf8.h\n"},{"id":"193987","messageId":"20120620200151.GA388@sigill.intra.peff.net","threadId":"30670","inReplyTo":"7vwr31u532.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv3 01/11] Makefile: sort LIB_H list","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-20T20:01:51Z","receivedAt":"2012-06-20T20:01:51Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 20, 2012 at 01:00:01PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > This was mostly sorted already, but put things like\n> > \"cache-tree.h\" after \"cache.h\", even though \"-\" comes before\n> > \".\" (at least in the C locale). This will make it easier to\n> > keep the list sorted later by piping it through \"sort\".\n> \n> I agree this would make it easier to blindly run \"sort\", but I think\n> the result hurts scannability. Is it a good change?\n\nI don't personally see a difference, but if you do, I don't care that\nmuch about this patch. The main motive was that the next patch involved\nadding 10 new entries, and I didn't want to have to insert them\nmanually into the correct sorting spots.\n\n-Peff\n"},{"id":"193988","messageId":"7vsjdpu4zt.fsf@alter.siamese.dyndns.org","threadId":"30670","inReplyTo":"20120620183055.GB30995@sigill.intra.peff.net","subject":"Re: [PATCHv3 02/11] Makefile: fold MISC_H into LIB_H","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-20T20:01:58Z","receivedAt":"2012-06-20T20:01:58Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Let's just fold MISC_H into LIB_H and get rid of these\n> manual rules. The worst case is some extra compilation, but\n> even that is unlikely to matter due to the reasons above.\n\nYay. Thanks.\n"},{"id":"193989","messageId":"7vobodu4sj.fsf@alter.siamese.dyndns.org","threadId":"30670","inReplyTo":"20120620183142.GE30995@sigill.intra.peff.net","subject":"Re: [PATCHv3 05/11] Makefile: do not replace @@GIT_USER_AGENT@@ in scripts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-20T20:06:20Z","receivedAt":"2012-06-20T20:06:20Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> No scripts actually care about this replacement. This was\n> erroneously added by d937411.\n\nd937411???\n\n>\n> Signed-off-by: Jeff King \n> ---\n> Makefile | 1 -\n> 1 file changed, 1 deletion(-)\n>\n> diff --git a/Makefile b/Makefile\n> index d2112f8..336cfc4 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -2010,7 +2010,6 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n> -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \\\n> -e 's|@@DIFF@@|$(DIFF_SQ)|' \\\n> -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \\\n> - -e 's|@@GIT_USER_AGENT@@|$(GIT_USER_AGENT_SQ)|g' \\\n> -e 's|@@LOCALEDIR@@|$(localedir_SQ)|g' \\\n> -e 's/@@NO_CURL@@/$(NO_CURL)/g' \\\n> -e 's/@@USE_GETTEXT_SCHEME@@/$(USE_GETTEXT_SCHEME)/g' \\\n"},{"id":"193990","messageId":"20120620200906.GA1397@sigill.intra.peff.net","threadId":"30670","inReplyTo":"7vobodu4sj.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv3 05/11] Makefile: do not replace @@GIT_USER_AGENT@@ in scripts","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-20T20:09:07Z","receivedAt":"2012-06-20T20:09:07Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 20, 2012 at 01:06:20PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > No scripts actually care about this replacement. This was\n> > erroneously added by d937411.\n> \n> d937411???\n\nWhoops. That's the version in my local repo (from which I sent a patch\nthat you applied). The version in your repo is 42dcbb7.\n\n-Peff\n"},{"id":"193991","messageId":"7vk3z1u4lw.fsf@alter.siamese.dyndns.org","threadId":"30670","inReplyTo":"20120620182855.GA26948@sigill.intra.peff.net","subject":"Re: [PATCHv2 1/8] Makefile: apply dependencies consistently to sparse/asm targets","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-20T20:10:19Z","receivedAt":"2012-06-20T20:10:19Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Here's an updated series that drops these lines and I hope will address\n> the commit message issues you brought up:\n>\n> [01/11]: Makefile: sort LIB_H list\n> [02/11]: Makefile: fold MISC_H into LIB_H\n>\n> New in this iteration to get rid of these largely pointless manual\n> dependencies.\n>\n> [03/11]: Makefile: do not have git.o depend on common-cmds.h\n>\n> New in this iteration. I noticed while double-checking that this\n> dependency is pointless.\n>\n> [04/11]: Makefile: apply dependencies consistently to sparse/asm targets\n>\n> Updated based on earlier patches, and with a new commit message\n> explaining a little more of what's going on.\n>\n> [05/11]: Makefile: do not replace @@GIT_USER_AGENT@@ in scripts\n> [06/11]: Makefile: split GIT_USER_AGENT from GIT-CFLAGS\n> [07/11]: Makefile: split prefix flags from GIT-CFLAGS\n> [08/11]: Makefile: do not replace @@GIT_VERSION@@ in shell scripts\n> [09/11]: Makefile: update scripts when build-time parameters change\n> [10/11]: Makefile: build instaweb similar to other scripts\n> [11/11]: Makefile: move GIT-VERSION-FILE dependencies closer to use\n>\n> The rest are largely the same, but with a few minor textual updates to\n> accomodate the earlier changes.\n\nLooks good modulo a minor nit. Will queue.\n\nThanks.\n"},{"id":"193996","messageId":"20120620210038.GA6142@burratino","threadId":"30670","inReplyTo":"20120620195709.GB32228@sigill.intra.peff.net","subject":"Re: [PATCHv2 1/8] Makefile: apply dependencies consistently to sparse/asm targets","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-06-20T21:00:38Z","receivedAt":"2012-06-20T21:00:38Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n> I just assumed that people who are actively hacking on individual header\n> files in git actually have a compiler that can do COMPUTE_HEADER_DEPENDENCIES.\n\nThat's probably true. And it is presumably possible to implement\nCOMPUTE_HEADER_DEPENDENCIES for Solaris cc and MSVC, so people using\nthose compilers would just have an incentive to do that sooner.\n\nSo it's not all that bad.\n\n> Maybe that is not the case. If it were such a big deal, then why is\n> everything in LIB_H? Why don't people use these manual rules, or convert\n> existing LIB_H entries to use them?\n\nOnce a header is included by cache.h (like most headers in LIB_H),\nthere is not much hope for avoiding recompilations when it changes.\n\n> For people who are not actively hacking on header files in git, the\n> arguments from that patch apply (namely that LIB_H is so gigantic that\n> you are unlikely to hit a specific change where one of the few manual\n> rules is triggered, but LIB_H is not).\n\nUnless they are bisecting, it would not be so bad for such people to\neffectively have to run \"make clean\" between compiles, as you've\nhinted. They are not the people it is possible to easily improve\nbuild performance for.\n\n>> On the other hand, if someone were proposing adding a simple awk\n>> script to implement a \"make dep\" fallback, I would understand that.\n>\n> I'd be OK with that. Do you have one in mind, or do we need to write it\n> from scratch? Surely somebody else has solved this problem before.\n\nThere are lots of \"make dep\" implementations out there, but it's hard\nto care enough to choose between them. :) No one who actually doesn't\nuse gcc has spoken up as caring. So if we're really feeling the pain\nof maintaining the detailed COMPUTE_HEADER_DEPENDENCIES=no fallback\ndependencies, let's just say so and drop them like you've suggested.\n\nBefore I thought you were saying \"nobody is going to notice\". And I'm\npretty sure that's not true. What I missed before is that a different\nstatement holds, namely \"sure, some people will notice, but they have\nan easy way to move forward and the outcome would be much better than\nthe status quo\".\n\nSorry to be so dense before.\n\nJonathan\n"},{"id":"193997","messageId":"20120620210730.GB6142@burratino","threadId":"30670","inReplyTo":"20120620183055.GB30995@sigill.intra.peff.net","subject":"Re: [PATCHv3 02/11] Makefile: fold MISC_H into LIB_H","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-06-20T21:07:30Z","receivedAt":"2012-06-20T21:07:30Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n> Let's just fold MISC_H into LIB_H and get rid of these\n> manual rules. The worst case is some extra compilation, but\n> even that is unlikely to matter due to the reasons above.\n\nShould XDIFF_H and VCSSVN_H be folded into STATIC_HEADERS, too?\n"},{"id":"193998","messageId":"20120620210926.GC6142@burratino","threadId":"30670","inReplyTo":"20120620183118.GC30995@sigill.intra.peff.net","subject":"Re: [PATCHv3 03/11] Makefile: do not have git.o depend on common-cmds.h","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-06-20T21:09:26Z","receivedAt":"2012-06-20T21:09:26Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n> This dependency has been stale since 70827b1 (Split up\n> builtin commands into separate files from git.c, 2006-04-21).\n\nGood catch, thanks.\n\nReviewed-by: Jonathan Nieder \n"},{"id":"193999","messageId":"20120620211225.GD6142@burratino","threadId":"30670","inReplyTo":"20120620183133.GD30995@sigill.intra.peff.net","subject":"Re: [PATCHv3 04/11] Makefile: apply dependencies consistently to sparse/asm targets","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-06-20T21:12:25Z","receivedAt":"2012-06-20T21:12:25Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n> These\n> flags may sometimes require extra dependencies to be added\n> (e.g., like GIT-VERSION-FILE; this is not the case for any\n> of the updated lines in this patch, but it is establishing a\n> style that will be used in later patches).\n[...] \n> This patch explicitly does not update the static header\n> dependencies used when COMPUTED_HEADER_DEPENDENCIES is off.\n\nI think you are asking the commit message to do more work than it\nneeds to, and to answer questions that no one just trying to\nunderstand the patch would ask. :)\n\nWouldn't it be simpler to put the ground rules in a comment or a\ndocument somewhere under Documentation/ where they can be easily\nfound?\n\nHope that helps,\nJonathan\n"},{"id":"194000","messageId":"20120620212135.GE6142@burratino","threadId":"30670","inReplyTo":"20120620183151.GF30995@sigill.intra.peff.net","subject":"Re: [PATCHv3 06/11] Makefile: split GIT_USER_AGENT from GIT-CFLAGS","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-06-20T21:21:35Z","receivedAt":"2012-06-20T21:21:35Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n> +\t@if test x'$(GIT_USER_AGENT_SQ)' != x\"`cat GIT-USER-AGENT 2>/dev/null`\"; then \\\n> +\t\techo >&2 \" * new user-agent flag\"; \\\n\nMicronit: the term \"new user-agent string\" would be clearer.\n\nRe the lack of _HTTP: interesting. Is the plan to use this for\nother protocols, too?\n\nLooks good.\nJonathan\n"},{"id":"194001","messageId":"20120620212833.GF6142@burratino","threadId":"30670","inReplyTo":"20120620183155.GG30995@sigill.intra.peff.net","subject":"Re: [PATCHv3 07/11] Makefile: split prefix flags from GIT-CFLAGS","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-06-20T21:28:33Z","receivedAt":"2012-06-20T21:28:33Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n> Most of the build targets do not care about the setting of\n> $prefix (or its derivative variables), but will be rebuilt\n> if the prefix changes. For most setups this doesn't matter\n> (they set prefix once and never change it), but for a setup\n> which puts each branch or version in its own prefix, this\n> unnecessarily causes a full rebuild whenever the branc is\n> changed.\n\nMicronit: s/branc/branch/.\n\n[...]\n> @@ -2385,14 +2391,22 @@ cscope:\n> \t$(FIND_SOURCE_FILES) | xargs cscope -b\n> \n> ### Detect prefix changes\n> -TRACK_CFLAGS = $(CC):$(subst ','\\'',$(ALL_CFLAGS)):\\\n> - $(bindir_SQ):$(gitexecdir_SQ):$(template_dir_SQ):$(prefix_SQ):\\\n> - $(localedir_SQ):$(USE_GETTEXT_SCHEME)\n> +TRACK_PREFIX = $(bindir_SQ):$(gitexecdir_SQ):$(template_dir_SQ):$(prefix_SQ):\\\n> +\t\t$(localedir_SQ)\n> +\n> +GIT-PREFIX: FORCE\n> +\t@FLAGS='$(TRACK_PREFIX)'; \\\n> +\tif test x\"$$FLAGS\" != x\"`cat GIT-PREFIX 2>/dev/null`\" ; then \\\n> +\t\techo 1>&2 \" * new prefix flags\"; \\\n> +\t\techo \"$$FLAGS\" >GIT-PREFIX; \\\n> +\tfi\n\nHmm, nothing to do with this particular patch but the above list\nincludes gitexecdir and not htmldir. Is there any particular logic\nbehind the list of variables?\n\nNit: I think it would be easier to understand a name like GIT-PATHS\n(making the absence of htmldir a bug, if not a particularly important\none). No other complaints. :)\n\nThanks.\nJonathan\n"},{"id":"194002","messageId":"20120620213147.GG6142@burratino","threadId":"30670","inReplyTo":"20120620183222.GK30995@sigill.intra.peff.net","subject":"Re: [PATCHv3 11/11] Makefile: move GIT-VERSION-FILE dependencies closer to use","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-06-20T21:31:47Z","receivedAt":"2012-06-20T21:31:47Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n> There is a list of all of the targets which depend on\n> GIT-VERSION-FILE, but it can be quite far from the actual\n> point where the targets actually use $(GIT_VERSION). This\n> can make it hard to verify that each use of $(GIT_VERSION)\n> has a matching dependency.\n>\n> This patch moves the dependency closer to the actual build\n> instructions, which makes verification easier. This also\n> fixes the generation of \"configure\", which did not properly\n> mark the dependency.\n\nVery nice.\n\nRegards,\nJonathan\n"},{"id":"194003","messageId":"20120620221125.GA3302@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120620210730.GB6142@burratino","subject":"Re: [PATCHv3 02/11] Makefile: fold MISC_H into LIB_H","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-20T22:11:25Z","receivedAt":"2012-06-20T22:11:25Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 20, 2012 at 04:07:30PM -0500, Jonathan Nieder wrote:\n\n> Jeff King wrote:\n> \n> > Let's just fold MISC_H into LIB_H and get rid of these\n> > manual rules. The worst case is some extra compilation, but\n> > even that is unlikely to matter due to the reasons above.\n> \n> Should XDIFF_H and VCSSVN_H be folded into STATIC_HEADERS, too?\n\nI stopped short of that, but I'd be tempted to do so. I don't think\nthose variables have any special meaning beyond the recompilation\ndependencies.\n\n-Peff\n"},{"id":"194004","messageId":"20120620221502.GB3302@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120620211225.GD6142@burratino","subject":"Re: [PATCHv3 04/11] Makefile: apply dependencies consistently to sparse/asm targets","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-20T22:15:02Z","receivedAt":"2012-06-20T22:15:02Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 20, 2012 at 04:12:25PM -0500, Jonathan Nieder wrote:\n\n> Jeff King wrote:\n> \n> > These\n> > flags may sometimes require extra dependencies to be added\n> > (e.g., like GIT-VERSION-FILE; this is not the case for any\n> > of the updated lines in this patch, but it is establishing a\n> > style that will be used in later patches).\n> [...] \n> > This patch explicitly does not update the static header\n> > dependencies used when COMPUTED_HEADER_DEPENDENCIES is off.\n> \n> I think you are asking the commit message to do more work than it\n> needs to, and to answer questions that no one just trying to\n> understand the patch would ask. :)\n\nYeah, when writing out the full discussion I was awfully tempted to go\nwith your simplified explanation. ;)\n\nIn fact, it's the later commits that really make use of this\nexplanation when they add lines.\n\n> Wouldn't it be simpler to put the ground rules in a comment or a\n> document somewhere under Documentation/ where they can be easily\n> found?\n\nI think a comment in the Makefile might make sense (especially if it\nintroduces the section as \"and this is the place to put weird\ntarget-specific cppflags and dependencies\"). Would you mind taking a\nstab at writing it? I feel like the explanation I wrote in the commit\nmessage ended up quite dense and possibly not very informative, and a\nfresh brain and fingers might turn out something a little more\nreasonable.\n\n-Peff\n"},{"id":"194005","messageId":"20120620221616.GC3302@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120620212135.GE6142@burratino","subject":"Re: [PATCHv3 06/11] Makefile: split GIT_USER_AGENT from GIT-CFLAGS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-20T22:16:16Z","receivedAt":"2012-06-20T22:16:16Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 20, 2012 at 04:21:35PM -0500, Jonathan Nieder wrote:\n\n> Jeff King wrote:\n> \n> > +\t@if test x'$(GIT_USER_AGENT_SQ)' != x\"`cat GIT-USER-AGENT 2>/dev/null`\"; then \\\n> > +\t\techo >&2 \" * new user-agent flag\"; \\\n> \n> Micronit: the term \"new user-agent string\" would be clearer.\n\nProbably. I am tempted to get rid of the informative message altogether.\nFor CFLAGS, you might be confused why everything is being rebuilt, since\nthe dependency chain is not straightforward. For this, it's kind of\nobvious.\n\n> Re the lack of _HTTP: interesting. Is the plan to use this for\n> other protocols, too?\n\nLook at the jk/version-string topic that this is based on. :)\n\n-Peff\n"},{"id":"194006","messageId":"20120620222103.GJ6142@burratino","threadId":"30670","inReplyTo":"20120620221616.GC3302@sigill.intra.peff.net","subject":"Re: [PATCHv3 06/11] Makefile: split GIT_USER_AGENT from GIT-CFLAGS","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-06-20T22:21:03Z","receivedAt":"2012-06-20T22:21:03Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n>> Jeff King wrote:\n\n>>> +\t@if test x'$(GIT_USER_AGENT_SQ)' != x\"`cat GIT-USER-AGENT 2>/dev/null`\"; then \\\n>>> +\t\techo >&2 \" * new user-agent flag\"; \\\n[...]\n> I am tempted to get rid of the informative message altogether.\n> For CFLAGS, you might be confused why everything is being rebuilt, since\n> the dependency chain is not straightforward. For this, it's kind of\n> obvious.\n\nMakes a lot of sense.\n"},{"id":"194007","messageId":"20120620222224.GD3302@sigill.intra.peff.net","threadId":"30670","inReplyTo":"20120620212833.GF6142@burratino","subject":"Re: [PATCHv3 07/11] Makefile: split prefix flags from GIT-CFLAGS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-20T22:22:24Z","receivedAt":"2012-06-20T22:22:24Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 20, 2012 at 04:28:33PM -0500, Jonathan Nieder wrote:\n\n> Jeff King wrote:\n> \n> > Most of the build targets do not care about the setting of\n> > $prefix (or its derivative variables), but will be rebuilt\n> > if the prefix changes. For most setups this doesn't matter\n> > (they set prefix once and never change it), but for a setup\n> > which puts each branch or version in its own prefix, this\n> > unnecessarily causes a full rebuild whenever the branc is\n> > changed.\n> \n> Micronit: s/branc/branch/.\n\nThe worst part is that Junio already corrected that in v1 and I didn't\npropagate it into my re-roll. \n\n> > ### Detect prefix changes\n> > -TRACK_CFLAGS = $(CC):$(subst ','\\'',$(ALL_CFLAGS)):\\\n> > - $(bindir_SQ):$(gitexecdir_SQ):$(template_dir_SQ):$(prefix_SQ):\\\n> > - $(localedir_SQ):$(USE_GETTEXT_SCHEME)\n> > +TRACK_PREFIX = $(bindir_SQ):$(gitexecdir_SQ):$(template_dir_SQ):$(prefix_SQ):\\\n> > +\t\t$(localedir_SQ)\n> > +\n> > +GIT-PREFIX: FORCE\n> > +\t@FLAGS='$(TRACK_PREFIX)'; \\\n> > +\tif test x\"$$FLAGS\" != x\"`cat GIT-PREFIX 2>/dev/null`\" ; then \\\n> > +\t\techo 1>&2 \" * new prefix flags\"; \\\n> > +\t\techo \"$$FLAGS\" >GIT-PREFIX; \\\n> > +\tfi\n> \n> Hmm, nothing to do with this particular patch but the above list\n> includes gitexecdir and not htmldir. Is there any particular logic\n> behind the list of variables?\n\nThe list came from what was in CFLAGS before. But looking at it again, I\nthink it is not right (e.g., git.o builds with $(htmldir_SQ), but nobody\nactually depends on it). IOW, it was broken before, and I have\npropagated that breakage. But nobody has noticed because they usually\nset prefix and not the htmldir separately.\n\nI probably need to go through the whole Makefile again and make sure\neverything is in the list. What a royal pain. I can't help but feel\nthat there is a better way to do this. GNU make supports user-defined\nfunctions via the \"call\" function. Maybe we can turn this into a\nsingle-line of Makefile per variable, and get per-variable resolution of\nrebuilding, and still end up with fewer lines.\n\nI'll take a look.\n\n> Nit: I think it would be easier to understand a name like GIT-PATHS\n> (making the absence of htmldir a bug, if not a particularly important\n> one). No other complaints. :)\n\nYeah, I hate the name GIT-PREFIX. But I was worried people might\nmisinterpret GIT-PATHS. Maybe these should all go into a subdir full of\nauto-built file magic, which will make it more obvious what they are\n(and we would want to do that anyway if we start having a larger number\nof them).\n\n-Peff\n"},{"id":"194010","messageId":"87ipeltwpp.fsf@thomas.inf.ethz.ch","threadId":"30670","inReplyTo":"7vk3z1u4lw.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv2 1/8] Makefile: apply dependencies consistently to sparse/asm targets","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2012-06-20T23:00:50Z","receivedAt":"2012-06-20T23:00:50Z","isPatch":false,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Junio C Hamano writes:\n\n> Jeff King writes:\n>\n>> [01/11]: Makefile: sort LIB_H list\n>> [02/11]: Makefile: fold MISC_H into LIB_H\n>> [03/11]: Makefile: do not have git.o depend on common-cmds.h\n>> [04/11]: Makefile: apply dependencies consistently to sparse/asm targets\n>> [05/11]: Makefile: do not replace @@GIT_USER_AGENT@@ in scripts\n>> [06/11]: Makefile: split GIT_USER_AGENT from GIT-CFLAGS\n>> [07/11]: Makefile: split prefix flags from GIT-CFLAGS\n>> [08/11]: Makefile: do not replace @@GIT_VERSION@@ in shell scripts\n>> [09/11]: Makefile: update scripts when build-time parameters change\n>> [10/11]: Makefile: build instaweb similar to other scripts\n>> [11/11]: Makefile: move GIT-VERSION-FILE dependencies closer to use\n>\n> Looks good modulo a minor nit. Will queue.\n\nI hate saying \"me too\", so instead I will say: I agree!\n\nAnd thanks for this massive cleanup in response to a small nit...\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"194015","messageId":"20120621051817.GA18102@sigill.intra.peff.net","threadId":"30670","inReplyTo":"7vk3z1u4lw.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv2 1/8] Makefile: apply dependencies consistently to sparse/asm targets","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-21T05:18:17Z","receivedAt":"2012-06-21T05:18:17Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 20, 2012 at 01:10:19PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > Here's an updated series that drops these lines and I hope will address\n> > the commit message issues you brought up:\n> [...]\n> \n> Looks good modulo a minor nit. Will queue.\n\nYou probably noticed a lot of nice review from Jonathan. I had hoped to\ndo another round exploring some alternatives we discussed, but didn't\nget to it tonight, and now I'm going to be away from email for about 5\ndays.\n\nI don't think there is anything _wrong_ in the series as it is, and\ncertainly the things we talked about changing could be built on top. So\nif you want to hold it in pu for a week, I can revisit it next week. But\nI'd also be just as happy to see it move to next, and revisit the topic\nlater by building on top.\n\n-Peff\n"},{"id":"194016","messageId":"7vwr31rzi4.fsf@alter.siamese.dyndns.org","threadId":"30670","inReplyTo":"20120621051817.GA18102@sigill.intra.peff.net","subject":"Re: [PATCHv2 1/8] Makefile: apply dependencies consistently to sparse/asm targets","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-21T05:43:31Z","receivedAt":"2012-06-21T05:43:31Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> You probably noticed a lot of nice review from Jonathan. I had hoped to\n> do another round exploring some alternatives we discussed, but didn't\n> get to it tonight, and now I'm going to be away from email for about 5\n> days.\n>\n> I don't think there is anything _wrong_ in the series as it is, and\n> certainly the things we talked about changing could be built on top. So\n> if you want to hold it in pu for a week, I can revisit it next week. But\n> I'd also be just as happy to see it move to next, and revisit the topic\n> later by building on top.\n\nI'd keep it in 'pu'; it may be replaced with Jonathan's reroll if it\ncomes.\n\nHave fun, whereever you are.\n"},{"id":"194032","messageId":"4FE2E0E7.3040009@gmail.com","threadId":"30670","inReplyTo":"20120620195709.GB32228@sigill.intra.peff.net","subject":"Automatic dependency tracking in the Git build system (was: Re: [PATCHv2 1/8] Makefile: apply dependencies consistently to sparse/asm targets)","fromName":"Stefano Lattarini","fromEmail":"stefano.lattarini@gmail.com","sentAt":"2012-06-21T08:52:55Z","receivedAt":"2012-06-21T08:52:55Z","isPatch":false,"sender":{"key":"stefano.lattarini@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1429199?v=4"},"body":"[Adding the Automake list in CC:]\n\nOn 06/20/2012 09:57 PM, Jeff King wrote:\n> On Wed, Jun 20, 2012 at 02:45:31PM -0500, Jonathan Nieder wrote:\n> \n>> Jeff King wrote:\n>>\n>>> Did you read the argument in patch 2? They are almost certainly not\n>>> helping anyone, anyway.\n>>\n>> Yes, I read patch 2. I hacked on git from time to time in the days\n>> before COMPUTE_HEADER_DEPENDENCIES, and it sometimes involved changing\n>> header files. When they were not in LIB_H, the experience was much\n>> nicer.\n>>\n>> Is that called \"not helping\"? I'm afraid I don't follow this line of\n>> argument at all.\n> \n> I just assumed that people who are actively hacking on individual header\n> files in git actually have a compiler that can do COMPUTE_HEADER_DEPENDENCIES.\n> Maybe that is not the case. If it were such a big deal, then why is\n> everything in LIB_H? Why don't people use these manual rules, or convert\n> existing LIB_H entries to use them?\n> \n> For people who are not actively hacking on header files in git, the\n> arguments from that patch apply (namely that LIB_H is so gigantic that\n> you are unlikely to hit a specific change where one of the few manual\n> rules is triggered, but LIB_H is not).\n> \n>> On the other hand, if someone were proposing adding a simple awk\n>> script to implement a \"make dep\" fallback, I would understand that.\n> \n> I'd be OK with that. Do you have one in mind, or do we need to write it\n> from scratch? Surely somebody else has solved this problem before.\n>\n\n[begin shameless plug]\n\nHave you taken a look at the 'depcomp' script that comes with Automake?\n \nOnce you get past some of its idiosyncrasies and few historical warts,\nit has a lot of built-in knowledge about automatic dependency tracking\nfor a lot of different compilers.\n\n[end shameless plug]\n\nHTH,\n Stefano\n"},{"id":"194761","messageId":"20120706224718.GB3728@burratino","threadId":"30670","inReplyTo":"20120620183055.GB30995@sigill.intra.peff.net","subject":"Re: [PATCHv3 02/11] Makefile: fold MISC_H into LIB_H","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-07-06T22:47:18Z","receivedAt":"2012-07-06T22:47:18Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nI finally found some moments to revisit this series, so I'm starting\nhere. I think the justification for this patch is something like\nthis:\n\n Keeping track of what files include each header is an error-prone\n chore. On top of that, for l10n, these days we have to keep a master\n list of all headers, too, which is double work when adding a new\n header that adds insult to injury.\n\n Active Makefile hackers tend to use compilers like gcc that support\n automatic dependency generation with -MMD. The precise header deps\n aren't even used when building with these compilers, so the people\n maintaining the precise header deps do not benefit from them at all.\n Unfair!\n\n Non-developers who can't fend for themselves do not rebuild after a\n small header change very often, so they would not be hurt much by a\n \"change one header, rebuild everything\" rule when automatic\n dependency generation is disabled, either.\n\n That leaves at least one important category of people to be hurt by\n this change: the glorious MSVC hackers. MSVC supports the\n appropriate magic to compute header dependencies, but no one's\n gotten around to teaching the Makefile to use it yet. So let's stop\n delaying the inevitable and drop the detailed dependencies. If\n anyone complains then we can work with them to finish support for\n computing header dependencies for the relevant compiler.\n\nFair enough. \n\nTwo details puzzle me:\n\nJeff King wrote:\n\n> The original point\n> of LIB_H was that it would force recompilation of C files\n> when any of the library headers changed.\n\nLIB_H was introduced by commit e590d694 (Add more header dependencies,\n2005-04-18). It only lists\n\n\tcache.h\n\tobject.h\n\neven though some translation units included tree.h, commit.h, or\nblob.h already. So at least back then, it seems to have been about\nlibrary headers and not about all headers (and \"all headers\" was\npuzzlingly not worth worrying about at all).\n\nSo isn't this a fundamentally new thing, rather than a return to the\nstate of nature?\n\nThe other remaining question is why we don't use something like\n$(wildcard *.h) and avoid listing individual headers altogether.\nIs the fear that some stray non-git header will find its way into\nthe cwd and poison the translation files? (If so, I'd like to\ndocument that as well to help readers understand why we keep doing\nthe work we do.)\n\nCiao,\nJonathan\n"},{"id":"194763","messageId":"20120707033918.GB3574@burratino","threadId":"30670","inReplyTo":"20120620221125.GA3302@sigill.intra.peff.net","subject":"[PATCH 02.5/11] Makefile: fold XDIFF_H and VCSSVN_H into LIB_H","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-07-07T03:39:18Z","receivedAt":"2012-07-07T03:39:18Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Just like MISC_H (see previous commit), there is no reason to track\nxdiff and vcs-svn headers separately from the rest of the headers.\nThe only purpose of these variables is to keep track of recompilation\ndependencies.\n\nAs a pleasant side effect, folding these into LIB_H lets us stop\ntracking GIT_OBJS and VCSSVN_TEST_OBJS separately from the list of all\nOBJECTS.\n\nSigned-off-by: Jonathan Nieder \n---\nJeff King wrote:\n> On Wed, Jun 20, 2012 at 04:07:30PM -0500, Jonathan Nieder wrote:\n\n>> Should XDIFF_H and VCSSVN_H be folded into STATIC_HEADERS, too?\n>\n> I stopped short of that, but I'd be tempted to do so.\n\nHere goes.\n\n Makefile | 60 ++++++++++++++++++++++++------------------------------------\n 1 file changed, 24 insertions(+), 36 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 500966b1..b24ca20d 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -392,11 +392,8 @@ BUILTIN_OBJS =\n BUILT_INS =\n COMPAT_CFLAGS =\n COMPAT_OBJS =\n-XDIFF_H =\n XDIFF_OBJS =\n-VCSSVN_H =\n VCSSVN_OBJS =\n-VCSSVN_TEST_OBJS =\n GENERATED_H =\n EXTRA_CPPFLAGS =\n LIB_H =\n@@ -558,21 +555,21 @@ LIB_FILE=libgit.a\n XDIFF_LIB=xdiff/lib.a\n VCSSVN_LIB=vcs-svn/lib.a\n \n-XDIFF_H += xdiff/xinclude.h\n-XDIFF_H += xdiff/xmacros.h\n-XDIFF_H += xdiff/xdiff.h\n-XDIFF_H += xdiff/xtypes.h\n-XDIFF_H += xdiff/xutils.h\n-XDIFF_H += xdiff/xprepare.h\n-XDIFF_H += xdiff/xdiffi.h\n-XDIFF_H += xdiff/xemit.h\n+LIB_H += xdiff/xinclude.h\n+LIB_H += xdiff/xmacros.h\n+LIB_H += xdiff/xdiff.h\n+LIB_H += xdiff/xtypes.h\n+LIB_H += xdiff/xutils.h\n+LIB_H += xdiff/xprepare.h\n+LIB_H += xdiff/xdiffi.h\n+LIB_H += xdiff/xemit.h\n \n-VCSSVN_H += vcs-svn/line_buffer.h\n-VCSSVN_H += vcs-svn/sliding_window.h\n-VCSSVN_H += vcs-svn/repo_tree.h\n-VCSSVN_H += vcs-svn/fast_export.h\n-VCSSVN_H += vcs-svn/svndiff.h\n-VCSSVN_H += vcs-svn/svndump.h\n+LIB_H += vcs-svn/line_buffer.h\n+LIB_H += vcs-svn/sliding_window.h\n+LIB_H += vcs-svn/repo_tree.h\n+LIB_H += vcs-svn/fast_export.h\n+LIB_H += vcs-svn/svndiff.h\n+LIB_H += vcs-svn/svndump.h\n \n GENERATED_H += common-cmds.h\n \n@@ -2110,13 +2107,6 @@ version.o git.spec \\\n \t$(patsubst %.perl,%,$(SCRIPT_PERL)) \\\n \t: GIT-VERSION-FILE\n \n-TEST_OBJS := $(patsubst test-%$X,test-%.o,$(TEST_PROGRAMS))\n-GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \\\n-\tgit.o\n-ifndef NO_CURL\n-\tGIT_OBJS += http.o http-walker.o remote-curl.o\n-endif\n-\n XDIFF_OBJS += xdiff/xdiffi.o\n XDIFF_OBJS += xdiff/xprepare.o\n XDIFF_OBJS += xdiff/xutils.o\n@@ -2132,9 +2122,14 @@ VCSSVN_OBJS += vcs-svn/fast_export.o\n VCSSVN_OBJS += vcs-svn/svndiff.o\n VCSSVN_OBJS += vcs-svn/svndump.o\n \n-VCSSVN_TEST_OBJS += test-line-buffer.o\n-\n-OBJECTS := $(GIT_OBJS) $(XDIFF_OBJS) $(VCSSVN_OBJS)\n+TEST_OBJS := $(patsubst test-%$X,test-%.o,$(TEST_PROGRAMS))\n+OBJECTS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \\\n+\t$(XDIFF_OBJS) \\\n+\t$(VCSSVN_OBJS) \\\n+\tgit.o\n+ifndef NO_CURL\n+\tOBJECTS += http.o http-walker.o remote-curl.o\n+endif\n \n dep_files := $(foreach f,$(OBJECTS),$(dir $f).depend/$(notdir $f).d)\n dep_dirs := $(addsuffix .depend,$(sort $(dir $(OBJECTS))))\n@@ -2233,15 +2228,8 @@ else\n # Dependencies on automatically generated headers such as common-cmds.h\n # should _not_ be included here, since they are necessary even when\n # building an object for the first time.\n-#\n-# XXX. Please check occasionally that these include all dependencies\n-# gcc detects!\n \n-$(GIT_OBJS): $(LIB_H)\n-\n-xdiff-interface.o $(XDIFF_OBJS): $(XDIFF_H)\n-\n-$(VCSSVN_OBJS) $(VCSSVN_TEST_OBJS): $(LIB_H) $(VCSSVN_H)\n+$(OBJECTS): $(LIB_H)\n endif\n \n exec_cmd.sp exec_cmd.s exec_cmd.o: EXTRA_CPPFLAGS = \\\n@@ -2334,7 +2322,7 @@ XGETTEXT_FLAGS_C = $(XGETTEXT_FLAGS) --language=C \\\n \t--keyword=_ --keyword=N_ --keyword=\"Q_:1,2\"\n XGETTEXT_FLAGS_SH = $(XGETTEXT_FLAGS) --language=Shell\n XGETTEXT_FLAGS_PERL = $(XGETTEXT_FLAGS) --keyword=__ --language=Perl\n-LOCALIZED_C := $(C_OBJ:o=c) $(LIB_H) $(XDIFF_H) $(VCSSVN_H) $(GENERATED_H)\n+LOCALIZED_C := $(C_OBJ:o=c) $(LIB_H) $(GENERATED_H)\n LOCALIZED_SH := $(SCRIPT_SH)\n LOCALIZED_PERL := $(SCRIPT_PERL)\n \n-- \n1.7.10.4\n"},{"id":"194764","messageId":"20120707041909.GC3574@burratino","threadId":"30670","inReplyTo":"20120620221502.GB3302@sigill.intra.peff.net","subject":"[PATCH/RFC] Makefile: document ground rules for target-specific dependencies","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-07-07T04:19:09Z","receivedAt":"2012-07-07T04:19:09Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"When a source file makes use of a makefile variable, there should be a\ncorresponding dependency on a file that changes when that variable\nchanges to ensure the build output is not left stale when the variable\nchanges.\n\nDocument this, even though we are not following the rule perfectly\nyet. Based on an explanation from Jeff King.\n\nSigned-off-by: Jonathan Nieder \n---\nJeff King wrote:\n> On Wed, Jun 20, 2012 at 04:12:25PM -0500, Jonathan Nieder wrote:\n\n>> Wouldn't it be simpler to put the ground rules in a comment or a\n>> document somewhere under Documentation/ where they can be easily\n>> found?\n>\n> I think a comment in the Makefile might make sense (especially if it\n> introduces the section as \"and this is the place to put weird\n> target-specific cppflags and dependencies\").\n\nHow about something like this?\n\n Makefile | 34 ++++++++++++++++++++++++++++++++++\n 1 file changed, 34 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex 3f82b51b..542856f0 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1970,6 +1970,40 @@ shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell\n strip: $(PROGRAMS) git$X\n \t$(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X\n \n+\n+### Target-specific flags and dependencies\n+\n+# The generic compilation pattern rule and automatically\n+# computed header dependencies (falling back to a dependency on\n+# LIB_H) are enough to describe how most targets should be built,\n+# but some targets are special enough to need something a little\n+# different.\n+#\n+# - When a source file \"foo.c\" #includes a generated header file,\n+# we need to list that dependency for the \"foo.o\" target.\n+#\n+# We also list it from other targets that are built from foo.c\n+# like \"foo.sp\" and \"foo.s\", even though that is easy to forget\n+# to do because the generated header is already present around\n+# after a regular build attempt.\n+#\n+# - Some code depends on configuration kept in makefile\n+# variables. The target-specific variable EXTRA_CPPFLAGS can\n+# be used to convey that information to the C preprocessor\n+# using -D options.\n+#\n+# The \"foo.o\" target should have a corresponding dependency on\n+# a file that changes when the value of the makefile variable\n+# changes. For example, targets making use of the\n+# $(GIT_VERSION) variable depend on GIT-VERSION-FILE.\n+#\n+# Technically the \".sp\" and \".s\" targets do not need this\n+# dependency because they are force-built, but they get the\n+# same dependency for consistency. This way, you do not have to\n+# know how each target is implemented. And it means the\n+# dependencies here will not need to change if the force-build\n+# details change some day.\n+\n git.sp git.s git.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_HTML_PATH=\"$(htmldir_SQ)\"' \\\n \t'-DGIT_MAN_PATH=\"$(mandir_SQ)\"' \\\n-- \n1.7.10.4\n"},{"id":"194765","messageId":"20120707044211.GD3574@burratino","threadId":"30670","inReplyTo":"20120620221616.GC3302@sigill.intra.peff.net","subject":"[RFC/PATCH v4 06/11] Makefile: split GIT_USER_AGENT from GIT-CFLAGS","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-07-07T04:42:11Z","receivedAt":"2012-07-07T04:42:11Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"The default user-agent depends on the GIT_VERSION, which means that\nanytime you switch versions, it causes a full rebuild. Instead, let's\nsplit it out into its own file and restrict the dependency to\nversion.o.\n\nTo avoid noise during builds, unlike the GIT-CFLAGS rule which prints\n\"* new build flags or prefix\" so the operator knows why all files are\nbeing rebuilt when it changes, GIT-USER-AGENT generation is silent.\n\nIf this code breaks and a target depending on GIT-USER-AGENT ends up\nbeing rebuilt when it shouldn't be, the full dependency chain can be\nretrieved with \"make --debug=b\".\n\nSigned-off-by: Jeff King \nSigned-off-by: Jonathan Nieder \n---\nJeff King wrote:\n\n> I am tempted to get rid of the informative message altogether.\n\nLike this?\n\n .gitignore | 1 +\n Makefile | 10 ++++++++--\n 2 files changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/.gitignore b/.gitignore\nindex bf66648e..7329cfe5 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -2,6 +2,7 @@\n /GIT-CFLAGS\n /GIT-LDFLAGS\n /GIT-GUI-VARS\n+/GIT-USER-AGENT\n /GIT-VERSION-FILE\n /bin-wrappers/\n /git\ndiff --git a/Makefile b/Makefile\nindex 7148cadd..2a84cd8b 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1922,7 +1922,10 @@ endif\n GIT_USER_AGENT_SQ = $(subst ','\\'',$(GIT_USER_AGENT))\n GIT_USER_AGENT_CQ = \"$(subst \",\\\",$(subst \\,\\\\,$(GIT_USER_AGENT)))\"\n GIT_USER_AGENT_CQ_SQ = $(subst ','\\'',$(GIT_USER_AGENT_CQ))\n-BASIC_CFLAGS += -DGIT_USER_AGENT='$(GIT_USER_AGENT_CQ_SQ)'\n+GIT-USER-AGENT: FORCE\n+\t@if test x'$(GIT_USER_AGENT_SQ)' != x\"`cat GIT-USER-AGENT 2>/dev/null`\"; then \\\n+\t\techo '$(GIT_USER_AGENT_SQ)' >GIT-USER-AGENT; \\\n+\tfi\n \n ALL_CFLAGS += $(BASIC_CFLAGS)\n ALL_LDFLAGS += $(BASIC_LDFLAGS)\n@@ -2021,8 +2024,10 @@ builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_MAN_PATH=\"$(mandir_SQ)\"' \\\n \t'-DGIT_INFO_PATH=\"$(infodir_SQ)\"'\n \n+version.sp version.s version.o: GIT-USER-AGENT\n version.sp version.s version.o: EXTRA_CPPFLAGS = \\\n-\t'-DGIT_VERSION=\"$(GIT_VERSION)\"'\n+\t'-DGIT_VERSION=\"$(GIT_VERSION)\"' \\\n+\t'-DGIT_USER_AGENT=$(GIT_USER_AGENT_CQ_SQ)'\n \n $(BUILT_INS): git$X\n \t$(QUIET_BUILT_IN)$(RM) $@ && \\\n@@ -2744,6 +2749,7 @@ ifndef NO_TCLTK\n \t$(MAKE) -C git-gui clean\n endif\n \t$(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-GUI-VARS GIT-BUILD-OPTIONS\n+\t$(RM) GIT-USER-AGENT\n \n .PHONY: all install profile-clean clean strip\n .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell\n-- \n1.7.10.4\n"},{"id":"194814","messageId":"7vfw91kmj6.fsf@alter.siamese.dyndns.org","threadId":"30670","inReplyTo":"20120707033918.GB3574@burratino","subject":"Re: [PATCH 02.5/11] Makefile: fold XDIFF_H and VCSSVN_H into LIB_H","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-09T14:59:57Z","receivedAt":"2012-07-09T14:59:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Just like MISC_H (see previous commit), there is no reason to track\n> xdiff and vcs-svn headers separately from the rest of the headers.\n> The only purpose of these variables is to keep track of recompilation\n> dependencies.\n>\n> As a pleasant side effect, folding these into LIB_H lets us stop\n> tracking GIT_OBJS and VCSSVN_TEST_OBJS separately from the list of all\n> OBJECTS.\n>\n> Signed-off-by: Jonathan Nieder \n> ---\n> Jeff King wrote:\n>> On Wed, Jun 20, 2012 at 04:07:30PM -0500, Jonathan Nieder wrote:\n>\n>>> Should XDIFF_H and VCSSVN_H be folded into STATIC_HEADERS, too?\n>>\n>> I stopped short of that, but I'd be tempted to do so.\n>\n> Here goes.\n\nVery tempting. Will try squeezing this in between 2/11 and 3/11 and\nsee how the result looks.\n"}]}