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

Jump to bottom

linuxPackages.e1000e: 3.3.5.3 -> 3.8.4 #91444

Merged
merged 1 commit into from Jul 6, 2020
Merged

linuxPackages.e1000e: 3.3.5.3 -> 3.8.4 #91444

merged 1 commit into from Jul 6, 2020

Conversation

lorenzleutgeb
Copy link
Member

@lorenzleutgeb lorenzleutgeb commented Jun 24, 2020 •
edited

Upgrade e1000e from version 3.3.5.3 to 3.8.4

Motivation for this change

I got a new laptop that requires a newer version of e1000e, and I saw that the derivation hasn't been updated in a while.

Things done

I am a new NixOS user and struggling to test this.

  • 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.

@lorenzleutgeb lorenzleutgeb changed the title os-specifix/linux/e1000e: 3.3.5.3 -> 3.8.4 os-specific/linux/e1000e: 3.3.5.3 -> 3.8.4 Jun 24, 2020
@nh2
Copy link
Contributor

nh2 commented Jun 28, 2020

@GrahamcOfBorg build

@nh2
Copy link
Contributor

nh2 commented Jun 28, 2020

Result of nixpkgs-review pr 91444 1

2 packages failed to build:
- linuxPackages_4_4.e1000e
- linuxPackages_4_9.e1000e

@nh2
Copy link
Contributor

nh2 commented Jun 28, 2020

I get this when building with nixpkgs-review above:

  source root is e1000e-3.8.4
  setting SOURCE_DATE_EPOCH to timestamp 1584260392 of file e1000e-3.8.4/SUMS
  patching sources
  configuring
  building
  build flags: -j12 -l12 SHELL=/nix/store/5hplxs3kprdp6v62qk3di6a1m1s1paiq-bash-4.4-p23/bin/bash BUILD_KERNEL=4.9.228
  common.mk:85: *** Kernel header files not in any of the expected locations.
  common.mk:86: *** Install the appropriate kernel development package, e.g.
  common.mk:87: *** kernel-devel, for building kernel modules and try again.  Stop.
  builder for '/nix/store/d1nvrxcj25g41qxqhz4dnfn261xmcdzh-e1000e-3.8.4-4.9.228.drv' failed with exit code 2

@lorenzleutgeb
Copy link
Member Author

lorenzleutgeb commented Jun 29, 2020 •
edited

Thank you for your feedback. I am new to NixOS, and this is my first (tentative) contribution to nixpkgs, so I appreciate your guiding comments.

To recreate the failure on my machine I used nixpkgs-review. I then amended my earlier commit to fix the issue, and ran nixpkgs-review rev HEAD which resulted in:

...
2 packages updated:
linuxPackages_4_4.e1000e (3.3.5.3-4.4.228 → 3.8.4-4.4.228) ↩
linuxPackages_4_9.e1000e (3.3.5.3-4.9.228 → 3.8.4-4.9.228)
...

(Newline added by me, for readbility.)

I interpreted this output as an indication that my fix indeed works and force-pushed. Please check again.

Another (slightly off-topic) question: I am running Linux 5.7.4 (via boot.kernelPackages = pkgs.linuxPackages_latest) and am wondering whether my changes to e1000e will be available with Linux 5.7.4 as well, since the output (pasted above) only talks about 4.4 and 4.9. Probably I do not understand how pkgs.linuxPackages_* is constructed well enough. Could you please give me some direction for my research?

@lorenzleutgeb lorenzleutgeb changed the title os-specific/linux/e1000e: 3.3.5.3 -> 3.8.4 linuxPackages.e1000e: 3.3.5.3 -> 3.8.4 Jun 29, 2020
danieldk
danieldk suggested changes Jul 6, 2020
View reviewed changes
Copy link
Contributor

@danieldk danieldk left a comment

Choose a reason for hiding this comment

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

Thank you for your first submission 👍 . Sorry that you have to wait so long for progress on this PR. I have added one suggested change.

Feel free to re-request a review once you have made the changes.

pkgs/os-specific/linux/e1000e/default.nix Outdated Show resolved Hide resolved
@lorenzleutgeb
linuxPackages.e1000e: 3.3.5.3 -> 3.8.4
587bb6e 
Upgrade e1000e from version 3.3.5.3 to 3.8.4
danieldk
danieldk approved these changes Jul 6, 2020
View reviewed changes
Copy link
Contributor

@danieldk danieldk left a comment

Choose a reason for hiding this comment

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

LGTM

Result of nixpkgs-review pr 91444 1

2 packages built:
- linuxPackages_4_4.e1000e
- linuxPackages_4_9.e1000e

I can't test the module, since I do not have the hardware. But since this is directly coming from the vendor it should be fine.

@danieldk danieldk merged commit 4f5c36b into NixOS:master Jul 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danieldk danieldk danieldk approved these changes

Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 1-10 12. first-time contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@lorenzleutgeb @nh2 @danieldk @Janik-Haag