Skip to content

update go#36546

Closed
wxiaoguang wants to merge 1 commit into
go-gitea:mainfrom
wxiaoguang:update-go
Closed

update go#36546
wxiaoguang wants to merge 1 commit into
go-gitea:mainfrom
wxiaoguang:update-go

Conversation

@wxiaoguang
Copy link
Copy Markdown
Contributor

@wxiaoguang wxiaoguang commented Feb 6, 2026

Just a demo for #36541. Can be closed if no progress.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 6, 2026
@wxiaoguang wxiaoguang mentioned this pull request Feb 6, 2026
@wxiaoguang wxiaoguang marked this pull request as draft February 6, 2026 16:18
@silverwind
Copy link
Copy Markdown
Member

silverwind commented Feb 8, 2026

The problem with go get is you can not exclude certain dependencies, for example when they contain incompatible breaking changes etc. A replace statements is not a full replacement when you want to pin to a certain semver range for example. While rare, a module could be releasing versions at both v1 and v2 tracks, for example. updates offers a pin option for that.

But we can still merge this as this is better than not having a update-go.

Actual dependency changes should be reverted, we have #36548 up.

@wxiaoguang
Copy link
Copy Markdown
Contributor Author

wxiaoguang commented Feb 8, 2026

A replace statements is not a full replacement when you want to pin to a certain semver range for example. While rare, a module could be releasing versions at both v1 and v2 tracks, for example. updates offers a pin option for that.

This update does include a "pin" requirement, I have made some changes to make the code compile in another branch but not here.

Using replace in go.mod is the only correct approach.

If you think your tool can handle it, just do it. Show the code.

@silverwind
Copy link
Copy Markdown
Member

silverwind commented Feb 8, 2026

If you think your tool can handle it, just do it. Show the code.

In updates.config.ts:

  pin: {
    "github.com/foo/bar": "^5.0.0",
  },

@wxiaoguang
Copy link
Copy Markdown
Contributor Author

wxiaoguang commented Feb 8, 2026

If you think your tool can handle it, just do it. Show the code.

In update.config.ts:

  pin: {
    "github.com/foo/bar": "^5.0.0",
  },

No, it won't work for Go dependency problems. Show your change in this PR

@silverwind
Copy link
Copy Markdown
Member

I don't understand what you want. In any case, revert the dependency updates here please.

@wxiaoguang
Copy link
Copy Markdown
Contributor Author

I don't understand what you want. In any case, revert the dependency updates here please.

I would really suggest you first spend enough time on maintaining Golang's related packages before reinventing your wheel.

You just never really handled the complete go.mod upgrade, but just imagine that "it should be like that".

You can check the change here: https://github.com/wxiaoguang/gitea/tree/temp-fix-go-dep , in this case, go.yaml.in/yaml/v4 should be locked, it is a dependency's dependency.


In any case, reinventing a tool which isn't necessary is not a right practice. You can't just say "it looks faster" or "it manage locked packages together" then introduce a untested toy into a production repo, actually Golang go.mod doesn't work that way.

This is my final conclusion for this problem.

@wxiaoguang
Copy link
Copy Markdown
Contributor Author

wxiaoguang commented Feb 8, 2026

Actual dependency changes should be reverted, we have #36548 up.

#36548 has been merged, it upgraded some packages which don't cause problems. This PR has been rebased on main branch.

Now, the remaining/renaming packages, challenge work, if you'd like to try, feel free to. Or maybe I can handle it one week later.

@silverwind
Copy link
Copy Markdown
Member

Another feature that go get lacks is import path rewriting, I will soon add it to updates.

@wxiaoguang
Copy link
Copy Markdown
Contributor Author

Another feature that go get lacks is import path rewriting, I will soon add it to updates.

That's not how go.mod works.

@silverwind
Copy link
Copy Markdown
Member

silverwind commented Feb 9, 2026

Another feature that go get lacks is import path rewriting, I will soon add it to updates.

That's not how go.mod works.

If you use go get to update to new major versions, the code will no longer compile. It's a inherent flaw in the go get method imho.

@wxiaoguang
Copy link
Copy Markdown
Contributor Author

I don't understand what you are talking about. You are just guessing and imagining.

You have never really maintained the Go packages or dependencies. It’s like someone on the street runs up to you and says, “Scalpels are terrible. I modified a kitchen knife myself—let me use it to perform brain surgery on you.”

@wxiaoguang
Copy link
Copy Markdown
Contributor Author

If you are not able to make this PR compile with proper dependency management, don't talk with me about this topic.

You can learn about how to use replace to lock a package's version (#36546 (comment)), how Golang resolves dependencies for different packages and resolve conflicts, why yaml/v3 and yaml/4 should co-exist, etc.

@silverwind
Copy link
Copy Markdown
Member

silverwind commented Feb 9, 2026

Aren't replace directives a bad practice because it's not obvious to the code reader what the actual version in use actually is? Say you load module/v2 in code but the go.mod replaces module/v2 with module/v3. A reader could think the code still uses v2.

I think it's bad practice to use replace directives for major version upgrades and it's much better to rewrite import paths in **/*.go after major upgrades, and go get -u can not do this so your update-go task is incomplete.

Comment thread Makefile
update-go: ## update go dependencies
$(GO) get -u ./...
$(MAKE) tidy
$(MAKE) go-licenses
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not needed, tidy already updates the license file

@silverwind
Copy link
Copy Markdown
Member

I can accept the current version with go get, it's better than nothing.

@wxiaoguang
Copy link
Copy Markdown
Contributor Author

I can accept the current version with go get, it's better than nothing.

You can also make the code compile to show that you really understand how dependency works.

@wxiaoguang wxiaoguang closed this Feb 13, 2026
@wxiaoguang wxiaoguang deleted the update-go branch February 13, 2026 19:04
@silverwind
Copy link
Copy Markdown
Member

silverwind commented Feb 18, 2026

I'm of the opinion that replace statements are dirty and should not be used. It's basically the same story as package.json overrides which is also dirty.

If you want a specific depdency version, specify it directly, no need to add needless indirections to the dependency resolution like replace or overrides.

@go-gitea go-gitea locked as resolved and limited conversation to collaborators May 15, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants