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

Implement repository_ctx.read() for reading local files. #7309

Closed
Closed
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 @@ -183,6 +183,26 @@ public static WorkspaceRuleEvent newFileEvent(
return new WorkspaceRuleEvent(result.build());
}

/** Creates a new WorkspaceRuleEvent for a file read event. */
public static WorkspaceRuleEvent newReadEvent(
String path, String ruleLabel, Location location) {
WorkspaceLogProtos.ReadEvent e =
WorkspaceLogProtos.ReadEvent.newBuilder()
.setPath(path)
.build();

WorkspaceLogProtos.WorkspaceEvent.Builder result =
WorkspaceLogProtos.WorkspaceEvent.newBuilder();
result = result.setReadEvent(e);
if (location != null) {
result = result.setLocation(location.print());
}
if (ruleLabel != null) {
result = result.setRule(ruleLabel);
}
return new WorkspaceRuleEvent(result.build());
}

/** Creates a new WorkspaceRuleEvent for an os event. */
public static WorkspaceRuleEvent newOsEvent(String ruleLabel, Location location) {
OsEvent e = WorkspaceLogProtos.OsEvent.getDefaultInstance();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,12 @@ message FileEvent {
bool executable = 3;
}

// Information on "read" event in repository_ctx.
message ReadEvent {
// Path to the file to read
string path = 1;
}

// Information on "os" event in repository_ctx.
message OsEvent {
// Takes no inputs
Expand Down Expand Up @@ -130,5 +136,6 @@ message WorkspaceEvent {
TemplateEvent template_event = 9;
WhichEvent which_event = 10;
ExtractEvent extract_event = 11;
ReadEvent read_event = 12;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ public void symlink(Object from, Object to, Location location)
fromPath.toString(), toPath.toString(), rule.getLabel().toString(), location);
env.getListener().post(w);
try {
checkInOutputDirectory(toPath);
checkInOutputDirectory("write", toPath);
makeDirectories(toPath.getPath());
toPath.getPath().createSymbolicLink(fromPath.getPath());
} catch (IOException e) {
Expand All @@ -192,30 +192,36 @@ public void symlink(Object from, Object to, Location location)
}
}

private void checkInOutputDirectory(SkylarkPath path) throws RepositoryFunctionException {
private void checkInOutputDirectory(String operation, SkylarkPath path) throws RepositoryFunctionException {
brandjon marked this conversation as resolved.
Show resolved Hide resolved
if (!path.getPath().getPathString().startsWith(outputDirectory.getPathString())) {
throw new RepositoryFunctionException(
new EvalException(
Location.fromFile(path.getPath()),
"Cannot write outside of the repository directory for path " + path),
"Cannot " + operation + " outside of the repository directory for path " + path),
Transience.PERSISTENT);
}
}

@Override
public void createFile(Object path, String content, Boolean executable, Location location)
public void createFile(Object path, String content, Boolean executable, Boolean legacyUtf8, Location location)
throws RepositoryFunctionException, EvalException, InterruptedException {
SkylarkPath p = getPath("file()", path);
byte[] contentBytes;
if (legacyUtf8) {
contentBytes = content.getBytes(StandardCharsets.UTF_8);
} else {
contentBytes = content.getBytes(StandardCharsets.ISO_8859_1);
}
WorkspaceRuleEvent w =
WorkspaceRuleEvent.newFileEvent(
p.toString(), content, executable, rule.getLabel().toString(), location);
env.getListener().post(w);
try {
checkInOutputDirectory(p);
checkInOutputDirectory("write", p);
makeDirectories(p.getPath());
p.getPath().delete();
try (OutputStream stream = p.getPath().getOutputStream()) {
stream.write(content.getBytes(StandardCharsets.UTF_8));
stream.write(contentBytes);
}
if (executable) {
p.getPath().setExecutable(true);
Expand Down Expand Up @@ -245,7 +251,7 @@ public void createFileFromTemplate(
location);
env.getListener().post(w);
try {
checkInOutputDirectory(p);
checkInOutputDirectory("write", p);
makeDirectories(p.getPath());
String tpl = FileSystemUtils.readContent(t.getPath(), StandardCharsets.UTF_8);
for (Map.Entry<String, String> substitution : substitutions.entrySet()) {
Expand All @@ -264,6 +270,22 @@ public void createFileFromTemplate(
}
}

@Override
public String readFile(Object path, Location location)
throws RepositoryFunctionException, EvalException, InterruptedException {
SkylarkPath p = getPath("read()", path);
WorkspaceRuleEvent w =
WorkspaceRuleEvent.newReadEvent(
p.toString(), rule.getLabel().toString(), location);
env.getListener().post(w);
try {
checkInOutputDirectory("read", p);
return FileSystemUtils.readContent(p.getPath(), StandardCharsets.ISO_8859_1);
} catch (IOException e) {
throw new RepositoryFunctionException(e, Transience.TRANSIENT);
}
}

// Create parent directories for the given path
private void makeDirectories(Path path) throws IOException {
Path parent = path.getParentDirectory();
Expand Down Expand Up @@ -397,7 +419,7 @@ public StructImpl download(
env.getListener().post(w);
Path downloadedPath;
try {
checkInOutputDirectory(outputPath);
checkInOutputDirectory("write", outputPath);
makeDirectories(outputPath.getPath());
downloadedPath =
httpDownloader.download(
Expand Down Expand Up @@ -481,7 +503,7 @@ public StructImpl downloadAndExtract(

// Download to outputDirectory and delete it after extraction
SkylarkPath outputPath = getPath("download_and_extract()", output);
checkInOutputDirectory(outputPath);
checkInOutputDirectory("write", outputPath);
createDirectory(outputPath.getPath());

Path downloadedPath;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,16 @@ public void symlink(Object from, Object to, Location location)
type = Boolean.class,
defaultValue = "True",
doc = "set the executable flag on the created file, true by default."),
@Param(
name = "legacy_utf8",
named = true,
type = Boolean.class,
defaultValue = "True",
doc =
"encode file content to UTF-8, true by default. Future versions will change"
+ " the default and remove this parameter."),
})
public void createFile(Object path, String content, Boolean executable, Location location)
public void createFile(Object path, String content, Boolean executable, Boolean legacyUtf8, Location location)
throws RepositoryFunctionExceptionT, EvalException, InterruptedException;

@SkylarkCallable(
Expand Down Expand Up @@ -189,6 +197,23 @@ public void createFileFromTemplate(
Location location)
throws RepositoryFunctionExceptionT, EvalException, InterruptedException;

@SkylarkCallable(
name = "read",
doc = "Reads the content of a file on the filesystem.",
useLocation = true,
parameters = {
@Param(
name = "path",
allowedTypes = {
@ParamType(type = String.class),
@ParamType(type = Label.class),
@ParamType(type = RepositoryPathApi.class)
},
doc = "path of the file to read from."),
})
public String readFile(Object path, Location location)
throws RepositoryFunctionExceptionT, EvalException, InterruptedException;

@SkylarkCallable(
name = "os",
structField = true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,16 +145,16 @@ public void testWhich() throws Exception {
@Test
public void testFile() throws Exception {
setUpContexForRule("test");
context.createFile(context.path("foobar"), "", true, null);
context.createFile(context.path("foo/bar"), "foobar", true, null);
context.createFile(context.path("bar/foo/bar"), "", true, null);
context.createFile(context.path("foobar"), "", true, true, null);
brandjon marked this conversation as resolved.
Show resolved Hide resolved
context.createFile(context.path("foo/bar"), "foobar", true, true, null);
context.createFile(context.path("bar/foo/bar"), "", true, true, null);

testOutputFile(outputDirectory.getChild("foobar"), "");
testOutputFile(outputDirectory.getRelative("foo/bar"), "foobar");
testOutputFile(outputDirectory.getRelative("bar/foo/bar"), "");

try {
context.createFile(context.path("/absolute"), "", true, null);
context.createFile(context.path("/absolute"), "", true, true, null);
fail("Expected error on creating path outside of the repository directory");
} catch (RepositoryFunctionException ex) {
assertThat(ex)
Expand All @@ -163,7 +163,7 @@ public void testFile() throws Exception {
.isEqualTo("Cannot write outside of the repository directory for path /absolute");
}
try {
context.createFile(context.path("../somepath"), "", true, null);
context.createFile(context.path("../somepath"), "", true, true, null);
fail("Expected error on creating path outside of the repository directory");
} catch (RepositoryFunctionException ex) {
assertThat(ex)
Expand All @@ -172,7 +172,7 @@ public void testFile() throws Exception {
.isEqualTo("Cannot write outside of the repository directory for path /somepath");
}
try {
context.createFile(context.path("foo/../../somepath"), "", true, null);
context.createFile(context.path("foo/../../somepath"), "", true, true, null);
fail("Expected error on creating path outside of the repository directory");
} catch (RepositoryFunctionException ex) {
assertThat(ex)
Expand All @@ -182,10 +182,47 @@ public void testFile() throws Exception {
}
}

@Test
public void testRead() throws Exception {
setUpContexForRule("test");
context.createFile(context.path("foo/bar"), "foobar", true, true, null);

String content = context.readFile(context.path("foo/bar"), null);
assertThat(content).isEqualTo("foobar");

try {
context.readFile(context.path("/absolute"), null);
fail("Expected error on reading path outside of the repository directory");
} catch (RepositoryFunctionException ex) {
assertThat(ex)
.hasCauseThat()
.hasMessageThat()
.isEqualTo("Cannot read outside of the repository directory for path /absolute");
}
try {
context.readFile(context.path("../somepath"), null);
fail("Expected error on reading path outside of the repository directory");
} catch (RepositoryFunctionException ex) {
assertThat(ex)
.hasCauseThat()
.hasMessageThat()
.isEqualTo("Cannot read outside of the repository directory for path /somepath");
}
try {
context.readFile(context.path("foo/../../somepath"), null);
fail("Expected error on reading path outside of the repository directory");
} catch (RepositoryFunctionException ex) {
assertThat(ex)
.hasCauseThat()
.hasMessageThat()
.isEqualTo("Cannot read outside of the repository directory for path /somepath");
}
}

@Test
public void testSymlink() throws Exception {
setUpContexForRule("test");
context.createFile(context.path("foo"), "foobar", true, null);
context.createFile(context.path("foo"), "foobar", true, true, null);

context.symlink(context.path("foo"), context.path("bar"), null);
testOutputFile(outputDirectory.getChild("bar"), "foobar");
Expand Down
69 changes: 69 additions & 0 deletions src/test/shell/bazel/bazel_workspaces_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,75 @@ function test_file() {
ensure_contains_exactly 'executable: true' 1
}

function test_file_nonascii() {
brandjon marked this conversation as resolved.
Show resolved Hide resolved
set_workspace_command 'repository_ctx.file("filefile.sh", "echo fïlëfïlë", True)'

build_and_process_log --exclude_rule "//external:local_config_cc"

ensure_contains_exactly 'location: .*repos.bzl:2:3' 1
ensure_contains_atleast 'rule: "//external:repo"' 1

# There are 3 file_event in external:repo as it is currently set up
ensure_contains_exactly 'file_event' 3
ensure_contains_exactly 'path: ".*filefile.sh"' 1
ensure_contains_exactly 'executable: true' 1

# This test file is in UTF-8, so the string passed to file() is UTF-8.
# Protobuf strings are Unicode encoded in UTF-8, so the logged text
# is double-encoded relative to the workspace command.
#
# >>> content = "echo f\u00EFl\u00EBf\u00EFl\u00EB".encode("utf8")
# >>> proto_content = content.decode("iso-8859-1").encode("utf8")
# >>> print("".join(chr(c) if c <= 0x7F else ("\\" + oct(c)[2:]) for c in proto_content))
# echo f\303\203\302\257l\303\203\302\253f\303\203\302\257l\303\203\302\253
# >>>

ensure_contains_exactly 'content: "echo f\\303\\203\\302\\257l\\303\\203\\302\\253f\\303\\203\\302\\257l\\303\\203\\302\\253"' 1
}

function test_read() {
jmillikin marked this conversation as resolved.
Show resolved Hide resolved
set_workspace_command '
content = "echo filefile"
repository_ctx.file("filefile.sh", content, True)
read_result = repository_ctx.read("filefile.sh")
if read_result != content:
fail("read(): expected %r, got %r" % (content, read_result))'

build_and_process_log --exclude_rule "//external:local_config_cc"

ensure_contains_exactly 'location: .*repos.bzl:4:3' 1
ensure_contains_exactly 'location: .*repos.bzl:5:17' 1
ensure_contains_atleast 'rule: "//external:repo"' 2

ensure_contains_exactly 'read_event' 1
ensure_contains_exactly 'path: ".*filefile.sh"' 2
}

function test_read_roundtrip_legacy_utf8() {
# See discussion on https://github.com/bazelbuild/bazel/pull/7309
set_workspace_command '
content = "echo fïlëfïlë"
repository_ctx.file("filefile.sh", content, True, legacy_utf8=True)
read_result = repository_ctx.read("filefile.sh")

corrupted_content = "echo fïlëfïlë"
if read_result != corrupted_content:
fail("read(): expected %r, got %r" % (corrupted_content, read_result))'

build_and_process_log --exclude_rule "//external:local_config_cc"
}

function test_read_roundtrip_nolegacy_utf8() {
set_workspace_command '
content = "echo fïlëfïlë"
repository_ctx.file("filefile.sh", content, True, legacy_utf8=False)
read_result = repository_ctx.read("filefile.sh")
if read_result != content:
fail("read(): expected %r, got %r" % (content, read_result))'

build_and_process_log --exclude_rule "//external:local_config_cc"
}

function test_os() {
set_workspace_command 'print(repository_ctx.os.name)'

Expand Down