systemd: allow setting timezone via timedatectl by lheckemann · Pull Request #26608 · NixOS/nixpkgs

lheckemann · 2017-06-15T19:28:12Z

Motivation for this change

I'm not sure about the exact process of getting this into nixpkgs proper. There's the matter of getting lheckemann/systemd@5352df7 into nixos's systemd (thankfully github allows referring to commits from forks in the source repo so this works even though it hasn't been merged anywhere) in addition to this. I've PRed this to staging because of the mass rebuild it causes.

Any feedback on the material contents of the PR in addition to process information is of course also very welcome!

Things done

Tested using sandboxing
(nix.useSandbox on NixOS,
or option build-use-sandbox in nix.conf
on non-NixOS)
Built on platform(s)
- NixOS
- macOS
- Linux
Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Fits CONTRIBUTING.md.

bjornfor · 2017-06-15T19:43:49Z

It's not clear to me what the timezone is if you leave time.timeZone at its new default (as in this PR) and don't set the timezone with timedatectl. It'd be nice to have that answer in the option description.

+1.

lheckemann · 2017-06-15T19:45:31Z

https://github.com/NixOS/nixpkgs/pull/26608/files#diff-de67a90fdc8239d085560b0393b3daf4R31 defaults to UTC, so this would not change behaviour from existing installations besides allowing imperative changes to the timezone. I'll add it to the option description.

joachifm · 2017-06-27T18:49:18Z

Was the systemd change proposed on the nixos systemd repo?

lheckemann · 2017-06-27T18:49:57Z

No, I should probably do that :)

lheckemann · 2017-06-27T18:51:29Z

NixOS/systemd#13

lheckemann · 2017-07-17T16:43:06Z

systemd change has been merged and I've rebased against staging, so this should be good to merge now.

Mic92 · 2017-07-17T17:15:20Z

nixos/tests/timezone.nix

this line has to be removed.

Whoopsies! Good catch, thanks.

Mic92 · 2017-07-17T17:19:14Z

nixos/tests/timezone.nix

this line has to be removed.

Mic92 · 2017-07-17T17:21:19Z

nixos/tests/timezone.nix

Should this not be the default anyway?

Mic92 · 2017-07-17T17:21:28Z

nixos/tests/timezone.nix

basvandijk · 2017-07-26T07:49:23Z

nixos/modules/config/timezone.nix

It would be slightly simpler to use optionalAttrs here.

Indeed, thanks for the feedback. I feel silly because I'm even using it just a few lines above…

Maybe some day we have a linter for Nix that can spot patterns like this. A man can wish...

joachifm · 2017-07-26T18:57:29Z

Looks to me like this PR by itself no longer is a mass rebuild, as it just contains changes to the nixos module.

lheckemann · 2017-07-26T18:58:13Z

Also correct! I'll change its target to master.

Fixes NixOS#26469.

lheckemann · 2017-07-27T21:37:34Z

Any further obstacles to merging, once ae26f29 has made it from staging into master?

joachifm · 2017-07-31T14:57:24Z

Thank you. If you care to, consider adding a changelog entry for 17.09.

lheckemann mentioned this pull request Jun 15, 2017

Allow setting timezone via timedatectl #26469

Closed

lheckemann force-pushed the timezone-pr branch from d8df30a to 1353db2 Compare June 15, 2017 19:47

fpletz approved these changes Jun 16, 2017

View reviewed changes

joachifm added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 1.severity: mass-rebuild labels Jun 26, 2017

lheckemann mentioned this pull request Jun 28, 2017

timedated: allow setting timezone NixOS/systemd#13

Closed

lheckemann force-pushed the timezone-pr branch from 1353db2 to 4d69ac6 Compare July 17, 2017 16:39

bjornfor approved these changes Jul 17, 2017

View reviewed changes

Mic92 reviewed Jul 17, 2017

View reviewed changes

lheckemann force-pushed the timezone-pr branch from 4d69ac6 to 555edb3 Compare July 17, 2017 17:16

Mic92 reviewed Jul 17, 2017

View reviewed changes

nixos/tests/timezone.nix Outdated

Copy link

Member

Mic92 Jul 17, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line has to be removed.

Mic92 reviewed Jul 17, 2017

View reviewed changes

nixos/tests/timezone.nix Outdated

Copy link

Member

Mic92 Jul 17, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here?

lheckemann force-pushed the timezone-pr branch from 555edb3 to 1acb3ec Compare July 17, 2017 17:22

basvandijk reviewed Jul 26, 2017

View reviewed changes

lheckemann force-pushed the timezone-pr branch from 1acb3ec to b5c2dcf Compare July 26, 2017 18:46

lheckemann changed the base branch from staging to master July 26, 2017 18:58

lheckemann force-pushed the timezone-pr branch from b5c2dcf to 8d05238 Compare July 26, 2017 18:59

joachifm removed the 1.severity: mass-rebuild label Jul 26, 2017

systemd: allow setting timezone via timedatectl

6e9d506

Fixes NixOS#26469.

lheckemann force-pushed the timezone-pr branch from 8d05238 to 6e9d506 Compare July 27, 2017 19:20

joachifm merged commit a0d4640 into NixOS:master Jul 31, 2017

fpletz added this to the 17.09 milestone Jul 31, 2017

fpletz added the 9.needs: changelog This PR needs a changelog entry label Jul 31, 2017

lheckemann deleted the timezone-pr branch August 4, 2017 19:35

lheckemann mentioned this pull request Aug 5, 2017

Document timezone changes in release notes #27954

Merged

8 tasks

lheckemann mentioned this pull request Nov 28, 2019

systemd: v243.3 -> v243.4 #74207

Closed

10 tasks

flokli mentioned this pull request Jun 18, 2020

NixOS doesn't support getting hostname from DHCP server #74847

Closed

NickCao mentioned this pull request Jan 29, 2024

nixos/timezone: stop using mode direct-symlink for linking localtime #284641

Closed

13 tasks

Uh oh!

Conversation

lheckemann commented Jun 15, 2017

Motivation for this change

Things done

Uh oh!

bjornfor commented Jun 15, 2017

Uh oh!

lheckemann commented Jun 15, 2017

Uh oh!

joachifm commented Jun 27, 2017

Uh oh!

lheckemann commented Jun 27, 2017

Uh oh!

lheckemann commented Jun 27, 2017

Uh oh!

lheckemann commented Jul 17, 2017

Uh oh!

Mic92 Jul 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lheckemann Jul 17, 2017

Choose a reason for hiding this comment

Uh oh!

Mic92 Jul 17, 2017

Choose a reason for hiding this comment

Uh oh!

Mic92 Jul 17, 2017

Choose a reason for hiding this comment

Uh oh!

lheckemann Jul 17, 2017

Choose a reason for hiding this comment

Uh oh!

Mic92 Jul 17, 2017

Choose a reason for hiding this comment

Uh oh!

basvandijk Jul 26, 2017

Choose a reason for hiding this comment

Uh oh!

lheckemann Jul 26, 2017

Choose a reason for hiding this comment

Uh oh!

basvandijk Jul 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joachifm commented Jul 26, 2017

Uh oh!

lheckemann commented Jul 26, 2017

Uh oh!

lheckemann commented Jul 27, 2017

Uh oh!

joachifm commented Jul 31, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Mic92 Jul 17, 2017 •

edited

Loading

basvandijk Jul 26, 2017 •

edited

Loading