feat: packaging #4270

LostNeophyte · 2023-07-02T07:27:50Z

adds the cmake setup from LunarVim-mono.
the ci job will create nightly releases on a schedule and create stable releases on tag push, if the stable release already exists it will upload files to it.

you can see it in action here
https://github.com/LostNeophyte/LunarVim/releases
https://github.com/LostNeophyte/LunarVim/actions/runs/5435290043

Packaging test

fix: appimage.sh

feat: add notes.md

kylo252

why not invoke the cmake script from that repo instead? basically clone it first and then run whatever build steps you want.
You can even create a custom github job from that repo to get a cleaner interface.

pros:

avoid adding a lot of files that are only relevant for distribution
won't clutter the repo's history with ci commits
adjusting build steps faster and easier

LostNeophyte · 2023-07-02T09:10:11Z

cmake needs to be together with the source code for homebrew

osalbahr · 2023-07-02T10:37:57Z

why not invoke the cmake script from that repo instead? basically clone it first and then run whatever build steps you want. You can even create a custom github job from that repo to get a cleaner interface.

pros:

avoid adding a lot of files that are only relevant for distribution

won't clutter the repo's history with ci commits

adjusting build steps faster and easier

Cloning would make the build artifacts non-deterministic and Homebrew compares against a shasum. See Homebrew/homebrew-core#132446 for more context. Specifically this comment.

kylo252 · 2023-07-03T08:31:41Z

Cloning would make the build artifacts non-deterministic and Homebrew compares against a shasum. See Homebrew/homebrew-core#132446 for more context. Specifically this comment.

@osalbahr, thanks for adding more context!

I'm still not sure about adding all those files into the repo, I was hoping we can get away with doing something along the these lines

class Lunarvim < Formula
  desc "IDE layer for Neovim. Completely free and community driven"
  homepage "https://www.lunarvim.org"
  url "https://github.com/LunarVim/LunarVim/archive/refs/tags/1.3.0.tar.gz"
  version "1.3.0"
  sha256 "2b6cefdf76d42e2d349381e63d42fa3c3e2527da25a464cb0a2af47bdea9e2a0"
  license "GPL-3.0-or-later"
  head "https://github.com/LunarVim/LunarVim.git", branch: "master"

  resource "LunarVim-mono" do
    url "https://github.com/LunarVim/LunarVim-mono/archive/refs/tags/stable.tar.gz" # replace stable with 1.3.0
    sha256 "4b63e968655d9a71ff32fdca653a56bd560555c1780d704669df578048b5f808"
  end

  depends_on "cmake" => :build
  depends_on "fd"
  depends_on "neovim"
  depends_on "node"
  depends_on "python@3.11"
  depends_on "ripgrep"
  depends_on "tree-sitter"

  def install
    resources.each do |r|
      r.stage(buildpath/"packaging"/r.name)
    end
    cd "packaging" do
      # ..
    end
  end

  test do
    assert shell_output("#{bin}/lvim -v").start_with?("NVIM")
    assert_equal "lmao", pipe_output("#{bin}/lvim -Es +%print", "lmao\n").strip
  end
end

@ChristianChiarulli, what's your stance on this?

LostNeophyte · 2023-07-03T20:13:30Z

why not invoke the cmake script from that repo instead? basically clone it first and then run whatever build steps you want. You can even create a custom github job from that repo to get a cleaner interface.

pros:
* avoid adding a lot of files that are only relevant for distribution

* won't clutter the repo's history with `ci` commits

* adjusting build steps faster and easier

I could probably make this pr quite a bit smaller. the build step would stay here, that would leave 151 loc (7 files 2 of which are icons) in this pr. the rest (github workflow, appimage, Packaging.cmake, BundlePlugins.cmake) would go to the LunarVim-mono repo.

kylo252 · 2023-07-04T09:42:10Z

I could probably make this pr quite a bit smaller. the build step would stay here, that would leave 151 loc (7 files 2 of which are icons) in this pr. the rest (github workflow, appimage, Packaging.cmake, BundlePlugins.cmake) would go to the LunarVim-mono repo.

Why not keep all "lunarvim-packaging" scripts/tools in a separate repo? You'd clone it into ./utils/packaging and invoke the cmake command there (both of these steps can be done through the main Makefile)

Keep in mind we probably need more scripts for integrating with other package formats (e.g. nix, scoop, winget,. etc.) so this packaging repo will only get more complex.

LostNeophyte · 2023-07-04T09:48:02Z

I could probably make this pr quite a bit smaller. the build step would stay here, that would leave 151 loc (7 files 2 of which are icons) in this pr. the rest (github workflow, appimage, Packaging.cmake, BundlePlugins.cmake) would go to the LunarVim-mono repo.

Why not keep all "lunarvim-packaging" scripts/tools in a separate repo? You'd clone it into ./utils/packaging and invoke the cmake command there (both of these steps can be done through the main Makefile)

Keep in mind we probably need more scripts for integrating with other package formats (e.g. nix, scoop, winget,. etc.) so this packaging repo will only get more complex.

I can agree with all the packaging stuff begin in a separate repo. just the build cmake would stay in the core, it makes packaging for others way easier and it makes sense for it to be versioned. I think it would be a nice compromise

kylo252 · 2023-07-05T08:41:45Z

I can agree with all the packaging stuff begin in a separate repo. just the build cmake would stay in the core, it makes packaging for others way easier and it makes sense for it to be versioned. I think it would be a nice compromise

Not sure which cmake files you mean here, since the main CMakeLists.txt wouldn't contain much logic anymore, and the cpack recipe is what I'm thinking can go in a separate repo.

Although, looking more into it, I see some ways to minimize the added files.

Please try moving the cmake logic into just two files: CMakeLists.txt and a cpack recipe. Let me know if you need help with that.

LostNeophyte · 2023-07-05T09:16:25Z

so now these files would be in the core:

CMakeLists.txt
utils/icons/*
utils/packaging/assets/lvim & - utils/packaging/assets/lvim.ps1 # these could be merged into the current bin templates and the cmake script would just remove the -u flag from the exec in the bash script

the rest would go to the lunarvim-mono repo

kylo252

Let's merge the icon files into utils/desktop
CMakeLists.txt should not be on the top level, it makes it look like it's necessary to use it to "build" the repo/config (I think should be possible to do)

LostNeophyte · 2023-07-07T08:48:54Z

CMakeLists.txt should not be on the top level, it makes it look like it's necessary to use it to "build" the repo/config (I think should be possible to do)

I tried to do that, and it seems like a pain because you'd need to access paths outside of the cmake project, but i'll do a bit more research on it

osalbahr · 2023-07-07T21:36:21Z

yea, we currently have no real way to install lvim from source, so packaging lvim is mostly impossible and that's what I want to change

I think this is a great feature that would benefit Homebrew and non-Homebrew users alike!

the aur package is broken since 1.2 and no longer maintained I believe (it's not official, I don't think the author of the package reached out to us)

Oh, makes sense. I actually don’t use the AUR very often so I don’t know how it works.

kylo252 · 2023-07-09T10:38:26Z

@LostNeophyte, I actually figured out a pretty simple solution that will, hopefully, satisfy everyone!

move the cmake files into utils/packaging
add appropriate make rules that will copy CMakeLists.txt and then invoke the correct cmake command, e.g. make package-brew

This will avoid any confusion about having a CMakeLists.txt in the repo's root when it's not really used for building (since this isn't a C/C++ project), basically solving my main complaint.

LostNeophyte · 2023-07-09T10:43:40Z

@LostNeophyte, I actually figured out a pretty simple solution that will, hopefully, satisfy everyone!
1. move the cmake files into `utils/packaging`

2. add appropriate `make` rules that will copy `CMakeLists.txt` and then invoke the correct cmake command, e.g. `make package-brew`
This will avoid any confusion about having a CMakeLists.txt in the repo's root when it's not really used for building (since this isn't a C/C++ project), basically solving my main complaint.

Sounds good to me, do I still put the ci workflow, appimage script and CPack in the lvim-mono repo?

osalbahr · 2023-07-09T15:33:46Z

when it's not really used for building (since this isn't a C/C++ project)

what is used for building LunarVim from source?

LostNeophyte · 2023-07-09T15:42:42Z

what is used for building LunarVim from source?

we don't build it, lunarvim doesn't require building. the installer only moves files to correct places. well, maybe you could argue that the lvim shell script is built, in that case we use sed to build it (just substituting a couple strings for LUNARVM_*_DIR env variables)

osalbahr · 2023-07-09T15:57:20Z

we don't build it, lunarvim doesn't require building. the installer only moves files to correct places. well, maybe you could argue that the lvim shell script is built, in that case we use sed to build it (just substituting a couple strings for LUNARVM_*_DIR env variables)

That’s great. If you list the exact step-by-step commands, we can just put these commands in the Homebrew formula. It seems no changes to the repo would be needed. Here is an simple example of a “def install” they can be just shell commands: https://docs.brew.sh/Formula-Cookbook#grab-the-url

but it would be easier to maintain if the commands were wrapped in a “./configure --PREFIX=” and then “make install” although not required.

The added benefit would be that someone manually installing from the master branch can also follow the same steps in the Homebrew formula. You can think of the “def install” section as a general and reproducible way of manually installing from source.

I think maybe that the script in the LunarVim basically does that? If so then we can probably use it.

LostNeophyte · 2023-07-09T16:25:20Z

we don't build it, lunarvim doesn't require building. the installer only moves files to correct places. well, maybe you could argue that the lvim shell script is built, in that case we use sed to build it (just substituting a couple strings for LUNARVM_*_DIR env variables)

That’s great. If you list the exact step-by-step commands, we can just put these commands in the Homebrew formula. It seems no changes to the repo would be needed. Here is an simple example of a “def install” they can be just shell commands: https://docs.brew.sh/Formula-Cookbook#grab-the-url

but it would be easier to maintain if the commands were wrapped in a “./configure --PREFIX=” and then “make install” although not required.

The added benefit would be that someone manually installing from the master branch can also follow the same steps in the Homebrew formula. You can think of the “def install” section as a general and reproducible way of manually installing from source.

yup, I don't want the commands to be in the formula, as they'd need to be replicated in many places.
I'll use cmake instead of ./configure because of windows, but since CMakeLists.txt will be in a subfolder it will be impossible to use cmake normally so I'll add a new make install target to move CMakeLists.txt and run cmake (with DESTDIR support) or a make configure target that will only move CMakeLists.txt to the root directory, I'm not sure which option makes more sense, or maybe there's a better option I'm missing

I think maybe that the script in the LunarVim basically does that? If so then we can probably use it.

it's close, not quite there. I think it's complex enough and I'd rather not add a new feature to it

osalbahr · 2023-07-09T16:37:28Z

yup, I don't want the commands to be in the formula, as they'd need to be replicated in many places.

Great point. I agree.

I'll use cmake instead of ./configure because of windows

Interesting. I haven’t done much development on windows this doesn’t surprise me.

but since CMakeLists.txt will be in a subfolder it will be impossible to use cmake normally so I'll add a new make install target to move CMakeLists.txt

That’s a nice workaround! Not sure if doing cd subfolder has any disadvantages, not really familiar with the cmake project structure. It seems it would simplify things as to not have to ../ everything.

kylo252 · 2023-07-10T08:05:19Z

Sounds good to me, do I still put the ci workflow, appimage script and CPack in the lvim-mono repo?

no, that's fine since it goes into .github/.

however, we should look into making it a github action if the scripts end up being too long/complex.

I'll add a new make install target to move CMakeLists.txt and run cmake (with DESTDIR support) or a make configure target that will only move CMakeLists.txt to the root directory, I'm not sure which option makes more sense, or maybe there's a better option I'm missing

it's something like this

diff --git a/Makefile b/Makefile
index 1f6844c5..bc842db5 100644
--- a/Makefile
+++ b/Makefile
@@ -39,4 +39,9 @@ style-sh:
 test:
 	bash ./utils/ci/run_test.sh "$(TEST)"
 
-.PHONY: install install-neovim-binary uninstall lint style test
+packaging-brew:
+	cp utils/packaging/CMakeLists.txt.in CMakeLists.txt
+	cmake --configure . -B build -D BUNDLE_PLUGINS=1
+	cpack
+
+.PHONY: packaging-brew install install-neovim-binary uninstall lint style test

chenrui333 · 2023-08-01T03:16:56Z

looks like homebrew PR is stale, might need another one to bring life to it.

LostNeophyte · 2023-08-11T13:26:17Z

@kylo252
I moved CMakeLists.txt out of the root,
moved the icons to the packaging folder,
added a make target configure that links CMakeLists to the root, this allows passing flags to cmake

we'll need to remember to change version strings before releases, i might create scripts/gh action to make it easier

LunarVim/utils/packaging/CMakeLists.txt.in

Lines 11 to 14 in 0465cf9

    
           set(LVIM_VERSION_MAJOR 1) 
        
           set(LVIM_VERSION_MINOR 4) 
        
           set(LVIM_VERSION_PATCH 0) 
        
           set(LVIM_VERSION_PRERELEASE "-dev")

kylo252

almost there! 🔥

utils/packaging/CMakeLists.txt.in

utils/packaging/assets/lvim

utils/packaging/cmake/BundlePlugins.cmake

kylo252 · 2023-08-18T11:54:23Z

utils/packaging/cmake/BundlePlugins.cmake

+set(XDG_ROOT ${CMAKE_BINARY_DIR}/xdg_root)
+set(LVIM_XDG_DATA_HOME ${XDG_ROOT}/share)
+set(LVIM_XDG_CONFIG_HOME ${XDG_ROOT}/.config)
+set(LVIM_XDG_CACHE_HOME ${XDG_ROOT}/.cache)


I don't have the full picture but I feel like this would require moving the plugins again since xdg_root is hardcoded in the path.

This should be easy to verify though, so please show the intended folder structure.

these folders are only used for the neovim instance to run. then the plugins are taken from there:

LunarVim/utils/packaging/cmake/BundlePlugins.cmake

Lines 52 to 54 in f1e610d

install(DIRECTORY

"${LUNARVIM_RUNTIME_DIR}/site/pack/lazy/opt/"

DESTINATION ${CMAKE_INSTALL_DATAROOTDIR}/lunarvim/plugins/)

utils/packaging/cmake/BundlePlugins.cmake

LostNeophyte · 2023-08-18T14:53:07Z

I didn't test it on windows yet, when i do I'll check if the / -> \\ substitution is necessary and make sure everything works

LostNeophyte · 2023-08-21T10:34:37Z

tested on windows and linux

kylo252 · 2023-08-23T11:13:22Z

utils/packaging/CMakeLists.txt.in

 if(PACKAGE_FOR_WINDOWS OR WIN32)
+  set(LVIM_INPUT_BIN_NAME lvim.ps1)
  set(LVIM_BIN_NAME lvim.ps1)
+  set(BASE_DIR_VAR "\$(Resolve-Path \"\$PSScriptRoot\\..\\${CMAKE_INSTALL_DATAROOTDIR}\\lunarvim\")")
 else()
+  set(LVIM_INPUT_BIN_NAME lvim.template)
  set(LVIM_BIN_NAME lvim)
+  set(BASE_DIR_VAR "\$(readlink -f \"\$(dirname \"\$(realpath \"\$0\")\")/..\")/${CMAKE_INSTALL_DATAROOTDIR}/lunarvim")


how about using PROJECT_SOURCE_DIR? unless you're referencing the destination, then it should just be using CMAKE_INSTALL_xxx.

if it's easier, you could make it "build" the scripts and then you can install them according to installation rules.

it's for the destination, it needs to be relative for the appimage, the paths are random (e.g /tmp/.mount_lvim(1mk5W7d/usr/share/lunarvim)

it needs to be relative for the appimage

do you mean inside the appimage rather than relative? What I'm trying to say is that having /../ somewhere in path doesn't necessarily make it relative. I don't see how BASE_DIR_VAR wouldn't look like this /some/path/../share/lunarvim, which might even get resolved when using readlink.

we can't use a fixed absolute path https://docs.appimage.org/reference/best-practices.html?highlight=relocatable#binaries-must-not-use-compiled-in-absolute-paths
the suggested way to deal with this is to make paths relative to the executable, so here the path is PATH_TO_EXE/../${CMAKE_INSTALL_DATAROOTDIR}/lunarvim (/tmp/.mount_lvim(1mk5W7d/usr/bin/../share/lunarvim in appimage, /usr/bin/../share/lunarvim in a normal installation)

kylo252 · 2023-08-23T11:15:22Z

utils/installer/install.ps1

@@ -184,7 +184,8 @@ function setup_shim() {
        New-Item "$INSTALL_PREFIX\bin" -ItemType Directory | Out-Null
    }

-    Copy-Item -Force "$env:LUNARVIM_BASE_DIR\utils\bin\lvim.ps1" "$INSTALL_PREFIX\bin\lvim.ps1"
+    ((Get-Content -path "$env:LUNARVIM_BASE_DIR\utils\bin\lvim.ps1" -Raw) -replace '@BASE_DIR_VAR@','$env:LUNARVIM_RUNTIME_DIR\lvim') |


was this difficult to handle with cmake as well?

I didn't think this one through, I also made this relative but appimage isn't for windows 🤦

LostNeophyte added 16 commits June 13, 2023 10:14

feat(packaging): add CMakeLists.txt

23533b5

feat(packaging): add GenerateVersion.cmake

3b1b381

Add CPack, bundle plugins

fe13877

bump version

6ae03b3

feat: add gh workflow

ff17c81

Merge branch 'LunarVim:master' into packaging

a321ddb

Merge pull request #8 from LostNeophyte/packaging

3664dc9

Packaging test

chore: format

79ed71d

chmod +x

6184d44

Merge pull request #9 from LostNeophyte/packaging

b918fb4

Packaging test

fix: appimage.sh

fead7e9

Merge pull request #10 from LostNeophyte/packaging

bf8b7d5

fix: appimage.sh

feat: add notes.md

78fa0a0

Merge pull request #11 from LostNeophyte/packaging

5e59c08

feat: add notes.md

feat: upload files to existing release

0381944

Update lvim.ps1

e8253d5

LostNeophyte requested a review from kylo252 July 2, 2023 07:27

LostNeophyte mentioned this pull request Jul 2, 2023

[Feature]: LunarVim Bottle for Home-brew #1021

Open

kylo252 requested changes Jul 2, 2023

View reviewed changes

merge cmake files

b779ec6

kylo252 requested changes Jul 7, 2023

View reviewed changes

kylo252 mentioned this pull request Jul 10, 2023

fix(installer): use a tmp config dir during lazy setup #4278

Open

LostNeophyte added 3 commits August 11, 2023 14:42

chore: move all icons to desktop folder

7611410

fix(lvim shim): allow changing NVIM_APPNAME

4951f90

refactor: move CMakeLists.txt out of the root

0465cf9

LostNeophyte requested a review from kylo252 August 11, 2023 13:26

fix: add make configure to release ci

d942bfd

kylo252 reviewed Aug 18, 2023

View reviewed changes

LostNeophyte added 3 commits August 18, 2023 14:55

increase bundle plugins timeout

f1e610d

refactor(packaging): use utils/bin templates

17fbfd4

refactor(packaging): use configure_file

a2dae16

LostNeophyte added 5 commits August 18, 2023 17:07

fix

b3bf596

fix

f01adae

fix path

9783563

chore: format

fbe9021

refactor: remove unnecessary substitutions

1d4123a

LostNeophyte requested a review from kylo252 August 21, 2023 10:45

kylo252 reviewed Aug 23, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: packaging #4270

feat: packaging #4270

LostNeophyte commented Jul 2, 2023

kylo252 left a comment

LostNeophyte commented Jul 2, 2023

osalbahr commented Jul 2, 2023 •

edited

kylo252 commented Jul 3, 2023

LostNeophyte commented Jul 3, 2023

kylo252 commented Jul 4, 2023

LostNeophyte commented Jul 4, 2023

kylo252 commented Jul 5, 2023

LostNeophyte commented Jul 5, 2023

kylo252 left a comment

LostNeophyte commented Jul 7, 2023

osalbahr commented Jul 7, 2023

kylo252 commented Jul 9, 2023

LostNeophyte commented Jul 9, 2023

osalbahr commented Jul 9, 2023

LostNeophyte commented Jul 9, 2023

osalbahr commented Jul 9, 2023 •

edited

LostNeophyte commented Jul 9, 2023

osalbahr commented Jul 9, 2023 •

edited

kylo252 commented Jul 10, 2023

chenrui333 commented Aug 1, 2023

LostNeophyte commented Aug 11, 2023 •

edited

kylo252 left a comment

kylo252 Aug 18, 2023

LostNeophyte Aug 18, 2023

LostNeophyte commented Aug 18, 2023

LostNeophyte commented Aug 21, 2023

kylo252 Aug 23, 2023

LostNeophyte Aug 26, 2023

kylo252 Aug 27, 2023

LostNeophyte Aug 27, 2023

kylo252 Aug 23, 2023

LostNeophyte Aug 26, 2023

	install(DIRECTORY
	"${LUNARVIM_RUNTIME_DIR}/site/pack/lazy/opt/"
	DESTINATION ${CMAKE_INSTALL_DATAROOTDIR}/lunarvim/plugins/)

feat: packaging #4270

Are you sure you want to change the base?

feat: packaging #4270

Conversation

LostNeophyte commented Jul 2, 2023

kylo252 left a comment

Choose a reason for hiding this comment

LostNeophyte commented Jul 2, 2023

osalbahr commented Jul 2, 2023 • edited

kylo252 commented Jul 3, 2023

LostNeophyte commented Jul 3, 2023

kylo252 commented Jul 4, 2023

LostNeophyte commented Jul 4, 2023

kylo252 commented Jul 5, 2023

LostNeophyte commented Jul 5, 2023

kylo252 left a comment

Choose a reason for hiding this comment

LostNeophyte commented Jul 7, 2023

osalbahr commented Jul 7, 2023

kylo252 commented Jul 9, 2023

LostNeophyte commented Jul 9, 2023

osalbahr commented Jul 9, 2023

LostNeophyte commented Jul 9, 2023

osalbahr commented Jul 9, 2023 • edited

LostNeophyte commented Jul 9, 2023

osalbahr commented Jul 9, 2023 • edited

kylo252 commented Jul 10, 2023

chenrui333 commented Aug 1, 2023

LostNeophyte commented Aug 11, 2023 • edited

kylo252 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LostNeophyte commented Aug 18, 2023

LostNeophyte commented Aug 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

osalbahr commented Jul 2, 2023 •

edited

osalbahr commented Jul 9, 2023 •

edited

osalbahr commented Jul 9, 2023 •

edited

LostNeophyte commented Aug 11, 2023 •

edited