Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat: Lockfile support#1010

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
EdenEast wants to merge36 commits intowbthomason:master
base:master
Choose a base branch
Loading
fromEdenEast:feat/lockfile

Conversation

@EdenEast
Copy link
Contributor

@EdenEastEdenEast commentedAug 11, 2022
edited
Loading

Implementation of feature request#1009.

Resolves:#1009

Items from issue discussion:

  • Add lockfile info to status
  • Manually specify lockfile generation output
  • Upgrade command moved to optional arguments
  • Command completion

smjonas, jalevin, jedwillick, obxhdx, younger-1, mutlusun, meuter, rewhile, wookayin, sarmong, and 8 more reacted with hooray emojismhc reacted with eyes emoji
@EdenEastEdenEastforce-pushed thefeat/lockfile branch 2 times, most recently fromd6cde5a toe9c523dCompareAugust 13, 2022 04:31
@EdenEastEdenEastforce-pushed thefeat/lockfile branch 2 times, most recently from2642894 to54ff9eeCompareAugust 22, 2022 13:29
@EdenEastEdenEast marked this pull request as ready for reviewAugust 29, 2022 19:15
@wbthomasonwbthomason self-requested a reviewSeptember 3, 2022 19:40
@smhc
Copy link
Contributor

smhc commentedSep 4, 2022

Couldn't this be achieved by the existing PR#849 ?

@wbthomason
Copy link
Owner

@smhc: First off, I'm sorry for missing your PR for so long! I was too busy forpacker for a while, and it looks to have come in during that time.

Second:@EdenEast has laid out some of the justification for lockfiles over snapshots here:#1009 (comment). Personally, I find the arguments of avoiding the snapshot rollup on every start and having more diff-friendly output compelling, but I'll also admit that I do not use and am unlikely to ever use either snapshots or lockfiles, so I may be missing some important points.

Perhaps there's a way we can combine the work from the two PRs?

@wbthomason
Copy link
Owner

Also@EdenEast do you mind fixing the conflicts before I review?

@smhc
Copy link
Contributor

smhc commentedSep 5, 2022
edited
Loading

Second:@EdenEast has laid out some of the justification for lockfiles over snapshots here:#1009 (comment).

The changes I made under my PR effectively resolve those issues. The snapshot file is loaded on startup and used as the 'commit' key for each plugin. Running 'upgrade' will ignore this key if it was set via the snapshot file. This allows upgrades even when a snapshot/lock file is in use.

It doesn't support pretty-print, but I added that myself independently via a custom mechanism through jq.

My changes aren't necessarily production quality, they were more just intended as a prototype to to explain how I think the snapshot stuff should work.

I'm happy to throw away my PR. However my 2c is that the snapshot behaviour should just be fixed (similar to how my PR fixes it) instead of an additional lockfile feature. I think the way I modified the snapshot functionality leaves it backwards compatible with how people may have been using it anyway.

If this PR improves the way clones are performed and has pretty printing that would be a useful addition.

@wbthomason
Copy link
Owner

@smhc: Ok, thanks. I need to read through both PRs in detail. I will note that I do not favor havingboth lockfiles and snapshots (as mentioned on#1009); a prerequisite for merging this PR would be replacing the snapshot feature.

desaster reacted with heart emoji

@EdenEast
Copy link
ContributorAuthor

In the context of a package/plugin manager a lockfile is a known concept and is understandable for users. The snapshot feature only partially converts this aspect. Having an integrated lockfile feature instead of a side snapshot feature would make the most sense. In the current issue we are talking about how to preserve the functionalitty of the snapshots for people that still want to save them. People where mostly trying to use the snapshots as a lockfile anyways. Guessing that there will be a deprecation period for the snapshot feature.@wbthomason can speak more on how it would be migrated (I can see the feature depricated when this is merged and removed by the v2 rewrite for example).

@EdenEastEdenEast marked this pull request as draftSeptember 6, 2022 20:52
@EdenEast
Copy link
ContributorAuthor

Converted this pr back to a draft while I implement the changes discussed in the feature request issue. Also have to update it with the recent PRs introduced.

@EdenEastEdenEastforce-pushed thefeat/lockfile branch 5 times, most recently from6136ac6 to865ef7eCompareSeptember 7, 2022 05:09
@EdenEastEdenEast marked this pull request as ready for reviewSeptember 7, 2022 05:10
@EdenEast
Copy link
ContributorAuthor

@wbthomason I updated this pr with the latest changes from master and the discussion in the issue and should be ready for a first pass review.

List of changes:

  • Moveupgrade command to optional arguments on existing commands
  • Create optional argument parser to facilitate option and flag parsing for commands
  • Lockfile command can now be overridden with optional args
  • Resolve issues with rebase against latestgit.lua changes
  • Add optional arguments to command completion
    • Options that take paths complete with filepaths

@EdenEast
Copy link
ContributorAuthor

Here is aminimal_init.lua file that I was using to test this branch:

minimal_init.lua
-- `minimal_init.lua` used for reproducible configuration-- Open with `nvim --clean -u minimal_init.lua`localis_windows=vim.fn.has'win32'==1localfunctionjoin(...)localsep=is_windowsand'\\'or'/'returntable.concat({... },sep)endlocalfunctionscript_path()localstr=debug.getinfo(2,'S').source:sub(2)returnstr:match'(.*/)'endlocalcwd=script_path()localroot_path=join(is_windowsandos.getenv'TEMP'or'/tmp','nvim')localpackage_root=join(root_path,'site','pack')localroot_plugin_path=join(package_root,'packer')localpacker_install_path=join(root_plugin_path,'start','packer.nvim')localpacker_compiled_path=join(root_path,'plugin','packer_compiled.lua')localpacker_lockfile_path=join(root_plugin_path,'start','packer.nvim','lockfile.lua')vim.opt.packpath=join(root_path,'site')vim.opt.runtimepath:prepend(root_path)vim.g.loaded_remote_plugins=1vim.opt.ignorecase=trueifvim.fn.isdirectory(packer_install_path)==0thenlocaldirname=vim.fs.dirname(packer_install_path)vim.fn.system {'mkdir','-p',dirname }vim.fn.system {'ln','-s',cwd,packer_install_path }vim.cmd.packadd'packer.nvim'endfunction_G.reload()forpack,_inpairs(package.loaded)doifvim.startswith('packer',pack)thenpackage.loaded[pack]=nilendendvim.cmd'source %'print'reloaded'endvim.keymap.set('n','<F1>',function()reload()end)vim.keymap.set('n','<F2>',function()vim.cmd'PackerUpdate'end)vim.keymap.set('n','<F3>',function()vim.cmd'PackerUpdate --nolockfile'end)localpacker=require'packer'localuse=packer.usepacker.init {compile_path=packer_compiled_path,package_root=package_root,log= {level='info'},lockfile= {enable=true,path=packer_lockfile_path,update_on_upgrade=true,  },}use(cwd)use {'christoomey/vim-tmux-navigator'}use {'EdenEast/nightfox.nvim',as='nightfox',config=function()vim.cmd.colorscheme'nightfox'end,}use {'rcarriga/nvim-notify',config=function()vim.notify=require'notify'end,}use {'sonph/onehalf',rtp='vim',}

@EdenEast
Copy link
ContributorAuthor

EdenEast commentedOct 15, 2022
edited
Loading

Reminder poke to@wbthomason for a review.😄

@mutlusun
Copy link

Thank you for your work!

I have tested your changes using my local config and so far everything works fine. The lockfile is created, taken into account when runningPackerUpdate and the changes written to the lockfile (if I passed--nolockfile).

Two small observations:

  • The name of the argument--nolockfile is maybe a bit confusing. It suggests that the lockfile is ignored, but it is actually updated.
  • If an extension is downgraded (in commit history) the git log from the desired commit to the latest commit is shown. I had the feeling that the extension is updated to the latest commit even though this was not the case. I'm not sure whether this is a problem of packer / this PR or more one of git log. (And it is not that severe 😄 )

Again, thanks for your work! This makes my setup much easier (unfortunately working with different nvim version on different clients)!

@EdenEast
Copy link
ContributorAuthor

Thanks for testing the PR!--nolockfile option only disables the lockfile from being applied for thatupdate command. The configuration optionlockfile.regen_on_update determines if the lockfile should be regenerated on the update command. This is defaulted tofalse. If you did not override this value then callingupdate should not regenerate the lockfile. Was this not the case for you?

As for the display, ya if a package is downgraded it does say that it has changed. I might be able to check how many commits behind the current commit is to origin. That might be something that would clarify the change. Would have to see how to show that in the display.

@EdenEast
Copy link
ContributorAuthor

Added the number of commits ahead or behind the new commit is from the prev one.

Screen Shot 2022-10-24 at 9 38 10 PM

younger-1 and budswa reacted with rocket emoji

EdenEastand others added22 commitsNovember 28, 2022 09:45
This change introduces a way to pass optional arguments to packercommands. Optional arguments are denoted by `--`. There are two types ofoptional arguments: `option` and `flags`.Option contains `=` with the attached valueEx: --path=/some/pathFlags are standalone and set their value to true.Ex: --nolockfile ({ nolockfile = true })This is used to add `--path` argument to the `lockfile` command.
There was an issue with a rebased change that was resolved.
If the status command is the first packer command run then the lockfilehas not been loaded yet. This would then end up not populating the`lockfile` key in the status command.
Co-authored-by: pynappo <43484729+pynappo@users.noreply.github.com>
@iverberk
Copy link

I'd like to mention that I've been using this functionality without issues and look forward to seeing it merged. Nice work!

@EdenEast
Copy link
ContributorAuthor

I am not sure on the plan for this PR. This PR might be merged in as it is now into the current version of packer ()v1) or I will have to port this functionality to v2 of packer once the core of it is stabilized.

I will keep this PR up to date with the master branch (v1). Anyone can use this branch until the feature is merged into either v1 or v2.

@iverberk
Copy link

@EdenEast thanks for keeping it up to date so that we can use this nice functionality. I'm quite happy with how things are setup with v1 and don't expect to benefit much from a rewrite (as an end-user), although I understand the motivation for it.

@EdenEast
Copy link
ContributorAuthor

@wbthomason could this pr be merged into packer. Version 2 will be some time before it is set as the main branch. This feature is done and can be used by users now. This feature will be added back into v2 when it is ready to be released. I know that there are a lot of people waiting for this feature to be added to packer.

vuki656, WilliamHsieh, jvcarli, meuter, rewhile, amar-laksh, and kyleamazza reacted with thumbs up emoji

jvcarli added a commit to jvcarli/cosmonauta.nvim that referenced this pull requestJan 2, 2023
[packer.nvim](https://github.com/wbthomason/packer.nvim)has some anti-features and bugs that were a deal breaker for me:- Bootstrapping is not straightforward- Snapshots are fundamentally broken, i.e.if I removed a plugin I tried to restore a snapshot packer would not work.- Luarocks install doesn't work on macos- Packer compilation step is annoying and sometimes makes config files out of syncwith the current setup, which makes debugging and plugin development awkward[lazy.nvim](https://github.com/folke/lazy.nvim) doesn't have a compilation step,doesn't require [impatient.nvim](https://github.com/lewis6991/impatient.nvim) for speeding upmodules initialization, has a straightforward bootstrap process and in general has a better design than[packer.nvim](https://github.com/wbthomason/packer.nvim).SEE:wbthomason/packer.nvim#814SEE:wbthomason/packer.nvim#1010SEE:wbthomason/packer.nvim#180
jvcarli added a commit to jvcarli/cosmonauta.nvim that referenced this pull requestJan 2, 2023
[packer.nvim](https://github.com/wbthomason/packer.nvim)has some anti-features and bugs that were a deal breaker for me:- Bootstrapping is not straightforward- Snapshots are fundamentally broken, i.e.if I removed a plugin I tried to restore a snapshot packer would not work.- Luarocks install doesn't work on macos- Packer compilation step is annoying and sometimes makes config files out of syncwith the current setup, which makes debugging and plugin development awkward[lazy.nvim](https://github.com/folke/lazy.nvim) doesn't have a compilation step,doesn't require [impatient.nvim](https://github.com/lewis6991/impatient.nvim) for speeding upmodules initialization, has a straightforward bootstrap process and in general has a better design than[packer.nvim](https://github.com/wbthomason/packer.nvim).SEE:wbthomason/packer.nvim#814SEE:wbthomason/packer.nvim#1010SEE:wbthomason/packer.nvim#180
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@wbthomasonwbthomasonAwaiting requested review from wbthomason

3 more reviewers

@pynappopynappopynappo left review comments

@shaunsinghshaunsinghshaunsingh left review comments

@bennypowersbennypowersbennypowers requested changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Feature: Lockfile support

9 participants

@EdenEast@smhc@wbthomason@mutlusun@meuter@shaunsingh@iverberk@bennypowers@pynappo

[8]ページ先頭

©2009-2025 Movatter.jp