Skip to content

revive complains about missing package comment since golangci-lint 1.44.1 #2610

Closed
@horgh

Description

@horgh

Welcome

  • Yes, I'm using a binary release within 2 latest major releases. Only such installations are supported.
  • Yes, I've searched similar issues on GitHub and didn't find any.
  • Yes, I've included all information below (version, config, etc).
  • Yes, I've tried with the standalone linter if available. (https://golangci-lint.run/usage/linters/)

Description of the problem

I am seeing errors like this since updating to golangci-lint 1.44.2. I tested 1.44.1 and the issue is there as well. 1.44.0 is fine. I also tried running revive 1.1.4 standalone and I could not reproduce it.

pkg/geoipupdate/version.go:1:1: package-comments: should have a package comment, unless it's in another file for this package (revive)
package geoipupdate

// Version defines current geoipupdate version.
const Version = "4.9.0"

For example in this test run in my project: https://github.com/maxmind/geoipupdate/runs/5292690662?check_suite_focus=true

The packages in question have a package comment in another file in the package.

I also noticed it complained that main did not have a package comment, which doesn't seem necessary either, though seems less of an issue.

Version of golangci-lint

wstorey@penguin:~/geoipupdate (horgh/lint)$ golangci-lint version
golangci-lint has version 1.44.2 built from d58dbde5 on 2022-02-17T20:58:06Z

Configuration file

[run]
  deadline = "10m"

  tests = true

[linters]
  disable-all = true
  enable = [
    "asciicheck",
    "bidichk",
    "bodyclose",
    "containedctx",
    "contextcheck",
    "deadcode",
    "depguard",
    "durationcheck",
    "errcheck",
    # Seems to cause stack overflows :-(
    # "errchkjson",
    "errname",
    "errorlint",
    "exhaustive",
    "exportloopref",
    "forbidigo",
    # We tried this liner but most places we do forced type asserts are
    # pretty safe, e.g., an atomic.Value when everything is encapsulated
    # in a small package.
    # "forcetypeassert",
    "goconst",
    "gocyclo",
    "gocritic",
    "godot",
    "gofumpt",
    "gomodguard",
    "gosec",
    "gosimple",
    "govet",
    "grouper",
    "ineffassign",
    "lll",
    "makezero",
    # Maintainability Index. Seems like it could be a good idea, but a
    # lot of things fail and we would need to make some decisions about
    # what to allow.
    # "maintidx",
    "misspell",
    "nakedret",
    "nilerr",
    "noctx",
    "nolintlint",
    "predeclared",
    "revive",
    "rowserrcheck",
    # https://github.com/golangci/golangci-lint/issues/287
    # "safesql",
    "sqlclosecheck",
    "staticcheck",
    "structcheck",
    "stylecheck",
    "tenv",
    "tparallel",
    "typecheck",
    "unconvert",
    "unparam",
    "unused",
    "varcheck",
    "vetshadow",
    "wastedassign",
    "wrapcheck",
  ]

# Please note that we only use depguard for stdlib as gomodguard only
# supports modules currently. See https://github.com/ryancurrah/gomodguard/issues/12
[linters-settings.depguard]
  list-type = "blacklist"
  include-go-root = true
  packages = [
    # We should use github.com/pkg/errors instead
    "errors",
  ]

[linters-settings.errcheck]
    # Ignoring Close so that we don't have to have a bunch of
    # `defer func() { _ = r.Close() }()` constructs when we
    # don't actually care about the error.
    ignore = "Close,fmt:.*"

[linters-settings.errorlint]
    errorf = true
    asserts = true
    comparison = true

[linters-settings.exhaustive]
    default-signifies-exhaustive = true

[linters-settings.gocritic]
    enabled-checks = [
        "appendAssign",
        "appendCombine",
        "argOrder",
        "assignOp",
        "badCall",
        "badCond",
        "badLock",
        "badRegexp",
        "badSorting",
        "boolExprSimplify",
        "builtinShadow",
        "builtinShadowDecl",
        "captLocal",
        "caseOrder",
        "codegenComment",
        "commentedOutCode",
        "commentedOutImport",
        "commentFormatting",
        "defaultCaseOrder",
        # Revive's defer rule already captures this. This caught no extra cases.
        # "deferInLoop",
        "deferUnlambda",
        "deprecatedComment",
        "docStub",
        "dupArg",
        "dupBranchBody",
        "dupCase",
        "dupImport",
        "dupSubExpr",
        "dynamicFmtString",
        "elseif",
        "emptyDecl",
        "emptyFallthrough",
        "emptyStringTest",
        "equalFold",
        "evalOrder",
        "exitAfterDefer",
        "exposedSyncMutex",
        "externalErrorReassign",
        # Given that all of our code runs on Linux and the / separate should
        # work fine, this seems less important.
        # "filepathJoin",
        "flagDeref",
        "flagName",
        "hexLiteral",
        "ifElseChain",
        "importShadow",
        "indexAlloc",
        "initClause",
        "mapKey",
        "methodExprCall",
        "nestingReduce",
        "newDeref",
        "nilValReturn",
        "octalLiteral",
        "offBy1",
        "paramTypeCombine",
        "preferDecodeRune",
        "preferFilepathJoin",
        "preferFprint",
        "preferStringWriter",
        "preferWriteByte",
        "ptrToRefParam",
        "rangeExprCopy",
        "rangeValCopy",
        "redundantSprint",
        "regexpMust",
        "regexpPattern",
        # This might be good, but I don't think we want to encourage
        # significant changes to regexes as we port stuff from Perl.
        # "regexpSimplify",
        "ruleguard",
        "singleCaseSwitch",
        "sliceClear",
        "sloppyLen",
        # This seems like it might also be good, but a lot of existing code
        # fails.
        # "sloppyReassign",
        "returnAfterHttpError",
        "sloppyTypeAssert",
        "sortSlice",
        "sprintfQuotedString",
        "sqlQuery",
        "stringXbytes",
        "switchTrue",
        "syncMapLoadAndDelete",
        "timeExprSimplify",
        "tooManyResultsChecker",
        "truncateCmp",
        "typeAssertChain",
        "typeDefFirst",
        "typeSwitchVar",
        "typeUnparen",
        "underef",
        "unlabelStmt",
        "unlambda",
        # I am not sure we would want this linter and a lot of existing
        # code fails.
        # "unnamedResult",
        "unnecessaryBlock",
        "unnecessaryDefer",
        "unslice",
        "valSwap",
        "weakCond",
        "wrapperFunc",
        "yodaStyleExpr",
        # This requires explanations for "nolint" directives. This would be
        # nice for gosec ones, but I am not sure we want it generally unless
        # we can get the false positive rate lower.
        # "whyNoLint"
    ]

[linters-settings.gofumpt]
    extra-rules = true
    lang-version = "1.16"

[linters-settings.gomodguard]
  [[linters-settings.gomodguard.blocked.modules]]
  [linters-settings.gomodguard.blocked.modules."github.com/avct/uasurfer"]
    recommendations = ["github.com/xavivars/uasurfer"]
    reason = "The original avct module appears abandoned."

  [[linters-settings.gomodguard.blocked.modules]]
  [linters-settings.gomodguard.blocked.modules."github.com/BurntSushi/toml"]
    recommendations = ["github.com/pelletier/go-toml"]
    reason = "This library panics frequently on invalid input."

  [[linters-settings.gomodguard.blocked.modules]]
  [linters-settings.gomodguard.blocked.modules."github.com/gofrs/uuid"]
    recommendations = ["github.com/google/uuid"]

  [[linters-settings.gomodguard.blocked.modules]]
  [linters-settings.gomodguard.blocked.modules."github.com/satori/go.uuid"]
    recommendations = ["github.com/google/uuid"]

  [[linters-settings.gomodguard.blocked.modules]]
  [linters-settings.gomodguard.blocked.modules."github.com/lib/pq"]
    recommendations = ["github.com/jackc/pgx"]
    reason = "This library is no longer actively maintained."

  [[linters-settings.gomodguard.blocked.modules]]
  [linters-settings.gomodguard.blocked.modules."github.com/neilotoole/errgroup"]
    recommendations = ["golang.org/x/sync/errgroup"]
    reason = "This library can lead to subtle deadlocks in certain use cases."

  [[linters-settings.gomodguard.blocked.modules]]
  [linters-settings.gomodguard.blocked.modules."github.com/pariz/gountries"]
    reason = "This library's data is not actively maintained. Use GeoInfo data."

  [[linters-settings.gomodguard.blocked.modules]]
  [linters-settings.gomodguard.blocked.modules."github.com/RackSec/srslog"]
    recommendations = ["github.com/RackSec/srslog"]
    reason = "This library's data is not actively maintained."

  [[linters-settings.gomodguard.blocked.modules]]
  [linters-settings.gomodguard.blocked.modules."github.com/ua-parser/uap-go/uaparser"]
    recommendations = ["github.com/xavivars/uasurfer"]
    reason = "The performance of this library is absolutely abysmal."

  [[linters-settings.gomodguard.blocked.modules]]
  [linters-settings.gomodguard.blocked.modules."github.com/ugorji/go/codec"]
    recommendations = ["encoding/json", "github.com/mailru/easyjson"]
    reason = "This library is poorly maintained. We should default to using encoding/json and use easyjson where performance really matters."

  [[linters-settings.gomodguard.blocked.versions]]
  [linters-settings.gomodguard.blocked.versions."github.com/jackc/pgx"]
    version = "< 4.0.0"
    reason = "Use github.com/jackc/pgx/v4"

