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

build: bazel build cockroach-short fails on BSD #56013

Closed
knz opened this issue Oct 27, 2020 · 8 comments · Fixed by #57039
Closed

build: bazel build cockroach-short fails on BSD #56013

knz opened this issue Oct 27, 2020 · 8 comments · Fixed by #57039
Assignees
Labels
A-build-system B-os-bsd Issues specific to a BSD OS (non-macOS) C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@knz
Copy link
Contributor

knz commented Oct 27, 2020

Running

bazel build //pkg/cmd/cockroach-short |& tee failure.log

produces this failure log

Then:

bazel build //pkg/cmd/cockroach-short --verbose_failures |& tee failure-verbose.log

produces this verbose log

Attached copy of bazel-out/.../libroach/logs/CMake.log

Futher instructions needed?

@knz knz added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-build-system labels Oct 27, 2020
@irfansharif
Copy link
Contributor

ln: illegal option -- t
usage: ln [-s [-F] | -L | -P] [-f | -i] [-hnv] source_file [target_file]
       ln [-s [-F] | -L | -P] [-f | -i] [-hnv] source_file ... target_dir
       link source_file target_file

Looks like the bazel internal linking scripts seem to use an illegal option.

...
_____ BEGIN BUILD SCRIPT _____
#!/usr/bin/env bash
function symlink_to_dir() {
local target="$2"
mkdir -p "$target"
if [[ -f "$1" ]]; then
ln -s -f -t "$target" "$1"

@knz
Copy link
Contributor Author

knz commented Oct 27, 2020

It's very odd because if $target already exists and is a directory (it is here! thanks to mkdir just before) then -t is not needed: ln -s will already do the right thing.

@knz
Copy link
Contributor Author

knz commented Oct 27, 2020

can we just have that remove the -t flag?

@irfansharif irfansharif removed their assignment Oct 27, 2020
@irfansharif
Copy link
Contributor

Probably, but it's not part of our bazel we've yet touched by hand. Would need further investigation (bazel on non-darwin systems generally still do). Mind sharing for posterity the machine/distro you're running it on? Because it's not an issue I'm seeing on my box.

@knz
Copy link
Contributor Author

knz commented Oct 27, 2020

BSD.

BSD ln does not support -t.

@otan
Copy link
Contributor

otan commented Oct 27, 2020

We'd need to copy this for BSD but remove the -t option: https://github.com/bazelbuild/rules_foreign_cc/blob/master/tools/build_defs/shell_toolchain/toolchains/impl/default_commands.bzl#L87

And then set a specific option for BSD: https://github.com/bazelbuild/rules_foreign_cc/blob/master/tools/build_defs/shell_toolchain/toolchains/toolchain_mappings.bzl#L33

Once grafted on, this needs to be placed on top of my branch in https://github.com/otan-cockroach/rules_foreign_cc/tree/autoconf and the WORKSPACE will need to change afterwards at https://github.com/cockroachdb/cockroach/blob/master/WORKSPACE#L68. We can submit a review in the main repo in the meantime, but my existing PR there has not been looked at.


As an aside, we should invest in dedicated machines so that everyone is compiling on same environment. Or more annoyingly, make bazel build inside the same docker container for everyone. Food for thought @jlinder? statement retracted - this would alienate the open source community.

@otan otan removed their assignment Oct 27, 2020
@knz
Copy link
Contributor Author

knz commented Oct 27, 2020

FWIW docker doesn't work on BSD systems (even macOS has problems). Native builds are a pre

@jlinder jlinder changed the title build: bazel build cockroach-short fails build: bazel build cockroach-short fails on BSD Oct 27, 2020
@otan
Copy link
Contributor

otan commented Oct 28, 2020

bazel-contrib/rules_foreign_cc#387

this would fix it. but there's no love from the maintainers :(

@knz knz added the B-os-bsd Issues specific to a BSD OS (non-macOS) label Nov 23, 2020
craig bot pushed a commit that referenced this issue Nov 24, 2020
57030: execgen: permit customization of template path r=jordanlewis a=jordanlewis

Closes #56982

This commit adds a new argument to execgen, -template, which allows
customization of the file path of the input template for a given output
file.

Here's an example of how to run execgen with cutomized paths:

```
execgen -template path/to/template_tmpl.go path/to/eventual/generated/code.eg.go > path/to/wherever/you/want/to/write/the/file.eg.o
```

The second argument is important because it is used as an argument to
`goimports`, which needs to the actual, eventual path that the generated
code will live at. Note that it doesn't actually need to *write* to that
filepath - just needs to know what the name will be, eventually.

Release note: None

57039: bazel: add freebsd support r=irfansharif a=irfansharif

Fixes #56013. We're picking up bazel-contrib/rules_foreign_cc/pull/387
(included in our new fork at [cockroachdb/rules_foreign_cc](https://github.com/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.

Co-authored-by: Jordan Lewis <[email protected]>
Co-authored-by: irfan sharif <[email protected]>
@craig craig bot closed this as completed in ed964c0 Nov 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build-system B-os-bsd Issues specific to a BSD OS (non-macOS) C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants