Skip to content

links2: fix darwin build#288767

Merged
7c6f434c merged 1 commit intoNixOS:masterfrom
afh:fix-darwin-links2
Feb 14, 2024
Merged

links2: fix darwin build#288767
7c6f434c merged 1 commit intoNixOS:masterfrom
afh:fix-darwin-links2

Conversation

@afh
Copy link
Member

@afh afh commented Feb 14, 2024

Description of changes

  • Modernize package by using finalAttrs instead of rec and hash instead of sha256
  • Disable implicit-int warning to fix failing configurePhase thinking compiler is unable to produce binaries due to implicit int being disallowed in C99 and above.
  • Add libavif to buildInputs to fix failing configurePhase due to avif.h header not being found
  • Disable graphics on Darwin to fix failing configurePhase due to graphics drivers not being found

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 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
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/3439

@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Feb 14, 2024
@ofborg ofborg bot requested a review from 7c6f434c February 14, 2024 11:04
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Feb 14, 2024
@7c6f434c 7c6f434c merged commit 174811a into NixOS:master Feb 14, 2024
@afh afh deleted the fix-darwin-links2 branch February 14, 2024 18:11
@afh
Copy link
Member Author

afh commented Feb 14, 2024

Thanks for merging @7c6f434c, much appreciated! 🙂

src = fetchurl {
url = "${meta.homepage}/download/links-${version}.tar.bz2";
sha256 = "sha256-IqqWwLOOGm+PftnXpBZ6R/w3JGCXdZ72BZ7Pj56teZg=";
url = "${finalAttrs.meta.homepage}/download/links-${finalAttrs.version}.tar.bz2";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
url = "${finalAttrs.meta.homepage}/download/links-${finalAttrs.version}.tar.bz2";
url = "http://links.twibright.com/download/links-${finalAttrs.version}.tar.bz2";

using meta.homepage here is not very correct or future proof. If meta.homepage gets changed in the future this will likely be forgotten

Copy link
Member Author

Choose a reason for hiding this comment

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

That makes sense, @SuperSandro, is this important enough to you, that you'd accept a PR for it or it is something that should rather be addressed with the next update to the package?

Copy link
Member

Choose a reason for hiding this comment

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

feel free to make PR now, me or @SuperSandro2000 happy to have a look at it

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure thing, @kirillrdy, here's the PR: #289130

Copy link
Member

Choose a reason for hiding this comment

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

If meta.homepage gets changed in the future this will likely be forgotten

Note that if the domain changes in the future, the URL 100% needs changing, but there is a chance that the URL structure is the same, so I am not sure how this cleanup can ever be useful.

Copy link
Member

Choose a reason for hiding this comment

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

this is similar to #277994

for links2 its most likely that url and homepage will go in sync, but that's not the case for a lot of other derivations

eg homepage can change to be different domain but src remains on github, or sometimes they go backwards

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: darwin Running or building packages on Darwin 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants