Renovate: Golang update PRs sometimes need more tidying

[djmitche]

Which Renovate are you using?

WhiteSource Renovate App

Which platform are you using?

GitHub.com

Have you checked the logs? Don't forget to include them if relevant

I have - they are here. I'm not sure what to make of them. It looks like renovate updated go.mod

DEBUG: gomod: need to update digest(branch="renovate/golang.org-x-net-digest")
{
  "depName": "golang.org/x/net",
  "lineToChange": "\tgolang.org/x/net v0.0.0-20200505041828-1ed23360d12c",
  "newDigestRightSized": "7e3656a0809f"
}

then ran go get and go mod tidy.

What would you like to do?

Our own CI runs on PRs and checks that go mod tidy has been run by looking for a diff. In this case it found:

diff --git a/go.sum b/go.sum
index 8c53a8ce1..2eec04d6e 100644
--- a/go.sum
+++ b/go.sum
@@ -227,6 +227,7 @@ golang.org/x/net v0.0.0-20190522155817-f3200d17e092/go.mod h1:HSz+uSET+XFnRR8LxR
 golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
 golang.org/x/net v0.0.0-20200202094626-16171245cfb2/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
 golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
+golang.org/x/net v0.0.0-20200506145744-7e3656a0809f h1:QBjCr1Fz5kw158VqdE9JfI9cJnl/ymnJWAdMuinqL7Y=
 golang.org/x/net v0.0.0-20200506145744-7e3656a0809f/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A=
 golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
 golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=

This has happened quite a bit in various PRs. In most cases, rebase! will fix it, but of course that means waiting an hour and running more CI tasks. We do have

    "postUpdateOptions": [
      "gomodTidy"
    ]   

in the config.

[rarkins]

Is this the PR that you're referring to?

If so, this issue title is a bit confusing, or am I missing something?

What it looks like to me is:

• Renovate did run go mod tidy
• It did do some tidying (note the removed lines)
• Your CI reports that more tidying is needed, but we don't know why unless go has a bug that requires you to tidy more than once

Is this accurate?

[rarkins]

but of course that means waiting an hour and running more CI tasks.

Rebases should happen almost immediately once you request them. I went back through the repo's closed PRs and the first one I found where you'd renamed it showed Renovate force pushed 5 minutes later. Do you have any recent examples of it taking too long?

[djmitche]

I think what's missing from the renovate run is the second line of go.sum that refers to the package name without the /go.mod suffix. Whether that line was added by go get and removed by go mod tidy or just never added is unclear.

It does seem likely it's a Go bug (go mod support is still pretty janky in general). Maybe running twice would help?

It's waiting for the CI to finish that's about an hour -- not renovate's fault :)

[rarkins]

Yeah.. I think maybe a double go mod tidy is in order. We already run it twice in a single PR in one other scenario (go mod tidy / go mod vendor / go mod tidy IIRC)

[HonkingGoose]

Yeah.. I think maybe a double go mod tidy is in order.

@rarkins That sounds like a feature to me, but the top post is a bug report. I don't known how to tag this one, so I'll nudge you into tagging it. :wink:

[rarkins]

There may be at least one other issue like this which are essentially duplicates. We actually did try the "double" go mod tidy and it didn't seem to solve it. We still haven't got to the bottom of the problem.

[HonkingGoose]

I count 3 issues about this: #7217, #6795, #6213.

[rarkins]

Let's:

• close the newer ones as duplicates
• @ mention any followers of the other closed issues here so that they get notified when we update this one

[HonkingGoose]

Hi @travisgroth, I'm letting you know that you should follow this issue instead of #7217.

[HonkingGoose]

Hi @sluongng and @tbpg, I'm letting you know that you should follow this issue instead of #6795.

[rarkins]

Thanks @HonkingGoose. This problem has been hard to nail down. The ultimate help would be a repo with a single go.mod/go.sum that can reproduce the "bad" (untidied) PRs every time. Although if you have a simple repo which reproduces it unreliably, that's still helpful - it would indicate that it's go itself producing inconsistent behavior and not our commands directly that are a problem.

[djmitche]

I have definitely seen rebases of the same PR produce this effect. So is at
least somewhat intermittent.

[HonkingGoose]

@rarkins Maybe this should be moved to blocked on your board? I don't think you can make progress on this until you get a (partial) reproduction?

[travisgroth]

@HonkingGoose @rarkins I set up a repro in https://github.com/renovatebot/renovate/issues/7217. https://github.com/travisgroth/renovate-test/pull/2 demonstrates the problem very minimally. What else is needed there for it to be useful in debugging? Would it be helpful if I add you to the repo permissions?

We also have very regular problems in https://github.com/pomerium/pomerium. At least 1 PR a week will exhibit this problem. Is there a way we can leverage that in debugging?

can reproduce the "bad" (untidied) PRs every time

Just to be clear, while go mod tidy will fix the problem, tidying should _not_ be required. The initial go get -d command should leave a good go.mod and go.sum. I believe it just won't clean up the old version from go.sum.

Finally, it's worth noting that I couldn't reproduce when running renovate locally. It is possible that there's something about the app environment at the heart of the problem. I am not sure what that would be, though.

[rarkins]

@travisgroth sorry for the late follow-up. What exactly is "the problem" you were intending to reproduce? This issue is about go.sum tidying either not running or not working, but you haven't enabled tidying. Are you referring to there being no h1 line added?

FYI forked that repo and installed but bot and got what seems to be a valid PR: https://github.com/renovate-tests/renovate-test-2/pull/1/files

[travisgroth]

Are you referring to there being no h1 line added?

Yes. That is the root problem I'm facing. However, as I've pointed out, this is more complicated - tidying will fix a missing the missing h1 line. https://github.com/pomerium/pomerium has tidying enabled and we still regularly get missing checksum lines. This means not only is the initial update failing, we're _also_ not running tidy.

This issue is about go.sum tidying either not running or not working,

Per @HonkingGoose my issue was deduplicated into this issue. https://github.com/renovatebot/renovate/issues/6213#issuecomment-717438723. If these are not being tracked as the same underlying problem, then should #7217 be reopened for the missing checksum line problem?

[HonkingGoose]

Per @HonkingGoose my issue was deduplicated into this issue. #6213 (comment). If these are not being tracked as the same underlying problem, then should #7217 be reopened for the missing checksum line problem?

I'll let @rarkins decide what to do here, he knows more about this than me. 😉

[rarkins]

I think if we were being careful we'd treat these as separate until proven otherwise, but now we already combined them we can keep it that way..

My best theory is that go get fails sometimes but does as partial update. And either go mod tidy is not run or it's thrown by the faulty go get. Either way I will try to work out why it's happening.

@travisgroth I think the best troubleshooting we could do for now is:

• Identify and save the go update log from a PR that has a wrong go.sum update
• Rebase the branch and capture the log again (hopefully fixing the problem)
• Compare

And of course if the error is reproducible over and over then even better.

I had also wondered about whether we were bypassing the goproxy and it would have something to do with it (e.g. we get rate limited when bypassing) but according to https://github.com/renovatebot/renovate/issues/7233 we resolved that to remove any GOPROXY override from the app. BTW we do set GOSUMDB=off though.