Build Raylet with Bazel by pcmoritz · Pull Request #3806 · ray-project/ray

pcmoritz · 2019-01-19T00:01:00Z

This allows to build the raylet with

bazel build -c opt ...

in the root directory.

This is based off #2899. Related to #2887.

pcmoritz · 2019-01-19T00:02:35Z

+      "src/ray/thirdparty/hiredis/*.h",
+      "src/ray/thirdparty/hiredis/adapters/*.h",
+      "src/ray/thirdparty/hiredis/dict.c",
+      "src/ray/thirdparty/ae/ae_kqueue.c"


@ericl What's the best way to do this in a platform independent way? On linux we need ae_epoll.c here.

ericl · 2019-01-19T00:30:55Z

Is it possible to move the logic into an ifdef? There's also https://docs.bazel.build/versions/master/platforms.html#defining-constraints-and-platforms

…

On Fri, Jan 18, 2019, 4:02 PM Philipp Moritz ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In BUILD <#3806 (comment)>: > + ], + includes = ["src/ray/thirdparty"], +) + +cc_library( + name = "hiredis", + srcs = glob([ + "src/ray/thirdparty/ae/ae.c", + "src/ray/thirdparty/hiredis/*.c", + ]), + hdrs = glob([ + "src/ray/thirdparty/ae/*.h", + "src/ray/thirdparty/hiredis/*.h", + "src/ray/thirdparty/hiredis/adapters/*.h", + "src/ray/thirdparty/hiredis/dict.c", + "src/ray/thirdparty/ae/ae_kqueue.c" @ericl <https://github.com/ericl> What's the best way to do this in a platform independent way? On linux we need qe_epoll.c here. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3806 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAA6SlbnazDNtixInSi2OY8Za3ruyNFCks5vEmChgaJpZM4aI1Oj> .

AmplabJenkins · 2019-01-19T02:41:04Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10948/
Test PASSed.

AmplabJenkins · 2019-01-19T04:37:34Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10949/
Test PASSed.

AmplabJenkins · 2019-01-19T04:47:45Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10952/
Test PASSed.

AmplabJenkins · 2019-01-19T05:00:22Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10950/
Test PASSed.

AmplabJenkins · 2019-01-19T05:10:19Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10951/
Test PASSed.

AmplabJenkins · 2019-01-19T05:29:13Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10953/
Test PASSed.

AmplabJenkins · 2019-01-19T10:44:10Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10963/
Test FAILed.

AmplabJenkins · 2019-01-19T13:17:01Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10964/
Test PASSed.

ericl · 2019-01-19T21:52:35Z

@@ -0,0 +1,80 @@
+load("@com_github_google_flatbuffers//:build_defs.bzl", "flatbuffer_cc_library")


Should this file be eventually moved to the plasma repo?

absolutely!

robertnishihara · 2019-01-19T21:56:46Z

@rsepassi could you take a quick look and see if things look reasonable?

AmplabJenkins · 2019-01-20T07:22:18Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10981/
Test PASSed.

AmplabJenkins · 2019-01-20T07:51:10Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10982/
Test PASSed.

robertnishihara · 2019-01-20T08:27:02Z


+      # Test Bazel build
+      - ./.travis/install-bazel.sh
+      - bazel build ...


It's fine to put it here for now, but this is temporary, right?

Yeah, eventually everything should be switched, for now it just makes sure it doesn't get broken. This is the build matrix entry that is the least loaded (aside from linting) and has java already installed, so it's the natural candidate.

robertnishihara · 2019-01-20T08:28:52Z

+
+OS=linux
+ARCH=x86_64
+if [[ "$TRAVIS_OS_NAME" == "osx" ]]; then OS=darwin; fi


Possibly not a huge deal, but might as well make this script run outside of Travis. The way we normally detect OS in Travis is

ray/.travis/install-dependencies.sh

Lines 7 to 18 in aad48ee

platform="unknown"

unamestr="$(uname)"

if [[ "$unamestr" == "Linux" ]]; then

echo "Platform is linux."

platform="linux"

elif [[ "$unamestr" == "Darwin" ]]; then

echo "Platform is macosx."

platform="macosx"

else

echo "Unrecognized platform."

exit 1

fi

good idea yeah, I fixed it

robertnishihara · 2019-01-20T08:30:02Z

@@ -0,0 +1,309 @@
+# Bazel build


Any reason to prefer BUILD or BUILD.bazel?

yes, BUILD wouldn't work because we already have a build directory and macOS is case-insensitive (I found out the hard way)

robertnishihara · 2019-01-20T08:33:10Z

+    srcs = glob([
+      "src/ray/thirdparty/ae/ae.c",
+      "src/ray/thirdparty/hiredis/*.c",
+    ], exclude = ["src/ray/thirdparty/hiredis/test.c"]),


exclude should probably be on a new line

or maybe not? not sure what the proper formatting is

exclude is part of the glob command so it makes sense to me to have it on the same line

robertnishihara · 2019-01-20T08:34:43Z

+
+cc_library(
+    name = "gcs",
+    srcs = glob(


The formatting here is a bit different from before

I'll try to make it more consistent

robertnishihara · 2019-01-20T08:36:30Z

+git_repository(
+    name = "com_github_nelhage_rules_boost",
+    commit = "6d6fd834281cb8f8e758dd9ad76df86304bf1869",
+    remote = "https://github.com/nelhage/rules_boost",


Is this official or just some random repo on the internet?

There is no official BAZEL support for boost, so we can be glad to have this random repo. It's the best I could find so far, short of writing build files for boost ourselves.

robertnishihara · 2019-01-20T08:37:18Z

@@ -0,0 +1,29 @@
+load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository", "new_git_repository")
+
+git_repository(


Do you need this git_repository command in addition to the load command below?

load brings "git_repository" into the namespace (this is needed to be compatible with the latest version of bazel)

robertnishihara · 2019-01-20T08:38:32Z

@@ -0,0 +1,82 @@
+load("@com_github_google_flatbuffers//:build_defs.bzl", "flatbuffer_cc_library")
+


Is the bazel/BUILD file necessary?

yes, so Bazel can find the plasma build file, which now needs to be a file registered with Bazel (this is needed since the new version of the new_git_command, but is backward compatible fortunately)

robertnishihara · 2019-01-20T08:39:53Z

+    name = "plasma",
+    visibility = ["//visibility:public"],
+    hdrs = [
+        "cpp/src/plasma/client.h",


What's the convention for when to list all the files versus to use *.h?

no convention, if all the .h are included, I put *.h, but if it's only a small subset I list them

robertnishihara · 2019-01-20T08:40:14Z

+    includes = ["cpp/src/plasma/format/common.fbs"]
+)
+
+exports_files(["cpp/src/plasma/format/common.fbs"])


What does this line do?

it makes the file available from outside of the plasma env (i.e. for ray)

AmplabJenkins · 2019-01-20T09:04:33Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10983/
Test PASSed.

AmplabJenkins · 2019-01-20T09:06:23Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10984/
Test FAILed.

AmplabJenkins · 2019-01-20T11:17:48Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10986/
Test PASSed.

AmplabJenkins · 2019-01-20T11:45:39Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/10987/
Test PASSed.

rsepassi · 2019-01-22T17:11:55Z

LGTM! Hope it makes builds easier. Was having the half-baked BUILD file I sent a while back helpful or was it easier to just start from scratch?

robertnishihara · 2019-01-22T19:40:37Z

Thanks @rsepassi the earlier PR was very helpful as a starting point. A lot has changed in Ray since then, but it definitely sped up the process. We're still in the process of switching entirely to Bazel, so may ask you to comment on a few more PRs in the future.

rsepassi · 2019-01-22T23:34:28Z

Sounds good!

…

On Tue, Jan 22, 2019 at 11:40 AM Robert Nishihara ***@***.***> wrote: Thanks @rsepassi <https://github.com/rsepassi> the earlier PR was very helpful as a starting point. A lot has changed in Ray since then, but it definitely sped up the process. We're still in the process of switching entirely to Bazel, so may ask you to comment on a few more PRs in the future. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3806 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABEGW5o2_1k2ySxESa9tJFSynn3N5I_kks5vF2k7gaJpZM4aI1Oj> .

pcmoritz · 2019-01-23T00:11:04Z

Yes, it was definitely helpful to start with the build file you provided.

The next step will be to integrate the python wrappers for the backend with Bazel.

pcmoritz added 4 commits January 18, 2019 15:44

build raylet with bazel

9088f67

update

03f48c1

format

c2a3d05

arrow changes

30417e0

pcmoritz assigned ericl Jan 19, 2019

pcmoritz commented Jan 19, 2019

View reviewed changes

pcmoritz added 7 commits January 18, 2019 17:56

add test

a506a3e

update

604502d

fix

d2be430

fix

ffd24d6

update

9ef4fb5

update

8e15dc0

fix linux

2452f4c

pcmoritz added 3 commits January 19, 2019 01:59

fix

dd52777

update

ee79e55

update

56abc3c

pcmoritz added 2 commits January 19, 2019 12:50

update

1b21c48

update arrow

d276725

ericl reviewed Jan 19, 2019

View reviewed changes

update bazel

7498b3c

fixes

9ce3d21

pcmoritz changed the title ~~[WIP] Build Raylet with Bazel~~ Jan 20, 2019

robertnishihara reviewed Jan 20, 2019

View reviewed changes

pcmoritz added 2 commits January 20, 2019 01:02

fixes

418b01c

fix

16f0963

update

818be0f

robertnishihara approved these changes Jan 20, 2019

View reviewed changes

robertnishihara merged commit 0dad4e6 into ray-project:master Jan 20, 2019

robertnishihara deleted the bazelize-raylet branch January 20, 2019 20:16

robertnishihara mentioned this pull request Jan 22, 2019

Partial Bazel BUILD file #2899

Closed

micafan mentioned this pull request Jan 28, 2019

Separate different libs for ray backend code. #3879

Closed

7 tasks

		@@ -0,0 +1,80 @@
		load("@com_github_google_flatbuffers//:build_defs.bzl", "flatbuffer_cc_library")

	platform="unknown"
	unamestr="$(uname)"
	if [[ "$unamestr" == "Linux" ]]; then
	echo "Platform is linux."
	platform="linux"
	elif [[ "$unamestr" == "Darwin" ]]; then
	echo "Platform is macosx."
	platform="macosx"
	else
	echo "Unrecognized platform."
	exit 1
	fi

		@@ -0,0 +1,29 @@
		load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository", "new_git_repository")

		git_repository(

		@@ -0,0 +1,82 @@
		load("@com_github_google_flatbuffers//:build_defs.bzl", "flatbuffer_cc_library")

Uh oh!

Conversation

pcmoritz commented Jan 19, 2019 • edited by robertnishihara Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

pcmoritz Jan 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

ericl commented Jan 19, 2019 via email

AmplabJenkins commented Jan 19, 2019

AmplabJenkins commented Jan 19, 2019

AmplabJenkins commented Jan 19, 2019

AmplabJenkins commented Jan 19, 2019

AmplabJenkins commented Jan 19, 2019

AmplabJenkins commented Jan 19, 2019

AmplabJenkins commented Jan 19, 2019

AmplabJenkins commented Jan 19, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robertnishihara commented Jan 19, 2019

AmplabJenkins commented Jan 20, 2019

AmplabJenkins commented Jan 20, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pcmoritz Jan 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

AmplabJenkins commented Jan 20, 2019

AmplabJenkins commented Jan 20, 2019

AmplabJenkins commented Jan 20, 2019

AmplabJenkins commented Jan 20, 2019

rsepassi commented Jan 22, 2019

robertnishihara commented Jan 22, 2019

rsepassi commented Jan 22, 2019 via email

pcmoritz commented Jan 23, 2019

Labels

5 participants

pcmoritz commented Jan 19, 2019 •

edited by robertnishihara

Loading

pcmoritz Jan 19, 2019 •

edited

Loading

pcmoritz Jan 20, 2019 •

edited

Loading