Skip to content
This repository has been archived by the owner on Oct 22, 2021. It is now read-only.

Add copy/move semantics to ops files #913

Merged
merged 4 commits into from
May 1, 2020
Merged

Conversation

jandubois
Copy link
Member

Fixes #900.

Switches cppforlive/go-patchSUSE/go-patch, which has been updated with support for qcopy and qmove ops.

There is still a residual indirect reference to cppforlive/go-patch from the bosh-cli module:

$ go mod why -m github.com/cppforlife/go-patch
# github.com/cppforlife/go-patch
code.cloudfoundry.org/cf-operator/pkg/bosh/manifest
github.com/cloudfoundry/bosh-cli/director/template
github.com/cppforlife/go-patch/patch

I believe the bosh-cli is only used for variable interpolation, but not for ops file application. If this understanding is wrong, then this PR may be incomplete.

go.sum has been pruned via go mod tidy.

Switches cppforlive/go-patch → SUSE/go-patch, which has been updated
with support for qcopy and qmove ops.

There is still a residual indirect reference to cppforlive/go-patch
from the bosh-cli module:

$ go mod why -m github.com/cppforlife/go-patch
# github.com/cppforlife/go-patch
code.cloudfoundry.org/cf-operator/pkg/bosh/manifest
github.com/cloudfoundry/bosh-cli/director/template
github.com/cppforlife/go-patch/patch

go.sum has been pruned via `go mod tidy`.
@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/172594552

The labels on this github issue will be updated when the story is started.

I still don't understand how the Go tools could not figure
this out themselves. Seems like a serious flaw is hiding in
the whole version management somewhere.
@jandubois
Copy link
Member Author

jandubois commented Apr 30, 2020

I had to manually bump the SUSE/go-patch dependency to v0.3.0.

I can see that the previous dependency on cppforlive/go-patch had the same issue:

- 	github.com/cppforlife/go-patch v0.0.0-20171006213518-250da0e0e68c
+ 	github.com/cppforlife/go-patch v0.2.0 // indirect

The cppforlife/go-patch@250da0e0e68c commit corresponds to v0.1.0 and not v0.2.0, so something seems to go wrong when go mod tries to determine the highest available version number. v0.2.0 was released in Dec 2018.

I would be grateful if somebody could explain to me what is going on here. Not being able to trust the dependency manager produces so much anxiety...

@mudler
Copy link
Member

mudler commented Apr 30, 2020

Fixes #900.

Switches cppforlive/go-patchSUSE/go-patch, which has been updated with support for qcopy and qmove ops.

There is still a residual indirect reference to cppforlive/go-patch from the bosh-cli module:

$ go mod why -m github.com/cppforlife/go-patch
# github.com/cppforlife/go-patch
code.cloudfoundry.org/cf-operator/pkg/bosh/manifest
github.com/cloudfoundry/bosh-cli/director/template
github.com/cppforlife/go-patch/patch

I believe the bosh-cli is only used for variable interpolation, but not for ops file application. If this understanding is wrong, then this PR may be incomplete.

In such case we could use go mod replace 🙂

For the pinning issue, which golang version are you using locally? I've experienced similar issues with go <= 1.13

@jandubois
Copy link
Member Author

In such case we could use go mod replace 🙂

Maybe, but I found golang/go#30354:

replace is intended to support local development, not permanent forks: if you need to permanently fork a dependency, your fork must have its own, unique import path.

So now I'm not sure if this is the right thing to do here or not. I assume the go-patch code from bosh-cli is never executed by the operator, so having it use the default module version doesn't really matter, except for some very modest size savings?

For the pinning issue, which golang version are you using locally? I've experienced similar issues with go <= 1.13

I've been using 1.14.2.

I would be suspicious of the Google man-in-the-middle service being out-of-date because it also didn't pick up on the new v0.3.0 tag within 20min:

$ go mod tidy
go: github.com/SUSE/[email protected]/go.mod: verifying module: github.com/SUSE/[email protected]/go.mod: reading https://sum.golang.org/lookup/github.com/!s!u!s!e/[email protected]: 410 Gone
	server response: not found: github.com/SUSE/[email protected]: invalid version: unknown revision v0.3.0

$ go list -m --versions github.com/SUSE/go-patch
github.com/SUSE/go-patch v0.1.0 v0.2.0

$ GOPRIVATE=github.com/SUSE/go-patch go mod tidy
go: downloading github.com/SUSE/go-patch v0.3.0

$ go list -m --versions github.com/SUSE/go-patch
github.com/SUSE/go-patch v0.1.0 v0.2.0 v0.3.0

But then the v0.2.0 tag has been out for over 2 years, so I still don't know why it was not chosen in the initial update.

I also find it somewhat disturbing that the service only starts indexing the new tag once I tell it that it is a private module, but that could be a coincidence, or some local Go module caching interference?

#include <rant about the Go tools being so brittle and unpredictable> here 😦

@linux-foundation-easycla
Copy link

CLA Check

@viovanov viovanov merged commit 82d5347 into master May 1, 2020
@viovanov viovanov deleted the jandubois/copy-merge-ops branch May 1, 2020 00:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add copy/move functionality to ops-file support
4 participants