Skip to content

feat: add support for Go 1.22+ net/http routing #1475

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
Merged
jamietanna merged 1 commit into from
Mar 4, 2024
Merged

feat: add support for Go 1.22+ net/http routing #1475

jamietanna merged 1 commit into from
Mar 4, 2024

Conversation

@thedadams
Copy link
Contributor

@thedadams thedadams commented Feb 25, 2024 •
edited
Loading

A couple notes on these changes:

  1. The added generated server code only works with Go 1.22+. This isn't a breaking change because there is no such generated server code now and the chi and gorilla/mux server work with Go versions < 1.22. However, it does require that the Go versions in the various testing go.mod files to be bumped. I also bumped the version in the main go.mod file, although I don't think that is required.
  2. I recognize that this WIP PR is open, but I am starting a new project and selfishly wanted this code for this new project.

thedadams
thedadams commented Feb 25, 2024
View reviewed changes
@thedadams thedadams force-pushed the http-code-gen branch 2 times, most recently from 9d4fa9c to 36b6852 Compare February 25, 2024 21:29
@jamietanna jamietanna self-requested a review February 27, 2024 22:06
@jamietanna
Copy link
Member

jamietanna commented Feb 27, 2024

Thanks for raising this! I'd prefer we keep the examples / tests as Go 1.20 (as that's the intended requirement for all uses) and use Go build tags to only build the net/http generated server when we're running 1.22+ - mind tweaking?

@thedadams
Copy link
Contributor Author

thedadams commented Feb 28, 2024

@jamietanna Thanks for your feedback! I made the adjustment as you suggested. In order to accomplish this, I also had to do the following:

  1. I added the //go:build go1.22 directive to the imports.tmpl file so that make generate would generate files containing the net/http server as expected. This directive is only added if the new net/http server is being generated. Otherwise, the make generate GitHub Action would fail because the git repo would report as dirty.
  2. Even when running the test with Go 1.22+, the newly added tests would fail because the "old" server mux implementation was being used because the go.mod files are still at 1.20. Therefore, when running make test with Go version 1.22+, the GODEBUG environment variable will now be set to httpmuxgo121=0 so that the tests will run with the "new" server mux implementation.

@jamietanna
Copy link
Member

jamietanna commented Mar 4, 2024

Thanks for the tweaks! I'm just reviewing now, and will push the changes to address this 👍

@jamietanna jamietanna mentioned this pull request Mar 4, 2024
Closed
@jamietanna jamietanna self-assigned this Mar 4, 2024
@jamietanna
Copy link
Member

jamietanna commented Mar 4, 2024

(Still WIP - got a few tweaks to get things hooked in right, and fix a couple of cases)

@jamietanna jamietanna mentioned this pull request Mar 4, 2024
Merged
@jamietanna jamietanna force-pushed the http-code-gen branch 5 times, most recently from 4a9859d to 1781f71 Compare March 4, 2024 12:19
@jamietanna
Copy link
Member

jamietanna commented Mar 4, 2024

Once #1482 is in I'll rebase, amend the commit message, then @thedadams did you want to have a look and decide if you'd like to add a Signed-off-by on the commit? I removed it, given I've made some changes that you didn't, and don't want to misrepresent what you'd signed off on!

jamietanna
jamietanna reviewed Mar 4, 2024
View reviewed changes
thedadams
thedadams commented Mar 4, 2024
View reviewed changes
@thedadams
Copy link
Contributor Author

thedadams commented Mar 4, 2024

Addressed both of the above, re-added my sign-off, and added the co-author tag (thanks for the help here @jamietanna!).

@thedadams @jamietanna
feat: add support for Go 1.22+ net/http routing
3b9748e 
As part of oapi-codegen#1068, we want to add support for the new Go 1.22+
`net/http`-only router, which will allow using `oapi-codegen` with
reduced external dependencies.

This requires we:

- wire in a new server, `std-http`
- add relevant templates for the router and strict server
- conditionally build/test/lint/etc the code when running on older
  versions of Go, which requires a bit of work in our `Makefile`
- use a separate module for the generated code, as it must set `go 1.22`
  in the `go.mod`
- document the fact that the `go.mod` needs updating, too, as it's
  caused some time to be lost in the past

Closes oapi-codegen#1068.

Co-authored-by: Jamie Tanna <[email protected]>
Signed-off-by: Donnie Adams <[email protected]>
@jamietanna jamietanna added the enhancement New feature or request label Mar 4, 2024
@jamietanna
Copy link
Member

jamietanna commented Mar 4, 2024

Thanks @thedadams 🙌 did a last tweak, don't know if you wanted to amend the commit so it's verified as your changes (with my co-authored) or if you're happy me merging as-is?

@thedadams
Copy link
Contributor Author

thedadams commented Mar 4, 2024

@jamietanna Looks good! Thank you.

@jamietanna jamietanna merged commit dd08298 into oapi-codegen:master Mar 4, 2024
@thedadams thedadams deleted the http-code-gen branch March 4, 2024 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jamietanna jamietanna jamietanna left review comments

Assignees

@jamietanna jamietanna

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@thedadams @jamietanna