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

feat: provide prebuilt release binaries and installer #93

Open
wants to merge 69 commits into
base: main
Choose a base branch
from

Conversation

airtonix
Copy link

@airtonix airtonix commented Dec 26, 2022

fixes #69

  • Re-organised workflows into reusable local composite actions
  • Implemented the creation of releases:
    • pushes to main will create or overwrite an existing dev release
    • pushed tags will create their own release, push the tag again to update that release
    • all binaries are uploaded with filenames that reflect the OS, arch and compiler used
  • Adapted the downloader.lua into a requireable postinstall step.
    • uses curl to do the http lifting via an async spawn call.
    • uses vim.loop.fs_mkdir to standardise creating an empty build folder for curl to put the download into

I tried several methods before landing on curl and vim.loop.fs_mkdir :

  • http.socket looked nice, except that lazy.nvim doesn't support luarocks like packer.nvim does, and vimplug i'm not sure of.
  • the github cli, not everyone will have this installed, and even then using it requires the user to first run gh auth login. not everyone will have a github account.

arch, platform and compiler supported

  • windows x86 vcc
  • linux x86 gcc
  • linux x86 clang
  • linux arm gcc
  • linux arm clang
  • osx x86 gcc
  • osx x86 clang

un tested

  • osx arm gcc
  • osx arm clang

Release examples

dev

0.0.2

Consumer advice

the README.md has been updated with:

download options

🤚 Note

You'll need to have curl installed.

On windows, this is done by installing git, and on linux and mac this should already be solved.
Technically speaking this shouldn't be a problem because most neovim plugin managers encourage git to be present.

{
  'nvim-telescope/telescope-fzf-native.nvim',
  build = function()
    require('telescope-fzf-native').download_library({
        platform = 'windows' -- windows | ubuntu | macos
        arch = 'x64', -- x64 | arm
        compiler = 'cc', -- windows: cc, unix: gcc | clang
        version = 'latest' -- any release name found on our github releases page
    })
  end
}

@airtonix airtonix marked this pull request as ready for review December 27, 2022 09:10
@airtonix
Copy link
Author

Let me know about any heresy I've committed. 😄

@airtonix airtonix force-pushed the feature/69-prebuilt-release-binaries branch from c526f25 to c3edf79 Compare December 27, 2022 10:19
@airtonix airtonix changed the title feat: 69 prebuilt release binaries feat: prebuilt release binaries Dec 28, 2022
@airtonix airtonix changed the title feat: prebuilt release binaries feat: provide prebuilt release binaries and installer Dec 28, 2022
@Conni2461
Copy link
Member

You havent commited any "heresy". Thank you for doing that.

I'll gonna nitpick a little bit and ci is also gonna nitpick, mostly about formatting

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@Conni2461
Copy link
Member

mostly looks great, i am happy that you went with curl over http.socket, because i am not to keen on having a luarocks dependency here.

I have to look closer into the release action because i am not that familiar with it, so i havent 100% completed the review

One thing that i am missing is, m1 support. I think github currently has no runners, so we cant provide binaries for that architecture?! Can we cross compile?

@airtonix
Copy link
Author

airtonix commented Dec 28, 2022

mostly looks great, i am happy that you went with curl over http.socket, because i am not to keen on having a luarocks dependency here.

I have to look closer into the release action because i am not that familiar with it, so i havent 100% completed the review

One thing that i am missing is, m1 support. I think github currently has no runners, so we cant provide binaries for that architecture?! Can we cross compile?

Not too familiar with macs, or even c++ for that matter. I'm basically a filthy frontend developer peasant that doesn't use apple.

But from all the work I did in my dayjob to automate the creation of our android and ios apps, I know that there's this tool that apple gives us : arch:

@airtonix
Copy link
Author

Also once you've looked over the changes to github actions, they'll need your approval to run here in this PR.

summary of changes there is basically:

  • make all the common steps re-usable as local actions. github calls these "composite actions".
  • to make defining build output consistent, use a matrix build for both windows and unix steps (this is where I'm not sure how trivial it is to make windows build with clang as well as vcc).
  • just in case you want to split the release process into two more more separate workflow files, move the actual grunt work of the release creating into a composite action.

Busy today outside, hopefully I'll get to testing the changes to the OS build settings you wanted.

@Conni2461
Copy link
Member

Conni2461 commented Dec 29, 2022

I already looked at all action files apart from the files that create the release binaries, the other files look good and it makes sense to split them up.

I still have to basically read about: ncipollo/release-action@v1 so yeah and i know that i have to approve CI for new contributors. I already ran it once ^^

I am fine with just providing vcc binaries on windows and not providing any clang or gcc binaries for that platform.

We also should build aarch64 for ubuntu (currently looking at https://github.com/romgrk/fzy-lua-native/blob/master/.github/workflows/main.yml). So we should include the architecture in the released filename and then we can use jit.arch to the the architecture in the downloader, it should return either x86_64 or arm64

One thing, please use stylua for formatting with the provided config, just run stylua lua/ in root dir. Thanks

@airtonix
Copy link
Author

airtonix commented Apr 4, 2023

https://andrewkelley.me/post/zig-cc-powerful-drop-in-replacement-gcc-clang.html

🤯 I've added another job: https://github.com/airtonix/telescope-fzf-native.nvim/blob/047f5c440d1b34ff1f448b0b7041827889bd9abb/.github/workflows/release.yml#L115-L174

If that works, we can get rid of the qemu/mac/windows runners and greatly simplify all of the github actions.

we'd also be able to reduce the platform specific checks down to something like

set TARGET to env.TARGET or libfzf.so

if platform is windows
  set TARGET to env.TARGET or libfzf.dll

then ditch the cmake stuff, and instead use zig everywhere.

edit:

T_T didn't work:

Prepare all required actions
Getting action download info
Download action repository 'goto-bus-stop/setup-zig@v[2](https://github.com/airtonix/telescope-fzf-native.nvim/actions/runs/4613438909/jobs/8155411976#step:3:2)' (SHA:869a4299cf8ac7db4ebffaec[3](https://github.com/airtonix/telescope-fzf-native.nvim/actions/runs/4613438909/jobs/8155411976#step:3:3)6ad82a682f88acb)
Run ./.github/actions/with-zig
Run goto-bus-stop/setup-zig@v2
/usr/bin/tar x --warning=no-unknown-keyword --overwrite -C /home/runner/work/_temp/7297acee-2663-[4](https://github.com/airtonix/telescope-fzf-native.nvim/actions/runs/4613438909/jobs/8155411976#step:3:4)c0e-ba[9](https://github.com/airtonix/telescope-fzf-native.nvim/actions/runs/4613438909/jobs/8155411976#step:3:10)5-ada7ee08fe91 -f /home/runner/work/_temp/b151603a-deb4-4ef0-93af-a701e1903ec3
Run zig cc --version
clang version 15.0.7 (https://github.com/ziglang/zig-bootstrap a3a6e85f9ec95b1772f5ace363e46df2f336c6b8)
Target: x86_64-unknown-linux-musl
Thread model: posix
InstalledDir: /usr/bin
Cloning into 'examiner'...
mkdir -p ./build
zig cc -target aarch64-macos-none -O2 -Wall -Werror -Wshadow -Wconversion -fpic -shared src/examiner.c -o ./build/libexaminer.so
install -m 755 -d /usr/local/lib/
install -m 644 build/libexaminer.so /usr/local/lib/
install -m 755 -d /usr/local/include/
install -m 644 src/examiner.h /usr/local/include/
install -m 755 -d /usr/local/lib/pkgconfig
install -m 644 libexaminer.pc.in /usr/local/lib/pkgconfig/libexaminer.pc
sed -e "s:^prefix=.*:prefix=/usr/local:" -e "s:@@VER@@::" < libexaminer.pc.in > /usr/local/lib/pkgconfig/libexaminer.pc
Run make test
mkdir -p build
zig cc -target aarch64-macos-none -O3 -Wall -Werror -fpic -std=gnu99 -shared src/fzf.c -o build/libfzf.so
zig cc -target aarch64-macos-none -Og -ggdb3 -Wall -Werror -fpic -std=gnu99 test/test.c -o build/test -I./src -L./build -lfzf -lexaminer
test/test.c:3:10: fatal error: 'examiner.h' file not found
#include <examiner.h>
         ^~~~~~~~~~~~
1 error generated.
make: *** [Makefile:[27](https://github.com/airtonix/telescope-fzf-native.nvim/actions/runs/4613438909/jobs/8155411976#step:3:30): build/test] Error 1
Error: Process completed with exit code 2.

@airtonix
Copy link
Author

airtonix commented Apr 5, 2023

Ok @Conni2461 this is ready for review again. I've updated the initial comment ☝🏻

@adelarsq
Copy link

adelarsq commented Apr 12, 2023

Cool! I am interested in something like this. I was able to compile using Zig on Windows from using:

zig cc -O3 -Wall -Werror -fpic -std=gnu99 -shared src/fzf.c -o build\libfzf.dll 

But wasn't possible to use the compiled library for some reason. I will take a look to discover what is happening.

@cnt0
Copy link

cnt0 commented Jun 12, 2023

I believe that prebuilt binaries should be installed using mason.

To do that, you need to add prebuilt binaries to github releases and either make a PR to mason core registry or set up your own registry.

Also, I added cmake stuff for cross-compilation with zig cc (here). @airtonix @adelarsq you might be interested.

@airtonix
Copy link
Author

Yeah, when this pr gets merged there'll be binaries published to the releases area.

@XantreDev
Copy link

Will it be merged?

@weirdgiraffe
Copy link

What is the status of this PR? Does anyone know why it still not merged?

@kohennigs
Copy link

Same question from here!

Would love to have prebuilt binaries or any kind of installer due to restricted Windows machines..

Thanks for you work!

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.

provide pre build binaries over CI or release
7 participants