Bump dependencies #24

MrDOS · 2022-08-22T21:36:33Z

At risk of burying the lede, the important change here is bumping golang.org/x/sys, because this adds native support for Darwin on ARM64. With the old version, building on an Apple Silicon Mac fails:

$ go build ./cmd/...
# golang.org/x/sys/unix
../../go/pkg/mod/golang.org/x/sys@v0.0.0-20200302150141-5c8b2ff67527/unix/syscall_darwin.1_13.go:25:3: //go:linkname must refer to declared function or variable
../../go/pkg/mod/golang.org/x/sys@v0.0.0-20200302150141-5c8b2ff67527/unix/zsyscall_darwin_arm64.1_13.go:27:3: //go:linkname must refer to declared function or variable
../../go/pkg/mod/golang.org/x/sys@v0.0.0-20200302150141-5c8b2ff67527/unix/zsyscall_darwin_arm64.1_13.go:40:3: //go:linkname must refer to declared function or variable
../../go/pkg/mod/golang.org/x/sys@v0.0.0-20200302150141-5c8b2ff67527/unix/zsyscall_darwin_arm64.go:28:3: //go:linkname must refer to declared function or variable
../../go/pkg/mod/golang.org/x/sys@v0.0.0-20200302150141-5c8b2ff67527/unix/zsyscall_darwin_arm64.go:43:3: //go:linkname must refer to declared function or variable
../../go/pkg/mod/golang.org/x/sys@v0.0.0-20200302150141-5c8b2ff67527/unix/zsyscall_darwin_arm64.go:59:3: //go:linkname must refer to declared function or variable
../../go/pkg/mod/golang.org/x/sys@v0.0.0-20200302150141-5c8b2ff67527/unix/zsyscall_darwin_arm64.go:75:3: //go:linkname must refer to declared function or variable
../../go/pkg/mod/golang.org/x/sys@v0.0.0-20200302150141-5c8b2ff67527/unix/zsyscall_darwin_arm64.go:90:3: //go:linkname must refer to declared function or variable
../../go/pkg/mod/golang.org/x/sys@v0.0.0-20200302150141-5c8b2ff67527/unix/zsyscall_darwin_arm64.go:105:3: //go:linkname must refer to declared function or variable
../../go/pkg/mod/golang.org/x/sys@v0.0.0-20200302150141-5c8b2ff67527/unix/zsyscall_darwin_arm64.go:121:3: //go:linkname must refer to declared function or variable
../../go/pkg/mod/golang.org/x/sys@v0.0.0-20200302150141-5c8b2ff67527/unix/zsyscall_darwin_arm64.go:121:3: too many errors

With this new version, the build succeeds, and produces a useful executable.

While I was making this change anyway, I thought it would be useful to attempt to bump other dependencies.

This meant resolving a minor breaking change in github.com/ekzhu/minhash-lsh.
I also wasn't able to bump gonum.org/v1/gonum past v0.8.2 because of a breaking change in v0.9.0. See the commit message of f4552ce for details.

This gets support for Darwin on ARM64.

The newest version of the library has breaking changes. Because the library is still v0, a blind invocation of `go get -u` in a downstream project will happily pull in the newest version of this dependency, causing compilation errors here.

I'm holding the package at v0.8.2 because commit 8c7eded changes the behaviour of `distuv.Gamma.Rand`, causing test failures. I don't know how these hashes are being used (e.g., whether they're being persisted), or to what degree their stability matters, so I'm steering clear of an obviously-breaking change.

bzz · 2022-10-06T15:23:13Z

Thank you for catching and fixing this @MrDOS 🙇
Closed&re-opened to trigger the CI.

bzz

LGTM!

bzz · 2022-10-06T15:26:16Z

licensedb/internal/db.go

@@ -221,7 +221,7 @@ func loadLicenses() *database {
 		db.tokens[token] = i
 		db.docfreqs[i] = docfreqs[token]
 	}
-	db.lsh = minhashlsh.NewMinhashLSH64(numHashes, similarityThreshold)
+	db.lsh = minhashlsh.NewMinhashLSH64(numHashes, similarityThreshold, len(tokenFreqs))


This seems to be needed for ekzhu/minhash-lsh@535c16f

bzz · 2022-10-06T15:28:41Z

Windows test were removed in #23 so it may be worth merging it first and rebasing this, to get a better CI results.

bzz · 2022-10-06T15:44:24Z

@MrDOS could you please rebase it on the latest master, now that win CI profile was removed?

Also, there seems to be a wired issue with linter CI profile, do you know what might be the reason? In #23 it seems to be working just fine.

level=warning msg="[runner] Can't run linter goanalysis_metalinter: buildir: failed to load package goarch: could not load export data: cannot import "internal/goarch" (unknown iexport format version 2), export data is newer version - update tool"

The project no longer depends on the versions being replaced, so the replacements can be removed.

The previous version was failing with an error after the preceding dependency bumps: Running error: buildir: failed to load package goarch: could not load export data: cannot import "internal/goarch" (unknown iexport format version 2), export data is newer version - update tool

MrDOS · 2022-10-06T21:38:03Z

Hi @bzz, thanks for taking a look at this.

I've rebased onto master.
I've re-bumped dependencies to catch any new versions that have been released over the last ~month.
I've also removed a couple of seemingly-obsolete dependency replacements.
I've bumped the version of Go used by the project to 1.17 across the test, race, and release actions. See below.
I've bumped the version of golangci-lint used by the linting action.

I think the error from golangci-lint is because some of the newer dependencies ask for Go 1.18 (github.com/kevinburke/ssh_config, golang.org/x/exp, and github.com/neurosnap/sentences), and the older version of golangci-lint didn't support it properly across all of its linters. I suppose that even though this project isn't using Go 1.18, golangci-lint still tries to explore its dependencies.

I wasn't familiar with what happened when a Go module calls for an older version of Go than its dependencies. My understanding now is that, because dependencies are imported as source files and fed into the same compiler as the rest of the build, dependencies which ask for a newer version of Go will still work if the imported packages don't use any features from the newer language version. And I suppose the version number in go.mod really is an ask, not a requirement – you decide what version of Go you're building with when you invoke the build process.

I tried building the application with Go 1.14, because that's the default that was used in the release action; but it failed due to the hard dependency on 1.16 introduced in #21. And Go 1.16 failed to build or test because of a hard dependency on 1.17 in the newest version of golang.org/x/sys. So even though a few of the dependencies (as noted) want 1.18, 1.17 seems like a safe enough target. The project builds and tests pass there. Particularly as this project is a library, you probably don't want to go too high to give consumers some margin to do their own updates. But 1.17 has been out for over a year, so it's hopefully sufficiently common-denominator enough by now.

Alternatively, I could roll dependency versions back down until I reach a set which are compatible with Go 1.16. Or, I could just not bump dependencies generally, and only specifically bump dependencies required to get the project to work properly with macOS ARM64. Your call. Either way, thanks for responding, and thanks for taking a look.

.github/workflows/lint.yml

bzz · 2022-10-09T08:35:48Z

Thank you very much for digging deeper into it, your analysis makes perfect sense and Go 1.17 as a target sounds very reasonable.

bzz · 2022-10-09T08:49:42Z

I wonder, why Race CI profile takes 40min and rolling... 🤔
Oh, my local machine tests take 2min and indeed claimed runtime overhear of execution time by 2-20x gives smth like that, although locally it's ~10min for me on M1.

Makes me wonder if that should be something optional rather than mandatory.. but I could not find Github equivalent of this Gitlab when: manual feature 😞

Only report issues in changed code & use latest linter

bump linter action version

MrDOS · 2022-10-09T11:12:44Z

Oh, cool, I'd never noticed that the golangci-lint action could take “latest” as a tool version. That's convenient.

It looks like v3 of the action wants the actions/setup-go action to be explicitly invoked to install Go in the runner environment. I'll make that change.

Were there any other changes you wanted me to make?

bzz · 2022-10-09T18:47:28Z

It looks like v3 of the action wants the actions/setup-go action to be explicitly invoked to install Go in the runner environment. I'll make that change.

Oh, I see. But it also seems to be working alright now, doesn't it?

Were there any other changes you wanted me to make?

No, I've committed the suggestions and just forgot to change the review status 😉

MrDOS · 2022-10-09T20:35:01Z

But it also seems to be working alright now, doesn't it?

Yeah, I think it's just using whatever version of Go is on the PATH in the runner. That's probably fine.

It does seem a little weird that the lint action runs tests, too, but also not actively harmful.

Thanks for the green checkmark! I'm happy with this if you're happy with this.

bzz · 2022-10-11T19:43:54Z

It does seem a little weird that the lint action runs tests, too

Thank you for bringing this up, indeed, this does not seem to be necessary. On the second thought, not relying on container's go version may be a good idea too.

Thank you very much for the improvements! Will merge and 🔪 a new release tomorrow.

To1ne · 2023-01-03T10:20:04Z

@bzz Happy new year! Friendly reminder, would you mind to create a release that includes these changes?

MrDOS added 3 commits August 22, 2022 22:28

Bump golang.org/x/sys

0b0b881

This gets support for Darwin on ARM64.

Bump github.com/ekzhu/minhash-lsh

cf9f407

The newest version of the library has breaking changes. Because the library is still v0, a blind invocation of `go get -u` in a downstream project will happily pull in the newest version of this dependency, causing compilation errors here.

bzz closed this Oct 6, 2022

bzz reopened this Oct 6, 2022

bzz approved these changes Oct 6, 2022

View reviewed changes

bzz mentioned this pull request Oct 6, 2022

Access additional license data #23

Merged

MrDOS added 4 commits October 6, 2022 21:33

Remove obsolete dependency replacements

1452a05

The project no longer depends on the versions being replaced, so the replacements can be removed.

Bump remaining dependencies

13c8db7

Bump Go to 1.17

73267aa

bzz requested changes Oct 9, 2022

View reviewed changes

.github/workflows/lint.yml Outdated Show resolved Hide resolved

.github/workflows/lint.yml Outdated Show resolved Hide resolved

bzz added 2 commits October 9, 2022 11:05

Update .github/workflows/lint.yml

873bbae

Only report issues in changed code & use latest linter

Update .github/workflows/lint.yml

ff81296

bump linter action version

bzz approved these changes Oct 9, 2022

View reviewed changes

ci: fixed min Go version & no tests in lint

f6b5690

bzz merged commit 18a439e into go-enry:master Oct 11, 2022

MrDOS deleted the improvement/bump-dependencies branch October 11, 2022 21:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump dependencies #24

Bump dependencies #24

MrDOS commented Aug 22, 2022

bzz commented Oct 6, 2022 •

edited

Loading

bzz left a comment

bzz Oct 6, 2022

bzz commented Oct 6, 2022

bzz commented Oct 6, 2022 •

edited

Loading

MrDOS commented Oct 6, 2022 •

edited

Loading

bzz commented Oct 9, 2022

bzz commented Oct 9, 2022 •

edited

Loading

MrDOS commented Oct 9, 2022

bzz commented Oct 9, 2022

MrDOS commented Oct 9, 2022

bzz commented Oct 11, 2022

To1ne commented Jan 3, 2023

Bump dependencies #24

Bump dependencies #24

Conversation

MrDOS commented Aug 22, 2022

bzz commented Oct 6, 2022 • edited Loading

bzz left a comment

Choose a reason for hiding this comment

bzz Oct 6, 2022

Choose a reason for hiding this comment

bzz commented Oct 6, 2022

bzz commented Oct 6, 2022 • edited Loading

MrDOS commented Oct 6, 2022 • edited Loading

bzz commented Oct 9, 2022

bzz commented Oct 9, 2022 • edited Loading

MrDOS commented Oct 9, 2022

bzz commented Oct 9, 2022

MrDOS commented Oct 9, 2022

bzz commented Oct 11, 2022

To1ne commented Jan 3, 2023

bzz commented Oct 6, 2022 •

edited

Loading

bzz commented Oct 6, 2022 •

edited

Loading

MrDOS commented Oct 6, 2022 •

edited

Loading

bzz commented Oct 9, 2022 •

edited

Loading