From 068490167b9a1e9d1988a75305b6ff110a8fd131 Mon Sep 17 00:00:00 2001 From: Shahzad Lone Date: Thu, 16 Feb 2023 01:29:27 +0500 Subject: [PATCH] chore: Bump to GoLang v1.19 (#818) - Resolves #817 - Resolves #1070 - This PR was initially started as: `ci: Bump workflow's golang-ci lint version` to fix the golang-ci lint issue. This work that was done in this PR was the following: - Remove the `deadcode` and `varcheck` linters that are now deprecated with `v1.49` - Remove all `nolint` directives where the deprecated linters were being referenced. - Update CI's linter version to use the latest version. However as the golang-ci lint team still didn't fix that issue but golang1.19 was released we can just fix those errors and bump the GoLang version for defradb and keeping the previous linter fixes. Inaddtion also re-enabling the `goheader` linter rule as the change was somehow dropped when we released defradb v0.4. --- .github/workflows/build-dependencies.yml | 2 +- .github/workflows/code-test-coverage.yml | 2 +- .github/workflows/detect-change.yml | 2 +- .github/workflows/lint-then-benchmark.yml | 4 +- .github/workflows/run-tests.yml | 2 +- .github/workflows/start-binary.yml | 2 +- Makefile | 4 +- config/config.go | 1 - db/collection.go | 10 ++--- db/collection_delete.go | 38 ++++++++++------- db/fetcher/versioned.go | 12 +++--- errors/errors.go | 4 ++ go.mod | 2 +- net/pb/custom.go | 2 +- planner/explain.go | 50 ++++++++++++----------- planner/group.go | 7 ++-- planner/multi.go | 22 +++++----- planner/planner.go | 2 +- request/graphql/schema/descriptions.go | 2 +- tests/bench/query/simple/utils.go | 2 +- tests/integration/utils.go | 40 ++++++------------ tools/configs/golangci.yaml | 19 +++------ tools/defradb-builder.containerfile | 2 +- tools/defradb.containerfile | 4 +- 24 files changed, 114 insertions(+), 123 deletions(-) diff --git a/.github/workflows/build-dependencies.yml b/.github/workflows/build-dependencies.yml index d48faaaac8..4eeb238726 100644 --- a/.github/workflows/build-dependencies.yml +++ b/.github/workflows/build-dependencies.yml @@ -34,7 +34,7 @@ jobs: - name: Setup Go environment explicitly uses: actions/setup-go@v3 with: - go-version: "1.18" + go-version: "1.19" check-latest: true - name: Build all dependencies diff --git a/.github/workflows/code-test-coverage.yml b/.github/workflows/code-test-coverage.yml index 8b8770e1ad..57361b4042 100644 --- a/.github/workflows/code-test-coverage.yml +++ b/.github/workflows/code-test-coverage.yml @@ -27,7 +27,7 @@ jobs: - name: Setup Go uses: actions/setup-go@v3 with: - go-version: "1.18" + go-version: "1.19" check-latest: true - name: Generate full test coverage report using go-acc diff --git a/.github/workflows/detect-change.yml b/.github/workflows/detect-change.yml index 6ba3e4aa44..6dfba9a27b 100644 --- a/.github/workflows/detect-change.yml +++ b/.github/workflows/detect-change.yml @@ -33,7 +33,7 @@ jobs: - name: Setup Go environment explicitly uses: actions/setup-go@v3 with: - go-version: "1.18" + go-version: "1.19" check-latest: true - name: Build dependencies diff --git a/.github/workflows/lint-then-benchmark.yml b/.github/workflows/lint-then-benchmark.yml index 043b46ac1d..b63aa14022 100644 --- a/.github/workflows/lint-then-benchmark.yml +++ b/.github/workflows/lint-then-benchmark.yml @@ -57,7 +57,7 @@ jobs: - name: Setup Go environment explicitly uses: actions/setup-go@v3 with: - go-version: "1.18" + go-version: "1.19" check-latest: true - name: Run the golangci-lint @@ -67,7 +67,7 @@ jobs: # Required: the version of golangci-lint is required. # Note: The version should not pick the patch version as the latest patch # version is what will always be used. - version: v1.47 + version: v1.51 # Optional: working directory, useful for monorepos or if we wanted to run this # on a non-root directory. diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index ba3bd7e27a..c27384fa09 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -25,7 +25,7 @@ jobs: - name: Setup Go environment explicitly uses: actions/setup-go@v3 with: - go-version: "1.18" + go-version: "1.19" check-latest: true - name: Build dependencies diff --git a/.github/workflows/start-binary.yml b/.github/workflows/start-binary.yml index 4b23434f10..97db056df9 100644 --- a/.github/workflows/start-binary.yml +++ b/.github/workflows/start-binary.yml @@ -34,7 +34,7 @@ jobs: - name: Setup Go environment explicitly uses: actions/setup-go@v3 with: - go-version: "1.18" + go-version: "1.19" check-latest: true - name: Build modules diff --git a/Makefile b/Makefile index a0f67281ee..36f2163c09 100644 --- a/Makefile +++ b/Makefile @@ -65,7 +65,7 @@ client\:add-schema: .PHONY: deps\:lint deps\:lint: - go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.47.3 + go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.51 .PHONY: deps\:test deps\:test: @@ -118,7 +118,7 @@ verify: .PHONY: tidy tidy: - go mod tidy -go=1.18 + go mod tidy -go=1.19 .PHONY: clean clean: diff --git a/config/config.go b/config/config.go index d8c0a5ea6d..820e1546e6 100644 --- a/config/config.go +++ b/config/config.go @@ -39,7 +39,6 @@ How to use, e.g. without using a rootdir: err := cfg.LoadWithoutRootDir() if err != nil { ... - */ package config diff --git a/db/collection.go b/db/collection.go index d1c2b9a169..6928fd5102 100644 --- a/db/collection.go +++ b/db/collection.go @@ -886,11 +886,11 @@ func (c *collection) getTxn(ctx context.Context, readonly bool) (datastore.Txn, } // discardImplicitTxn is a proxy function used by the collection to execute the Discard() -// transaction function only if its an implicit transaction. -// Implicit transactions are transactions that are created *during* an operation -// execution as a side effect. -// Explicit transactions are provided to the collection object via -// the "WithTxn(...)" function. +// transaction function only if its an implicit transaction. +// +// Implicit transactions are transactions that are created *during* an operation execution as a side effect. +// +// Explicit transactions are provided to the collection object via the "WithTxn(...)" function. func (c *collection) discardImplicitTxn(ctx context.Context, txn datastore.Txn) { if c.txn == nil { txn.Discard(ctx) diff --git a/db/collection_delete.go b/db/collection_delete.go index dbe6e1be4a..0f65832ab7 100644 --- a/db/collection_delete.go +++ b/db/collection_delete.go @@ -29,8 +29,11 @@ import ( "github.com/sourcenetwork/defradb/merkle/clock" ) -// DeleteWith deletes a target document. Target can be a Filter statement, -// a single docKey, a single document, an array of docKeys, or an array of documents. +// DeleteWith deletes a target document. +// +// Target can be a Filter statement, a single docKey, a single document, +// an array of docKeys, or an array of documents. +// // If you want more type safety, use the respective typed versions of Delete. // Eg: DeleteWithFilter or DeleteWithKey func (c *collection) DeleteWith( @@ -250,16 +253,20 @@ func newDagDeleter(bstore datastore.DAGStore) dagDeleter { } } +// applyFullDelete deletes from all the stores. +// +// Stores the deletion will act upon: +// 1. Deleting the actual blocks (blockstore). +// 2. Deleting datastore state. +// 3. Deleting headstore state. +// // Here is what our db stores look like: -// /db -// -> block /blocks => /db/blocks -// -> datastore /data => /db/data -// -> headstore /heads => /db/heads -// -> systemstore /system => /db/system -// For the delete operation we are concerned with: -// 1) Deleting the actual blocks (blockstore). -// 2) Deleting datastore state. -// 3) Deleting headstore state. +// +// /db +// -> block /blocks => /db/blocks +// -> datastore /data => /db/data +// -> headstore /heads => /db/heads +// -> systemstore /system => /db/system func (c *collection) applyFullDelete( ctx context.Context, txn datastore.Txn, dockey core.PrimaryDataStoreKey) error { @@ -353,10 +360,11 @@ func (d dagDeleter) run(ctx context.Context, targetCid cid.Cid) error { return d.delete(ctx, targetCid, block) } -// (ipld.Block -// (ipldProtobufNode{ -// Data: (cbor(crdt deltaPayload)), -// Links: (_head => parentCid, fieldName => fieldCid))) +// (ipld.Block +// (ipldProtobufNode{ +// Data: (cbor(crdt deltaPayload)), +// Links: (_head => parentCid, fieldName => fieldCid))) + func (d dagDeleter) delete( ctx context.Context, targetCid cid.Cid, diff --git a/db/fetcher/versioned.go b/db/fetcher/versioned.go index 5637913082..7d7197c3a3 100644 --- a/db/fetcher/versioned.go +++ b/db/fetcher/versioned.go @@ -38,13 +38,12 @@ var ( // to a specific version in the documents history graph, and return the fetched // state at that point exactly. // -// Given the following Document state graph -// +// Given the following Document state graph: // {} --> V1 --> V2 --> V3 --> V4 +// // ^ ^ // | | -// Target Version Current State -// +// Target Version Current State // // A regular DocumentFetcher fetches and returns the state at V4, but the // VersionsedFetcher would step backwards through the update graph, recompose @@ -64,10 +63,9 @@ var ( // the scanNode request planner system. // // Current limitations: -// - We can only return a single record from an VersionedFetcher -// instance. +// - We can only return a single record from an VersionedFetcher instance. // - We can't request related sub objects (at the moment, as related objects -// ids aren't in the state graphs. +// ids aren't in the state graphs. // - Probably more... // // Future optimizations: diff --git a/errors/errors.go b/errors/errors.go index fc78d9836e..45c1202e77 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -45,6 +45,7 @@ func NewKV(key string, value any) KV { // A stacktrace will be yielded if formatting with a `+`, e.g `fmt.Sprintf("%+v", err)`. // This function will not be inlined by the compiler as it will spoil any stacktrace // generated. +// //go:noinline func New(message string, keyvals ...KV) error { return withStackTrace(message, 1, keyvals...) @@ -54,6 +55,7 @@ func New(message string, keyvals ...KV) error { // the given inner error, suffixing any key-value pairs provided. // This function will not be inlined by the compiler as it will spoil any stacktrace // generated. +// //go:noinline func Wrap(message string, inner error, keyvals ...KV) error { err := withStackTrace(message, 1, keyvals...) @@ -67,6 +69,7 @@ func Is(err, target error) bool { // This function will not be inlined by the compiler as it will spoil any stacktrace // generated. +// //go:noinline func WithStack(err error, keyvals ...KV) error { return withStackTrace(err.Error(), 1, keyvals...) @@ -79,6 +82,7 @@ func WithStack(err error, keyvals ...KV) error { // // This function will not be inlined by the compiler as it will spoil any stacktrace // generated. +// //go:noinline func withStackTrace(message string, depthToSkip int, keyvals ...KV) *defraError { stackBuffer := make([]uintptr, MaxStackDepth) diff --git a/go.mod b/go.mod index 6c397d1440..c894a40f82 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/sourcenetwork/defradb -go 1.18 +go 1.19 require ( github.com/bxcodec/faker v2.0.1+incompatible diff --git a/net/pb/custom.go b/net/pb/custom.go index 71e00c8ac4..c71f585fd4 100644 --- a/net/pb/custom.go +++ b/net/pb/custom.go @@ -77,7 +77,7 @@ func (id ProtoPeerID) Size() int { } // ProtoAddr is a custom type used by gogo to serde raw multiaddresses into -// the ma.Multiaddr type, and back. +// the ma.Multiaddr type, and back. type ProtoAddr struct { ma.Multiaddr } diff --git a/planner/explain.go b/planner/explain.go index 7e94ae2c8a..37f0500724 100644 --- a/planner/explain.go +++ b/planner/explain.go @@ -55,31 +55,33 @@ const ( // buildSimpleExplainGraph builds the explainGraph from the given top level plan. // // Request: -// query @explain { -// user { -// _key -// age -// name -// } -// } // -// Response: -// { -// "data": [ -// { -// "explain": { -// "selectTopNode": { -// "selectNode": { -// ... -// "scanNode": { -// ... -// } -// } -// } -// } -// } -// ] -// } +// query @explain { +// user { +// _key +// age +// name +// } +// } +// +// Response: +// +// { +// "data": [ +// { +// "explain": { +// "selectTopNode": { +// "selectNode": { +// ... +// "scanNode": { +// ... +// } +// } +// } +// } +// } +// ] +// } func buildSimpleExplainGraph(source planNode) (map[string]any, error) { explainGraph := map[string]any{} diff --git a/planner/group.go b/planner/group.go index cb58c0eace..9a6b19da86 100644 --- a/planner/group.go +++ b/planner/group.go @@ -38,9 +38,10 @@ type groupNode struct { currentIndex int } -// Creates a new group node. The function is recursive and will construct the node-chain for any -// child (`_group`) collections. `groupSelect` is optional and will typically be nil if the -// child `_group` is not requested. +// Creates a new group node. + +// The function is recursive and will construct the node-chain for any child (`_group`) collections. +// `groupSelect` is optional and will typically be nil if the child `_group` is not requested. func (p *Planner) GroupBy(n *mapper.GroupBy, parsed *mapper.Select, childSelects []*mapper.Select) (*groupNode, error) { if n == nil { return nil, nil diff --git a/planner/multi.go b/planner/multi.go index e898b8a705..c158b818bf 100644 --- a/planner/multi.go +++ b/planner/multi.go @@ -54,22 +54,22 @@ type appendNode interface { // if a single request has multiple Select statements at the // same depth in the request. // Eg: -// user { -// _key -// name -// friends { -// name -// } -// _version { -// cid -// } -// } +// +// user { +// _key +// name +// friends { +// name +// } +// _version { +// cid +// } +// } // // In this example, both the friends selection and the _version // selection require their own planNode sub graphs to complete. // However, they are entirely independent graphs, so they can // be executed in parallel. -// type parallelNode struct { // serialNode? documentIterator docMapper diff --git a/planner/planner.go b/planner/planner.go index 4ada413302..079071874a 100644 --- a/planner/planner.go +++ b/planner/planner.go @@ -439,7 +439,7 @@ func walkAndFindPlanType[T planNode](plan planNode) (T, bool) { } // explainRequest walks through the plan graph, and outputs the concrete planNodes that should -// be executed, maintaing their order in the plan graph (does not actually execute them). +// be executed, maintaing their order in the plan graph (does not actually execute them). func (p *Planner) explainRequest( ctx context.Context, plan planNode, diff --git a/request/graphql/schema/descriptions.go b/request/graphql/schema/descriptions.go index f88b8d5136..0f18e31e5d 100644 --- a/request/graphql/schema/descriptions.go +++ b/request/graphql/schema/descriptions.go @@ -26,7 +26,7 @@ var ( // directly. As it will yield incorrect and unexpected // results - //nolint:deadcode,unused,varcheck + //nolint:unused gqlTypeToFieldKindReference = map[gql.Type]client.FieldKind{ gql.ID: client.FieldKind_DocKey, gql.Boolean: client.FieldKind_BOOL, diff --git a/tests/bench/query/simple/utils.go b/tests/bench/query/simple/utils.go index edc8109909..352383e9f2 100644 --- a/tests/bench/query/simple/utils.go +++ b/tests/bench/query/simple/utils.go @@ -24,7 +24,7 @@ import ( ) var ( -//log = logging.MustNewLogger("defra.bench") +// log = logging.MustNewLogger("defra.bench") ) func runQueryBenchGet( diff --git a/tests/integration/utils.go b/tests/integration/utils.go index e51c1c062b..a030ba46f8 100644 --- a/tests/integration/utils.go +++ b/tests/integration/utils.go @@ -19,7 +19,6 @@ import ( "path" "runtime" "strings" - "syscall" "testing" "time" @@ -50,19 +49,6 @@ const ( documentationDirectoryName = "data_format_changes" ) -// The integration tests open many files. This increases the limits on the number of open files of -// the process to fix this issue. This is done by default in Go 1.19. -func init() { - var lim syscall.Rlimit - if err := syscall.Getrlimit(syscall.RLIMIT_NOFILE, &lim); err == nil && lim.Cur != lim.Max { - lim.Cur = lim.Max - err = syscall.Setrlimit(syscall.RLIMIT_NOFILE, &lim) - if err != nil { - log.ErrorE(context.Background(), "error setting rlimit", err) - } - } -} - var ( log = logging.MustNewLogger("defra.tests.integration") badgerInMemory bool @@ -148,19 +134,19 @@ If this is set to true the integration test suite will instead of it's normal pr the following: On [package] Init: -- Get the (local) latest commit from the target/parent branch // code assumes - git fetch has been done -- Check to see if a clone of that commit/branch is available in the temp dir, and - if not clone the target branch -- Check to see if there are any new .md files in the current branch's data_format_changes - dir (vs the target branch) + - Get the (local) latest commit from the target/parent branch // code assumes + git fetch has been done + - Check to see if a clone of that commit/branch is available in the temp dir, and + if not clone the target branch + - Check to see if there are any new .md files in the current branch's data_format_changes + dir (vs the target branch) For each test: -- If new documentation detected, pass the test and exit -- Create a new (test/auto-deleted) temp dir for defra to live/run in -- Run the test setup (add initial schema, docs, updates) using the target branch (test is skipped - if test does not exist in target and is new to this branch) -- Run the test request and assert results (as per normal tests) using the current branch + - If new documentation detected, pass the test and exit + - Create a new (test/auto-deleted) temp dir for defra to live/run in + - Run the test setup (add initial schema, docs, updates) using the target branch (test is skipped + if test does not exist in target and is new to this branch) + - Run the test request and assert results (as per normal tests) using the current branch */ var DetectDbChanges bool var SetupOnly bool @@ -222,9 +208,9 @@ func IsDetectingDbChanges() bool { } // AssertPanicAndSkipChangeDetection asserts that the code of function actually panics, -// also ensures the change detection is skipped so no false fails happen. +// also ensures the change detection is skipped so no false fails happen. // -// Usage: AssertPanicAndSkipChangeDetection(t, func() { executeTestCase(t, test) }) +// Usage: AssertPanicAndSkipChangeDetection(t, func() { executeTestCase(t, test) }) func AssertPanicAndSkipChangeDetection(t *testing.T, f assert.PanicTestFunc) bool { if IsDetectingDbChanges() { // The `assert.Panics` call will falsely fail if this test is executed during diff --git a/tools/configs/golangci.yaml b/tools/configs/golangci.yaml index 9ac6068d9a..02c5c1d694 100644 --- a/tools/configs/golangci.yaml +++ b/tools/configs/golangci.yaml @@ -56,9 +56,8 @@ run: allow-parallel-runners: false # Define the Go version limit. - # Mainly related to generics support since go1.18. - # Default: use Go version from the go.mod file, fallback on the env var `GOVERSION`, fallback on 1.18 - go: "1.18" + # Default: use Go version from the go.mod file, fallback on the env var `GOVERSION`. + go: "1.19" #=====================================================================================[ Output Configuration Options ] output: @@ -112,14 +111,13 @@ linters: disable-all: true enable: - - deadcode - errcheck - errorlint - exportloopref - forbidigo - goconst - gofmt - # - goheader + - goheader - goimports - gosimple - govet @@ -130,13 +128,8 @@ linters: - staticcheck - typecheck - unused - - varcheck - whitespace - # https://github.com/golangci/golangci-lint/issues/2649 - # Linters below are disabled for now because of go1.18 issue above^ - # - structcheck - #====================================================[ Tweaks To Fix Issues or Exclude linter(s) on Select Locations ] issues: # List of regexps of issue texts to exclude, empty list by default. @@ -270,7 +263,7 @@ linters-settings: gosimple: # Select the Go version to target. - go: "1.18" + go: "1.19" # https://staticcheck.io/docs/options#checks checks: ["all", "-S1038"] # Turn on all except (these are disabled): @@ -362,13 +355,13 @@ linters-settings: staticcheck: # Select the Go version to target. - go: "1.18" + go: "1.19" # https://staticcheck.io/docs/options#checks checks: ["all"] unused: # Select the Go version to target. - go: "1.18" + go: "1.19" whitespace: # Enforces newlines (or comments) after every multi-line if statement. diff --git a/tools/defradb-builder.containerfile b/tools/defradb-builder.containerfile index ad2da36782..3daadb9b61 100644 --- a/tools/defradb-builder.containerfile +++ b/tools/defradb-builder.containerfile @@ -2,7 +2,7 @@ # An image with defradb's code and go tooling available, to assemble in a larger container. -FROM docker.io/golang:1.18 AS BUILD +FROM docker.io/golang:1.19 AS BUILD WORKDIR /lib/defradb/ diff --git a/tools/defradb.containerfile b/tools/defradb.containerfile index f9f959f110..77b5bc9bb1 100644 --- a/tools/defradb.containerfile +++ b/tools/defradb.containerfile @@ -4,7 +4,7 @@ # Stage: BUILD # Several steps are involved to enable caching and because of the behavior of COPY regarding directories. -FROM docker.io/golang:1.18 AS BUILD +FROM docker.io/golang:1.19 AS BUILD WORKDIR /repo/ COPY go.mod go.sum Makefile ./ RUN make deps:modules @@ -23,4 +23,4 @@ EXPOSE 9181 # Default command provided for convenience. # e.g. docker run -p 9181:9181 source/defradb start --url 0.0.0.0:9181 -ENTRYPOINT [ "/defradb" ] \ No newline at end of file +ENTRYPOINT [ "/defradb" ]