{"thread":{"id":"46424","subject":"Handling of paths","startedAt":"2017-07-19T16:49:17Z","lastAt":"2017-07-24T16:52:29Z","messageCount":8,"participants":["Victor Toni","Junio C Hamano","Charles Bailey","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"324768","messageId":"CAG0OSgdEE3g-ugEJU4EZqfbxZ=3h2WPdLC4W4mG7b6UeTaXQ-Q@mail.gmail.com","threadId":"46424","inReplyTo":null,"subject":"Handling of paths","fromName":"Victor Toni","fromEmail":"victor.toni@gmail.com","sentAt":"2017-07-19T16:48:41Z","receivedAt":"2017-07-19T16:49:17Z","isPatch":false,"sender":{"key":"victor.toni@gmail.com","avatar":null},"body":"Hello,\n\nI have a .gitconfig in which I try to separate work and private stuff\nby using includes which works great.\n\nWhen using [include] the path is treated either\n- relative to the including file (if the path itself relative)\n- relative to the home directory if it starts with ~\n- absolute if the path is absolute\n\nThis is fine and expected.\n\nWhat's unexpected is that paths used for sslKey or sslCert are treated\ndifferently insofar as they are expected to be absolute.\nRelative paths (whether with or without \"~\") don't work.\n\nIt would't be an issue to use absoulte paths if I wouldn't use the\nsame config for Linux and Windows and each OS has its own semantic\nwhere it $HOME ishould be.\n\nTo avoid double configurations I tried to use the same directory\nstructure within my $HOME for both OS.\n\nThis approach fails since paths other than for [include] seem to have\nto be absolute which seems like a bug to me.\n\nDo you have any suggestions how I could make this work?\n\nThank you,\nVictor\n"},{"id":"324827","messageId":"xmqq7ez2lwsv.fsf@gitster.mtv.corp.google.com","threadId":"46424","inReplyTo":"CAG0OSgdEE3g-ugEJU4EZqfbxZ=3h2WPdLC4W4mG7b6UeTaXQ-Q@mail.gmail.com","subject":"Re: Handling of paths","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-20T19:42:40Z","receivedAt":"2017-07-20T19:42:51Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Victor Toni writes:\n\n> What's unexpected is that paths used for sslKey or sslCert are treated\n> differently insofar as they are expected to be absolute.\n> Relative paths (whether with or without \"~\") don't work.\n\nLooking at http.c::http_options(), I see that \"sslcapath\" and\n\"sslcainfo\" do use git_config_pathname() when grabbing their values,\nbut \"sslcert\" and \"sslkey\" treat the value as a plain vanilla string\nwithout expecting \"~[username]/\" at all.\n\nThe modern http.c codestructure was introduced at 29508e1e (\"Isolate\nshared HTTP request functionality\", 2005-11-18) and was corrected\nfor interation between the multiple configuration files in 7059cd99\n(\"http_init(): Fix config file parsing\", 2009-03-09), but back in\nthese versions, all of them including \"sslcapath\" and \"sslcainfo\"\nwere all treated as plain vanilla strings.\n\nIt appears that only two of these among four were made aware of the\n\"~[username]/\" prefix in bf9acba2 (\"http: treat config options\nsslCAPath and sslCAInfo as paths\", 2015-11-23), but \"sslkey\" and\n\"sslcert\" were still left as plain vanilla strings. I do not know\nif that was an elaborate omission, or a mere oversight, as it seems\nthat it happened while I was away, so...\n\n"},{"id":"324831","messageId":"20170720200523.GA13792@hashpling.org","threadId":"46424","inReplyTo":"xmqq7ez2lwsv.fsf@gitster.mtv.corp.google.com","subject":"Re: Handling of paths","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2017-07-20T20:05:23Z","receivedAt":"2017-07-20T20:13:06Z","isPatch":false,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"On Thu, Jul 20, 2017 at 12:42:40PM -0700, Junio C Hamano wrote:\n> Victor Toni writes:\n> \n> > What's unexpected is that paths used for sslKey or sslCert are treated\n> > differently insofar as they are expected to be absolute.\n> > Relative paths (whether with or without \"~\") don't work.\n> \n> It appears that only two of these among four were made aware of the\n> \"~[username]/\" prefix in bf9acba2 (\"http: treat config options\n> sslCAPath and sslCAInfo as paths\", 2015-11-23), but \"sslkey\" and\n> \"sslcert\" were still left as plain vanilla strings. I do not know\n> if that was an elaborate omission, or a mere oversight, as it seems\n> that it happened while I was away, so...\n\nIt was more of an oversight than a deliberate omission, but more\naccurately I didn't actively consider whether the other http.ssl*\nvariables were pathname-like or not.\n\nAt the time I was trying to make a config which needed to set\nhttp.sslCAPath and/or http.sslCAInfo more portable between users and\nthese were \"obviously\" pathname-like to me. Now that I read\nthe help for http.sslCert and http.sslKey, I see no reason that they\nshouldn't also use git_config_pathname. If I'd been more thorough I\nwould have proposed this at the time.\n\nCharles.\n"},{"id":"324833","messageId":"xmqqwp72kg03.fsf@gitster.mtv.corp.google.com","threadId":"46424","inReplyTo":"20170720200523.GA13792@hashpling.org","subject":"Re: Handling of paths","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-20T20:30:52Z","receivedAt":"2017-07-20T20:31:07Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Charles Bailey writes:\n\n> On Thu, Jul 20, 2017 at 12:42:40PM -0700, Junio C Hamano wrote:\n>> Victor Toni writes:\n>> \n>> > What's unexpected is that paths used for sslKey or sslCert are treated\n>> > differently insofar as they are expected to be absolute.\n>> > Relative paths (whether with or without \"~\") don't work.\n>> \n>> It appears that only two of these among four were made aware of the\n>> \"~[username]/\" prefix in bf9acba2 (\"http: treat config options\n>> sslCAPath and sslCAInfo as paths\", 2015-11-23), but \"sslkey\" and\n>> \"sslcert\" were still left as plain vanilla strings. I do not know\n>> if that was an elaborate omission, or a mere oversight, as it seems\n>> that it happened while I was away, so...\n>\n> It was more of an oversight than a deliberate omission, but more\n> accurately I didn't actively consider whether the other http.ssl*\n> variables were pathname-like or not.\n>\n> At the time I was trying to make a config which needed to set\n> http.sslCAPath and/or http.sslCAInfo more portable between users and\n> these were \"obviously\" pathname-like to me. Now that I read\n> the help for http.sslCert and http.sslKey, I see no reason that they\n> shouldn't also use git_config_pathname. If I'd been more thorough I\n> would have proposed this at the time.\n\nThanks.\n\nI've read the function again and I think the attached patch covers\neverything that ought to be a filename.\n\nBy the way, to credit you, do you prefer your bloomberg or hashpling\naddress?\n\n-- >8 --\nSubject: http.c: http.sslcert and http.sslkey are both pathnames\n\nBack when the modern http_options() codepath was created to parse\nvarious http.* options at 29508e1e (\"Isolate shared HTTP request\nfunctionality\", 2005-11-18), and then later was corrected for\ninteration between the multiple configuration files in 7059cd99\n(\"http_init(): Fix config file parsing\", 2009-03-09), we parsed\nconfiguration variables like http.sslkey, http.sslcert as plain\nvanilla strings, because git_config_pathname() that understands\n\"~[username]/\" prefix did not exist. Later, we converted some of\nthem (namely, http.sslCAPath and http.sslCAInfo) to use the\nfunction, and added variables like http.cookeyFile http.pinnedpubkey\nto use the function from the beginning. Because of that, these\nvariables all understand \"~[username]/\" prefix.\n\nMake the remaining two variables, http.sslcert and http.sslkey, also\naware of the convention, as they are both clearly pathnames to\nfiles.\n\nNoticed-by: Victor Toni \nHelped-by: Charles Bailey \nSigned-off-by: Junio C Hamano \n---\n http.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex c6c010f881..76ff63c14d 100644\n--- a/http.c\n+++ b/http.c\n@@ -272,10 +272,10 @@ static int http_options(const char *var, const char *value, void *cb)\n \tif (!strcmp(\"http.sslversion\", var))\n \t\treturn git_config_string(&ssl_version, var, value);\n \tif (!strcmp(\"http.sslcert\", var))\n-\t\treturn git_config_string(&ssl_cert, var, value);\n+\t\treturn git_config_pathname(&ssl_cert, var, value);\n #if LIBCURL_VERSION_NUM >= 0x070903\n \tif (!strcmp(\"http.sslkey\", var))\n-\t\treturn git_config_string(&ssl_key, var, value);\n+\t\treturn git_config_pathname(&ssl_key, var, value);\n #endif\n #if LIBCURL_VERSION_NUM >= 0x070908\n \tif (!strcmp(\"http.sslcapath\", var))\n"},{"id":"324837","messageId":"20170720205237.GA14368@hashpling.org","threadId":"46424","inReplyTo":"xmqqwp72kg03.fsf@gitster.mtv.corp.google.com","subject":"Re: Handling of paths","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2017-07-20T20:52:37Z","receivedAt":"2017-07-20T20:52:44Z","isPatch":false,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"On Thu, Jul 20, 2017 at 01:30:52PM -0700, Junio C Hamano wrote:\n> \n> I've read the function again and I think the attached patch covers\n> everything that ought to be a filename.\n> \n> By the way, to credit you, do you prefer your bloomberg or hashpling\n> address?\n\nThe patch looks good to me.\n\nIt's not critical which address you credit.\n\nI mark patches which result from my work at Bloomberg with my Bloomberg\nemail address and anything that I do entirely outside of work with my\nhashpling address, although I will tend to use my hashpling email for\nall communications because it co-operates with the mailing list\nconventions a lot better.\n\nIn this case, this is a follow on from a cbailey32@bloomberg.net patch\nso crediting that address seems the more appropriate option.\n\nCharles.\n"},{"id":"324839","messageId":"CAG0OSgfWZAbr1_j-SYYZyAzOvW4mrSFa7bBkfhRbJskgdGmsZQ@mail.gmail.com","threadId":"46424","inReplyTo":"xmqqwp72kg03.fsf@gitster.mtv.corp.google.com","subject":"Re: Handling of paths","fromName":"Victor Toni","fromEmail":"victor.toni@gmail.com","sentAt":"2017-07-20T21:03:48Z","receivedAt":"2017-07-20T21:04:24Z","isPatch":false,"sender":{"key":"victor.toni@gmail.com","avatar":null},"body":"2017-07-20 22:30 GMT+02:00 Junio C Hamano :\n>\n> I've read the function again and I think the attached patch covers\n> everything that ought to be a filename.\n>\nYour swift reaction is very much appreciated.\nWith the background you gave I just started to to create a patch\nmyself just to see that you already finished the patch.\n\nThanks a lot!\n\nBest regards,\nVictor\n"},{"id":"324865","messageId":"xmqq1sp9izy2.fsf@gitster.mtv.corp.google.com","threadId":"46424","inReplyTo":"CAG0OSgfWZAbr1_j-SYYZyAzOvW4mrSFa7bBkfhRbJskgdGmsZQ@mail.gmail.com","subject":"Re: Handling of paths","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-21T15:15:17Z","receivedAt":"2017-07-21T15:15:33Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Victor Toni writes:\n\n> 2017-07-20 22:30 GMT+02:00 Junio C Hamano :\n>>\n>> I've read the function again and I think the attached patch covers\n>> everything that ought to be a filename.\n>>\n> Your swift reaction is very much appreciated.\n> With the background you gave I just started to to create a patch\n> myself just to see that you already finished the patch.\n\nHeh, I guess I could have waited to save time ;-) \n\nI did the patch myself in order to avoid wasting your effort to find\nand report the issue and time and distraction cost from Charles to\nremember what happened 2 years ago and reply to me, because I will\ncertainly forget if I didn't have some patch readily usable in the\nlist archive.\n\nIn general, I (and other experienced reviewers here) prefer to give\nchances to people who are new to the Git development community and\nare inclined to do so to scratch their own itch, by giving analysis\nof the problem and a suggested route to solve it, but without giving\nthe final solution in a patch form. After all, many developers\n(including me) started from small changes before getting involved\nmore deeply to the project and starting to play more important\nroles.\n\nSome reviewers are much better than myself in judging if a new\nperson wants satisfaction of solving himself or herself[*1*], and\nthey end their analysis and suggestion with a phrase like \"Want to\ntry doing a patch yourself?\" I try to follow their example myself,\nbut I do not always succeed, and this is one of such cases. I guess\nyou could have immediately responded \"OK, let me try to see if I can\nfix it myself\" before starting to actually work on it ;-)\n\nHaving said all that, I suspect that your original problem\ndescription might point at another thing we may want to look into.\n\nThe patch under discussion may have solved the \"~[username]/\" prefix\nissue, but I offhand am not sure if a path-like variable that holds\na relative path behaves sensibly when they appear in configuration\nfiles and in a file that has configuration snippets that is included\nwith the \"[include] path=...\" thing, and if there is a need to clarify\nand/or update the rules.\n\nIn any case, thanks for reporting the bugs on two variables, and\nwelcome to the Git development community.\n\n\n[Footnote]\n\n*1* Some people just want to report an issue and move on, which is\n understandable.\n"},{"id":"324953","messageId":"20170724165221.2biudhpfyyp5ytfc@sigill.intra.peff.net","threadId":"46424","inReplyTo":"xmqq1sp9izy2.fsf@gitster.mtv.corp.google.com","subject":"Re: Handling of paths","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-07-24T16:52:22Z","receivedAt":"2017-07-24T16:52:29Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 21, 2017 at 08:15:17AM -0700, Junio C Hamano wrote:\n\n> In general, I (and other experienced reviewers here) prefer to give\n> chances to people who are new to the Git development community and\n> are inclined to do so to scratch their own itch, by giving analysis\n> of the problem and a suggested route to solve it, but without giving\n> the final solution in a patch form. After all, many developers\n> (including me) started from small changes before getting involved\n> more deeply to the project and starting to play more important\n> roles.\n\nThis is a good point, and I should remember to do it more, too.\nIt's often faster to do a small patch yourself than to help walk a\nfirst-timer through it, but keeping the community healthy is an\nimportant step.\n\nAt any rate, your patch to use config_pathname() looks like the right\nthing to me.\n\n> Having said all that, I suspect that your original problem\n> description might point at another thing we may want to look into.\n> \n> The patch under discussion may have solved the \"~[username]/\" prefix\n> issue, but I offhand am not sure if a path-like variable that holds\n> a relative path behaves sensibly when they appear in configuration\n> files and in a file that has configuration snippets that is included\n> with the \"[include] path=...\" thing, and if there is a need to clarify\n> and/or update the rules.\n\nThe \"[include]path\" behavior is intentional and documented: it takes the\npath relative to the including file. I think that would be a reasonable\nbehavior for path-like variables in general (and a path-like variable in\nan included file would be relative to that included file; this should\nJust Work because the include mechanism keeps a stack of files).\n\nI could probably sketch out a patch, but per the above discussion I'll\nleave it for now. Also, I'm lazy. ;)\n\n-Peff\n"}]}