Comment by jcgl

Comment by jcgl 5 days ago

I'm not sure I understand why you think the solution proposed there is so bad.

The question in that issue is around the semantics of time-sync.target. Targets are synchronization points for the system and don't (afaik) generally make promises about the units that are ordered before them (in this case chrony-wait.service.

Does that answer your specific objection of "proposing that the solution to that is breaking Requires is just wild to me"? Basically, what is proposed in that issue is not breaking Requires=. The proposition is that the user add their own, specific Requires= as a drop-in configuration since that's not a generally-applicable default.

magicalhippo 5 days ago

No, that does not make sense, because it goes against the systemd documentation.

Targets[1]: Target units do not offer any additional functionality on top of the generic functionality provided by units. They merely group units, allowing a single target name to be used in Wants= and Requires= settings to establish a dependency on a set of units defined by the target, and in Before= and After= settings to establish ordering.

boot-complete.target[2]: Order units that shall only run when the boot process is considered successful after the target unit and pull in the target from it, also with Requires=.

Note use of "only run" with a reference to Requires=.

time-sync.target[3]: This target provides stricter clock accuracy guarantees than time-set.target (see above), but likely requires network communication and thus introduces unpredictable delays. Services that require clock accuracy and where network communication delays are acceptable should use this target.

Especially note the last sentence there.

The documentation clearly indicates that targets can fail, and that services that needs the target to be successful, should use Requires= to specify that.

If the above is not true, the Requires= and Targets documentation should be rewritten to explicitly say that targets might fulfill Requires= regardless of state. Also, the documentation for time-sync.target should explicitly remove the last sentence and instead state there is no functional difference between Requires=time-sync.target and Wants=time-sync.target, it is best-effort only.

[1]: https://www.freedesktop.org/software/systemd/man/latest/syst...

[2]: https://www.freedesktop.org/software/systemd/man/latest/syst...

[3]: https://www.freedesktop.org/software/systemd/man/latest/syst...

Reply View 4 replies

jcgl 5 days ago

That seems like a fair point about the documentation! As far as I can see, you're right.

Reply View | 3 replies
- magicalhippo 5 days ago
  
  So that's why I find his statements disturbing.
  If he really don't want targets to deliver failed/success guarantees, then they've massively miscommunicated in their documentation. That in my book is a huge deal.
  In either case the issue should in no circumstance be casually dismissed as not-a-bug without further action.
  
  Reply View | 2 replies
  
  jcgl 5 days ago
  
  I don't personally find it as disturbing as you do, I think. Which isn't to say that I don't think it should be fixed, etc. etc.
  I'm sure the project would accept a documentation patch to amend this discrepancy. At the end of the day (despite what some people on the internet might like to allege), systemd is a free software project that, despite having (more or less) a BIFL, is ultimately a relatively bazaar-like project.
  Though since these targets and unit properties are very core to systemd-the-service manager, I do think that this is a bigger documentation oversight than most.
  
  Reply View | 1 reply
  
  magicalhippo 5 days ago
  
  The disturbing part isn't the bug in time-sync.target or documentation, the disturbing part is how casually he brushes the issue away.
  To me this is a huge red flag for a senior contributer to a core systems component, signalling some fundamental lack of understanding or imagination.
  I very much disagree with not fixing time-sync.target, but if he had instead written a well-reasoned explanation for why time-sync.target should not propagate failed states and flagging it as a documentation bug, then that's something I'd respect and would be fine with. Or, even better IMHO, he'd fix time-sync.target and state that users who wants to boot regardless should use Wants instead.
  
  Reply View | 0 replies