{"thread":{"id":"38714","subject":"[GSoC microproject] Add XDG support to the credential-store helper","startedAt":"2015-03-05T20:51:46Z","lastAt":"2015-03-06T19:21:02Z","messageCount":9,"participants":["Luis Ressel","Junio C Hamano","Christian Couder","Paul Tan","Matthieu Moy"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"257106","messageId":"20150305215146.394caa71@gentp.lnet","threadId":"38714","inReplyTo":null,"subject":"[GSoC microproject] Add XDG support to the credential-store helper","fromName":"Luis Ressel","fromEmail":"aranea@aixah.de","sentAt":"2015-03-05T20:51:46Z","receivedAt":"2015-03-05T20:51:46Z","isPatch":false,"sender":{"key":"aranea@aixah.de","avatar":"https://avatars.githubusercontent.com/u/864193?v=4"},"body":"I'm contributing this patch in preparation for a GSoC15 application with\nthe git project. In particular, I'm interested in the two bisect\nimprovements listed on the Ideas page. Does anyone have other\nsuggestions for potential improvements of git bisect that would be\nsuitable for such a GSoC project?\n\nOh, and should I add a testcase for the new functionality introduced by\nthis patch? And if yes, what exactly should I test?\n\nAlso, apologies if these mails arrive twice. I'm resending them because\nMajordomo seems to have ignored my initial attempt 6 hours ago.\n\nRegards,\nLuis Ressel\n"},{"id":"257107","messageId":"20150305215306.65aab57c@gentp.lnet","threadId":"38714","inReplyTo":"20150305215146.394caa71@gentp.lnet","subject":"[PATCH] Add XDG support to the credential-store helper","fromName":"Luis Ressel","fromEmail":"aranea@aixah.de","sentAt":"2015-03-05T20:53:06Z","receivedAt":"2015-03-05T20:53:06Z","isPatch":true,"sender":{"key":"aranea@aixah.de","avatar":"https://avatars.githubusercontent.com/u/864193?v=4"},"body":"credential-store will use ${XDG_CONFIG_HOME}/credentials to store\ncredentials if this file already exists and no --file option is given.\nOtherwise it'll fall back to ~/.git-credentials (status quo).\n\nSigned-off-by: Luis Ressel \n---\n Documentation/git-credential-store.txt | 4 +++-\n credential-store.c | 2 ++\n 2 files changed, 5 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-credential-store.txt\nb/Documentation/git-credential-store.txt index bc97071..cab8f72 100644\n--- a/Documentation/git-credential-store.txt\n+++ b/Documentation/git-credential-store.txt\n@@ -34,7 +34,9 @@ OPTIONS\n \tUse `` to store credentials. The file will have its\n \tfilesystem permissions set to prevent other users on the system\n \tfrom reading it, but will not be encrypted or otherwise\n-\tprotected. Defaults to `~/.git-credentials`.\n+\tprotected. Defaults to `~/.git-credentials` or\n+\t`$XDG_CONFIG_HOME/git/credentials` if the latter exists\n+\t($XDG_CONFIG_HOME defaults to ~/.config).\n \n EXAMPLES\n --------\ndiff --git a/credential-store.c b/credential-store.c\nindex 925d3f4..9720b42 100644\n--- a/credential-store.c\n+++ b/credential-store.c\n@@ -138,6 +138,8 @@ int main(int argc, char **argv)\n \top = argv[0];\n \n \tif (!file)\n+\t\thome_config_paths(NULL, &file, \"credentials\");\n+\tif (!file || access(file, R_OK|W_OK))\n \t\tfile = expand_user_path(\"~/.git-credentials\");\n \tif (!file)\n \t\tdie(\"unable to set up default path; use --file\");\n-- \n2.3.1\n"},{"id":"257111","messageId":"xmqqsidj14f8.fsf@gitster.dls.corp.google.com","threadId":"38714","inReplyTo":"20150305215146.394caa71@gentp.lnet","subject":"Re: [GSoC microproject] Add XDG support to the credential-store helper","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-05T22:10:35Z","receivedAt":"2015-03-05T22:10:35Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Luis Ressel writes:\n\n> Oh, and should I add a testcase for the new functionality introduced by\n> this patch? And if yes, what exactly should I test?\n\nAbsolutely.\n\nThinking things through to design what needs to be tested is part of\nthe exercise ;-) You need to demonstrate that the new feature kicks\nin when it should, and also you need to make sure the new feature\ndoes not get used when it shouldn't.\n"},{"id":"257113","messageId":"CAP8UFD3kHKsUWqmfr3mEtTYwVFwGGqMF_wJgVQyoaH=2i9Ge=A@mail.gmail.com","threadId":"38714","inReplyTo":"20150305215146.394caa71@gentp.lnet","subject":"Re: [GSoC microproject] Add XDG support to the credential-store helper","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-03-05T22:38:19Z","receivedAt":"2015-03-05T22:38:19Z","isPatch":false,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Hi,\n\nOn Thu, Mar 5, 2015 at 9:51 PM, Luis Ressel wrote:\n> I'm contributing this patch in preparation for a GSoC15 application with\n> the git project. In particular, I'm interested in the two bisect\n> improvements listed on the Ideas page. Does anyone have other\n> suggestions for potential improvements of git bisect that would be\n> suitable for such a GSoC project?\n\nLast year, on the ideas page:\n\nhttp://git.github.io/SoC-2014-Ideas.html\n\nwe also had the following improvement:\n\n\"in some cases, git bisect may test too many merge bases, thus slowing\ndown the bisection (making it closer to linear than logarithmic).\"\n\nso you could work on that too.\n\nAnd if you really want and have time you can also rewrite\ngit-bisect.sh into a builtin command. There is already\nbuiltin/bisect--helper.c where you can put some migrated code step by\nstep.\n\n> Oh, and should I add a testcase for the new functionality introduced by\n> this patch? And if yes, what exactly should I test?\n\nFirst, it looks like another student started working on the same\nmicroproject. Did you have a look at what he did? If you didn't, you\nprobably should, so that reviewers don't need to tell you what they\nalready told the other student. And if you did, you should tell it,\nand maybe explain what you did differently and why.\n\nIf it looks like the other student is more advanced on this subject,\nyou might want to try another microproject. And if all the\nmicroprojects are already taken, you might want to ask the list for\nother microprojects, or you may want to have a look at the many merge\nbases improvement above. (You could start by having a look at\ncheck_merge_bases() in \"bisect.c\" and by creating a script or better a\ntest case that reproduces the problem, see\nt/t6030-bisect-porcelain.sh.)\n\nWelcome to the Git community!\n\nBest,\nChristian.\n"},{"id":"257117","messageId":"20150306001534.06882282@gentp.lnet","threadId":"38714","inReplyTo":"CAP8UFD3kHKsUWqmfr3mEtTYwVFwGGqMF_wJgVQyoaH=2i9Ge=A@mail.gmail.com","subject":"Re: [GSoC microproject] Add XDG support to the credential-store helper","fromName":"Luis Ressel","fromEmail":"aranea@aixah.de","sentAt":"2015-03-05T23:15:34Z","receivedAt":"2015-03-05T23:15:34Z","isPatch":false,"sender":{"key":"aranea@aixah.de","avatar":"https://avatars.githubusercontent.com/u/864193?v=4"},"body":"On Thu, 5 Mar 2015 23:38:19 +0100\nChristian Couder wrote:\n\n> Hi,\n> \n> Last year, on the ideas page:\n> \n> http://git.github.io/SoC-2014-Ideas.html\n> \n> we also had the following improvement:\n> \n> \"in some cases, git bisect may test too many merge bases, thus slowing\n> down the bisection (making it closer to linear than logarithmic).\"\n> \n> so you could work on that too.\n> \n> And if you really want and have time you can also rewrite\n> git-bisect.sh into a builtin command. There is already\n> builtin/bisect--helper.c where you can put some migrated code step by\n> step.\n> \n\nThanks for those suggestions!\n\n> First, it looks like another student started working on the same\n> microproject. Did you have a look at what he did? If you didn't, you\n> probably should, so that reviewers don't need to tell you what they\n> already told the other student. And if you did, you should tell it,\n> and maybe explain what you did differently and why.\n> \n> If it looks like the other student is more advanced on this subject,\n> you might want to try another microproject. And if all the\n> microprojects are already taken, you might want to ask the list for\n> other microprojects, or you may want to have a look at the many merge\n> bases improvement above. (You could start by having a look at\n> check_merge_bases() in \"bisect.c\" and by creating a script or better a\n> test case that reproduces the problem, see\n> t/t6030-bisect-porcelain.sh.)\n> \n\nOops. I had a quick glance at the list archives, but apparently I\nmissed this. I'll have a look.\n\n> Welcome to the Git community!\n> \n> Best,\n> Christian.\n\nThanks!\n\n\nRegards,\nLuis Ressel\n"},{"id":"257118","messageId":"20150306004139.2dac2502@gentp.lnet","threadId":"38714","inReplyTo":"20150306001534.06882282@gentp.lnet","subject":"Re: [GSoC microproject] Add XDG support to the credential-store helper","fromName":"Luis Ressel","fromEmail":"aranea@aixah.de","sentAt":"2015-03-05T23:41:39Z","receivedAt":"2015-03-05T23:41:39Z","isPatch":false,"sender":{"key":"aranea@aixah.de","avatar":"https://avatars.githubusercontent.com/u/864193?v=4"},"body":"Okay, I've had a look at Paul's patch for this. Sorry again for the\ndupe, I must've missed it before. I guess I'll attempt another\nmicroproject.\n\nHowever, I feel like my patch is really all that's neccessary; I don't\nthink we should try to use both files if they exist -- I consider\nPaul's approach to be a bit overcomplicated.\n\nMy patch still uses ~/.git-credentials as a default location, which\nshould be sufficient for compability. A user that wants to use\n~/.config/git/credentials instead has to manually create that file\nfirst, which he just shouldn't do if he has to stay compatible to\nprevious git versions.\n\nOf course, my patch is definitly lacking a test case, and the\ndocumentation could be a tad better, too.\n\n\nRegards,\nLuis Ressel\n"},{"id":"257133","messageId":"CACRoPnR3i3pTyqcxm0iELP0uR9cL2kvm9fd7BQPVpbLB0UBNVw@mail.gmail.com","threadId":"38714","inReplyTo":"20150306004139.2dac2502@gentp.lnet","subject":"Re: [GSoC microproject] Add XDG support to the credential-store helper","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-03-06T08:04:39Z","receivedAt":"2015-03-06T08:04:39Z","isPatch":false,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi Luis,\n\nOn Fri, Mar 6, 2015 at 7:41 AM, Luis Ressel wrote:\n> Okay, I've had a look at Paul's patch for this. Sorry again for the\n> dupe, I must've missed it before. I guess I'll attempt another\n> microproject.\n>\n> However, I feel like my patch is really all that's neccessary; I don't\n> think we should try to use both files if they exist -- I consider\n> Paul's approach to be a bit overcomplicated.\n>\n> My patch still uses ~/.git-credentials as a default location, which\n> should be sufficient for compability.\n\nIndeed, that was my initial plan for implementation as well[1]. Matthieu,\nhowever, wanted the behavior to follow that of git-config[2].\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/264666\n[2] http://article.gmane.org/gmane.comp.version-control.git/264669\n\n> A user that wants to use\n> ~/.config/git/credentials instead has to manually create that file\n> first, which he just shouldn't do if he has to stay compatible to\n> previous git versions.\n\nYes, I totally agree. I wonder if the current behavior to read the home\nconfig file in addition to the xdg config file is actually\nuseful to the end-user. (However, that behavior has been in git since 2012, so\nfor consistency purposes it may not be desirable to make credential-store act\ndifferently from git-config.)\n\nWhat I do believe, however, is that there *should* be a way for\ncredential-store to lookup, erase and (maybe) store credentials to multiple\nfiles. This opens the door for git to be fully xdg base dir spec compliant\nby supporing both $XDG_CONFIG_HOME and $XDG_CONFIG_DIRS.\nI quote from the spec[3]:\n\n The order of base directories denotes their importance; the first directory\n listed is the most important. When the same information is defined in\n multiple places the information defined relative to the more important base\n directory takes precedent. The base directory defined by $XDG_DATA_HOME is\n considered more important than any of the base directories defined by\n $XDG_DATA_DIRS. The base directory defined by $XDG_CONFIG_HOME is\n considered more important than any of the base directories defined by\n $XDG_CONFIG_DIRS.\n\n[3] http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html\n\nReading from multiple credential stores could be useful, for example, if the\nsystem administrator wanted to setup credentials for all users on the system.\n\nThus, if we are implementing the machinery for the XDG dir spec, it would make\nsense to just add the home git-credentials file to the search path as well.\n\nRegards,\nPaul\n"},{"id":"257178","messageId":"vpqa8zq9grq.fsf@anie.imag.fr","threadId":"38714","inReplyTo":"CACRoPnR3i3pTyqcxm0iELP0uR9cL2kvm9fd7BQPVpbLB0UBNVw@mail.gmail.com","subject":"Re: [GSoC microproject] Add XDG support to the credential-store helper","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-03-06T17:28:57Z","receivedAt":"2015-03-06T17:28:57Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Paul Tan writes:\n\n> Hi Luis,\n>\n> On Fri, Mar 6, 2015 at 7:41 AM, Luis Ressel wrote:\n>> Okay, I've had a look at Paul's patch for this. Sorry again for the\n>> dupe, I must've missed it before. I guess I'll attempt another\n>> microproject.\n>>\n>> However, I feel like my patch is really all that's neccessary; I don't\n>> think we should try to use both files if they exist -- I consider\n>> Paul's approach to be a bit overcomplicated.\n>>\n>> My patch still uses ~/.git-credentials as a default location, which\n>> should be sufficient for compability.\n>\n> Indeed, that was my initial plan for implementation as well[1]. Matthieu,\n> however, wanted the behavior to follow that of git-config[2].\n\nThe fact that I suggested doing it this way does not mean it _has_ to be\ndone this way. Decisions are taken by trying to reach a consensus with\ndiscussion, so everyone is welcome to argue.\n\nI don't remember all the discussions we had about the ~/.gitconfig, but\none issue with considering only one file is if you create\n~/.git/config/foo and initially make sure you don't have ~/.gitfoo, and\nthen one tool creates ~/.gitfoo (either an old Git, or another tool\ntrying to edit the config file), then you totally break your\nconfiguration.\n\nI argued for not taking backward compatibility too much into account in\nanother thread, but that was about precedence of one file over the other\nwhich is far less important. Here, any tool creating even an empty home\nfile would break your configuration.\n\nThat also breaks the least surprise principle if you have a ~/.gitfoo\nfile that you forgot about: edit ~/.config/git/foo, nothing is taken\ninto account, at all (or the other way around, depending on the\nprecedence you choose). I remember loosing some time with two vlc\nconfiguration files like this.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"257189","messageId":"CACRoPnQe67OFta276aQRiAoMcWdWO-3njHpnx8MHnkm5kieW=Q@mail.gmail.com","threadId":"38714","inReplyTo":"vpqa8zq9grq.fsf@anie.imag.fr","subject":"Re: [GSoC microproject] Add XDG support to the credential-store helper","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-03-06T19:21:02Z","receivedAt":"2015-03-06T19:21:02Z","isPatch":false,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi,\n\nReading my previous message again, I apologize if it sounded\nconflicting. Truth to be told, I see merits in both proposed\nbehaviors, but it all depends on whether we want git-credentials-store\nto support an arbitrary number of config files (now or in the future)\nor just two. I'm not sitting on the fence though, personally I think\nthat we should go with supporting an arbitrary number of config files\n(and the behavior it entails for xdg file vs home file), because it\nwill open up more possibilities in the future with regards to\nsupporting multiple config sources.\n\nOn Sat, Mar 7, 2015 at 1:28 AM, Matthieu Moy\n wrote:\n> The fact that I suggested doing it this way does not mean it _has_ to be\n> done this way. Decisions are taken by trying to reach a consensus with\n> discussion, so everyone is welcome to argue.\n\nWell, I think we need to decide if git is going to implement support\nfor XDG_CONFIG_DIRS as well, as support for reading/writing an\narbitrary number of config files will affect my views on the behavior.\nPersonally, I think git-credentials-store should implement support for\nXDG_CONFIG_DIRS because, as I mentioned in the previous message,\nadministrators may wish to provide users with default saved\ncredentials.\n\nIf machinery is being added to support reading/writing to an arbitrary\nnumber of config files, it would lead to simpler behavior (and simpler\ncode) if the old ~/.git-credentials is just treated as just another\nconfig file to load from. (So yes, I agree with implementing your\nproposed behavior)\n\nHowever, if we are just going to support 2 configuration files (the\nxdg file and the home file), then I think Luis' proposed behavior has\nsome merit. See below.\n\n(Just mentioning for completeness) The third option would be to\nimplement a hybrid of the above two approaches (support arbitrary\nnumber of config files, but only choose 1 between the xdg file and\nhome file), but this behavior is unnecessarily complex.\n\n> I don't remember all the discussions we had about the ~/.gitconfig, but\n> one issue with considering only one file is if you create\n> ~/.git/config/foo and initially make sure you don't have ~/.gitfoo, and\n> then one tool creates ~/.gitfoo (either an old Git, or another tool\n> trying to edit the config file), then you totally break your\n> configuration.\n> I argued for not taking backward compatibility too much into account in\n> another thread, but that was about precedence of one file over the other\n> which is far less important. Here, any tool creating even an empty home\n> file would break your configuration.\n\nLuis mentioned that if the user expects to use an old version of git,\nthe user would (or should) not create the xdg file in the first place.\nI think that automated tools (and users) should call git-config to\nedit the config files anyway and not roll their own. In fact, I think\nthat this issue will not occur at all if git prioritized\n~/.config/git/foo over ~/.gitfoo instead of the other way around. When\nthe user creates the xdg file, the user is signaling that old versions\nof git will not be used. Thus, if a tool creates/updates the old home\nfile (and it should not if it calls git-config), then configuration\nchanges won't take effect at all, and it _shouldn't_ because the tool\nis _broken_.\n\nBut yes, the above strategy does not scale at all for multiple\nconfiguration sources, which there will be if support for\nXDG_CONFIG_DIRS is implemented.\n\n(As an aside, I find it weird that git-config allows values in\n~/.gitconfig to override ~/.config/git/config, given that the xdg file\nis opt-in and introduced after ~/.gitconfig. Furthermore, it conflicts\nwith its writing behavior -- it writes to ~/.config/git/config and not\n~/.gitconfig if it exists.)\n\n> That also breaks the least surprise principle if you have a ~/.gitfoo\n> file that you forgot about: edit ~/.config/git/foo, nothing is taken\n> into account, at all (or the other way around, depending on the\n> precedence you choose). I remember loosing some time with two vlc\n> configuration files like this.\n\nHmm, I don't know the exact specifics of what happened with VLC, so I\ncan't judge. As mentioned above, if the user wants compatibility with\nold tools, the user will not create the xdg file. If the user has an\nupdated toolset, the user will create the xdg file and delete the old\nhome file. The old home file will not be created at all because all\ntools would have been updated to support the xdg file, and hence the\nuser will not be confused.\n\nOf course, in the context of git-config, it has to read the files in\n/etc/gitconfig, $GIT_DIR/config etc, and thus as mentioned above,\nreading from the home file as well would lead to simpler behavior and\ncode.\n\nRegards,\nPaul\n"}]}