search

asteris-llc / converge

A powerful and easy-to-use configuration management system.
Apache License 2.0
250 stars 31 forks source link

vendor: Constrain HCL dependency to version 1.0.0 #641

Closed apparentlymart closed 6 years ago

apparentlymart commented 6 years ago

A new major version of HCL, with significant breaking changes to its API, will be published in the upstream HCL repository in a couple months, as described in the HCL 2.0 Transition page.

Since converge has a vendored copy of HCL, there is no particular urgency to pin to the 1.0.0 release we've just retroactively tagged to enable this transition, but this PR is the result of pinning the HCL dependency to that tag, in the hope of avoiding issues once the HCL repository master branch becomes version 2.

This has evidently moved to a newer commit of HCL than was previously vendored, so there are some minor HCL implementation changes in this PR.

It also seems that the version of dep I used here is generating Gopkg.lock in a slightly different shape than was here previously, adding some extra digests and re-formatting some of the package lists.

stevendborrelli commented 6 years ago

Thanks for the PR! We'll review this shortly.

stevendborrelli commented 6 years ago

It looks like HCL PR #139 changed the behavior of newline processing, breaking one of our tests, where we look for newlines in nodes. I'm looking at fixing the test.

stevendborrelli commented 6 years ago

I added one of our test fixes to your branch at https://github.com/asteris-llc/converge/pull/642. I'm closing this PR.

Thanks for submitting the PR! I'm interested in seeing the changes in HCL 2.0

apparentlymart commented 6 years ago

Oh, I'd forgotten about that change! Sorry about that, and also glad your tests caught it! Fortunately, looks like that made HCL behave more consistently with what Converge wanted anyway (forbidding newlines).

I believe it's still possible to include escaped newlines in the label strings, like "foo\nbar", so maybe you'd like to also include "\\n" in the test list there, but I'll leave that for you to decide. :grinning: