`ffmpeg` -> `frei0r` -> `opencv` -> `ffmpeg` circular dependency

Describe the bug

ffmpeg optionally depends on frei0r if the withFrei0r option is set (or if withFullDeps is set or if using the ffmpeg-full variant)
frei0r depends on opencv
opencv depends on ffmpeg

This means that

ffmpeg-full depends on ffmpeg, so you essentially have to build ffmpeg twice
You can't override the default ffmpeg to be ffmpeg-full (or any variation of ffmpeg that sets withFrei0r = true)

Steps To Reproduce

Steps to reproduce the behavior:

Apply the following overlay to nixpkgs:

final: prev: {
  ffmpeg = prev.ffmpeg.override { withFrei0r = true; };
  # or
  # ffmpeg = prev.ffmpeg-full;
};

Try to build ffmpeg

Observe "infinite recursion encountered" error:

error:
       … while evaluating derivation 'opencv-4.9.0'
       … while evaluating attribute 'buildInputs' of derivation 'opencv-4.9.0'
       … while evaluating derivation 'ffmpeg-6.1.1'
       … while evaluating attribute 'buildInputs' of derivation 'ffmpeg-6.1.1'
       … while evaluating derivation 'frei0r-plugins-2.3.2'
       … while evaluating attribute 'buildInputs' of derivation 'frei0r-plugins-2.3.2'
       error: infinite recursion encountered

Expected behavior

Build succeeds. I am honestly not sure, how this circular dependency could be avoided, though.

Notify maintainers

@Atemu @arthsmn @jopejoe1 @cillianderoiste @basvandijk

Add a :+1: reaction to issues you find important.

Thats one of the reasons why we have multiple ffmpeg tiers (another one would be size), and this is just one of a few dependencie circles ffmpeg is involved in another one would be chromaprint.

this is just one of a few dependencie circles ffmpeg is involved in another one would be chromaprint.

Are you sure? I was able to fix the frei0r circular dependency by doing

final: prev: {
  ffmpeg = final.ffmpeg-full.override {
    frei0r = final.frei0r.override {
      opencv = final.opencv.override {
        ffmpeg = prev.ffmpeg;
      };
    };
  };
  # instead of
  # ffmpeg = final.ffmpeg-full;
}

It seems that chromaprint actually explicitly depends on ffmpeg_6 instead of ffmpeg, so there is no infinite recursion in that case (although it still ends up including two instances of ffmpeg in its closure).

Of course, this isn't a very nice fix, because the resulting packages end up building and using multiple instances of ffmpeg/opencv. I wonder what is the idiomatic way to fix such issues in nix/nixpkgs?

final: prev: {
  ffmpeg = final.ffmpeg-full.override {
    frei0r = final.frei0r.override {
      opencv = final.opencv.override {
        ffmpeg = prev.ffmpeg;
      };
    };
  };
  # instead of
  # ffmpeg = final.ffmpeg-full;
}

It ought to be enough to just ffmpeg-full.override { frei0r = prev.frei0r; }.

It seems that chromaprint actually explicitly depends on ffmpeg_6 instead of ffmpeg, so there is no infinite recursion in that case (although it still ends up including two instances of ffmpeg in its closure).

That might be the case currently but we strive to keep everything using the maximum version of ffmpeg.

Of course, this isn't a very nice fix, because the resulting packages end up building and using multiple instances of ffmpeg/opencv.

You need at minimum two ffmpeg instances in any case: A "bootstrap" instance and the actual instance.

We already do this: In the case of ffmpeg-full, the "bootstrap ffmpeg" is ffmpeg. That's why overriding ffmpeg = ffmpeg-full causes a cycle.
We actually do this twice as some deps of the regular (non-full) ffmpeg variant also depend on ffmpeg. That's what ffmpeg-headless was invented for.

So what you actually want to do is overlay the "bootstrap ffmpeg" and ffmpeg separately and then pass the bootstrap instance to any dependency of ffmpeg which itself depends on ffmpeg.

I've been meaning to make opencv and friends depend on ffmpeg-headless too such that ffmpeg-full does not depend on ffmpeg and ffmpeg-headless thereby becomes this "bootstrap ffmpeg". I'm not sure whether these themselves require deps only present in non-headless ffmpeg though and haven't found the time yet.

I wonder what is the idiomatic way to fix such issues in nix/nixpkgs?

There is no idiomatic fix for this. Circular dependencies are not allowed period.

That's why bootstrap is quite complex.

It ought to be enough to just ffmpeg-full.override { frei0r = prev.frei0r; }.

No, it seems that

overlay = final: prev: {
  ffmpeg = final.ffmpeg-full.override {
    frei0r = prev.frei0r;
  };
};

still causes an infinite recursion. AFAIK, this happens because callPackage essentially uses the packages from final when filling the dependency arguments. This means that prev.frei0r is the "previous" version of the package definition itself, but it still receives final.opencv (and final.ffmpeg) as its inputs.

I guess, the actual "correct" way to do this would be something like

