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: support jq 1.7 toolchain #520

Merged
merged 1 commit into from
Sep 20, 2023
Merged

Conversation

kormide
Copy link
Collaborator

@kormide kormide commented Sep 7, 2023

Fixes #268

Type of change

  • New feature or functionality (change which adds functionality)

For changes visible to end-users

  • Suggested release notes are provided below:
  • Jq 1.7 is now supported which has a wider selection of platform binaries
  • BREAKING: jq stamp variables are now of the form $stamp[0].MY_VARIABLE rather than $stamp.MY_VARIABLE.

Test plan

  • Covered by existing test cases

@kormide
Copy link
Collaborator Author

kormide commented Sep 7, 2023

There are some issues with the use of --argfile with the stamping feature which I need to work through. It appears that flag has been deprecated in 1.7.

@kormide kormide force-pushed the jq-1.7 branch 4 times, most recently from 8befbba to 9a0f66b Compare September 7, 2023 09:37
@kormide kormide changed the base branch from main to 2.x September 15, 2023 22:57
@kormide kormide requested review from alexeagle and removed request for alexeagle September 15, 2023 22:58
lib/private/jq_toolchain.bzl Outdated Show resolved Hide resolved
@alexeagle alexeagle mentioned this pull request Sep 16, 2023
lib/jq.bzl Show resolved Hide resolved
],
),
_JQ_PLATFORMS = {
"<1.7": {
Copy link
Collaborator

Choose a reason for hiding this comment

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

if it's a breaking change, why bother preserving compat with older versions of jq?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was assuming we should still support older versions of jq with newer versions of bazel-lib. Wdyt?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The breaking change only needs to be in stamping.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Unless it provides some utility to users, it seems to me that we should take the opportunity to simplify our code. Unlike the yq upgrade we attempted, I think pretty much everyone will be happy to have only 1.7 now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can't think of any utility it provides. I'll drop the old versions, and if a need becomes apparent then we can add back what I have here for pre-1.7 support.

@kormide kormide force-pushed the jq-1.7 branch 2 times, most recently from 18092b4 to a7a5d30 Compare September 20, 2023 07:15
@alexeagle alexeagle merged commit 332fc10 into bazel-contrib:2.x Sep 20, 2023
alexeagle pushed a commit that referenced this pull request Sep 26, 2023
alexeagle pushed a commit that referenced this pull request Oct 2, 2023
alexeagle pushed a commit that referenced this pull request Oct 3, 2023
alexeagle pushed a commit that referenced this pull request Oct 8, 2023
alexeagle pushed a commit that referenced this pull request Dec 23, 2023
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.

Support jq on arm64 linux
2 participants