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

execgen: permit customization of template path #57030

Merged
merged 1 commit into from
Nov 24, 2020

Conversation

jordanlewis
Copy link
Member

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

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
@jordanlewis jordanlewis requested review from irfansharif and a team November 23, 2020 18:01
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@jordanlewis
Copy link
Member Author

@irfansharif I don't necessarily need a review, but please LMK if the proposed change would fix the Bazel issue.

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.

This works. Sorry for the delay, wanted to see what the bazel rules would change to.

diff --git i/pkg/sql/colexec/BUILD.bazel w/pkg/sql/colexec/BUILD.bazel
index ff48f4de4d..a39d8b0eea 100644
--- i/pkg/sql/colexec/BUILD.bazel
+++ w/pkg/sql/colexec/BUILD.bazel
@@ -52,6 +52,7 @@ go_library(
         "unordered_distinct.go",
         "utils.go",
         ":gen-exec",  # keep
+        ":gen-and-or-projection",  # keep
     ],
     importpath = "github.com/cockroachdb/cockroach/pkg/sql/colexec",
     visibility = ["//visibility:public"],
@@ -201,7 +202,6 @@ go_test(

 # Define a file group for all the .eg.go targets.
 targets = [
-    "and_or_projection.eg.go",
     "cast.eg.go",
     "const.eg.go",
     "default_cmp_expr.eg.go",
@@ -254,3 +254,15 @@ generate(
     targets = targets,
     templates = glob(["*_tmpl.go"]),
 )
+
+# XXX: Rule generation for and_or_projection.eg.go. We'll want one per eg.go
+# file; we'll write a helper method to make it look nicer.
+genrule(
+    name = "gen-and-or-projection",
+    srcs = ["and_or_projection_tmpl.go"],
+    outs = ["and_or_projection.eg.go"],
+    cmd = """
+      $(location //pkg/sql/colexec/execgen/cmd/execgen) -template $(location and_or_projection_tmpl.go) -fmt=false pkg/sql/colexec/and_or_projection.eg.go > $@
+      $(location //vendor/github.com/cockroachdb/gostdlib/x/tools/cmd/goimports) -w $@
+    """,
+    tools = ["//pkg/sql/colexec/execgen/cmd/execgen", "//vendor/github.com/cockroachdb/gostdlib/x/tools/cmd/goimports"],
+)

@jordanlewis
Copy link
Member Author

Nice - that seems pretty close to the optgen one, so victory?

I'm happy to help out with creating the genrules if it's annoying, if you tell me how to do it.

bors r+

@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
Copy link
Contributor

I'm happy to help out with creating the genrules if it's annoying.

Not annoying at all, thanks for helping me out here Jordan. See #57075.

@jordanlewis jordanlewis deleted the execgen-updates branch November 27, 2020 20:36
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.

execgen: make file i/o more "unix"-ey
3 participants