-
-
Notifications
You must be signed in to change notification settings - Fork 263
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
Packer v2 Rewrite #1042
Draft
wbthomason
wants to merge
25
commits into
master
Choose a base branch
from
refactor/packer-v2
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Packer v2 Rewrite #1042
Changes from all commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
0bafe57
Add some initial TODOs
wbthomason 8febf39
Put configuration into its own module
wbthomason 35ac1b2
WIP main module rewrite
wbthomason f8bb182
Implement specification flattening
wbthomason 999d0b7
Add ensure_table function
wbthomason a83314b
WIP flattening iterator for plugin specs
wbthomason 29cd719
Minor code cleanup, preparing to restart work
wbthomason d9941ec
Simplify headless check
wbthomason 5722bc5
Simplify ensure_table
wbthomason 4de55a4
Use new Lua commands API for Packer commands
wbthomason 10fb672
Use new Lua autocommands API for PackerComplete
wbthomason c6177f8
WIP moving to runtime-handlers implementation
wbthomason bf2fec5
Add path lib
wbthomason f603132
Make profile lib from compile functions
wbthomason f4b593f
Always ensure dependencies
wbthomason 395d0f6
Use path lib in config
wbthomason ff520c2
Signal existence of profiling threshold config var
wbthomason 0f13f80
Add hooks for modules that need to know about updated config values
wbthomason ae95b69
WIP everything-as-handlers-no-compile approach
wbthomason cb44036
Belay changing load for now
wbthomason b28cc1a
Add proper timed_load and rename old one to timed_packadd
wbthomason 5ca0cc0
Cleanup log module use a bit
wbthomason 681d95d
WIP handler definitions
wbthomason a076100
Add note about feature request from lewis6991
wbthomason c77c86a
First draft of rewrite roadmap
wbthomason File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
# Roadmap for Packer v2 | ||
|
||
`packer` has become bloated, compilation (at least as it currently stands) has proven confusing and probably not necessary, and the codebase has become messy and hard to maintain. | ||
|
||
As such, I'm proposing/working on (very slowly, as my OSS time is quite limited in my current job) an incremental rewrite of `packer`. | ||
The below is a high-level overview of this plan. | ||
Comments and additions are welcome and encouraged! | ||
|
||
## General principles | ||
|
||
`packer`'s code currently suffers from some poor software development practices largely stemming from getting "discovered" before it had time to become polished. | ||
We want to avoid these pitfalls in the rewrite: | ||
|
||
- Document all functions with emmylua-style comments: `packer` has very sparse documentation in its codebase. New/updated functions need to be appropriately documented. | ||
- Add tests where possible: testing a package manager can be a pain because of requirements to interact with the file system, etc. However, `packer`'s current tests do not cover much, and new code should at least strongly consider adding unit tests. | ||
- Avoid giant functions: `packer`'s logic contains some monstrous functions (e.g., `manage`, the main compilation logic, etc.) that are difficult to work with. New code should strive to use more, simpler functions in a more modular design. | ||
- Prioritize clean, performant code over total flexibility: `packer`'s design allows for a very flexible range of input formats. Although we don't want to completely lose this property, supporting all of these formats (and other use cases) has contributed to code bloat. If there's a choice to be made between losing some flexibility and significantly increasing the complexity of the code, seriously consider removing the complexity. | ||
- Reduce redundancy: `packer` currently has an annoying amount of redundant/overlapping functionality (the worst offenders are the `requires`/`wants`/`after` family of keywords, but there's also duplication of logic in the utilities, etc.). The rewrite should aim to reduce this. | ||
|
||
## Stage 1: Remove compilation and clean up main interface. Breaking changes | ||
|
||
We want the rewrite process to be as unintrusive as possible while still allowing for significant, potentially breaking changes. | ||
As such, the first stage should contain all of the significant breaking changes to reduce the number of times users need to adapt. | ||
The goals of the first stage are: | ||
|
||
- [ ] Mostly remove compilation: this step involves moving to dynamic handlers for plugin specs and only "compiling" (really, caching) information that we can automatically recompile as needed, like additional filepaths to source, etc. | ||
- [x] Make main module lightweight: if `packer` is no longer relying on the compiled file being lightweight, its main module must become cheaper to `require`, as this will be necessary at all startups. This is mostly a question of reducing the amount that `packer` pulls in at the top level (e.g., moving things like management operations more fully into their own modules, reducing the reliance on utils modules, etc.). | ||
- [x] Implement fast runtime handler framework: To replace the compiled file, we will introduce a notion of "keyword handlers": simple functions which are responsible for implementing, at runtime, the behavior of a single previously-compiled keyword. See `lua/packer/handlers.lua` for the current spec and implementation of this idea. | ||
- [ ] Implement compiled keywords as handlers: Work in progress/current state of the rewrite. All the existing keywords from `packer/compile.lua` need to be ported to handlers. This involves dealing with `compile.lua`'s tricky logic in some places, and is a key point for simplification. | ||
- [ ] Implement `load` function to process and execute handlers: The final step of moving away from compilation is to write a function that runs the handlers on the plugin specs to generate lazy-loaders, run `setup` and `config` functions, etc. The trick here will be making this sufficiently fast. | ||
- [ ] Potentially implement path caching, etc.: Once the bulk of compilation is gone, we may wish to investigate adding a little bit back by seeing where the runtime system spends time during startup. If this includes a significant amount of time checking information that only changes when a plugin is installed or updated (e.g., searching for runtime paths), then we may want to start caching this information in a file that gets updated on updates to save startup time. | ||
- [ ] Simplify `requires`/`wants`/etc. | ||
- [ ] Unify `requires`/`wants` into `depends_on`: `requires` and `wants` currently duplicate functionality. I propose merging the two into a single keyword `depends_on`, which will (1) ensure that dependencies are installed and (2) ensure that dependencies are loaded before the dependent plugin. | ||
- [ ] Make interface for `depends_on` and `after` consistent: the correct way to specify dependent plugins/sequential load plugins is confusing, since it's based on a plugin's short name in some cases but the full name in others (I think?). We should define and implement a clear way of referring to plugins in these settings. | ||
- [ ] Clean up interface | ||
- [ ] Use new Neovim v0.7.0 functions instead of `vim.cmd`: the codebase makes use of a lot of `vim.cmd` with string building for things like generating keymaps, commands, etc. Neovim v0.7.0 introduced the ability to create these directly from Lua, with direct Lua callbacks. `packer` should move to use these functions. | ||
- [ ] Make management operations sequenceable steps: To clean up and simplify the code for management operations (e.g., installs, updates, cleans, etc.), we want to redesign the operation interface into sequenceable steps (e.g., "check current FS state", "clone missing plugins", "pull installed plugins", etc.) so that the actual operations become a sequence of calls to functions implementing these steps, rather than the current highly-duplicated logic. | ||
|
||
## Stage 2: Simplify internal git functions and use of async | ||
|
||
The nastiest part of the codebase is the `git` logic. | ||
We would like to simplify this as much as possible. | ||
Also, a significant contributor to the difficulty of working on `packer` is its aggressive use of `async` everywhere. | ||
We should investigate how to minimize the async surface area to allow more flexibility and require less of the codebase to need to think about `async`. | ||
|
||
## Stage 3: Issue triage | ||
|
||
`packer` has accumulated a large number of outstanding issues. | ||
At this point, we should go through and try to reproduce issues with the updated codebase. | ||
During this process, issues should either be fixed or closed as "Wontfix" or "No longer relevant" unless they are feature requests or questions. | ||
The goal of this stage is to more or less close out the open issues. | ||
|
||
## Stage 4: Gradual porting to Teal | ||
|
||
Teal offers significant benefits for maintainability and has improved since the last time I took a look at porting `packer`. | ||
After `packer`'s code is at a cleaner, more reliable state, it makes sense to start porting modules incrementally to Teal. | ||
|
||
## Stage 5 and onward: new features, Luarocks improvements | ||
|
||
Finally, we can start thinking about new features and improvements. | ||
Things like the proposed unified plugin specification format, improvements to Luarocks (e.g., updating rocks, putting binary rocks on the Neovim `PATH`, etc.) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,86 @@ | ||
--- Configuration | ||
|
||
local path_utils = require 'packer.path' | ||
local stdpath = vim.fn.stdpath | ||
local join_paths = path_utils.join_paths | ||
local path_separator = path_utils.path_separator | ||
|
||
local defaults = { | ||
package_root = join_paths(stdpath 'data', 'site', 'pack'), | ||
compile_path = join_paths(stdpath 'config', 'plugin', 'packer_compiled.lua'), | ||
plugin_package = 'packer', | ||
max_jobs = nil, | ||
auto_clean = true, | ||
compile_on_sync = true, | ||
disable_commands = false, | ||
opt_default = false, | ||
transitive_opt = true, | ||
transitive_disable = true, | ||
auto_reload_compiled = true, | ||
git = { | ||
mark_breaking_changes = true, | ||
cmd = 'git', | ||
subcommands = { | ||
update = 'pull --ff-only --progress --rebase=false', | ||
install = 'clone --depth %i --no-single-branch --progress', | ||
fetch = 'fetch --depth 999999 --progress', | ||
checkout = 'checkout %s --', | ||
update_branch = 'merge --ff-only @{u}', | ||
current_branch = 'rev-parse --abbrev-ref HEAD', | ||
diff = 'log --color=never --pretty=format:FMT --no-show-signature HEAD@{1}...HEAD', | ||
diff_fmt = '%%h %%s (%%cr)', | ||
git_diff_fmt = 'show --no-color --pretty=medium %s', | ||
get_rev = 'rev-parse --short HEAD', | ||
get_header = 'log --color=never --pretty=format:FMT --no-show-signature HEAD -n 1', | ||
get_bodies = 'log --color=never --pretty=format:"===COMMIT_START===%h%n%s===BODY_START===%b" --no-show-signature HEAD@{1}...HEAD', | ||
submodules = 'submodule update --init --recursive --progress', | ||
revert = 'reset --hard HEAD@{1}', | ||
}, | ||
depth = 1, | ||
clone_timeout = 60, | ||
default_url_format = 'https://github.com/%s.git', | ||
}, | ||
display = { | ||
non_interactive = false, | ||
open_fn = nil, | ||
open_cmd = '65vnew', | ||
working_sym = '⟳', | ||
error_sym = '✗', | ||
done_sym = '✓', | ||
removed_sym = '-', | ||
moved_sym = '→', | ||
header_sym = '━', | ||
header_lines = 2, | ||
title = 'packer.nvim', | ||
show_all_info = true, | ||
prompt_border = 'double', | ||
keybindings = { quit = 'q', toggle_info = '<CR>', diff = 'd', prompt_revert = 'r' }, | ||
}, | ||
luarocks = { python_cmd = 'python' }, | ||
log = { level = 'warn' }, | ||
profile = { enable = false, threshold = nil }, | ||
} | ||
|
||
local M = { _hooks = {} } | ||
M.config = {} | ||
function M.configure(user_config) | ||
user_config = user_config or {} | ||
local config = M.config | ||
vim.tbl_deep_extend('force', defaults, user_config) | ||
config.package_root = string.gsub(vim.fn.fnamemodify(config.package_root, ':p'), path_separator .. '$', '', 1) | ||
config.pack_dir = join_paths(config.package_root, config.plugin_package) | ||
config.opt_dir = join_paths(config.pack_dir, 'opt') | ||
config.start_dir = join_paths(config.pack_dir, 'start') | ||
config.display.non_interactive = #vim.api.nvim_list_uis() == 0 | ||
for i = 1, #M._hooks do | ||
M._hooks[i](config) | ||
end | ||
|
||
return M.config | ||
end | ||
|
||
function M.register_hook(hook) | ||
M._hooks[#M._hooks + 1] = hook | ||
end | ||
|
||
return M |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally this looks great!
Regarding redundancy, there's a lot of code-duplication in
packer.update
andpacker.sync
. Maybe there are reasons for this but if things are structured well, it seems likesync
could just be something like:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the idea! I just haven't gotten to that part of the rewrite yet.