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

firefox*: fix collisions due to identical binaryName #294971

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zeorin
Copy link
Contributor

@zeorin zeorin commented Mar 11, 2024

All nixpkgs Firefox builds are built with the same binaryName, and as a result these packages collide when installing the unwrapped packages, or even in the case of the wrapped packages, although the nameSuffix is set differently for firefox-beta, firefox-devedition, and firefox-esr-115, inside of the wrapper a ".${binaryName}-wrapper" file is created, and these still collide with one another.

In NixOS, this is not an issue because its call to buildEnv allows collisions, but in the case of Home Manager, its call to buildEnv does not, and so one cannot install multiple versions of these packages at the same time:

error: builder for '/nix/store/anwr04997ywn8qm1f2mcwav0d1qip8f5-home-manager-path.drv' failed with exit code 25;
       last 1 log lines:
       > error: collision between `/nix/store/in499wgsizvkzdvvrkykycvx7f0cq5dw-firefox-124.0b5/bin/.firefox-wrapped' and `/nix/store/zc4zrgmr1r2csm8xcwnlbvmmyrf6220z-firefox-123.0/bin/.firefox-wrapped'
       For full logs, run 'nix log /nix/store/anwr04997ywn8qm1f2mcwav0d1qip8f5-home-manager-path.drv'.
error: 1 dependencies of derivation '/nix/store/f06n5k5xr69bl54cfm4v98pxacciifn3-home-manager-generation.drv' failed to build
error: 1 dependencies of derivation '/nix/store/i1mfkn07ryf4j1cpkh6flj3sak2iwdwi-user-environment.drv' failed to build
error: 1 dependencies of derivation '/nix/store/aws7kf1a5jd129j4alj71mc8x7vlbs5x-etc.drv' failed to build
error: 1 dependencies of derivation '/nix/store/x8yvvb6kxp32pplclxk6wkiam7gcbz65-nixos-system-guru-23.11.20240301.79baff8.drv' failed to build

The -bin variants already have different binary names, as Mozilla builds them with different binary names, and thus for them these collisions don't occur.

Should the release notes include a note about the changed binary names?

Description of changes

I have adjusted the nixpkgs builds of Firefox to use different binary names.

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.

All `nixpkgs` Firefox builds are built with the same `binaryName`, and
as a result these packages collide when installing the unwrapped
packages, or even in the case of the wrapped packages, although the
`nameSuffix` is set differently for `firefox-beta`,
`firefox-devedition`, and `firefox-esr-115`, inside of the wrapper a
`".${binaryName}-old"` file is created, and _these_ still collide with
one another.

In NixOS, this is not an issue because the call to `buildEnv` allows
collisions, but in the case of Home Manager, its call to `buildEnv` does
not, and so one cannot install multiple versions of these packages at
the same time.

The `-bin` variants already have different binary names, as Mozilla
builds them with different binary names. I have adjusted the `nixpkgs`
builds of Firefox to use different binary names.
Copy link
Contributor

@l0b0 l0b0 left a comment

Choose a reason for hiding this comment

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

Idea & code changes LGTM; haven't compiled or tested it. Thank you!

Comment on lines 65 to +66
pname = "firefox-devedition";
binaryName = "firefox-developer-edition";
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
pname = "firefox-devedition";
binaryName = "firefox-developer-edition";
pname = "firefox-developer-edition";

And I think we could probably fallback to pname if no binaryName is set, since now most of them are matching

@@ -94,6 +96,7 @@

firefox-esr-115 = buildMozillaMach rec {
pname = "firefox-esr-115";
binaryName = "firefox-esr";
Copy link
Member

@mweinelt mweinelt Jun 29, 2024

Choose a reason for hiding this comment

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

There is a short time when two ESR versions will overlap. Would be good to disambiguate them through their major version.

@zeorin zeorin closed this Jul 10, 2024
@zeorin zeorin deleted the fix-firefox-collisions branch July 10, 2024 08:55
@zeorin zeorin restored the fix-firefox-collisions branch July 10, 2024 09:11
@zeorin
Copy link
Contributor Author

zeorin commented Jul 10, 2024

Was pruning branches and accidentally deleted this branch on my repo…

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

Successfully merging this pull request may close these issues.

None yet

6 participants