{"thread":{"id":"39501","subject":"[PATCH 1/2] config.mak.uname: Darwin: define HAVE_GETDELIM for modern OS X releases","startedAt":"2015-06-02T18:18:56Z","lastAt":"2015-06-02T20:17:38Z","messageCount":9,"participants":["Eric Sunshine","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"262757","messageId":"1433269138-10890-1-git-send-email-sunshine@sunshineco.com","threadId":"39501","inReplyTo":null,"subject":"[PATCH 0/2] auto-detect getdelim()","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-06-02T18:18:56Z","receivedAt":"2015-06-02T18:18:56Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"As an optimization, strbuf takes advantage of getdelim() when available\n(HAVE_GETDELIM). Currently, HAVE_GETDELIM is defined automatically only\nfor Linux. This patch series updates config.mak.uname to define\nHAVE_GETDELIM on Mac OS X (Darwin) based upon version (\"uname -r\"), and\nmore generally via a configure script check.\n\nEric Sunshine (2):\n  config.mak.uname: Darwin: define HAVE_GETDELIM for modern OS X\n    releases\n  configure: add getdelim() check\n\n config.mak.uname | 3 +++\n configure.ac     | 6 ++++++\n 2 files changed, 9 insertions(+)\n\n-- \n2.4.2.598.gb4379f4\n"},{"id":"262756","messageId":"1433269138-10890-2-git-send-email-sunshine@sunshineco.com","threadId":"39501","inReplyTo":"1433269138-10890-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH 1/2] config.mak.uname: Darwin: define HAVE_GETDELIM for modern OS X releases","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-06-02T18:18:57Z","receivedAt":"2015-06-02T18:18:57Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mac OS X, getdelim() first became available with Xcode 4.1[1], which\nwas released the same day as OS X 10.7 \"Lion\", so assume getdelim()\navailability from 10.7 onward. (As of this writing, OS X is at 10.10\n\"Yosemite\".)\n\nAccording to Wikipedia[2], 4.1 was also available for download by paying\ndevelopers on OS X 10.6 \"Snow Leopard\", so it's possible that some 10.6\nmachines may have getdelim(). However, as strbuf's use of getdelim() is\npurely an optimization, let's be conservative and assume 10.6 and\nearlier lack getdelim().\n\n[1]: Or, possibly with Xcode 4.0, but that version is no longer\n     available for download, or not available to non-paying developers,\n     so testing is not possible.\n\n[2]: http://en.wikipedia.org/wiki/Xcode\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n\nTested on OS X 10.10.3 \"Yosemite\" with Xcode 6.3.2 and OS X 10.5.8\n\"Leopard\" with Xcode 3.1.\n\nThe use of 'expr' in this new test is decidedly different from existing\ninstances which merely check if `uname -R` matches a particular single\ndigit and a period. If the new test took the same approach, it would\nhave to match either one digit (in a particular range) plus a period, or\ntwo digits with the first being \"1\", plus a period. The resulting 'expr'\nexpression quickly becomes ugly and quite difficult to decipher. Hence,\nthe new test instead takes advantage of expr's relational operator '>='\nto keep things simple and make the test easy to understand at a glance\n(\"if version >= 11\" where 11 is the Darwin major version number of OS X\n10.7).\n\n config.mak.uname | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/config.mak.uname b/config.mak.uname\nindex d26665f..46a415c 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -102,6 +102,9 @@ ifeq ($(uname_S),Darwin)\n \tifeq ($(shell expr \"$(uname_R)\" : '[15]\\.'),2)\n \t\tNO_STRLCPY = YesPlease\n \tendif\n+\tifeq ($(shell expr $(shell expr \"$(uname_R)\" : '\\([0-9][0-9]*\\)\\.') '>=' 11),1)\n+\t\tHAVE_GETDELIM = YesPlease\n+\tendif\n \tNO_MEMMEM = YesPlease\n \tUSE_ST_TIMESPEC = YesPlease\n \tHAVE_DEV_TTY = YesPlease\n-- \n2.4.2.598.gb4379f4\n"},{"id":"262758","messageId":"1433269138-10890-3-git-send-email-sunshine@sunshineco.com","threadId":"39501","inReplyTo":"1433269138-10890-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH 2/2] configure: add getdelim() check","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-06-02T18:18:58Z","receivedAt":"2015-06-02T18:18:58Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"As an optimization, strbuf will take advantage of getdelim() if\navailable, so add a configure check which defines HAVE_GETDELIM if\nfound.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n\nTested on:\n* OS X 10.10.3 \"Yosemite\" with Xcode 6.3.2\n* OS X 10.5.8 \"Leopard\" with Xcode 3.1\n* Linux\n* FreeBSD\n\n configure.ac | 6 ++++++\n 1 file changed, 6 insertions(+)\n\ndiff --git a/configure.ac b/configure.ac\nindex bbdde85..14012fa 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -1041,6 +1041,12 @@ GIT_CHECK_FUNC(initgroups,\n [NO_INITGROUPS=YesPlease])\n GIT_CONF_SUBST([NO_INITGROUPS])\n #\n+# Define HAVE_GETDELIM if you have getdelim in the C library.\n+GIT_CHECK_FUNC(getdelim,\n+[HAVE_GETDELIM=YesPlease],\n+[HAVE_GETDELIM=])\n+GIT_CONF_SUBST([HAVE_GETDELIM])\n+#\n #\n # Define NO_MMAP if you want to avoid mmap.\n #\n-- \n2.4.2.598.gb4379f4\n"},{"id":"262761","messageId":"20150602184413.GA11437@peff.net","threadId":"39501","inReplyTo":"1433269138-10890-2-git-send-email-sunshine@sunshineco.com","subject":"Re: [PATCH 1/2] config.mak.uname: Darwin: define HAVE_GETDELIM for modern OS X releases","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-06-02T18:44:13Z","receivedAt":"2015-06-02T18:44:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 02, 2015 at 02:18:57PM -0400, Eric Sunshine wrote:\n\n> The use of 'expr' in this new test is decidedly different from existing\n> instances which merely check if `uname -R` matches a particular single\n> digit and a period. If the new test took the same approach, it would\n> have to match either one digit (in a particular range) plus a period, or\n> two digits with the first being \"1\", plus a period. The resulting 'expr'\n> expression quickly becomes ugly and quite difficult to decipher. Hence,\n> the new test instead takes advantage of expr's relational operator '>='\n> to keep things simple and make the test easy to understand at a glance\n> (\"if version >= 11\" where 11 is the Darwin major version number of OS X\n> 10.7).\n\nI think that is OK with respect to portability; we are already inside a\n$(uname_S) check, so we know we are on some form of Mac OS. But...\n\n> +\tifeq ($(shell expr $(shell expr \"$(uname_R)\" : '\\([0-9][0-9]*\\)\\.') '>=' 11),1)\n\nDo you need to spawn two shells? It seems like:\n\n  $(shell expr `expr \"$(uname_R)\" : '\\([0-9][0-9]*\\)'` '>=' 11),1)\n\nshould do the same thing.\n\n-Peff\n"},{"id":"262762","messageId":"20150602184519.GB11437@peff.net","threadId":"39501","inReplyTo":"1433269138-10890-3-git-send-email-sunshine@sunshineco.com","subject":"Re: [PATCH 2/2] configure: add getdelim() check","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-06-02T18:45:20Z","receivedAt":"2015-06-02T18:45:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 02, 2015 at 02:18:58PM -0400, Eric Sunshine wrote:\n\n> As an optimization, strbuf will take advantage of getdelim() if\n> available, so add a configure check which defines HAVE_GETDELIM if\n> found.\n\nThanks, looks good.\n\n-Peff\n"},{"id":"262767","messageId":"20150602190403.GA12660@peff.net","threadId":"39501","inReplyTo":"20150602184413.GA11437@peff.net","subject":"Re: [PATCH 1/2] config.mak.uname: Darwin: define HAVE_GETDELIM for modern OS X releases","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-06-02T19:04:03Z","receivedAt":"2015-06-02T19:04:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 02, 2015 at 02:44:13PM -0400, Jeff King wrote:\n\n> > +\tifeq ($(shell expr $(shell expr \"$(uname_R)\" : '\\([0-9][0-9]*\\)\\.') '>=' 11),1)\n> \n> Do you need to spawn two shells? It seems like:\n> \n>   $(shell expr `expr \"$(uname_R)\" : '\\([0-9][0-9]*\\)'` '>=' 11),1)\n\nOops, I missed the trailing '.' in the regex there, and it probably\nneeds double-quotes in case the inner expr fails to match anything.\n\nWe could also use \"test -gt\" instead of the outer expr, which is more\nidiomatic shell. But it reports via exit code, so you'd need \"&& echo 1\"\nat the end.\n\n-Peff\n"},{"id":"262770","messageId":"CAPig+cQLSF4=-4b1qOY4ChBj00=trvAO0qFEh0kOjLonJL4MBg@mail.gmail.com","threadId":"39501","inReplyTo":"20150602190403.GA12660@peff.net","subject":"Re: [PATCH 1/2] config.mak.uname: Darwin: define HAVE_GETDELIM for modern OS X releases","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-06-02T19:57:44Z","receivedAt":"2015-06-02T19:57:44Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Jun 2, 2015 at 3:04 PM, Jeff King <peff@peff.net> wrote:\n> On Tue, Jun 02, 2015 at 02:44:13PM -0400, Jeff King wrote:\n>> > +   ifeq ($(shell expr $(shell expr \"$(uname_R)\" : '\\([0-9][0-9]*\\)\\.') '>=' 11),1)\n>>\n>> Do you need to spawn two shells? It seems like:\n>>\n>>   $(shell expr `expr \"$(uname_R)\" : '\\([0-9][0-9]*\\)'` '>=' 11),1)\n\nI considered that and waffled on it. Either approach uses an extra\nprocess, but I suppose `...` would likely be less expensive since it's\njust forking the shell rather than exec()ing a new one.\n\n> Oops, I missed the trailing '.' in the regex there, and it probably\n> needs double-quotes in case the inner expr fails to match anything.\n\nWhich is messy considering the double quotes already surrounding\n$(uname_R). Suggestions?\n\n> We could also use \"test -gt\" instead of the outer expr, which is more\n> idiomatic shell. But it reports via exit code, so you'd need \"&& echo 1\"\n> at the end.\n\nYes, I messed around with that as well but didn't want to stray too\nfar from existing practice.\n\nI suppose the combination of `...` with built-in 'test' and built-in\n'echo' would be the most efficient choice. Do you want it re-rolled?\n"},{"id":"262771","messageId":"20150602200134.GA13382@peff.net","threadId":"39501","inReplyTo":"CAPig+cQLSF4=-4b1qOY4ChBj00=trvAO0qFEh0kOjLonJL4MBg@mail.gmail.com","subject":"Re: [PATCH 1/2] config.mak.uname: Darwin: define HAVE_GETDELIM for modern OS X releases","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-06-02T20:01:34Z","receivedAt":"2015-06-02T20:01:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 02, 2015 at 03:57:44PM -0400, Eric Sunshine wrote:\n\n> > Oops, I missed the trailing '.' in the regex there, and it probably\n> > needs double-quotes in case the inner expr fails to match anything.\n> \n> Which is messy considering the double quotes already surrounding\n> $(uname_R). Suggestions?\n\nThe shell should do the right thing with nested quotes inside backticks.\nSo just (untested):\n\n  $(shell expr \"`expr \"$(uname_R)\" : '\\([0-9][0-9]*\\.\\)'`\" '>=' 11),1)\n\n> I suppose the combination of `...` with built-in 'test' and built-in\n> 'echo' would be the most efficient choice. Do you want it re-rolled?\n\nI can live with it either way. It's all pretty horrible and ugly; the\nsaving grace is that we hopefully never have to touch that line again.\n;)\n\n-Peff\n"},{"id":"262774","messageId":"CAPig+cQsrxgMcK0+1jPLY8YXtNF4y17ivTUrFE8L8RS3MeffgA@mail.gmail.com","threadId":"39501","inReplyTo":"20150602200134.GA13382@peff.net","subject":"Re: [PATCH 1/2] config.mak.uname: Darwin: define HAVE_GETDELIM for modern OS X releases","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-06-02T20:17:38Z","receivedAt":"2015-06-02T20:17:38Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Jun 2, 2015 at 4:01 PM, Jeff King <peff@peff.net> wrote:\n> On Tue, Jun 02, 2015 at 03:57:44PM -0400, Eric Sunshine wrote:\n>\n>> > Oops, I missed the trailing '.' in the regex there, and it probably\n>> > needs double-quotes in case the inner expr fails to match anything.\n>>\n>> Which is messy considering the double quotes already surrounding\n>> $(uname_R). Suggestions?\n>\n> The shell should do the right thing with nested quotes inside backticks.\n> So just (untested):\n>\n>   $(shell expr \"`expr \"$(uname_R)\" : '\\([0-9][0-9]*\\.\\)'`\" '>=' 11),1)\n\nRight. Temporary brain derailment on my part.\n\n>> I suppose the combination of `...` with built-in 'test' and built-in\n>> 'echo' would be the most efficient choice. Do you want it re-rolled?\n>\n> I can live with it either way. It's all pretty horrible and ugly; the\n> saving grace is that we hopefully never have to touch that line again.\n\nI'll re-roll, taking advantage of `...` and (typically, builtin)\n'test' and 'echo'. This:\n\n$(shell test \"`expr \"$(uname_R)\" : '\\([0-9][0-9]*\\)\\.'`\" -ge 11 && echo 1)\n\nAlready tested on OS X 10.10 (Yosemite) and 10.5 (Leopard).\n"}]}