romgrk / fzy-lua-native

Luajit FFI bindings to FZY
116 stars 15 forks source link

Consider not storing the built executables in version control #23

Closed itaranto closed 1 year ago

itaranto commented 1 year ago

I don't really think I need to explain why you shouldn't store complied binaries in a source control system.

I get that's more "convenient", but binary files don't play nice why Git. And even worse, this are just not random binary files, they are derived from the source code.

Why not using Github CI and release features?

Build artifacts should have not place in version control.

romgrk commented 1 year ago

I don't really think I need to explain why you shouldn't store complied binaries in a source control system.

That's an interesting assertion. Fact: I did put my compiled binaries in source control. Therefore it would be fairly reasonable of you to assume that you should need to explain why it's a bad idea to do so.

I get that's more "convenient"

So tell me, what would be "better".

but binary files don't play nice why Git

They're not there for git. They're there for distribution.

And even worse, this are just not random binary files, they are derived from the source code.

Yes but you need something to convert source code to binary. You might have that something on your machine, but some other random person might not have it.

Why not using Github CI and release features?

Additional complexity, additional places where deployment & installation can fail. Simplicity is a feature.

Build artifacts should have not place in version control.

I am yet to hear a good reason. But I am impatiently waiting. Tell me how I should store my 180kb total of binaries with Github CI auto-deployed to Github releases with a script that I will spend time to write myself. All that just in time for the next change in the fzy algorithm, which should happen in approximately never.

romgrk commented 1 year ago

Closing, you can re-open if you have answers to the above.

Arniiiii commented 8 months ago

@romgrk Why it's not ok:

Binary can be not portable through other systems (Windows, MacOs, maybe BSDs, Solaris, w/e ) and/or other CPU architectures ( MIPS ? RISC-V ? w/e ? )

Someone wants build the binary in his own way and use his own binaries.

Someone just don't believe that those binary that you have are for the source code.

I had a problem with this:

Lazy package manager for neovim tries to "update" the package.

If I specify to build it (aka use make) , it fails to update the git clone'd repo because I replaced some binaries with what I built with your makefile.

I guess, It's enough reasoning I made a pull request to partially solve this.

romgrk commented 8 months ago

Pre-built binaries are just too convenient. Some users don't have a compiler installed and aren't familiar with building their own stuff. The unsafety point is moot, people are pulling this repo and executing the lua code anyway without checking what it does. Besides, it can be verified that those binaries are produced & committed into git by the github-actions user, and the CI runs are public.

The binaries are split by OS and CPU architecture btw, so either it's compatible or it's not run. I think maybe rare linux setups with muslc as the system libc might fail, but I've yet to receive a bug report.

A solution I could accept is that we store local builds to a gitignored path, e.g. ./static/fzy-local.so, and we try to import that path before importing the pre-built binaries. Do you want to re-open your PR with this approach?