[build] Error out on abs paths in depfiles
Also fixed the last batch of offending targets/templates.
Fixed: 75451
Change-Id: I5f81f0b5543c6e729adcf833db81a464cfe8a78b
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/521540
Commit-Queue: Jay Zhuang <jayzhuang@google.com>
Reviewed-by: Shai Barack <shayba@google.com>
diff --git a/build/dart/dart_library.gni b/build/dart/dart_library.gni
index 673df43..d582bd1 100644
--- a/build/dart/dart_library.gni
+++ b/build/dart/dart_library.gni
@@ -342,7 +342,7 @@
"--source_lists",
"@" + rebase_path(_all_deps_sources_list_file, root_build_dir),
"--sources",
- ] + rebased_sources
+ ] + rebase_path(rebased_sources, root_build_dir)
inputs = [ _all_deps_sources_list_file ]
deps = [ ":${_all_deps_sources_list_target}" ]
@@ -443,21 +443,21 @@
args = [
"--source-file",
- rebase_path(source_file),
+ rebase_path(source_file, root_build_dir),
"--dot-packages",
- rebase_path(_packages_path),
+ rebase_path(_packages_path, root_build_dir),
"--dartanalyzer",
- rebase_path(dart_analyzer_binary),
+ rebase_path(dart_analyzer_binary, root_build_dir),
"--dart-sdk",
- rebase_path(dart_sdk),
+ rebase_path(dart_sdk, root_build_dir),
"--options",
- rebase_path(options_file),
+ rebase_path(options_file, root_build_dir),
"--stamp",
- rebase_path(output_file),
+ rebase_path(output_file, root_build_dir),
"--depname",
rebase_path(output_file, root_build_dir),
"--depfile",
- rebase_path(depfile),
+ rebase_path(depfile, root_build_dir),
"--all-deps-sources-file",
rebase_path(_all_deps_sources_file, root_build_dir),
]
diff --git a/build/dart/run_analysis.py b/build/dart/run_analysis.py
index e5bbf19..1e1be3a 100755
--- a/build/dart/run_analysis.py
+++ b/build/dart/run_analysis.py
@@ -57,9 +57,6 @@
options = args.options
while True:
- if not os.path.isabs(options):
- print('Expected absolute path, got {}'.format(options))
- return 1
if not os.path.exists(options):
print('Could not find options file: {}'.format(options))
return 1
diff --git a/build/go/build.py b/build/go/build.py
index 0e9cae4..7eb15dd 100755
--- a/build/go/build.py
+++ b/build/go/build.py
@@ -383,7 +383,7 @@
output = subprocess.check_output(
go_list_args, env=env, cwd=gopath_src, text=True)
with open(args.depfile, 'w') as into:
- into.write(dist)
+ into.write(os.path.relpath(dist))
into.write(':')
while output:
try:
@@ -419,6 +419,7 @@
src_dir = t + src_dir[len(f):]
break
+ src_dir = os.path.relpath(src_dir)
for field in files_fields:
files = package.get(field)
if files:
diff --git a/build/go/go_build.gni b/build/go/go_build.gni
index 0f5aa206..469ef96 100644
--- a/build/go/go_build.gni
+++ b/build/go/go_build.gni
@@ -170,7 +170,7 @@
"--root-out-dir",
rebase_path(root_out_dir, root_build_dir),
"--depfile",
- rebase_path(depfile),
+ rebase_path(depfile, root_build_dir),
"--current-cpu",
current_cpu,
"--current-os",
diff --git a/build/tracer/action_tracer.py b/build/tracer/action_tracer.py
index 815435b..bec5918 100755
--- a/build/tracer/action_tracer.py
+++ b/build/tracer/action_tracer.py
@@ -276,11 +276,118 @@
@dataclasses.dataclass
+class DepEdges(object):
+ ins: FrozenSet[str] = dataclasses.field(default_factory=set)
+ outs: FrozenSet[str] = dataclasses.field(default_factory=set)
+
+ def abspaths(self) -> "DepEdges":
+ return DepEdges(ins=_abspaths(self.ins), outs=_abspaths(self.outs))
+
+
+def parse_dep_edges(depfile_line: str) -> DepEdges:
+ """Parse a single line of a depfile.
+
+ This assumes that all depfile entries are formatted onto a single line.
+ TODO(fangism): support more generalized forms of input, e.g. multi-line.
+ See https://github.com/ninja-build/ninja/blob/master/src/depfile_parser_test.cc
+
+ Args:
+ depfile_line: has the form "OUTPUT1 [OUTPUT2 ...]: INPUT [INPUT ...]"
+
+ Returns:
+ A DepEdges object represending a dependency between inputs and outputs.
+
+ Raises:
+ ValueError if unable to parse dependency entry.
+ """
+ outs, sep, ins = depfile_line.strip().partition(":")
+ if not sep:
+ raise ValueError("Failed to parse depfile entry:\n" + depfile_line)
+ return DepEdges(ins=set(shlex.split(ins)), outs=set(shlex.split(outs)))
+
+
+@dataclasses.dataclass
+class DepFile(object):
+ """DepFile represents a collection of dependency edges."""
+ deps: Collection[DepEdges] = dataclasses.field(default_factory=list)
+
+ @property
+ def all_ins(self) -> AbstractSet[str]:
+ """Returns a set of all dependency inputs."""
+ return {f for dep in self.deps for f in dep.ins}
+
+ @property
+ def all_outs(self) -> AbstractSet[str]:
+ """Returns a set of all dependency outputs."""
+ return {f for dep in self.deps for f in dep.outs}
+
+
+def parse_depfile(depfile_lines: Iterable[str]) -> DepFile:
+ """Parses a depfile into a set of inputs and outputs.
+
+ See https://github.com/ninja-build/ninja/blob/master/src/depfile_parser_test.cc
+ for examples of format using Ninja syntax.
+
+ TODO(fangism): ignore blank/comment lines
+
+ Args:
+ depfile_lines: lines from a depfile
+
+ Returns:
+ DepFile object, collection of dependencies.
+ """
+
+ # Go through all lines and join continuations. Doing this manually to avoid
+ # copies as much as possible.
+ lines = []
+ current_line = ""
+ for line in depfile_lines:
+ # We currently don't allow consecutive backslashes in filenames to
+ # simplify depfile parsing. Support can be added if use cases come up.
+ #
+ # Ninja's implementation:
+ # https://github.com/ninja-build/ninja/blob/5993141c0977f563de5e064fbbe617f9dc34bb8d/src/depfile_parser.cc#L39
+ if r"\\" in line:
+ raise ValueError(
+ f'Consecutive backslashes found in depfile line "{line}", this is not supported by action tracer'
+ )
+ # We currently don't have any use cases with trailing whitespaces in
+ # file names, so treat them as errors when they show up at the end of a
+ # line, because users usually want a line continuation. We can
+ # reconsider this check when use cases come up.
+ if trailing_white_spaces.match(line):
+ raise ValueError(
+ f'Backslash followed by trailing whitespaces at end of line "{line}", remove whitespaces for proper line continuation'
+ )
+
+ if line.endswith(("\\\n", "\\\r\n")):
+ current_line += line.rstrip("\\\r\n")
+ continue
+ current_line += line
+ lines.append(current_line)
+ current_line = ""
+
+ if current_line:
+ raise ValueError("Line continuation found at end of file")
+
+ return DepFile(deps=[parse_dep_edges(line) for line in lines])
+
+
+def abspaths_from_depfile(depfile: DepFile,
+ allowed_abspaths: FrozenSet[str]) -> Collection[str]:
+ return [
+ f for f in (depfile.all_ins | depfile.all_outs)
+ if f not in allowed_abspaths and os.path.isabs(f)
+ ]
+
+
+@dataclasses.dataclass
class Action(object):
"""Represents a set of parameters of a single build action."""
inputs: Sequence[str] = dataclasses.field(default_factory=list)
outputs: Collection[str] = dataclasses.field(default_factory=list)
depfile: Optional[str] = None
+ parsed_depfile: Optional[DepFile] = None
response_file_name: Optional[str] = None
def access_constraints(
@@ -300,13 +407,13 @@
allowed_writes.add(self.depfile)
if os.path.exists(self.depfile):
with open(self.depfile, "r") as f:
- depfile = parse_depfile(f)
+ self.parsed_depfile = parse_depfile(f)
if (writeable_depfile_inputs):
- allowed_writes.update(depfile.all_ins)
+ allowed_writes.update(self.parsed_depfile.all_ins)
else:
- allowed_reads.update(depfile.all_ins)
- allowed_writes.update(depfile.all_outs)
+ allowed_reads.update(self.parsed_depfile.all_ins)
+ allowed_writes.update(self.parsed_depfile.all_outs)
# Everything writeable is readable.
allowed_reads.update(allowed_writes)
@@ -448,104 +555,6 @@
}
-@dataclasses.dataclass
-class DepEdges(object):
- ins: FrozenSet[str] = dataclasses.field(default_factory=set)
- outs: FrozenSet[str] = dataclasses.field(default_factory=set)
-
- def abspaths(self) -> "DepEdges":
- return DepEdges(ins=_abspaths(self.ins), outs=_abspaths(self.outs))
-
-
-def parse_dep_edges(depfile_line: str) -> DepEdges:
- """Parse a single line of a depfile.
-
- This assumes that all depfile entries are formatted onto a single line.
- TODO(fangism): support more generalized forms of input, e.g. multi-line.
- See https://github.com/ninja-build/ninja/blob/master/src/depfile_parser_test.cc
-
- Args:
- depfile_line: has the form "OUTPUT1 [OUTPUT2 ...]: INPUT [INPUT ...]"
-
- Returns:
- A DepEdges object represending a dependency between inputs and outputs.
-
- Raises:
- ValueError if unable to parse dependency entry.
- """
- outs, sep, ins = depfile_line.strip().partition(":")
- if sep != ":":
- raise ValueError("Failed to parse depfile entry:\n" + depfile_line)
- return DepEdges(ins=set(shlex.split(ins)), outs=set(shlex.split(outs)))
-
-
-@dataclasses.dataclass
-class DepFile(object):
- """DepFile represents a collection of dependency edges."""
- deps: Collection[DepEdges] = dataclasses.field(default_factory=list)
-
- @property
- def all_ins(self) -> AbstractSet[str]:
- """Returns a set of all dependency inputs."""
- return {f for dep in self.deps for f in dep.ins}
-
- @property
- def all_outs(self) -> AbstractSet[str]:
- """Returns a set of all dependency outputs."""
- return {f for dep in self.deps for f in dep.outs}
-
-
-def parse_depfile(depfile_lines: Iterable[str]) -> DepFile:
- """Parses a depfile into a set of inputs and outputs.
-
- See https://github.com/ninja-build/ninja/blob/master/src/depfile_parser_test.cc
- for examples of format using Ninja syntax.
-
- TODO(fangism): ignore blank/comment lines
-
- Args:
- depfile_lines: lines from a depfile
-
- Returns:
- DepFile object, collection of dependencies.
- """
-
- # Go through all lines and join continuations. Doing this manually to avoid
- # copies as much as possible.
- lines = []
- current_line = []
- for line in depfile_lines:
- # We currently don't allow consecutive backslashes in filenames to
- # simplify depfile parsing. Support can be added if use cases come up.
- #
- # Ninja's implementation:
- # https://github.com/ninja-build/ninja/blob/5993141c0977f563de5e064fbbe617f9dc34bb8d/src/depfile_parser.cc#L39
- if r"\\" in line:
- raise ValueError(
- f'Consecutive backslashes found in depfile line "{line}", this is not supported by action tracer'
- )
- # We currently don't have any use cases with trailing whitespaces in
- # file names, so treat them as errors when they show up at the end of a
- # line, because users usually want a line continuation. We can
- # reconsider this check when use cases come up.
- if trailing_white_spaces.match(line):
- raise ValueError(
- f'Backslash followed by trailing whitespaces at end of line "{line}", remove whitespaces for proper line continuation'
- )
-
- if line.endswith(("\\\n", "\\\r\n")):
- current_line.append(line.rstrip("\\\r\n"))
- continue
- current_line.append(line)
- lines.append("".join(current_line))
- current_line = []
-
- if current_line:
- raise ValueError("Line continuation found at end of file")
-
- return DepFile(deps=[parse_dep_edges(line) for line in lines])
-
-
def _verbose_path(path: str) -> str:
"""When any symlinks are followed, show this."""
realpath = os.path.realpath(path)
@@ -1002,6 +1011,22 @@
file=sys.stderr)
# do not set the exit code
+ if action.parsed_depfile:
+ allowed_abspaths = {"/usr/bin/env"}
+ abspaths = abspaths_from_depfile(
+ action.parsed_depfile, allowed_abspaths)
+
+ if abspaths:
+ exit_code = args.failed_check_status
+ one_path_per_line = '\n'.join(sorted(abspaths))
+ print(
+ f"""
+Found the following files with absolute paths in depfile {action.depfile} for {args.label}:
+
+{one_path_per_line}
+""",
+ file=sys.stderr)
+
if retval != 0:
# Always forward the action's non-zero exit code, regardless of tracer findings.
return retval
diff --git a/build/zircon/sdk_build_id.py b/build/zircon/sdk_build_id.py
index 772f4db..5474f6d 100755
--- a/build/zircon/sdk_build_id.py
+++ b/build/zircon/sdk_build_id.py
@@ -81,7 +81,7 @@
deps = []
for dest, source in manifest.items():
mappings.append(f'{dest}={source}')
- deps.append(source)
+ deps.append(os.path.relpath(source))
args.manifest.write('\n'.join(mappings))
args.depfile.write(
'{} {}: {}\n'.format(
diff --git a/src/sys/pkg/bin/pm/cmd/pm/publish/publish.go b/src/sys/pkg/bin/pm/cmd/pm/publish/publish.go
index 50a9286..b0d2d14 100644
--- a/src/sys/pkg/bin/pm/cmd/pm/publish/publish.go
+++ b/src/sys/pkg/bin/pm/cmd/pm/publish/publish.go
@@ -21,7 +21,7 @@
"go.fuchsia.dev/fuchsia/src/sys/pkg/bin/pm/build"
"go.fuchsia.dev/fuchsia/src/sys/pkg/bin/pm/pkg"
"go.fuchsia.dev/fuchsia/src/sys/pkg/bin/pm/repo"
- "go.fuchsia.dev/fuchsia/src/sys/pkg/lib/far/go"
+ far "go.fuchsia.dev/fuchsia/src/sys/pkg/lib/far/go"
)
const (
@@ -312,6 +312,19 @@
if *depfilePath != "" {
timestampPath := filepath.Join(config.RepoDir, "repository", "timestamp.json")
for i, str := range deps {
+ // Avoid absolute paths in depfiles.
+ if filepath.IsAbs(str) {
+ wd, err := os.Getwd()
+ if err != nil {
+ return fmt.Errorf("getting current working directory: %s", err)
+ }
+ rel, err := filepath.Rel(wd, str)
+ if err != nil {
+ return fmt.Errorf("rebasing %q to current working directory %q: %s", str, rel, err)
+ }
+ str = rel
+ }
+
// It is not clear if this is appropriate input for the depfile, which is
// underspecified - it's a "make format file". For the most part this should
// not affect Fuchsia builds, as we do not use "interesting" characters in
diff --git a/src/sys/pkg/bin/pm/cmd/pm/publish/publish_test.go b/src/sys/pkg/bin/pm/cmd/pm/publish/publish_test.go
index c95883a..4a29e91 100644
--- a/src/sys/pkg/bin/pm/cmd/pm/publish/publish_test.go
+++ b/src/sys/pkg/bin/pm/cmd/pm/publish/publish_test.go
@@ -60,8 +60,8 @@
t.Fatalf("got %#v, wanted one input", inputPaths)
}
- if inputPaths[0] != archivePath {
- t.Errorf("depfile inputs: %#v != %#v", inputPaths[0], archivePath)
+ if got, want := inputPaths[0], mustRelativePath(t, archivePath); got != want {
+ t.Errorf("depfile inputs: got %s, want %s", got, want)
}
}
@@ -75,7 +75,7 @@
outputManifestPath := filepath.Join(cfg.OutputDir, "package_manifest.json")
packagesListPath := filepath.Join(cfg.OutputDir, "packages.list")
- if err := ioutil.WriteFile(packagesListPath, []byte(outputManifestPath+"\n"), 0o600); err != nil {
+ if err := ioutil.WriteFile(packagesListPath, []byte(outputManifestPath+"\n"), 0600); err != nil {
t.Fatal(err)
}
@@ -91,11 +91,11 @@
t.Errorf("depfile output: got %q, want %q", outName, ex)
}
- if inputPaths[0] != packagesListPath {
- t.Errorf("depfile inputs: %q != %q", inputPaths[0], packagesListPath)
+ if got, want := inputPaths[0], mustRelativePath(t, packagesListPath); got != want {
+ t.Errorf("depfile inputs: got %q, want %q", got, want)
}
- if inputPaths[1] != outputManifestPath {
- t.Errorf("depfile inputs: %q != %q", inputPaths[1], outputManifestPath)
+ if got, want := inputPaths[1], mustRelativePath(t, outputManifestPath); got != want {
+ t.Errorf("depfile inputs: got %q, want %q", got, want)
}
inputPaths = inputPaths[2:]
@@ -110,17 +110,37 @@
}
sourcePaths := []string{}
for _, blob := range blobs {
- sourcePaths = append(sourcePaths, blob.SourcePath)
+ sourcePaths = append(sourcePaths, mustRelativePath(t, blob.SourcePath))
}
sort.Strings(sourcePaths)
for i := range sourcePaths {
if inputPaths[i] != sourcePaths[i] {
- t.Errorf("deps entry: %q != %q", inputPaths[i], sourcePaths[i])
+ t.Errorf("deps entry: got %q, want %q", inputPaths[i], sourcePaths[i])
}
}
}
+// mustRelativePath converts the input path relative to the current working
+// directory. Input path is unchanged if it's not an absolute path.
+func mustRelativePath(t *testing.T, p string) string {
+ t.Helper()
+
+ if !filepath.IsAbs(p) {
+ return p
+ }
+
+ wd, err := os.Getwd()
+ if err != nil {
+ t.Fatalf("Failed to get current working directory: %v", err)
+ }
+ rel, err := filepath.Rel(wd, p)
+ if err != nil {
+ t.Fatalf("Failed to rebase %s to %s: %v", p, wd, err)
+ }
+ return rel
+}
+
func readDepfile(t *testing.T, depfilePath string) (string, []string) {
b, err := ioutil.ReadFile(depfilePath)
if err != nil {
