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.

Sign up for GitHub

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

opam: fix opam sandboxing on nixos #363770

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

opam: fix opam sandboxing on nixos #363770

wants to merge 1 commit into from

Conversation

eilvelia
Copy link
Contributor

@eilvelia eilvelia commented Dec 10, 2024 •
edited
Loading

Before this commit, executing opam init would display that sandboxing fails with "bwrap: execvp sh: No such file or directory". makeWrapper with --set OPAM_USER_PATH_RO /run/current-system/sw/bin:/nix/ had been used here to fix that, however, OPAM_USER_PATH_RO has been removed since 2021: ocaml/opam@9b6370d (released as opam 2.2.0 in 2024)

This also fixes a funny bug which caused the changelog link to be broken because of with lib:

meta = with lib; {                                                   
  description = "Package manager for OCaml";                         
  homepage = "https://opam.ocaml.org/";                              
  changelog = "https://github.com/ocaml/opam/raw/${version}/CHANGES";
  maintainers = [ ];                                                 
  license = licenses.lgpl21Only;                                     
  platforms = platforms.all;                                         
};

It uses meta = with lib;; at the same time, the derivation's attrset is not rec, and ${version} ends to be lib.version.

cc @kit-ty-kate

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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 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.

@eilvelia
Copy link
Contributor Author

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 363770

x86_64-linux

✅ 3 packages built:
  • dune-release
  • opam
  • opam.installer

kit-ty-kate
kit-ty-kate approved these changes Dec 10, 2024
View reviewed changes
Copy link
Contributor

@kit-ty-kate kit-ty-kate left a comment

Choose a reason for hiding this comment

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

i'm no nixos expert but from afar it looks reasonable

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Dec 10, 2024
@kit-ty-kate kit-ty-kate mentioned this pull request Dec 10, 2024
Merged
@kit-ty-kate
Copy link
Contributor

I've opened a PR upstream fixing this issue: ocaml/opam#6333
If you think this is the correct fix it will be part of the upcoming opam 2.4.

@vbgl vbgl added the backport release-24.11 Backport PR automatically label Dec 11, 2024
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/5027

@thiagokokada
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 363770

x86_64-linux

✅ 3 packages built:
  • dune-release
  • opam
  • opam.installer

@wegank wegank removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Dec 30, 2024
@eilvelia
opam: fix opam sandboxing on nixos, cleanup
5609319 
To make opam sandboxing (via bwrap) work on nixos, the following had been used
here:
  --set OPAM_USER_PATH_RO /run/current-system/sw/bin:/nix/
However, OPAM_USER_PATH_RO has been removed in opam 2.2.0, requiring a
new workaround: ocaml/opam@9b6370d
(Before this commit, executing `opam init` would display that sandboxing fails
with "bwrap: execvp sh: No such file or directory".)

- Removes outdated workarounds for ocp-build and argv0, cleans postInstall
- Fixes link to the changelog which was broken because of "with lib;"
@eilvelia
Copy link
Contributor Author

(The ofborg failure does not look related to this.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thiagokokada thiagokokada thiagokokada left review comments

@kit-ty-kate kit-ty-kate kit-ty-kate approved these changes

Assignees
No one assigned
Labels
6.topic: ocaml 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 backport release-24.11 Backport PR automatically
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@eilvelia @kit-ty-kate @nixos-discourse @thiagokokada @vbgl @wegank