Skip to content

Commit

Permalink
Implement repository_ctx.read() for reading local files.
Browse files Browse the repository at this point in the history
Fixes #3766

Closes #7309.

PiperOrigin-RevId: 238515082
  • Loading branch information
jmillikin authored and Copybara-Service committed Mar 14, 2019
1 parent d1c44d0 commit b11203d
Show file tree
Hide file tree
Showing 6 changed files with 197 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,23 @@ 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,38 @@ 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 {
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 +253,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 +272,21 @@ 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 +420,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 @@ -447,7 +470,7 @@ public void extract(Object archive, Object output, String stripPrefix, Location
}

SkylarkPath outputPath = getPath("extract()", output);
checkInOutputDirectory(outputPath);
checkInOutputDirectory("write", outputPath);

WorkspaceRuleEvent w =
WorkspaceRuleEvent.newExtractEvent(
Expand Down Expand Up @@ -491,7 +514,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,17 @@ 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 +198,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);
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 @@ -342,6 +342,75 @@ function test_file() {
ensure_contains_exactly 'executable: true' 1
}

function test_file_nonascii() {
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() {
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

0 comments on commit b11203d

Please sign in to comment.