-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Why all the new dependencies? #707
Comments
Hi @markbates, I wondered about the same question too. The added dependencies turns out to be a direct result of merely running Go Wiki provides an answer to "Why does 'go mod tidy' record indirect and test dependencies in my 'go.mod'?" at https://github.com/golang/go/wiki/Modules#why-does-go-mod-tidy-record-indirect-and-test-dependencies-in-my-gomod And here is an example command showing why GRPC is added: $ go mod why -m google.golang.org/grpc
# google.golang.org/grpc
github.com/spf13/viper/remote
github.com/xordataexchange/crypt/config
github.com/xordataexchange/crypt/backend/etcd
github.com/coreos/etcd/client
github.com/coreos/etcd/client.test
github.com/coreos/etcd/integration
google.golang.org/grpc Granted, being a relatively Go perpetual newbie myself, I am still trying to wrap my head around it, but looks like it is what the Go developers would call #WAI, working as intended. |
I'm looking into this. I'm also not quite clear why this is a problem. Viper + remote does depend on all of these but I believe that if you are using a mirror, it won't download or build them unless you are using the remote functionality. |
It definitely needs all these new deps, but my question is more abstract in that I question whether this featured which brings all these deps is one that the majority of people will use or not. I know I don’t need the feature and now I have a bunch of crazy deps. :( I’m considering moving all my projects off of Cobra to cut out all of these extra deps. |
I'm confused about what you mean by "deps" or really, why you are concerned. Right now they are just lines in a config file. They aren't dependencies until you import them. Viper doesn't import them by default, it requires you to add an additional import. What additional cost are these lines having on you? (I'm not being sarcastic, I'm genuinely trying to understand better). |
I’m not using that feature but all of these new dependencies are part of my app now. For example none of my apps use grpc, but do use cobra and now I have all these dependencies in all of my applications. Go modules downloads all of these deps even though I’m not using them. That’s my concern. |
@spf13 to give you an idea:
Notice things like this:
It's pulling down all of those packages to my machine that I'm not actually using. On my hotel wifi it means I'm basically stuck from working because of the downloading modules needs to do. Since nothing in my app uses consul, prometheus, grpc, coreos, etc... they're all still being downloaded. :( |
Ok. I understand now. I wouldn't say they are part of your app (as you aren't really importing them), but they are definitely part of your download right now and that's obviously a problem. However, once the Go module mirror is in place, which should be soon, it won't be. The mirror will permit you to only download the modules you use. Without the mirror there's no mechanism to get just the go.mod files from the dependencies so it downloads the entire repo. For what it's worth, these dependencies were added several years ago, but moved into another package to require an additional import to avoid exactly this problem. Unfortunately we're in this odd transition time between modules and the mirror being live so this problem appeared again. I'm not sure what the right short term solution is TBH. I'm open to suggestions though. |
Right, the dependencies existed already. The We do have some fixes on the go command side on the way too, though. The (I am not sure about the difference between viper and cobra here, but cobra is what the transcript said even though we are in the viper issue tracker.) |
This is very helpful to know. We should share this more broadly :)
Viper is a config management framework (flags, env vars, config files, etc)
that optionally supports remote config systems like etcd.
Cobra is a CLI framework (commands, subcommands, flags).
They work well together and also work well independently.
Cobra includes an application generator (github.com/spf13/cobra/cobra)
which optionally can add viper support to the CLI.
So it seems this new behavior is hitting Cobra users doubly hard importing
not only Viper but also all the remote functionality even if you are only
using Cobra.
…On Tue, Jun 11, 2019 at 5:07 PM Russ Cox ***@***.***> wrote:
Right, the dependencies existed already. The go mod tidy commit just made
them more visible. Because dependencies were invisible pre-modules, many
popular packages in the Go ecosystem carry far more dependencies than you
might expect. Viper is one example; there are more. The right long-term fix
is to think more carefully about which dependencies make sense and which
don't. We do have some fixes on the go command side on the way too, though.
The go get -u github.com/spf13/cobra in @markbates
<https://github.com/markbates>'s transcript is resolving not just the
latest cobra but the latest of every module listed in cobra's go.mod, and
their go.mod files, recursively. For Go 1.13, we have cut this back to be
more like the old GOPATH mode: instead of working a module granularity it
will work at package granularity, so that go get -u github.com/spf13/cobra
will get the latest cobra, and then the latest modules for the packages
github.com/spf13/cobra actually imports, and so on, recursively. If
cobra/remote is what pulls in tons of deps and your program does not import
cobra/remote even indirectly, then go get -u will not go looking for the
latest copies of those anymore (again, in Go 1.13). Go 1.14 may improve
things further.
(I am not sure about the difference between viper and cobra here, but
cobra is what the transcript said even though we are in the viper issue
tracker.)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#707?email_source=notifications&email_token=AABKKZHAE3X6MK6YV5MIRFTP2AHXPA5CNFSM4HPVMEF2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXOQDTY#issuecomment-501023183>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABKKZGOAEXCRSMXRLNNH2LP2AHXPANCNFSM4HPVMEFQ>
.
|
An almost identical question on viper happen to come up today in another forum, so adding some quick comments here as well. One thing that might help is to at least initially not use the Also, a quick test with a simple viper client does show things like
Finally, in advance of Go 1.13, there is a healthy chance |
That's something I would love for the long term, but that's a breaking change for the existing code. Update: Looks like the comment I was responding to had been deleted. I meant separate packages would be a nice solution for remote implementations, but would also be a breaking change. |
Something to consider might be making I've done something similar with Whether this is a recommended or idiomatic solution I can't say, but at least it works. |
Although tightly coupled dependencies were probably always the case, only recently did we start digging deep into viper after #635. These issues have unfortunately pushed us to remove viper from a couple dozen of our projects and reinvent the wheel (github.com/knadh/koanf). The following example, although not a 1:1 comparison, is illustrative of the effects of hardcoded dependencies (go1.12 linux/amd64). This produces a 2.4 MB binary. package main
import (
"fmt"
"encoding/json"
)
func main() {
// Dummy, just to bring in a parser.
var o map[string]interface{}
json.Unmarshal([]byte(`{"hello": "world}`), &o)
fmt.Println(o)
} Here, simply including viper, irrespective of the features used, produces a 12 MB binary. package main
import (
"fmt"
"github.com/spf13/viper"
)
func main() {
fmt.Println(viper.GetString("hello world"))
} I agree with @sagikazarmark . The only probable solution is to split dependencies into separate packages, but like he said, it would be a breaking change. |
I'm ok with making a breaking change provided we clearly tag the release
prior to and after the change and communicate it clearly. It's not 1.0 yet
allowing for such changes.
If I understand it all correctly, the fix for the break is likely just
adding some additional import statements if you happen to be using a
feature in the separated package. Pretty trivial.
…On Thu, Jun 20, 2019 at 2:05 AM Kailash Nadh ***@***.***> wrote:
Although tightly coupled dependencies were probably always the case, only
recently did we start digging deep into viper after #635
<#635>. These issues have
unfortunately pushed us to remove viper from a couple dozen of our projects
and reinvent the wheel (github.com/knadh/koanf).
The following example, although not a 1:1 comparison, is illustrative of
the effects of hardcoded dependencies (go1.12 linux/amd64).
This produces a *2.4 MB* binary.
package main
import (
"fmt"
"encoding/json"
)
func main() {
// Dummy, just to bring in a parser.
var o map[string]interface{}
json.Unmarshal([]byte(`{"hello": "world}`), &o)
fmt.Println(o)
}
Here, simply including viper, irrespective of the features used, produces
a *12 MB* binary.
package main
import (
"fmt"
"github.com/spf13/viper"
)
func main() {
fmt.Println(viper.GetString("hello world"))
}
I agree with @sagikazarmark <https://github.com/sagikazarmark> . The only
probable solution is to split dependencies into separate packages, but like
he said, it would be a breaking change.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#707?email_source=notifications&email_token=AABKKZGDXVUUPHPJQJ6IFITP3MMZ7A5CNFSM4HPVMEF2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYELVAQ#issuecomment-503888514>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABKKZETA75T3LDOZFO5MJ3P3MMZ7ANCNFSM4HPVMEFQ>
.
|
Hi, I need some assistance but is remote config reader library xordataexchange/crypt deprecated? |
etcd 3.5 will improve this significantly: client packages will be released as separate modules allowing to import only what's necessary for client functions. See etcd-io/etcd#12330 and etcd-io/etcd#12498 Sadly, there is no ETA for the release. I prepared a draft PR for the crypt library: bketelsen/crypt#12 As you can see there is a significant change in dependencies. |
@sagikazarmark it seems that etcd is sort of not-happening anytime soonish. Update I've just recompiled my application with viper 1.7.1 instead of my local fork using go 1.16 and can no longer find a significant difference in binary size. I'm not sure if that was expected but it sort of removes the need for action. |
etcd 3.5 will be released around June. Until then, here is the set of changes that we will get the new version: #1115 |
#1154 should improve the situation considerably. |
Hi Folks, I have a related question. I see that even though I am pulling latest viper (1.10.1), and my code is only relying on that, it ends up referencing two versions of gogo/protobuf from go.sum My go.mod My go.sum (only showing gogo/protobuf part, but the list is huge!)
And I can't figure out why. From the code, I see only pkg/mod/github.com/gogo/[email protected]/. Where's that extra ref for v1.1.1 coming from? I see same for a lot of other packages:
|
Without investing it thoroughly, my best guess is that two different
dependencies Viper imports each specify a different protobuf library and Go
is ok with satisfying both requirements without creating a diamond
dependency problem.
|
Funk is that I can’t find the code anywhere for v1.1.1 of protobuf. Only v1.3.2. Also nothing yields in recursive search in go.mod.
So I am wondering how it is getting into go.sum.
***@***.***
about.me/rajhimanshu
…________________________________
From: Steve Francia ***@***.***>
Sent: Saturday, March 5, 2022 6:57:11 AM
To: spf13/viper ***@***.***>
Cc: Himanshu Raj ***@***.***>; Comment ***@***.***>
Subject: Re: [spf13/viper] Why all the new dependencies? (#707)
Without investing it thoroughly, my best guess is that two different
dependencies Viper imports each specify a different protobuf library and Go
is ok with satisfying both requirements without creating a diamond
dependency problem.
—
Reply to this email directly, view it on GitHub<#707 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ADEIYKJQKRNVH4KROSETQRTU6NY4PANCNFSM4HPVMEFQ>.
Triage notifications on the go with GitHub Mobile for iOS<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you commented.Message ID: ***@***.***>
|
So I have tracked it down to Command that helped track it down So the real culprit is prometheus. This problem exists even in their latest version 1.12.1. |
@sagikazarmark it is possible to split out
When downstreams try to upgrade to the new release, those using See https://github.com/skitt/viper-demo for verification. This does constitute a breaking change, technically, but |
@skitt Thanks for the suggestion. I thought about that before, but honestly, I thought v2 would move along much faster. I had some good and some bad experiences with doing this in the past (admittedly, Go modules became much more mature since then). I wrote up a proposal based on your comment: #1845 I leave it open for a while to see how others react. But overall, I think it's a solid plan. |
I tagged an alpha version of the upcoming release that contains a change that removes most of the third-party dependencies: https://github.com/spf13/viper/releases/tag/v1.20.0-alpha.1 Please give it a try and report back any issues! Thanks! Also: thanks @skitt for the suggestion! |
Fantastic, this seems to be working fine at least in projects without a Thanks for taking the plunge! |
As of v1.20.0-alpha.3 three additional dependencies are dropped from Viper: HCL, Java properties, INI. Although we could externalize other dependencies (YAML, TOML), I'm satisfied with the current state for v1, so I'm going to close this issue once 1.20.0 is tagged. |
Why do I need GRPC for configuration?
The commit that did this, b5bf975, added
24
dependencies, while only removing5
to thego.mod
and a rather large148
additions to5
deletions in thego.sum
I believe it is the "remote" feature that is causing all of these imports, but I'm not sure. Perhaps that feature should be behind a build tag? Or perhaps it's introduction should be considered a breaking change?
cc @spf13
The text was updated successfully, but these errors were encountered: