Re: [GSoC microproject] Add XDG support to the credential-store helper
Hi, Reading my previous message again, I apologize if it sounded conflicting. Truth to be told, I see merits in both proposed behaviors, but it all depends on whether we want git-credentials-store to support an arbitrary number of config files (now or in the future) or just two. I'm not sitting on the fence though, personally I think that we should go with supporting an arbitrary number of config files (and the behavior it entails for xdg file vs home file), because it will open up more possibilities in the future with regards to supporting multiple config sources. On Sat, Mar 7, 2015 at 1:28 AM, Matthieu Moy wrote: > The fact that I suggested doing it this way does not mean it _has_ to be > done this way. Decisions are taken by trying to reach a consensus with > discussion, so everyone is welcome to argue. Well, I think we need to decide if git is going to implement support for XDG_CONFIG_DIRS as well, as support for reading/writing an arbitrary number of config files will affect my views on the behavior. Personally, I think git-credentials-store should implement support for XDG_CONFIG_DIRS because, as I mentioned in the previous message, administrators may wish to provide users with default saved credentials. If machinery is being added to support reading/writing to an arbitrary number of config files, it would lead to simpler behavior (and simpler code) if the old ~/.git-credentials is just treated as just another config file to load from. (So yes, I agree with implementing your proposed behavior) However, if we are just going to support 2 configuration files (the xdg file and the home file), then I think Luis' proposed behavior has some merit. See below. (Just mentioning for completeness) The third option would be to implement a hybrid of the above two approaches (support arbitrary number of config files, but only choose 1 between the xdg file and home file), but this behavior is unnecessarily complex. > I don't remember all the discussions we had about the ~/.gitconfig, but > one issue with considering only one file is if you create > ~/.git/config/foo and initially make sure you don't have ~/.gitfoo, and > then one tool creates ~/.gitfoo (either an old Git, or another tool > trying to edit the config file), then you totally break your > configuration. > I argued for not taking backward compatibility too much into account in > another thread, but that was about precedence of one file over the other > which is far less important. Here, any tool creating even an empty home > file would break your configuration. Luis mentioned that if the user expects to use an old version of git, the user would (or should) not create the xdg file in the first place. I think that automated tools (and users) should call git-config to edit the config files anyway and not roll their own. In fact, I think that this issue will not occur at all if git prioritized ~/.config/git/foo over ~/.gitfoo instead of the other way around. When the user creates the xdg file, the user is signaling that old versions of git will not be used. Thus, if a tool creates/updates the old home file (and it should not if it calls git-config), then configuration changes won't take effect at all, and it _shouldn't_ because the tool is _broken_. But yes, the above strategy does not scale at all for multiple configuration sources, which there will be if support for XDG_CONFIG_DIRS is implemented. (As an aside, I find it weird that git-config allows values in ~/.gitconfig to override ~/.config/git/config, given that the xdg file is opt-in and introduced after ~/.gitconfig. Furthermore, it conflicts with its writing behavior -- it writes to ~/.config/git/config and not ~/.gitconfig if it exists.) > That also breaks the least surprise principle if you have a ~/.gitfoo > file that you forgot about: edit ~/.config/git/foo, nothing is taken > into account, at all (or the other way around, depending on the > precedence you choose). I remember loosing some time with two vlc > configuration files like this. Hmm, I don't know the exact specifics of what happened with VLC, so I can't judge. As mentioned above, if the user wants compatibility with old tools, the user will not create the xdg file. If the user has an updated toolset, the user will create the xdg file and delete the old home file. The old home file will not be created at all because all tools would have been updated to support the xdg file, and hence the user will not be confused. Of course, in the context of git-config, it has to read the files in /etc/gitconfig, $GIT_DIR/config etc, and thus as mentioned above, reading from the home file as well would lead to simpler behavior and code. Regards, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GSoC microproject] Add XDG support to the credential-store helper
Paul Tan writes: > Hi Luis, > > On Fri, Mar 6, 2015 at 7:41 AM, Luis Ressel wrote: >> Okay, I've had a look at Paul's patch for this. Sorry again for the >> dupe, I must've missed it before. I guess I'll attempt another >> microproject. >> >> However, I feel like my patch is really all that's neccessary; I don't >> think we should try to use both files if they exist -- I consider >> Paul's approach to be a bit overcomplicated. >> >> My patch still uses ~/.git-credentials as a default location, which >> should be sufficient for compability. > > Indeed, that was my initial plan for implementation as well[1]. Matthieu, > however, wanted the behavior to follow that of git-config[2]. The fact that I suggested doing it this way does not mean it _has_ to be done this way. Decisions are taken by trying to reach a consensus with discussion, so everyone is welcome to argue. I don't remember all the discussions we had about the ~/.gitconfig, but one issue with considering only one file is if you create ~/.git/config/foo and initially make sure you don't have ~/.gitfoo, and then one tool creates ~/.gitfoo (either an old Git, or another tool trying to edit the config file), then you totally break your configuration. I argued for not taking backward compatibility too much into account in another thread, but that was about precedence of one file over the other which is far less important. Here, any tool creating even an empty home file would break your configuration. That also breaks the least surprise principle if you have a ~/.gitfoo file that you forgot about: edit ~/.config/git/foo, nothing is taken into account, at all (or the other way around, depending on the precedence you choose). I remember loosing some time with two vlc configuration files like this. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GSoC microproject] Add XDG support to the credential-store helper
Hi Luis, On Fri, Mar 6, 2015 at 7:41 AM, Luis Ressel wrote: > Okay, I've had a look at Paul's patch for this. Sorry again for the > dupe, I must've missed it before. I guess I'll attempt another > microproject. > > However, I feel like my patch is really all that's neccessary; I don't > think we should try to use both files if they exist -- I consider > Paul's approach to be a bit overcomplicated. > > My patch still uses ~/.git-credentials as a default location, which > should be sufficient for compability. Indeed, that was my initial plan for implementation as well[1]. Matthieu, however, wanted the behavior to follow that of git-config[2]. [1] http://article.gmane.org/gmane.comp.version-control.git/264666 [2] http://article.gmane.org/gmane.comp.version-control.git/264669 > A user that wants to use > ~/.config/git/credentials instead has to manually create that file > first, which he just shouldn't do if he has to stay compatible to > previous git versions. Yes, I totally agree. I wonder if the current behavior to read the home config file in addition to the xdg config file is actually useful to the end-user. (However, that behavior has been in git since 2012, so for consistency purposes it may not be desirable to make credential-store act differently from git-config.) What I do believe, however, is that there *should* be a way for credential-store to lookup, erase and (maybe) store credentials to multiple files. This opens the door for git to be fully xdg base dir spec compliant by supporing both $XDG_CONFIG_HOME and $XDG_CONFIG_DIRS. I quote from the spec[3]: The order of base directories denotes their importance; the first directory listed is the most important. When the same information is defined in multiple places the information defined relative to the more important base directory takes precedent. The base directory defined by $XDG_DATA_HOME is considered more important than any of the base directories defined by $XDG_DATA_DIRS. The base directory defined by $XDG_CONFIG_HOME is considered more important than any of the base directories defined by $XDG_CONFIG_DIRS. [3] http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html Reading from multiple credential stores could be useful, for example, if the system administrator wanted to setup credentials for all users on the system. Thus, if we are implementing the machinery for the XDG dir spec, it would make sense to just add the home git-credentials file to the search path as well. Regards, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GSoC microproject] Add XDG support to the credential-store helper
Okay, I've had a look at Paul's patch for this. Sorry again for the dupe, I must've missed it before. I guess I'll attempt another microproject. However, I feel like my patch is really all that's neccessary; I don't think we should try to use both files if they exist -- I consider Paul's approach to be a bit overcomplicated. My patch still uses ~/.git-credentials as a default location, which should be sufficient for compability. A user that wants to use ~/.config/git/credentials instead has to manually create that file first, which he just shouldn't do if he has to stay compatible to previous git versions. Of course, my patch is definitly lacking a test case, and the documentation could be a tad better, too. Regards, Luis Ressel pgpUI1loUbPdU.pgp Description: OpenPGP digital signature
Re: [GSoC microproject] Add XDG support to the credential-store helper
On Thu, 5 Mar 2015 23:38:19 +0100 Christian Couder wrote: > Hi, > > Last year, on the ideas page: > > http://git.github.io/SoC-2014-Ideas.html > > we also had the following improvement: > > "in some cases, git bisect may test too many merge bases, thus slowing > down the bisection (making it closer to linear than logarithmic)." > > so you could work on that too. > > And if you really want and have time you can also rewrite > git-bisect.sh into a builtin command. There is already > builtin/bisect--helper.c where you can put some migrated code step by > step. > Thanks for those suggestions! > First, it looks like another student started working on the same > microproject. Did you have a look at what he did? If you didn't, you > probably should, so that reviewers don't need to tell you what they > already told the other student. And if you did, you should tell it, > and maybe explain what you did differently and why. > > If it looks like the other student is more advanced on this subject, > you might want to try another microproject. And if all the > microprojects are already taken, you might want to ask the list for > other microprojects, or you may want to have a look at the many merge > bases improvement above. (You could start by having a look at > check_merge_bases() in "bisect.c" and by creating a script or better a > test case that reproduces the problem, see > t/t6030-bisect-porcelain.sh.) > Oops. I had a quick glance at the list archives, but apparently I missed this. I'll have a look. > Welcome to the Git community! > > Best, > Christian. Thanks! Regards, Luis Ressel pgpwC6oLSqAfN.pgp Description: OpenPGP digital signature
Re: [GSoC microproject] Add XDG support to the credential-store helper
Hi, On Thu, Mar 5, 2015 at 9:51 PM, Luis Ressel wrote: > I'm contributing this patch in preparation for a GSoC15 application with > the git project. In particular, I'm interested in the two bisect > improvements listed on the Ideas page. Does anyone have other > suggestions for potential improvements of git bisect that would be > suitable for such a GSoC project? Last year, on the ideas page: http://git.github.io/SoC-2014-Ideas.html we also had the following improvement: "in some cases, git bisect may test too many merge bases, thus slowing down the bisection (making it closer to linear than logarithmic)." so you could work on that too. And if you really want and have time you can also rewrite git-bisect.sh into a builtin command. There is already builtin/bisect--helper.c where you can put some migrated code step by step. > Oh, and should I add a testcase for the new functionality introduced by > this patch? And if yes, what exactly should I test? First, it looks like another student started working on the same microproject. Did you have a look at what he did? If you didn't, you probably should, so that reviewers don't need to tell you what they already told the other student. And if you did, you should tell it, and maybe explain what you did differently and why. If it looks like the other student is more advanced on this subject, you might want to try another microproject. And if all the microprojects are already taken, you might want to ask the list for other microprojects, or you may want to have a look at the many merge bases improvement above. (You could start by having a look at check_merge_bases() in "bisect.c" and by creating a script or better a test case that reproduces the problem, see t/t6030-bisect-porcelain.sh.) Welcome to the Git community! Best, Christian. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GSoC microproject] Add XDG support to the credential-store helper
Luis Ressel writes: > Oh, and should I add a testcase for the new functionality introduced by > this patch? And if yes, what exactly should I test? Absolutely. Thinking things through to design what needs to be tested is part of the exercise ;-) You need to demonstrate that the new feature kicks in when it should, and also you need to make sure the new feature does not get used when it shouldn't. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GSoC microproject] Add XDG support to the credential-store helper
I'm contributing this patch in preparation for a GSoC15 application with the git project. In particular, I'm interested in the two bisect improvements listed on the Ideas page. Does anyone have other suggestions for potential improvements of git bisect that would be suitable for such a GSoC project? Oh, and should I add a testcase for the new functionality introduced by this patch? And if yes, what exactly should I test? Also, apologies if these mails arrive twice. I'm resending them because Majordomo seems to have ignored my initial attempt 6 hours ago. Regards, Luis Ressel -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html