final: prev: {
  ffmpeg = final.ffmpeg-full.override (prev1: {
    frei0r = prev1.frei0r.override (prev2: {
      opencv = prev2.opencv.override {
        ffmpeg = prev.ffmpeg;
      };
    });
  });
}

so that only the innermost ffmpeg is actually pinned, and any other overrides applied to frei0r inside ffmpeg and opencv inside frei0r aren't lost. But realistically, this won't matter in 99% of cases. You could also just overwrite the top-level opencv derivation.

final: prev: {
  ffmpeg = final.ffmpeg-full;
  opencv = prev.opencv.override { ffmpeg = prev.ffmpeg; };
}

So what you actually want to do is overlay the "bootstrap ffmpeg" and ffmpeg separately and then pass the bootstrap instance to any dependency of ffmpeg which itself depends on ffmpeg.

Isn't this a bit problematic, since you'd need to manually enumerate all the relevant packages, no?

From the names of ffmpeg-headless, ffmpeg and ffmpeg-full (and from how ffmpeg is normally packaged in other distros), I was under the impression that these variants of ffmpeg were all "user-facing" variations with different feature sets.

Instead, it seems that these names were also co-opted for the bootstrapping process (which is arguably an internal implementation detail of nixpkgs). It seems to me, that the bootstrapping logic and the user-facing ffmpegVariant classification ought to be decoupled.

So instead of

graph RL;
    bd{{Minimal Dependencies}};
    bd --> FH[ffmpeg-headless];

    FH --> gd{{GUI Dependencies}};
    gd --> FN[ffmpeg];
    bd --> FN;

    FN --> fd{{Full Dependencies}};
    fd --> FF[ffmpeg-full];
    gd --> FF;
    bd --> FF;

the dependency graph should look something like this:

graph RL;
    bd{{Minimal Dependencies}};
    bd --> FB[ffmpeg-internal-bootstrap];
    bd ----> FH[ffmpeg-headless];

    FB --> gd{{GUI Dependencies}};
    gd --> FN[ffmpeg];
    bd --> FN;

    FB --> fd{{Full Dependencies}};
    fd --> FF[ffmpeg-full];
    gd --> FF;
    bd --> FF;

    style FB stroke-dasharray: 5 5;

Where ffmpeg-internal-bootstrap is a special derivation that should only be used by packages like frei0r and chromaprint that can't be linked to the "proper" version of ffmpeg due to dependency cycles.

This way, the user can overwrite/override the user-facing ffmpeg-headless, ffmpeg and ffmpeg-full derivations without breaking the bootstrapping logic. Also, in this case, you should get at most one "extra" instance of ffmpeg instead of two.

I've been meaning to make opencv and friends depend on ffmpeg-headless

Having a separate "headless" variant might still be useful for end users (e.g. for ultra light server/embedded deployments). It might be better to keep ffmpeg-headless and the (hypothetical) ffmpeg-internal-bootstrap as separate names, even if ffmpeg-headless is essentially just an alias for ffmpeg-internal-bootstrap (unless the user overwrites/overrides it, of course).

From the names of ffmpeg-headless, ffmpeg and ffmpeg-full (and from how ffmpeg is normally packaged in other distros), I was under the impression that these variants of ffmpeg were all "user-facing" variations with different feature sets.

Instead, it seems that these names were also co-opted for the bootstrapping process (which is arguably an internal implementation detail of nixpkgs). It seems to me, that the bootstrapping logic and the user-facing ffmpegVariant classification ought to be decoupled.

It's both: User-facing and bootstrapping.

While it was originally born out of the necessity for bootstrap, ffmpeg-headless is also useful to minimise closure size.
A server deployment has no need for i.e. SDL. This is also relevant for components that depend on ffmpeg/libav such as e.g. a service that generates thumbnails; such applications do not need to be able to capture Xorg windows.

Also, in this case, you should get at most one "extra" instance of ffmpeg instead of two.

That is not caused by separating the ffmpeg-bootstrap but rather by using the same instance of ffmpeg to build the regular ffmpeg's and ffmpeg-full's deps.

In fact, this would cause you to require an additional instances of ffmpeg in closures where ffmpeg-headless is present.

It might be better to keep ffmpeg-headless and the (hypothetical) ffmpeg-internal-bootstrap as separate names, even if ffmpeg-headless is essentially just an alias for ffmpeg-internal-bootstrap (unless the user overwrites/overrides it, of course).

I had that idea in my head too. I see it as the best solution to your little problem here.

Making ffmpeg-bootstrap = ffmpeg-headless would also prevent the +1 instance issue I mentioned.

Making ffmpeg-bootstrap = ffmpeg-headless would also prevent the +1 instance issue I mentioned.

It seems that we're in agreement then. One small thing though: I think, the alias should be the other way around (ffmpeg-headless = ffmpeg-bootstrap), so that overwriting/overriding ffmpeg-headless wouldn't accidentally break the bootstrapping. Or maybe even completely decoupled (headless = mkFFmpeg "headless"; bootstrap = mkFFmpeg "headless") despite evaluating to the same derivation by default.

NixOS / nixpkgs