remove replace directives #2590
Conversation
carnott-snap
force-pushed
the
no-replace
branch
from
42856a5 to
843b0ee
Compare
go.mod contains many replace entries that all consumers must include to import logic. These are cancerous and tend to make consumers builds difficult or outright fail. All have been removed, except for the one required by flux. This entry, while retained, has converted into a noop and should be removed when fluxcd/flux#2590 is fixed.
carnott-snap
force-pushed
the
no-replace
branch
from
843b0ee to
5b630d1
Compare
go.mod contains many replace entries that all consumers must include to import logic. These are cancerous and tend to make consumers builds difficult or outright fail. All have been removed, except for the one required by flux. This entry, while retained, has been converted into a noop and should be removed when fluxcd/flux#2590 is fixed.
go.mod contains many replace entries that all consumers must include to import logic. These are cancerous and tend to make consumers builds difficult or outright fail. All have been removed, except for the one required by flux. This entry, while retained, has been converted into a noop and should be removed when fluxcd/flux#2590 is fixed.
|
Hi. Thanks for the contribution! However, I am afraid that those replacements are needed. What makes you think they are not? It may be that flux even compiles without them but that doesn't mean they are not needed.
I would generally agree with that, but, they are needed. Also, could you provided an existing scenario in which the replacements are an issue? |
go.mod contains many replace entries that all consumers must include to import logic. These are cancerous and tend to make consumers builds difficult or outright fail. All have been removed, except for the one required by flux. This entry, while retained, has been converted into a noop and should be removed when fluxcd/flux#2590 is fixed.
The repo builds and tests successfully without them, and I have a basic consumer package that compiles successfully. Replace lines do not affect dependent modules (per the spec), so I am unclear why they are necessary, can you explain this requirement?
I have interest in using transitive dependencies of flux, and do not want to maintain a list of eight replace directives for modules that I do not use (directly). There is no automated way to keep them in sync, and upgrading the flux version will cause breakage when these replace lines change. |
|
The docker distribution replacement is needed to honor timeouts when accessing repositories. The Kubernetes replacements are used to ensure Flux backwards compatible with Kubernetes and to make sure that the go client version we use is compatible with the Kubernetes libraries. They are needed because different dependencies import different versions of the Kubernetes libraries, and unfortunately Golang doesn't support using multiple dependency versions, not letting us specify our own versions directly.
Sorry, but I don't see how that would cause things to break. As I see it, the impact of changing the replacements would not be worse than bumping the dependency versions without an explicit replacement rule. Regardless, the replacements are there for a reason and we cannot simply remove them. We could consider bumping the replacement versions, but that's about it. |
|
@carnott-snap to understand your usecase, why are you importing Flux as a library? |
I am sympathetic to this concern, however that change lives in a stale fork, and is over six months out of date. Is there work to upstream this fix? If so, we can leave the replace, but should link against the PR and warn about impending removal. If not, the logic should be internalised, so that consumers do not see the side effects and the maintenance burden is clear to the maintainers of this library.
I do not see how
I should be clear, while there the mentioned breakage relates to upgrades and will happen with
We are attempting to integrate with the |
go.mod contains many replace entries that all consumers must include to import logic. These are cancerous and tend to make consumers builds difficult or outright fail. All unnecessary replaces have been removed. One is required by flux, and while retained, has been converted into a noop and should be removed when fluxcd/flux#2590 is fixed. A co-dependant transitive upgrade was detected, that bumps k8s.io/code-generator above kubernetes-1.14.4. To correct this the go.mod has been left untidy, but with the correct version required, and a replace directive has added so that build/test will pass.
go.mod contains many replace entries that all consumers must include to import logic. These are cancerous and tend to make consumers builds difficult or outright fail. All unnecessary replaces have been removed. One is required by flux, and while retained, has been converted into a noop and should be removed when fluxcd/flux#2590 is fixed. A co-dependant transitive upgrade was detected, that bumps k8s.io/code-generator above kubernetes-1.14.4. To correct this the go.mod has been left untidy, but with the correct version required, and a replace directive has added so that build/test will pass.
go.mod contains many replace entries that all consumers must include to import logic. These are cancerous and tend to make consumers builds difficult or outright fail. All unnecessary replaces have been removed. One is required by flux, and while retained, has been converted into a noop and should be removed when fluxcd/flux#2590 is fixed. A co-dependant transitive upgrade was detected, that bumps k8s.io/code-generator above kubernetes-1.14.4. To correct this the go.mod has been left untidy, but with the correct version required, and a replace directive has added so that build/test will pass.
distribution/distribution#2905 , which has been ignored by the maintainers so far.
Not really, in my experience Go will override the version you require by the version required by your dependencies.
I don't think that's the case. You don't need to add a replace rule if you don't care about the specific version of your dependencies.
I wasn't trying to imply that. In fact, Flux intentionally exports the |
You are right, we should add a comment in |
Let's put it in a different way. If you manage to remove the replace directives without changing I doubt that's possible though. |
carnott-snap
force-pushed
the
no-replace
branch
from
5b630d1 to
23325e1
Compare
I believe 23325e1 meets these standards. There is one additional line in the My initial change also included safe dependency upgrades, using
With the lack of traction on the PR, and the importance of this feature, it suggests we should make a real fork. I dislike imposing the This will involve publishing a valid module to
Since I am unsure if you are willing to take the fork route, I have amened
This is not true for the current state of the repo: a |
|
This is starting to look good, thanks.
How do you know that? |
carnott-snap
force-pushed
the
no-replace
branch
from
23325e1 to
44bd127
Compare
There are two types of hashes in As such, we can read these three lines as saying: you will find two module versions, Before you locked The use of the correct version can also be confirmed by running |
|
@carnott-snap makes sense, would you mind rebasing on master before I merge? |
go.mod contains many replace entries that all consumers must include to import logic. These are cancerous and tend to make consumers builds difficult or outright fail. All have been removed, except for the one required by helm-operator. This entry, required transitively by helm-operator, should be removed when distribution/distribution#2905 closes.
carnott-snap
force-pushed
the
no-replace
branch
from
44bd127 to
988fc0b
Compare
go.mod contains many replace entries that all consumers must include to
import logic. These are cancerous and tend to make consumers builds
difficult or outright fail. All have been removed, except for the one
required by helm-operator. This entry, while retained, has been
converted into a noop and should be removed when
fluxcd/helm-operator#97 is fixed.