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.

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

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

Closed
wants to merge 2 commits into from
Closed

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

wants to merge 2 commits into from

Conversation

@flokli
Copy link
Contributor

flokli commented Nov 25, 2019

This bumps systemd to the v243.4 stable release.

#74026 already bumped to 243.3, but that was not sufficient enough to include the fixes mentioned in #73504 (comment).

I'd appreciate some thorough looks, especially given upstream also did some changes in src/timedate/timedated.c, which had to be treated manually while merging in the v243.4 branch.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
flokli added 2 commits Nov 25, 2019
@flokli flokli requested review from Mic92 and andir Nov 25, 2019
@@ -36,10 +36,10 @@ in stdenv.mkDerivation {
# When updating, use https://github.com/systemd/systemd-stable tree, not the development one!
# Also fresh patches should be cherry-picked from that tree to our current one.
src = fetchFromGitHub {
owner = "NixOS";
owner = "flokli";

This comment has been minimized.

@flokli

flokli Nov 25, 2019 Author Contributor

I'll push this to NixOS/systemd (and change back the expression) once everybody agrees with the changes there.

This comment has been minimized.

@lheckemann

lheckemann Nov 26, 2019 Member

The commit should be reachable through the NixOS repo as well as long as it exists somewhere in the fork network, so this shouldn't even be necessary. (Here this feature is convenient, but it can also be bad, see e.g. 7b4c9e9)

This comment has been minimized.

@flokli

flokli Nov 27, 2019 Author Contributor

Yes, I know. Still, I want to push it properly.

@andir
Copy link
Member

andir commented Nov 25, 2019

IF you are referring to flokli/systemd@cc103c7 then that shouldn't matter for us since we disabled manual changing of timezones anyway.

In general please provide links to those places that did have to be manually changed instead of sending people on a hunt when asking for feedback. GitHub is terrible at displaying merges and which commits were introduced in which merge....

@flokli
Copy link
Contributor Author

flokli commented Nov 25, 2019

Yes, that was what I meant, and I agree it shouldn't matter for NixOS.

@lheckemann
Copy link
Member

lheckemann commented Nov 26, 2019

IF you are referring to flokli/systemd@cc103c7 then that shouldn't matter for us since we disabled manual changing of timezones anyway.

Only if time.timeZone is set! Otherwise timezones can still be set using timedatectl set-timezone— I use this since I don't consider my laptop's current physical location as part of its config.

@globin
globin approved these changes Nov 27, 2019
@flokli
Copy link
Contributor Author

flokli commented Nov 27, 2019

Only if time.timeZone is set! Otherwise timezones can still be set using timedatectl set-timezone— I use this since I don't consider my laptop's current physical location as part of its config.

I'm not sure if this is properly supported at all.

Does this mean this PR should be done differently?

@lheckemann
Copy link
Member

lheckemann commented Nov 28, 2019

I'm not sure if this is properly supported at all.

Well, it's worked since #26608 ;)

Does this mean this PR should be done differently?

I'm not sure, haven't looked at the actual code yet. I just wanted to correct the assumption that the timezone is always fixed on nixos for now :)

EDIT: Just saw that I introduced a test with the change, so if nixos/tests/timezone.nix still passes it should all be good.

@lheckemann
Copy link
Member

lheckemann commented Nov 28, 2019

This breaks nixos/tests/timezone.nix attr timezone-imperative. I can have a look at it tomorrow.

@FRidh FRidh added this to Needs review in Staging Nov 30, 2019
@flokli
Copy link
Contributor Author

flokli commented Dec 2, 2019

@lheckemann ping ;-)

@tomfitzhenry tomfitzhenry mentioned this pull request Dec 4, 2019
3 of 10 tasks complete
@flokli
Copy link
Contributor Author

flokli commented Dec 6, 2019

@lheckemann ping ;-)

This is required to fix #73504 and blocking #73505.

@FRidh FRidh added this to the 20.03 milestone Dec 10, 2019
@lheckemann
Copy link
Member

lheckemann commented Dec 12, 2019

mayflower/systemd@17b4843 fixes it, should probably be squashed into flokli/systemd@cc103c7 with a corresponding explanation

@flokli
Copy link
Contributor Author

flokli commented Dec 14, 2019

@lheckemann feel free to prepare another branch at github.com/mayflower/systemd and take over this PR - I think you know best how to wire this into the existing commit history ;-)

@lheckemann lheckemann mentioned this pull request Dec 21, 2019
3 of 3 tasks complete
@flokli
Copy link
Contributor Author

flokli commented Dec 27, 2019

closing in favour of #76134.

@flokli flokli closed this Dec 27, 2019
Staging automation moved this from Needs review to Done Dec 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Staging
  
Done
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.