Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.1.0] Error on invalid path characters in .bazelignore #21259

Merged
merged 1 commit into from
Feb 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
import java.io.IOException;
import java.io.InputStreamReader;
import java.nio.charset.StandardCharsets;
import java.nio.file.InvalidPathException;
import java.nio.file.Path;
import javax.annotation.Nullable;

/**
Expand Down Expand Up @@ -60,13 +62,16 @@ public static void getIgnoredPackagePrefixes(
+ " is not readable because: "
+ errorMessage
+ ". Was it modified mid-build?"));
} catch (InvalidPathException e) {
throw new IgnoredPatternsFunctionException(
new InvalidIgnorePathException(e, patternFile.asPath().toString()));
}
}

@Nullable
@Override
public SkyValue compute(SkyKey key, Environment env)
throws SkyFunctionException, InterruptedException {
throws IgnoredPatternsFunctionException, InterruptedException {
RepositoryName repositoryName = (RepositoryName) key.argument();

ImmutableSet.Builder<PathFragment> ignoredPackagePrefixesBuilder = ImmutableSet.builder();
Expand Down Expand Up @@ -122,6 +127,15 @@ private static final class PathFragmentLineProcessor
public boolean processLine(String line) {
if (!line.isEmpty() && !line.startsWith("#")) {
fragments.add(PathFragment.create(line));

// This is called for its side-effects rather than its output.
// Specifically, it validates that the line is a valid path. This
// doesn't do much on UNIX machines where only NUL is an invalid
// character but can reject paths on Windows.
//
// This logic would need to be adjusted if wildcards are ever supported
// (https://github.com/bazelbuild/bazel/issues/7093).
var unused = Path.of(line);
}
return true;
}
Expand All @@ -132,9 +146,19 @@ public ImmutableSet<PathFragment> getResult() {
}
}

private static class InvalidIgnorePathException extends Exception {
public InvalidIgnorePathException(InvalidPathException e, String path) {
super("Invalid path in " + path + ": " + e);
}
}

private static final class IgnoredPatternsFunctionException extends SkyFunctionException {
public IgnoredPatternsFunctionException(InconsistentFilesystemException e) {
super(e, Transience.TRANSIENT);
}

public IgnoredPatternsFunctionException(InvalidIgnorePathException e) {
super(e, Transience.PERSISTENT);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3127,7 +3127,9 @@ protected WorkspaceInfoFromDiff handleDiffs(
// Ignored package prefixes are specified relative to the workspace root
// by definition of .bazelignore. So, we only use ignored paths when the
// package root is equal to the workspace path.
if (workspacePath != null && workspacePath.equals(pathEntry.asPath())) {
if (workspacePath != null
&& workspacePath.equals(pathEntry.asPath())
&& ignoredPackagePrefixesValue != null) {
ignoredPaths =
ignoredPackagePrefixesValue.getPatterns().stream()
.map(pathEntry::getRelative)
Expand Down
11 changes: 11 additions & 0 deletions src/test/shell/bazel/bazelignore_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -206,4 +206,15 @@ EOI
assert_not_contains "ignoreme" output
}

test_invalid_path() {
rm -rf work && mkdir work && cd work
create_workspace_with_default_repos WORKSPACE
echo -e "foo/\0/bar" > .bazelignore
echo 'filegroup(name="f", srcs=glob(["**"]))' > BUILD
if bazel build //... 2> "$TEST_log"; then
fail "Bazel build should have failed"
fi
expect_log "java.nio.file.InvalidPathException: Nul character not allowed"
}

run_suite "Integration tests for .bazelignore"
Loading