cargo: build in Cargo's out dir if DENO_BUILD_PATH is not set

Plus some minor improvements and clean-ups:

* Resolve DENO_BUILD_PATH to an absolute path if necessary.
* Rename DENO_BUILD_PATH to GN_OUT_DIR in places where it is supposed to
  be set by the build system (and not a user configuration variable).
* Output Cargo `rerun-if-*-changed` instructions first, so even if the
  build itself fails, configuration changes will still trigger a re-run
  of build.rs.
* Remove TODOs that are impossible.
* Re-run build.rs when the flatbuffer definition file changes.

.appveyor.yml

@@ -8,15 +8,15 @@ clone_depth: 1
 
 environment:
   APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2017
+  CARGO_HOME: $(USERPROFILE)\.cargo
+  CARGO_TARGET_DIR: $(APPVEYOR_BUILD_FOLDER)\target
   DENO_BUILD_MODE: release
   DENO_BUILD_PATH: $(APPVEYOR_BUILD_FOLDER)\out\release
   DENO_THIRD_PARTY_PATH: $(APPVEYOR_BUILD_FOLDER)\third_party
   MTIME_CACHE_DB: $(APPVEYOR_BUILD_FOLDER)\mtime_cache.xml
-  CARGO_HOME: $(USERPROFILE)\.cargo
-  CARGO_TARGET_DIR: $(APPVEYOR_BUILD_FOLDER)\out\target
+  RELEASE_ARTIFACT: deno_win_x64.zip
   RUSTUP_HOME: $(USERPROFILE)\.rustup
   RUST_BACKTRACE: 1
-  RELEASE_ARTIFACT: deno_win_x64.zip
 
   # Appveyor uses 7zip to pack cache directories. We use these options:
   #   -t7z  : Use '7z' format.
@@ -234,6 +234,7 @@ cache:
   # Cache file mtimes in the main git repo, also to enable incremental builds.
   - $(MTIME_CACHE_DB)
   # Build incrementally.
+  - $(CARGO_TARGET_DIR)
   - $(DENO_BUILD_PATH)
 
 init:
@@ -319,6 +320,7 @@ install:
 before_build:
   # Mark all files in the build dir 'not needed' until proven otherwise.
   # TODO: also track files in third_party that aren't checked into the repo.
+  # TODO: also track files in $CARGO_TARGET_DIR.
   - ps: Start-TraceFilesNeeded $env:DENO_BUILD_PATH -Recurse
 
   # Download clang and gn, generate ninja files.

.travis.yml

@@ -14,7 +14,7 @@ env:
   - HOMEBREW_PATH=$HOME/homebrew/
   - DENO_BUILD_ARGS="use_custom_libcxx=false use_sysroot=false"
   - DENO_BUILD_PATH=$HOME/out/Default
-  - CARGO_TARGET_DIR=$DENO_BUILD_PATH
+  - CARGO_TARGET_DIR=$HOME/target
   - DENO_BUILD_MODE=release
   - PATH=$TRAVIS_BUILD_DIR/third_party/llvm-build/Release+Asserts/bin:$CARGO_HOME/bin:$PATH
   - CCACHE_CPP2=yes

build.rs

 // Copyright 2018 the Deno authors. All rights reserved. MIT license.
 
 // Run "cargo build -vv" if you want to see gn output.
-// TODO For the time being you must set an env var DENO_BUILD_PATH
-// which might be `pwd`/out/debug or `pwd`/out/release.
-// TODO Currently DENO_BUILD_PATH must be absolute.
-// TODO Combine DENO_BUILD_PATH and OUT_DIR.
+
+#![deny(warnings)]
 
 use std::env;
