search

olets / zsh-abbr

The zsh manager for auto-expanding abbreviations, inspired by fish. ~18,000 clones by ~12,000 unique cloners as of Sept '24, and averaging ~60 Homebrew installs monthly since June 2023
https://zsh-abbr.olets.dev
Other
555 stars 19 forks source link

Drop .zwc files from repo #65

Closed z0rc closed 1 year ago

z0rc commented 1 year ago

I believe those files were committed by mistake. History doesn't hint any reason why they we added in the first place, and they aren't following changes in .zsh files:

As you may know .zwc is compiled script, internal representation of which depends zsh version, machine architecture and probably some other factors. So it doesn't make sense to keep .zwc files in repo, users who need them are able to generate them locally.

olets commented 1 year ago

Thanks for the feedback!

Possible they were added unintentionally in main (I agree they don't seem conceptually part of 2df61f96f142e7ba13ffecbaae1256254608cf25) but they're intentionally in v5 (in beta). My understanding is that providing the binaries could confer some (presumably v small in this case) performance benefit. zsh favors zwc files if they're present, so I think my thinking at the time (thanks to history rewriting it looks recent, but it was actually probably a year ago… know I researched a bit but don't know how I decided) was that distributing them would have a small first-load benefit for users with compatible environments.

Happy to be educated/corrected — it's not an aspect of zsh I know well. What's the reason to not distribute them?

I did notice when testing installing the v5 beta from GitHub that Git hit binaries' conflicts. Worked through it manually and haven't put more time into debugging yet, but I suppose dropping them is one easy solution. But maybe with (presumably v small) performance drawbacks?

NorthIsUp commented 1 year ago

My understanding is that providing the binaries could confer some (presumably v small in this case) performance benefit

i believe the zwc files are specific to the version/architecture of the zsh binary you are running. They do provide startup time benefits but it is generally up to the plugin manager to do the compilation and caching of them.

After installing with zinit on my system you can see it overwrite the .zwc files as a part of installation.

$ git --no-pager diff
diff --git a/zsh-abbr.plugin.zsh.zwc b/zsh-abbr.plugin.zsh.zwc
index 5a0be22..78857ab 100644
Binary files a/zsh-abbr.plugin.zsh.zwc and b/zsh-abbr.plugin.zsh.zwc differ
diff --git a/zsh-abbr.zsh.zwc b/zsh-abbr.zsh.zwc
index 066d8c4..da244b7 100644
Binary files a/zsh-abbr.zsh.zwc and b/zsh-abbr.zsh.zwc differ

Generally best practice is to not include them.

olets commented 1 year ago

can see it overwrite the .zwc files as a part of installation

Presumably a compatible environment wouldn't need to regenerate them though.

Generally best practice is to not include them.

Do you have a link or rationale? Interested in whether it's best practice… or common practice reflecting plugin authors (myself included) not necessarily knowing the cost/benefit (looking at "generally up to the plugin manager" here too). Not at all against dropping the binaries if it's net positive or neutral.

Are the binaries giving either of you trouble? Guessing yes or this Issue probably wouldn't exist.

z0rc commented 1 year ago

Are the binaries giving either of you trouble? Guessing yes or this Issue probably wouldn't exist.

Yes. I manage my dotfiles and zsh plugins via git submodules. Also I run zrecompile periodically on zsh files. This results constant unclean submodule state on number of machines, that don't generate zwc to same exact byte as committed in repo. With this state I have manually resolve conflicts on next time changes to zwc are committed in repo.

NorthIsUp commented 1 year ago

I'm having the same git issue with conflicts

Presumably a compatible environment wouldn't need to regenerate them though.

Yes! But this is what releases are for! You would need to make an architecture and zsh version specific build. But since .zwc files are a runtime optimization this is not a common distribution method. Python does the same thing. It generates .pyc cache file that are machine specific.

olets commented 1 year ago

Thanks @z0rc. Fixed in https://github.com/olets/zsh-abbr/releases/tag/v4.8.2