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

bazel: add freebsd support #57039

Merged
merged 1 commit into from
Nov 24, 2020
Merged

Conversation

irfansharif
Copy link
Contributor

@irfansharif irfansharif commented Nov 24, 2020

Fixes #56013. We're picking up bazel-contrib/rules_foreign_cc/pull/387
(included in our new fork at cockroachdb/rules_foreign_cc). We've also
picked up Oliver's PR adding autoconf support
(bazel-contrib/rules_foreign_cc/pull/432); we were pointing to Oliver's own
fork previously to pick up those changes.

Release note: None


@knz, wanna try this branch out and see if it works? If not, I should probably spin up a freebsd AMI next once someone shows me how.

@irfansharif irfansharif requested review from knz, otan and a team November 24, 2020 00:05
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Fixes cockroachdb#56013. We're picking up bazel-contrib/rules_foreign_cc/pull/387
(included in our new fork at cockroachdb/rules_foreign_cc). We've also
picked up Oliver's PR adding autoconf support
(bazel-contrib/rules_foreign_cc/pull/432); we were pointing to Oliver's own
fork previously to pick up those changes.

Release note: None
@otan
Copy link
Contributor

otan commented Nov 24, 2020

i think bazel-contrib/rules_foreign_cc#387 needs to pick up changes from bazel-contrib/rules_foreign_cc#377. looks like a straightforward delete of unsupported linux flags in bsd, if you were looking for copy paste.

@irfansharif
Copy link
Contributor Author

irfansharif commented Nov 24, 2020

This is the diff I have after cherry-picking those SHAs (I think bazel-contrib/rules_foreign_cc#387 was already rebased atop bazel-contrib/rules_foreign_cc#377).

$ pwd
/Users/irfansharif/Software/src/github.com/cockroachdb/rules_foreign_cc/tools/build_defs/shell_toolchain/toolchains/impl
$ git diff HEAD:./bsd_commands.bzl ./linux_commands.bzl
diff --git o/tools/build_defs/shell_toolchain/toolchains/impl/bsd_commands.bzl w/tools/build_defs/shell_toolchain/toolchains/impl/linux_commands.bzl
index 99b73f7..4fdceb8 100644
--- o/tools/build_defs/shell_toolchain/toolchains/impl/bsd_commands.bzl
+++ w/tools/build_defs/shell_toolchain/toolchains/impl/linux_commands.bzl
@@ -3,7 +3,7 @@ load("@rules_foreign_cc//tools/build_defs/shell_toolchain/toolchains:function_an
 _REPLACE_VALUE = "\\${EXT_BUILD_DEPS}"

 def os_name():
-    return "bsd"
+    return "linux"

 def pwd():
     return "$(pwd)"
@@ -61,7 +61,7 @@ fi
     )

 def copy_dir_contents_to_dir(source, target):
-    return """gcp -L -r --no-target-directory "{}" "{}" """.format(source, target)
+    return """cp -L -r --no-target-directory "{}" "{}" """.format(source, target)

 def symlink_contents_to_dir(source, target):
     text = """local target="$2"
@@ -84,9 +84,9 @@ def symlink_to_dir(source, target):
     text = """local target="$2"
 mkdir -p "$target"
 if [[ -f "$1" ]]; then
-  gln -s -t "$target" "$1"
+  ln -s -f -t "$target" "$1"
 elif [[ -L "$1" ]]; then
-  gcp $1 $2
+  cp $1 $2
 elif [[ -d "$1" ]]; then
   local children=$(find -H "$1" -maxdepth 1 -mindepth 1)
   local dirname=$(basename "$1")

Where it's reaching for the gnu coreutils ls/cp instead. Are these typically installed on freebsd machines? Too cumbersome to ask as a pre-req?

Copy link
Contributor

@otan otan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah makes sense. in that case, LGTM

feel free to merge and ask @knz to check when he's up the next morning :)

@irfansharif
Copy link
Contributor Author

irfansharif commented Nov 24, 2020

Cause we still haven't really pinned toolchains, it'll still probably not work for Rafa (though we're one step closer). Just tried it on my gceworker and I think it's not using the right compilers. References for myself for later:

@knz
Copy link
Contributor

knz commented Nov 24, 2020

  1. the original error is not there any more 🎉

  1. still doesn't build, but due to something else:
make: "/tmp/tmp.q7mGn1T3/Makefile" line 3: Need an operator
make: "/tmp/tmp.q7mGn1T3/Makefile" line 4: Need an operator
make: "/tmp/tmp.q7mGn1T3/Makefile" line 72: Missing dependency operator
make: "/tmp/tmp.q7mGn1T3/Makefile" line 74: Need an operator
make: "/tmp/tmp.q7mGn1T3/Makefile" line 75: Missing dependency operator
make: "/tmp/tmp.q7mGn1T3/Makefile" line 77: Need an operator
make: "/tmp/tmp.q7mGn1T3/Makefile" line 79: Need an operator

The problem here is that BSD make does not work, I need to be able to override it (to gmake). Typically with a Makefile I'd run gmake at the top level and it will "inherit" to all sub-directories via $MAKE. However, our rules do not seem to recognize $MAKE.

If I go and manually change the strings in c-deps/BUILD.bazel to change occurrences of make to gmake, then this brings me one step further!

(Ideally these rules would default to make but accept an override via an env var.)

Finally after make goes forward, I get a different link error


    • this time, probably for @otan
ERROR: /data/home/kena/src/go/src/github.com/cockroachdb/cockroach/pkg/geo/geoproj/BUILD.bazel:3:11: GoCompilePkg pkg/geo/geoproj/geoproj.a failed (Exit 1): builder failed: error executing command bazel-out/host/bin/external/go_sdk/builder compilepkg -sdk external/go_sdk -installsuffix freebsd_amd64 -src pkg/geo/geoproj/geoproj.go -src pkg/g
eo/geoproj/proj.cc -src pkg/geo/geoproj/proj.h -arc ... (remaining 31 argument(s) skipped)

Use --sandbox_debug to see verbose messages from the sandbox builder failed: error executing command bazel-out/host/bin/external/go_sdk/builder compilepkg -sdk external/go_sdk -installsuffix freebsd_amd64 -src pkg/geo/geoproj/geoproj.go -src pkg/geo/geoproj/proj.cc -src pkg/geo/geoproj/proj.h -arc ... (remaining 31 argument(s) skipped)
rules_foreign_cc: Cleaning temp directories
Use --sandbox_debug to see verbose messages from the sandbox
ld: error: undefined symbol: sin
>>> referenced by pj_ell_set.c
>>>               pj_ell_set.c.o:(pj_ell_set) in archive bazel-out/freebsd-fastbuild/bin/c-deps/libproj/lib/libproj.a
>>> referenced by PJ_aea.c
>>>               PJ_aea.c.o:(setup) in archive bazel-out/freebsd-fastbuild/bin/c-deps/libproj/lib/libproj.a
>>> referenced by PJ_aea.c
>>>               PJ_aea.c.o:(setup) in archive bazel-out/freebsd-fastbuild/bin/c-deps/libproj/lib/libproj.a
>>> referenced 531 more times

ld: error: undefined symbol: cos
>>> referenced by PJ_aea.c
>>>               PJ_aea.c.o:(setup) in archive bazel-out/freebsd-fastbuild/bin/c-deps/libproj/lib/libproj.a
>>> referenced by PJ_aea.c
>>>               PJ_aea.c.o:(setup) in archive bazel-out/freebsd-fastbuild/bin/c-deps/libproj/lib/libproj.a
>>> referenced by PJ_aea.c
>>>               PJ_aea.c.o:(e_inverse) in archive bazel-out/freebsd-fastbuild/bin/c-deps/libproj/lib/libproj.a
>>> referenced 484 more times
...

This is because this link is missing a -lm somewhere. On BSD the math functions are not part of libc, they must be linked in explicitly.

(This problem does not exist when I run make.)

So I went forward and manually edited pkg/geo/geoproj/BUILD.bazel to add the missing freebsd rule alongside linux with the same link flags (IMHO: we should cover more variants, the way it's already done in other directories), which brings me one step further.

Now my build pkg/cmd/cockroach-short has completed 🎉 however ...


I can't find my executable!?

It's not in the top directory, and it's also not in pkg/cmd. Where am I supposed to find my program?

There are 3 issues here:

  1. I want my binary in the top level directory so I can type ./cockroach
  2. I want buildshort to create a symlink from cockroach to cockroachshort, so I can type ./cockroach regardless of whether I used build or buildshort to build
  3. this command bazel build pkg/cmd/cockroach-short is really loooooooong. Good case for dev buildshort!

@otan
Copy link
Contributor

otan commented Nov 24, 2020

It's not in the top directory, and it's also not in pkg/cmd. Where am I supposed to find my program?

yeah the idea is that you run bzl run //pkg/cmd/buildshort to do so.
copying it off the bazel-bin dir (where it is installed) can be a step the wrapper does.

@otan
Copy link
Contributor

otan commented Nov 24, 2020

This is because this link is missing a -lm somewhere. On BSD the math functions are not part of libc, they must be linked in explicitly.

do you mind sending a PR for this change? I think we want a copy of that linux line for freebsd/dragonfly/netbsd.

@otan
Copy link
Contributor

otan commented Nov 24, 2020

(Ideally these rules would default to make but accept an override via an env var.)

yikes. we'd have to change all the make commands to be an argument to the bazel rule script generator, and do something like

make_command = select({
   '//...:freebsd': 'gmake'
   '//conditions:default': 'make'
})

or something of the ilk.

@irfansharif
Copy link
Contributor Author

Where am I supposed to find my program?

$ file bazel-bin/pkg/cmd/cockroach-short/cockroach-short_/cockroach-short
bazel-bin/pkg/cmd/cockroach-short/cockroach-short_/cockroach-short: Mach-O 64-bit executable x86_64

But like Oliver said, the wrapper will place it in the top-level dir. Will be happy to take over any WIP PR/branch you have with the changes you needed. I'll merge this one for now.

bors r+

@knz
Copy link
Contributor

knz commented Nov 24, 2020

yikes. we'd have to change all the make commands to be an argument to the bazel rule script generator, and do something like

I assume the thing uses $PATH like everything else? If so I can place a symlink called make in cockroach/bin, I am assuming cockroach/bin is in $PATH already?

@craig
Copy link
Contributor

craig bot commented Nov 24, 2020

Build succeeded:

@craig craig bot merged commit fccf248 into cockroachdb:master Nov 24, 2020
@irfansharif irfansharif deleted the 201123.bsd-bazel branch November 24, 2020 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

build: bazel build cockroach-short fails on BSD
4 participants