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

use jq instead of python to format the pinned JSON (fixes #395) #396

Merged
merged 2 commits into from
Mar 28, 2020

Conversation

kevingessner
Copy link
Contributor

The formatted output is consistent with python2's json.tool. No changes are required to how rules_jvm_external is added in the workspace nor how :pin is run. I've tested on linux (centos) and OS X.

I didn't add jq for windows because I wasn't sure if pinning worked for windows at all, and I can't test it. Should I add that as well?

@jin
Copy link
Member

jin commented Mar 19, 2020

Yes, pinning does work on Windows. We test them on CI here:

windows:
environment:
# This tests custom cache locations.
# https://github.com/bazelbuild/rules_jvm_external/pull/316
COURSIER_CACHE: /tmp/custom_coursier_cache
shell_commands:
- bazel run @unpinned_regression_testing//:pin
- bazel run @unpinned_maven_install_in_custom_location//:pin
- bazel run @duplicate_artifacts_test//:pin
test_targets:
- "--"
- "//..."
# rules_kotlin is not tested / does not work on Windows.
# https://github.com/bazelbuild/rules_kotlin/issues/179
# https://github.com/bazelbuild/rules_kotlin/blob/master/.bazelci/presubmit.yml
- "-//tests/unit/kotlin/..."
- "-//tests/integration/override_targets"

@kevingessner
Copy link
Contributor Author

OK, green now on all platforms!

Copy link
Member

@jin jin left a comment

Choose a reason for hiding this comment

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

Thank you!

coursier.bzl Outdated Show resolved Hide resolved
coursier.bzl Show resolved Hide resolved
@@ -22,6 +22,7 @@ genquery(
"@testonly_testing//:com_google_code_findbugs_jsr305",
"@testonly_testing//:com_google_auto_value_auto_value_annotations_1_6_3",
"@testonly_testing//:com_google_auto_value_auto_value_annotations",
"@testonly_testing//:pin",
Copy link
Member

Choose a reason for hiding this comment

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

The pinned repo should not have the pin build target.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

testonly_testing isn't pinned (no maven_install_json), so it should have a :pin target to create the initial JSON. right?

name = "testonly_testing",

Copy link
Member

Choose a reason for hiding this comment

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

Oops, you're right.

coursier.bzl Show resolved Hide resolved
Copy link
Member

@jin jin left a comment

Choose a reason for hiding this comment

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

Thanks again!

@@ -22,6 +22,7 @@ genquery(
"@testonly_testing//:com_google_code_findbugs_jsr305",
"@testonly_testing//:com_google_auto_value_auto_value_annotations_1_6_3",
"@testonly_testing//:com_google_auto_value_auto_value_annotations",
"@testonly_testing//:pin",
Copy link
Member

Choose a reason for hiding this comment

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

Oops, you're right.

@jin jin merged commit bad9e25 into bazelbuild:master Mar 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants