refactor(scanner): remove C++ taglib adapter (#5349)
Some checks are pending
Pipeline: Test, Lint, Build / Test JS code (push) Waiting to run
Pipeline: Test, Lint, Build / Get version info (push) Waiting to run
Pipeline: Test, Lint, Build / Lint Go code (push) Waiting to run
Pipeline: Test, Lint, Build / Test Go code (push) Waiting to run
Pipeline: Test, Lint, Build / Lint i18n files (push) Waiting to run
Pipeline: Test, Lint, Build / Check Docker configuration (push) Waiting to run
Pipeline: Test, Lint, Build / Build (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-1 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-2 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Cleanup digest artifacts (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-3 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-4 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-5 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-6 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-7 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-8 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-9 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-10 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Push to GHCR (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Push to Docker Hub (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build Windows installers (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Package/Release (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Upload Linux PKG (push) Blocked by required conditions

* refactor(build): remove CPP taglib adapter

Remove the CGO-based TagLib adapter (adapters/taglib/) and all
cross-taglib build infrastructure. The WASM-based go-taglib adapter
(adapters/gotaglib/) is now the sole metadata extractor.

- Delete adapters/taglib/ (CPP/CGO wrapper)
- Delete .github/actions/download-taglib/
- Remove CROSS_TAGLIB_VERSION, CGO_CFLAGS_ALLOW, and all taglib-related
  references from Dockerfile, Makefile, CI pipeline, and devcontainer

* fix(scanner): gracefully fallback to default extractor instead of crashing

Replace log.Fatal with a graceful fallback when the configured scanner
extractor is not found. Instead of terminating the process, the code now
warns and falls back to the default taglib extractor using the existing
consts.DefaultScannerExtractor constant. A fatal log is retained only for
the case where the default extractor itself is not registered, which
indicates a broken build.

* test(scanner): cover default extractor fallback and suppress redundant warn

Address review feedback on the extractor fallback in newLocalStorage:
- Only log the "using default" warning when the configured extractor
  differs from the default, so a broken build (default extractor itself
  missing) logs only the fatal — not a misleading "falling back" warn
  followed immediately by the fatal.
- Add a unit test that registers a mock under consts.DefaultScannerExtractor,
  sets the configured extractor to an unknown name, and asserts the local
  storage is constructed using the default extractor's constructor.
This commit is contained in:
Deluan Quintão 2026-04-12 21:52:29 -04:00 committed by GitHub
parent 52e47b896a
commit 0a6b5519cc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
19 changed files with 32 additions and 1440 deletions

View file

@ -11,6 +11,7 @@ import (
"github.com/djherbis/times"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/consts"
"github.com/navidrome/navidrome/core/storage"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model/metadata"
@ -28,7 +29,13 @@ type localStorage struct {
func newLocalStorage(u url.URL) storage.Storage {
newExtractor, ok := extractors[conf.Server.Scanner.Extractor]
if !ok || newExtractor == nil {
log.Fatal("Extractor not found", "path", conf.Server.Scanner.Extractor)
if conf.Server.Scanner.Extractor != consts.DefaultScannerExtractor {
log.Warn("Extractor not found, using default", "extractor", conf.Server.Scanner.Extractor, "default", consts.DefaultScannerExtractor)
}
newExtractor = extractors[consts.DefaultScannerExtractor]
if newExtractor == nil {
log.Fatal("Default extractor not registered", "extractor", consts.DefaultScannerExtractor)
}
}
isWindowsPath := filepath.VolumeName(u.Host) != ""
if u.Scheme == storage.LocalSchemaID && isWindowsPath {

View file

@ -10,6 +10,7 @@ import (
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/conf/configtest"
"github.com/navidrome/navidrome/consts"
"github.com/navidrome/navidrome/core/storage"
"github.com/navidrome/navidrome/model/metadata"
. "github.com/onsi/ginkgo/v2"
@ -135,16 +136,31 @@ var _ = Describe("LocalStorage", func() {
})
})
Context("with invalid extractor", func() {
It("should handle extractor validation correctly", func() {
// Note: The actual implementation uses log.Fatal which exits the process,
// so we test the normal path where extractors exist
Context("when the configured extractor is not registered", func() {
var defaultExtractor *mockTestExtractor
BeforeEach(func() {
defaultExtractor = &mockTestExtractor{results: make(map[string]metadata.Info)}
RegisterExtractor(consts.DefaultScannerExtractor, func(fs.FS, string) Extractor {
return defaultExtractor
})
DeferCleanup(func() {
lock.Lock()
delete(extractors, consts.DefaultScannerExtractor)
lock.Unlock()
})
})
It("falls back to the default extractor instead of crashing", func() {
conf.Server.Scanner.Extractor = "nonexistent-extractor"
u, err := url.Parse("file://" + tempDir)
Expect(err).ToNot(HaveOccurred())
storage := newLocalStorage(*u)
Expect(storage).ToNot(BeNil())
ls, ok := storage.(*localStorage)
Expect(ok).To(BeTrue())
Expect(ls.extractor).To(BeIdenticalTo(defaultExtractor))
})
})
})