Avoid requiring Bazaar to compile

[bltavares]

There are two transitive dependencies present on the current three which
are hosted on bzr, which increases the tooling required to compile
Terraform, and fails to compile the new version on Windwos. This
affects plugin providers as well, making it harder to compile plugins
on Winows.

If bzr is not installed, fetching dependencies will fail:

Error when Bazaar is not installed

go: launchpad.net/gocheck@v0.0.0-20140225173054-000000000087: bzr
branch --use-existing-dir https://launchpad.net/~niemeyer/gocheck/trunk
. in
C:\Users\bltav\go\pkg\mod\cache\vcs\f46ce2ae80d31f9b0a29099baa203e3b6d269dace4e5357a2cf74bd109e13339:
exec: "bzr": executable file not found in %PATH%
go: labix.org/v2/mgo@v0.0.0-20140701140051-000000000287: bzr branch
--use-existing-dir https://launchpad.net/mgo/v2 . in
C:\Users\bltav\go\pkg\mod\cache\vcs\ca61c737a32b1e09a0919e15375f9c2b6aa09860cc097f1333b3c3d29e040ea8:
exec: "bzr": executable file not found in %PATH%
go: error loading module requirements

When bazaar is installed, but running on Windows, we have a different
failure

Error executing Bazaar checkout on Windows

o: launchpad.net/[email blocked].0-20140225173054-000000000087: bzr branch --use-existing-dir https://launchpad.net/~niemeyer/gocheck/trunk . in c:\gopath\pkg\mod\cache\vcs\f46ce2ae80d31f9b0a29099baa203e3b6d269dace4e5357a2cf74bd109e13339: exit status 4:
bzr: ERROR: httplib.IncompleteRead: IncompleteRead(34 bytes read)

Traceback (most recent call last):
File "bzrlib\commands.pyo", line 920, in exception_to_return_code
File "bzrlib\commands.pyo", line 1131, in run_bzr
File "bzrlib\commands.pyo", line 673, in run_argv_aliases
File "bzrlib\commands.pyo", line 695, in run
File "bzrlib\cleanup.pyo", line 136, in run_simple
File "bzrlib\cleanup.pyo", line 166, in _do_with_cleanups
File "bzrlib\builtins.pyo", line 1438, in run
File "bzrlib\controldir.pyo", line 779, in open_tree_or_branch
File "bzrlib\controldir.pyo", line 459, in _get_tree_branch
File "bzrlib\bzrdir.pyo", line 1082, in open_branch
File "bzrlib\branch.pyo", line 2375, in open
File "bzrlib\controldir.pyo", line 687, in open
File "bzrlib\controldir.pyo", line 716, in open_from_transport
File "bzrlib\transport\__init__.pyo", line 1718, in do_catching_redirections
File "bzrlib\controldir.pyo", line 704, in find_format
File "bzrlib\controldir.pyo", line 1149, in find_format
File "C:/Program Files (x86)/Bazaar/plugins\git\__init__.py", line 235, in probe_transport
File "C:/Program Files (x86)/Bazaar/plugins\git\__init__.py", line 182, in probe_http_transport
File "socket.pyo", line 348, in read
File "httplib.pyo", line 522, in read
File "httplib.pyo", line 565, in _read_chunked
IncompleteRead: IncompleteRead(34 bytes read)

bzr 2.5.1 on python 2.6.6 (Windows-post2008Server-6.2.9200)
arguments: ['bzr', 'branch', '--use-existing-dir',
'https://launchpad.net/~niemeyer/gocheck/trunk', '.']
plugins: bzrtools[2.5.0], changelog_merge[2.5.1], colo[0.4.0],
explorer[1.2.2], fastimport[0.14.0dev], git[0.6.8], launchpad[2.5.1],
loom[2.3.0dev], netrc_credential_store[2.5.1], news_merge[2.5.1],
pipeline[1.4.0], qbzr[0.22.3], rewrite[0.6.4dev], svn[1.2.2],
upload[1.2.0dev], xmloutput[0.8.8]
encoding: 'cp1252', fsenc: 'mbcs', lang: None

*** Bazaar has encountered an internal error. This probably indicates a
bug in Bazaar. You can help us fix it by filing a bug report at
https://bugs.launchpad.net/bzr/+filebug
including this traceback and a description of the problem.

There are a couple of error reports starting to show up on GitHub and on
the internet regarding those dependencies.

The alternative here is to provide another path to resolve the
dependencies, where bzr is not involved.

This commit uses the replace directive to point the transitive
dependencies to HTTPS and git mirrors of the dependencies, making it
possible to resolve dependencies and compile again on Windows.

The other changes are caused by running go mod tidy.

[hashicorp-cla]

All committers have signed the CLA.

[apparentlymart]

Hi @bltavares!

It looks like there are a lot of dependency changes here. Are all of these required to eliminate the Bazaar requirement, or would the replace directives alone suffice? I'm nervous about making such significant changes to the dependencies while we're late in the v0.12.0 release cycle since it may invalidate testing that's already been carried out against the alpha and beta versions.

I'm also a little concerned that using replace to switch to forks of packages doesn't seem like a particularly sustainable option, but I'm open to it in the short term while we formulate a better plan, which might entail waiting for the Go community's module caching strategy to become more established so that the dependencies can more easily come from a cache rather than from their original source control.

[bltavares]

Hi @apparentlymart !

