Skip to content

Commit

Permalink
[engine] Resolve relative paths correctly
Browse files Browse the repository at this point in the history
When running `shac check --root <other-dir>`, relative paths of
executables run by `ctx.os.exec()` should be resolved relative to the
root directory, not the current working directory (which may differ from
the root directory).

Change-Id: I8ddf2169b7a51a975890852aa7292dc30d93a8b0
Reviewed-on: https://fuchsia-review.googlesource.com/c/shac-project/shac/+/909854
Reviewed-by: Marc-Antoine Ruel <[email protected]>
Fuchsia-Auto-Submit: Oliver Newman <[email protected]>
Commit-Queue: Auto-Submit <[email protected]>
  • Loading branch information
orn688 authored and CQ Bot committed Aug 30, 2023
1 parent d09e857 commit 4bfd1e0
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 7 deletions.
4 changes: 4 additions & 0 deletions internal/engine/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1664,6 +1664,10 @@ func TestTestDataPrint(t *testing.T) {
"ctx-os-exec-parallel.star",
strings.Repeat("[//ctx-os-exec-parallel.star:27] Hello, world\n", 10),
},
{
"ctx-os-exec-relpath.star",
"[//ctx-os-exec-relpath.star:17] Hello from a nested file\n\n",
},
{
"ctx-os-exec-stdin.star",
"[//ctx-os-exec-stdin.star:30] stdout given NoneType for stdin:\n" +
Expand Down
25 changes: 18 additions & 7 deletions internal/engine/runtime_ctx_os.go
Original file line number Diff line number Diff line change
Expand Up @@ -286,13 +286,24 @@ func ctxOsExec(ctx context.Context, s *shacState, name string, args starlark.Tup
exeParts[0] = s.root
fullCmd[0] = strings.Join(exeParts, string(os.PathSeparator))
} else {
// nsjail doesn't do $PATH-based resolution of the command it's given, so it
// must either be an absolute or relative path. Do this resolution
// unconditionally for consistency across platforms even though it's not
// necessary when not using nsjail.
fullCmd[0], err = exec.LookPath(fullCmd[0])
if err != nil && !errors.Is(err, exec.ErrDot) {
return nil, err
// nsjail doesn't do $PATH-based resolution of the command it's
// given, so it must either be an absolute or relative path. Do this
// resolution unconditionally for consistency across platforms even
// though it's not necessary when not using nsjail.
if strings.Contains(fullCmd[0], "/") && !filepath.IsAbs(fullCmd[0]) {
// Make the path absolute if it's relative and contains slashes.
// We can't use exec.LookPath in this case because it tries to
// evaluate the path relative to the cwd, but we want to evaluate it
// relative to the shac.star file being evaluated.
//
// filepath.Join ignores empty elements, so s.subdir can be included
// unconditionally.
fullCmd[0] = filepath.Join(s.root, s.subdir, fullCmd[0])
} else {
fullCmd[0], err = exec.LookPath(fullCmd[0])
if err != nil && !errors.Is(err, exec.ErrDot) {
return nil, err
}
}
}

Expand Down
23 changes: 23 additions & 0 deletions internal/engine/testdata/print/ctx-os-exec-relpath.star
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Copyright 2023 The Shac Authors
#
# 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.

def cb(ctx):
ext = "bat" if ctx.platform.os == "windows" else "sh"
print(ctx.os.exec(
# Implicit relative path should be evaluated relative to the calling
# Starlark file.
["dir/nested_hello_world." + ext]
).wait().stdout)

shac.register_check(cb)
16 changes: 16 additions & 0 deletions internal/engine/testdata/print/dir/nested_hello_world.bat
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
@echo off
:: Copyright 2023 The Shac Authors
::
:: 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.

echo Hello from a nested file
16 changes: 16 additions & 0 deletions internal/engine/testdata/print/dir/nested_hello_world.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#!/bin/sh
# Copyright 2023 The Shac Authors
#
# 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.

echo "Hello from a nested file"

0 comments on commit 4bfd1e0

Please sign in to comment.