musly: add patches for FFmpeg 7, C++17, and external deps; kissfft: build with CMake; libresample: 0.1.3 -> 0.1.4-unstable-2024-08-23#332035
Conversation
ofborg
bot
added
10.rebuild-darwin: 1-10
wegank
added
12.approvals: 1
|
Against all odds, libresample upstream… is accepting my pull request to add a complete rewrite of the build system! Drafting this for now until I can get it merged and integrated. |
This builds fine with `pkgsCross.mingwW64`.
This should work in principle, but fails in practice.
emilazy
force-pushed
the
push-lqmvmtqtsnuo
branch
from
88e2faf to
95459e7
Compare
emilazy
changed the title
emilazy
force-pushed
the
push-lqmvmtqtsnuo
branch
from
95459e7 to
4c51454
Compare
|
Updated to add myself as a Marking this as ready for review because I don’t want to block FFmpeg 4 work on |
|
Result of 18 packages built:
Result of 15 packages built:
|
There was a problem hiding this comment.
Tbis is the default and should explicitly not be added to reduce noise
There was a problem hiding this comment.
Please see my previous response (and the one I linked last time with more thoughts from others). It adds value, does no harm, and the argument that it is redundant also applies to other possible settings of sourceProvenance. Nobody objects to the equivalent license = lib.licenses.free;. (Anyway, just like licence information and main programs, we should be moving in the direction of requiring explicit sourceProvenance for all packages, not away from it.)
There was a problem hiding this comment.
We add or licenses as both
There was a problem hiding this comment.
As far as I can tell, this would make any software bill of materials for a Nix project using libresample strictly less useful, given that LGPL 2.1+’s requirements are a superset of BSD3’s, and there are examples of doing it the other way in‐tree (I suspect many dual licenses just aren’t represented at all, actually). However if you want me to make it less useful I can.
There was a problem hiding this comment.
Right now meta.licenses is always the collection of all licenses the project uses and we don't reflect the details. Anything else is done wrong.
There was a problem hiding this comment.
I’m not sure you understand how dual licensing works: any downstream can pick any licence, at their option. libresample can be used entirely under the terms of a BSD licence without any consideration of the LGPL terms. Therefore simply declaring BSD is entirely correct: Nixpkgs has the option to choose that licence, is exercising it, and neither Nixpkgs nor any package depending on this library is under any LGPL‐related obligations.
Consider a program that is available under either the GPL or a proprietary, non‐free licence. There are many historical examples of this, for instance Qt. Do you really think that it ought to be declared licenses = [ lib.licenses.gpl3Only lib.licenses.unfreeRedistributable ] and flag up unfree warnings, when the whole point is that anyone who receives the software can choose to use it under the terms of the GPL?
There was a problem hiding this comment.
No, it’s the correct commit. The tools were not being built before.
|
Tried musly with a bunch of audio files in the following way: and it seems to work fine. |
emilazy
force-pushed
the
push-lqmvmtqtsnuo
branch
from
4c51454 to
ea188a6
Compare
|
Result of 1 package marked as broken and skipped:
14 packages built:
|
|
Looks ok to me, although that's a lot of patches to maintain. My only gripe is that I think we shouldn't be merging branches with random names. |
|
The The The The branch name is a Jujutsu change ID and makes pushing out changes a lot easier for me. I know I’m not the only Nixpkgs contributor using Jujutsu, although I may be the most prolific. Agreed that they don’t make for fantastic merge commit subject lines, but scanning the merge log of Nixpkgs is kind of hopeless anyway, and I’ve seen many merged PRs with manually‐written branch names that are somewhere between unhelpful and actively inaccurate. The PR title is included in the second line of the merge commit messages, which is a much more sensible way to browse the merge history, but I do wish that GitHub would include it in the subject line instead. If you’re implacably opposed I can attach another branch name, but it would require closing this PR and opening another so I’m not sure it’s exactly worth it. |
No, it would be pointless. I don't have strong feelings in this case. We can merge this. Now, if you ask me about that jj tool design choices (which I didn't know), then I would start arguing that having developed a tool that puts random branches names is not so useful and not very Git-compatible in most other contexts 🤭 |
|
Back to a draft again as the |
The Debian CMake build system rewrite patch was broken and generated an unusable `.pc` file, so I rewrote it again in Meson. This was accepted upstream, but is still awaiting a final release. The licence also changed to BSD-2-Clause OR LGPL-2.1-or-later in 2013.
Fix the library install names on macOS and replace a patch with a smaller one.
Please use `pkg-config`, people!
emilazy
changed the title
emilazy
force-pushed
the
push-lqmvmtqtsnuo
branch
from
ea188a6 to
7245ee2
Compare
|
Okay, I don’t know how long it’ll be before upstream cuts a release and this is one of the few things remaining for the big FFmpeg 4 PR so I’ve just bumped it to drop the vendored Meson patch for libresample but keeping the smaller tests one for now. Now it just needs merging before any further developments happen! |
|
Result of 1 package marked as broken and skipped:
14 packages built:
|
5 participants
Description of changes
More time spent on software that probably nobody cares about at this point. One fewer FFmpeg 4 dependency in the world, and a couple more packages that work on macOS!
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.