The only changes I've manually introduced was the replace directive, removed the // indirect dependencies and run go mod tidy to have it recalculate the needed dependencies.

I'm not quite versed on how vgo was supposed to work, and if that is the correct strategy, but I've noticed contributing to some plugins that if you remove // indirect dependencies, go mod tidy will sometimes add them back, or avoid them completely if the dependency is already a go mod.

I understand the concern regarding the amount of changes. The only needed directive is the two lines declaring replace. I could revert the // indirect cleanup and only propose it. Would that be better?

I would be fine installing bzr on Windows, but even after installed, it does not work with Launchpad dependencies. I would be fine with alternative proposals as well, that was the first one that I've imagined that could work, which at the same time benefits plugins to avoid extra tooling.
I was worried that upgrading providers after 0.12.0 is officially released, those dependencies would show up and silently break many other plugins until someone uses Windows or a system without bzr to contribute.

I'm not aware of the current state of the proposal of Go module caching, as I'm not used to write Go code, to comment if this is a viable long-term strategy, or if there is a better fix down the dependency tree.

Let me know how to help. I could test alternative approaches on my setup later this week, for example.

[apparentlymart]

Hi again @bltavares! Thanks for the additional information.

Based on my experience with Go Modules so far, I think the following steps should get the smallest possible result:

• Add the replace directives as you did already, but perhaps use the exact revision ids from the original commit instead of the newer ones:

  replace (
  	labix.org/v2/mgo => gopkg.in/mgo.v2 v0.0.0-20140701140051-000000000287
  	launchpad.net/gocheck => github.com/go-check/check v0.0.0-20140225173054-000000000087
  )

• Run go mod vendor to resync the vendor directory, which is what we use for release builds and most everyday development. (This should resolve any necessary dependency version changes as a side-effect.)
• Run go mod tidy to clean up anything no longer used.

Hopefully that'll keep the set of dependency changes to a minimum. If the resulting set is relatively small then we can potentially devise some tailored tests for those dependencies to get some confidence that they don't cause regressions -- the changeset for the above will include changes to files in vendor, so we can get a sense of what exactly changed upstream. If the set is still large then we'll need to be more cautious, and perhaps consider other workarounds until we complete the v0.12.0 release process, such as:

• testing whether bzr works better inside Windows Subsystem for Linux
• using another OS briefly to complete the Terraform SDK upgrade for the provider (including go mod vendor) and then using -mod=vendor when developing on Windows to avoid the need to re-fetch the dependency at all. Once the SDK code and dependencies are copied into the provider repository the bzr source will no longer matter.

Thanks for looking in to this! I'm sorry these indirect dependencies are causing trouble right now. I'm definitely keen to find a resolution here, just also want to minimize risk to the release already in progress.

[bltavares]

I've updated the changeset using the steps you've mentioned and that resulted in a much smaller change.

I'm still able to compile, without hitting the errors with bazaar. The versions don't exactly match, as I had to find the corresponding git commit that the bazaar changeset on the last part of the version represented. They should be the same exact code, looking at the commit messages of both repos.

I was really worried of the ripple effect it could cause on providers and plugin, making it harder to contribute there now that his new dependency exists. I'm not sure how to test, but I would expect that downstream plugins don't have the issue with bzr anymore after applying this to terraform.

[bltavares]

I've just replacing Terraform version on a downstream provider, which only requires Terraform, pointing to my commit.

Sadly, it used the require version, and not the replace version.

go: labix.org/v2/mgo@v0.0.0-00010101000000-000000000000: bzr branch --use-existing-dir https://launchpad.net/mgo/v2 . in C:\Users\bltav\go\pkg\mod\cache\vcs\ca61c737a32b1e09a0919e15375f9c2b6aa09860cc097f1333b3c3d29e040ea8: exec: "bzr": executable file not found in %PATH%
go: launchpad.net/gocheck@v0.0.0-00010101000000-000000000000: bzr branch --use-existing-dir https://launchpad.net/~niemeyer/gocheck/trunk . in C:\Users\bltav\go\pkg\mod\cache\vcs\f46ce2ae80d31f9b0a29099baa203e3b6d269dace4e5357a2cf74bd109e13339: exec: "bzr": executable file not found in %PATH%
go: error loading module requirements

I'll try to find which dependency introduced this requirement, and try upgrading it. Hopefully, it will be an easy bump. 🤞

[bltavares]

After running go mod why labix.org/v2/mgo, there was no reason reported to use those dependencies. Removing them from requires and not adding a replace directive allowed go mod tidy to not add it again.

Maybe I needed to run go mod vendor to sync the dependencies or something. I have no idea why it still compiles and don't add the dependency again, nor why it didn't remove the dependency before.

This version of the PR does compile a provider correctly, with no need to use bzr.

[apparentlymart]

Thanks, @bltavares!

After looking at this again with fresh eyes I remembered that the replace directive wouldn't actually have helped here anyway, because the go command only considers it in the "main module". That is, it is considered when building github.com/hashicorp/terraform itself, to produce the terraform executable, but anything other program depending on Terraform would need to repeat the same statement to get the same result.

I'm not really sure where these indirect dependencies originally came from, and it seems reasonable for us to remove them as you propose here. If one of our dependencies does still depend on them then they'll be installed at whatever minimum version satisfies those dependencies. If you find that these still show up after upgrading the vendored SDK in a provider, you may find that placing these same replace directives in the provider's own go.mod will do the trick.

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.