fix: devserver not working on windows
refactor: remove unnecessary runfile stat
Closes #385
PiperOrigin-RevId: 233100030
diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml
index f587cc9..5b60a2c 100644
--- a/.bazelci/presubmit.yml
+++ b/.bazelci/presubmit.yml
@@ -52,6 +52,4 @@
- "--action_env=PATH"
- "--test_env=PATH"
test_targets:
- - "--"
- "..."
- - "-//devserver/devserver:go_default_test"
diff --git a/devserver/devserver/devserver.go b/devserver/devserver/devserver.go
index 4748908..9bd3a86 100644
--- a/devserver/devserver/devserver.go
+++ b/devserver/devserver/devserver.go
@@ -97,13 +97,7 @@
func CreateFileHandler(servingPath, manifest string, pkgs []string, base string) http.HandlerFunc {
// We want to add the root runfile path because by default developers should be able to request
// runfiles through their absolute manifest path (e.g. "my_workspace_name/src/file.css")
- // We use the empty string package because of the different algorithm for
- // file resolution used internally and externally.
- pkgPaths := dirHTTPFileSystem{[]string{"./"}, base}
- for _, pkg := range pkgs {
- pkgPaths.files = append(pkgPaths.files, pathReplacer.Replace(pkg))
- }
- pkgPaths.files = append(pkgPaths.files, base)
+ pkgPaths := dirHTTPFileSystem{append(pkgs, "./"), base}
fileHandler := http.FileServer(pkgPaths).ServeHTTP
@@ -127,8 +121,8 @@
userIndexFile, err := runfiles.Runfile(base, pathReplacer.Replace(filepath.Join(pkg, "index.html")))
// In case the potential user index file couldn't be found in the runfiles,
- // just continue searching.
- if _, statErr := os.Stat(userIndexFile); err != nil || statErr != nil {
+ // continue searching within other packages.
+ if err != nil {
continue
}
@@ -175,34 +169,40 @@
// dirHTTPFileSystem implements http.FileSystem by looking in the list of dirs one after each other.
type dirHTTPFileSystem struct {
- files []string
+ packageDirs []string
base string
}
func (fs dirHTTPFileSystem) Open(name string) (http.File, error) {
- for _, packageName := range fs.files {
- filePackageName := filepath.Join(packageName, name)
- realFilePath, err := runfiles.Runfile(fs.base, filePackageName)
- stat, statErr := os.Stat(realFilePath)
+ for _, packageName := range fs.packageDirs {
+ manifestFilePath := filepath.Join(packageName, name)
+ realFilePath, err := runfiles.Runfile(fs.base, manifestFilePath)
- if err != nil || statErr != nil {
+ if err != nil {
// In case the runfile could not be found, we also need to check that the requested
// path does not refer to a directory containing an "index.html" file. This can
// happen if Bazel runs without runfile symlinks, where only files can be resolved
// from the manifest. In that case we dirty check if there is a "index.html" file.
- realFilePath, err = runfiles.Runfile(fs.base, filepath.Join(filePackageName, "index.html"))
+ realFilePath, err = runfiles.Runfile(fs.base, filepath.Join(manifestFilePath, "index.html"))
- // Continue searching if the runfile couldn't be found for the request filed.
- if _, statErr := os.Stat(realFilePath); err != nil || statErr != nil {
+ // Continue searching if the runfile couldn't be found for the requested file.
+ if err != nil {
continue
}
}
+ stat, err := os.Stat(realFilePath)
+ if err != nil {
+ // This should actually never happen because runfiles resolved through the runfile helpers
+ // should always exist. Just in order to properly handle the error, we add this error handling.
+ return nil, fmt.Errorf("could not read runfile %s", manifestFilePath)
+ }
+
// In case the resolved file resolves to a directory. This can only happen if
// Bazel runs with symlinked runfiles (e.g. on MacOS, linux). In that case, we
// just look for a index.html in the directory.
if stat.IsDir() {
- realFilePath, err = runfiles.Runfile(fs.base, filepath.Join(filePackageName, "index.html"))
+ realFilePath, err = runfiles.Runfile(fs.base, filepath.Join(manifestFilePath, "index.html"))
// In case the index.html file of the requested directory couldn't be found,
// we just continue searching.
diff --git a/devserver/main.go b/devserver/main.go
index 29c224b..7b0a2a0 100644
--- a/devserver/main.go
+++ b/devserver/main.go
@@ -8,15 +8,17 @@
"io/ioutil"
"net/http"
"os"
- "path/filepath"
"strings"
"github.com/bazelbuild/rules_typescript/devserver/concatjs"
"github.com/bazelbuild/rules_typescript/devserver/devserver"
+ "github.com/bazelbuild/rules_typescript/devserver/runfiles"
)
var (
port = flag.Int("port", 5432, "server port to listen on")
+ // The "base" CLI flag is only kept because within Google3 because removing would be a breaking change due to
+ // ConcatJS and "devserver/devserver.go" still respecting the specified base flag.
base = flag.String("base", "", "server base (required, runfiles of the binary)")
pkgs = flag.String("packages", "", "root package(s) to serve, comma-separated")
manifest = flag.String("manifest", "", "sources manifest (.MF)")
@@ -28,23 +30,30 @@
func main() {
flag.Parse()
- if *base == "" || len(*pkgs) == 0 || (*manifest == "") || (*scriptsManifest == "") {
+ if len(*pkgs) == 0 || (*manifest == "") || (*scriptsManifest == "") {
fmt.Fprintf(os.Stderr, "Required argument not set\n")
os.Exit(1)
}
- if _, err := os.Stat(*base); err != nil {
- fmt.Fprintf(os.Stderr, "Cannot read server base %s: %v\n", *base, err)
+ manifestPath, err := runfiles.Runfile(*base, *scriptsManifest)
+
+ if err != nil {
+ fmt.Fprintf(os.Stderr, "Failed to find scripts_manifest in runfiles: %v\n", err)
os.Exit(1)
}
- scriptsManifestPath := filepath.Join(*base, *scriptsManifest)
- scriptFiles, err := manifestFiles(scriptsManifestPath)
+ scriptFiles, err := manifestFiles(manifestPath)
if err != nil {
fmt.Fprintf(os.Stderr, "Failed to read scripts_manifest: %v\n", err)
os.Exit(1)
}
+ if !strings.HasPrefix(*servingPath, "/") {
+ fmt.Fprintf(os.Stderr, "The specified serving_path does not start with a slash. "+
+ "This causes the serving path to not have any effect.\n")
+ os.Exit(1)
+ }
+
preScripts := make([]string, 0, 100)
postScripts := make([]string, 0, 1)
@@ -76,7 +85,12 @@
// the requirejs script which is added to scriptFiles by the devserver
// skylark rule.
for _, v := range scriptFiles {
- js, err := loadScript(filepath.Join(*base, v))
+ runfile, err := runfiles.Runfile(*base, v)
+ if err != nil {
+ fmt.Fprintf(os.Stderr, "Could not find runfile %s, got error %s", v, err)
+ }
+
+ js, err := loadScript(runfile)
if err != nil {
fmt.Fprintf(os.Stderr, "Failed to read script %s: %v\n", v, err)
} else {
@@ -90,7 +104,8 @@
postScripts = append(postScripts, fmt.Sprintf("require([\"%s\"]);", *entryModule))
}
- http.Handle(*servingPath, concatjs.ServeConcatenatedJS(*manifest, *base, preScripts, postScripts, nil /* realFileSystem */))
+ http.Handle(*servingPath, concatjs.ServeConcatenatedJS(*manifest, *base, preScripts, postScripts,
+ &RunfileFileSystem{}))
pkgList := strings.Split(*pkgs, ",")
http.HandleFunc("/", devserver.CreateFileHandler(*servingPath, *manifest, pkgList, *base))
diff --git a/devserver/runfile-filesystem.go b/devserver/runfile-filesystem.go
index 3f99c2f..090ded6 100644
--- a/devserver/runfile-filesystem.go
+++ b/devserver/runfile-filesystem.go
@@ -30,6 +30,6 @@
// ResolvePath resolves the specified path within a given root using Bazel's runfile resolution.
// This is necessary because on Windows, runfiles are not symlinked and need to be
// resolved using the runfile manifest file.
-func (fs *RunfileFileSystem) ResolvePath(root string, file string) (string, error) {
- return runfiles.Runfile(root, file)
+func (fs *RunfileFileSystem) ResolvePath(root string, manifestFilePath string) (string, error) {
+ return runfiles.Runfile(root, manifestFilePath)
}
diff --git a/devserver/runfiles/runfiles.go b/devserver/runfiles/runfiles.go
index 1d03448..cff9acc 100644
--- a/devserver/runfiles/runfiles.go
+++ b/devserver/runfiles/runfiles.go
@@ -4,7 +4,7 @@
import "github.com/bazelbuild/rules_go/go/tools/bazel"
// Runfile returns the base directory to the bazel runfiles
-func Runfile(_ string, path string) (string, error) {
- return bazel.Runfile(path)
+func Runfile(_base string, manifestPath string) (string, error) {
+ return bazel.Runfile(manifestPath)
}
diff --git a/internal/devserver/BUILD b/internal/devserver/BUILD
index d6e9733..2dfa86b 100644
--- a/internal/devserver/BUILD
+++ b/internal/devserver/BUILD
@@ -19,6 +19,7 @@
])
exports_files([
+ "launcher_template.sh",
# Exported to be consumed for generating skydoc.
"ts_devserver.bzl",
])
diff --git a/internal/devserver/launcher_template.sh b/internal/devserver/launcher_template.sh
new file mode 100644
index 0000000..13722a9
--- /dev/null
+++ b/internal/devserver/launcher_template.sh
@@ -0,0 +1,59 @@
+#!/bin/bash
+
+# Copyright 2018 The Bazel Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+set -e
+
+# --- begin runfiles.bash initialization ---
+# Source the runfiles library:
+# https://github.com/bazelbuild/bazel/blob/master/tools/bash/runfiles/runfiles.bash
+# The runfiles library defines rlocation, which is a platform independent function
+# used to lookup the runfiles locations. This code snippet is needed at the top
+# of scripts that use rlocation to lookup the location of runfiles.bash and source it
+if [[ ! -d "${RUNFILES_DIR:-/dev/null}" && ! -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then
+ if [[ -f "$0.runfiles_manifest" ]]; then
+ export RUNFILES_MANIFEST_FILE="$0.runfiles_manifest"
+ elif [[ -f "$0.runfiles/MANIFEST" ]]; then
+ export RUNFILES_MANIFEST_FILE="$0.runfiles/MANIFEST"
+ elif [[ -f "$0.runfiles/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then
+ export RUNFILES_DIR="$0.runfiles"
+ fi
+fi
+if [[ -f "${RUNFILES_DIR:-/dev/null}/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then
+ source "${RUNFILES_DIR}/bazel_tools/tools/bash/runfiles/runfiles.bash"
+elif [[ -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then
+ source "$(grep -m1 "^bazel_tools/tools/bash/runfiles/runfiles.bash " \
+ "$RUNFILES_MANIFEST_FILE" | cut -d ' ' -f 2-)"
+else
+ echo >&2 "ERROR: cannot find @bazel_tools//tools/bash/runfiles:runfiles.bash"
+ exit 1
+fi
+# --- end runfiles.bash initialization ---
+
+
+readonly main=$(rlocation "TEMPLATED_main")
+readonly manifest=$(rlocation "TEMPLATED_manifest")
+readonly scripts_manifest=$(rlocation "TEMPLATED_scripts_manifest")
+
+# Workaround for https://github.com/bazelbuild/bazel/issues/6764
+# If this issue is incorporated into Bazel, the workaround here should be removed.
+MSYS2_ARG_CONV_EXCL="*" "${main}" \
+ -packages=TEMPLATED_packages \
+ -serving_path=TEMPLATED_serving_path \
+ -entry_module=TEMPLATED_entry_module \
+ -port=TEMPLATED_port \
+ -manifest="${manifest}" \
+ -scripts_manifest="${scripts_manifest}" \
+ "$@"
diff --git a/internal/devserver/ts_devserver.bzl b/internal/devserver/ts_devserver.bzl
index 3fd9519..fe377ec 100644
--- a/internal/devserver/ts_devserver.bzl
+++ b/internal/devserver/ts_devserver.bzl
@@ -24,6 +24,14 @@
"html_asset_inject",
)
+# Helper function to convert a short path to a path that is
+# found in the MANIFEST file.
+def _short_path_to_manifest_path(ctx, short_path):
+ if short_path.startswith("../"):
+ return short_path[3:]
+ else:
+ return ctx.workspace_name + "/" + short_path
+
def _ts_devserver(ctx):
files = depset()
for d in ctx.attr.deps:
@@ -52,7 +60,7 @@
amd_names_shim = ctx.actions.declare_file(
"_%s.amd_names_shim.js" % ctx.label.name,
- sibling = ctx.outputs.executable,
+ sibling = ctx.outputs.script,
)
write_amd_names_shim(ctx.actions, amd_names_shim, ctx.attr.bootstrap)
@@ -76,6 +84,7 @@
]
devserver_runfiles += ctx.files.static_files
devserver_runfiles += script_files
+ devserver_runfiles += ctx.files._bash_runfile_helpers
if ctx.file.index_html:
injected_index = ctx.actions.declare_file("index.html")
@@ -96,37 +105,24 @@
)
devserver_runfiles += [injected_index]
- serving_arg = ""
- if ctx.attr.serving_path:
- serving_arg = "-serving_path=%s" % ctx.attr.serving_path
-
packages = depset(["/".join([workspace_name, ctx.label.package])] + ctx.attr.additional_root_paths)
- # FIXME: more bash dependencies makes Windows support harder
- ctx.actions.write(
- output = ctx.outputs.executable,
+ ctx.actions.expand_template(
+ template = ctx.file._launcher_template,
+ output = ctx.outputs.script,
+ substitutions = {
+ "TEMPLATED_entry_module": ctx.attr.entry_module,
+ "TEMPLATED_main": _short_path_to_manifest_path(ctx, ctx.executable._devserver.short_path),
+ "TEMPLATED_manifest": _short_path_to_manifest_path(ctx, ctx.outputs.manifest.short_path),
+ "TEMPLATED_packages": ",".join(packages.to_list()),
+ "TEMPLATED_port": str(ctx.attr.port),
+ "TEMPLATED_scripts_manifest": _short_path_to_manifest_path(ctx, ctx.outputs.scripts_manifest.short_path),
+ "TEMPLATED_serving_path": ctx.attr.serving_path if ctx.attr.serving_path else "",
+ "TEMPLATED_workspace": workspace_name,
+ },
is_executable = True,
- content = """#!/bin/sh
-RUNFILES="$PWD/.."
-{main} {serving_arg} \
- -base="$RUNFILES" \
- -packages={packages} \
- -manifest={workspace}/{manifest} \
- -scripts_manifest={workspace}/{scripts_manifest} \
- -entry_module={entry_module} \
- -port={port} \
- "$@"
-""".format(
- main = ctx.executable._devserver.short_path,
- serving_arg = serving_arg,
- workspace = workspace_name,
- packages = ",".join(packages.to_list()),
- manifest = ctx.outputs.manifest.short_path,
- scripts_manifest = ctx.outputs.scripts_manifest.short_path,
- entry_module = ctx.attr.entry_module,
- port = str(ctx.attr.port),
- ),
)
+
return [DefaultInfo(
runfiles = ctx.runfiles(
files = devserver_runfiles,
@@ -191,6 +187,7 @@
allow_files = True,
aspects = [sources_aspect],
),
+ "_bash_runfile_helpers": attr.label(default = Label("@bazel_tools//tools/bash/runfiles")),
"_devserver": attr.label(
# For local development in rules_typescript, we build the devserver from sources.
# This requires that we have the go toolchain available.
@@ -206,33 +203,52 @@
executable = True,
cfg = "host",
),
+ "_launcher_template": attr.label(allow_single_file = True, default = Label("//internal/devserver:launcher_template.sh")),
"_requirejs_script": attr.label(allow_single_file = True, default = Label("@build_bazel_rules_typescript_devserver_deps//node_modules/requirejs:require.js")),
},
outputs = {
"manifest": "%{name}.MF",
+ "script": "%{name}.sh",
"scripts_manifest": "scripts_%{name}.MF",
},
- executable = True,
)
"""ts_devserver is a simple development server intended for a quick "getting started" experience.
Additional documentation at https://github.com/alexeagle/angular-bazel-example/wiki/Running-a-devserver-under-Bazel
"""
-def ts_devserver_macro(tags = [], **kwargs):
- """ibazel wrapper for `ts_devserver`
+def ts_devserver_macro(name, data = [], args = [], visibility = None, tags = [], testonly = 0, **kwargs):
+ """Macro for creating a `ts_devserver`
- This macro re-exposes the `ts_devserver` rule with some extra tags so that
- it behaves correctly under ibazel.
-
+ This macro re-exposes a `sh_binary` and `ts_devserver` target that can run the
+ actual devserver implementation.
+ The `ts_devserver` rule is just responsible for generating a launcher script
+ that runs the Go devserver implementation. The `sh_binary` is the primary
+ target that matches the specified "name" and executes the generated bash
+ launcher script.
This is re-exported in `//:defs.bzl` as `ts_devserver` so if you load the rule
from there, you actually get this macro.
-
Args:
- tags: standard Bazel tags, this macro adds a couple for ibazel
+ name: Name of the devserver target
+ data: Runtime dependencies for the devserver
+ args: Command line arguments that will be passed to the devserver Go implementation
+ visibility: Visibility of the devserver targets
+ tags: Standard Bazel tags, this macro adds a couple for ibazel
+ testonly: Whether the devserver should only run in `bazel test`
**kwargs: passed through to `ts_devserver`
"""
ts_devserver(
+ name = "%s_launcher" % name,
+ data = data + ["@bazel_tools//tools/bash/runfiles"],
+ testonly = testonly,
+ visibility = ["//visibility:private"],
+ tags = tags,
+ **kwargs
+ )
+
+ native.sh_binary(
+ name = name,
+ args = args,
# Users don't need to know that these tags are required to run under ibazel
tags = tags + [
# Tell ibazel not to restart the devserver when its deps change.
@@ -241,5 +257,8 @@
# this program.
"ibazel_live_reload",
],
- **kwargs
+ srcs = ["%s_launcher.sh" % name],
+ data = [":%s_launcher" % name],
+ testonly = testonly,
+ visibility = visibility,
)
