Use ctime in file digest cache key (#18101)
File digests are now additionally keyed by ctime for supported file system implementations. Since Bazel has a non-zero default for `--cache_computed_file_digests`, this may be required for correctness in cases where different files have identical mtime and inode number. For example, this can happen on Linux when files are extracted from a tar file with fixed mtime and are then replaced with `mv`, which preserves inodes.
Since Java (N)IO doesn't have support for reading file ctimes on Windows, a new method backed by a native implementation is added to `WindowsFileOperation`. Adding a call to this function to `stat` uncovered previously silent bugs where Unix-style `PathFragment`s were created on Windows:
1. Bzlmod's `createLocalRepoSpec` did not correctly extract the path from a registry's `file://` URI on Windows.
2. `--package_path` isn't usable with absolute paths on Windows as it splits on `:`. Since the flag is deprecated, this commit fixes the tests rather than the implementation.
Fixes #14723
Closes #18003.
PiperOrigin-RevId: 524297459
Change-Id: I96bfc0210e2f71bf8603c7b7cc5eb06a04048c85
Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD
index e6a4c7d..a2f01a4 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD
@@ -74,6 +74,7 @@
"//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/events",
+ "//src/main/java/com/google/devtools/build/lib/util:os",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//third_party:gson",
"//third_party:guava",
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistry.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistry.java
index 869ae3b..83266c3 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistry.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistry.java
@@ -24,6 +24,7 @@
import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
+import com.google.devtools.build.lib.util.OS;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.gson.FieldNamingPolicy;
import com.google.gson.Gson;
@@ -175,7 +176,15 @@
path = moduleBase + "/" + path;
if (!PathFragment.isAbsolute(moduleBase)) {
if (uri.getScheme().equals("file")) {
- path = uri.getPath() + "/" + path;
+ if (uri.getPath().isEmpty() || !uri.getPath().startsWith("/")) {
+ throw new IOException(
+ String.format(
+ "Provided non absolute local registry path for module %s: %s",
+ key, uri.getPath()));
+ }
+ // Unix: file:///tmp --> /tmp
+ // Windows: file:///C:/tmp --> C:/tmp
+ path = uri.getPath().substring(OS.getCurrent() == OS.WINDOWS ? 1 : 0) + "/" + path;
} else {
throw new IOException(String.format("Provided non local registry for module %s", key));
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
index af848e3..f0efff1 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
@@ -719,7 +719,10 @@
}
private void setPathPermissionsIfFile(Path path) throws IOException {
- if (path.isFile(Symlinks.NOFOLLOW)) {
+ FileStatus stat = path.statIfFound(Symlinks.NOFOLLOW);
+ if (stat != null
+ && stat.isFile()
+ && stat.getPermissions() != outputPermissions.getPermissionsMode()) {
setPathPermissions(path);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java b/src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java
index c92fba6..0e12321 100644
--- a/src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java
@@ -120,7 +120,8 @@
return status.getInodeNumber();
}
- int getPermissions() {
+ @Override
+ public int getPermissions() {
return status.getPermissions();
}
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/DigestUtils.java b/src/main/java/com/google/devtools/build/lib/vfs/DigestUtils.java
index 6c2f2b5..77bd98d 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/DigestUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/DigestUtils.java
@@ -46,6 +46,9 @@
/** File system identifier of the file (typically the inode number). */
private final long nodeId;
+ /** Last change time of the file. */
+ private final long changeTime;
+
/** Last modification time of the file. */
private final long modifiedTime;
@@ -62,6 +65,7 @@
public CacheKey(Path path, FileStatus status) throws IOException {
this.path = path.asFragment();
this.nodeId = status.getNodeId();
+ this.changeTime = status.getLastChangeTime();
this.modifiedTime = status.getLastModifiedTime();
this.size = status.getSize();
}
@@ -76,6 +80,7 @@
CacheKey key = (CacheKey) object;
return path.equals(key.path)
&& nodeId == key.nodeId
+ && changeTime == key.changeTime
&& modifiedTime == key.modifiedTime
&& size == key.size;
}
@@ -86,6 +91,7 @@
int result = 17;
result = 31 * result + path.hashCode();
result = 31 * result + Longs.hashCode(nodeId);
+ result = 31 * result + Longs.hashCode(changeTime);
result = 31 * result + Longs.hashCode(modifiedTime);
result = 31 * result + Longs.hashCode(size);
return result;
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/FileStatus.java b/src/main/java/com/google/devtools/build/lib/vfs/FileStatus.java
index f6112be..5827576 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/FileStatus.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/FileStatus.java
@@ -85,4 +85,16 @@
* ought to cause the node ID of b to change, but appending / modifying b should not.
*/
long getNodeId() throws IOException;
+
+ /**
+ * Returns the file's permissions in POSIX format (e.g. 0755) if possible without performing
+ * additional IO, otherwise (or if unsupported by the file system) returns -1.
+ *
+ * <p>If accurate group and other permissions aren't available, the returned value should attempt
+ * to mimic a umask of 022 (i.e. read and execute permissions extend to group and other, write
+ * does not).
+ */
+ default int getPermissions() {
+ return -1;
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryContentInfo.java b/src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryContentInfo.java
index 5686308..51b9b65 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryContentInfo.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryContentInfo.java
@@ -121,6 +121,22 @@
}
@Override
+ public final int getPermissions() {
+ int permissions = 0;
+ // Emulate the default umask of 022.
+ if (isReadable) {
+ permissions |= 0444;
+ }
+ if (isWritable) {
+ permissions |= 0200;
+ }
+ if (isExecutable) {
+ permissions |= 0111;
+ }
+ return permissions;
+ }
+
+ @Override
public final InMemoryContentInfo inode() {
return this;
}
diff --git a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileOperations.java b/src/main/java/com/google/devtools/build/lib/windows/WindowsFileOperations.java
index 25cba40..7b8e691 100644
--- a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileOperations.java
+++ b/src/main/java/com/google/devtools/build/lib/windows/WindowsFileOperations.java
@@ -15,7 +15,9 @@
package com.google.devtools.build.lib.windows;
import com.google.devtools.build.lib.jni.JniLoader;
+import java.io.FileNotFoundException;
import java.io.IOException;
+import java.nio.file.AccessDeniedException;
/** File operations on Windows. */
public class WindowsFileOperations {
@@ -82,6 +84,12 @@
// IS_SYMLINK_OR_JUNCTION_ERROR = 1;
private static final int IS_SYMLINK_OR_JUNCTION_DOES_NOT_EXIST = 2;
+ // Keep GET_CHANGE_TIME_* values in sync with src/main/native/windows/file.cc.
+ private static final int GET_CHANGE_TIME_SUCCESS = 0;
+ // private static final int GET_CHANGE_TIME_ERROR = 1;
+ private static final int GET_CHANGE_TIME_DOES_NOT_EXIST = 2;
+ private static final int GET_CHANGE_TIME_ACCESS_DENIED = 3;
+
// Keep CREATE_JUNCTION_* values in sync with src/main/native/windows/file.h.
private static final int CREATE_JUNCTION_SUCCESS = 0;
// CREATE_JUNCTION_ERROR = 1;
@@ -114,6 +122,9 @@
private static native int nativeIsSymlinkOrJunction(
String path, boolean[] result, String[] error);
+ private static native int nativeGetChangeTime(
+ String path, boolean followReparsePoints, long[] result, String[] error);
+
private static native boolean nativeGetLongPath(String path, String[] result, String[] error);
private static native int nativeCreateJunction(String name, String target, String[] error);
@@ -143,6 +154,25 @@
throw new IOException(String.format("Cannot tell if '%s' is link: %s", path, error[0]));
}
+ /** Returns the time at which the file was last changed, including metadata changes. */
+ public static long getLastChangeTime(String path, boolean followReparsePoints)
+ throws IOException {
+ long[] result = new long[] {0};
+ String[] error = new String[] {null};
+ switch (nativeGetChangeTime(asLongPath(path), followReparsePoints, result, error)) {
+ case GET_CHANGE_TIME_SUCCESS:
+ return result[0];
+ case GET_CHANGE_TIME_DOES_NOT_EXIST:
+ throw new FileNotFoundException(path);
+ case GET_CHANGE_TIME_ACCESS_DENIED:
+ throw new AccessDeniedException(path);
+ default:
+ // This is GET_CHANGE_TIME_ERROR (1). The JNI code puts a custom message in 'error[0]'.
+ break;
+ }
+ throw new IOException(String.format("Cannot get last change time of '%s': %s", path, error[0]));
+ }
+
/**
* Returns the long path associated with the input `path`.
*
diff --git a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java b/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java
index 30afecc..2c4eda7 100644
--- a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java
@@ -156,6 +156,8 @@
}
final boolean isSymbolicLink = !followSymlinks && fileIsSymbolicLink(file);
+ final long lastChangeTime =
+ WindowsFileOperations.getLastChangeTime(getNioPath(path).toString(), followSymlinks);
FileStatus status =
new FileStatus() {
@Override
@@ -193,8 +195,7 @@
@Override
public long getLastChangeTime() {
- // This is the best we can do with Java NIO...
- return attributes.lastModifiedTime().toMillis();
+ return lastChangeTime;
}
@Override
@@ -202,6 +203,12 @@
// TODO(bazel-team): Consider making use of attributes.fileKey().
return -1;
}
+
+ @Override
+ public int getPermissions() {
+ // Files on Windows are implicitly readable and executable.
+ return 0555 | (attributes.isReadOnly() ? 0 : 0200);
+ }
};
return status;
diff --git a/src/main/native/windows/file-jni.cc b/src/main/native/windows/file-jni.cc
index d920b08..4ed2470 100644
--- a/src/main/native/windows/file-jni.cc
+++ b/src/main/native/windows/file-jni.cc
@@ -62,6 +62,29 @@
return static_cast<jint>(result);
}
+extern "C" JNIEXPORT jint JNICALL
+Java_com_google_devtools_build_lib_windows_WindowsFileOperations_nativeGetChangeTime(
+ JNIEnv* env, jclass clazz, jstring path, jboolean follow_reparse_points,
+ jlongArray result_holder, jobjectArray error_msg_holder) {
+ std::wstring wpath(bazel::windows::GetJavaWstring(env, path));
+ std::wstring error;
+ jlong ctime = 0;
+ int result =
+ bazel::windows::GetChangeTime(wpath.c_str(), follow_reparse_points,
+ reinterpret_cast<int64_t*>(&ctime), &error);
+ if (result == bazel::windows::GetChangeTimeResult::kSuccess) {
+ env->SetLongArrayRegion(result_holder, 0, 1, &ctime);
+ } else {
+ if (!error.empty() && CanReportError(env, error_msg_holder)) {
+ ReportLastError(
+ bazel::windows::MakeErrorMessage(
+ WSTR(__FILE__), __LINE__, L"nativeGetChangeTime", wpath, error),
+ env, error_msg_holder);
+ }
+ }
+ return static_cast<jint>(result);
+}
+
extern "C" JNIEXPORT jboolean JNICALL
Java_com_google_devtools_build_lib_windows_WindowsFileOperations_nativeGetLongPath(
JNIEnv* env, jclass clazz, jstring path, jobjectArray result_holder,
diff --git a/src/main/native/windows/file.cc b/src/main/native/windows/file.cc
index 72d3e4a..f20831e 100644
--- a/src/main/native/windows/file.cc
+++ b/src/main/native/windows/file.cc
@@ -21,6 +21,7 @@
#include <WinIoCtl.h>
#include <stdint.h> // uint8_t
#include <versionhelpers.h>
+#include <winbase.h>
#include <windows.h>
#include <memory>
@@ -119,6 +120,54 @@
}
}
+int GetChangeTime(const WCHAR* path, bool follow_reparse_points,
+ int64_t* result, wstring* error) {
+ if (!IsAbsoluteNormalizedWindowsPath(path)) {
+ if (error) {
+ *error = MakeErrorMessage(WSTR(__FILE__), __LINE__, L"GetChangeTime",
+ path, L"expected an absolute Windows path");
+ }
+ return GetChangeTimeResult::kError;
+ }
+
+ AutoHandle handle;
+ DWORD flags = FILE_FLAG_BACKUP_SEMANTICS;
+ if (!follow_reparse_points) {
+ flags |= FILE_FLAG_OPEN_REPARSE_POINT;
+ }
+ handle = CreateFileW(path, 0,
+ FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
+ nullptr, OPEN_EXISTING, flags, nullptr);
+
+ if (!handle.IsValid()) {
+ DWORD err = GetLastError();
+ if (err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND) {
+ return GetChangeTimeResult::kDoesNotExist;
+ } else if (err == ERROR_ACCESS_DENIED) {
+ return GetChangeTimeResult::kAccessDenied;
+ }
+ if (error) {
+ *error =
+ MakeErrorMessage(WSTR(__FILE__), __LINE__, L"CreateFileW", path, err);
+ }
+ return GetChangeTimeResult::kError;
+ }
+
+ FILE_BASIC_INFO info;
+ if (!GetFileInformationByHandleEx(handle, FileBasicInfo, (LPVOID)&info,
+ sizeof(FILE_BASIC_INFO))) {
+ DWORD err = GetLastError();
+ if (error) {
+ *error = MakeErrorMessage(WSTR(__FILE__), __LINE__,
+ L"GetFileInformationByHandleEx", path, err);
+ }
+ return GetChangeTimeResult::kError;
+ }
+
+ *result = info.ChangeTime.QuadPart;
+ return GetChangeTimeResult::kSuccess;
+}
+
wstring GetLongPath(const WCHAR* path, unique_ptr<WCHAR[]>* result) {
if (!IsAbsoluteNormalizedWindowsPath(path)) {
return MakeErrorMessage(WSTR(__FILE__), __LINE__, L"GetLongPath", path,
diff --git a/src/main/native/windows/file.h b/src/main/native/windows/file.h
index 590137f..2850357 100644
--- a/src/main/native/windows/file.h
+++ b/src/main/native/windows/file.h
@@ -83,6 +83,16 @@
};
// Keep in sync with j.c.g.devtools.build.lib.windows.WindowsFileOperations
+struct GetChangeTimeResult {
+ enum {
+ kSuccess = 0,
+ kError = 1,
+ kDoesNotExist = 2,
+ kAccessDenied = 3,
+ };
+};
+
+// Keep in sync with j.c.g.devtools.build.lib.windows.WindowsFileOperations
struct DeletePathResult {
enum {
kSuccess = 0,
@@ -136,6 +146,13 @@
// see http://superuser.com/a/343079. In Bazel we only ever create junctions.
int IsSymlinkOrJunction(const WCHAR* path, bool* result, wstring* error);
+// Retrieves the FILETIME at which `path` was last changed, including metadata.
+//
+// `path` should be an absolute, normalized, Windows-style path, with "\\?\"
+// prefix if it's longer than MAX_PATH.
+int GetChangeTime(const WCHAR* path, bool follow_reparse_points,
+ int64_t* result, wstring* error);
+
// Computes the long version of `path` if it has any 8dot3 style components.
// Returns the empty string upon success, or a human-readable error message upon
// failure.
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java
index ebc04c3..b780e03 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java
@@ -305,9 +305,10 @@
// Reset this output, which will make the handler stat the file again.
handler.resetOutputs(ImmutableList.of(artifact));
- chmodCalls.clear(); // Permit a second chmod call for the artifact.
+ chmodCalls.clear();
assertThat(handler.getMetadata(artifact).getSize()).isEqualTo(10);
- assertThat(chmodCalls).containsExactly(outputPath, 0555);
+ // The handler should not have chmodded the file as it already has the correct permission.
+ assertThat(chmodCalls).isEmpty();
}
@Test
diff --git a/src/test/shell/integration/execution_phase_tests.sh b/src/test/shell/integration/execution_phase_tests.sh
index 3458210..73ff18e 100755
--- a/src/test/shell/integration/execution_phase_tests.sh
+++ b/src/test/shell/integration/execution_phase_tests.sh
@@ -392,4 +392,102 @@
>& "$TEST_log" || fail "Expected success"
expect_log "WARNING: Directory artifact foo/dir crosses package boundary into"
}
+
+# Regression test for https://github.com/bazelbuild/bazel/issues/14723
+function test_fixed_mtime_move_detected_as_change() {
+ mkdir -p pkg
+ cat > pkg/BUILD <<'EOF'
+load("rules.bzl", "my_expand")
+
+genrule(
+ name = "my_templates",
+ srcs = ["template_archive.tar"],
+ outs = ["template1"],
+ cmd = "tar -C $(RULEDIR) -xf $<",
+)
+
+my_expand(
+ name = "expand1",
+ input = "template1",
+ output = "expanded1",
+ to_sub = {"test":"foo"}
+)
+EOF
+ cat > pkg/rules.bzl <<'EOF'
+def _my_expand_impl(ctx):
+ ctx.actions.expand_template(
+ template = ctx.file.input,
+ output = ctx.outputs.output,
+ substitutions = ctx.attr.to_sub
+ )
+
+my_expand = rule(
+ implementation = _my_expand_impl,
+ attrs = {
+ "input": attr.label(allow_single_file=True),
+ "output": attr.output(),
+ "to_sub" : attr.string_dict(),
+ }
+)
+EOF
+
+ echo "test : alpha" > template1
+ touch -t 197001010000 template1
+ tar -cf pkg/template_archive_alpha.tar template1
+
+ echo "test : delta" > template1
+ touch -t 197001010000 template1
+ tar -cf pkg/template_archive_delta.tar template1
+
+ mv pkg/template_archive_alpha.tar pkg/template_archive.tar
+ bazel build //pkg:expand1 || fail "Expected success"
+ assert_equals "foo : alpha" "$(cat bazel-bin/pkg/expanded1)"
+
+ mv pkg/template_archive_delta.tar pkg/template_archive.tar
+ bazel build //pkg:expand1 || fail "Expected success"
+ assert_equals "foo : delta" "$(cat bazel-bin/pkg/expanded1)"
+}
+
+# Regression test for https://github.com/bazelbuild/bazel/issues/14723
+function test_fixed_mtime_source_file() {
+ mkdir -p pkg
+ cat > pkg/BUILD <<'EOF'
+load("rules.bzl", "my_expand")
+
+my_expand(
+ name = "expand1",
+ input = "template1",
+ output = "expanded1",
+ to_sub = {"test":"foo"}
+)
+EOF
+ cat > pkg/rules.bzl <<'EOF'
+def _my_expand_impl(ctx):
+ ctx.actions.expand_template(
+ template = ctx.file.input,
+ output = ctx.outputs.output,
+ substitutions = ctx.attr.to_sub
+ )
+
+my_expand = rule(
+ implementation = _my_expand_impl,
+ attrs = {
+ "input": attr.label(allow_single_file=True),
+ "output": attr.output(),
+ "to_sub" : attr.string_dict(),
+ }
+)
+EOF
+
+ echo "test : alpha" > pkg/template1
+ touch -t 197001010000 pkg/template1
+ bazel build //pkg:expand1 || fail "Expected success"
+ assert_equals "foo : alpha" "$(cat bazel-bin/pkg/expanded1)"
+
+ echo "test : delta" > pkg/template1
+ touch -t 197001010000 pkg/template1
+ bazel build //pkg:expand1 || fail "Expected success"
+ assert_equals "foo : delta" "$(cat bazel-bin/pkg/expanded1)"
+}
+
run_suite "Integration tests of ${PRODUCT_NAME} using the execution phase."
diff --git a/src/test/shell/integration/loading_phase_test.sh b/src/test/shell/integration/loading_phase_test.sh
index 478410c..d997109 100755
--- a/src/test/shell/integration/loading_phase_test.sh
+++ b/src/test/shell/integration/loading_phase_test.sh
@@ -289,24 +289,24 @@
local -r pkg="${FUNCNAME}"
mkdir -p "$pkg" || fail "could not create \"$pkg\""
- local other_root=$TEST_TMPDIR/other_root/${WORKSPACE_NAME}
+ local other_root=other_root/${WORKSPACE_NAME}
mkdir -p $other_root/$pkg/a
touch $other_root/WORKSPACE
echo 'sh_library(name="external")' > $other_root/$pkg/a/BUILD
mkdir -p $pkg/a
echo 'sh_library(name="internal")' > $pkg/a/BUILD
- bazel query --package_path=$other_root:. $pkg/a:all >& $TEST_log \
+ bazel query --package_path=%workspace%/$other_root:. $pkg/a:all >& $TEST_log \
|| fail "Expected success"
expect_log "//$pkg/a:external"
expect_not_log "//$pkg/a:internal"
rm -r $other_root
- bazel query --package_path=$other_root:. $pkg/a:all >& $TEST_log \
+ bazel query --package_path=%workspace%/$other_root:. $pkg/a:all >& $TEST_log \
|| fail "Expected success"
expect_log "//$pkg/a:internal"
expect_not_log "//$pkg/a:external"
mkdir -p $other_root
- bazel query --package_path=$other_root:. $pkg/a:all >& $TEST_log \
+ bazel query --package_path=%workspace%/$other_root:. $pkg/a:all >& $TEST_log \
|| fail "Expected success"
expect_log "//$pkg/a:internal"
expect_not_log "//$pkg/a:external"
