Make 'tools/build.py -h' warn the user to run it from the repo root.

Make 'tools/build.py -h' warn the user to run it from the repo root.

The script by default runs 'make all' in the current directory. Since it's common to have a Makefile with an 'all' rule, this can lead to silent failure when a user runs the script from outside the repo root while intending
to do a default build. That behavior is error prone: in my case it made my regression tests pass when they should fail, because I was not actually rebuilding when I thought I was.

It turns out that the build script is sometimes run outside the repo root on purpose, so we can't simply require it be run from the repo root.  So, we document the relative nature of the build script.

R=ricow@google.com, jwren@google.com
BUG= https://code.google.com/p/dart/issues/detail?id=20958

Committed: https://code.google.com/p/dart/source/detail?r=40465

[Nathan Collins, 2014-09-15 20:23:26]

Added @zra and @ricow as reviewers per 'git blame' and @jwren suggestion.

[zra, 2014-09-15 20:31:39]

zra@google.com changed reviewers:
+ zra@google.com

[zra, 2014-09-15 20:31:40]

https://codereview.chromium.org/569343003/diff/1/tools/build.py
File tools/build.py (right):

https://codereview.chromium.org/569343003/diff/1/tools/build.py#newcode519
tools/build.py:519: os.chdir(repo_root)
instead maybe:
if os.getcwd() != utils.DART_DIR:
  os.chdir(utils.DART_DIR)

[Nathan Collins, 2014-09-16 01:05:08]

PTAL.

https://codereview.chromium.org/569343003/diff/1/tools/build.py
File tools/build.py (right):

https://codereview.chromium.org/569343003/diff/1/tools/build.py#newcode519
tools/build.py:519: os.chdir(repo_root)
On 2014/09/15 20:31:40, zra wrote:
> instead maybe:
> if os.getcwd() != utils.DART_DIR:
>   os.chdir(utils.DART_DIR)

Yes, that's much simpler! I don't see the point of checking if we need to cd
before cd-ing, so I also dropped the check and went with simply:

os.chdir(utils.DART_DIR)

[ricow1, 2014-09-16 08:11:29]

ricow@google.com changed reviewers:
+ ricow@google.com

[ricow1, 2014-09-16 08:11:29]

https://codereview.chromium.org/569343003/diff/20001/tools/build.py
File tools/build.py (right):

https://codereview.chromium.org/569343003/diff/20001/tools/build.py#newcode516
tools/build.py:516: os.chdir(utils.DART_DIR)
to be nice and change back you could do:
with utils.ChangedWorkingDirectory(utils.DART_DIR):
  utils.ConfigureJava()
  ....


When you exit the with block (either finish or error) the actual running cwd
will be restored

[Nathan Collins, 2014-09-16 17:38:57]

PTAL.

Commit message for latest patch set:

    Make fix more conservative: simply fail on wrong cwd.
    
    It turns out that some of the command line options can be paths.
    If those paths are relative the cwd, and the cwd is different
    from the repo root, then cd-ing into the repo root will
    break the relative paths.  An easy fix is to make any
    relative paths absolute before cd-ing, but I'm not
    prepared to test on Windows or Mac, so I'm going with
    a simpler but less user friendly fix: simply complain loudly
    if the cwd is not the repo root.

https://codereview.chromium.org/569343003/diff/20001/tools/build.py
File tools/build.py (right):

https://codereview.chromium.org/569343003/diff/20001/tools/build.py#newcode516
tools/build.py:516: os.chdir(utils.DART_DIR)
On 2014/09/16 08:11:29, ricow1 wrote:
> to be nice and change back you could do:
> with utils.ChangedWorkingDirectory(utils.DART_DIR):
>   utils.ConfigureJava()
>   ....
> 
> 
> When you exit the with block (either finish or error) the actual running cwd
> will be restored

This couldn't hurt, but the cd was happening in the beginning of 'Main', so I
expect the script would exit right before cd-ing back anway.  However, I
realized it's not so simple to fix this bug: the command line options may
include relative paths!

[Ivan Posva, 2014-09-17 03:48:54]

iposva@google.com changed reviewers:
+ iposva@google.com

[Ivan Posva, 2014-09-17 03:48:55]

-Ivan

https://codereview.chromium.org/569343003/diff/40001/tools/build.py
File tools/build.py (right):

https://codereview.chromium.org/569343003/diff/40001/tools/build.py#newcode518
tools/build.py:518: if os.path.abspath(utils.DART_DIR) !=
os.path.abspath(os.getcwd()):
This is not entirely correct. Sometimes you need to run the build.py script from
within a subdirectory of the dart repo.

[Nathan Collins, 2014-09-17 19:31:02]

On 2014/09/17 03:48:55, Ivan Posva wrote:
> -Ivan
> 
> https://codereview.chromium.org/569343003/diff/40001/tools/build.py
> File tools/build.py (right):
> 
> https://codereview.chromium.org/569343003/diff/40001/tools/build.py#newcode518
> tools/build.py:518: if os.path.abspath(utils.DART_DIR) !=
> os.path.abspath(os.getcwd()):
> This is not entirely correct. Sometimes you need to run the build.py script
from
> within a subdirectory of the dart repo.

Hmmm, in that there's not much we can do? I could add a note to the '--help'
output which says that the script tries to run 'make <target>' in the current
directory, so you shouldn't run it from outside the repo root unless you mean
to.

I had thought something more subtle was going on with this "bug", since the
directory I was running 'build.py' from didn't contain a Makefile: it turns out
it contained a shell script called 'all.sh', and that make has an implicit rule
that generates '<target>' from '<target>.sh' :P  So, I guess I'm just very
unlucky.

[Nathan Collins, 2014-09-17 19:54:23]

PTAL.

I reverted the forced-repo-root-cwd and added a help message.

[ricow1, 2014-09-18 05:45:14]

lgtm

[Nathan Collins, 2014-09-18 21:22:47]

Committed patchset #4 manually as 40465 (presubmit successful).