You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Senthil Kumaran S <se...@collab.net> on 2008/07/14 12:09:17 UTC
Re: [PATCH] Fix issue #2324
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Senthil Kumaran S wrote:
> I am attaching an updated patch along with this email as per Dave's
> suggestion for adding the username in the servers file.
I am attaching an updated patch which applies cleanly on r32089 of trunk.
[[[
Fix issue #2324.
Disable username assumption.
* subversion/include/svn_config.h
(): Add SVN_CONFIG_OPTION_USERNAME.
* subversion/libsvn_subr/config_file.c
(svn_config_ensure): Add text about the new config file parameter 'username'.
* subversion/libsvn_subr/simple_providers.c
(simple_first_creds_helper): Fetch username from servers file if it is
defined.
(prompt_for_simple_creds): If we didnt get a password in
simple_first_creds_helper then we need to get the username again here.
Patch by: stylesen
]]]
Thank You.
- --
Senthil Kumaran S
http://www.stylesen.org/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
iD8DBQFIe0Hq9o1G+2zNQDgRAoMLAJ90xC/kK+D3Q4I0vh5fci2qBLJ0ewCgvO82
VcdJpKGQlL3Kdj+m/z2VQXU=
=qzcB
-----END PGP SIGNATURE-----
Re: [PATCH] Fix issue #2324
Posted by Senthil Kumaran S <se...@collab.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
C. Michael Pilato wrote:
> I've not tested the patch *extensively*, but simple testing shows stuff
> working as expected. +1 to commit.
Committed in r32403.
Thank You.
- --
Senthil Kumaran S
http://www.stylesen.org/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
iD8DBQFIm+vY9o1G+2zNQDgRAvHgAJwO9YDKtqd2IAjjYflTaE2i7cedhACcCNMD
6qRUF9xGt1EVW2Eq1q9nIjU=
=Wh1d
-----END PGP SIGNATURE-----
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Fix issue #2324
Posted by "C. Michael Pilato" <cm...@collab.net>.
I've not tested the patch *extensively*, but simple testing shows stuff
working as expected. +1 to commit.
Senthil Kumaran S wrote:
> Hi Mike,
>
> Thanks for your review, my comments inline.
>
> C. Michael Pilato wrote:
>> I have a question, though -- should the 'servers' file "win" over the
>> auth cache in terms of priority? Seems to me that the auth cache --
>> which typically is populated with known-good credentials, should be
>> given precedence over the 'servers' file. That would allow me to put
>> "cmpilato" in the [global] section of my servers file and yet not have
>> that contradict creds in the auth cache which use a different username.
>
>> In fact, I guess I kinda expected to see code something like this:
>
>> creds = check-auth-cache
>> if (! creds)
>> creds = check-servers-file
>> if (! creds)
>> creds = apr_getusername()
>
>> With such an ordering, the servers file effectively becomes just a way
>> to override the username assumption.
>
> This sounds like a good ordering to me. I ve updated the patch accordingly and
> attached along with this email.
>
>> But maybe this discussion was already played out previously and I missed
>> it?
>
> No we didnt had a discussion about this. We had discussions whether to have
> username option in config file or the servers file.
>
> [[[
> Fix issue #2324: Disable username assumption.
>
> Rather than disabling the username assumption, this takes the
> different approach of consulting the runtime configuration for a
> default username before falling back to other methods (which include
> using the current user id's name).
>
> The order in which default username is determined is as follows:
> 1. username supplied in command line
> 2. username in the auth cache
> 3. username given in servers file
> 4. current user id's name
>
> * subversion/include/svn_config.h
> (SVN_CONFIG_OPTION_USERNAME): New config option.
>
> * subversion/libsvn_subr/config_file.c
> (svn_config_ensure): Add text about the new config file parameter 'username'.
>
> * subversion/libsvn_subr/simple_providers.c
> (simple_first_creds_helper, prompt_for_simple_creds): Try to find a
> username in the 'servers' file.
>
> Patch by: stylesen
> Suggested by: glasser
> cmpilato
> Review by: cmpilato
> ]]]
>
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: [PATCH] Fix issue #2324
Posted by Senthil Kumaran S <se...@collab.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hi Mike,
Thanks for your review, my comments inline.
C. Michael Pilato wrote:
> I have a question, though -- should the 'servers' file "win" over the
> auth cache in terms of priority? Seems to me that the auth cache --
> which typically is populated with known-good credentials, should be
> given precedence over the 'servers' file. That would allow me to put
> "cmpilato" in the [global] section of my servers file and yet not have
> that contradict creds in the auth cache which use a different username.
>
> In fact, I guess I kinda expected to see code something like this:
>
> creds = check-auth-cache
> if (! creds)
> creds = check-servers-file
> if (! creds)
> creds = apr_getusername()
>
> With such an ordering, the servers file effectively becomes just a way
> to override the username assumption.
This sounds like a good ordering to me. I ve updated the patch accordingly and
attached along with this email.
> But maybe this discussion was already played out previously and I missed
> it?
No we didnt had a discussion about this. We had discussions whether to have
username option in config file or the servers file.
[[[
Fix issue #2324: Disable username assumption.
Rather than disabling the username assumption, this takes the
different approach of consulting the runtime configuration for a
default username before falling back to other methods (which include
using the current user id's name).
The order in which default username is determined is as follows:
1. username supplied in command line
2. username in the auth cache
3. username given in servers file
4. current user id's name
* subversion/include/svn_config.h
(SVN_CONFIG_OPTION_USERNAME): New config option.
* subversion/libsvn_subr/config_file.c
(svn_config_ensure): Add text about the new config file parameter 'username'.
* subversion/libsvn_subr/simple_providers.c
(simple_first_creds_helper, prompt_for_simple_creds): Try to find a
username in the 'servers' file.
Patch by: stylesen
Suggested by: glasser
cmpilato
Review by: cmpilato
]]]
- --
Senthil Kumaran S
http://www.stylesen.org/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
iD8DBQFImr8O9o1G+2zNQDgRAvYhAKCXVsetTWAmJtDDt5L7wN7EptkEKACgl7jL
4QNSHO/DPjwEszlvd1j5srA=
=Bzcw
-----END PGP SIGNATURE-----
Re: [PATCH] Fix issue #2324
Posted by "C. Michael Pilato" <cm...@collab.net>.
Senthil, I've made some minor changes to your patch:
* Adding a trailing period, and losing an extra blank line from the
default 'servers' file content.
* Tweaking comments in the code so that neighboring comments make sense.
I have a question, though -- should the 'servers' file "win" over the auth
cache in terms of priority? Seems to me that the auth cache -- which
typically is populated with known-good credentials, should be given
precedence over the 'servers' file. That would allow me to put "cmpilato"
in the [global] section of my servers file and yet not have that contradict
creds in the auth cache which use a different username.
In fact, I guess I kinda expected to see code something like this:
creds = check-auth-cache
if (! creds)
creds = check-servers-file
if (! creds)
creds = apr_getusername()
With such an ordering, the servers file effectively becomes just a way to
override the username assumption.
But maybe this discussion was already played out previously and I missed it?
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: [PATCH] Fix issue #2324
Posted by Senthil Kumaran S <se...@collab.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hi,
Senthil Kumaran S wrote:
> I am attaching an updated patch which applies cleanly on r32089 of trunk.
I am attaching a new updated patch which applies cleanly on r32366 of trunk.
[[[
Fix issue #2324.
Disable username assumption.
* subversion/include/svn_config.h
(SVN_CONFIG_OPTION_USERNAME): New config option.
* subversion/libsvn_subr/config_file.c
(svn_config_ensure): Add text about the new config file parameter 'username'.
* subversion/libsvn_subr/simple_providers.c
(simple_first_creds_helper): Fetch username from servers file if it is
defined.
(prompt_for_simple_creds): If we didnt get a password in
simple_first_creds_helper then we need to get the username again here.
Patch by: stylesen
]]]
Thank You.
- --
Senthil Kumaran S
http://www.stylesen.org/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
iD8DBQFIl/St9o1G+2zNQDgRAqaMAKCKswqALIWMqXz9F4T9S0un9Og+cQCeIooQ
ktEDFZlfuqRlNgk0fuf4ZLg=
=u/13
-----END PGP SIGNATURE-----