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

Jump to bottom

Drop spidermonkey from aliases #100026

Merged
merged 1 commit into from Oct 8, 2020
Merged

Drop spidermonkey from aliases #100026

merged 1 commit into from Oct 8, 2020

Conversation

ardumont
Copy link
Contributor

@ardumont ardumont commented Oct 8, 2020 •
edited

It's already aliased in all-packages.

This creates issues at least for the mediatomb build.

Related to #93450#issuecomment-705408544

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs: nix-shell -p nixpkgs-review --run "nixpkgs-review pr 100026" [1]
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

[1]

nixpkgs-review pr 100026
$ git -c fetch.prune=false fetch --force https://github.com/NixOS/nixpkgs master:refs/nixpkgs-review/0 pull/100026/head:refs/nixpkgs-review/1
From https://github.com/NixOS/nixpkgs
 + 2c4cf27d61b...b1062d4dcda refs/pull/100026/head -> refs/nixpkgs-review/1  (forced update)
$ git worktree add /home/tony/.cache/nixpkgs-review/pr-100026/nixpkgs b50ef4e3f226dde6c34817e3f92ca0aa3e18a72e
Preparing worktree (detached HEAD b50ef4e3f22)
Updating files: 100% (22856/22856), done.
HEAD is now at b50ef4e3f22 ocamlPackages.fmt: 0.8.8 -> 0.8.9
$ nix-env -f /home/tony/.cache/nixpkgs-review/pr-100026/nixpkgs -qaP --xml --out-path --show-trace
$ git merge --no-commit b1062d4dcdafdcdb36ad5939f2c3a438eb02e902
Auto-merging pkgs/top-level/all-packages.nix
Automatic merge went well; stopped before committing as requested
$ nix-env -f /home/tony/.cache/nixpkgs-review/pr-100026/nixpkgs -qaP --xml --out-path --show-trace --meta
2 packages added:
mediatomb (init at 0.12.1) spidermonkey (init at 68.10.0)

1 package removed:
spidermonkey (†68.10.0)

$ nix build --no-link --keep-going --option build-use-sandbox relaxed -f /home/tony/.cache/nixpkgs-review/pr-100026/build.nix
building '/nix/store/h5ccfai75h93jjw47wm02spnlba9kqlp-env.drv'...
[1 built, 0.0 MiB DL]
https://github.com/NixOS/nixpkgs/pull/100026
2 packages built:
mediatomb spidermonkey

[0.0 MiB DL]
error: build log of '/nix/store/2k0v219m2ilrq7y1g559mpvffxn15ypl-spidermonkey-68.10.0.drv' is not available
$ nix-shell /home/tony/.cache/nixpkgs-review/pr-100026/shell.nix

@ardumont
Drop spidermonkey from aliases
b1062d4 
It's already aliased in all-packages.

This creates issues at least for the mediatomb build.

Related to NixOS#93450#issuecomment-705408544
@timokau timokau merged commit b836d19 into NixOS:master Oct 8, 2020
@ardumont ardumont mentioned this pull request Oct 8, 2020
Merged
10 tasks
@ardumont ardumont deleted the drop-spidermonkey-alias branch October 8, 2020 15:37
@mkg20001
Copy link
Member

mkg20001 commented Oct 8, 2020

The idea was to have it as a "people that need a nix-shell with spidermonkey" thing for example, yet not have it get used for packages that get built, since spidermonkey isn't ABI upwards-compatible thus breaking stuff on every update

Putting it in aliases.nix means packages can't use it, which makes it obvious it's not meant to

Having it in all-packages was a bug from a rebase

@timokau
Copy link
Member

timokau commented Oct 8, 2020

That makes sense. I'm not entirely sure its the best way to do it, since it makes it very difficult to deprecate old versions. You know much more about that than I though. I merged this without waiting for input since it fixed a bug for now. Feel free to move it back to aliases.nix of course (although a comment that explains the reasoning would be good I think).

@mkg20001 mkg20001 mentioned this pull request Oct 9, 2020
Merged
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
8.has: package (new) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ardumont @mkg20001 @timokau