Provide a way to name a solution as part of the config command.

Provide a way to name a solution as part of the config command.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=43847

[iposva, 2010-04-06 21:25:46]

Fixed a TODO(darin) which described exactly the functionality I was missing.

-Ivan

[darin (slow to review), 2010-04-07 05:39:38]

I would feel more comfortable having maruel review this since it has been a
very long time since I hacked on gclient.
-Darin

On Tue, Apr 6, 2010 at 2:25 PM, <iposva@chromium.org> wrote:

> Reviewers: darin,
>
> Message:
> Fixed a TODO(darin) which described exactly the functionality I was
> missing.
>
> -Ivan
>
>
> Description:
> - Provide a way to name a solution as part of the config command.
>
>
> Please review this at http://codereview.chromium.org/1611009
>
> SVN Base: svn://chrome-svn/chrome/trunk/tools/depot_tools/
>
> Affected files:
>  M     gclient.py
>  M     tests/gclient_test.py
>
>
> Index: gclient.py
> ===================================================================
> --- gclient.py  (revision 43759)
> +++ gclient.py  (working copy)
> @@ -153,11 +153,13 @@
>  top-level DEPS files in each module are read to determine dependent
>  modules to operate on as well.  If optional [url] parameter is
>  provided, then configuration is read from a specified Subversion server
> -URL.  Otherwise, a --spec option must be provided.
> +URL.  Otherwise, a --spec option must be provided.  A --name option
> overrides
> +the default name for the solutions.
>
>  usage: config [option | url] [safesync url]
>
>  Valid options:
> +  --name path           : alternate relative path for the solution
>   --spec=GCLIENT_SPEC   : contents of .gclient are read from string
> parameter.
>                           *Note that due to Cygwin/Python brokenness, it
>                           probably can't contain any newlines.*
> @@ -165,6 +167,7 @@
>  Examples:
>   gclient config https://gclient.googlecode.com/svn/trunk/gclient
>       configure a new client to check out gclient.py tool sources
> +  gclient config --name tools
> https://gclient.googlecode.com/svn/trunk/gclient
>   gclient config --spec='solutions=[{"name":"gclient","""
>     '"url":"https://gclient.googlecode.com/svn/trunk/gclient",'
>     '"custom_deps":{}}]',
> @@ -959,10 +962,12 @@
>   if options.spec:
>     client.SetConfig(options.spec)
>   else:
> -    # TODO(darin): it would be nice to be able to specify an alternate
> relpath
> -    # for the given URL.
>     base_url = args[0].rstrip('/')
> -    name = base_url.split("/")[-1]
> +    if not options.name:
> +      name = base_url.split("/")[-1]
> +    else:
> +      # specify an alternate relpath for the given URL.
> +      name = options.name
>     safesync_url = ""
>     if len(args) > 1:
>       safesync_url = args[1]
> @@ -1204,6 +1209,8 @@
>   option_parser.add_option("", "--snapshot", action="store_true",
> default=False,
>                            help=("(revinfo only), create a snapshot file "
>                                  "of the current version of all
> repositories"))
> +  option_parser.add_option("", "--name", default=None,
> +                           help=("specify alternate relative solution
> path"))
>   option_parser.add_option("", "--gclientfile", default=None,
>                            metavar="FILENAME",
>                            help=("specify an alternate .gclient file"))
> Index: tests/gclient_test.py
> ===================================================================
> --- tests/gclient_test.py       (revision 43759)
> +++ tests/gclient_test.py       (working copy)
> @@ -58,6 +58,7 @@
>                  deps_file='a_deps_file_name', force=False, nohooks=False):
>       self.verbose = verbose
>       self.spec = spec
> +      self.name = None
>       self.config_filename = config_filename
>       self.entries_filename = entries_filename
>       self.deps_file = deps_file
>
>
>

[darin (slow to review), 2010-04-07 06:21:12]

LGTM, but I would like to have maruel approve this CL before you commit it.  He
might have strong opinions.

[M-A Ruel, 2010-04-07 13:17:27]

lgtm with three nits.

nit: remove the dash from the description.

http://codereview.chromium.org/1611009/diff/4001/5001
File gclient.py (right):

http://codereview.chromium.org/1611009/diff/4001/5001#newcode1212
gclient.py:1212: option_parser.add_option("", "--name", default=None,
nit: default=None is the default so no need to specify it.

http://codereview.chromium.org/1611009/diff/4001/5001#newcode1213
gclient.py:1213: help=("specify alternate relative solution path"))
nit: No need to put the string in ().

[iposva, 2010-04-07 17:20:04]

Addressed review comments.

-Ivan

http://codereview.chromium.org/1611009/diff/4001/5001
File gclient.py (right):

http://codereview.chromium.org/1611009/diff/4001/5001#newcode1212
gclient.py:1212: option_parser.add_option("", "--name", default=None,
On 2010/04/07 13:17:28, Marc-Antoine Ruel wrote:
> nit: default=None is the default so no need to specify it.

Done.

http://codereview.chromium.org/1611009/diff/4001/5001#newcode1213
gclient.py:1213: help=("specify alternate relative solution path"))
On 2010/04/07 13:17:28, Marc-Antoine Ruel wrote:
> nit: No need to put the string in ().

Done. I just tried to follow the style of the surrounding code in both these
cases.

[M-A Ruel, 2010-04-07 17:41:45]

lgtm