init: devshell for hands-on onboarding #119966

blaggacao · 2021-04-20T17:31:03Z

Motivation for this change

https://discourse.nixos.org/t/contributor-retention/12412/77?u=blaggacao

From a drive-by contributor perspective, I can imagine how this could have massive impact:

numtide/devshell 1 (with friendly useful MOTD!)

started up with nix develop

with a decent and well organized set of commands (not more than X)

and also some templates per language / framework

so that I can do something around the lines of nix-tmpl goModule > folder/default.nix

and of course other shift-left amenities such as nixpkgs-review

Regulars within every package subsystem would be encouraged to keep their templates up to date according to latest best practices, so they can also save on chore tasks / reviews.

stripped down to:

The good thing: we could start with a devshell that incorporates and wires up the obvious parts, such as nixpkgs-fmt, nixpkgs-review.

Current look & feel:

$ nix develop
🔨 Welcome to nixpkgs

[general commands]

  menu    - prints this menu

[linters]

  evalnix - Check Nix parsing
  fmt     - Check Nix formatting

[nixpkgs]$

Other low hanging fruits:

git hooks
github cli
... ? ← please, make your suggestions

nixos-discourse · 2021-04-20T17:35:08Z

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

https://discourse.nixos.org/t/contributor-retention/12412/81

flake.nix

SuperSandro2000 · 2021-04-20T18:19:10Z

shell.nix

@@ -0,0 +1,17 @@
+{ system ? builtins.currentSystem }:


Is this file supposed to only use builtins?

Yes, it shall have no other dependency.

I'd rather avoid public interfaces, where we can. Everything needed will be conveniently contained within devshell and nixpkgs.

fricklerhandwerk

I think this is a very good idea in general. Providing contributor and maintainer tools right in the repo is great, and so is printing a helpful message to get going. We should consider making it optional though, such as by only providing a menu command, or keeping it really really short, with a reference to more detailed instructions. Just imagine regular developers seeing it every time they cd nixpkgs.

My question is why you think this should be devshell and not a regular shell.nix. I understand the advertised difference is a cleaner environment, but for me that would not warrant introducing a (circular!) dependency on an unstable tool. To get going it should be enough to include the necessary tools in the shell and print a message in shellHook.

fricklerhandwerk · 2021-04-20T18:08:52Z

shell.nix

+  };
+
+  pkgs = import nixpkgsSrc { inherit system; };
+  devshell = import devshellSrc { inherit system pkgs; };


I see a fundamental problem here: There is no way to introduce breaking changes in nixpkgs that touch devshell without patching the latter along. Otherwise the shell will simply break. We just added quite an arbitrary dependency.

Technically correct solution would be to pass the historic version of nixpkgs that devshell was developed against, but that's quite expensive.

Technically correct solution would be to pass the historic version of nixpkgs that devshell was developed against, but that's quite expensive.

I can see the problem. For the reasonable future, I think devshell will be well maintained and any breakage will be resolved within days, not weeks.

The problem manifests once/if (core) maintainers or the CI becomes heavily dependent on it. By then, devshell, whose stated objective is to completment mkShell here in nixpkgs, will either be already upstreamed or heavily maintained.

Historical nixpkgs, without further guardrails, would imply historical nixpkgs-fmt, historical nixpkgs-review, etc, etc.

I think cost benefit tips slightly towards accepting the risk that it (might evnetually maybe) break. ... and fix it when we get there.

blaggacao · 2021-04-20T18:41:42Z

My question is why you think this should be devshell and not a regular shell.nix. I understand the advertised difference is a cleaner environment, but for me that would not warrant introducing a (circular!) dependency on an unstable tool. To get going it should be enough to include the necessary tools in the shell and print a message in shellHook.

It's really just a knowledge pool, I want to tap into. I wouldn't be able to implement anything alike (with all the optimizations and polished edge cases) myself. It would mean I'm in the git log and I'll be pinged to the rescue. I can correspond for devshell, though, (to a certain extend) since I'm actively using it wherever I can and also am a contributor, there.

I just believe that plain mkShell behaves pretty awkward for what we want to achieve here.

We should consider making it optional though, such as by only providing a menu command, or keeping it really really short, with a reference to more detailed instructions. Just imagine regular developers seeing it every time they cd nixpkgs.

It happens so that invoking via nix-shell doesn't expose the menu. We can also lobby upstream for a documented solution. If this gets merged, such solution should emerge within a matter of weeks.

I also want to note, how easy it is via this to implement git hooks or a language specific templating module by analogy to how existing devshell modules are made.

blaggacao · 2021-04-20T19:24:22Z

Update: I prototyped a githooks implementation here. But that is not part of this PR. It just showcases how convenient and powerful the devshell module system can be.

nixos-discourse · 2021-04-20T21:53:13Z

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

https://discourse.nixos.org/t/into-the-nix-ecosystem-with-domen-kozar-the-changelog-437/12574/3

tomberek · 2021-04-20T22:08:17Z

Works as expected. There is a mentioned concern about a dependency on unstable devshell, but it does do a good job of bringing in minimal changes (derivation vs mkShell). If this helps people become involved, review PRs, clean up submissions, then it can be a great addition.

nixos-discourse · 2021-04-20T22:10:06Z

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

https://discourse.nixos.org/t/nix-onboarding-story-prs/12581/2

blaggacao · 2021-04-20T22:54:08Z

@tomberek if you have suggestions for follow up PRs to organize / include tools, let me know. For example I'm outdated on the current tooling around nixpkgs-review / nixpkgs-update, etc.

nixpkgs-review does even implement commenting on PRs and stuff, but not sure if it's good enough for making a recommended workflow out of it in here.

blaggacao · 2021-04-21T00:20:17Z

Another Update: I prototyped a gihub section to help make freshmen workflows more efficient

Mic92 · 2021-04-21T03:32:44Z

There is one practical issue with nixpkgs, not yet solved yet. If we are working on staging or recent master branches than devshell dependencies can potential trigger a lot of rebuilds making it unusable

blaggacao · 2021-04-21T04:32:27Z

There is one practical issue with nixpkgs, not yet solved yet. If we are working on staging or recent master branches than devshell dependencies can potential trigger a lot of rebuilds making it unusable

Oh that's actually a bigger inconvenience. So we would have to pin one version, instead, and update from time to time as needed.

That also solves the issue @fricklerhandwerk raised. I'll push a fix tomorrow.

nixos-discourse · 2021-04-21T13:49:31Z

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

https://discourse.nixos.org/t/tweag-nix-dev-update-9/12357/2

edolstra · 2021-04-21T14:43:58Z

Adding a devshell to nixpkgs seems like a good idea, but I don't understand why it needs a dependency on an external project (numtide/devshell) and use a non-standard way of specifying devshells (devshell.toml). There are some ideas about adding TOML as an alternative format for specifying flakes (see NixOS/nix@1dc3f53) but until we have that in Nix we shouldn't do it here, it just makes things confusing to users.

edolstra · 2021-04-21T14:56:45Z

devshell.toml

+name = "fmt"
+help = "Check Nix formatting"
+category = "linters"
+command = "nixpkgs-fmt ${@} ."


Maybe I misunderstand what this command section does, but the nixpkgs repo does not follow a standardized formatting yet. So running this command reformats most of nixpkgs, which is not desirable at this point.

Oh sry, havn't seen those comments on the celphone github app at first.

Indeed!

The idea is to quickly format a subtree. Eg.:

$ cd pkgs/applications/mynewpackge/ $ fmt

edolstra · 2021-04-21T15:00:51Z

shell.nix

+
+  nixpkgsSrc = ./.;
+
+  devshellSrc = fetchTarball {


Flakes shouldn't use `builtins.{fetchTarball,fetchGit,...}. This may even become impossible in the future. Dependencies should be expressed at the flake level so they can be queried and updated in a uniform way.

Thanks for pointing this out! We had a large discussion over at divnix/devos if this is an antipattern or actually a good pattern. Most lock files, support some notion of development dependencies and "usage" dependency (for not to say "runtime").

For example flake-utils-plus like numtide/flake-utils has a policy to depend on nothing, but builtins. This makes a lot of sense so that no subtle incompatibilities would be introduced since coded against one nixpkgs version it could be instantiated with another.

Importing those dependencies via flake inputs, for those packages it would be absolutely out of question to provide such development amentities. Until flakes support dev-only (optional) dependencies, which are not required for using a flake, this is the only pattern we could find to map this use case concisely into code.

Would you be ok to remain explicit in a sense that once flake supports that, we can change it, or would you still prefer to bury that subtle distinction and make it explicit once flake's support it? I don't have any vested preferences here.

blaggacao · 2021-04-21T15:04:41Z

I'll yield on the toml argument and will translate this to good old nix.

devshell is the best tool for the job (and was specifically designed for it). I use it and I'm very familiar with it. I am also very confident that should we have any issues, we'll enjoy upstream support in a matter of days, not weeks, should we need it.

Since my familiarity with devshell, another toolset is out of scope for this particular PR and my initiative.

However, I'd have personal attachement to the idea that this initiative would not die off / stale, as the cost benefit is overwhelmingly positive.

edolstra · 2021-04-21T15:23:38Z

I would keep it simple for now, e.g. just have shell.nix with a buildInputs = [ nixpkgs-fmt ... ] (and even that might be going to far, since nixpkgs doesn't have a standard formatting at the moment). I don't want to enshrine numtide/devshell as the "blessed" way of doing devshells, in particular because some of the things you listed are already (intended to be) addressed in the Nix UI. For instance, nix already has a command to create projects from a template. And there is an issue for adding MotD to nix develop and other commands (NixOS/nix#4610).

blaggacao · 2021-04-21T16:06:48Z

I don't want to enshrine numtide/devshell as the "blessed" way of doing devshells

I fully understand where you are coming from and I fully support that intention.

I'm absolutely certain @zimbatm would, too. So he probably would very happily yield into upstream support, once that is accomplished.

You might look at it this way: devshell is a very agile opportunity to refine those use cases. If we can accomplish that with massive community involvement, all the better.

Since I guess, we can realistically and easily secure future yielding into upstream capabilities, It's a win-win all the way down.

And there is an issue for adding MotD to nix develop and other commands (NixOS/nix#4610).

Immediate feedback from an active numtide/devshell user: NixOS/nix#4610 (comment) 😉 — a pretty good proof of the sort of win-win, we can leverage here.

blaggacao · 2021-04-21T16:17:24Z

My planned (optimistic) time-frame is to get this merged in the coming days so that I can propose for discussion further amenities (like github workflow support, further formatting, hooks, etc) by the weekend.

I feel obliged to transparently let you know, that competing commitments won't allow me to impulse this initiative well into the next week. I hate those circumstances, but I guess we are all subjected to resource scarcity.

😟

Still, I'll do what I can to tackle the underlying issue with my available resources and knowledge.

it would be too costly for users to rebuild the devshell on every change in nixpkgs. Also, an unpinned nixpkgs version might suddenly become broken if nixpkgs changes in ways that devshell is not yet compatible with.

blaggacao · 2021-04-21T23:11:32Z

ping, done:

toml → nix as per @edolstra 's request
pinned nixpkgs as per @Ma27 / @fricklerhandwerk concerns (impractical frequent rebuild of devshell when switching branches and possibly breakage of devhsell by an incompatible update to nixpkgs)

@edolstra I also PMd you on discord with details on my availablility to follow through, nut sure if you've seen it. Not meaning to pressure, just want to be transparent, and let not expectations mismatch circumstances.

alyssais · 2021-04-22T09:29:21Z

flake.nix

+      devShell = {
+        x86_64-linux = import ./shell.nix { system = "x86_64-linux"; };
+        x86_64-darwin = import ./shell.nix { system = "x86_64-darwin"; };
+        aarch64-linux = import ./shell.nix { system = "aarch64-linux"; };


Do we really have to manually list every platform here? What happens to somebody who isn't on one of those three platforms? (aarch64-darwin and powerpc64le-linux come to mind.)

When writing I had a bad stomach feeling, already. 😉

numtide/flake-utils has a defaultSystem. Do we have something similar to enumerate on the "blessed" set of systems as flake spec requires?

Maybe we also could just add those two to suport 'most common' platforms? What do you think?

Somebody on a different platfrom currenrly still can try nix-shell directly using shell.nix circumventing flake.nix.

asymmetric · 2021-04-22T12:29:01Z

I agree that it's weird to add a dependency to a 3rd-party project. If devshell is to be an integral part of nixpkgs, then let's move it under the NixOS org and sanction it as an official component of the stack.

Otherwise, I'd try to keep things lean and contained within the capabilities offered by nixpkgs.

I'm absolutely certain @zimbatm would, too. So he probably would very happily yield into upstream support, once that is accomplished.

@blaggacao I notice @zimbatm hasn't expressed an opinion on the matter yet ;)

In general, I think the two commands in this PR are better suited to a per-file/buffer scenario, i.e. integrated into the editor. Massive diffs due to nixpkgs-fmt would not be fun for a beginner (or anyone for that matter), and the proposal that the fmt be only run on a subtree is not very intuitive.

(On the topic of formatting: I think the right place to start enforcing it would be CI)

So as I mentioned in NixOS/nix#4610 (comment), I think we should first figure out what (if anything) we want to make available to a devshell. I think this PR is a good thing because it got the conversation started, but I don't agree with the specific approach taken.

zimbatm · 2021-04-22T13:06:29Z

I don't have much time to weigh in on all the issues here.

One thing that would be great is to copy flake-utils's eachDefaultSystem to nixpkgs. Nixpkgs is in a better position to decide what systems it wants to support. And it would remove flake-utils as a dependency for most flakes. That would be a quick win in my book.

devshell is still a bit of an experiment in my mind. There are parts like https://github.com/numtide/devshell/blob/master/nix/mkNakedShell.nix which could be moved to nixpkgs. One issue with it, and the nix-shell approach in general, is that the closure tends to grow quickly as all the dependencies get pushed to the top of the repo. I had one customer with a 5GB closure to download on each nixpkgs bump. I have some ideas on how to solve this but it's not for soon so I think it's best to stick with a hand-rolled shell.nix for now. The original motivation of @blaggacao for providing a nice environment is good!

blaggacao · 2021-04-22T13:24:10Z

So as I mentioned in NixOS/nix#4610 (comment), I think we should first figure out what (if anything) we want to make available to a devshell. I think this PR is a good thing because it got the conversation started, but I don't agree with the specific approach taken.

All right then, I guess we don't have / or will have consensus on this PR.

I'll yield completely.

Please anyone feel encouraged to take over this initiative and bring it home. 👍

blaggacao · 2021-04-22T13:50:22Z

I'd at least explicitly close the (social) feedback cycle here, in case it is not evident to every reader, already:

the community has gained not to introduce an external dependency
the community has lost a motivated person to solve that age old problem
the community has also lost (for now) a solution to that problem and forgoes (for the time being) any benefits it undoubtetly would have yielded.

My very own intepretation is that this is a very "luxurious" outcome, for not to say "hypocritical", for a community that claims, from what I hear in the forums, to be in constant resource starvation.

zimbatm · 2021-04-22T14:02:45Z

Don't give up! Gathering consensus can be difficult sometimes but it doesn't mean that it's not useful.

nixos-discourse · 2021-04-22T17:24:23Z

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

https://discourse.nixos.org/t/flag-packages-out-of-date/12612/12

blaggacao · 2021-04-23T02:36:06Z

Folks, wait a sec!

the community has also lost (for now) a solution to that problem and forgoes (for the time being) any benefits it undoubtetly would have yielded.

It hasn't! Nor did it loose a motivated person to bring this issue home.

I've got an idea .... Dumdidumdidum.

Thanks to @zimbatm @fricklerhandwerk & @tomberek for their enablement and those really great, positive, self-enhancing and kind social feedback loops.

:nix_parrot:

blaggacao · 2021-04-23T23:07:27Z

... and it's taking shape over at https://github.com/blaggacao/nixpkgs-devshell.

Please don't spare me with your feedback.

ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Apr 20, 2021

SuperSandro2000 reviewed Apr 20, 2021

View reviewed changes

fricklerhandwerk reviewed Apr 20, 2021

View reviewed changes

blaggacao force-pushed the da/devhsell branch from b411b94 to 7c08d64 Compare April 20, 2021 18:46

Mic92 requested a review from zimbatm April 20, 2021 19:54

blaggacao force-pushed the da/devhsell branch from d1d681d to a3d70fd Compare April 21, 2021 00:31

init: devshell for hands-on onboarding

5044f8d

blaggacao force-pushed the da/devhsell branch from a3d70fd to 5044f8d Compare April 21, 2021 00:31

Mic92 requested a review from edolstra April 21, 2021 03:26

edolstra reviewed Apr 21, 2021

View reviewed changes

David Arnold added 2 commits April 21, 2021 17:53

devhsell: pin nixpkgs version

b9bd449

it would be too costly for users to rebuild the devshell on every change in nixpkgs. Also, an unpinned nixpkgs version might suddenly become broken if nixpkgs changes in ways that devshell is not yet compatible with.

devshell: use good old nix

4c24484

alyssais reviewed Apr 22, 2021

View reviewed changes

blaggacao closed this Apr 22, 2021

init: devshell for hands-on onboarding #119966

init: devshell for hands-on onboarding #119966

Conversation

blaggacao commented Apr 20, 2021 • edited Loading

Motivation for this change

Current look & feel:

Other low hanging fruits:

nixos-discourse commented Apr 20, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fricklerhandwerk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blaggacao commented Apr 20, 2021 • edited Loading

blaggacao commented Apr 20, 2021

nixos-discourse commented Apr 20, 2021

tomberek commented Apr 20, 2021

nixos-discourse commented Apr 20, 2021

blaggacao commented Apr 20, 2021

blaggacao commented Apr 21, 2021

Mic92 commented Apr 21, 2021

blaggacao commented Apr 21, 2021

nixos-discourse commented Apr 21, 2021

edolstra commented Apr 21, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blaggacao Apr 21, 2021 • edited Loading

Choose a reason for hiding this comment

blaggacao commented Apr 21, 2021

edolstra commented Apr 21, 2021

blaggacao commented Apr 21, 2021 • edited Loading

blaggacao commented Apr 21, 2021 • edited Loading

blaggacao commented Apr 21, 2021 • edited Loading

Choose a reason for hiding this comment

blaggacao Apr 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asymmetric commented Apr 22, 2021

zimbatm commented Apr 22, 2021

blaggacao commented Apr 22, 2021 • edited Loading

blaggacao commented Apr 22, 2021 • edited Loading

zimbatm commented Apr 22, 2021

nixos-discourse commented Apr 22, 2021

blaggacao commented Apr 23, 2021 • edited Loading

blaggacao commented Apr 23, 2021

blaggacao commented Apr 20, 2021 •

edited

Loading

blaggacao commented Apr 20, 2021 •

edited

Loading

blaggacao Apr 21, 2021 •

edited

Loading

blaggacao commented Apr 21, 2021 •

edited

Loading

blaggacao commented Apr 21, 2021 •

edited

Loading

blaggacao commented Apr 21, 2021 •

edited

Loading

blaggacao Apr 22, 2021 •

edited

Loading

blaggacao commented Apr 22, 2021 •

edited

Loading

blaggacao commented Apr 22, 2021 •

edited

Loading

blaggacao commented Apr 23, 2021 •

edited

Loading