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

efivar: fix build on i686, use clickable homepage #200916

Closed
wants to merge 1 commit into from

Conversation

SuperSandro2000
Copy link
Member

Description of changes
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@vcunat
Copy link
Member

vcunat commented Nov 12, 2022

Maybe at least do this conditionally, so that QA isn't lowered on the main platforms?

@SuperSandro2000
Copy link
Member Author

If the build would fail on x84_64 because of a warning and Werror we would ignore, too, so that wouldn't change much.

@vcunat
Copy link
Member

vcunat commented Nov 12, 2022

If the current error happened on x86_64-linux, I'd write a patch and submitted it upstream.

@SuperSandro2000
Copy link
Member Author

I wouldn't especially if it is some c specific memory overflow because I do not know anything about C except that you need to manually do memory management and there are lots of bugs about that. -Werror is for upstream developers and we drop it often because over time the build can fail especially if upstream also added -Wall.

@vcunat
Copy link
Member

vcunat commented Nov 12, 2022

Well sure, pure C is the main language in my paid job... but it depends on particular error – these looked easy to fix (at quick glance at least).

@vs49688
Copy link
Contributor

vs49688 commented Nov 13, 2022

Seems to be fixed upstream, am testing if it backports:
rhboot/efivar@15622b7

vs49688 added a commit to vs49688/nixpkgs that referenced this pull request Nov 13, 2022
@vs49688 vs49688 mentioned this pull request Nov 13, 2022
14 tasks
@vs49688
Copy link
Contributor

vs49688 commented Nov 13, 2022

Backport PR: #200964

@vcunat vcunat closed this Nov 13, 2022
@SuperSandro2000 SuperSandro2000 deleted the efivar-i686 branch November 15, 2022 20:42
rtimush pushed a commit to rtimush/nixpkgs that referenced this pull request Sep 21, 2023
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.

3 participants