[linters-settings.govet]
    "enable-all" = true

[linters-settings.lll]
    line-length = 120
    tab-width = 4

[linters-settings.nolintlint]
    allow-leading-space = false
    allow-unused = false
    allow-no-explanation = ["lll", "misspell"]
    require-explanation = true
    require-specific = true

[linters-settings.revive]
    ignore-generated-header = true
    severity = "warning"

    # This might be nice but it is so common that it is hard
    # to enable.
    # [[linters-settings.revive.rules]]
    # name = "add-constant"

    # [[linters-settings.revive.rules]]
    # name = "argument-limit"

    [[linters-settings.revive.rules]]
    name = "atomic"

    [[linters-settings.revive.rules]]
    name = "bare-return"

    [[linters-settings.revive.rules]]
    name = "blank-imports"

    [[linters-settings.revive.rules]]
    name = "bool-literal-in-expr"

    [[linters-settings.revive.rules]]
    name = "call-to-gc"

    # [[linters-settings.revive.rules]]
    # name = "cognitive-complexity"

    # Probably a good rule, but we have a lot of names that
    # only have case differences.
    # [[linters-settings.revive.rules]]
    # name = "confusing-naming"

    [[linters-settings.revive.rules]]
    name = "confusing-results"

    [[linters-settings.revive.rules]]
    name = "constant-logical-expr"

    [[linters-settings.revive.rules]]
    name = "context-as-argument"

    [[linters-settings.revive.rules]]
    name = "context-keys-type"

    # [[linters-settings.revive.rules]]
    # name = "cyclomatic"

    [[linters-settings.revive.rules]]
    name = "deep-exit"

    [[linters-settings.revive.rules]]
    name = "defer"

    [[linters-settings.revive.rules]]
    name = "dot-imports"

    [[linters-settings.revive.rules]]
    name = "duplicated-imports"

    [[linters-settings.revive.rules]]
    name = "early-return"

    [[linters-settings.revive.rules]]
    name = "empty-block"

    [[linters-settings.revive.rules]]
    name = "empty-lines"

    [[linters-settings.revive.rules]]
    name = "errorf"

    [[linters-settings.revive.rules]]
    name = "error-naming"

    [[linters-settings.revive.rules]]
    name = "error-return"

    [[linters-settings.revive.rules]]
    name = "error-strings"

    [[linters-settings.revive.rules]]
    name = "exported"

    # [[linters-settings.revive.rules]]
    # name = "file-header"

    # We have a lot of flag parameters. This linter probably makes
    # a good point, but we would need some cleanup or a lot of nolints.
    # [[linters-settings.revive.rules]]
    # name = "flag-parameter"

    # [[linters-settings.revive.rules]]
    # name = "function-result-limit"

    [[linters-settings.revive.rules]]
    name = "get-return"

    [[linters-settings.revive.rules]]
    name = "identical-branches"

    [[linters-settings.revive.rules]]
    name = "if-return"

    [[linters-settings.revive.rules]]
    name = "imports-blacklist"

    [[linters-settings.revive.rules]]
    name = "import-shadowing"

    [[linters-settings.revive.rules]]
    name = "increment-decrement"

    [[linters-settings.revive.rules]]
    name = "indent-error-flow"

    # [[linters-settings.revive.rules]]
    # name = "line-length-limit"

    # [[linters-settings.revive.rules]]
    # name = "max-public-structs"

    [[linters-settings.revive.rules]]
    name = "modifies-parameter"

    [[linters-settings.revive.rules]]
    name = "modifies-value-receiver"

    # We frequently use nested structs, particularly in tests.
    # [[linters-settings.revive.rules]]
    # name = "nested-structs"

    [[linters-settings.revive.rules]]
    name = "optimize-operands-order"

    [[linters-settings.revive.rules]]
    name = "package-comments"

    [[linters-settings.revive.rules]]
    name = "range"

    [[linters-settings.revive.rules]]
    name = "range-val-address"

    [[linters-settings.revive.rules]]
    name = "range-val-in-closure"

    [[linters-settings.revive.rules]]
    name = "receiver-naming"

    [[linters-settings.revive.rules]]
    name = "redefines-builtin-id"

    [[linters-settings.revive.rules]]
    name = "string-of-int"

    [[linters-settings.revive.rules]]
    name = "struct-tag"

    [[linters-settings.revive.rules]]
    name = "superfluous-else"

    [[linters-settings.revive.rules]]
    name = "time-naming"

    [[linters-settings.revive.rules]]
    name = "unconditional-recursion"

    [[linters-settings.revive.rules]]
    name = "unexported-naming"

    [[linters-settings.revive.rules]]
    name = "unexported-return"

    # This is covered elsewhere and we want to ignore some
    # functions such as fmt.Fprintf.
    # [[linters-settings.revive.rules]]
    # name = "unhandled-error"

    [[linters-settings.revive.rules]]
    name = "unnecessary-stmt"

    [[linters-settings.revive.rules]]
    name = "unreachable-code"

    [[linters-settings.revive.rules]]
    name = "unused-parameter"

    # We generally have unused receivers in tests for meeting the
    # requirements of an interface.
    # [[linters-settings.revive.rules]]
    # name = "unused-receiver"

    [[linters-settings.revive.rules]]
    name = "useless-break"

    [[linters-settings.revive.rules]]
    name = "var-declaration"

    [[linters-settings.revive.rules]]
    name = "var-naming"

    [[linters-settings.revive.rules]]
    name = "waitgroup-by-value"

