Code review comment for ~ogayot/curtin:systemd-offline

Revision history for this message
Olivier Gayot (ogayot) wrote :

Thanks for the comments, Dan and Chris! I updated the MP accordingly.

SYSTEMD_OFFLINE will now take values '0' or '1' instead of 'true'. Originally I found out about this variable while reading the systemd code and I didn't look at the documentation. But, I agree that it is better to follow the doc.

I also liked the idea of `systemd_force_offline` defaulting to None so I implemented the change.

Lastly, I moved the systemd_force_offline variable to a more global location in `curtin.util.subp`. The logic is as follows now:

* if systemd_force_offline is True or False, then SYSTEMD_OFFLINE` is set accordingly in the environment (any previous value is overridden).
* if systemd_force_offline is None, then SYSTEMD_OFFLINE is set to '1' if and only if the two following conditions are met:
  * We are executing the command in a chroot (i.e., target is not /)
  * The SYSTEMD_OFFLINE variable is not present in the environment

« Back to merge proposal