* Show proper subcommand prefix in generated config CLI.
* Remove useless author info and copy command group description.
* Really accept (implicit) -h and --help flags.
These were disabled by HideHelp, leading to an error message in every
usage output. This way, the flags get documented as well.
* Override AppHelpTemplate to better match Kong's style.
* Override (Sub)commandHelpTemplate to better match Kong's style.
* Use <command> and [flags] like Kong.
Signed-off-by: André Colomb <src@andre.colomb.de>
This updates our logging framework from legacy freetext strings using
the `log` package to structured log entries using `log/slog`. I have
updated all INFO or higher level entries, but not yet DEBUG (😓)... So,
at a high level:
There is a slight change in log levels, effectively adding a new warning
level:
- DEBUG is still debug (ideally not for users but developers, though
this is something we need to work on)
- INFO is still info, though I've added more data here, effectively
making Syncthing more verbose by default (more on this below)
- WARNING is a new log level that is different from the _old_ WARNING
(more below)
- ERROR is what was WARNING before -- problems that must be dealt with,
and also bubbled as a popup in the GUI.
A new feature is that the logging level can be set per package to
something other than just debug or info, and hence I feel that we can
add a bit more things into INFO while moving some (in fact, most)
current INFO level warnings into WARNING. For example, I think it's
justified to get a log of synced files in INFO and sync failures in
WARNING. These are things that have historically been tricky to debug
properly, and having more information by default will be useful to many,
while still making it possible get close to told level of inscrutability
by setting the log level to WARNING. I'd like to get to a stage where
DEBUG is never necessary to just figure out what's going on, as opposed
to trying to narrow down a likely bug.
Code wise:
- Our logging object, generally known as `l` in each package, is now a
new adapter object that provides the old API on top of the newer one.
(This should go away once all old log entries are migrated.) This is
only for `l.Debugln` and `l.Debugf`.
- There is a new level tracker that keeps the log level for each
package.
- There is a nested setup of handlers, since the structure mandated by
`log/slog` is slightly convoluted (imho). We do this because we need to
do formatting at a "medium" level internally so we can buffer log lines
in text format but with separate timestamp and log level for the API/GUI
to consume.
- The `debug` API call becomes a `loglevels` API call, which can set the
log level to `DEBUG`, `INFO`, `WARNING` or `ERROR` per package. The GUI
is updated to handle this.
- Our custom `sync` package provided some debugging of mutexes quite
strongly integrated into the old logging framework, only turned on when
`STTRACE` was set to certain values at startup, etc. It's been a long
time since this has been useful; I removed it.
- The `STTRACE` env var remains and can be used the same way as before,
while additionally permitting specific log levels to be specified,
`STTRACE=model:WARN,scanner:DEBUG`.
- There is a new command line option `--log-level=INFO` to set the
default log level.
- The command line options `--log-flags` and `--verbose` go away, but
are currently retained as hidden & ignored options since we set them by
default in some of our startup examples and Syncthing would otherwise
fail to start.
Sample format messages:
```
2009-02-13 23:31:30 INF A basic info line (attr1="val with spaces" attr2=2 attr3="val\"quote" a=a log.pkg=slogutil)
2009-02-13 23:31:30 INF An info line with grouped values (attr1=val1 foo.attr2=2 foo.bar.attr3=3 a=a log.pkg=slogutil)
2009-02-13 23:31:30 INF An info line with grouped values via logger (foo.attr1=val1 foo.attr2=2 a=a log.pkg=slogutil)
2009-02-13 23:31:30 INF An info line with nested grouped values via logger (bar.foo.attr1=val1 bar.foo.attr2=2 a=a log.pkg=slogutil)
2009-02-13 23:31:30 WRN A warning entry (a=a log.pkg=slogutil)
2009-02-13 23:31:30 ERR An error (a=a log.pkg=slogutil)
```
---------
Co-authored-by: Ross Smith II <ross@smithii.com>
(v2 change)
This cleans up the command line parsing a little:
- Remove the hack for supporting legacy single-dash long options (e.g.
`-home`), thus enabling actual short options
- Move legacy imperative flags from under the serve command into
separate commands, e.g. `syncthing serve --paths` to see the paths list
is now `syncthing paths`, `syncthing --upgrade-check` is now `syncthing
upgrade --check`
- Add environment variable support for all remaining flags for the
`serve` command (with one exception, left for the reader to discover),
as these are now all modifiers and not imperative
```
% syncthing --help
Usage: syncthing <command>
Flags:
-h, --help Show context-sensitive help.
Commands:
serve Run Syncthing (default)
cli Command line interface for Syncthing
browser Open GUI in browser, then exit
decrypt Decrypt or verify an encrypted folder
device-id Show device ID, then exit
generate Generate key and config, then exit
paths Show configuration paths, then exit
upgrade Perform or check for upgrade, then exit
version Show current version, then exit
debug Various debugging commands
install-completions Print commands to install shell completions
Run "syncthing <command> --help" for more information on a command.
```
```
% syncthing serve --help
Usage: syncthing serve [flags]
Run Syncthing (default)
Flags:
-h, --help Show context-sensitive help.
-C, --config=PATH Set configuration directory (config and keys) ($STCONFDIR)
-D, --data=PATH Set data directory (database and logs) ($STDATADIR)
-H, --home=PATH Set configuration and data directory ($STHOMEDIR)
--allow-newer-config Allow loading newer than current config version ($STALLOWNEWERCONFIG)
--audit Write events to audit file ($STAUDIT)
--auditfile=PATH Specify audit file (use "-" for stdout, "--" for stderr) ($STAUDITFILE)
--db-maintenance-interval=8h Database maintenance interval ($STDBMAINTINTERVAL)
--gui-address=URL Override GUI address (e.g. "http://192.0.2.42:8443") ($STGUIADDRESS)
--gui-apikey=API-KEY Override GUI API key ($STGUIAPIKEY)
--no-console Hide console window ($STHIDECONSOLE)
--logfile=PATH Log file name (see below) ($STLOGFILE)
--logflags=BITS Select information in log line prefix (see below) ($STLOGFLAGS)
--log-max-old-files=N Number of old files to keep (zero to keep only current) ($STNUMLOGFILES)
--log-max-size=BYTES Maximum size of any file (zero to disable log rotation) ($STLOGMAXSIZE)
--no-browser Do not start browser ($STNOBROWSER)
--no-default-folder Don't create the "default" folder on first startup ($STNODEFAULTFOLDER)
--no-port-probing Don't try to find free ports for GUI and listen addresses on first startup ($STNOPORTPROBING)
--no-restart Do not restart Syncthing when exiting due to API/GUI command, upgrade, or crash ($STNORESTART)
--no-upgrade Disable automatic upgrades ($STNOUPGRADE)
--paused Start with all devices and folders paused ($STPAUSED)
--unpaused Start with all devices and folders unpaused ($STUNPAUSED)
--verbose Print verbose log output ($STVERBOSE)
--debug-gui-assets-dir=PATH Directory to load GUI assets from ($STGUIASSETS)
--debug-perf-stats Write running performance statistics to perf-$pid.csv (Unix only) ($STPERFSTATS)
--debug-profile-block Write block profiles to block-$pid-$timestamp.pprof every 20 seconds ($STBLOCKPROFILE)
--debug-profile-cpu Write a CPU profile to cpu-$pid.pprof on exit ($STCPUPROFILE)
--debug-profile-heap Write heap profiles to heap-$pid-$timestamp.pprof each time heap usage increases ($STHEAPPROFILE)
--debug-profiler-listen=ADDR Network profiler listen address ($STPROFILER)
--debug-reset-delta-idxs Reset delta index IDs, forcing a full index exchange
...
```
Switch the database from LevelDB to SQLite, for greater stability and
simpler code.
Co-authored-by: Tommy van der Vorst <tommy@pixelspark.nl>
Co-authored-by: bt90 <btom1990@googlemail.com>
At a high level, this is what I've done and why:
- I'm moving the protobuf generation for the `protocol`, `discovery` and
`db` packages to the modern alternatives, and using `buf` to generate
because it's nice and simple.
- After trying various approaches on how to integrate the new types with
the existing code, I opted for splitting off our own data model types
from the on-the-wire generated types. This means we can have a
`FileInfo` type with nicer ergonomics and lots of methods, while the
protobuf generated type stays clean and close to the wire protocol. It
does mean copying between the two when required, which certainly adds a
small amount of inefficiency. If we want to walk this back in the future
and use the raw generated type throughout, that's possible, this however
makes the refactor smaller (!) as it doesn't change everything about the
type for everyone at the same time.
- I have simply removed in cold blood a significant number of old
database migrations. These depended on previous generations of generated
messages of various kinds and were annoying to support in the new
fashion. The oldest supported database version now is the one from
Syncthing 1.9.0 from Sep 7, 2020.
- I changed config structs to be regular manually defined structs.
For the sake of discussion, some things I tried that turned out not to
work...
### Embedding / wrapping
Embedding the protobuf generated structs in our existing types as a data
container and keeping our methods and stuff:
```
package protocol
type FileInfo struct {
*generated.FileInfo
}
```
This generates a lot of problems because the internal shape of the
generated struct is quite different (different names, different types,
more pointers), because initializing it doesn't work like you'd expect
(i.e., you end up with an embedded nil pointer and a panic), and because
the types of child types don't get wrapped. That is, even if we also
have a similar wrapper around a `Vector`, that's not the type you get
when accessing `someFileInfo.Version`, you get the `*generated.Vector`
that doesn't have methods, etc.
### Aliasing
```
package protocol
type FileInfo = generated.FileInfo
```
Doesn't help because you can't attach methods to it, plus all the above.
### Generating the types into the target package like we do now and
attaching methods
This fails because of the different shape of the generated type (as in
the embedding case above) plus the generated struct already has a bunch
of methods that we can't necessarily override properly (like `String()`
and a bunch of getters).
### Methods to functions
I considered just moving all the methods we attach to functions in a
specific package, so that for example
```
package protocol
func (f FileInfo) Equal(other FileInfo) bool
```
would become
```
package fileinfos
func Equal(a, b *generated.FileInfo) bool
```
and this would mostly work, but becomes quite verbose and cumbersome,
and somewhat limits discoverability (you can't see what methods are
available on the type in auto completions, etc). In the end I did this
in some cases, like in the database layer where a lot of things like
`func (fv *FileVersion) IsEmpty() bool` becomes `func fvIsEmpty(fv
*generated.FileVersion)` because they were anyway just internal methods.
Fixes#8247
`syncthing cli` subcommand was using urfave/cli as the command parser.
This PR replace it with kong, which the main command uses.
Some help texts and error message format are changed. Other than that,
all the command usage and logic remains unchanged.
There's only one place which still uses urfave/cli, which is `syncthing
cli config`, because it uses some magic to dynamically build commands
from struct reflects. I used kong's `passthrough:""` tag to pass any
argument following `syncthing cli config` to urfave/cli parser.
This PR also fixes#9041
---------
Co-authored-by: Jakob Borg <jakob@kastelo.net>
This replaces old style errors.Wrap with modern fmt.Errorf and removes
the (direct) dependency on github.com/pkg/errors. A couple of cases are
adjusted by hand as previously errors.Wrap(nil, ...) would return nil,
which is not what fmt.Errorf does.
Currently the `Run()` function of the CLI always uses `os.Args` directly.
This change adds an additional `RunWithArgs()` function that allows passing
arguments as `[]string` instead. This is useful when linking against
Syncthing as a library to be able to also expose the CLI.
* lib/api: Note ItemStarted and ItemFinished for default filtering.
The reasoning why LocalChangeDetected and RemoteChangeDetected events
are not included in the event stream by default (without explicit
filter mask requested) also holds for the ItemStarted and ItemFinished
events. They should be excluded as well when we start to break the
API compatibility for some reason.
* gui: Enumerate unused event types in the eventService.
Define constants for the unused event types as well, for completeness'
sake. They are intentionally not handled in the GUI currently.
* cmd/syncthing: Harmonize uppercase CLI argument placeholders.
Use ALL-UPPERCASE and connecting dashes to distinguish argument
placeholders from literal argument options (e.g. "cpu" or "heap" for
profiling). The dash makes it clear which words form a single
argument and where a new argument starts.
This style is already used for the "syncthing cli debug file" command.
* lib/model: Simplify event data structure.
Using map[string]interface{} is not necessary when all values are
known to be strings.
Consistently use double dashes and fix typos -conf, -data-dir and
-verify.
Applies also to tests running the syncthing binary for consistency.
* Fix mismatched option name --conf in cli subcommand.
According to the source code comments, the cli option flags should
mirror those from the serve subcommand where applicable. That one is
actually called --config though.
* cli: Fix help text option placeholders.
The urfave/cli package uses the Value field of StringFlag to provide a
default value, not to name the placeholder. That is instead done with
backticks around some part of the Usage field.
* cli: Add missing --data flag in subcommand help text.
The urfave/cli based option parsing uses a fake flags collection to
generate help texts matching the used global options. But the --data
option was omitted from it, although it is definitely required when
using --config as well. Note that it cannot just be ignored, as some
debug stuff actually uses the DB:
syncthing cli --data=/bar --config=/foo debug index dump
* cmd/syncthing: Don't fail early on api setup error (fixes 7558)
* switch to factory pattern
* refactor config command to show help on nothing
* wip
* wip
* already abort in before
André Colomb
Jakob Borg
Gusted
cjc7373andJakob Borg
Nick Busey
luzpaz
Martchus
deepsource-autofix[bot]anddeepsource-autofix[bot] <62050782+deepsource-autofix[bot]@users.noreply.github.com>
sec65
greatroar
Daniel Barczyk
Simon Frei
ignacy123
Audrius Butkevicius
Audrius Butkevicius