refactor(client): split the vault and prefs into leaf packages - #60
Merged
Merged
Conversation
Importing client for programmatic vault access meant inheriting its whole closure -- tview/tcell and the terminfo database, viper and zerolog via cli/common, and bbolt -- to read a YAML file. The cost isn't binary size, which the linker handles, but the module graph a consumer carries. The vault now lives in client/vault and the prefs store in client/prefs, both leaves. A consumer importing them pulls 10 modules instead of 38, dropping tview, tcell, viper, bbolt, cobra, afero and 22 others. Three things had to move that the issue didn't account for: - AppConfigDir, into a new leaf appdir. It was already stdlib-only, but Go compiles a package wholesale, so importing cli/common for it dragged in viper and zerolog anyway. cli/common re-exports it. - LoadPrefs/SavePrefs/VaultIdentityRef, which the vault needs to read and write its own keypair reference. That pulled the rest of the Prefs type with it, since methods live with their type. TargetsFromPrefs and TargetsFromRelayList stay behind -- they return a TargetsSpec, which is what bbolt hangs off. - ResolveRelayURL, which prefs needs to validate relay entries. It now sits with the relay list it validates, and its tests moved with it. client re-exports every previous name, permanently: types as aliases, so a *client.VaultEntry and a *vault.Entry are the same type. No call site changed -- the existing client/vault_test.go now doubles as a test that the aliases really are transparent. zerolog is still in the leaf closure, via nmilat/utils. That one is upstream's to fix. Closes #59
4 tasks
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Closes #59.
The vault moves to
client/vaultand the prefs store toclient/prefs, both leaf packages.clientre-exports every previous name, so no existing call site changes — all ~149 of them still compile untouched.Measured result
Module closure a consumer inherits, which is what the issue was actually about:
client(before, and still)client/vault(new)28 modules dropped, including
rivo/tview,gdamore/tcell,spf13/viper,go.etcd.io/bbolt,spf13/cobra,spf13/afero,pelletier/go-toml,fsnotify,sourcegraph/concandgorilla/websocket.go list -deps ./client/vault | grep -cE 'tview|tcell|viper|bbolt|terminfo|afero|mapstructure'→0.Three couplings the issue didn't account for
The issue read
client/vault.go's import block, which understates what the file actually uses:AppConfigDir→ new leafappdir. It was already stdlib-only, but Go compiles a package wholesale, so importingcli/commonfor it dragged viper and zerolog in regardless.cli/commonre-exports it, so the CLI is unchanged.LoadPrefs/SavePrefs/VaultIdentityRef— the vault reads and writes its own keypair reference through prefs. That pulled the wholePrefstype along, since methods must live with their type.TargetsFromPrefs/TargetsFromRelayListstay inclient: they return aTargetsSpec, which is what bbolt hangs off.ResolveRelayURL—prefsneeds it to validate relay entries, but it lived in the bbolt-heavyspec.go. It now sits with the relay list it validates; its tests (including the unexportedlooksLikeRelayHost) moved with it.Answering the issue's two questions
client/vaultas suggested, with de-stuttered members —vault.Exists,vault.Path,vault.Entry,vault.FindEntry,vault.CreateIdentity,vault.Unlock.=) not definitions, so*client.VaultEntryand*vault.Entryare the same type and can be mixed freely.GenerateIdentityandIdentitymoved too, as the issue suggested — generating a key and saving it are now reachable together without the streaming stack.One thing still outstanding, upstream
zerologremains in the leaf closure vianmilat/utils, which imports it. Nothing ncli can do from this side; worth an upstream issue if the 10 is to become 9.Verification
gofmt,go build ./...,go vet ./...,go mod tidy(no diff),go test -short -race ./..., golangci-lint v2.13.2 → 0 issues.client/vault_test.gowas left in packageclienton purpose: it exercises the vault entirely through the re-exported names, so it now doubles as proof the aliases are transparent.