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

resholved: init at hopes and dreams #85827

Open
wants to merge 1 commit into
base: master
from
Open

Conversation

@abathur
Copy link
Contributor

abathur commented Apr 23, 2020

Motivation for this change

Give the Nix ecosystem a new superpower: resolving external dependencies in shell scripts.

Shell scripts/libraries with external dependencies are an odd duck in the Nix ecosystem. Packaging them with Nix doesn't ensure dependencies are available at runtime or address the possibility that the script could run executables provided by packages other than those specified as dependencies, for example.

More information

This PR packages a project of mine named "resholved", and provides a Nix build function. Resholved is itself built atop the parser from the Oil shell project. A fair portion of the Nix code in this PR is just packaging/building a development version of Oil.

The PR itself isn't terribly useful for seeing what it does; it'll help to look at some additional resources:

  1. I repackaged two existing shell projects in Nixpkgs with resholved (these include a copy of the resolved script from the Nix store for reference):
  2. The resholved repo README also includes two demos that show how the underlying script handles some situations and demonstrate how the Nix build function can be used to build shell projects that depend on other shell projects. You can also see these in a CI run at:

FWIW, I dogfood resholved in my own darwin-configuration, where it is responsible for building a chain of Nix packages structured like:

  • my bashrc
    • a bash history-management module I'm developing
      • an event-based shell-profile "orchestration" framework I'm developing to support both of the above
        • a third-party bash event/promise library that I'd like to PR if this is merged (WIP commit)

cc @grahamc @worldofpeace

Things I could use a little help/scrutiny on (added May 21 2020)

If you have time, I could use some extra scrutiny/feedback on the ergonomics and consistency of both the Nix interface and underlying CLI. It's not essential since we can iterate on it, but if you spot any obvious renames I'd rather fix them while the changes won't break workflows.

  • meta: the fact that some jerk named the library resholved and the executable resholver :)
  • resholver CLI:
    • RESHOLVE_PATH
    • RESHOLVE_ALLOW, --allow (scope:allowed_identifier pairs for selectively exempting some things); scopes:
      • builtin (allow a builtin that isn't in the current hardcoded list)
      • function (allow a function that couldn't be looked up for some reason (for example, a callback the user is supposed to define))
      • resholved_inputs (exempt already-resholved absolute paths--resholved writes these into end-of-script directives to exempt them from the rule that blocks absolute paths)
      • unresholved_inputs (absolute path exempted from resolution; perhaps something like /usr/bin/env)
      • variable_as_command (commented out for now, still exploring)
      • . source sudo command eval exec alias (exempt specific 2nd arg to this command)
  • Nix interface:
    • resholved.resholved (actual executable is here; this feels ham-handed to me but I don't know what's normal here)
    • buildResholvedPackage (Nix builder for resolving shell code)
      • inputs (list) -> RESHOLVE_PATH
      • allow (map) -> RESHOLVE_ALLOW
      • scripts (list of files passed to resholver)
    • it isn't officially represented here, but graham asked about something like mkResholvedScript {...} ''inline script''
Things done
  • 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 that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • 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.
@abathur abathur force-pushed the abathur:add_resholved branch 2 times, most recently from e29da15 to b6b79b1 Apr 23, 2020
@abathur abathur changed the title resholved: init at hopes and dreams [WIP] resholved: init at hopes and dreams May 10, 2020
@abathur abathur marked this pull request as draft May 10, 2020
@abathur abathur changed the title [WIP] resholved: init at hopes and dreams resholved: init at hopes and dreams May 10, 2020
@abathur abathur force-pushed the abathur:add_resholved branch 2 times, most recently from 55c6945 to 71d5ae4 May 17, 2020
@abathur abathur marked this pull request as ready for review May 17, 2020
@abathur
Copy link
Contributor Author

abathur commented May 17, 2020

I marked this as draft for a few days while I refactored how it packages and consumes its Oil-shell dependency. Not super relevant, but the main changes are:

  • It previously built from a fork of Oil where I fixed a few issues and added a setup.py, but I collapsed these changes into a set of patches maintained in the resholved repo. It now builds against a commit in the official repo.
  • Oil isn't structured with being turned into a Python package in mind, so I also hacked through forcing an oil namespace on its subpackages, which were previously just leaking into the Python package namespace. Some of them have very generic names (like core, misc, frontend, tools, etc.) so I've been a little concerned about them causing namespace clashes. To make this work (without patching all of the oil source) resholved now rewrites oil's imports.
@abathur abathur force-pushed the abathur:add_resholved branch from 71d5ae4 to 54c53ba May 22, 2020
inherit pname version src;
buildInputs = [ resholved ];
# tentatively disabled because gc probably knows things I don't :)
# propagatedBuildInputs = inputs;

This comment has been minimized.

Copy link
@abathur

abathur May 22, 2020

Author Contributor

I've tentatively disabled this after graham commented on IRC:

gchristensen | hmm yeah I'm not sure why this is a thing: propagatedBuildInputs = inputs;

I think he's right (and this is vestigial) but I wanted to comment on it here until this is reviewed since this is something I wasn't sure how to handle in the first place.


Since the first working draft, resholved has followed source statements and resolved them along with the parent script. This behavior is important--it helps resholved weed functions from sourced libraries out of the list of commands it needs to resolve. The naive first version needed to propagate inputs to ensure they were available while resolving a consumer.

A later update added the tail "directive" comments that encode this resolve-time state for later reference, making it no longer necessary to propagate the inputs in order to resolve shell libraries.


While reasoning through this, I realized there's a related question about how to square shell norms and conventions with Nix's. Some executables used in shell scripts/libraries are mere implementation details that a consumer shouldn't rely on (not propagated)--but sometimes they are integral and I'm a little less certain about whether they should come along?

Take, for example, a fairly popular bash dotfile manager like homeschick. It's a fairly thin wrapper around git that relies on the user also running plain git commands.

  • If someone made a Nix package for homeshick, should it propagate git?
  • If so, and if buildResholvedPackage had homeshick as an input for another script with a mix of homeschick and git commands, should it complain if git isn't specified as an input, or should it be picking up the propagated dependency?

This comment has been minimized.

Copy link
@emilazy

emilazy May 22, 2020

Member

I would err on the side of not propagating whenever it isn't required to make the software work. Explicit environment specification is a great thing, and propagation is a hack that should be avoided whenever possible; in an ideal world we wouldn't need it at all. (I can imagine cases where you'd want to deploy dotfiles from git without wanting to do actual commits and repository manipulation on that machine, for instance, though I admit this specific example isn't hugely convincing.)

Resholved attempts to resolve executables in shell scripts.
Includes Nix builder for resolving dependencies in Nix-built
shell projects.
@abathur abathur force-pushed the abathur:add_resholved branch from 54c53ba to 83f77de Jun 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.