3975: Incorrect detection of `HOME` under Windows

VZ

What version are you running?

0.7.4

What steps will reproduce the problem?

Run e.g. rbt diff, notice that it ignores the contents of $HOME/.reviewboardrc such as e.g. GIT_USE_EXT_DIFF = True option.

What is the expected output? What do you see instead?

I'd strongly expect it to use ~/.reviewboardrc, especially as I'm using the Cygwin Python (but actually even with the native Windows Python too), and not $APPDATA/.reviewboardrc.

What operating system are you using?

Windows 7

Please provide any additional information below.

Please fix this by exchanging the order of checks for HOME and APPDATA in get_home_path() in utils/filesystem.py, preferring the always defined APPDATA and ignoring HOME (which is unlikely to be defined accidentally) is just wrong.

david
#1 david
  • +EasyFix
chipx86
#2 chipx86

Since you have a good repro case for this, would you be up for writing a patch?

  • -New
    +NeedInfo
  • +Release-0.7.x
  • +OpSys:Windows
#3 VZ

Sure, as I said, it's a trivial change, here it is.

  • +
    commit 72045b94d2f99ea36f6a8fdaf63bae9990bb7528
    Author: Vadim Zeitlin <vadim@zeitlins.org>
    Date:   Tue Sep 29 14:05:43 2015 +0200
        Prefer HOME to APPDATA when detecting user home directory
        
        This ensures that HOME can be used under Windows, otherwise it was always
        ignored in favour of APPDATA which is always defined under this platform,
        unlike HOME which is only going to be defined if the user wants to use it.
        
        Signed-off-by: Vadim Zeitlin <vadim@zeitlins.org>
    diff --git a/rbtools/utils/filesystem.py b/rbtools/utils/filesystem.py
    index 8ab0b1e..4578435 100644
    --- a/rbtools/utils/filesystem.py
    +++ b/rbtools/utils/filesystem.py
    @@ -109,10 +109,10 @@ def walk_parents(path):
     
     def get_home_path():
         """Retrieve the homepath."""
    -    if 'APPDATA' in os.environ:
    -        return os.environ['APPDATA']
    -    elif 'HOME' in os.environ:
    +    if 'HOME' in os.environ:
             return os.environ['HOME']
    +    elif 'APPDATA' in os.environ:
    +        return os.environ['APP
david
#4 david

Thank you! We typically require patches to be posted on https://reviews.reviewboard.org/, but since this one is so simple, I'll just push it.

Fixed in release-0.7.x (8df5e56). Thanks!

  • -NeedInfo
    +Fixed