Skip to content

WIP: Use pure go sqlite driver instead of cgo go-sqlite3 and remove go build tags related with sqlite #15002

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.

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

Closed
wants to merge 1 commit into from

Conversation

lunny
Copy link
Member

@lunny lunny commented Mar 15, 2021

This PR tries to use pure go sqlite driver instead of cgo go-sqlite3 for better compile time.

@lunny lunny added the pr/wip This PR is not ready for review label Mar 15, 2021
@stale
Copy link

stale bot commented Jul 21, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions.

@stale stale bot added the issue/stale label Jul 21, 2021
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 21, 2021
@stale stale bot removed the issue/stale label Jul 21, 2021
@lunny lunny added the issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented label Jul 21, 2021
Copy link
Member

@delvh delvh left a comment

Choose a reason for hiding this comment

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

I have two questions:

  1. Why does this library bring its own C library?
  2. How does adding far more code improve compile time?

@@ -69,7 +69,7 @@ func initDBEngine(ctx context.Context) (err error) {
for i := 0; i < setting.Database.DBConnectRetries; i++ {
select {
case <-ctx.Done():
return fmt.Errorf("Aborted due to shutdown:\nin retry ORM engine initialization")
return fmt.Errorf("aborted due to shutdown:\nin retry ORM engine initialization")

This comment was marked as resolved.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's GO styling. GO standard says: every error message starts with lower-case. Please read the standards.

This comment was marked as resolved.

Copy link
Contributor

Choose a reason for hiding this comment

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

@lunny
Copy link
Member Author

lunny commented Nov 1, 2021

I have two questions:

  1. Why does this library bring its own C library?
  2. How does adding far more code improve compile time?
  1. I think since the pure go sqlite library was translated from original sqlite, so the author also translate them. Maybe he will change them at some point.
  2. This is a CGO problem. When compile with CGO, the Go compiler will spend more time. But when compile a pure Go project which even has more source files, it's faster.

@wxiaoguang
Copy link
Contributor

After reading the code of modernc.org/sqlite, I'd like to say, it's surprisingly interesting and extremely impressive (well, I can not find other words to describe it because I only know a few words to describe an awesome thing ...)

The modernc/sqlite team wrote a C99 compiler frontend in Go, and wrote a lot of cross-platform code for their Go-based libc, then they compile the sqlite official code by their C compiler using their libc into Go, and embed it into the modernc/sqlite ....

ps: I haven't done any test and I am not sure how much time could be save by this Go-sqlite during Gitea compiling, just leave a comment about how it works.

@lunny
Copy link
Member Author

lunny commented May 26, 2022

After reading the code of modernc.org/sqlite, I'd like to say, it's surprisingly interesting and extremely impressive (well, I can not find other words to describe it because I only know a few words to describe an awesome thing ...)

The modernc/sqlite team wrote a C99 compiler frontend in Go, and wrote a lot of cross-platform code for their Go-based libc, then they compile the sqlite official code by their C compiler using their libc into Go, and embed it into the modernc/sqlite ....

ps: I haven't done any test and I am not sure how much time could be save by this Go-sqlite during Gitea compiling, just leave a comment about how it works.

I think much time and memory in developer's machine and CI agents will be saved when CGO is gone. Because we can drop the xgo to do cross-compile. When using xgo, you have to check or download a very big image(about 5GB) and spent many time to compile because c++ compile is slower than pure Go.

@jolheiser jolheiser mentioned this pull request Aug 2, 2022
@lunny lunny closed this Aug 2, 2022
@lunny lunny deleted the lunny/sqlite branch August 2, 2022 06:19
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. pr/wip This PR is not ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants