Dyn port svc dispatch #4380
Closed
Dyn port svc dispatch #4380
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Many packages that are explicitly installed in the debian case were left implicit in the rhel case (in the assumption that they were part of the Software Development package group). While at it, separated the command from the list of packages as is doen for debian. The pip3 package was used before there is a chance to install it. Since pip3/deps isn't needed by the rest of the install scripts. Do it last.
. go.mod is a real file, so we do not need a separate phony target name. . since go.mod is a real file, we can have go-deps.bzl depend on it, no need to make it explicitly. . make gazelle depend on go_deps.bzl so there's no need to make go_deps.bzl explicitly either,
Added the modernc go sqlite implementation and made it the default. The reason is that the mattn implementation links statically to C code, which creates a dependency on the glibc version of the build host. That version can be incompatible with the one contained in the test containers, causing the integraton tests to fail. There are other ways to fix the problem in general: * Full cross-build for the specific test distribution. * Building the test containers to match the build host. * Both (ie. build and test for different platforms, including the build host's) Each of those have different merits and require a lot of work. In the meantime removing that dependency puts the issue out of the critical path and lowers complexity. The mattn sqlite implementation is retained as an option. To make it easy to switch back-and-forth between implementations, all direct references to the sqlite implementation are confined to exactly one go file per implementation: private/storage/db/sqlite_<impl>.go. To switch between them, just edit the srcs attribute in private/storage/db/BUILD.bazel. To make this possible, I had to prevent gazelle from updating that target. If you know of a better way, chime in.
The build option is now compatible with go build. To build with mattn sqlite, pass the flag "-tag sqlite_mattn". To build with modernc sqlite, pass the flag "-tag sqlite_modernc". When building via Bazel, that flag is injected via .blazerc with the line: "common --show_timestamps --define gotags=sqlite_modernc" Gazelle also needs to be supplied with that tag. For gazelle gotags must be passed as: "-build_tags tag1,tag2,..." This is accomplished by adding a pair of config_setting() calls that translate the gotags define into a setting tag and by using a select keyed on that to pass the appropriate flag to Gazelle. Gazelle update-repos does not need to know about the go tag. On the Makefile side, Gazelle is now called with "-args" to pass those arguments that aare variables in the Makefile, allowing the others to be chosen and appended by Blaze rules. It was noticed that make licenses implies a full build and therefore should be done last. (At least not before gazelle.)
… sqlite implementation drivers. The drivers are now reduced to two functions; one to set the pragmas in the connection uri and the other to return the implementation's registration name.
…od is the target.
Passing the build tags to the linter causes a bit of complication. To make sure that we have the tags specified in only one place and that bazel can be invoked without using Make, Make has to obtain them from bazel.
… are inherited from build already.
…n the same package only.
sqlite.go is now common to all sqlite implementations it hould not import any specific one.
Ooooo Ummmmmm. buildkit adds a build flag that the linter can't process. Made sure we only add the go build tags to the linter flags.
--noansi has been deprecated 2 years ago and I'm getting warnings. This could have stayed out of the changes I am currently working on, but since the pull-request is stuck two CLs back, I am hoping that adding one change will un-stick it.
(Sigh...) sometimes there's no option after the gotags, so matching the space that may follow is a bad idea and not needed.
I had missed that the ci tests add "build --nostamp" into ~/.bazelrc which causes two "'build' options:" lines to be output by "bazel --anounce_rc" one of the lines does not match the regex so isn't edited but still output. Fixed it by not outputing anything by default (-n) and by addint /p to the substitution sed command. That way we really output only build-tags.
…ead. That's not ideal since whoever builds the code is free to edit .bazelrc and chose sqlite_mattn instead (which would cause the license file to re-appear. However, I have no good idea for a better solution and it can be solved later, rather than continuing to block this PR.
Yet another invocation of "go" outside the control of bazel.
. Use dashes for lists in doc. . Mention the new required go build tag "sqlite_mattn"/"sqlite_modernc" for folks who use go build. . Unexport the build guard constant and refer to it with an anoymous var. The goal here is to make any build tag mistake easier to understand from the compiler's error message.
To that end: * Change the go:build directive in sqlite_modernc to match the absence of the sqlite_mattn tag. * Retire the old +build syntax which is outdated and makes logical expressions error prone. * Add a warning about the limitations of the approach.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #4379
This change is