Skip to content

Commit

Permalink
[singlejar] Port test_util to Windows
Browse files Browse the repository at this point in the history
- Port various functions in `test_util` to Windows.
- Change `@local_jdk//:jdk-default` (target does not seem to exist) to `@local_jdk//:jdk`.

See #2241

Closes #6248.

PiperOrigin-RevId: 217138180
  • Loading branch information
rongjiecomputer authored and Copybara-Service committed Oct 15, 2018
1 parent 21c975d commit 68611b3
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 26 deletions.
40 changes: 21 additions & 19 deletions src/tools/singlejar/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,6 @@
# singlejar C++ implementation.
package(default_visibility = ["//src:__subpackages__"])

JAR_TOOL_PATH_COPT_TPL = "-DJAR_TOOL_PATH=\\\"external/local_jdk/bin/jar%s\\\""

JAR_TOOL_PATH_COPTS = select({
"//src/conditions:windows": [JAR_TOOL_PATH_COPT_TPL % ".exe"],
"//conditions:default": [JAR_TOOL_PATH_COPT_TPL % ""],
})

filegroup(
name = "srcs",
srcs = glob(["**"]),
Expand Down Expand Up @@ -99,7 +92,7 @@ cc_test(
cc_test(
name = "desugar_checking_test",
srcs = [
"desugar_checking_test.cc",
"combiners_test.cc",

This comment has been minimized.

Copy link
@rongjiecomputer

rongjiecomputer Oct 16, 2018

Author Contributor

@laszlocsomor Why this line was reverted to combiners_test.cc again? Broke internal test?

This comment has been minimized.

Copy link
@laszlocsomor

laszlocsomor Oct 16, 2018

Contributor

Huh, good question! I thought it was in the original PR by you, but maybe it was a bad merge. It looks wrong indeed. Let me fix that in #6404. Sorry about this mess. :(

":zip_headers",
":zlib_interface",
],
Expand Down Expand Up @@ -151,13 +144,13 @@ cc_test(
"input_jar_scan_entries_test.h",
"input_jar_scan_jartool_test.cc",
],
copts = JAR_TOOL_PATH_COPTS,
data = [
"@local_jdk//:jar",
"@local_jdk//:jdk",
],
# TODO(@rongjiecomputer): update copts to handle Windows and add
# ".exe" extension.
copts = ["-DJAR_TOOL_PATH=$(JAVA)/bin/jar"],
data = ["@bazel_tools//tools/jdk:current_java_runtime"],
# Timing out, see https://github.com/bazelbuild/bazel/issues/1555
tags = ["manual"],
toolchains = ["@bazel_tools//tools/jdk:current_java_runtime"],
deps = [
":input_jar",
":test_util",
Expand Down Expand Up @@ -223,20 +216,23 @@ cc_test(
srcs = [
"output_jar_simple_test.cc",
],
copts = JAR_TOOL_PATH_COPTS,
# TODO(@rongjiecomputer): update copts to handle Windows and add
# ".exe" extension.
copts = ["-DJAR_TOOL_PATH=$(JAVA)/bin/jar"],
data = [
":data1",
":data2",
":stored_jar",
":test1",
":test2",
"@local_jdk//:jar",
"@local_jdk//:jdk-default",
"@bazel_tools//tools/jdk:current_java_runtime",
],
toolchains = ["@bazel_tools//tools/jdk:current_java_runtime"],
deps = [
":input_jar",
":options",
":output_jar",
":port",
":test_util",
"//src/main/cpp/util",
"@com_google_googletest//:gtest_main",
Expand Down Expand Up @@ -305,8 +301,8 @@ sh_test(
"//src/test/shell:bashunit",
"@bazel_tools//tools/bash/runfiles",
"@bazel_tools//tools/jdk:current_java_runtime",
"@local_jdk//:jar",
],
toolchains = ["@bazel_tools//tools/jdk:current_java_runtime"],
deps = ["//src/test/shell:bashunit"],
)

Expand Down Expand Up @@ -409,10 +405,15 @@ cc_library(

cc_library(
name = "test_util",
testonly = 1,
srcs = ["test_util.cc"],
hdrs = ["test_util.h"],
deps = [
"//src/main/cpp/util",
# TODO(laszlocsomor) Use @bazel_tools//tools/cpp/runfiles after Bazel is
# released with
# https://github.com/bazelbuild/bazel/commit/23bc3bee79d8a8b8dc15bbfb6072ec9f965dff96.
"//tools/cpp/runfiles:runfiles_src_for_singlejar_only",
"@com_google_googletest//:gtest_main",
],
)
Expand Down Expand Up @@ -486,6 +487,7 @@ genrule(
"@bazel_tools//tools/jdk:current_java_runtime",
],
outs = ["stored.jar"],
cmd = "$(location @local_jdk//:jar) -0cf \"$@\" $(location :output_jar.cc)",
tools = ["@local_jdk//:jar"],
cmd = "$(location @bazel_tools//tools/jdk:current_java_runtime) -0cf \"$@\" $(location :output_jar.cc)",
toolchains = ["@bazel_tools//tools/jdk:current_java_runtime"],
tools = ["@bazel_tools//tools/jdk:current_java_runtime"],
)
27 changes: 27 additions & 0 deletions src/tools/singlejar/test_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@
#include <stdlib.h>
#include <sys/types.h>

#ifndef _WIN32
#include <unistd.h>
#endif

#include <string>

#include "src/main/cpp/util/file.h"
Expand All @@ -27,9 +31,25 @@

#include "googletest/include/gtest/gtest.h"

#ifdef _WIN32
#define popen _popen
#define pclose _pclose
#endif

namespace singlejar_test_util {

bool AllocateFile(const string &name, size_t size) {
#ifdef _WIN32
int fd = _sopen(name.c_str(), _O_RDWR | _O_CREAT | _O_BINARY, _SH_DENYNO,
_S_IREAD | _S_IWRITE);
int success = _chsize_s(fd, size);
_close(fd);
if (success < 0) {
perror(strerror(errno));
return false;
}
return true;
#else
int fd = open(name.c_str(), O_CREAT | O_RDWR | O_TRUNC, 0777);
if (fd < 0) {
perror(name.c_str());
Expand All @@ -47,6 +67,7 @@ bool AllocateFile(const string &name, size_t size) {
} else {
return close(fd) == 0;
}
#endif
}

int RunCommand(const char *cmd, ...) {
Expand Down Expand Up @@ -87,7 +108,13 @@ string GetEntryContents(const string &zip_path, const string &entry_name) {
string command;
blaze_util::StringPrintf(&command, "unzip -p %s %s", zip_path.c_str(),
entry_name.c_str());
#ifdef _WIN32
// "b" is Microsoft-specific. It's necessary, because without it the popen
// implementation will strip "\r\n" to "\n" and make many tests fail.
FILE *fp = popen(command.c_str(), "rb");
#else
FILE *fp = popen(command.c_str(), "r");
#endif
if (!fp) {
ADD_FAILURE() << "Command " << command << " failed.";
return string("");
Expand Down
3 changes: 2 additions & 1 deletion src/tools/singlejar/test_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@
// See the License for the specific language governing permissions and
// limitations under the License.

#include <unistd.h>
#include <string>

#include "tools/cpp/runfiles/runfiles_src.h"

namespace singlejar_test_util {
using std::string;

Expand Down
4 changes: 1 addition & 3 deletions tools/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ filegroup(
"//tools/platforms:srcs",
"//tools/genrule:srcs",
"//tools/cpp:srcs",
"//tools/cpp/runfiles:srcs",
"//tools/j2objc:srcs",
"//tools/objc:srcs",
"//tools/osx:srcs",
Expand All @@ -45,8 +44,7 @@ filegroup(
"//tools/build_rules:embedded_tools_srcs",
"//tools/buildstamp:srcs",
"//tools/coverage:srcs",
"//tools/cpp:srcs",
"//tools/cpp/runfiles:embedded_tools",
"//tools/cpp:embedded_tools",
"//tools/genrule:srcs",
"//tools/java:embedded_tools",
"//tools/j2objc:srcs",
Expand Down
11 changes: 10 additions & 1 deletion tools/cpp/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,16 @@ filegroup(

filegroup(
name = "srcs",
srcs = glob(["**"]),
srcs = glob(["**"]) + [
"//tools/cpp/runfiles:srcs",
],
)

filegroup(
name = "embedded_tools",
srcs = glob(["**"]) + [
"//tools/cpp/runfiles:embedded_tools",
],
)

filegroup(
Expand Down
21 changes: 19 additions & 2 deletions tools/cpp/runfiles/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ filegroup(
"*~",
],
),
visibility = ["//tools:__pkg__"],
visibility = ["//tools/cpp:__pkg__"],
)

filegroup(
Expand All @@ -18,7 +18,7 @@ filegroup(
"BUILD.tools",
":srcs_for_embedded_tools",
],
visibility = ["//tools:__pkg__"],
visibility = ["//tools/cpp:__pkg__"],
)

genrule(
Expand All @@ -44,6 +44,23 @@ cc_library(
hdrs = ["runfiles_src.h"],
)

# As of 2018-10-01, singlejar needs to temporarily use the C++ runfiles library
# from source, not from @bazel_tools.
# Nothing else should depend on this target.
#
# TODO(laszlocsomor): remove this library after Bazel is released with
# https://github.com/bazelbuild/bazel/commit/23bc3bee79d8a8b8dc15bbfb6072ec9f965dff96.
# Once Bazel is released with that patch, do the following:
# 1. remove this library
# 2. let Singlejar depend on @bazel_tools//tools/cpp/runfiles
# 3. change Singlejar's sources to include "tools/cpp/runfiles.h" instead of
# "tools/cpp/runfiles_src.h"
cc_library(
name = "runfiles_src_for_singlejar_only",
visibility = ["//src/tools/singlejar:__pkg__"],
deps = [":runfiles"],
)

cc_test(
name = "runfiles_test",
srcs = ["runfiles_test.cc"],
Expand Down

0 comments on commit 68611b3

Please sign in to comment.