Skip to content

Conversation

Luap99
Copy link
Member

@Luap99 Luap99 commented Aug 28, 2025

Make sure all go module related changes are committed properly.

@Luap99 Luap99 marked this pull request as draft August 28, 2025 16:12
@Luap99 Luap99 force-pushed the go-tidy-ci branch 4 times, most recently from cf55501 to 73662ac Compare August 28, 2025 17:38
@Luap99 Luap99 changed the title cirrus: ensure make tidy is clean gh action: add go tidy check Aug 28, 2025
@Luap99 Luap99 marked this pull request as ready for review August 28, 2025 17:39
@Luap99
Copy link
Member Author

Luap99 commented Aug 28, 2025

@jankaluza
Copy link
Member

LGTM

Copy link
Contributor

@mtrmac mtrmac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes please — one nit.

@github-actions github-actions bot added the storage Related to "storage" package label Aug 29, 2025
@Luap99
Copy link
Member Author

Luap99 commented Aug 29, 2025

Looks like #292 worked as well and added the storage label so that is good.

Copy link
Contributor

@mtrmac mtrmac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Luap99 Note #27 : this updates google.golang.org/protobuf in common, but it did not update the reference in image; so, if we enforced consistency with go work sync, Renovate PRs would break that (or, hopefully, fail tests).

Can Renovate be configured to do a go work-wide update?

@Luap99
Copy link
Member Author

Luap99 commented Aug 29, 2025

Can Renovate be configured to do a go work-wide update?

I though it already did, at least when I looked at it with @jankaluza why the workspace vendoring was broken. It definably has workspace aware code so maybe some extra config option.

@Luap99
Copy link
Member Author

Luap99 commented Aug 29, 2025

Mhh if I look at the logs https://developer.mend.io/github/containers/container-libs/-/job/d0c5143f-0e63-4056-b583-398cdd67ac40 it definitely claims it runs go work sync, I guess it might be the same issue with the vendor dir where the resulting diff is not added to the commit

renovatebot/renovate#37653

@mtrmac
Copy link
Contributor

mtrmac commented Aug 29, 2025

Yes, previously #283 did sync things — But that one had a reason to update */go.* anyway.

@Luap99
Copy link
Member Author

Luap99 commented Aug 29, 2025

Looking at the renovate config I am not sure we can make it work, first our config is wrong as we still ask for vendoring

but when we remove it will no longer run go work sync as this is put inside the useVendor condition.
https://github.com/renovatebot/renovate/blob/6e00de5daf57f7ab240b0d085b4edd469f738f30/lib/modules/manager/gomod/artifacts.ts#L291
you can even see it the current log it still runs go work vendor.

So to me I guess renovate is just not working no matter what config we use, I guess best case we just make a PR to renovate to fix the behavior. I am not sure if the discussion will gain much traction otherwise, the repo seems quite busy.

@mtrmac
Copy link
Contributor

mtrmac commented Aug 29, 2025

For a while, it wouldn’t be so bad for us to manually amend every Renovate-filed PR with a go work sync. Longer-term, it would be an annoyance.

I agree contributing a PR to Renovate upstream would be the cleanest approach.

We could, also, give up on keeping the repo consistent with go work sync (and I guess add go.work.sum to .gitignore). That would be consistent with how it used to work, where we sync indirect dependencies only when tagging releases. It’s clearly worse than keeping the workspace synchronized.

I guess the decision ~depends on a guess how much effort+time it would take to improve Renovate.

Make sure all go module related changes are committed properly.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
@Luap99
Copy link
Member Author

Luap99 commented Sep 1, 2025

Dropped the commit which added go work sync to the Makefile, however as it seems just go tidy in eahc module will still populate the go.work.sum file

tree is dirty, please run "make tidy" and commit all changes.

 M go.work.sum

---------------------- Diff below ----------------------

diff --git a/go.work.sum b/go.work.sum
index 614095b..ca28494 100644
--- a/go.work.sum
+++ b/go.work.sum
@@ -114,6 +114,7 @@ github.com/golang/protobuf v1.4.0-rc.4.0.20200313231945-b860323f09d0/go.mod h1:W
 github.com/golang/protobuf v1.4.0/go.mod h1:jodUvKwWbYaEsadDk5Fwe5c77LiNKVO9IDvqG2KuDX0=
 github.com/golang/protobuf v1.4.1/go.mod h1:U8fpvMrcmy5pZrNK1lt4xCsGvpyWQ/VVv6QDs8UjoX8=
 github.com/golang/protobuf v1.4.3/go.mod h1:oDoupMAO8OvCJWAcko0GGGIgR6R6ocIYbsSw735rRwI=
+github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk=
 github.com/golang/snappy v0.0.4/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q=
 github.com/google/certificate-transparency-go v1.3.1/go.mod h1:gg+UQlx6caKEDQ9EElFOujyxEQEfOiQzAt6782Bvi8k=
 github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M=
@@ -121,6 +122,7 @@ github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMyw
 github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.5.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.5.3/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
+github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/s2a-go v0.1.9/go.mod h1:YA0Ei2ZQL3acow2O62kdp9UlnvMmU7kA6Eutn0dXayM=
 github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/googleapis/enterprise-certificate-proxy v0.3.4/go.mod h1:YKe7cfqYXjKGpGvmSg28/fFvhNzinZQm8DGnaburhGA=
@@ -282,6 +284,8 @@ google.golang.org/protobuf v1.22.0/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2
 google.golang.org/protobuf v1.23.0/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2l/sGQquU=
 google.golang.org/protobuf v1.23.1-0.20200526195155-81db48ad09cc/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2l/sGQquU=
 google.golang.org/protobuf v1.25.0/go.mod h1:9JNX74DMeImyA3h4bdi1ymwjUzf21/xIlbajtzgsN7c=
+google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw=
+google.golang.org/protobuf v1.33.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos=
 gopkg.in/ini.v1 v1.67.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k=
 gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7/go.mod h1:dt/ZhP58zS4L8KSrWDmTeBkI65Dw0HsyUHuEVlX15mw=
 honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
Error: Process completed with exit code 1.

So I guess that means renovate PRs would still break on that check thus we cannot merge until renovate gets fixed unless we like to amend all the PRs which I don't

@mtrmac
Copy link
Contributor

mtrmac commented Sep 1, 2025

We could explicitly ignore the top-level go.work.sum, and still enforce within-module tidy validation. It’s not as good, maybe still worthwhile.

OTOH if we think Renovate is going to be fixed shortly, it might be easier to wait than to do things twice.

@Luap99
Copy link
Member Author

Luap99 commented Sep 1, 2025

OTOH if we think Renovate is going to be fixed shortly, it might be easier to wait than to do things twice.

Well that is the problem, we cannot know that side. Even if Jan submits a fix, it doesn't mean it gets merged anytime soon. And then we use the hosted mend instance so we get to wait until the fix gets deployed there and who knows how long that will take.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

storage Related to "storage" package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants