use multierror instead of errors.Join

[bharling]

just a small fix to help with backwards compatibility for projects stuck on go < 1.20. This just postpones use of errors.Join in favour of multierror.Append. No functional changes.

[mdb]

Thanks @bharling - I'm in support of this!

[minamijoyo]

Aside from acceptance tests failing, could you give me more context about why this is needed?
From my point of view, it's hard to justify because:

• This project is developing a CLI rather than Go libraries.
• This project uses the latest Go 1.21 at the time of this writing.
• The errors.Join was introduced in Go 1.20.
• Go 1.19 has already reached EOL in August 2023.

Merging this patch means I can no longer use the Go 1.20+ feature until you finish upgrading, right?

[mdb]

@minamijoyo I work closely with @bharling and can offer some context...

From my point of view, it's hard to justify

I completely understand; it may not be worth catering to a niche use case. Nonetheless, here's some thoughts to consider...

While it's beyond your original intended use case -- and perhaps not worth supporting -- I do think there's some utility to using tfmigrate as a library as well as a CLI:

• its use as a library enables tfmigrate usage in concert with CDK for Terraform, within Go
• its use as a library enables Go-based processes to easily read and validate tfmigrate migration HCL files within a CI/CD build/test process, ensuring against problematic migrations, similar to how tools like terratest, tflint, and policy-as-code frameworks are used to vet Terraform configurations. Specifically, this might be useful in scenarios where -- for practical reasons -- users must skip_*_plan, but still want to offset that compromise by performing other validations/assertions against tfmigrate migration HCL files, such as checks protecting against incompatible migrations across multiple migration files (For example, even without skip_*_plan-ing, tfmigrate plan, when used in history mode, can overlook that the migrations codified across multiple migration files are incompatible and that tfmigrate apply will fail).
• its use as a library enables an ecosystem of peripheral tfmigrate tooling, just as tfmigrate is to terraform itself. Again, perhaps this isn't ultimately something you care to support, but I could imagine future tfmigrate features such as a before = "./some-command" hook that enables users to specify an arbitrary command for tfmigrate to exec out to before plan-ing/apply-ing. In such instances, ./some-command itself may want to use tfmigrate as a library in its underlying implementation.

Merging this patch means I can no longer use the Go 1.20+ feature until you finish upgrading, right?

Again, I completely understand if you don't want to support tfmigrate's usage as a library, and especially don't want to support its use as a backwards-compatible library for Go < 1.20. However...

• tfmigrate does already require github.com/hashicorp/go-multierror; @bharling's PR does not introduce new dependencies
• currently, I believe there's only 3 instances where errors.Join is used; I believe this minimal usage of errors.Join is the only non-< Go 1.20 compatible code within tfmigrate's codebase

That all said, I defer to @minamijoyo :)

And, a big disclaimer: I haven't looked into the acceptance test failures; I value a solution to #129 over < Go 1.20 support. I'm assuming @bharling 's intent is that tfmigrate continues to address issue #129 , but does so without errors.Join.

[bharling]

@minamijoyo I work closely with @bharling and can offer some context...

From my point of view, it's hard to justify

Completely understand and echo the points raised by @mdb above - this is essentially a fairly selfish PR on our part just to unblock our usage of tfmigrate in our current project, totally appreciate it's not helping to advance tfmigrate itself any.

I will take a look into the tests asap though regardless

[minamijoyo]

I do care about compatibility as a CLI but not as a library. That said, I don't intend to hide everything in the internal package as terraform does, so it's ok to use tfmigrate as a library at your own risk with the understanding that breaking changes could be merged at any time. Having that in mind, my answer would be to upgrade your Go version to the latest in general.

Even though you are the author of patch #129 and using the multierror looks relatively harmless, I can accept it as a temporary workaround. If you still wish to do so, could you provide the following information?

• What is the minimum required version you expect for the time being? < Go 1.20 is insufficient information to care for Go compatibility. Some linters may complain about deprecated features and force us to use new ones.
• When can I expect to revert the temporary workaround and use the latest Go? It will be out of my control; worst of all, there is no way to know when I can.

Without them, unblocking your project will block this project. It's hard to accept without any timeline.

[bharling]

I do care about compatibility as a CLI but not as a library. That said, I don't intend to hide everything in the internal package as terraform does, so it's ok to use tfmigrate as a library at your own risk with the understanding that breaking changes could be merged at any time. Having that in mind, my answer would be to upgrade your Go version to the latest in general.

Even though you are the author of patch #129 and using the multierror looks relatively harmless, I can accept it as a temporary workaround. If you still wish to do so, could you provide the following information?

• What is the minimum required version you expect for the time being? < Go 1.20 is insufficient information to care for Go compatibility. Some linters may complain about deprecated features and force us to use new ones.
• When can I expect to revert the temporary workaround and use the latest Go? It will be out of my control; worst of all, there is no way to know when I can.

Without them, unblocking your project will block this project. It's hard to accept without any timeline.

Thanks very much for taking the time to reply - on reflection I think the best option for tfmigrate would be to close this PR and for us to focus on resolving the upgrade issues our side. I don't want to block progress on tfmigrate nor create a dependency on our own work to revert the changes I'm proposing here.

Thanks again for looking into this and apologies for the waste of time!

[minamijoyo]

I appreciate your understanding, and It's also good to hear your use case 👍