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

feat: yq #80

Merged
merged 1 commit into from
Apr 20, 2022
Merged

feat: yq #80

merged 1 commit into from
Apr 20, 2022

Conversation

kormide
Copy link
Collaborator

@kormide kormide commented Apr 15, 2022

@alexeagle @gregmagolan

It turns out yq is quite functional and can parse and output to several different formats. This will help us convert our yaml pnpm lock to json in rules_js.

issue: #78
buildkite: https://buildkite.com/bazel/bazel-lib/builds?branch=kormide%3Ayq

lib/private/yq_toolchain.bzl Outdated Show resolved Hide resolved
lib/yq.bzl Outdated Show resolved Hide resolved
Comment on lines 42 to 64
# https://github.com/mikefarah/yq/releases
#
# The integrity hashes can be computed with
# shasum -b -a 384 [downloaded file] | awk '{ print $1 }' | xxd -r -p | base64
YQ_VERSIONS = {
"v4.24.4": {
"darwin_amd64": "sha384-H5JnUD7c0jpbOvvN1pGz12XFi3XrX+ism4iGnH9wv37i+qdkD2AdTbTe4MIFtMR+",
"darwin_arm64": "sha384-9B85+dFTGRmMWWP2M+PVOkl8CtAb/HV4+XNGC0OBfdBvdJU85FyiTb12XGEgNjFp",
"linux_386": "sha384-TiesqbEG9ITqnOyFNMilVnciVM65dCAlRNYp/pK19jrqs2x5MhbpJ0a7Q9XwZmz8",
"linux_amd64": "sha384-y8vr5fWIqSvJhMoHwldoVPOJpAfLi4iHcnhfTcm/nuJAxGAJmI2MiBbk3t7lQNHC",
"windows_386": "sha384-YJTz4Y+5rcy6Ii/J44Qb6J2JZuzfh40WHGTc6jFTHFhJ47Ht+y9s4bS6h8WX6S0m",
"windows_amd64": "sha384-f8jkaz3oRaDcn8jiXupeDO665t6d2tTnFuU0bKwLWszXSz8r29My/USG+UoO9hOr",
},
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@alexeagle Is there an easier way to automate this? This project is actively maintained, unlike jq.

Copy link
Collaborator

Choose a reason for hiding this comment

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

They publish checksums with each release
https://github.com/mikefarah/yq/releases/download/v4.24.5/checksums
so I'd guess this can just be a tiny script.
I just did rules_esbuild with a simple approach where we manually, periodically add a new version
https://github.com/aspect-build/rules_esbuild/blob/main/scripts/mirror_release.sh

We also have fancier integration in rules_nodejs to send auto-PRs when new versions are published, my guess is almost no users expect to have the very latest yq so not a very important place to invest time.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added a script to do this. See tools/yq_mirror_release.sh.

@kormide
Copy link
Collaborator Author

kormide commented Apr 19, 2022

Ready for final review.

Copy link
Collaborator

@alexeagle alexeagle left a comment

Choose a reason for hiding this comment

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

super nice :)

@@ -12,7 +12,11 @@ tasks:
test_targets:
- "//..."
windows:
Copy link
Collaborator

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...

Copy link
Collaborator Author

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

Copy link
Collaborator

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?

Copy link
Collaborator Author

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..

Copy link
Collaborator

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?

Copy link
Collaborator Author

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?

Copy link
Collaborator

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.

Copy link
Collaborator

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...

docs/yq.md Show resolved Hide resolved
return True
return False

def _escape_path(path):
Copy link
Collaborator

Choose a reason for hiding this comment

The 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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah I thought paths.relativize might do what we need but it doesn't.

lib/private/yq.bzl Outdated Show resolved Hide resolved
lib/private/yq.bzl Outdated Show resolved Hide resolved
lib/private/yq_toolchain.bzl Show resolved Hide resolved
diff_test(
name = "case_json_output_no_out_test",
file1 = "//lib/tests/jq:a_pretty.json",
file2 = "case_json_output_no_out.json",
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice, I like the naming conventions that detect the output type

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.

2 participants