Add initial nix dev environment for yarn workflows
Details
Details
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- joringer/add-nix-clean
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
There are a very large number of changes, so older changes are hidden. Show Older Changes
| nix/overlay.nix | ||
|---|---|---|
| 5 ↗ | (On Diff #9874) | final references the package set after all changes, which allows use to refer to the androidDevEnv created above. |
Comment Actions
Tested it (by running yarn dev in native directory) and it works nice, but it would be better if we could just run nix develop instead of nix develop --extra-experimental-features nix-command --extra-experimental-features flakes. Assigning @jimpo as a blocking reviewer as he had some questions
Comment Actions
but it would be better if we could just run nix develop instead of nix develop --extra-experimental-features nix-command --extra-experimental-features flakes
Maybe a crude approach, but we could add something like yarn nix-dev to package.json as an alias for nix develop --extra-experimental-features nix-command --extra-experimental-features flakes?
Comment Actions
Maybe a crude approach, but we could add something like yarn nix-dev to package.json as an alias for nix develop --extra-experimental-features nix-command --extra-experimental-features flakes?
You just need to put "experimental-features = nix-command flakes" in your nix.conf, which should be documented somewhere. Or, put NIX_CONFIG in the envrc.
Last question here is whether we're OK merging this without documentation, or whether I'm just missing where the documentation is? Probably a question for @ashoat.
Comment Actions
Last question here is whether we're OK merging this without documentation, or whether I'm just missing where the documentation is? Probably a question for @ashoat.
The documentation is in D2921, which needs revision from @jonringer-comm
Comment Actions
Remove nix's flow, fix NixOS support
Nix's flow is more up-to-date, and will likely cause
work to bump flow version each time nix sees an update
Comment Actions
You just need to put "experimental-features = nix-command flakes" in your nix.conf, which should be documented somewhere. Or, put NIX_CONFIG in the envrc.
This is documented in https://phabricator.ashoat.com/D2921
Comment Actions
iOS and Android scenarios are not yet taken care of, as they are quite a bit more complex than the yarn workflows.
| .envrc | ||
|---|---|---|
| 1 ↗ | (On Diff #11099) | Is this a directive to the reader, or a description of what happens? |
| 1–11 ↗ | (On Diff #11099) | What's this file? What's the format... it looks like bash, but I'm not really sure? |
| 2 ↗ | (On Diff #11099) | Is watch_file a bash command? |
| 11 ↗ | (On Diff #11099) | Is this || saying "if the stuff in the { } block fails, only then should we run use nix"? |
| docs/dev_environment.md | ||
| 5 ↗ | (On Diff #11099) | It appears that you've made the same edit in two diffs. These diffs should be in a stack (ie. in the same branch), and so it should impossible for this to happen. It seems pretty clear that these two diffs are not in the same branch. You also haven't specified any dependency between the two |
Comment Actions
Setting @varun as a blocking reviewer because he's taking the lead on reviewing Nix stuff and I think we should have at least one full-time person who understands the nuances of the Haskell-like syntax that .nix files use. I'm having a hard time reading through / grokking these files and think it's important that at least one reviewer here is able to understand what is being written
Comment Actions
small nit in addition to ashoat's questions
| nix/overlay.nix | ||
|---|---|---|
| 1 ↗ | (On Diff #11208) | "refers to" |
Comment Actions
sorry, just a few more q's -- i think this is really close
| flake.nix | ||
|---|---|---|
| 29 ↗ | (On Diff #11672) | still unclear what the parentheses around legacyPackages means. can you clarify? |
| 33–34 ↗ | (On Diff #11672) | From the NixOS wiki:
Can you clarify why we need both overlays and overlay? Didn't really understand that. Also can we not just inherit overlays from the outer scope? |
| nix/dev-shell.nix | ||
| 11 ↗ | (On Diff #11672) | why is this a recursive set? |
| flake.nix | ||
|---|---|---|
| 29 ↗ | (On Diff #11672) | This is syntax sugar for devShell = legacyPackages.devShell. It can be extended to pull out many packages as well. E.g. inherit (legacyPackages) devShell tunnelbroker; |
| 33–34 ↗ | (On Diff #11672) | We don't need to expose overlays yet as we only have one, this is more or less about completeness. I can remove if it's confusing, but in the future we may want to expose more than one overlay (or more likely, another overlay which we might use). |
| nix/dev-shell.nix | ||
| 11 ↗ | (On Diff #11672) | Most likely muscle memory, this doesn't need it. However, I did prune this a bit for this diff, and may have required rec before the changes and I forgot to remove the rec. From an implications standpoint, this shouldn't really affect much, generally recursive sets are a little slower (~1.5x slower) to evaluate, but shouldn't be noticeable as this attr set is quite small |
| nix/dev-shell.nix | ||
|---|---|---|
| 11 ↗ | (On Diff #11672) | Sure |
| flake.nix | ||
|---|---|---|
| 21–23 ↗ | (On Diff #11208) | over looked this, fixed. |
Comment Actions
@jonringer-comm why did you re-request review here? You still need to strip the rec.
The only scenario you should ever use the "Re-request review" feature in is if you want to send the diff back to your reviewers' queue without any changes