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

feat(entrypoint): fixes/improvements to support nix run (devshell-as-flake-app) #243

Merged
merged 14 commits into from Jun 5, 2023

Conversation

tomeon
Copy link
Contributor

@tomeon tomeon commented Jan 21, 2023

I was poking at a devshell derivation in nix repl recently when I noticed its flakeApp attribute. This is a cool feature! I would love to be able to nix run '.#my-devshell' -- <command> and access commands from my devshell without having to first enter a nix develop sesson. It looks like the flakeApp definition needs some updates to account for devshell.nix refactors; this PR is my attempt to apply those updates. It also adds some convenience features, like a CLI option for specifying the project root path.

Have a few questions I'd like to resolve before removing "Draft" status from this PR:

  1. Assuming you'd like to support flakeApp/nix run, where would you like me to document it? I took a look at the files in ./docs and nothing jumped out as an appropriate spot.
  2. Any thoughts on whether it's possible or desirable to automatically infer a value for PRJ_ROOT when running the devshell entrypoint as a flake app? Seems like PRJ_ROOT should refer to the flake's root directory? For instance, maybe a devshell command references files in the flake by expanding them relative to PRJ_ROOT. Using PWD as PRJ_ROOT in this circumstance might work, as long as PWD is the working tree toplevel of the git repository for the flake providing the devshell. But it almost certainly wouldn't work if trying to nix run 'some-other-flake#devshell-flake-app'. On the hand, maybe there are also cases where PRJ_ROOT should refer to (say) the current working directory -- for instance, maybe I want to run a linting command provided by a "foreign" flake, and that command searches for files that live under PRJ_ROOT.
  3. One of the changes in this PR is exposing devshell's own devshell as the flake output apps.<system>.devshell, as a kind of demo of the flakeApp feature. Is this worthwhile? If so, should it be advertised somehow (in the README, or elsewhere)?

Thanks!

Copy link
Member

@zimbatm zimbatm left a comment

Choose a reason for hiding this comment

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

Very nice

modules/devshell.nix Show resolved Hide resolved
@zimbatm
Copy link
Member

zimbatm commented Jan 27, 2023

  1. I don't know
  2. Yes, I think it's the responsibility of nix to set a FLAKE_ROOT environment variable before running anything. Otherwise, we're bound to re-implement the heuristic and it will drift over time.
  3. Sounds good, it will act as a docs placeholder for now

tomeon added 12 commits June 3, 2023 14:27
rather than the source/raw entrypoint that contains `@DEVSHELL_DIR@`
instead of the substituted output path.
which was, previously, the raw/source `entrypoint` (prior to
substitution of `@DEVSHELL_DIR@`), and is now the `env.bash` file from
the final `devshell_dir`.
even when it is already set when sourcing env.bash.
via doing `return 0` in a subshell instead of testing whether `$0` and
`${BASH_SOURCE[0]}` are identical (the latter method is subject to
false negatives).
by dynamically setting "$@" on the basis of, e.g., whether or not we saw
the `--pure` option.
that supports options in parameters other than "$1" and lays the
groundwork for introducing additional options.
for specifying "$PRJ_ROOT" prior to sourcing "env.bash" (which will bail
out if "$PRJ_ROOT" is unset or empty).  Using "--prj-root" may be more
convenient than setting an environment variable when, e.g., using the
entrypoint as a Nix flake app.
for specifying the "env" executable.  This is mostly for the automated
test suite, as "/usr/bin/env" is not available within the test script
environment.
thus demonstrating support for `nix run`.
@tomeon tomeon marked this pull request as ready for review June 3, 2023 23:45
@tomeon
Copy link
Contributor Author

tomeon commented Jun 3, 2023

I've just pushed some updates:

  1. Added some documentation to README.md and docs/flake-app.md covering running devshells as Nix applications.
  2. Added the option devshell.prj_root_fallback for controlling the last-ditch definition of PRJ_ROOT if it isn't already defined and non-empty. The default value is whatever PWD expands to, which may run afoul of the concern that "the heuristic [...] will drift over time" -- devshell.prj_root_fallback also supports being set to null for signifying "no fallback", and I'm happy to switch to that as the default value. I'll just note that I think nix run 'github:somebody/cool-thing' -- do-cool-thing is a nicer experience than nix run 'github:somebody/cool-thing' -- --prj-root "$PWD" -- do-cool-thing (or bailing out if --prj-root is absent and PRJ_ROOT is empty or unset).

Edit: I also broke some stuff, but it's fixed now :)

via the new option "devshell.prj_root_fallback".  This option, if set to
a non-null value, is used to set the value of "$PRJ_ROOT" in case it is
not set in the controlling environment, the entrypoint script did not
get a `--prj-root` option, "$IN_NIX_SHELL" is empty or unset, and
"$DIRENV_IN_ENVRC" is unset or any value other than "1".

The default option definition sets "$PRJ_ROOT" to the value of "$PWD".
Copy link
Member

@zimbatm zimbatm left a comment

Choose a reason for hiding this comment

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

nice work.

bors merge

@bors
Copy link
Contributor

bors bot commented Jun 5, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit 6b2554d into numtide:main Jun 5, 2023
7 checks passed
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.

None yet

2 participants