[tfs] Allow overriding tf.cmd location

Review Request #7999 — Created Feb. 25, 2016 and submitted

bmartin
RBTools
master
rbtools
Allow an environment variable to specify location of tf.cmd for rbtools.
This will allow tf commands from visual studio 2013+ to work alongside
an installation of Team Explorer Everywhere.

Before:

C:\>rbt post -d
>>> RBTools 0.8 alpha 0 (dev)
>>> Python 2.7.11 (v2.7.11:6d1b6a68f775, Dec  5 2015, 20:32:19) [MSC v.1500 32 bit (Intel)]
>>> Running on Windows-10-10.0.10586
Checking location "%RBT_TF_PATH%\tf.cmd"
Caught OSError: [Error 2] The system cannot find the file specified

After:

C:\>rbt post -d
>>> RBTools 0.8 alpha 0 (dev)
>>> Python 2.7.11 (v2.7.11:6d1b6a68f775, Dec  5 2015, 20:32:19) [MSC v.1500 32 bit (Intel)]
>>> Running on Windows-10-10.0.10586
Checking location C:\Program Files (x86)\TEE-CLC-14.0.2\tf.cmd
Using location C:\Program Files (x86)\TEE-CLC-14.0.2\tf.cmd
Description From Last Updated

These shouldn't be one sentence per line.

brenniebrennie

Perhaps this should be exposed as a .reviewboardrc option instead?

brenniebrennie

Blank line between these.

brenniebrennie

The config in __init__ is from load_config :) We'll also want to use options (also included above) instead, see my ...

brenniebrennie

I believe we'll want to add an option group to rbtools.commands.Command, similar to perforce_options circa line 435. That way, we ...

brenniebrennie

tf_locations.extend Otherwise, your array will look like ['my-custom-path', ['tf.cmd', '...']] which is not what you were intending.

brenniebrennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        rbtools/clients/tfs.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        rbtools/clients/tfs.py
    
    
  2. 
      
SM
  1. 
      
  2. rbtools/clients/tfs.py (Diff revision 1)
     
     

    This environmental variable gets evaluated by the shell, when check_install gets run, right? Because since it's a raw string, I think Python won't do it.

    1. Yep, exactly. The same way the two installation locations are checked.

    2. Well, I mean, not the exact same way - there's no %ENV_VARIABLE% getting evaluated in the other cases! - but yeah, I'm satisfied. Thumbs-up.

    3. r'%programfiles(x86)%\Microsoft Visual Studio 12.0\Common7\IDE\tf.cmd',
      r'%programfiles%\Microsoft Team Foundation Server 12.0\Tools\tf.cmd',
      

      %programfiles(x86)% and %programfiles% are environment variables that, for the large majority of people, will resolve to C:\Program Files (x86) and C:\Program Files respectively.

      C:\>echo %programfiles(x86)%
      C:\Program Files (x86)
      
      C:\>echo %programfiles%
      C:\Program Files
      

      Thanks for the review and the thumbs-up! ^_^

      So this is my first submission, do I need another review, just commit, or do I create a master branch pull request?

  3. 
      
BM
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        rbtools/clients/tfs.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        rbtools/clients/tfs.py
    
    
  2. 
      
brennie
  1. 
      
  2. rbtools/clients/tfs.py (Diff revision 2)
     
     
     
     

    These shouldn't be one sentence per line.

  3. rbtools/clients/tfs.py (Diff revision 2)
     
     
     

    Perhaps this should be exposed as a .reviewboardrc option instead?

    1. I'm not sure about this. In general, .reviewboardrc contains the settings around a RB Repository and a local workspace of the same repo, right? However the installation location for Team Explorer Everywhere is very likely to be quite different from machine to machine (It's just an unzipped directory, there is no installer for it).

    2. That is true, but we also support .reviewboardrc in higher up directories, so it seems like ~/.reviewboardrc would be a good place for this (or whatever the equivalent would be in windows with %HOME%\.reviewboardrc)

    3. Ahh, cool. I'll go ahead and make the change.

  4. rbtools/clients/tfs.py (Diff revision 2)
     
     
     

    Blank line between these.

  5. 
      
BM
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        rbtools/clients/tfs.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        rbtools/clients/tfs.py
    
    
  2. 
      
BM
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        rbtools/clients/tfs.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        rbtools/clients/tfs.py
    
    
  2. 
      
brennie
  1. 
      
  2. rbtools/clients/tfs.py (Diff revision 4)
     
     
     
     

    The config in __init__ is from load_config :)

    We'll also want to use options (also included above) instead, see my notes below.

  3. rbtools/clients/tfs.py (Diff revision 4)
     
     
     

    I believe we'll want to add an option group to rbtools.commands.Command, similar to perforce_options circa line 435.

    That way, we can also specify, e.g. --tf-path on the command line, and we can get a named options.

    Then we can do:

    if options.tf_cmd_path:
        tf_locations.append(options.tf_cmd_path)
    
  4. rbtools/clients/tfs.py (Diff revision 4)
     
     
     
     
     
     

    tf_locations.extend

    Otherwise, your array will look like

    ['my-custom-path', ['tf.cmd', '...']]
    

    which is not what you were intending.

    1. Sorry for the churn. My python-fu is rudimentary at best. ^_^

  5. 
      
BM
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        rbtools/clients/tfs.py
        rbtools/commands/__init__.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        rbtools/clients/tfs.py
        rbtools/commands/__init__.py
    
    
  2. 
      
BM
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        rbtools/clients/tfs.py
        rbtools/commands/__init__.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        rbtools/clients/tfs.py
        rbtools/commands/__init__.py
    
    
  2. 
      
david
  1. Looks good! Thanks so much for doing this!

    1. Cool, thanks for the review! ^_^

  2. 
      
BM
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-0.7.x (847a34e)
Loading...