* main:
build: push artifacts to Azure (#10044)
fix(syncthing): use separate lock file instead of locking the certificate (fixes#10053) (#10054)
fix(syncthing): use separate lock file instead of locking the certificate (fixes#10053) (#10054)
* release-1.29.5:
build: push artifacts to Azure (#10044)
fix(syncthing): use separate lock file instead of locking the certificate (fixes#10053) (#10054)
* main:
fix(gui): fix previous commit
fix(gui): mark unseen disconnected devices as inactive (#10048)
fix(strings): differentiate setup(n) and set(v) up (#10024)
chore(fs): changes to allow Filesystem to be implemented externally (#10040)
chore(config): resolve primary STUN servers via SRV record (fixes#10029) (#10031)
build: push artifacts to Azure (#10044)
chore(gui, man, authors): update docs, translations, and contributors
This adds a simple delay to the process for starting the pull, by
default one second. In practice this means we're likely to wait for
initial index transfer, or multiple messages sent as part of a larger
change. This is better because we're more likely to have the whole
change for the purpose of handling renames etc, and also it's more
efficient to do one larger puller iteration instead of multiple while
also processing changes.
It does however introduce a certain amount of delay into the sync
process, so it can be tuned down or turned off entirely.
This changes the database structure to use one database per folder, with
a small main database to coordinate. Reverts the prior change to buffer
all files in memory when pulling, meaning there is now a phase where the
WAL file will grow significantly, at least for initial sync of folders
with many directories.
---------
Co-authored-by: bt90 <btom1990@googlemail.com>
* main:
fix(config): properly apply defaults when reading folder configuration (#10034)
chore(model): add metric for total number of conflicts (#10037)
build: replace underscore in Debian version (#10032)
This reduces the number of file entries we carry in the database,
sometimes significantly. The downside is that if a file is deleted while
a device is offline, and that device comes back more than the cutoff
interval (six months) later, those files will get resurrected at some
point.
(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>
We've had weak/rolling hashing in the code for quite a while. It was a
popular request for a while, based on the belief that rsync does this
and we should too. However, the benefit is quite small; we save on
average about 0.8% of transferred blocks over the population as a whole:
<img width="974" alt="Screenshot 2025-03-28 at 17 09 02"
src="https://github.com/user-attachments/assets/bbe10dea-f85e-4043-9823-7cef1220b4a2"
/>
This would be fine if the cost was comparably low, however the downside
of attempting rolling hash matching is that we (by default) do a
complete file read on the destination in order to look for matches
before we starting pulling blocks for the file. For any larger file this
means a sometimes long, I/O-intensive pause before the file starts
syncing, for usually no benefit.
I propose we simply rip off the bandaid and save the effort.
The requirements for Windows code signing changed in 2023, so that newly
generated certificates can only be stored in hardware modules. Luckily,
I managed to snag a three year certificate before that so it hasn't
affected us so much. Now though, it does, because our cert is expiring
in March.
This changes the code signing process for Windows to use a cloud
service, Azure Trusted Signing. This appears to work equally well and
outsources the problem entirely, while also being cheaper than the
actual certificate was to begin with. 🤷
The signing entity will be Kastelo AB and not the Syncthing Foundation,
because the latter is almost impossible to get a certificate for as it's
not a normal corporate entity whose existence can be verified, etc. This
is also how it was prior to the latest certificate; it's not ideal, but
I think it's acceptable under the circumstances.
Currently, this just results in a very ambiguous `setting metadata: lookup
failed` while it could report what it's looking up and why it failed
(not found, etc).
Marshal() was called with the read lock held, and in turn called
Created() which also takes the read lock. This is fine by itself, but
there is a risk of deadlock if another call to lock the mutex happens
concurrently, as the lock call will block the inner rlock and the outer
rlock can never become unlocked.
It's an easy fix as marshalling is guaranteed to be called with a read
lock and does not need to call any methods that read lock themselves.
This was broken in the Protobuf refactor. The reason is that with the
previous library, generated types would have a JSON marshalling method
that would automatically return strings for enums. In the current
library you need to use the jsonpb marshaller for that, but these are
hand crafted structs so we can't do that. The easy solution is to just
use strings directly, since this is an API-only type anyway.
The GitHub checkout action does weird stuff with tags which breaks `git
describe`. This works around that so we get proper release builds on tag
pushes.
We must skip unix sockets, fifos, etc when scanning as these are not
filetypes we can handle. Currently we return a "bug" error, which
results in the walk being aborted and the rest of the tree being
essentially invisible to Syncthing. Instead, just ignore these files and
continue onwards.
This might well be #9859 as well but I can't confirm.
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
I came accross this in another context and didn't investigate fully, but
literally ten lines above this code, in another method, we say that
filesets _must_ be created under the lock. It's either one or the other
and I'm taking the safer route here.
---------
Co-authored-by: Simon Frei <freisim93@gmail.com>
Encrypted-to-encrypted connections (i.e., ones where both sides set a
password) used to work but were broken in the 1.28.0 release. The
culprit is the 5342bec1b refactor which slightly changed how the request
was constructed, resulting in a bad block hash field.
Co-authored-by: Simon Frei <freisim93@gmail.com>
This adds `allow_contributor: true` which allows approvals by
contributors to the PR (but still not the author themself, which is a
different thing). This allows things like pushing minor fixups while
also approving.
The `ignore_update_merges: true` option makes it so that someone is not
considered a "contributor" just because they push the merge button to
update the branch. In principle this is not needed given the above, but
I like it for clarity.
Skipping these makes the sequence numbering inconcistent; we've received
a file and suppsedly added it to the database, but if you check the
sequence number afterwards it didn't increase, i.e., we trigger [this
failure
condition](https://github.com/syncthing/syncthing/blob/47f48faed7331b7ba4ad3d6775d5cffacf8931b5/lib/model/indexhandler.go#L447-L459)
and, similarly, a future update will look like there was a hole in the
numbering.
I propose to at least temporarily remove this optimisation in order for
things to make more sense. Is there a reason to keep this beyond saving
some database operations?
There was a bug that the unique ID was not set when reporting was
enabled, and thus the reports where rejected by the server. The unique
ID got set only on startup, so next time Syncthing restarted.
This makes sure to set the unique ID when blank.
I can see already in our Sentry data that there are a fair amount of
these warnings, and mostly the shape of it. Asking users to report them
will likely cause a lot of reporting effort to fairly little additional
value. We can do that when/if we have something more targeted to ask
for.
This codifies a review policy which is closer to what I always
envisioned, but which isn't expressible using the normal checks in the
GitHub GUI. It would move the commit approval check from GitHub into the
policy-bot check which is already present to enforce the
conventional-commits standard. Approvals in general would still work the
same -- it's just that the bot picks it up and toggles the status
accordingly. From a GitHub side when this is enabled we'd remove the
requires-review check from there and let the bot decide that part. We
would still require builds and tests to pass of course.
There are a couple of relexations from the current policy, details in
the code but briefly:
- Changes to translations or dependencies by a trusted person don't
require review
- Trivial changes by a trusted person, explicitly marked as such, don't
require review
This enables less bureaucracy for things like adding new translated
languages and updating dependencies, and enables the trivial-change
workflow to a larger audience than, like, me, who could always just
bypass the rules by way of being admin.
Jakob Borg