[linters-settings.unparam]
    check-exported = true

[linters-settings.wrapcheck]
    "ignoreSigs" = [
        ".Errorf(",
        "errgroup.NewMultiError(",
        "errors.New(",
        ".Wait(",
        ".WithMessage(",
        ".WithMessagef(",
        ".WithStack(",
        ".Wrap(",
        ".Wrapf(",
        "v4.Retry(",
        "v4.RetryNotify(",
    ]

[issues]
exclude-use-default = false

  # This goes off for MD5 usage, which we use heavily
  [[issues.exclude-rules]]
  text = "weak cryptographic primitive"
  linters = ["gosec"]

  [[issues.exclude-rules]]
  linters = [
    "gocritic"
  ]
  # For some reason the imports stuff in ruleguard doesn't work in golangci-lint.
  # Perhaps it has an outdated version or something
  path = "_test.go"
  text = "ruleguard: Prefer the alternative Context method instead"

  [[issues.exclude-rules]]
  linters = [
    "gocritic"
  ]
  ## The nolintlint linter behaves oddly with ruleguard rules
  source = "// *no-ruleguard"


  [[issues.exclude-rules]]
  linters = [
    "gosec"
  ]
  # G306 - "Expect WriteFile permissions to be 0600 or less".
  text = "G306"

  [[issues.exclude-rules]]
  linters = [
    "govet"
  ]
  # we want to enable almost all govet rules. It is easier to just filter out
  # the ones we don't want:
  #
  # * fieldalignment - way too noisy. Although it is very useful in particular
  #   cases where we are trying to use as little memory as possible, having
  #   it go off on every struct isn't helpful.
  # * shadow - although often useful, it complains about _many_ err
  #   shadowing assignments and some others where shadowing is clear.
  text = "^(fieldalignment|shadow)"

  [[issues.exclude-rules]]
  linters = [
    "govet"
  ]
  text = "shadow: declaration of \"err\" shadows declaration"

  [[issues.exclude-rules]]
  linters = [
    "contextcheck",
    "nilerr",
    "wrapcheck",
  ]
  path = "_test.go"

  [[issues.exclude-rules]]
  linters = [
    "stylecheck",
  ]
  # ST1016 - methods on the same type should have the same receiver name.
  #    easyjson doesn't interact well with this.
  text = "ST1016"

  [[issues.exclude-rules]]
  linters = [
    "staticcheck",
  ]
  # SA5008: unknown JSON option "intern" - easyjson specific option.
  text = 'SA5008: unknown JSON option "intern"'

  [[issues.exclude-rules]]
  linters = [
    "wrapcheck"
  ]
  text = "github.com/maxmind/geoipupdate"

  [[issues.exclude-rules]]
  linters = [
    "wrapcheck",
  ]
  path = "_easyjson.go"

  [[issues.exclude-rules]]
  linters = [
    "gocritic",
  ]
  source = "Chmod|WriteFile"
  text = "octalLiteral"

Go environment

go version go1.17.7 linux/amd64
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/wstorey/.cache/go-build"
GOENV="/home/wstorey/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/wstorey/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/wstorey/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/wstorey/bin/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/wstorey/bin/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.17.7"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/wstorey/geoipupdate/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3541699771=/tmp/go-build -gno-record-gcc-switches"

Verbose output of running

wstorey@penguin:~/geoipupdate (horgh/lint)$ golangci-lint cache clean
wstorey@penguin:~/geoipupdate (horgh/lint)$ golangci-lint run -v
INFO [config_reader] Config search paths: [./ /home/wstorey/geoipupdate /home/wstorey /home /]
INFO [config_reader] Used config file .golangci.toml
INFO [lintersdb] Active 48 linters: [asciicheck bidichk bodyclose containedctx contextcheck deadcode depguard durationcheck errcheck errname errorlint exhaustive exportloopref forbidigo goconst gocritic gocyclo godot gofumpt gomodguard gosec gosimple govet grouper ineffassign lll makezero misspell nakedret nilerr noctx nolintlint predeclared revive rowserrcheck sqlclosecheck staticcheck structcheck stylecheck tenv tparallel typecheck unconvert unparam unused varcheck wastedassign wrapcheck]
INFO [loader] Go packages loading at mode 575 (exports_file|files|imports|name|types_sizes|deps|compiled_files) took 161.49081ms
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 2.043217ms
INFO [linters context/goanalysis] analyzers took 16.690070895s with top 10 stages: gocritic: 4.987684727s, buildir: 3.814613442s, buildssa: 654.912324ms, wastedassign: 549.040957ms, the_only_name: 543.453052ms, exhaustive: 525.883617ms, bidichk: 407.544995ms, inspect: 322.505771ms, godot: 300.879013ms, nilness: 290.722731ms
INFO [runner/max_same_issues] 5/8 issues with text "package-comments: should have a package comment, unless it's in another file for this package" were hidden, use --max-same-issues
INFO [runner] Issues before processing: 60, after processing: 3
INFO [runner] Processors filtering stat (out/in): max_from_linter: 3/3, filename_unadjuster: 60/60, exclude: 60/60, nolint: 8/34, diff: 8/8, path_prefixer: 3/3, skip_files: 60/60, skip_dirs: 60/60, identifier_marker: 60/60, severity-rules: 3/3, max_same_issues: 3/8, path_shortener: 3/3, sort_results: 3/3, cgo: 60/60, path_prettifier: 60/60, autogenerated_exclude: 60/60, exclude-rules: 34/60, uniq_by_line: 8/8, max_per_file_from_linter: 8/8, source_code: 3/3
INFO [runner] processing took 5.865401ms with stages: nolint: 3.022995ms, identifier_marker: 1.077587ms, exclude-rules: 828.492µs, path_prettifier: 401.658µs, autogenerated_exclude: 294.529µs, source_code: 113.184µs, skip_dirs: 64.366µs, max_same_issues: 40.425µs, uniq_by_line: 7.827µs, cgo: 5.178µs, filename_unadjuster: 2.707µs, path_shortener: 1.513µs, max_from_linter: 1.338µs, max_per_file_from_linter: 1.159µs, skip_files: 826ns, diff: 528ns, severity-rules: 428ns, exclude: 291ns, sort_results: 268ns, path_prefixer: 102ns
INFO [runner] linters took 4.50578492s with stages: goanalysis_metalinter: 4.499824686s
cmd/geoipupdate/args.go:1:1: package-comments: should have a package comment, unless it's in another file for this package (revive)
package main

import (
        "log"
        "os"

        flag "github.com/spf13/pflag"
)

// Args are command line arguments.
type Args struct {
        ConfigFile        string
        DatabaseDirectory string
        StackTrace        bool
        Verbose           bool
}

func getArgs() *Args {
        configFile := flag.StringP(
                "config-file",
                "f",
                defaultConfigFile,
                "Configuration file",
        )
        databaseDirectory := flag.StringP(
                "database-directory",
                "d",
                "",
                "Store databases in this directory (uses config if not specified)",
        )
        help := flag.BoolP("help", "h", false, "Display help and exit")
        stackTrace := flag.Bool("stack-trace", false, "Show a stack trace along with any error message.")
        verbose := flag.BoolP("verbose", "v", false, "Use verbose output")
        displayVersion := flag.BoolP("version", "V", false, "Display the version and exit")

        flag.Parse()

        if *help {
                printUsage()
        }
        if *displayVersion {
                log.Printf("geoipupdate %s", version)
                //nolint: revive // deep exit from main package
                os.Exit(0)
        }

        if *configFile == "" {
                log.Printf("You must provide a configuration file.")
                printUsage()
        }

        return &Args{
                ConfigFile:        *configFile,
                DatabaseDirectory: *databaseDirectory,
                StackTrace:        *stackTrace,
                Verbose:           *verbose,
        }
}

func printUsage() {
        log.Printf("Usage: %s <arguments>\n", os.Args[0])
        flag.PrintDefaults()
        //nolint: revive // deep exit from main package
        os.Exit(1)
}
cmd/geoipupdate/version.go:4:1: package-comments: should have a package comment, unless it's in another file for this package (revive)
package main

import "runtime/debug"

func init() {
        if info, ok := debug.ReadBuildInfo(); ok && info.Main.Version != "(devel)" {
                version = info.Main.Version
        }
}
pkg/geoipupdate/database/local_file_writer.go:1:1: package-comments: should have a package comment, unless it's in another file for this package (revive)
package database

import (
        "crypto/md5"
        "fmt"
        "hash"
        "io"
        "log"
        "os"
        "path/filepath"
        "strings"
        "time"

        "github.com/gofrs/flock"
        "github.com/pkg/errors"
)

// LocalFileDatabaseWriter is a database.Writer that stores the database to the
// local file system.
type LocalFileDatabaseWriter struct {
        filePath      string
        lockFilePath  string
        verbose       bool
        lock          *flock.Flock
        oldHash       string
        fileWriter    io.Writer
        temporaryFile *os.File
        md5Writer     hash.Hash
}

// NewLocalFileDatabaseWriter create a LocalFileDatabaseWriter. It creates the
// necessary lock and temporary files to protect the database from concurrent
// writes.
func NewLocalFileDatabaseWriter(filePath, lockFilePath string, verbose bool) (*LocalFileDatabaseWriter, error) {
        dbWriter := &LocalFileDatabaseWriter{
                filePath:     filePath,
                lockFilePath: lockFilePath,
                verbose:      verbose,
        }

        var err error
        if dbWriter.lock, err = CreateLockFile(lockFilePath, verbose); err != nil {
                return nil, err
        }
        if err = dbWriter.createOldMD5Hash(); err != nil {
                return nil, err
        }

        temporaryFilename := fmt.Sprintf("%s.temporary", dbWriter.filePath)
        //nolint:gosec // We want the permission to be world readable
        dbWriter.temporaryFile, err = os.OpenFile(
                temporaryFilename,
                os.O_WRONLY|os.O_CREATE|os.O_TRUNC,
                0o644,
        )
        if err != nil {
                return nil, errors.Wrap(err, "error creating temporary file")
        }
        dbWriter.md5Writer = md5.New()
        dbWriter.fileWriter = io.MultiWriter(dbWriter.md5Writer, dbWriter.temporaryFile)

        return dbWriter, nil
}

func (writer *LocalFileDatabaseWriter) createOldMD5Hash() error {
        currentDatabaseFile, err := os.Open(writer.filePath)
        if err != nil {
                if os.IsNotExist(err) {
                        writer.oldHash = ZeroMD5
                        return nil
                }
                return errors.Wrap(err, "error opening database")
        }

        defer func() {
                err := currentDatabaseFile.Close()
                if err != nil {
                        log.Println(errors.Wrap(err, "error closing database"))
                }
        }()
        oldHash := md5.New()
        if _, err := io.Copy(oldHash, currentDatabaseFile); err != nil {
                return errors.Wrap(err, "error calculating database hash")
        }
        writer.oldHash = fmt.Sprintf("%x", oldHash.Sum(nil))
        if writer.verbose {
                log.Printf("Calculated MD5 sum for %s: %s", writer.filePath, writer.oldHash)
        }
        return nil
}

// Write writes to the temporary file.
func (writer *LocalFileDatabaseWriter) Write(p []byte) (int, error) {
        n, err := writer.fileWriter.Write(p)
        if err != nil {
                return 0, errors.Wrap(err, "error writing")
        }
        return n, nil
}

// Close closes the temporary file and releases the file lock.
func (writer *LocalFileDatabaseWriter) Close() error {
        err := writer.temporaryFile.Close()
        if err != nil {
                var perr *os.PathError
                if !errors.As(err, &perr) || !errors.Is(perr.Err, os.ErrClosed) {
                        return errors.Wrap(err, "error closing temporary file")
                }
        }

        if err := os.Remove(writer.temporaryFile.Name()); err != nil && !os.IsNotExist(err) {
                return errors.Wrap(err, "error removing temporary file")
        }
        if err := writer.lock.Unlock(); err != nil {
                return errors.Wrap(err, "error releasing lock file")
        }
        return nil
}

// ValidHash checks that the temporary file's MD5 matches the given hash.
func (writer *LocalFileDatabaseWriter) ValidHash(expectedHash string) error {
        actualHash := fmt.Sprintf("%x", writer.md5Writer.Sum(nil))
        if !strings.EqualFold(actualHash, expectedHash) {
                return errors.Errorf("md5 of new database (%s) does not match expected md5 (%s)", actualHash, expectedHash)
        }
        return nil
}

// SetFileModificationTime sets the database's file access and modified times
// to the given time.
func (writer *LocalFileDatabaseWriter) SetFileModificationTime(lastModified time.Time) error {
        if err := os.Chtimes(writer.filePath, lastModified, lastModified); err != nil {
                return errors.Wrap(err, "error setting times on file")
        }
        return nil
}

// Commit renames the temporary file to the name of the database file and syncs
// the directory.
func (writer *LocalFileDatabaseWriter) Commit() error {
        if err := writer.temporaryFile.Sync(); err != nil {
                return errors.Wrap(err, "error syncing temporary file")
        }
        if err := writer.temporaryFile.Close(); err != nil {
                return errors.Wrap(err, "error closing temporary file")
        }
        if err := os.Rename(writer.temporaryFile.Name(), writer.filePath); err != nil {
                return errors.Wrap(err, "error moving database into place")
        }

        // fsync the directory. http://austingroupbugs.net/view.php?id=672
        dh, err := os.Open(filepath.Dir(writer.filePath))
        if err != nil {
                return errors.Wrap(err, "error opening database directory")
        }

        // We ignore Sync errors as they primarily happen on file systems that do
        // not support sync.
        _ = dh.Sync()

        if err := dh.Close(); err != nil {
                return errors.Wrap(err, "closing directory")
        }
        return nil
}

// GetHash returns the hash of the current database file.
func (writer *LocalFileDatabaseWriter) GetHash() string {
        return writer.oldHash
}
INFO File cache stats: 22 entries of total size 59.6KiB
INFO Memory: 47 samples, avg is 279.3MB, max is 547.8MB
INFO Execution took 4.687589506s

Code example or link to a public repository

https://github.com/maxmind/geoipupdate

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions