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: apply systemd-stable 242 backports #63784

Merged
merged 1 commit into from Jul 1, 2019

Conversation

flokli
Copy link
Contributor

@flokli flokli commented Jun 25, 2019

from NixOS/systemd#29

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@flokli flokli requested review from fpletz and andir June 25, 2019 21:01
@ofborg ofborg bot requested a review from edolstra June 25, 2019 21:14
@flokli flokli changed the base branch from master to staging June 25, 2019 21:52
@flokli flokli force-pushed the nixos-v242-stable-backports branch 2 times, most recently from e4b18e5 to 174446b Compare June 28, 2019 19:32
@flokli
Copy link
Contributor Author

flokli commented Jun 28, 2019

updated to include the latest cherry-picks from NixOS/systemd#29 (comment) - we should however still change the ref in this PR after NixOS/systemd#29 is merged.

@arianvp
Copy link
Member

arianvp commented Jun 29, 2019

LGTM! Just update the ref before you do though @flokli

@flokli flokli force-pushed the nixos-v242-stable-backports branch 2 times, most recently from 246733d to 86d00c5 Compare June 29, 2019 13:32
@flokli
Copy link
Contributor Author

flokli commented Jun 29, 2019

With NixOS/systemd#29, updated rev to point to nixos-v${version} again.

I'm however not sure anymore if these should point to branch names (which change over time and, by that, make it hard to build again in the future), or tags (we'd then have to invent our own versioning scheme of backports), or just referencing commit ids.

Personally, I'd favour to point to commit ids, but don't want to start bikeshedding here.

@andir
Copy link
Member

andir commented Jun 29, 2019 via email

@flokli
Copy link
Contributor Author

flokli commented Jun 29, 2019

That might be true, but doesn't really work with a pull-request-based workflow - people can't PR nixos/systemd to create a new branch.

@flokli flokli force-pushed the nixos-v242-stable-backports branch from 86d00c5 to df489f8 Compare June 29, 2019 14:10
@flokli
Copy link
Contributor Author

flokli commented Jun 29, 2019

Same applies to creating releases / applying tags in the nixos/systemd repo.

I flipped this PR to refer the commit id in ref, doesn't make much sense otherwise.

@arianvp
Copy link
Member

arianvp commented Jun 29, 2019

Tags and branches are mutable. I'd prefer having immutable URLs. This saves us from a whole class of bugs . I'm fine with just referring to the commit id directly

flokli added a commit to flokli/nixpkgs that referenced this pull request Jun 29, 2019
branch names are mutable, and with
NixOS/systemd#29 being merged in, the nixos-v242
branch advanced from 5c20aab77900f478fd380ab189787d80e4a35963 to
40eb070cb309ec09def0ecdeaf7514c702200835, causing systemd's
fetchFromGitHub to fail with a sha256sum mismatch (when not relying on
the cache).

Fix this, by pointing systemd.src to the commit id before the branch
advancement. This won't cause a rebuild, as the sha256 stayed the same.

Fast-forwarding systemd to 40eb070cb309ec09def0ecdeaf7514c702200835 will
be done in NixOS#63784 , which also uses
the commit id, and not a branch name for rev.
Copy link
Member

@andir andir left a comment

Choose a reason for hiding this comment

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

I did run a complete nixos release build of this PR. It shows the same issues as before (with the other systemd bump that was merged a while ago).

Did not run this on any of my systems so it might still eat all our data but we will have to try at some point :-)

@flokli
Copy link
Contributor Author

flokli commented Jun 30, 2019

It shows the same issues as before (with the other systemd bump that was merged a while ago).

What do you mean by that? This one should (among other things) fix the ListenPort of wireguard links configured through systemd-networkd.

@andir
Copy link
Member

andir commented Jun 30, 2019

It shows the same issues as before (with the other systemd bump that was merged a while ago).

What do you mean by that?

That we had 41 tests failing before and 41 tests failing right now. All others seem good.

This one should (among other things) fix the ListenPort of wireguard links configured through systemd-networkd.

Then we should probably add tests for that?

@picnoir
Copy link
Member

picnoir commented Jun 30, 2019

Then we should probably add tests for that?

I'll add those tests in a subsequent PR adding the wireguard-related networkd options: picnoir@f46b528#diff-18749bc468ae39fb421058284e39fc32R1

This PR fixes the test previously linked. (it's actually been running on my router for a couple of days.)

@flokli
Copy link
Contributor Author

flokli commented Jul 1, 2019

Alright, let's merge this in then :-)

Looking forward to the wireguard follow-up commits!

@flokli flokli merged commit a7a0d11 into NixOS:staging Jul 1, 2019
@flokli flokli deleted the nixos-v242-stable-backports branch July 1, 2019 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants