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.

Sign up for GitHub

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

Jump to bottom

build(deps): bump github.com/go-git/go-git/v5 from 5.4.2 to 5.5.1 #82

Closed
wants to merge 1 commit into from
Closed

build(deps): bump github.com/go-git/go-git/v5 from 5.4.2 to 5.5.1 #82

wants to merge 1 commit into from

Conversation

dependabot[bot]
Copy link
Contributor

@dependabot dependabot bot commented on behalf of github Dec 13, 2022

Bumps github.com/go-git/go-git/v5 from 5.4.2 to 5.5.1.

Release notes

Sourced from github.com/go-git/go-git/v5's releases.

v5.5.1

What's Changed

Full Changelog: go-git/go-git@v5.5.0...v5.5.1

v5.5.0

What's Changed

Full Changelog: go-git/go-git@v5.4.2...v5.5.0

Commits
  • 736622f .github: test, remove coveralls
  • e43edee Merge pull request #617 from doxsch/616-update-ssh-agent-to-master
  • f62ac39 Merge pull request #625 from pjbgf/bump-sha1cd-nocgo
  • c7050e7 Merge pull request #623 from pjbgf/empty-commit
  • 08db65f fix: Upgrade github.com/xanzy/ssh-agent to v0.3.3 to fix panic
  • a513415 Return error instead of creating empty commits
  • 223e732 build: Bump github.com/pjbgf/sha1cd to v0.2.3
  • a0b612a build: Add CI check for CGO_ENABLED=0
  • 3e07c50 Merge pull request #620 from fluxcd/update-deps
  • f2d68c4 build: bump git workflow to Go 1.19
  • Additional commits viewable in compare view

Dependabot compatibility score

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.

Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

@dependabot
build(deps): bump github.com/go-git/go-git/v5 from 5.4.2 to 5.5.1
fcd5c1f 
Bumps [github.com/go-git/go-git/v5](https://github.com/go-git/go-git) from 5.4.2 to 5.5.1.
- [Release notes](https://github.com/go-git/go-git/releases)
- [Commits](go-git/go-git@v5.4.2...v5.5.1)

---
updated-dependencies:
- dependency-name: github.com/go-git/go-git/v5
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
@dependabot dependabot bot added the dependencies Auto-pinning label Dec 13, 2022
@dependabot dependabot bot mentioned this pull request Dec 13, 2022
Closed
@kmoe
Copy link
Member

kmoe commented Dec 13, 2022

windows-latest test failure looks intermittent:

==4492==ERROR: ThreadSanitizer failed to allocate 0x0000013e1000 (20844544) bytes at 0x200da79068000 (error code: 87)

rerunning

@kmoe kmoe self-assigned this Dec 13, 2022
@kmoe
Copy link
Member

kmoe commented Dec 13, 2022

Failed again. Apparently the ThreadSanitizer issue is something to do with mingw. Looking into it. actions/runner-images#5841

This may be neither here nor there, but interesting to note that hashicorp/go-getter doesn't use https://github.com/go-git/go-git: https://github.com/hashicorp/go-getter/blob/main/get_git.go#L26 Not sure if it's worth reviewing this decision depending on our expectations of the hc-install environment. cc @radeksimko if you have a strong opinion about this, please make an issue, otherwise I will do so later.

@radeksimko
Copy link
Member

@kmoe I originally thought it's resource (memory) usage related #81 (comment) but I may be wrong tbh. It's mostly theory based on:

  • go-git deals with cloning pretty big repositories
  • the impacted test likely spends a lot of resources to also build it (i.e. go build is presumably not cheap to run)
  • Although Linux and Windows GHA environments have the same resource constraints, I assume that some of that will be eaten by the system itself, so there may be differences between the two.

It's still hard to believe that we'd consume all 7GB of RAM, but I also don't have the confidence to rule it out.

I was planning to write and run some benchmarks and/or to reproduce it in a Windows VM, but I haven't had the chance to try either yet.

@radeksimko
Copy link
Member

radeksimko commented Dec 19, 2022 •
edited

@kmoe Some findings from my local Windows VM, which confirm your theory about MinGW. 😄

Testing build package fails w/ a freshly installed go1.19.4 windows/amd64 on a freshly installed Windows 11.
gcc is not installed on Windows by default and CGO_ENABLED is also set to 1 by default. Disabling CGO does make the test pass.

> go test ./...
# runtime/cgo
cgo: C compiler "gcc" not found: exec: "gcc": executable file not found in %PATH%
ok      github.com/hashicorp/hc-install (cached)
FAIL    github.com/hashicorp/hc-install/build [build failed]
> go env
# ...
set CGO_ENABLED=1
# ...

Using the same default settings/configuration (w/ CGO enabled), the build tests are just passing on main.

The easy fix here could be just claiming no support for CGO and adding the corresponding variable to the CI runs, but I'd still like to reproduce the original problem - i.e. install gcc (or its equivalent) in my Windows VM and understand why it's happening.

@radeksimko
Copy link
Member

radeksimko commented Dec 19, 2022 •
edited

btw. This problem was likely introduced in go-git/go-git#618 and there was a related PR which fixed similar problem when CGO is disabled:

go-git/go-git#625

@radeksimko
Copy link
Member

After choco install mingw:

C:\Users\radeksimko\hc-install>go test ./...
ok      github.com/hashicorp/hc-install (cached)
# github.com/hashicorp/hc-install/build.test
C:\Program Files\Go\pkg\tool\windows_amd64\link.exe: running gcc failed: exit status 1
c:/programdata/chocolatey/lib/mingw/tools/install/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/12.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: C:\Users\RADEKS~1\AppData\Local\Temp\go-link-4057618357\000008.o: in function `_cgo_preinit_init':\\_\_\runtime\cgo/gcc_libinit_windows.c:40: undefined reference to `__imp___iob_func'
c:/programdata/chocolatey/lib/mingw/tools/install/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/12.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: C:\Users\RADEKS~1\AppData\Local\Temp\go-link-4057618357\000008.o: in function `x_cgo_notify_runtime_init_done':
\\_\_\runtime\cgo/gcc_libinit_windows.c:105: undefined reference to `__imp___iob_func'
c:/programdata/chocolatey/lib/mingw/tools/install/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/12.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: C:\Users\RADEKS~1\AppData\Local\Temp\go-link-4057618357\000008.o: in function `_cgo_beginthread':
\\_\_\runtime\cgo/gcc_libinit_windows.c:149: undefined reference to `__imp___iob_func'
c:/programdata/chocolatey/lib/mingw/tools/install/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/12.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: C:\Users\RADEKS~1\AppData\Local\Temp\go-link-4057618357\000009.o: in function `x_cgo_thread_start':
\\_\_\runtime\cgo/gcc_util.c:18: undefined reference to `__imp___iob_func'
collect2.exe: error: ld returned 1 exit status

FAIL    github.com/hashicorp/hc-install/build [build failed]

😑 I'm kind of clueless at this point honestly.

It would be great if hc-install had minimal expectations, as a library, but this leaves us with only a few options:

  1. kindly ask go-git/go-git maintainers whether they'd reconsider reverting/re-implementing sha1: Add collision resistent implementation go-git/go-git#618 such that it doesn't require CGO
  2. remove go-git as a dependency and introduce git binary dependency
  3. continue down the rabbit hole and find out if there's any possible way (changing anything about the Windows environment as provided by GitHub) we can make the CGO part compile

None of these are ideal, but starting with the first one may not hurt.

@radeksimko
Copy link
Member

Looking more closely at the upstream changes in go-git and https://github.com/pjbgf/sha1cd it appears that it does have some reasonable fallbacks for non-CGO environments.

My understanding of the problem is that making CGO work on Windows would be struggle in general, and introduction of a CGO dependency, in combination w/ CGO_ENABLED=1 default just surfaced that fact. That is, if someone really does want to use CGO and Windows as downstream consumer of hc-install, they're likely to run into problems either way - which are not necessarily related to hc-install and/or that dependency.

Therefore I am inclined to go for the "easy fix" of adding CGO_ENABLED=0 to the CI. However, I think we'd still need to add a very visible call-out in the changelog, as it's likely that most of downstream also runs with Go defaults and tests on Windows, but never actually attempts to compile any CGO code.

@kmoe Any objections to this resolution?

@radeksimko radeksimko mentioned this pull request Dec 19, 2022
Closed
@radeksimko
Copy link
Member

Raised a question upstream in go-git/go-git#637

@dependabot @github
Copy link
Contributor Author

dependabot bot commented on behalf of github Jan 6, 2023

Superseded by #84.

@dependabot dependabot bot closed this Jan 6, 2023
@dependabot dependabot bot deleted the dependabot/go_modules/github.com/go-git/go-git/v5-5.5.1 branch January 6, 2023 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@kmoe kmoe

Labels
dependencies Auto-pinning
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@kmoe @radeksimko