feat: enforce dependency on released versions of packages

[galargh]

Related Issues

Resolves #7131

Proposed Changes

This PR implements dependency on released versions enforcement as described in #7131.

Additional Info

In this iteration of the workflow, the information about the violations is described in the workflow run details. Here's an example from the following run - https://github.com/filecoin-project/lotus/actions/runs/12073744091?pr=12740

The list of approved unreleased dependencies is stored inline, in the workflow, see

lotus/.github/workflows/dependency-check.yml

Lines 20 to 77 in 36da613

	ALLOWED: |
	[
	{
	"Path": "github.com/filecoin-project/go-data-transfer/v2",
	"Version": "v2.0.0-rc7",
	"Reason": "unknown"
	},
	{
	"Path": "github.com/filecoin-project/go-state-types",
	"Version": "v0.16.0-dev",
	"Reason": "unknown"
	},
	{
	"Path": "github.com/syndtr/goleveldb",
	"Version": "v1.0.1-0.20210819022825-2ae1ddf74ef7",
	"Reason": "unknown"
	},
	{
	"Path": "github.com/xorcare/golden",
	"Version": "v0.6.1-0.20191112154924-b87f686d7542",
	"Reason": "unknown"
	},
	{
	"Path": "github.com/xordataexchange/crypt",
	"Version": "v0.0.3-0.20170626215501-b2862e3d0a77",
	"Reason": "unknown"
	},
	{
	"Path": "github.com/yugabyte/pgx/v5",
	"Version": "v5.5.3-yb-2",
	"Reason": "unknown"
	},
	{
	"Path": "go.dedis.ch/kyber/v4",
	"Version": "v4.0.0-pre2.0.20240924132404-4de33740016e",
	"Reason": "unknown"
	},
	{
	"Path": "gopkg.in/check.v1",
	"Version": "v1.0.0-20201130134442-10cb98267c6c",
	"Reason": "unknown"
	},
	{
	"Path": "gopkg.in/tomb.v1",
	"Version": "v1.0.0-20141024135613-dd632973f1e7",
	"Reason": "unknown"
	},
	{
	"Path": "honnef.co/go/tools",
	"Version": "v0.0.1-2020.1.4",
	"Reason": "unknown"
	},
	{
	"Path": "github.com/quic-go/webtransport-go",
	"Version": "v0.8.1-0.20241018022711-4ac2c9250e66",
	"Reason": "unknown"
	}
	]

Checklist

Before you mark the PR ready for review, please make sure that:

• Commits have a clear commit message.
• PR title conforms with contribution conventions
• Update CHANGELOG.md or signal that this change does not need it per contribution conventions
• New features have usage guidelines and / or documentation updates in
  • Lotus Documentation
  • Discussion Tutorials
• Tests exist for new functionality or change in behavior
• CI is green

[BigLep]

Thanks @galargh . This is an improvement I believe. I would like other maintainers to give the approval like @rvagg , @Stebalien , and/or @rjan90 .

A couple of things standing out to me:

1. I like that we're capturing the "reason" in our exception list.
2. I wonder though if it would be better to inline the exception list / reason inline go.mod:

require (
    github.com/yugabyte/pgx/v5 v5.5.3-yb-2     //  @unrelease-version-exceptoin-reason: "fill this in with the reason"
)

The presence of @unrelease-version-exceptoin-reason signals that this should have an exception and it also provides a reason.

Pros I see:

• All dependency info is in one file

Cons:

• There is more code to write for extracting which dependencies should have an exception. Basically, we can't use go list -m -json all and do easy jq commands like you have since the dependency comments aren't going to be included with with go list commands.
• Not one quick place to check for all dependencies that have an exception (although this is easy to get with a grep)

I also want to recognize that it was @Stebalien who suggested having a separate config file like this.

3. Every network upgrade we temporarily introduce some dev dependencies (e.g., go-state-types). Would we want to have the way to communicate a regex for a given dependency?

              {
                "Path": "github.com/filecoin-project/go-state-types",
                "VersionRegex": "v0.\d+.\d+-dev",
                "Reason": "Temporary dependency bubbling as part of a network upgrade release."
              },

Pros:

• One less step during network upgrades of having to update the exclusion list.

Cons:

• Complicates the checking logic (i.e., can't use the concise jq commands you have now).

@galargh : please don't act on any of my feedback yet. Lets get input from @Stebalien , @rvagg , and/or @rjan90 first before any action is taken (if any action is to be taken).

Thanks!

[Stebalien]

I wonder though if it would be better to inline the exception list / reason inline go.mod:

That would be nice but we'll have to be a bit careful to avoid bad interactions with tooling. From my basic testing it looks like go mod tidy preserves comments, although comments on indirect dependencies turn into // indirect; my comment.

Every network upgrade we temporarily introduce some dev dependencies (e.g., go-state-types). Would we want to have the way to communicate a regex for a given dependency?

I would at least allow a regex. Forcing the user to edit this file every time they want to update a dependency is going to be rather annoying.

Alternatively, we could support a set of flags (to avoid having to type out the same regular expressions over and over):

• "AllowUntaggedRelease" - allows arbitrary untagged versions.
• "AllowZeroVersion" - allows untagged versions if and only if there aren't any other tagged releases (we have 40 of these).
• "AllowDevRelease" - allows pre-release tags.

[rvagg]

I don't mind this, having an override is nice, having to put in notes is good too but it would be great to figure out what should go in for all of the ones listed so far.

[galargh]

In e18cc37, I was able to implement extracting the dependencies directly from the go.mod file and parsing special comment - dependency-check-ignore.

In 01b45ef I added these special comments to the currently offending dependencies to showcase how they would look like. I left the reason as unknown since I lacked the context to fill it out myself.

[Stebalien]

Ooh. That's nice.

[BigLep]

Thanks for the updates. I like the the inline dependency-check-ignore.

A few thoughts:

1. There is boiler plate reading in and out the GITHIB_OUTPUT for each step, but it doesn't look like breaking into steps actually gives us better presentation when I look at https://github.com/filecoin-project/lotus/actions/runs/12237619056/job/34133776622?pr=12740

Would it be cleaner to just do it all in one step and print out along the way?

2. My only other main concern is that we don't have a dependency-check-ignore-regex and it if it's going to be annoying to maintainers like @rjan90 who bubble up go-state-types vX.Y.z-dev versions periodically. That is the one area I can see where this new check could be inducing undesired work. I am fine to wait to see if that actually turns out to be a nuisance, but my worry with the current dependency-check.yml is that this will be a non-trivial addition since we have so far been relying on shell scripting and shell scripting maybe gets ugly to parse regexes to then apply to a given line. (I don't have experience here doing this in shell script land - it's just my gut feel). Things that would allay my concern are comments from maintainers saying that they don't need the regex or a sketch for how we'd quickly add it in future if we do want it.

3. For other reviewers, I don't think we should block this PR on the dependency-check-ignore: UNKNOWN cases. I'm all for figuring these out and lets do it if someone can quickly figure it out (less than 30 minutes total for all of them). If not, it should get backlogged since this PR isn't making things worse (it's just exposing the truth of the current situation).

[rvagg]

Suggested change

	github.com/filecoin-project/go-state-types v0.16.0-rc1 // dependency-check-ignore: unknown
	github.com/filecoin-project/go-state-types v0.16.0-rc1 // dependency-check-ignore: preparing for nv25

[rvagg]

inline ignore comments are nice, this is looking good

[rvagg]

@galargh see #12774 for my proposed changes btw, not sure if you saw my ping there

[BigLep]

I am good with this merging, once #12774 is merged in as well. At that point, all comments have been addressed. Thanks again @galargh and @rvagg !

[galargh]

There is boiler plate reading in and out the GITHIB_OUTPUT for each step, but it doesn't look like breaking into steps actually gives us better presentation when I look at https://github.com/filecoin-project/lotus/actions/runs/12237619056/job/34133776622?pr=12740

I added step names to all the steps. https://github.com/filecoin-project/lotus/actions/runs/12314231229/job/34369897245?pr=12740 should read better now :) I used the comments that we included for the step names as they explain what happens inside pretty well.

My only other main concern is that we don't have a dependency-check-ignore-regex and it if it's going to be annoying to maintainers like @rjan90 who bubble up go-state-types vX.Y.z-dev versions periodically. That is the one area I can see where this new check could be inducing undesired work. I am fine to wait to see if that actually turns out to be a nuisance, but my worry with the current dependency-check.yml is that this will be a non-trivial addition since we have so far been relying on shell scripting and shell scripting maybe gets ugly to parse regexes to then apply to a given line. (I don't have experience here doing this in shell script land - it's just my gut feel). Things that would allay my concern are comments from maintainers saying that they don't need the regex or a sketch for how we'd quickly add it in future if we do want it.

To be fair, I'm not sure if the regex support is necessary with the inline comments implementation. For example, now that we have a dependency-check-ignore comment next to go-state-types dependency, it will stay there until it is explicitly removed. In particular, it would survive any updates made through go get go-state-types@some-new-unreleased-version.

However, if we end up needing to add any more logic to the check, I think we should switch to a friendlier programming language, which should be pretty straightforward at this point.

@galargh see #12774 for my proposed changes btw, not sure if you saw my ping there

@rvagg Thanks! That's a great addition to the check 🚀 I think we can either a) merge as-is if you just rebase on top of this branch, or b) I could rework it in the style of the rest of the check myself (and possibly use a proper programming language while at it). @BigLep If we go with option b, I'd need a couple more hours to get it fully done if that's alright.

[rvagg]

I could rework it in the style of the rest of the check myself

please go ahead, this branch has changed a bit too much for a clean rebase and I don't have time just at the moment

[BigLep]

@galargh:

To be fair, I'm not sure if the regex support is necessary with the inline comments implementation. For example, now that we have a dependency-check-ignore comment next to go-state-types dependency, it will stay there until it is explicitly removed. In particular, it would survive any updates made through go get go-state-types@some-new-unreleased-version.

You're right. What you have is good and I don't think more is needed.

---

I think we just get the "released versions after v0.0.0" support from #12774 in and we ship this. Thanks!

[BigLep]

Per slack DM with @galargh, Lotus maintainers will take on merging #12774 into this PR (dealing with merge conflicts) and then merging the PR. I will plan on doing it next week unless someone gets to it first. Thanks all.