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

emacs: Add basic tree-sitter support #219559

Merged
merged 3 commits into from
Mar 15, 2023

Conversation

clhodapp
Copy link
Contributor

@clhodapp clhodapp commented Mar 4, 2023

Description of changes

From the commit message:

This commit adds basic support for tree-sitter in the emacs build, such that (if the user opts into tree-sitter support), tree-sitter will be enabled and binary library files for tree-sitter can be included in the lib directory of packages passed to emacsWithPackages. The libraries will be aggregated and included in treesit-extra-load-path.

The previous pattern for this in the community was to add tree-sitter libaries by patching emacs's RUNPATH with patchelf in a post-fixup phase. However, this has the substantial drawback that two different emacs installations with different lists of available tree-sitter libraries must be entirely separate builds. By supplying the tree-sitter libraries in the wrapping layer of emacsWithpackages, it becomes possible to share a single, more-cacheable "core emacs".

This support defaults to "on" only in emacs 29 and up, since previous versions do not support tree-sitter out of the box.

Additional notes:

  • I don't believe that this should cause a rebuild of anything currently published in nixpkgs, since nixpkgs doens't currently contain any emacs 29 packages directly. I checked the hashes of several top-level emacs packages and they appeared unchanged. So, at present, this is more in service of users who are using the infrastructure from nixpkgs to build their own emacs 29 packages out-of-tree.
  • I believe that it's still appropriate to incrementally start putting stuff to support of emacs 29 into nixpkgs because it won't be too long before it's actually out. Also, there's already some stuff in nixpkgs to help support emacs 29.
  • I have tested this with my own out-of-tree emacs 29 package and it seems to work !
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

This commit adds basic support for tree-sitter in the emacs build,
such that (if the user opts into tree-sitter support), tree-sitter
will be enabled and binary library files for tree-sitter can be
included in the `lib` directory of packages passed to
`emacsWithPackages`. The libraries will be aggregated and included in
treesit-extra-load-path.

The previous pattern for this in the community was to add tree-sitter
libaries by patching emacs's `RUNPATH` with `patchelf` in a post-fixup
phase. However, this has the substantial drawback that two different
emacs installations with different lists of available tree-sitter
libraries must be entirely separate builds. By supplying the
tree-sitter libraries in the wrapping layer of `emacsWithpackages`, it
becomes possible to share a single, more-cacheable "core emacs".

This support defaults to "on" only in emacs 29 and up, since previous
versions do not support tree-sitter out of the box.
@clhodapp clhodapp requested a review from adisbladis as a code owner March 4, 2023 19:52
@github-actions github-actions bot added the 6.topic: emacs Text editor label Mar 4, 2023
@ofborg 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 Mar 4, 2023
@marsam
Copy link
Contributor

marsam commented Mar 4, 2023

@clhodapp
Copy link
Contributor Author

clhodapp commented Mar 4, 2023

Does setting that variable on the emacs mkDerivation args actually do anything?

I'm not aware of anything that reads it and everything seems to work just fine without it on my Linux system. Is it possibly relevant on Darwin?

@marsam
Copy link
Contributor

marsam commented Mar 7, 2023

Sorry, you are right, it's not needed. I was using the emacs overlay as reference, but it seems to be outdated.

@clhodapp
Copy link
Contributor Author

clhodapp commented Mar 8, 2023

No worries! I just followed the upstream instructions from Emacs.

@clhodapp
Copy link
Contributor Author

@adisbladis Do you think you could review this?

It seems like some others are eager to see this go in ( ❤️'s on the PR) plus, selfishly, it would let me stop having to update my nixpkgs fork 😉.

@adisbladis adisbladis merged commit 1a8edfe into NixOS:master Mar 15, 2023
@adisbladis
Copy link
Member

This looks great!

@clhodapp
Copy link
Contributor Author

Thanks for the merge!

@clhodapp clhodapp deleted the feature/emacs-treesitter branch April 4, 2023 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: emacs Text editor 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants