Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Test failure because of umask #6551

Closed
acolomb opened this issue Apr 19, 2020 · 0 comments · Fixed by #7241
Closed

Test failure because of umask #6551

acolomb opened this issue Apr 19, 2020 · 0 comments · Fixed by #7241
Labels
build Issues caused by or requiring changes to the build system (scripts or Docker image) frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion
Milestone
v1.13.0

Comments

@acolomb
Copy link
Member

acolomb commented Apr 19, 2020

The umask for my user under Linux is set to 0027. When running go run build.go test locally with that umask, the test suite fails:

--- FAIL: TestChmodDir (0.00s)
    basicfs_test.go:126: wrong perm: true 0750
FAIL
FAIL	github.com/syncthing/syncthing/lib/fs	0.408s

This is caused by the Mkdir() function subtracting the effective umask from the given mode argument, as documented. So the test can only succeed if the developer's umask allows creation of directories with a mode of 755.

I'm not sure whether this has any real-world effects besides the test suite failing. Might just be an overly cautious test case.

Version Information

Syncthing Version: v1.5.0-rc.1
Development environment: Linux Ubuntu 18.04 LTS
@calmh calmh added the build Issues caused by or requiring changes to the build system (scripts or Docker image) label Apr 20, 2020
acolomb added a commit to acolomb/syncthing that referenced this issue Dec 29, 2020
@acolomb
lib/fs: Fix TestChmodDir depending on umask (fixes syncthing#6551)
a1c3cb5 
The test would fail if the umask on UNIX is greater than 0022, because
the OS transparently subtracts it from the mode passed to Mkdir(), as
the Go documentation confirms.

Our goal here is not to test os.Mkdir(), so just make sure the desired
mode is actually set by forcing it afterwards.
@acolomb acolomb linked a pull request Dec 29, 2020 that will close this issue
lib/fs: Fix TestChmodDir depending on umask (fixes #6551) #7241
Merged
calmh pushed a commit that referenced this issue Dec 30, 2020
@acolomb
lib/fs: Fix TestChmodDir depending on umask (fixes #6551) (#7241)
3d1edd2 
The test would fail if the umask on UNIX is greater than 0022, because
the OS transparently subtracts it from the mode passed to Mkdir(), as
the Go documentation confirms.

Our goal here is not to test os.Mkdir(), so just make sure the desired
mode is actually set by forcing it afterwards.
calmh added a commit to calmh/syncthing that referenced this issue Jan 5, 2021
@calmh
Merge branch 'main' into conlimit
35954fd 
* main:
  lib/model: Fix child-check when deleting dirs in pull (syncthing#7236)
  lib/model: Handle index sender terminating due to error (fixes syncthing#7231) (syncthing#7232)
  lib/fs: Fix TestChmodDir depending on umask (fixes syncthing#6551) (syncthing#7241)
  gui, man, authors: Update docs, translations, and contributors
  lib/model: Cleanup redundant filesystem variables in folders (syncthing#7237)
  gui: Do not replace zero versioning cleanup interval (syncthing#7234)
@calmh calmh added this to the v1.13.0 milestone Jan 11, 2021
hiqua pushed a commit to hiqua/syncthing that referenced this issue Jan 11, 2021
@acolomb @hiqua
lib/fs: Fix TestChmodDir depending on umask (fixes syncthing#6551) (s…
cddaae2 
…yncthing#7241)

The test would fail if the umask on UNIX is greater than 0022, because
the OS transparently subtracts it from the mode passed to Mkdir(), as
the Go documentation confirms.

Our goal here is not to test os.Mkdir(), so just make sure the desired
mode is actually set by forcing it afterwards.
SimonPickup added a commit to MobiusSync/syncthing that referenced this issue Feb 9, 2021
@SimonPickup
Merge tag 'v1.13.1' into ios_try_syncthing_v1.13.1
7b0fd48 
v1.13.1

This release adds configuration options for min/max connections (see
https://docs.syncthing.net/advanced/option-connection-limits.html) and
moves the storage of pending devices/folders from the config to the
database (see
https://docs.syncthing.net/dev/rest.html#cluster-endpoints).

Bugfixes:

- syncthing#7005: panic: nil pointer dereference because (*db.Lowlevel)getMetaAndCheck() returns nil
- syncthing#7076: File not detected due to watching reporting events on old, deleted path
- syncthing#7165: Connections aren't actually closed when closing a protocol connection
- syncthing#7184: Spurious unexpected directory in untrusted folder
- syncthing#7197: In Recent Changes, the Device column is empty
- syncthing#7231: panic: deadlock detected at fmut

Enhancements:

- syncthing#7176: Active connections min/max setting
- syncthing#7178: Pending devices and folders should live in the database instead of configuration

Other issues:

- syncthing#6551: Test failure because of umask
@st-review st-review added the frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion label Dec 31, 2021
@syncthing syncthing locked and limited conversation to collaborators Dec 31, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
build Issues caused by or requiring changes to the build system (scripts or Docker image) frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion
Projects
None yet
Milestone
v1.13.0
Development

Successfully merging a pull request may close this issue.

lib/fs: Fix TestChmodDir depending on umask (fixes #6551) acolomb/syncthing
3 participants
@calmh @acolomb @st-review