Skip to content
This repository has been archived by the owner on Jul 31, 2021. It is now read-only.

Bring bazel generate up-to-date with make bazel-generate at HEAD #5

Merged
merged 1 commit into from
Feb 23, 2021

Conversation

rickystewart
Copy link
Contributor

@rickystewart rickystewart commented Feb 16, 2021

No description provided.

Copy link
Contributor

@irfansharif irfansharif left a comment

Choose a reason for hiding this comment

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

Personally I think it's better to duplicate those 3 lines in-line here, as this diff ends up coupling the crdb SHA and the one here. It becomes difficult to amend that script/location once dev is stable, as it could behave differently depending on which crdb SHA is currently checked out.

(It's also more of a "go-ism")

@rickystewart rickystewart changed the title Call into bazel-generate.sh to generate Bazel files. Bring bazel generate up-to-date with make bazel-generate at HEAD Feb 17, 2021
@rickystewart
Copy link
Contributor Author

Mm, well, I don't agree with that advice but I guess that's not the first time Rob Pike and I disagreed on something :p

Now generate bazel is failing though, and the error looks like this:

cockroach$ ../dev/dev generate bazel
executing: /usr/local/bin/bazel run //:gazelle -- update-repos -from_file=go.mod -build_file_proto_mode=disable_global -to_macro=DEPS.bzl%go_deps -prune=true
INFO: Build option --run_under has changed, discarding analysis cache.
INFO: Analyzed target //:gazelle (0 packages loaded, 7195 targets configured).
INFO: Found 1 target...
Target //:gazelle up-to-date:
  bazel-bin/gazelle-runner.bash
  bazel-bin/gazelle
INFO: Elapsed time: 0.549s, Critical Path: 0.01s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
INFO: Build completed successfully, 1 total action
executing: /usr/local/bin/bazel run //:gazelle
INFO: Analyzed target //:gazelle (3 packages loaded, 106 targets configured).
INFO: Found 1 target...
Target //:gazelle up-to-date:
  bazel-bin/gazelle-runner.bash
  bazel-bin/gazelle
INFO: Elapsed time: 2.043s, Critical Path: 0.01s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
INFO: Build completed successfully, 1 total action
exit status 127

The error message is incredibly uninformative so I don't know what this means. Any ideas?

@irfansharif
Copy link
Contributor

	command := exec.Command("bazel", "run", "//pkg/cmd/generate-test-suites", fmt.Sprintf("--run_under='cd %s && '", cwd))
	command.Stderr = os.Stderr
	command.Stdout = os.Stdout
	if err := command.Run(); err != nil {
		return err
	}
	return nil
	// TODO: Capture output + write to pkg/BUILD.bazel.
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
INFO: Running command line: /bin/bash -c ''\''cd /Users/irfansharif/Software/src/github.com/cockroachdb/cockroach && '\'' bazel-bin/pkg/cmd/generate-test-suites/generate-
INFO: Build completed successfully, 1 total action
/bin/bash: cd /Users/irfansharif/Software/src/github.com/cockroachdb/cockroach && : No such file or directory
exit status 127

I tried capturing the stdout/err for the raw exec command, and I think the cd $PWD bit is the problematic bit (though I'm unsure how).

@irfansharif
Copy link
Contributor

irfansharif commented Feb 22, 2021

Oh I think it's a string escape issue (/bin/bash -c ''\''cd /Users/irfansharif/). Try:

	buf, err := exec.Command("bazel", "run", "//pkg/cmd/generate-test-suites", "--run_under", fmt.Sprintf("cd %s &&", cwd)).Output()
	if err != nil {
		return err
	}
	return ioutil.WriteFile(path.Join(cwd, "pkg/BUILD.bazel"), buf, 0644)

@irfansharif
Copy link
Contributor

(we could also carry over the re-ordering from cockroachdb/cockroach#60939)

@rickystewart
Copy link
Contributor Author

That works, thanks for your help!!

@rickystewart rickystewart merged commit 5b17a1f into cockroachdb:master Feb 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants