feat: add and document devenv usage with standard nix

[balintbarna]

Fixes #2126

This PR add a thin wrapper to be used with standard nix files, such as a common project level default.nix file, to define devenv shells for the project. There is also a concise guide added to the documentaiton describing the usage with this new wrapper and npins to pin dependencies.

[sandydoo]

Thanks for writing this up.

Would you also be able to add a test for this?

In tests:

1. Create a folder with the default.nix
2. .test-config.yml: use_shell: false
3. .test.sh: Run commands to launch the shell. If possible with npins, don't commit the lock.

[balintbarna]

Thany you @sandydoo for the comments and guidance. I pushed a new commit that tries to address some your concerns and also added some more documentation on how to handle more complex scenarios with this wrapper.

I still have to add the tests.

[balintbarna]

@sandydoo I now pushed the tests as well, let me know if this is how you wanted it. I ran the test bash files locally and that works well but I'm not sure how your CI is structured.

[balintbarna]

I'm having a bit of difficulty understanding what's wrong with the tests.

[sandydoo]

I've fixed the tests here. I dropped the advanced test as it doesn't add much coverage for us and just kept the simple one (renamed to npins).

[sandydoo]

Now that we have a test for this, I'm happy. Not entirely sold on the naming of nonFlakeMkShell, but tentatively approving.

Thoughts, @domenkozar?

[balintbarna]

Now that we have a test for this, I'm happy. Not entirely sold on the naming of nonFlakeMkShell, but tentatively approving.

I also dislike the name, feel free to change it. mkShell is already taken unfortunately. Could be mkStandardShell for example.

[balintbarna]

@sandydoo should we try to get this merged in in the next couple days?

[domenkozar]

I'm against having npins guide part of devenv, because don't want to support that (mostly because already supporting Nix inputs is a lot of work).

We can expose things on devenv side and the guide should be upstreamed to npins instead, to claim it has devenv support and how to use it.

Does this make sense?

[balintbarna]

@domenkozar Thanks for your input. I don't think it makes sense to add anything to npins regarding this change. This PR adds a function to devenv and I think that having the documentation here on how to use this new function is very useful and seems to me like the most reasonable place to have it.

I used npins in the guide because I think it provides the best user experience but this function should work with any standard nix workflow, using npins or niv or fetchTarball, it shouldn't matter.

[balintbarna]

Perhaps we could change the documentation and the tests to use fetchgit instead of npins, is that something you would prefer?

[domenkozar]

@domenkozar Thanks for your input. I don't think it makes sense to add anything to npins regarding this change. This PR adds a function to devenv and I think that having the documentation here on how to use this new function is very useful and seems to me like the most reasonable place to have it.

I used npins in the guide because I think it provides the best user experience but this function should work with any standard nix workflow, using npins or niv or fetchTarball, it shouldn't matter.

Imagine a scenario where npins makes a breaking change, then documentation for npins and devenv needs to be tested and upated. All of that should be handled by npins, that's why I prefer that maintenance burden is not transfered to us because we don't have the capacity to handle it.

We should definitely expose the bits on the devenv side for npins to integrate well and I'm also fine having a guide link to npins+devenv guide as a link.

[balintbarna]

This feature of devenv doesn't really have anything to do with npins so I won't submit any changes to npins.

I will update the documentation in this PR to not use npins but instead use built in nix functions like fetchGit or fetchTarball. That way there won't be any npins related maintenance burden on you.

[balintbarna]

I updated the tests and documentation to work with the built-in Nix function fetchTarball. No more npins, except it's mentioned as one of the tools that can be used in the documentation.

I'm not sure how to update the test override so that it would take the current branch of devenv instead of main. @sandydoo can you help with that?

[balintbarna]

Hi everyone!

I understand this is not a high priority ticket for your team, but for me it would be really nice to not have to use my fork of devenv anymore. I removed all the npins related documentation/guide and replaced it with built-in nix functions.

Could we take a final look and push and see if we can get it merged in? I'm happy to make any final changes if you have any further concerns. Let me know what you think.

@domenkozar @sandydoo

[balintbarna]

I simplified the shell function by passing in self directly, and updated the test configuration to use the DEVENV_REPO environment variable.

[balintbarna]

@sandydoo I rebased the branch on upstream main, would love to get it merged. Anything else needed to be changed?