-use std::path::PathBuf;
+use std::path::{self, Path, PathBuf};
 use std::process::Command;
 
 fn main() {
+  // Cargo sets PROFILE to either "debug" or "release", which conveniently
+  // matches the build modes we support.
   let mode = env::var("PROFILE").unwrap();
-  let deno_build_path = env::var("DENO_BUILD_PATH").unwrap();
+
+  // Normally we configure GN+Ninja to build into Cargo's OUT_DIR.
+  // However, when DENO_BUILD_PATH is set, perform the ninja build in that dir
+  // instead. This is used by CI to avoid building V8 etc twice.
+  let out_dir = env::var_os("OUT_DIR").unwrap();
+  let gn_out_dir = match env::var_os("DENO_BUILD_PATH") {
+    None => abs_path(out_dir),
+    Some(deno_build_path) => abs_path(deno_build_path),
+  };
+
+  // Give cargo some instructions. We do this first so the `rerun-if-*-changed`
+  // directives can take effect even if something the build itself fails.
+  println!("cargo:rustc-env=GN_OUT_DIR={}", gn_out_dir);
+  println!("cargo:rustc-link-search=native={}/obj", gn_out_dir);
+  println!("cargo:rustc-link-lib=static=deno_deps");
+
+  println!("cargo:rerun-if-changed={}", abs_path("src/msg.fbs"));
+  println!("cargo:rerun-if-env-changed=DENO_BUILD_PATH");
+  // TODO: this is obviously not appropriate here.
+  println!("cargo:rerun-if-env-changed=APPVEYOR_REPO_COMMIT");
 
   // Detect if we're being invoked by the rust language server (RLS).
   // Unfortunately we can't detect whether we're being run by `cargo check`.
@@ -33,21 +52,15 @@ fn main() {
   };
 
   let status = Command::new("python")
-    .env("DENO_BUILD_PATH", &deno_build_path)
+    .env("DENO_BUILD_PATH", &gn_out_dir)
     .env("DENO_BUILD_MODE", &mode)
     .arg("./tools/setup.py")
     .status()
     .expect("setup.py failed");
   assert!(status.success());
 
-  // These configurations must be outputted after tools/setup.py is run.
-  println!("cargo:rustc-link-search=native={}/obj", deno_build_path);
-  println!("cargo:rustc-link-lib=static=deno_deps");
-  // TODO Remove this and only use OUT_DIR at some point.
-  println!("cargo:rustc-env=DENO_BUILD_PATH={}", deno_build_path);
-
   let status = Command::new("python")
-    .env("DENO_BUILD_PATH", &deno_build_path)
+    .env("DENO_BUILD_PATH", &gn_out_dir)
     .env("DENO_BUILD_MODE", &mode)
     .arg("./tools/build.py")
     .arg(gn_target)
@@ -56,3 +69,17 @@ fn main() {
     .expect("build.py failed");
   assert!(status.success());
 }
+
+// Utility function to make a path absolute, normalizing it to use forward
+// slashes only. The returned value is an owned String, otherwise panics.
+fn abs_path<P: AsRef<Path>>(path: P) -> String {
+  env::current_dir()
+    .unwrap()
+    .join(path)
+    .to_str()
+    .unwrap()
+    .to_owned()
+    .chars()
+    .map(|c| if path::is_separator(c) { '/' } else { c })
+    .collect()
+}

build_extra/rust/run.py

 #!/usr/bin/env python
-# This file just executes its arguments, except that also adds OUT_DIR to the
+# This file just executes its arguments, except that also adds GN_OUT_DIR to the
 # environ. This is for compatibility with cargo.
 import subprocess
 import sys
 import os
 
-# TODO This is for src/msg.rs to know where to find msg_generated.rs
-# In the future we should use OUT_DIR here.
-os.environ["DENO_BUILD_PATH"] = os.path.abspath(".")
-assert os.path.isdir(os.environ["DENO_BUILD_PATH"])
+# This is for src/msg.rs to know where to find msg_generated.rs.
+# When building with Cargo this variable is set by build.rs.
+os.environ["GN_OUT_DIR"] = os.path.abspath(".")
+assert os.path.isdir(os.environ["GN_OUT_DIR"])
 
-sys.exit(subprocess.call(sys.argv[1:], env=os.environ))
+sys.exit(subprocess.call(sys.argv[1:]))

src/msg.rs

 #![allow(unused_imports)]
 #![allow(dead_code)]
 use flatbuffers;
-// TODO Replace DENO_BUILD_PATH with OUT_DIR. gn/ninja should generate into
-// the same output directory as cargo uses.
-include!(concat!(env!("DENO_BUILD_PATH"), "/gen/msg_generated.rs"));
+// GN_OUT_DIR is set either by build.rs (for the Cargo build), or by
+// build_extra/rust/run.py (for the GN+Ninja build).
+include!(concat!(env!("GN_OUT_DIR"), "/gen/msg_generated.rs"));