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

syslinux: 2015-11-09 -> 2019-02-07 #77703

Merged
merged 1 commit into from Jan 19, 2020
Merged

syslinux: 2015-11-09 -> 2019-02-07 #77703

merged 1 commit into from Jan 19, 2020

Conversation

lblasc
Copy link
Contributor

@lblasc lblasc commented Jan 14, 2020

Motivation for this change

Bump syslinux to 2019-02-06 following debian packaging:

  • gPXE is removed in upstream
  • added few meaningful patches from debian
  • switched to python 3

syslinux is blocker for #66528

Thanks!

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 nixpkgs-review --run "nixpkgs-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.

@samueldr
Copy link
Member

Hi, thanks for the PR!

I don't know if you had seen #55500, which had to be reverted? I haven't followed through and re-updated, so that's a great help.

Though, noted there:

 # This is syslinux-6.04-pre3^1; syslinux-6.04-pre3 fails to run.
 # Same issue here https://www.syslinux.org/archives/2019-February/026330.html

I had to go with b40487005223a78c3bb4c300ef6c436b3f6ec1f7, as the next commit didn't boot once built.

diff --git a/pkgs/os-specific/linux/syslinux/default.nix b/pkgs/os-specific/linux/syslinux/default.nix
index 0e0f4c945b6..e36a9274b65 100644
--- a/pkgs/os-specific/linux/syslinux/default.nix
+++ b/pkgs/os-specific/linux/syslinux/default.nix
@@ -3,10 +3,12 @@
 stdenv.mkDerivation {
   name = "syslinux-2019-02-06";
 
+  # This is syslinux-6.04-pre3^1; syslinux-6.04-pre3 fails to run.
+  # Same issue here https://www.syslinux.org/archives/2019-February/026330.html
   src = fetchFromRepoOrCz {
     repo = "syslinux";
-    rev = "05ac953c23f90b2328d393f7eecde96e41aed067";
-    sha256 = "07a9jfm5957daziq28vdnb21gnlfsvc85aqyf3fz8hxbpx1krl1l";
+    rev = "b40487005223a78c3bb4c300ef6c436b3f6ec1f7";
+    sha256 = "1qrxl1114sr2i2791z9rf8v53g200aq30f08808d7i8qnmgvxl2w";
   };
 
   patches = let

This can be tested using:

nix-build nixos/tests/boot.nix -A biosCdrom

Which ends hanging at:

machine # Booting from DVD/CD...
machine #
machine # ISOLINUX 6.04  ETCD Copyright (C) 1994-2015 H. Peter Anvin et al
machine #
machine # Failed to load ldlinux.c32
machine # Boot failed: press a key to retry...

Copy link
Member

@samueldr samueldr left a comment

Choose a reason for hiding this comment

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

(See other comment.)

@lblasc lblasc changed the title syslinux: 2015-11-09 -> 2019-02-06 syslinux: 2015-11-09 -> 2019-02-07 Jan 18, 2020
@ofborg ofborg bot requested a review from samueldr January 18, 2020 08:53
@lblasc
Copy link
Contributor Author

lblasc commented Jan 18, 2020

I missed #55500. With b40487005223a78c3bb4c300ef6c436b3f6ec1f7 and debian patches:
nix-build nixos/tests/boot.nix -A biosCdrom
works just fine.

I think this is good to go for staging.

@samueldr
Copy link
Member

Question: why staging? This is has a really limited rebuild impact. Additionally it doesn't have a wide-ranging impact that needs to be tested with other packages, mainly bootloader tests and the isos.

Am I missing something?

@samueldr samueldr dismissed their stale review January 18, 2020 21:05

(Waiting on the answer to my question, otherwise it tests fine)

@lblasc lblasc changed the base branch from staging to master January 18, 2020 22:44
@ofborg ofborg bot added 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: haskell 6.topic: nixos 6.topic: pantheon The Pantheon desktop environment 6.topic: python 6.topic: qt/kde 6.topic: TeX Issues regarding texlive and TeX in general labels Jan 18, 2020
@samueldr
Copy link
Member

@GrahamcOfBorg eval

Something's weird with the eval. Likely the branch switch-a-roo.

Copy link
Member

@samueldr samueldr left a comment

Choose a reason for hiding this comment

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

Oh, looks like your fixes as previously requested were lost. Compare with the previous force push. It's currently like the initial PR state.

  • Version number has reverted.
  • Revision has reverted.

Likely trivial to fix :).

@lblasc
Copy link
Contributor Author

lblasc commented Jan 19, 2020

I accidentally pushed the old branch, now it should be ok.

@lblasc lblasc requested a review from samueldr January 19, 2020 09:36
@samueldr
Copy link
Member

Made a small whitespace correction. Didn't have the heart to "reject" the PR for only a byte :).

@samueldr samueldr merged commit 3293835 into NixOS:master Jan 19, 2020
@samueldr
Copy link
Member

@vcunat I still don't know what happened the last time we bumped syslinux that needed it to be reverted. If things break again, this PR updated syslinux.

@ofborg ofborg bot requested a review from samueldr January 19, 2020 20:08
@vcunat
Copy link
Member

vcunat commented Jan 19, 2020

Last time syslinux itself failed to build. Now it does, so hopefully it will hold :-)

@lblasc lblasc deleted the syslinux branch January 19, 2020 20:55
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

3 participants