Skip to content
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

fix a Sirupsen/logrus import issue in CI #527

Merged
merged 1 commit into from
Jan 24, 2024

Conversation

tych0
Copy link
Member

@tych0 tych0 commented Jan 23, 2024

we are seeing:

2.947 go: github.com/opencontainers/image-tools/cmd/oci-image-tool imports
2.947 github.com/Sirupsen/logrus: github.com/Sirupsen/[email protected]: parsing go.mod:
2.947 module declares its path as: github.com/sirupsen/logrus
2.947 but was required as: github.com/Sirupsen/logrus

in CI, hopefully this replace fixes it?

@tych0
Copy link
Member Author

tych0 commented Jan 23, 2024

that wasn't it, trying something else...

@codecov-commenter
Copy link

codecov-commenter commented Jan 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (501944d) 45.71% compared to head (8de4875) 73.48%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main     #527       +/-   ##
===========================================
+ Coverage   45.71%   73.48%   +27.77%     
===========================================
  Files          57       60        +3     
  Lines        4843     4888       +45     
===========================================
+ Hits         2214     3592     +1378     
+ Misses       2363      937     -1426     
- Partials      266      359       +93     

see 37 files with indirect coverage changes

@tych0
Copy link
Member Author

tych0 commented Jan 23, 2024

@cyphar i'm not sure if zypper has a "don't include prereleases" option, that's probably better than just hard coding 1.21, since we'll have to keep bumping it forever...

@cyphar
Copy link
Member

cyphar commented Jan 23, 2024

The reason is that our packaging in openSUSE has (had?) a bug where trying to install just "go" didn't give you the latest version. It should've been fixed, but since I handed off the Go packaging to a different guy at SUSE a few years ago it's been harder to get answers to these kinds of questions (I used to do Go packaging for (open)SUSE). We can try to remove and see what happens...

go.mod Outdated Show resolved Hide resolved
@@ -32,7 +32,7 @@ RUN zypper -n in \
curl \
git \
gnu_parallel \
"go>=1.18" \
"go==1.21" \
Copy link
Member

Choose a reason for hiding this comment

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

You've changed from >= to ==. 😅

Copy link
Member Author

@tych0 tych0 Jan 24, 2024

Choose a reason for hiding this comment

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

yup, that's the goal of the patch. otherwise we end up installing golang 1.22 which complains about the capitalization.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, that's the issue? That is very weird. I'll need to look into it...

we are seeing:

2.947 go: github.com/opencontainers/image-tools/cmd/oci-image-tool imports
2.947 	github.com/Sirupsen/logrus: github.com/Sirupsen/[email protected]: parsing go.mod:
2.947 	module declares its path as: github.com/sirupsen/logrus
2.947 	        but was required as: github.com/Sirupsen/logrus

in CI, but that's (maybe) because of:

 opencontainers#8 22.11 Retrieving: go1.22-1.22rc1-lp155.7.1.x86_64 (obs-go) (67/68),  34.9 MiB
 opencontainers#8 22.11 Retrieving: go1.22-1.22rc1-lp155.7.1.x86_64.rpm [....................done (18.3 MiB/s)]
 opencontainers#8 24.27 Retrieving: go-1.22-lp155.1.1.x86_64 (obs-go) (68/68),  35.4 KiB
 opencontainers#8 24.27 Retrieving: go-1.22-lp155.1.1.x86_64.rpm [.....done (53.7 KiB/s)]

go 1.22 is not yet released, so perhaps it's a regression?

Signed-off-by: Tycho Andersen <[email protected]>
Copy link
Member

@cyphar cyphar left a comment

Choose a reason for hiding this comment

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

LGTM, just to fix CI. It is very concerning that Go 1.22 might break working projects?

Also I'm confused why there is a pre-release version of Go in the repos... I need to speak to the Go maintainer for openSUSE...

@tych0
Copy link
Member Author

tych0 commented Jan 24, 2024

yeah, i'm hopeful it's just a regression.

@tych0 tych0 merged commit 9da0fd4 into opencontainers:main Jan 24, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants