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

chore(nix): use nix-overlays for the slim devShell #6546

Merged
merged 1 commit into from
Nov 22, 2022

Conversation

anmonteiro
Copy link
Collaborator

I noticed a few problems with the regular shell:

  • every time the source tree changes, opam2nix does full resolution again and rebuilds everything
  • somehow, using the nixpkgs input doesn't give me editor completion on the slim shell, but using nix-overlays as the input fixes it for me

Steps to reproduce:

$ nix develop .#slim -c zsh
$ make clean && make dev

# In another tmux window, for me
$ nix develop .#slim -c nvim

Signed-off-by: Antonio Nuno Monteiro anmonteiro@gmail.com

@anmonteiro
Copy link
Collaborator Author

Another note: I don't know if anyone else is using the slim shell other than myself (added in #6327).

@Alizter
Copy link
Collaborator

Alizter commented Nov 22, 2022

every time the source tree changes, opam2nix does full resolution again and rebuilds everything

This is (unfortunately) intentional since we need to build dev deps with the new dune in case we break how things are installed. Dune has done this in the past.

@anmonteiro
Copy link
Collaborator Author

That's fair. My proposal here is that users of the slim devShell don't have to pay that price (I only work on a very limited subsystem of Dune, so far).

@Alizter
Copy link
Collaborator

Alizter commented Nov 22, 2022

I think that is fine to have and is essentially what we have for Coq today too.

Copy link
Member

@rgrinberg rgrinberg left a comment

Choose a reason for hiding this comment

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

Looks OK, just needs a rebase.

Note that running opam2nix is the cause of the slowdown. The issue is that every change to the source requires to rebuild the downstream packages. What would be nice is to have a middle ground environment that uses the dune from opam to build all the dev deps.

@anmonteiro anmonteiro force-pushed the anmonteiro/slim-shell-overlays branch from f947f89 to 7fd2da7 Compare November 22, 2022 23:26
Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>
@anmonteiro anmonteiro force-pushed the anmonteiro/slim-shell-overlays branch from 7fd2da7 to 6ba768b Compare November 22, 2022 23:26
@anmonteiro
Copy link
Collaborator Author

Rebased

@rgrinberg rgrinberg merged commit 4819bf4 into ocaml:main Nov 22, 2022
@anmonteiro anmonteiro deleted the anmonteiro/slim-shell-overlays branch November 22, 2022 23:27
jchavarri added a commit to jchavarri/dune that referenced this pull request Nov 24, 2022
* main: (58 commits)
  test: formatting of alternative dune files (ocaml#6567)
  refactor: remove Modules.is_empty (ocaml#6564)
  refactor: module kinds (ocaml#6562)
  refactor(coq): resolve lack of coqc properly
  Cache file contents in action builder by name. (ocaml#6555)
  fix: re-enable dune on older macos sdk's (ocaml#6515)
  fix: do not hide lib interface module (ocaml#6549)
  test: remove pkg-config output for reproducibility (ocaml#6543)
  melange: add test for ocaml flags (ocaml#6548)
  fix: improve virtual library error messages
  test: virtual library and impl locations
  test: alias module regression (ocaml#6544)
  refactor(merlin): dump config sub command (ocaml#6547)
  refactor: simplify merlin (ocaml#6508)
  chore(nix): use nix-overlays for the slim devShell (ocaml#6546)
  fix: module compilation rule env (ocaml#6527)
  chore: update nix (ocaml#6536)
  fix: merlin rules with pp's (ocaml#6474)
  Call [Dune_util.Log.init] as soon as possible (ocaml#6542)
  refactor: speed up stdlib build (ocaml#6524)
  ...
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.

3 participants