Skip to content

Commit

Permalink
runfiles,C++: move to //tools/cpp/runfiles
Browse files Browse the repository at this point in the history
Move the C++ runfiles library to the location of
the rest of the C++ tools.

Also change the C++ namespace to reflect the
directory hierarchy.

We have not yet announced nor released the C++
runfiles library so these refactorings are fine.

See #4460

Closes #4873.

Change-Id: I1732ef1eaff880cae05b7d218a3b1c0461a6b029
PiperOrigin-RevId: 189874201
  • Loading branch information
laszlocsomor authored and Copybara-Service committed Mar 21, 2018
1 parent cdc269b commit 8a5a0a3
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 46 deletions.
2 changes: 1 addition & 1 deletion src/main/cpp/util/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ cc_library(
":ijar",
"//src/test/cpp/util:__pkg__",
"//src/tools/launcher:__subpackages__",
"//src/tools/runfiles:__pkg__",
"//src/tools/singlejar:__pkg__",
"//third_party/def_parser:__pkg__",
"//tools/cpp/runfiles:__pkg__",
],
deps = [
":blaze_exit_code",
Expand Down
31 changes: 0 additions & 31 deletions src/tools/runfiles/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ filegroup(
name = "embedded_tools",
srcs = [
"BUILD.tools",
"runfiles.cc",
"runfiles.h",
"runfiles.py",
"//src/tools/runfiles/java/com/google/devtools/build/runfiles:embedded_tools",
],
Expand All @@ -40,35 +38,6 @@ py_test(
deps = [":py-runfiles"],
)

cc_library(
name = "cc-runfiles",
srcs = ["runfiles.cc"],
hdrs = ["runfiles.h"],
# This library is available to clients under
# @bazel_tools//tools/runfiles:cc-runfiles, with the same source files.
# The include statement in runfiles.cc that includes runfiles.h must work
# both here in the //src/tools/runfiles package, and in the
# @bazel_tools//tools/runfiles package.
# runfiles.cc includes "tools/runfiles/runfiles.h" so we need to tell the
# compiler to prepend "src" to it so the include path is valid.
# Alternatively we could omit this "copts" attribute here and add some
# include path manipulating attributes to
# @bazel_tools//tools/runfiles:cc-runfiles instead -- that would work too,
# but I (laszlocsomor@) find this solution (i.e. the "copts" attribute on
# this rule) to be simpler.
copts = ["-Isrc"],
)

cc_test(
name = "cc-runfiles-test",
srcs = ["runfiles_test.cc"],
deps = [
":cc-runfiles",
"//src/main/cpp/util:file",
"//third_party:gtest",
],
)

sh_library(
name = "runfiles_sh_lib",
srcs = ["runfiles.sh"],
Expand Down
2 changes: 1 addition & 1 deletion tools/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ filegroup(
"//tools/build_rules:embedded_tools_srcs",
"//tools/buildstamp:srcs",
"//tools/coverage:srcs",
"//tools/cpp:srcs",
"//tools/cpp:embedded_tools",

This comment has been minimized.

Copy link
@hwright

hwright Mar 21, 2018

After pulling this commit locally, I get the following compilation error:

$ bazel build //src:bazel
ERROR: /Users/hwright/dev/bazel/tools/BUILD:39:1: no such target '//tools/cpp:embedded_tools': target 'embedded_tools' not declared in package 'tools/cpp' defined by /Users/hwright/dev/bazel/tools/cpp/BUILD and referenced by '//tools:embedded_tools_srcs'
ERROR: Analysis of target '//src:bazel' failed; build aborted: Loading failed
INFO: Elapsed time: 0.193s
FAILED: Build did NOT complete successfully (0 packages loaded)
    currently loading: src/main/java/com/google/devtools/build/lib/actions ... (19 packages)
$

This comment has been minimized.

Copy link
@laszlocsomor

laszlocsomor Mar 21, 2018

Author Contributor

Ah, sorry about that! Let me roll it back.

This comment has been minimized.

Copy link
@laszlocsomor

laszlocsomor Mar 21, 2018

Author Contributor

Rollback submission is still in progress...
Meanwhile I'm investigating how could this happen.

This comment has been minimized.

Copy link
@hwright

hwright Mar 21, 2018

Thanks!

This comment has been minimized.

Copy link
@laszlocsomor

laszlocsomor Mar 21, 2018

Author Contributor

It's clear that I made a mistake: I failed to notice that internal tests were legitimately broken, dismissing them as flakes, and submitting anyway.
There was also potentially an infrastructure failure: either we didn't import the changes of //tools/cpp/BUILD and so the PR was merged without those (which was the direct cause of the build breakage), or we did and there was another infrastructure or human error. I'm still investigating which one.

This comment has been minimized.

Copy link
@laszlocsomor

laszlocsomor Mar 21, 2018

Author Contributor

FYI: rolled back in 33c0885

"//tools/genrule:srcs",
"//tools/j2objc:srcs",
"//tools/jdk:package-srcs",
Expand Down
31 changes: 31 additions & 0 deletions tools/cpp/runfiles/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
filegroup(
name = "srcs",
srcs = glob(["**"]),
visibility = ["//tools/cpp:__pkg__"],
)

filegroup(
name = "embedded_tools",
srcs = [
"BUILD.tools",
"runfiles.cc",
"runfiles.h",
],
visibility = ["//tools/cpp:__pkg__"],
)

cc_library(
name = "runfiles",
srcs = ["runfiles.cc"],
hdrs = ["runfiles.h"],
)

cc_test(
name = "runfiles-test",
srcs = ["runfiles_test.cc"],
deps = [
":runfiles",
"//src/main/cpp/util:file",
"//third_party:gtest",
],
)
2 changes: 2 additions & 0 deletions tools/cpp/runfiles/BUILD.tools
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# This package will host the C++ runfiles library when it's ready.
# Follow the progress on https://github.com/bazelbuild/bazel/issues/4460
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
#include "tools/runfiles/runfiles.h"
#include "tools/cpp/runfiles/runfiles.h"

#ifdef COMPILER_MSVC
#include <windows.h>
Expand All @@ -32,6 +32,8 @@
#endif // COMPILER_MSVC

namespace bazel {
namespace tools {
namespace cpp {
namespace runfiles {

using std::function;
Expand Down Expand Up @@ -314,4 +316,6 @@ Runfiles* Runfiles::CreateDirectoryBased(const string& directory_path,
}

} // namespace runfiles
} // namespace cpp
} // namespace tools
} // namespace bazel
21 changes: 12 additions & 9 deletions src/tools/runfiles/runfiles.h → tools/cpp/runfiles/runfiles.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@
//
// Usage:
//
// #include "tools/runfiles/runfiles.h"
// #include "tools/cpp/runfiles/runfiles.h"
// ...
// using namespace bazel::tools::cpp::runfiles::Runfiles;
//
// int main(int argc, char** argv) {
// std::string error;
// std::unique_ptr<bazel::runfiles::Runfiles> runfiles(
// bazel::runfiles::Runfiles::Create(argv[0], &error));
// std::unique_ptr<Runfiles> runfiles(Runfiles::Create(argv[0], &error));
// if (runfiles == nullptr) {
// ... // error handling
// }
Expand All @@ -39,19 +39,18 @@
// If you want to explicitly create a manifest- or directory-based
// implementation, you can do so as follows:
//
// std::unique_ptr<bazel::runfiles::Runfiles> runfiles1(
// bazel::runfiles::Runfiles::CreateManifestBased(
// std::unique_ptr<Runfiles> runfiles1(
// Runfiles::CreateManifestBased(
// "path/to/foo.runfiles/MANIFEST", &error));
//
// std::unique_ptr<bazel::runfiles::Runfiles> runfiles2(
// bazel::runfiles::Runfiles::CreateDirectoryBased(
// std::unique_ptr<Runfiles> runfiles2(
// Runfiles::CreateDirectoryBased(
// "path/to/foo.runfiles", &error));
//
// If you want to start child processes that also need runfiles, you need to set
// the right environment variables for them:
//
// std::unique_ptr<bazel::runfiles::Runfiles> runfiles(
// bazel::runfiles::Runfiles::Create(argv[0], &error));
// std::unique_ptr<Runfiles> runfiles(Runfiles::Create(argv[0], &error));
//
// for (const auto i : runfiles->EnvVars()) {
// setenv(i.first, i.second, 1);
Expand All @@ -70,6 +69,8 @@
#include <vector>

namespace bazel {
namespace tools {
namespace cpp {
namespace runfiles {

class Runfiles {
Expand Down Expand Up @@ -179,6 +180,8 @@ bool TestOnly_IsAbsolute(const std::string& path);

} // namespace testing
} // namespace runfiles
} // namespace cpp
} // namespace tools
} // namespace bazel

#endif // BAZEL_SRC_TOOLS_RUNFILES_RUNFILES_H_
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

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

#ifdef COMPILER_MSVC
#include <windows.h>
Expand All @@ -31,11 +31,13 @@
#define LINE() T(__LINE__)

namespace bazel {
namespace tools {
namespace cpp {
namespace runfiles {
namespace {

using bazel::runfiles::testing::TestOnly_CreateRunfiles;
using bazel::runfiles::testing::TestOnly_IsAbsolute;
using bazel::tools::cpp::runfiles::testing::TestOnly_CreateRunfiles;
using bazel::tools::cpp::runfiles::testing::TestOnly_IsAbsolute;
using std::cerr;
using std::endl;
using std::function;
Expand Down Expand Up @@ -470,4 +472,6 @@ TEST_F(RunfilesTest, IsAbsolute) {

} // namespace
} // namespace runfiles
} // namespace cpp
} // namespace tools
} // namespace bazel

0 comments on commit 8a5a0a3

Please sign in to comment.