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

applicationName is now binaryName in upstream firefox #290

Closed
wants to merge 2 commits into from

Conversation

marcenuc
Copy link

I have been using this patch for a couple of days to get firefox nightly working in NixOS with the nixpkgs-unstable channel.

The error is the same as in #262, but I think it is related to https://github.com/NixOS/nixpkgs/pull/165161/files.

This is the snippet of /etc/nixos/configuration.nix that I am using:

{ config, pkgs, ... }:
{ # ...
  nixpkgs.config.allowUnfree = true;
  nixpkgs.overlays =
    let
      moz-rev = "master";
      moz-url = builtins.fetchTarball { url = "https://github.com/marcenuc/nixpkgs-mozilla/archive/${moz-rev}.tar.gz";};
      nightlyOverlay = (import "${moz-url}/firefox-overlay.nix");
    in [
      nightlyOverlay
    ];
  environment.systemPackages = with pkgs; [
    latest.firefox-nightly-bin
    # ...
  ];
  # ...
}

It looks like NixOS/nixpkgs#165161 changed the attribute name.
This is an attempt to update the overlay to use the newer name.
Try with applicationName
@Artturin
Copy link
Contributor

#289

@marcenuc
Copy link
Author

@Artturin Sorry for the duplicate! I searched for it but, somehow, I failed to find it.

I see that your patch is different. But I'm new to Nix, and then I do not know if your fix is better than mine. Do you have any suggestions?

@@ -157,7 +157,9 @@ let
+ optionalString (96 >= getMajorVersion version) (":" + makeLibraryPath [self.xorg.libXtst]);
})) {
${
if super.firefox-unwrapped ? applicationName then
if super.firefox-unwrapped ? binaryName then
"applicationName"

Choose a reason for hiding this comment

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

You meant "binaryName" here, I guess?

Copy link
Contributor

Choose a reason for hiding this comment

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

@ilya-fedin No, "applicationName" is correct. The parameter is still called applicationName; it’s just now passed through as binaryName, breaking the previous detection.

Copy link
Contributor

Choose a reason for hiding this comment

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

Though #284 has a better, less confusing detection strategy that also fixes this as a side effect.

Choose a reason for hiding this comment

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

The parameter is still called applicationName;

Oh, I see now

@marcenuc
Copy link
Author

Closing because #284 fixes it.

@marcenuc marcenuc closed this Apr 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants