Open airtonix opened 1 year ago
Let me know about any heresy I've committed. 😄
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
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?
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
:
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:
Busy today outside, hopefully I'll get to testing the changes to the OS build settings you wanted.
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
One thing, please use stylua for formatting with the provided config, just run stylua lua/ in root dir. Thanks
Will do.
I'm probably going to slow down on this one for the next few weeks, I'm back at work from holidays and have another project that's soaking up my free time.
ok changing the matrix builds to macos-12
and ubuntu-22.04
both seem to build successfully:
https://github.com/airtonix/telescope-fzf-native.nvim/actions/runs/4193778994
https://github.com/airtonix/telescope-fzf-native.nvim/releases/tag/dev
ok this works for me (on linux) with the following lazy config:
It does require exiting and running nvim again though.
I'll test out on windows soon
@Conni2461 Works on windows too now.
When you're happy with this I'll rebase the PR to clean it up.
the thing i am still thinking about is architecture, aarch64 and x86, we kinda need both for linux (doable: https://github.com/nvim-telescope/telescope-fzf-native.nvim/pull/93#issuecomment-1367177968) and macos, idk how to
but thanks for your work on this :)
the thing i am still thinking about is architecture, aarch64 and x86, we kinda need both for linux (doable: #93 (comment)) and macos, idk how to
The linux part seems trivial, I'll just grab the usage of that action you linked:
for macos, here's some thoughts:
but thanks for your work on this :)
👍🏻
jit.os
Contains the target OS name: "Windows", "Linux", "OSX", "BSD", "POSIX" or "Other".
jit.arch
Contains the target architecture name: "x86", "x64", "arm", "ppc", "ppcspe", or "mips".
Did some further searching around for how to support building arm binaries on osx, it should be possible since we're doing it at work for react native builds.
I'm not familiar enough with the topic of cmake/qmake/c/gcc etc but here's some stuff i found that might be relevant:
Will make a build run on my own repo for all the changes so far to test out the downloader.
❗ 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.
Ok @Conni2461 this is ready for review again. I've updated the initial comment ☝🏻
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.
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.
Yeah, when this pr gets merged there'll be binaries published to the releases area.
Will it be merged?
What is the status of this PR? Does anyone know why it still not merged?
Same question from here!
Would love to have prebuilt binaries or any kind of installer due to restricted Windows machines..
Thanks for you work!
fixes #69
main
will create or overwrite an existingdev
releasedownloader.lua
into a requireable postinstall step.curl
to do the http lifting via an async spawn call.vim.loop.fs_mkdir
to standardise creating an emptybuild
folder forcurl
to put the download intoI tried several methods before landing on
curl
andvim.loop.fs_mkdir
:http.socket
looked nice, except thatlazy.nvim
doesn't support luarocks likepacker.nvim
does, and vimplug i'm not sure of.gh auth login
. not everyone will have a github account.arch, platform and compiler supported
un tested
Release examples
dev
0.0.2
Consumer advice
the
README.md
has been updated with: