-
-
Notifications
You must be signed in to change notification settings - Fork 89
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
feat: yq #80
feat: yq #80
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
docs/*.md linguist-generated=true |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
docs/*.md | ||
lib/tests/jq/*.json | ||
lib/tests/yq/empty.yaml | ||
lib/lib/tests/write_source_files/*.js | ||
lib/lib/tests/write_source_files/subdir/*.js | ||
lib/lib/tests/write_source_files/subdir/subsubdir/*.js |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
"""Implementation for yq rule""" | ||
|
||
_yq_attrs = { | ||
"srcs": attr.label_list( | ||
allow_files = [".yaml", ".json", ".xml"], | ||
mandatory = True, | ||
allow_empty = True, | ||
), | ||
"expression": attr.string(mandatory = False), | ||
"args": attr.string_list(), | ||
"outs": attr.output_list(mandatory = True), | ||
} | ||
|
||
def is_split_operation(args): | ||
for arg in args: | ||
if arg.startswith("-s") or arg.startswith("--split-exp"): | ||
return True | ||
return False | ||
|
||
def _escape_path(path): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. did you check bazel_skylib//rules:paths.bzl to see if anything useful? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I thought |
||
return "/".join([".." for t in path.split("/")]) + "/" | ||
|
||
def _yq_impl(ctx): | ||
yq_bin = ctx.toolchains["@aspect_bazel_lib//lib:yq_toolchain_type"].yqinfo.bin | ||
|
||
outs = ctx.outputs.outs | ||
args = ctx.attr.args[:] | ||
inputs = ctx.files.srcs[:] | ||
|
||
split_operation = is_split_operation(args) | ||
|
||
if "eval" in args or "eval-all" in args: | ||
fail("Do not pass 'eval' or 'eval-all' into yq; this is already set based on the number of srcs") | ||
if not split_operation and len(outs) > 1: | ||
fail("Cannot specify multiple outputs when -s or --split-exp is not set") | ||
if "-i" in args or "--inplace" in args: | ||
fail("Cannot use arg -i or --inplace as it is not bazel-idiomatic to update the input file; consider using write_source_files to write back to the source tree") | ||
if len(ctx.attr.srcs) == 0 and "-n" not in args and "--null-input" not in args: | ||
args = args + ["--null-input"] | ||
|
||
# For split operations, yq outputs files in the same directory so we | ||
# must cd to the correct output dir before executing it | ||
bin_dir = ctx.bin_dir.path + "/" + ctx.label.package | ||
escape_bin_dir = _escape_path(bin_dir) | ||
cmd = "cd {bin_dir} && {yq} {args} {eval_cmd} {expression} {sources} {maybe_out}".format( | ||
bin_dir = ctx.bin_dir.path + "/" + ctx.label.package, | ||
yq = escape_bin_dir + yq_bin.path, | ||
eval_cmd = "eval" if len(inputs) <= 1 else "eval-all", | ||
args = " ".join(args), | ||
expression = "'%s'" % ctx.attr.expression if ctx.attr.expression else "", | ||
sources = " ".join(["'%s%s'" % (escape_bin_dir, file.path) for file in ctx.files.srcs]), | ||
# In the -s/--split-exr case, the out file names are determined by the yq expression | ||
maybe_out = (" > %s%s" % (escape_bin_dir, outs[0].path)) if len(outs) == 1 else "", | ||
) | ||
|
||
ctx.actions.run_shell( | ||
tools = [yq_bin], | ||
inputs = inputs, | ||
outputs = outs, | ||
command = cmd, | ||
mnemonic = "yq", | ||
) | ||
|
||
return DefaultInfo(files = depset(outs), runfiles = ctx.runfiles(outs)) | ||
|
||
yq_lib = struct( | ||
attrs = _yq_attrs, | ||
implementation = _yq_impl, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh... this file does nothing right now :(
I think we probably aren't going to finish the project bazelbuild/continuous-integration#1313 so maybe we should just delete the file, but then we're still nowhere on getting test coverage :(
Maybe FasterCI could add platforms...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this what controls the buildkite ci? See here: https://buildkite.com/bazel/bazel-lib/builds?branch=kormide%3Ayq
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh wow ... we never get the webhooks back from buildkite so we don't post the statuses on PRs, so they are green regardless. How did you even know to look in buildkite there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because I'm the one who set up buildkite :). And clearly forgot to add any sort of hooks..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it was a forget thing - I tried to work with Yun and Florian to setup the webhooks into our github org but never saw evidence they were getting sent from bazel's buildkite. I guess we could pick that back up, want to work with them directly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, happy to do that. But for my own understanding, why set up buildkite when we can use github actions? Can't we set up multiple platforms there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we could make Mac and windows under GitHub Actions, it's just another project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah and look at this bazelbuild/continuous-integration#1328
They have not done terraform applies? That is way too much toil for them to keep up with...