Skip to content

ffmpeg: make version part of the interface#280904

Closed
Atemu wants to merge 947 commits intoNixOS:masterfrom
Atemu:ffmpeg-version-interface
Closed

ffmpeg: make version part of the interface#280904
Atemu wants to merge 947 commits intoNixOS:masterfrom
Atemu:ffmpeg-version-interface

Conversation

@Atemu
Copy link
Copy Markdown
Member

@Atemu Atemu commented Jan 14, 2024

Description of changes

This allows overriders to declare a different ABI. Doing so via overrideAttrs
would not affect the flags in the arguments; effectively retaining the
overridden package's ABI.
See #280645 for an instance of that.

By overriding the arguments using

ffmpeg.override { version = "..."; ... }

the ABI will now be overridden as expected.

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.

cc @anund

@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. labels Jan 14, 2024
@anund
Copy link
Copy Markdown
Contributor

anund commented Jan 14, 2024

Should this reference also be version? I don't currently understand how finalAttrs works in this context.
https://github.com/NixOS/nixpkgs/blob/a5ff0757c8aa0b4be88c866d87ee9824dc9a5239/pkgs/development/libraries/ffmpeg/generic.nix#L343-L347

@delroth delroth added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jan 14, 2024
@Atemu
Copy link
Copy Markdown
Member Author

Atemu commented Jan 14, 2024

I'd almost prefer for it not to be but, really, you should be overriding src anyways if you're overriding the version. The hash won't match anyways.

I guess we could also make hash and src an explicit part of the interface; making it fetch ffmpeg versions from the upstream repo.

mweinelt added 18 commits March 12, 2024 18:24
@jopejoe1
Copy link
Copy Markdown
Member

I think version and src should be part of the interface and hash should be removed from it.

@Atemu
Copy link
Copy Markdown
Member Author

Atemu commented Mar 13, 2024

Why?

Having the hash in there has the nice property that one could make the ffmpeg drv build any upstream version by specifying version and hash as an override argument.

@Atemu Atemu marked this pull request as draft March 13, 2024 12:19
@Atemu
Copy link
Copy Markdown
Member Author

Atemu commented Mar 13, 2024

(I've overhauled this in a local tree that I'm still having some issues with, drafting for now.)

@jopejoe1
Copy link
Copy Markdown
Member

Why?

I think that having an src part of the interface would make it redundant as with src you could specify any srource and not just versions avaivable upstream.

python311Packages.radios: 0.3.0 -> 0.3.1
@Atemu
Copy link
Copy Markdown
Member Author

Atemu commented Mar 13, 2024

You could still specify any source if src was part of the interface; you'd just override it.

Atemu added 4 commits March 13, 2024 19:49
This effectively obsoletes extraPatches
This is (to my knowledge) a novel pattern that is similar to how
callPackages (note the s) is used. Whereas callPackages would try to make the
arguments of default.nix overrideable, this instead exposes the individual
packages' arguments.

This pattern is a lot more robust than the custom import pattern I had
implemented here before.

It also moves the implementation detail out of all-packages which is great.

Making the version part of the interface allows overriders to declare a
different ABI. Doing so via overrideAttrs would not affect the flags in the
arguments; effectively retaining the overridden package's ABI.

See NixOS#280645 for an instance of that.

By overriding the arguments using

    ffmpeg.override { version = "..."; ... }

the ABI will now be overridden as expected.

This means you could theoretically turn ffmpeg_5-full into ffmpeg_4-headless by overriding it with

    {
      version = "4.4.4";
      hash = ...;
      ffmpegVariant = "headless";
    }

Having these implicit parameters be explicit parameters feels a lot cleaner and
neater to work with.
You should use the override interface to modify ABI version, not overrideAttrs.
@Atemu Atemu force-pushed the ffmpeg-version-interface branch from a5ff075 to 4033f2b Compare March 13, 2024 19:30
@Atemu
Copy link
Copy Markdown
Member Author

Atemu commented Mar 13, 2024

It's a draft, why did it do the ping? Urrrggghhh.

@Atemu Atemu closed this Mar 13, 2024
@NixOS NixOS locked and limited conversation to collaborators Mar 13, 2024
@github-actions github-actions bot added 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: haskell General-purpose, statically typed, purely functional programming language 8.has: module (update) This PR changes an existing module in `nixos/` 6.topic: emacs Text editor 6.topic: rust General-purpose programming language emphasizing performance, type safety, and concurrency. 6.topic: golang Go is a high-level general purpose programming language that is statically typed and compiled. 6.topic: vim Advanced text editor 6.topic: ocaml OCaml is a general-purpose, high-level, multi-paradigm programming language. 6.topic: stdenv Standard environment 6.topic: nodejs Node.js is a free, open-source, cross-platform JavaScript runtime environment 6.topic: lua Lua is a powerful, efficient, lightweight, embeddable scripting language. 6.topic: jupyter Interactive computing tooling: kernels, notebook, jupyterlab labels Mar 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

2.status: merge conflict This PR has merge conflicts with the target branch 6.topic: emacs Text editor 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: golang Go is a high-level general purpose programming language that is statically typed and compiled. 6.topic: haskell General-purpose, statically typed, purely functional programming language 6.topic: jupyter Interactive computing tooling: kernels, notebook, jupyterlab 6.topic: lua Lua is a powerful, efficient, lightweight, embeddable scripting language. 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: nodejs Node.js is a free, open-source, cross-platform JavaScript runtime environment 6.topic: ocaml OCaml is a general-purpose, high-level, multi-paradigm programming language. 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: rust General-purpose programming language emphasizing performance, type safety, and concurrency. 6.topic: stdenv Standard environment 6.topic: vim Advanced text editor 8.has: module (update) This PR changes an existing module in `nixos/` 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. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.