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

shell: use naked shell concept with env var prepends for leaner shell with autocompletion #191

Merged
merged 21 commits into from
Dec 20, 2022

Conversation

thenonameguy
Copy link
Contributor

fixes #79
closes #175

This way pkgs dependencies compile with most low-level toolchains.
By patching XDG_DATA_DIRS, shell completions are picked up for installed packages.
@thenonameguy
Copy link
Contributor Author

@shyim can you verify if this solves #167 for you?
$DEVENV_PROFILE/bin should be the equivalent of your symlink in your PR.

@shyim
Copy link
Contributor

shyim commented Dec 5, 2022

devenv flake lock seems out of sync?
image

@shyim
Copy link
Contributor

shyim commented Dec 5, 2022

@thenonameguy That does not solve my problem. I need a fixed PATH that never changes. So I can say in the IDE this is my Binary and does not have to touch it again.

image

The given path is nix store and changes 😢

@thenonameguy
Copy link
Contributor Author

thenonameguy commented Dec 5, 2022

@thenonameguy That does not solve my problem. I need a fixed PATH that never changes. So I can say in the IDE this is my Binary and does not have to touch it again.

image

The given path is nix store and changes cry

I see. 😞
Let's see what @domenkozar says. I think creating this static symlink might be too specific to your environment (or JetBrains product users in general).
The question is whether to put this logic:

  1. direnv, generally (your PR)
  2. direnv, as a jetbrains module
  3. in your project devenv.nix as custom enterShell

I would favor option 2 atm, as it allows for greater extensibility for those products in the future.

@shyim
Copy link
Contributor

shyim commented Dec 5, 2022

I have no problem to do that in my own project, but you have this problem any GUI software as the PATH is only altered in the shell and not in the OS

@thenonameguy
Copy link
Contributor Author

For these kind of use-cases we usually use:

Or any other direnv editor plugin. This auto-detects changes just like your shell does.

@domenkozar
Copy link
Member

Thanks for all this hard work, this looks like a good step forward!

How would we address for example PYTHONPATH using this change? Or any language setup-hook that would work currently?
That's something I'm worried quite a bit since we'd have to do a lot of work to replicate all those setup-hooks.

@thenonameguy
Copy link
Contributor Author

thenonameguy commented Dec 6, 2022

Thanks @domenkozar !

I would add those to language-specific modules.
I already wanted to add these PKG_CONFIG/LD_LIBRARY_PATH patches to the C language one. Since if you need these, you will most likely need those binaries in your env anyway.
I decided not to do it as they are low-level enough that they are used in many contexts (like the Ruby example).

That's something I'm worried quite a bit since we'd have to do a lot of work to replicate all those setup-hooks.

I understand your worry, but do not share it.

devshell has quite the market share in this space without any explicit support for these env extensions.
Users get by fine (although we can assume more mature Nix users there).
Patching them on a case by case basis is acceptable IMO, especially if people upstream them in devenv language modules.

I would want to reiterate that nixpkgs package definitions do not have a nix-less env extension concept in mind.
All the scripts/env vars are geared towards a succesful nix-build. This distinction is important, ie. Ubuntu users are running devenv and are using not the same nix-provided toolchain as in the given nixpkgs rev.

A good example is the gem install rmagick line in the process-compose example.
Since the ruby language module does not provide pkg-config from nix, the installer pulls down a pkg-config version for itself, the version being up to it's discretion. This pkg-config does not support the setup-hook PKGCONFIG_PATH_FOR_TARGET env at all, causing a failure.

The above showcases that the status quo has it's own faults and needs custom code anyway.

Lorri also does this naked shell approach and seem to achieve some better out-of-the-box experience by relying on a few nix fns:
https://github.com/nix-community/lorri/blob/a20fafa4e79ac19409634d0c69bd2e7b1208706a/release.nix#L305-L323

This should cover a few more of the top10 languages. I would be interested to do this as a follow-up PR.

@thenonameguy
Copy link
Contributor Author

thenonameguy commented Dec 7, 2022

@domenkozar what would you consider necessary to get this merged?

I was thinking along the lines of making more elaborate examples for the top 10 most loved languages:
https://survey.stackoverflow.co/2022/#technology-most-loved-dreaded-and-wanted
(Assuming the people who still use Fortran won't use devenv, Rust users are more the target audience)

Process being:

  1. Find a non-trivial sample application for the language:
    https://codebase.show/projects/realworld?category=backend is a good candidate
    example: https://github.com/snamiki1212/realworld-v1-rust-actix-web-diesel/

  2. Fork these repos and replace docker-compose with devenv + process-compose:
    https://github.com/snamiki1212/realworld-v1-rust-actix-web-diesel/blob/main/compose.yaml

  3. Try to keep project devenv.nix as small as possible, preferably diff is in our favor.
    if devenv modules are expressive enough (no env var overrides needed :) )

This would serve multiple purposes:

  1. Make sure the language specific tooling work in a real use-case in a limited scope context
  2. The current testing stack just initializes devenv, has no real-life testing equivalent. GHA could be integrated for major releases to check for breakages.
  3. A good intution builder for language experts who haven't touched nix yet (modify landing page with language select?).
  4. A test for this PR, such that within a devenv shell the language tooling works.

How does that plan sound? We could ask the language module maintainers to do this (I happily volunteer for Clojure).

This was my original goal for: NixOS/nix.dev#367 (just using numtide/devshell and a lot more Flake code).

@domenkozar
Copy link
Member

Thanks for all this awesome work - I want to do a bit of testing - hoping to get a chance on Monday!

@domenkozar
Copy link
Member

@domenkozar what would you consider necessary to get this merged?

I was thinking along the lines of making more elaborate examples for the top 10 most loved languages: https://survey.stackoverflow.co/2022/#technology-most-loved-dreaded-and-wanted (Assuming the people who still use Fortran won't use devenv, Rust users are more the target audience)

Process being:

  1. Find a non-trivial sample application for the language:
    https://codebase.show/projects/realworld?category=backend is a good candidate
    example: https://github.com/snamiki1212/realworld-v1-rust-actix-web-diesel/
  2. Fork these repos and replace docker-compose with devenv + process-compose:
    https://github.com/snamiki1212/realworld-v1-rust-actix-web-diesel/blob/main/compose.yaml
  3. Try to keep project devenv.nix as small as possible, preferably diff is in our favor.
    if devenv modules are expressive enough (no env var overrides needed :) )

This would serve multiple purposes:

  1. Make sure the language specific tooling work in a real use-case in a limited scope context
  2. The current testing stack just initializes devenv, has no real-life testing equivalent. GHA could be integrated for major releases to check for breakages.
  3. A good intution builder for language experts who haven't touched nix yet (modify landing page with language select?).
  4. A test for this PR, such that within a devenv shell the language tooling works.

How does that plan sound? We could ask the language module maintainers to do this (I happily volunteer for Clojure).

This was my original goal for: NixOS/nix.dev#367 (just using numtide/devshell and a lot more Flake code).

This is excellent! Would you do it as part of this PR?

@domenkozar
Copy link
Member

Also relevant is #209

@thenonameguy
Copy link
Contributor Author

@domenkozar what would you consider necessary to get this merged?
I was thinking along the lines of making more elaborate examples for the top 10 most loved languages: https://survey.stackoverflow.co/2022/#technology-most-loved-dreaded-and-wanted (Assuming the people who still use Fortran won't use devenv, Rust users are more the target audience)
Process being:

  1. Find a non-trivial sample application for the language:
    https://codebase.show/projects/realworld?category=backend is a good candidate
    example: https://github.com/snamiki1212/realworld-v1-rust-actix-web-diesel/
  2. Fork these repos and replace docker-compose with devenv + process-compose:
    https://github.com/snamiki1212/realworld-v1-rust-actix-web-diesel/blob/main/compose.yaml
  3. Try to keep project devenv.nix as small as possible, preferably diff is in our favor.
    if devenv modules are expressive enough (no env var overrides needed :) )

This would serve multiple purposes:

  1. Make sure the language specific tooling work in a real use-case in a limited scope context
  2. The current testing stack just initializes devenv, has no real-life testing equivalent. GHA could be integrated for major releases to check for breakages.
  3. A good intution builder for language experts who haven't touched nix yet (modify landing page with language select?).
  4. A test for this PR, such that within a devenv shell the language tooling works.

How does that plan sound? We could ask the language module maintainers to do this (I happily volunteer for Clojure).
This was my original goal for: NixOS/nix.dev#367 (just using numtide/devshell and a lot more Flake code).

This is excellent! Would you do it as part of this PR?

Only if you deem it so that it is required for merging.
Clojure works with this branch, I'm using it for an internal company project already.
Doing it would not showcase the edge cases that come up from abandoning the setup-hooks. (point 4)
That would be more of Python/Ruby/dynamic lang with $LANG's package manager compiling C code problem, as shown above.

I'll clarify that this is a breaking change if it ever gets merged.
The tradeoff for breakage is that language users will have to extend their specific language module necessary env var settings, which makes them explicit in the devenv context, instead of pulling it sort-of-accidentally for purposes of Nix compilation.

If you deem it necessary to showcase that we are not breaking devenv users too badly, I can do a realworld project transform to devenv using a $LANG selected by you.

@domenkozar
Copy link
Member

Could you handle Ruby and Rust? Those two are most popular.

@shyim can you check that PHP works with this PR?

@thenonameguy
Copy link
Contributor Author

thenonameguy commented Dec 17, 2022

Could you handle Ruby and Rust? Those two are most popular.

@shyim can you check that PHP works with this PR?

Here is the Rust example, had to extend the postgres module to be on par with the Postgres Docker image:
snamiki1212/realworld-v1-rust-actix-web-diesel#15

I decided to use Nix Flake + nix-direnv as it has superior caching so I get my profile's binaries instantly, when not running devenv commands.

Having a second hidden flake via devenv feels off and surprising when going through this route. Something for the future :)

@@ -3525,7 +3525,7 @@ attribute set of attribute set of (INI atom (null, bool, int, float or string) o


## services.postgres.createDatabase
Create a database named like current user on startup.
Create a database named like current user on startup. Only applies when initialDatabases is an empty list.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe makes more sense to remove it and the user migrates to the new option :)

Copy link
Contributor Author

@thenonameguy thenonameguy Dec 19, 2022

Choose a reason for hiding this comment

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

I did not want to break the current users of this flag.
Also there is no way of specifying at the moment that $USER with a program fallback should be used with the initialDatabases syntax. The name is static at Nix eval time.
In my personal projects I almost always have a static database name locally, so it can be hard-coded for maximum ease of use. This way we don't have to pass the $USER everywhere.

Copy link
Member

Choose a reason for hiding this comment

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

If it makes sense to deprecate a flag you can do it like:

imports = [
    (lib.mkRenamedOptionModule [ "postgres" "enable" ] [ "services" "postgres" "enable" ])
  ];

@thenonameguy
Copy link
Contributor Author

thenonameguy commented Dec 18, 2022

I made the Ruby example without flakes. This one uses by default a transient sqlite database, so the devenv.nix is really small. Nokogiri was installed successfully using native extensions (with the updated dependencies) 🎉
blrB/hanami-realworld-example-app#1

flake.nix Show resolved Hide resolved
flake.nix Show resolved Hide resolved
examples/process-compose/devenv.nix Outdated Show resolved Hide resolved
@@ -1 +1,4 @@
use flake --impure
# https://github.com/nix-community/nix-direnv
nix_direnv_watch_file devenv.nix
Copy link
Member

Choose a reason for hiding this comment

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

What happens if the user doesn't have nix-direnv installed?

@domenkozar
Copy link
Member

@thenonameguy added a few nitpicks, other than that we're good to go here! I'll merge it after devenv 0.5 will be released, hopefully tomorrow.

@thenonameguy
Copy link
Contributor Author

@domenkozar thanks for the review and the great pointers!

I addressed all of them and updated both realworld devenv projects as a smoke test:
blrB/hanami-realworld-example-app#1
snamiki1212/realworld-v1-rust-actix-web-diesel#15

Both work well, see for yourself.
Please tell if you have any further changes in mind. 🚀

@domenkozar
Copy link
Member

Nice work 🚀

I see that in the examples you're using nix flakes interface instead of devenv.nix. Is that a personal preference or is there something that plain devenv doesn't support?

Copy link
Member

@domenkozar domenkozar left a comment

Choose a reason for hiding this comment

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

Almost there :)

@@ -3525,7 +3525,7 @@ attribute set of attribute set of (INI atom (null, bool, int, float or string) o


## services.postgres.createDatabase
Create a database named like current user on startup.
Create a database named like current user on startup. Only applies when initialDatabases is an empty list.
Copy link
Member

Choose a reason for hiding this comment

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

If it makes sense to deprecate a flag you can do it like:

imports = [
    (lib.mkRenamedOptionModule [ "postgres" "enable" ] [ "services" "postgres" "enable" ])
  ];

examples/process-compose/devenv.nix Show resolved Hide resolved
templates/simple/.envrc Show resolved Hide resolved
@thenonameguy
Copy link
Contributor Author

Nice work rocket

I see that in the examples you're using nix flakes interface instead of devenv.nix. Is that a personal preference or is there something that plain devenv doesn't support?

It's a mix of both. There is no way at the moment to easily package the project with Nix for advanced Nix users. For this you still have to use Flakes. I did not do the build packaging for that project though.
I expected this feedback, that's why I included a Flake-less example as well for the Ruby project.

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.

Direnv is extremely verbose
3 participants