Open eriksw opened 2 years ago
We have the same issue in https://github.com/keptn/keptn :( Renovate PR: https://github.com/keptn/keptn/pull/6377 Update PR to fix the dependencies: https://github.com/keptn/keptn/pull/6381
We would also like to see this feature. It looks like Renovate used to update indirect dependencies by default, but that was removed in https://github.com/renovatebot/renovate/pull/4650. I don't fully understand the reasoning for why this was done, but it seems like updating indirect dependencies should be an option at the very least.
With golang 1.17
and above the indirect dependencies should be updated too, otherwise it'll fail with:
12:51:56 === Errors
12:51:56 go: updates to go.mod needed; to update it:
12:51:56 go mod tidy
I hope someone will pick this up and make the necessary changes to make this happen.
@woohgit are you able to create a minimal reproduction for that type of failure?
@rarkins
Yes probably I'll need to create a module (shared) which uses aws-go-sdk for example. Then create 2 more modules that uses the shared
module and for go 1.17
+ you have to have the indirect deps for the shared for all other modules too, even if they don't directly use the aws-sdk-go.
If you wan't I can create a minimal app with the proper setup to reproduce the issue.
@woohgit thanks, that would be great. I would really like to get this issue solved, and this might be the root cause
@rarkins On it. I'm waiting for the first dependency PR to be created by renovatebot and I can show it to you then.
@rarkins And it happened.
There you go: https://github.com/woohgit/renovate-12999/pull/3/files
This repo consists of a shared module (which depends on aws-sdk-go) and 2 other consumer modules which depend on the shared. For go 1.17+ the go.sum and go.mod has to contain indirect entries for the shared module but that's not updated with the PR. So if you want to build the consumer1 or consumer2 using this PR you'll get the error:
❯ go build .
go: updates to go.mod needed; to update it:
go mod tidy
❯ pwd
/home/wooh/repos/renovate-12999/consumer1
It think at least it should be a config option to update it everywhere (and run go mod tidy in all relevant directories) and maybe set it to true
when the group:monorepo
is enabled and the project is golang.
@rarkins Any comment / update / more help needed etc?
@woohgit thanks for your updates to the issue. Previously I think people had been reporting "I need to run go mod tidy in the same directory one more time". In your case though it sounds like you'd need Renovate to build up a kind of "dependency tree" of each module to know which depends on which and then run "go mod tidy" on each directory which depends on the one which just got updated?
Actually everyone is reporting the same as me:
Look at the original description of this issue: https://github.com/eriksw/renovate-peer-go-mod-indirect/pull/1
This is exactly the same setup as I presented to you.
It's not that we need to run go mod tidy multiple times in the same directory. It's that the //indirect
dependencies are not updated, but they should be because from go 1.17+
you need to keep them sync too.
Actually we have a few Go tidying issues which seem to be bugs, some pre-dating 1.17. I thought we may have found the root cause but it seems not.
As discussed above, this will require Renovate to build a dependency tree of related go.mod files, which wasn't necessary before.
Classification as a feature request instead of bug was correct in this case.
@rarkins It's fine! I know it's getting a bit more complex with 1.17. I hope you guys can figure it out quickly how to do it :)
Whoever implements this will need to decide whether the go.mod dependency tree should be built during the extract phase (extractAllPackageFiles) or instead can be determined dynamically during this artifacts updating stage.
there is currently ongoing work for nuget, where we need to detect dependent projects. i think we can refactor and reuse some of the code from that PR so solve this issue afterwards.
I believe the nuget feature was implemented here: https://github.com/renovatebot/renovate/pull/15698 maybe this unblocks the feature request in this PR?
A simple solution for this issue might be to provide an option that just runs go mod tidy
for all modules in the repository, instead of evaluating the replace statements and creating a dependency tree. Currently, the postUpdateOption gomodTidy seem to only update modules that were changed by renovate.
@malt3 nuget manager has nothing to do with golang manager
I'm open to work on a fix for this!
The easiest way I see to implement this is to run go mod tidy
inside each go modules.
Does that would save everyone use case?
I'm currently running the following command locally to patch renovate PRs:
for i in $(find . -name go.mod); do cd $(dirname "$i") && go mod tidy && cd -; done && git commit -am "chore: go mod tidy in every go modules" && git push
However, it is a pain to do that inside every PR.
I was thinking about adding an option to renovate to implement a similar behavior, for example a postUpdateOptions: ["goModTidyAll"]
. But I can't find where I could implement it.
go mod tidy
is currently runs individually in each module here:
Is there a package where we could implement "repository wide" operations?
goModTidyAll
would be a good way to opt into it.
Check out https://github.com/renovatebot/renovate/blob/main/lib/modules/manager/nuget/package-tree.ts#L16 for an example of how nuget does it
I would love to see a goModTidyAll
option. That would indeed solve my use case!
sigh this seems to be impacting golang vulnerability related updates, if they happen to be against indirect dependencies (disabled by default). Dependabot is catching these, but renovate is not. Perhaps a workaround could be (untested, no idea if this would work):
"vulnerabilityAlerts": {
"packageRules": [
{
"matchManagers": ["gomod"],
"matchDepTypes": ["indirect"],
"enabled": true
}
]
}
Obviously that wouldn't do anything WRT a goModTidyAll
feature, but at least I might have a PR open to do that manually?
Clarification: The intent of the above is - It's better to get a broken proposed update PR than no update PR at all (for a vulnerability) - assuming Dependabot is turned off.
Edit: Opened https://github.com/renovatebot/renovate/discussions/22599
@cevich your requirements don't appear directly related to this topic, so please start a separate Discussion with your own reproduction repo
Almost all of the indirect major updates from renovate cause problems, mostly due to broken semver repos.
https://github.com/asaskevich/govalidator/issues/491
github.com/asaskevich/govalidator@v11.0.1: invalid version: module contains a go.mod file, so module path must match major version ("github.com/asaskevich/govalidator/v11"
https://github.com/twmb/franz-go/issues/494
require github.com/twmb/franz-go/pkg/kmsg: version "v2.0.2" invalid: should be v0 or v1, not v2
I don't think renovate should be updating any indirects at all; if go get -u ./...
and go mod tidy
miss them, that's a go mod
bug.
Please, restore the original behavior of not touching // indirect
dependencies, or at least not trying to do major
updates on them. There is absolutely NO reason why the major version of an indirect should get bumped, period. That is up to the actual mod that directly has that dependency.
@nyetwurk please start a new discussion and provide a minimal reproduction repo with such a failing case so that we can fix it
Also, comments like "The whole thing is hopelessly broken" are unwelcome here - please refrain from such negativity in this repo
@nyetwurk please start a new discussion and provide a minimal reproduction repo with such a failing case so that we can fix it
Also, comments like "The whole thing is hopelessly broken" are unwelcome here - please refrain from such negativity in this repo
I don't understand the rationale of bumping a major version of the indirect. If the included module is not compatible with the major version (for example, because the paths are all wrong), there is no amount of work in the local repo that can fix it. The major version must be updated in the actual mod that includes it directly, if only because of the path changes.
Note that https://github.com/asaskevich/govalidator is abandoned, so nobody is going to fix it, and in https://github.com/twmb/franz-go/issues/494 it is clear that repo will also never be fixed.
The former is being imported due to github.com/go-ozzo/ozzo-validation/v4/is, perhaps I can build a minimal repo around that that demonstrates the issue.
https://github.com/renovatebot/renovate/issues/4586 seems to be taking the correct approach; I'm not sure why after that was fixed, renovate is trying to update indirect dependencies - or even worse, updating to different major versions.
To clarify: I'm asking you to provide a minimal reproduction repo showing Renovate (incorrectly) updating an indirect dependency with a new major version. If that's not the case, I don't understand what it is you're objecting to. Neither of the repos you link to use Renovate so I'm also not sure why you're referring to them either.
I've also asked you to start a new discussion, please do that and discontinue in this issue.
Hi!
We are interested in using renovate
as we thought it would handle this case.
I tested it on my fork and sadly it does not handle running go mod tidy
in examples/
:
https://github.com/eiffel-fl/inspektor-gadget/actions/runs/9516190108/job/26231972803#step:6:308
For the reference, here is a schematic view of my repository:
$ find . -name go.mod main % u=
./tools/eks-cleanup/go.mod
./tools/testjson2md/go.mod
./tools/dnstester/go.mod
./go.mod
./examples/go.mod
$ cat examples/go.mod
module examples
go 1.22.2
require (
github.com/cilium/ebpf v0.15.0
github.com/inspektor-gadget/inspektor-gadget v0.28.1
...
// use current code of inspektor-gadget
replace github.com/inspektor-gadget/inspektor-gadget => ../
From my understanding, the goModTidyAll
was only a proposition and was not implemented so far.
I am not fluent in typescript, so can you please share more information on a potential implementation of such an option?
Note that, I prefer to be 100% honest on this, I cannot guarantee you I would work on this feature, as I rather lean toward having only one "main" go.mod
in my repository.
Best regards.
@eiffel-fl please start a separate discussion, and create a reproduction repo to demonstrate
What would you like Renovate to be able to do?
Because go modules include indirect dependencies in the
go.mod
andgo.sum
, when Renovate updates a go module, it also needs to update dependent modules.Scenario:
When renovate updates
monorepo/a/...
it needs to also updatemonorepo/go/b/go.*
at the same time, in the same commit.Because renovate currently does not do this, the PR to update module a fails because we have a test that ensures our module files are always tidy. That fails because when the test runs
go mod tidy
in b, it is stale and updated/rewritten by thego mod tidy
invocation in b, which updates b's indirect dependencies in go.mod and go.sum.Reproduction repo/PR: https://github.com/eriksw/renovate-peer-go-mod-indirect/pull/1
Log:
If you have any ideas on how this should be implemented, please tell us here.
Renovate needs to be aware of relative path replace directives amongst go modules and ensure update/tidy actions propagate following those relationships.
Is this a feature you are interested in implementing yourself?
No