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.
When creating an initial default config, we usually probe for a free
TCP port. But when a UNIX socket is specified via the `STGUIADDRESS=`
override or the `--gui-address=unix:///...` command line syntax, parsing
that option will fail during port probing.
The solution is to just skip the port probing when the address is
determined to specify something other than a TCP socket.
### Testing
Start with a fresh home directory each time.
1. Specify a UNIX socket for the GUI (works with this PR):
TMPHOME=$(mktemp -d); ./syncthing --home=$TMPHOME
--gui-address=unix://$TMPHOME/socket
2. Specify no GUI address (probes for a free port if default is taken,
as before):
TMPHOME=$(mktemp -d); ./syncthing --home=$TMPHOME
3. Specify a TCP GUI address (probes whether the given port is taken,
as before):
TMPHOME=$(mktemp -d); ./syncthing --home=$TMPHOME
--gui-address=127.0.0.1:8385
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
Each section in the advanced settings dialog has similar code to insert
repeated input fields for each option. But only the first section (GUI
options) was adjusted in #9743 to avoid calling the `inputTypeFor()`
function repeatedly.
Apply the same caching to a locally scoped variable for each ng-repeat
entry by defining it in an ng-init directive.
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>
### Purpose
When generating a new `config.xml` file with default options, the GUI
address is populated with a hard-coded default value of
`127.0.0.1:8384`, except for a random free port if that default one is
occupied. This is independent from the GUI configuration default address
defined in the protobuf description. More importantly, it ignores any
`STGUIADDRESS` override given via environment variable or command-line
option, thus probing for the default port instead of the one specified
via override.
The `ProbeFreePorts()` function now respects the override, by reading
the `GUIConfiguration.Address()` method instead of using hard-coded
defaults.
When not calling `ProbeFreePorts()`, the override should still be
persisted rather than the default address. This happens only when
generating a fresh default `config.xml`, never on an existing one.
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.
### Purpose
This closes#9400 by always expanding tildes when parent/subdir checks
are done.
### Testing
I tested this by creating folders with paths to parent or subdirectories
of the default folder that include a tilde in their path as shown in the
attached screenshots.
With this change, overlap will be detected regardless of wether or not
tildes are used in other folder paths.
### Screenshots
Default Folder:
![2024-10-26-At-08h40m33s](https://github.com/user-attachments/assets/07df090c-4481-41ec-b741-d2785fc848d5)
Newly created folder (parent directory in this case)
![2024-10-26-At-08h40m13s](https://github.com/user-attachments/assets/636fa1fd-41dc-44d9-ac90-0a4937c9921c)
---------
Signed-off-by: tobifroe <froeltob@pm.me>
### Purpose
As discussed in #9686
Syncthing currently does not check folderstate on remote device before
pulling. If no devices have a valid folderstate (i.e all devices have
the folder paused) it will still attempt to pull. On large folders this
will cause a hanging "Syncing" status.
This checks whether at least one connected device has the file available
and has a valid folderstate.
### Testing
Tested locally on multiple devices.
We're new to Go (all our stuff is Python) so please bear with!
Interested if there may be a better place to slot this in.
Thanks,
Jon
---------
Co-authored-by: Simon Frei <freisim93@gmail.com>
Currently, the "Restart" and "Shutdown" buttons are displayed in the
middle of the Actions menu. On the other hand, the "Log Out" button is
displayed at the very bottom. However, in other cases, e.g. the menus in
operating systems like Windows or macOS, these kind of buttons are
usually grouped together.
Therefore, move the "Restart" and "Shutdown" buttons down, so that they
are listed together with the "Log Out" button. Also, change the order,
so that it goes from the least impactful ("Log Out") to the most
impactful ("Shutdown").
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
### Screenshots
#### Before
![image](https://github.com/user-attachments/assets/a51438ef-bb6f-4535-a972-8c1bc1dffa02)
#### After
![image](https://github.com/user-attachments/assets/535762d6-6f26-44ab-a402-db87bdcbfb36)
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
### Purpose
This fixes#9775. I also improved the comments as they were lacking.
My apologies for introducing this bug. In summary, the bug was
```
mode = mode ^ (ModeSymlink | ModeIrregular)
```
didn't correctly reset those bits. This correctly resets them:
```
mode = mode &^ ModeSymlink &^ ModeIrregular
```
Tested and working in Windows 11 version 10.0.22631.4317. I didn't test
in other versions, but I'm sure this is the only issue.
Currently we log on every single one of 10 retries deep in the upnp
stack. However we also return the failure as an error, which is bubbled
up a while until it's logged at debug level. Switch that around, such
that the repeat logging happens at debug level but the top-level happens
at info. There's some chance that this will newly log errors from
nat-pmp that were previously hidden in debug level - I hope those are
useful and not too numerous.
Also potentially this can even close#9324, my (very limited)
understanding of the reports/discussion there is that there's likely no
problem with syncthing beyond the excessive logging, it's some weird
router behaviour.
These CSS overrides address issues that are already present on wider
screens, so apply it there. Some experiments show we might even want to
up the limit more, but I am chicken and lazy, so I propose to use the
existing 470px media block.
Supersedes another PR after not getting any reaction to feedback there:
https://github.com/syncthing/syncthing/pull/9591#issuecomment-2212586134
Co-authored-by: Jakob Borg <jakob@kastelo.net>
As discussed in
https://github.com/syncthing/syncthing/pull/9175#discussion_r1730431703,
entries in advanced settings are unusable if they are comprised of a
list of objects. It just displays `[object Object], [object Object],
[object Object]`, e.g. for the devices a folder is shared with.
Filter out these config elements by detecting an array whose members are
not all strings or numbers, and setting them to `skip` type.
Fix some unnecessary repetition in calling `inputTypeFor()`, since it is
already cached in the `ng-init` directive.
### Purpose
Fixes#9776 by tweaking the text/background colours of disabled checkbox
panels when dark mode is enabled.
It was [noted on that
issue](https://github.com/syncthing/syncthing/issues/9776#issuecomment-2424828520)
that there's a bigger issue around the correctness of using the
`disabled` attribute on a `<div>` in the first place, but this PR does
not attempt to change that.
### Testing
I've hooked up the GUI files against a release build as suggested below.
### Screenshots
Using the dark theme, or the default theme with a system dark scheme:
![image](https://github.com/user-attachments/assets/3c6bfa77-cc7a-4f3e-a5c2-83daf54dcc34)
Using the black theme:
![image](https://github.com/user-attachments/assets/768db657-aa52-4db0-8455-5194a00fc143)
These borrow the colours from dark theme text inputs and black theme
tabs for a consistent look (initially I tried the text colour of
disabled text inputs, but that produced some poor contrast).