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

syncthing generate command writes the deviceId to stderr instead of stdout as INFO #8682

Closed
MinaKhamesi opened this issue Nov 18, 2022 · 4 comments · Fixed by #8685
Closed

syncthing generate command writes the deviceId to stderr instead of stdout as INFO #8682

MinaKhamesi opened this issue Nov 18, 2022 · 4 comments · Fixed by #8685
Labels
bug A problem with current functionality, as opposed to missing functionality (enhancement) frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion needs-triage New issues needed to be validated
Milestone
v1.22.2

Comments

@MinaKhamesi
Copy link

  • syncthing version v1.22.1
  • ubuntu 18

I had a script in which I was running syncthing -generate=my_dir and I was grabbing the DevideId from the output, after upgrading to new version of syncthing, that part of code broke, because apperantly now the deviceId is getting printed to stderr instead of stdout and my code cannot capture it. I did fix it by redirecting stderr to stdout. But it would be nice if I didn't have to do that since that is not actually an error.

the output of syncthing -generate before:

16:28:02 INFO: Device ID: 6C4FCZ7-N2OUG4Y-YYNLDGP-FSRR6RY-SQUUGRH-TIV4AAK-C3SDB62-Q4QBDQC
16:28:02 INFO: Default folder created and/or linked to new config

and the output I am getting now:

2022/11/16 16:34:33 INFO: Generating ECDSA key and certificate for syncthing...
2022/11/16 16:34:33 Device ID: 3IPJSF4-TWXI57O-P64DONH-DRCPFEL-JRWFFJT-4MLAFUR-CTJNSQG-U2JFZAO
2022/11/16 16:34:33 INFO: Default folder created and/or linked to new config

the first and third are INFo and are in stdout, but the middle one, devideId, is getting printed in stderr
@MinaKhamesi MinaKhamesi added bug A problem with current functionality, as opposed to missing functionality (enhancement) needs-triage New issues needed to be validated labels Nov 18, 2022
@calmh
Copy link
Member

calmh commented Nov 18, 2022

Huh. Which version is "before"?

MinaKhamesi pushed a commit to MinaKhamesi/syncthing that referenced this issue Nov 18, 2022
cmd/syncthing/generate: Log DeviceId to stdout instead of stderr (fixes
9e78501 
syncthing#8682)
@MinaKhamesi MinaKhamesi mentioned this issue Nov 18, 2022
Closed
MinaKhamesi pushed a commit to MinaKhamesi/syncthing that referenced this issue Nov 18, 2022
cmd/syncthing/generate: Log DeviceId to stdout instead of stderr (fixes
b3da072 
syncthing#8682)
@calmh
Copy link
Member

calmh commented Nov 19, 2022

This changed between 1.18.4 and 1.18.5, inadvertently, as part of moving the generate command.

calmh added a commit to calmh/syncthing that referenced this issue Nov 19, 2022
@calmh
cmd/syncthing: Use main logger in generate subcommand (fixes syncthin…
a7a87cd 
…g#8682)

We had some unholy mix of our own logger and the stdlib logger, probably
because for historical reasons we wanted the device ID to stdout and the
rest to stderr? But that's not the case any more, and the mix of formats
is weird. Ideally I think the generate command should be silent and just
print the device ID and nothing else, but that's tricky to accomplish
since we have other methods do logging on their own. Hence this just
harmonizes it so that we at least use the same logger with the same
format and target...
@calmh
Copy link
Member

calmh commented Nov 19, 2022

I added an alternate implementation.

@acolomb acolomb linked a pull request Nov 20, 2022 that will close this issue
cmd/syncthing: Use main logger in generate subcommand (fixes #8682) #8685
Merged
acolomb pushed a commit that referenced this issue Nov 20, 2022
@calmh
cmd/syncthing: Use main logger in generate subcommand (fixes #8682) (#…
24275b4 
…8685)

We had some unholy mix of our own logger and the stdlib logger, probably
because for historical reasons we wanted the device ID to stdout and the
rest to stderr? But that's not the case any more, and the mix of formats
is weird. Ideally I think the generate command should be silent and just
print the device ID and nothing else, but that's tricky to accomplish
since we have other methods do logging on their own. Hence this just
harmonizes it so that we at least use the same logger with the same
format and target...
@MinaKhamesi
Copy link
Author

@calmh Thanks for quick reply and quick action 👍🏼 . I see the change is in main now, do you know roughly when can I expect a release with this new change?

Thank you so much @calmh

@calmh calmh added this to the v1.22.2 milestone Nov 22, 2022
calmh added a commit to calmh/syncthing that referenced this issue Nov 22, 2022
@calmh
Merge branch 'main' into linuxbuild
4ee1015 
* main: (23 commits)
  lib/fs: Optimize WindowsInvalidFilename (syncthing#8687)
  gui, man, authors: Update docs, translations, and contributors
  cmd/syncthing: Use main logger in generate subcommand (fixes syncthing#8682) (syncthing#8685)
  build: Update all dependencies (fixes syncthing#8679) (syncthing#8680)
  gui, man, authors: Update docs, translations, and contributors
  lib/model: Correctly set xattrs on temp files (fixes syncthing#8667) (syncthing#8670)
  gui: Automatically dismiss authentication reminder when in LDAP mode (fixes syncthing#8661) (syncthing#8663)
  lib/model: Correctly handle xattrs on directories (fixes syncthing#8657) (syncthing#8658)
  lib/protocol: Ignore inode time when xattr&ownership is ignored (fixes syncthing#8654) (syncthing#8655)
  lib/fs: Try to remove read only Windows files (fixes syncthing#3744) (syncthing#8650)
  gui: Add copy to clipboard, share by email, and share by SMS buttons to device IDs (fixes syncthing#2771, ref syncthing#3868) (syncthing#7984)
  gui, man, authors: Update docs, translations, and contributors
  build: Add GitHub actions build for Windows (syncthing#8627)
  gui: Fix connection type icon width (fixes syncthing#8592) (syncthing#8644)
  gui: Adjust connection type icon size scaling and alignment (syncthing#8645)
  docker: Use healthcheck endpoint (syncthing#8640)
  lib/connections: Use adaptive write size for rate limited connections (fixes syncthing#8630) (syncthing#8631)
  gui: Mark devices that haven't connected for a long time (fixes syncthing#7703) (syncthing#8530)
  gui: Fix rescan interval when add encrypted folder with watch for changes enabled (fixes syncthing#8570) (syncthing#8571)
  gui: Always show Out of Sync Items for remote devices (syncthing#8632)
  ...
@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 Nov 20, 2023
@syncthing syncthing locked and limited conversation to collaborators Nov 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
bug A problem with current functionality, as opposed to missing functionality (enhancement) frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion needs-triage New issues needed to be validated
Projects
None yet
Milestone
v1.22.2
Development

Successfully merging a pull request may close this issue.

cmd/syncthing: Use main logger in generate subcommand (fixes #8682) calmh/syncthing
3 participants
@calmh @st-review @MinaKhamesi