Increase maximum SQLite variables count to 32766

[CirnoT]

Increase limit of variables in a query for SQLite to 32766 via CGO_CFLAGS SQLITE_MAX_VARIABLE_NUMBER

This puts our SQLite in same state as versions 3.32.0+ (since 2020-05-22)

Fixes #10690
Fixes #11236
Fixes #7750

Closes #11664

---

Ref. https://www.sqlite.org/limits.html

the maximum value of a host parameter number is SQLITE_MAX_VARIABLE_NUMBER, which defaults to 999 for SQLite versions prior to 3.32.0 (2020-05-22) or 32766 for SQLite versions after 3.32.0.

Ref. https://github.com/mattn/go-sqlite3#compilation

If you need to add additional CFLAGS or LDFLAGS to the build command, and do not want to modify this package. Then this can be achieved by using the CGO_CFLAGS and CGO_LDFLAGS environment variables.

Ref. https://raw.githubusercontent.com/mattn/go-sqlite3/master/sqlite3-binding.c

/*
** The maximum value of a ?nnn wildcard that the parser will accept.
*/
#ifndef SQLITE_MAX_VARIABLE_NUMBER
# define SQLITE_MAX_VARIABLE_NUMBER 999
#endif

[CirnoT]

This workaround won't be required anymore after https://github.com/mattn/go-sqlite3 is updated to SQLite3 3.32.0 (currently at 3.28.0) or newer, however this PR is backportable into 1.12.

[CirnoT]

This PR also removes maxIssueIDs hack which is undocumented limit of search results and was introduced in #9758 to accommodate SQLite.

[CirnoT]

Wondering if this overrides default CGO_CFLAGS="-g -O2"

[zeripath]

Good call on running about the default cgo flags - I think that this could well override them but we'd need to check the documentation for cgo ... However, it certainly overrides any user provided flags so we would need to use the makefile only set if not already set notation for it.

[CirnoT]

Updated it to behave same as LDFLAGS

[CirnoT]

but we'd need to check the documentation for cgo

Sadly nothing interesting there.

When building, the CGO_CFLAGS, CGO_CPPFLAGS, CGO_CXXFLAGS, CGO_FFLAGS and CGO_LDFLAGS environment variables are added to the flags derived from these directives. Package-specific flags should be set using the directives, not the environment variables, so that builds work in unmodified environments. Flags obtained from environment variables are not subject to the security limitations described above.

Need to test it with enabled verbose to see what commands are passed exactly.

[CirnoT]

Confirmed that it removes default -g -O2 from gcc commands...

[silverwind]

You can pull in the defaults from go env CGO_CFLAGS in a subshell. It's a bit clumsy though:

$(GO) build CGO_CFLAGS="-DSQLITE_MAX_VARIABLE_NUMBER=32766 $(shell $(GO) env CGO_CFLAGS) $(CGO_CFLAGS)"

I would do this only in the targets that need it because otherwise you'd be adding +150ms to unrelated make targets too.

Edit: updated to use shell

[silverwind]

Another alternative:

CGO_CFLAGS ?= $(shell $(GO) env CGO_CFLAGS) -DSQLITE_MAX_VARIABLE_NUMBER=32766

?= makes it overrideable from the environment. I guess one more go env call is probably fine overall.

[6543]

well at some point if you have that mouch issues & repos ... you should move away from sqlite in general ...

[CirnoT]

@silverwind That is fine for Makefile but we also need something for Dockerfile

[silverwind]

Something like this:

RUN if [ -n "${GITEA_VERSION}" ]; then git checkout "${GITEA_VERSION}"; fi && \
 export CGO_CFLAGS="$(go env CGO_CFLAGS) -DSQLITE_MAX_VARIABLE_NUMBER=32766" && \
 make clean-all build

I guess we don't want to inherit CGO_CFLAGS from the host system during the docker build because those defaults could be platform-specific (e.g. ARM).

[silverwind]

I'd actually call it CGO_EXTRA_CFLAGS and then inherit those into it Dockerfile so we only have to define them once.

[CirnoT]

I'd actually call it CGO_EXTRA_CFLAGS and then inherit those into it Dockerfile so we only have to define them once.

You assume docker is being built from makefile, but nothing forces you to do that; you can just docker build . -t gitea

[techknowlogick]

Dockerfile uses makefile https://github.com/go-gitea/gitea/blob/master/Dockerfile#L22 so these lines https://github.com/go-gitea/gitea/pull/11696/files#diff-b67911656ef5d18c4ae36cb6741b7965R38 would be used. You can confirm this behaviour by reviewing the CI run https://drone.gitea.io/go-gitea/gitea/26552/5/2 (specifically L457, but I am unable to link to a specific line in a build in drone)

[silverwind]

Isn't this $CGO_EXTRA_CFLAGS duplicating with the variable before? Should probably remove it or fix inheritance from the main makefile which seems to not work based on @techknowlogick's link above.

[techknowlogick]

Indeed, let me revert the changes to dockerfile.

[techknowlogick]

I've pushed changes, and will review CI output before merge.

[techknowlogick]

ping LG-TM

[techknowlogick]

@CirnoT please send backport :)