nixpkgs icon indicating copy to clipboard operation
nixpkgs copied to clipboard

Published 20 hours ago • verified publisher icon NixOS

luaPackages.neorg: init at 8.4.0-1

Open GaetanLepage opened this issue 10 months ago • 21 comments

Description of changes

Add lua package neorg.

Things done

  • Built on platform(s)
    • [ ] x86_64-linux
    • [ ] aarch64-linux
    • [ ] x86_64-darwin
    • [ ] aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • [ ] sandbox = relaxed
    • [ ] sandbox = true
  • [ ] 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/)
  • 24.05 Release Notes (or backporting 23.05 and 23.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.

Add a :+1: reaction to pull requests you find important.

GaetanLepage avatar Apr 07 '24 22:04 GaetanLepage

Currently neorg has a nvim-nio ~> 1.7 constraint, which means it won't work with version 1.9 and beyond.

mrcjkb avatar Apr 08 '24 10:04 mrcjkb

@mrcjkb what do you recommand as a fix ? I checked the last release https://luarocks.org/modules/neorg/neorg with no change

@vhyrro is it a big job to support nio 1.9 ?

Note to myself when seeing this:

dependencies = { 'lua >= 5.1', 'nvim-nio ~> 1.7', 'lua-utils.nvim == 1.0.2', 'plenary.nvim == 0.1.4', 'nui.nvim == 0.3.0', 'pathlib.nvim ~> 2.2' }

"oh no, it's haskell all again". Does it work well even with rocks.nvim when one plugin mandates plenary.nvim == 0.1.4 and the other plenary.nvim == 0.1.5 ? What's the ~> ? respect major version ? Might need to add support for those in luarocks-nix :o

teto avatar Apr 10 '24 13:04 teto

@teto both rocks.nvim and luarocks.nvim properly handle multiple versions of the same dependency. It probably isn't a large deal to upgrade to nio 1.9 but upper bounded dependencies are important to prevent breaking changes of dependencies to break the upstream package like Neorg in this case, therefore you being able to support it is quite important :p

vhyrro avatar Apr 10 '24 14:04 vhyrro

@mrcjkb what do you recommand as a fix ?

I don't think there's a clean way to solve this nicely in nixpkgs. The reason luarocks can support different versions of the same dependency is because it has a luarocks.loader module that is aware of the dependency tree. To make use of that feature, lua applications have to require("luarocks.loader"), and the luarocks installation would have to be aware of all the nix store paths where luarocks packages are installed. Might not be an impossible task, but I have no idea if that works.

Without that, if a package that neorg depends on becomes outdated, we would have to either wait for neorg to update the dependency pins, or add the required dependencies (e.g. with a _<version> suffix) and hope that nothing breaks.

What's the ~> ?

  • ~> 1 = >= 1, < 2
  • ~> 1.5 = >= 1.5, < 1.6
  • ~> 1.1.5 = >= 1.1.5, < 1.1.6

(comes from ruby)

@vhyrro Don't you think ~> <major> is enough to ensure backward compatibility with semver if major > 0? If such a dependency were to introduce a breaking change with a minor version bump, it would be a quick fix. And once the rocks.nvim lockfile support is ready, people who don't want to upgrade could still pin dependencies.

mrcjkb avatar Apr 10 '24 15:04 mrcjkb

If there were luarocks lockfiles with checksums that can be checked via nix, we could support the dependency pinning. The shortterm solution looks to me like doJailbreak in haskell, aka ignore the dependencies constraints (or convert them to lower bounds ?) and rely on the tests to check it works.

teto avatar Apr 10 '24 21:04 teto

doJailbreak is relatively safe in Haskell because it likely won't compile if something breaks. I'm not sure if relying on tests will give us the same guarantees for lua packages.

mrcjkb avatar Apr 11 '24 20:04 mrcjkb

Although, it looks like some lua packages are already being "jailbroken":

https://github.com/NixOS/nixpkgs/blob/a945f3fda983b86d6fe9711c8e5f6e118990ca02/pkgs/development/lua-modules/overrides.nix#L312

mrcjkb avatar Apr 12 '24 14:04 mrcjkb

@mrcjkb what do you recommand as a fix ?

I don't think there's a clean way to solve this nicely in nixpkgs. The reason luarocks can support different versions of the same dependency is because it has a luarocks.loader module that is aware of the dependency tree. To make use of that feature, lua applications have to require("luarocks.loader"), and the luarocks installation would have to be aware of all the nix store paths where luarocks packages are installed. Might not be an impossible task, but I have no idea if that works.

Without that, if a package that neorg depends on becomes outdated, we would have to either wait for neorg to update the dependency pins, or add the required dependencies (e.g. with a _<version> suffix) and hope that nothing breaks.

What's the ~> ?

* `~> 1` = `>= 1, < 2`

* `~> 1.5` = `>= 1.5, < 1.6`

* `~> 1.1.5` = `>= 1.1.5, < 1.1.6`

(comes from ruby)

@vhyrro Don't you think ~> <major> is enough to ensure backward compatibility with semver if major > 0? If such a dependency were to introduce a breaking change with a minor version bump, it would be a quick fix. And once the rocks.nvim lockfile support is ready, people who don't want to upgrade could still pin dependencies.

For making rocks.nvim aware, that should just involve symlinking the nix store paths into a folder which rocks.nvim would be aware of; I think it looks wherever the default install directory for the luarocks binary is, at least if I'm reading this code correctly: https://github.com/nvim-neorocks/rocks.nvim/blob/master/lua/rocks/loader.lua

purepani avatar Apr 15 '24 01:04 purepani

I relaxed the dependency for nvim-nio but it still fails to find other deps:

Missing dependencies for neorg 8.4.0-1:
   plenary.nvim 0.1.4 (not installed)
   nui.nvim 0.3.0 (not installed)

GaetanLepage avatar Apr 20 '24 12:04 GaetanLepage

I relaxed the dependency for nvim-nio but it still fails to find other deps:

Missing dependencies for neorg 8.4.0-1:
   plenary.nvim 0.1.4 (not installed)
   nui.nvim 0.3.0 (not installed)

Does it work if you relax the constraints for those, too?

mrcjkb avatar Apr 20 '24 13:04 mrcjkb

Everything seems to build fine now. Could someone actually test the plugin ?

GaetanLepage avatar May 13 '24 12:05 GaetanLepage

@teto is he neorg = ... snippet in vim/plugins/overrides.nix needed anymore now ? https://github.com/NixOS/nixpkgs/blob/b7488ba3b9b62fa8f1b08ea60200583750cde86f/pkgs/applications/editors/vim/plugins/overrides.nix#L968-L970

GaetanLepage avatar May 13 '24 12:05 GaetanLepage

Result of nixpkgs-review pr 302442 run on aarch64-darwin 1

1 package marked as broken and skipped:
  • lua54Packages.neorg
55 packages built:
  • lua51Packages.neorg
  • lua52Packages.neorg
  • lua53Packages.neorg
  • luajitPackages.neorg
  • vimPlugins.ChatGPT-nvim
  • vimPlugins.LazyVim
  • vimPlugins.LeaderF
  • vimPlugins.cmp-fuzzy-buffer
  • vimPlugins.cmp-fuzzy-path
  • vimPlugins.cmp-treesitter
  • vimPlugins.completion-treesitter
  • vimPlugins.crates-nvim
  • vimPlugins.flash-nvim
  • vimPlugins.fuzzy-nvim
  • vimPlugins.fzf-lua
  • vimPlugins.go-nvim
  • vimPlugins.hardhat-nvim
  • vimPlugins.haskell-scope-highlighting-nvim
  • vimPlugins.haskell-tools-nvim
  • vimPlugins.image-nvim
  • vimPlugins.jsonfly-nvim
  • vimPlugins.lazy-nvim
  • vimPlugins.lazygit-nvim
  • vimPlugins.leap-ast-nvim
  • vimPlugins.lspsaga-nvim
  • vimPlugins.multicursors-nvim
  • vimPlugins.neogit
  • vimPlugins.neorg
  • vimPlugins.nerdcommenter
  • vimPlugins.nerdtree
  • vimPlugins.nlsp-settings-nvim
  • vimPlugins.nvim-pqf
  • vimPlugins.nvim-tree-lua
  • vimPlugins.nvim-treesitter
  • vimPlugins.nvim-treesitter-textsubjects
  • vimPlugins.octo-nvim
  • vimPlugins.onedarkpro-nvim
  • vimPlugins.orgmode
  • vimPlugins.playground
  • vimPlugins.refactoring-nvim
  • vimPlugins.rest-nvim
  • vimPlugins.ssr
  • vimPlugins.telescope-cheat-nvim
  • vimPlugins.telescope-frecency-nvim
  • vimPlugins.telescope-fzf-native-nvim
  • vimPlugins.telescope-fzf-writer-nvim
  • vimPlugins.telescope-fzy-native-nvim
  • vimPlugins.telescope-media-files-nvim
  • vimPlugins.telescope-nvim
  • vimPlugins.telescope-symbols-nvim
  • vimPlugins.telescope-undo-nvim
  • vimPlugins.telescope-z-nvim
  • vimPlugins.telescope-zoxide
  • vimPlugins.unison
  • vimPlugins.vim-spirv

GaetanLepage avatar May 13 '24 13:05 GaetanLepage

Result of nixpkgs-review pr 302442 run on x86_64-linux 1

1 package marked as broken and skipped:
  • lua54Packages.neorg
55 packages built:
  • lua51Packages.neorg
  • lua52Packages.neorg
  • lua53Packages.neorg
  • luajitPackages.neorg
  • vimPlugins.ChatGPT-nvim
  • vimPlugins.LazyVim
  • vimPlugins.LeaderF
  • vimPlugins.cmp-fuzzy-buffer
  • vimPlugins.cmp-fuzzy-path
  • vimPlugins.cmp-treesitter
  • vimPlugins.completion-treesitter
  • vimPlugins.crates-nvim
  • vimPlugins.flash-nvim
  • vimPlugins.fuzzy-nvim
  • vimPlugins.fzf-lua
  • vimPlugins.go-nvim
  • vimPlugins.hardhat-nvim
  • vimPlugins.haskell-scope-highlighting-nvim
  • vimPlugins.haskell-tools-nvim
  • vimPlugins.image-nvim
  • vimPlugins.jsonfly-nvim
  • vimPlugins.lazy-nvim
  • vimPlugins.lazygit-nvim
  • vimPlugins.leap-ast-nvim
  • vimPlugins.lspsaga-nvim
  • vimPlugins.multicursors-nvim
  • vimPlugins.neogit
  • vimPlugins.neorg
  • vimPlugins.nerdcommenter
  • vimPlugins.nerdtree
  • vimPlugins.nlsp-settings-nvim
  • vimPlugins.nvim-pqf
  • vimPlugins.nvim-tree-lua
  • vimPlugins.nvim-treesitter
  • vimPlugins.nvim-treesitter-textsubjects
  • vimPlugins.octo-nvim
  • vimPlugins.onedarkpro-nvim
  • vimPlugins.orgmode
  • vimPlugins.playground
  • vimPlugins.refactoring-nvim
  • vimPlugins.rest-nvim
  • vimPlugins.ssr
  • vimPlugins.telescope-cheat-nvim
  • vimPlugins.telescope-frecency-nvim
  • vimPlugins.telescope-fzf-native-nvim
  • vimPlugins.telescope-fzf-writer-nvim
  • vimPlugins.telescope-fzy-native-nvim
  • vimPlugins.telescope-media-files-nvim
  • vimPlugins.telescope-nvim
  • vimPlugins.telescope-symbols-nvim
  • vimPlugins.telescope-undo-nvim
  • vimPlugins.telescope-z-nvim
  • vimPlugins.telescope-zoxide
  • vimPlugins.unison
  • vimPlugins.vim-spirv

GaetanLepage avatar May 13 '24 13:05 GaetanLepage

Result of nixpkgs-review pr 302442 run on x86_64-darwin 1

1 package marked as broken and skipped:
  • lua54Packages.neorg
55 packages built:
  • lua51Packages.neorg
  • lua52Packages.neorg
  • lua53Packages.neorg
  • luajitPackages.neorg
  • vimPlugins.ChatGPT-nvim
  • vimPlugins.LazyVim
  • vimPlugins.LeaderF
  • vimPlugins.cmp-fuzzy-buffer
  • vimPlugins.cmp-fuzzy-path
  • vimPlugins.cmp-treesitter
  • vimPlugins.completion-treesitter
  • vimPlugins.crates-nvim
  • vimPlugins.flash-nvim
  • vimPlugins.fuzzy-nvim
  • vimPlugins.fzf-lua
  • vimPlugins.go-nvim
  • vimPlugins.hardhat-nvim
  • vimPlugins.haskell-scope-highlighting-nvim
  • vimPlugins.haskell-tools-nvim
  • vimPlugins.image-nvim
  • vimPlugins.jsonfly-nvim
  • vimPlugins.lazy-nvim
  • vimPlugins.lazygit-nvim
  • vimPlugins.leap-ast-nvim
  • vimPlugins.lspsaga-nvim
  • vimPlugins.multicursors-nvim
  • vimPlugins.neogit
  • vimPlugins.neorg
  • vimPlugins.nerdcommenter
  • vimPlugins.nerdtree
  • vimPlugins.nlsp-settings-nvim
  • vimPlugins.nvim-pqf
  • vimPlugins.nvim-tree-lua
  • vimPlugins.nvim-treesitter
  • vimPlugins.nvim-treesitter-textsubjects
  • vimPlugins.octo-nvim
  • vimPlugins.onedarkpro-nvim
  • vimPlugins.orgmode
  • vimPlugins.playground
  • vimPlugins.refactoring-nvim
  • vimPlugins.rest-nvim
  • vimPlugins.ssr
  • vimPlugins.telescope-cheat-nvim
  • vimPlugins.telescope-frecency-nvim
  • vimPlugins.telescope-fzf-native-nvim
  • vimPlugins.telescope-fzf-writer-nvim
  • vimPlugins.telescope-fzy-native-nvim
  • vimPlugins.telescope-media-files-nvim
  • vimPlugins.telescope-nvim
  • vimPlugins.telescope-symbols-nvim
  • vimPlugins.telescope-undo-nvim
  • vimPlugins.telescope-z-nvim
  • vimPlugins.telescope-zoxide
  • vimPlugins.unison
  • vimPlugins.vim-spirv

GaetanLepage avatar May 13 '24 13:05 GaetanLepage

Result of nixpkgs-review pr 302442 run on aarch64-linux 1

1 package marked as broken and skipped:
  • lua54Packages.neorg
55 packages built:
  • lua51Packages.neorg
  • lua52Packages.neorg
  • lua53Packages.neorg
  • luajitPackages.neorg
  • vimPlugins.ChatGPT-nvim
  • vimPlugins.LazyVim
  • vimPlugins.LeaderF
  • vimPlugins.cmp-fuzzy-buffer
  • vimPlugins.cmp-fuzzy-path
  • vimPlugins.cmp-treesitter
  • vimPlugins.completion-treesitter
  • vimPlugins.crates-nvim
  • vimPlugins.flash-nvim
  • vimPlugins.fuzzy-nvim
  • vimPlugins.fzf-lua
  • vimPlugins.go-nvim
  • vimPlugins.hardhat-nvim
  • vimPlugins.haskell-scope-highlighting-nvim
  • vimPlugins.haskell-tools-nvim
  • vimPlugins.image-nvim
  • vimPlugins.jsonfly-nvim
  • vimPlugins.lazy-nvim
  • vimPlugins.lazygit-nvim
  • vimPlugins.leap-ast-nvim
  • vimPlugins.lspsaga-nvim
  • vimPlugins.multicursors-nvim
  • vimPlugins.neogit
  • vimPlugins.neorg
  • vimPlugins.nerdcommenter
  • vimPlugins.nerdtree
  • vimPlugins.nlsp-settings-nvim
  • vimPlugins.nvim-pqf
  • vimPlugins.nvim-tree-lua
  • vimPlugins.nvim-treesitter
  • vimPlugins.nvim-treesitter-textsubjects
  • vimPlugins.octo-nvim
  • vimPlugins.onedarkpro-nvim
  • vimPlugins.orgmode
  • vimPlugins.playground
  • vimPlugins.refactoring-nvim
  • vimPlugins.rest-nvim
  • vimPlugins.ssr
  • vimPlugins.telescope-cheat-nvim
  • vimPlugins.telescope-frecency-nvim
  • vimPlugins.telescope-fzf-native-nvim
  • vimPlugins.telescope-fzf-writer-nvim
  • vimPlugins.telescope-fzy-native-nvim
  • vimPlugins.telescope-media-files-nvim
  • vimPlugins.telescope-nvim
  • vimPlugins.telescope-symbols-nvim
  • vimPlugins.telescope-undo-nvim
  • vimPlugins.telescope-z-nvim
  • vimPlugins.telescope-zoxide
  • vimPlugins.unison
  • vimPlugins.vim-spirv

GaetanLepage avatar May 13 '24 13:05 GaetanLepage

@teto is he neorg = ... snippet in vim/plugins/overrides.nix needed anymore now ?

https://github.com/NixOS/nixpkgs/blob/b7488ba3b9b62fa8f1b08ea60200583750cde86f/pkgs/applications/editors/vim/plugins/overrides.nix#L968-L970

No. That was for neorg 7.x

mrcjkb avatar May 13 '24 20:05 mrcjkb

No. That was for neorg 7.x

Ok, I removed it !

GaetanLepage avatar May 13 '24 20:05 GaetanLepage

Everything seems to build fine now. Could someone actually test the plugin ?

I just set my flake to use your fork on the neorg branch, and upon calling require('neorg').setup(), I get

Warning [neorg]: lua-utils not found. If you're just installing the plugin, ignore this message, when in doubt run :Lazy build neorg. If you're not on lazy please rerun the build scripts.

So I think we need to wait for @teto's fix.

mrcjkb avatar May 13 '24 20:05 mrcjkb

Everything seems to build fine now. Could someone actually test the plugin ?

I just set my flake to use your fork on the neorg branch, and upon calling require('neorg').setup(), I get

Warning [neorg]: lua-utils not found. If you're just installing the plugin, ignore this message, when in doubt run :Lazy build neorg. If you're not on lazy please rerun the build scripts.

So I think we need to wait for @teto's fix.

Isn't staging-next(which I'm assuming what you're referring to) already merged?

purepani avatar May 14 '24 03:05 purepani

Isn't staging-next(which I'm assuming what you're referring to) already merged?

If it is, then I don't know why it didn't find the dependency. I'd have to look into that later. @teto any ideas?

mrcjkb avatar May 14 '24 04:05 mrcjkb

in staging-next, I mentioned lua fixes that helped with a neovim solution but not that it would contain a neovim fix. Because I wanted to quickly fix it in my fork I just go through the deps and add them to the lua environment seen by neovim: https://github.com/teto/nixpkgs/blob/1726887536c7fce3c877839100267cbae6d330f1/pkgs/applications/editors/neovim/wrapper.nix#L150 . It works but I am not sure it's the correct answer: this info should already be available in the lua hook. I tried to add LUA_PATH and LUA_CPATH in the neovim wrapper but seems like the lua hook does not look into propagatedBuildInputs (it should!) so it was useless for luasnip.

I was probably too optimistic for getting this into the release :/ I dont have much time in the upcoming days and it's delicate as it could break home-managers' implementation as well.

teto avatar May 15 '24 13:05 teto

@teto is he neorg = ... snippet in vim/plugins/overrides.nix needed anymore now ?

https://github.com/NixOS/nixpkgs/blob/b7488ba3b9b62fa8f1b08ea60200583750cde86f/pkgs/applications/editors/vim/plugins/overrides.nix#L968-L970

Then we should probably override the dependencies for the vim plugin for now (if we want to get this in the release). Perhaps with a note to remove it when the fix has landed. Some of the lua dependencies would have to be mapped to neovim plugins.

mrcjkb avatar May 15 '24 14:05 mrcjkb

if you rebase, I can try to see if this works with my patch on staging-next. I've tried with rocks.nvim and it worked so I am tempted to say yes but my neovim config is not "pure" :)

teto avatar Jun 19 '24 21:06 teto

Hey there. It should be working now.

Better late than never...

GaetanLepage avatar Sep 22 '24 22:09 GaetanLepage

Works for me locally targeting this PR. Will post nixpkgs-reviews

khaneliman avatar Sep 22 '24 22:09 khaneliman

Result of nixpkgs-review pr 302442 run on x86_64-linux 1

1 package marked as broken and skipped:
  • lua54Packages.neorg
6 packages built:
  • lua51Packages.neorg
  • lua52Packages.neorg
  • lua53Packages.neorg
  • luajitPackages.neorg
  • vimPlugins.neorg
  • vimPluginsUpdater

khaneliman avatar Sep 22 '24 22:09 khaneliman

Result of nixpkgs-review pr 302442 run on aarch64-darwin 1

1 package marked as broken and skipped:
  • lua54Packages.neorg
6 packages built:
  • lua51Packages.neorg
  • lua52Packages.neorg
  • lua53Packages.neorg
  • luajitPackages.neorg
  • vimPlugins.neorg
  • vimPluginsUpdater

khaneliman avatar Sep 22 '24 22:09 khaneliman

Result of nixpkgs-review pr 302442 run on x86_64-darwin 1

1 package marked as broken and skipped:
  • lua54Packages.neorg
6 packages built:
  • lua51Packages.neorg
  • lua52Packages.neorg
  • lua53Packages.neorg
  • luajitPackages.neorg
  • vimPlugins.neorg
  • vimPluginsUpdater

khaneliman avatar Sep 22 '24 22:09 khaneliman

nixpkgs-review result

Generated using nixpkgs-review.

  • Command: nixpkgs-review pr 302442
  • Systems: x86_64-linux,aarch64-linux,x86_64-darwin,aarch64-darwin

x86_64-linux

:fast_forward: 1 package marked as broken and skipped:
  • lua54Packages.neorg
:white_check_mark: 6 packages built:
  • lua51Packages.neorg
  • lua52Packages.neorg
  • lua53Packages.neorg
  • luajitPackages.neorg
  • vimPlugins.neorg
  • vimPluginsUpdater

aarch64-linux

:fast_forward: 1 package marked as broken and skipped:
  • lua54Packages.neorg
:white_check_mark: 6 packages built:
  • lua51Packages.neorg
  • lua52Packages.neorg
  • lua53Packages.neorg
  • luajitPackages.neorg
  • vimPlugins.neorg
  • vimPluginsUpdater

x86_64-darwin

:fast_forward: 1 package marked as broken and skipped:
  • lua54Packages.neorg
:white_check_mark: 6 packages built:
  • lua51Packages.neorg
  • lua52Packages.neorg
  • lua53Packages.neorg
  • luajitPackages.neorg
  • vimPlugins.neorg
  • vimPluginsUpdater

aarch64-darwin

:fast_forward: 1 package marked as broken and skipped:
  • lua54Packages.neorg
:white_check_mark: 6 packages built:
  • lua51Packages.neorg
  • lua52Packages.neorg
  • lua53Packages.neorg
  • luajitPackages.neorg
  • vimPlugins.neorg
  • vimPluginsUpdater

GaetanLepage avatar Sep 24 '24 06:09 GaetanLepage