-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Golang update PRs sometimes need more tidying #6213
Comments
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:
Is this accurate? |
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? |
I think what's missing from the renovate run is the second line of 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 :) |
Yeah.. I think maybe a double |
@rarkins That sounds like a |
There may be at least one other issue like this which are essentially duplicates. We actually did try the "double" |
Let's:
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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 |
I have definitely seen rebases of the same PR produce this effect. So is at
least somewhat intermittent.
|
This comment has been minimized.
This comment has been minimized.
@HonkingGoose @rarkins I set up a repro in #7217. travisgroth/renovate-test#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?
Just to be clear, while 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. |
@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 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 |
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
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. 😉 |
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 @travisgroth I think the best troubleshooting we could do for now is:
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 #7233 we resolved that to remove any GOPROXY override from the app. BTW we do set GOSUMDB=off though. |
Hi there, Help us by making a minimal reproduction repository. Before we can start work on your issue we first need to know exactly what's causing the current behavior. A minimal reproduction helps us with this. To get started, please read our guide on creating a minimal reproduction to understand what is needed. We may close the issue if you (or someone else) have not provided a minimal reproduction within two weeks. If you need more time, or are stuck, please ask for help or more time in a comment. Good luck, The Renovate team |
I'll ensure this issue doesn't get autoclosed due to lack of reproduction, but wanted to add this label to make it clear that the number one thing standing in the way of us fixing this is being able to reproduce it locally |
I know one area we still see more tidying required is when we have replace statements in our module that relate to another local module. I just had to update GoogleCloudPlatform/golang-samples#2478. You can see the additional go mod tidy that was required in a subsequent commit, because we replace another local module that had dep updates: commit |
@codyoss did you run just |
Just |
Not stale, still a priority |
this won't work on this repo until renovatebot/renovate#6213 (comment) is addressed
I think we may have finally got to the root cause of this go mod tidying problem. There is a new In short, configure like this:
|
Thank you @rarkins, I will work to update our config and report back next week how it went! |
FYI I will switch this to default behavior as soon as I get another positive feedback. |
We most certainly had issues with the With the new option I can confirm the issue has disappeared. |
🎉 This issue has been resolved in version 32.67.1 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
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
then ran
go get
andgo 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: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 havein the config.
The text was updated successfully, but these errors were encountered: