Bug 251420 - editors/vim /usr/local/share/vim/vim82/defaults.vim overrides /usr/local/etc/vim/vimrc
Summary: editors/vim /usr/local/share/vim/vim82/defaults.vim overrides /usr/local/etc/...
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Adam Weinberger
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-11-27 08:01 UTC by mack
Modified: 2022-09-04 16:34 UTC (History)
5 users (show)

See Also:
bugzilla: maintainer-feedback? (adamw)


Attachments
vim version info and order of processed config files (6.05 KB, text/plain)
2020-11-27 08:01 UTC, mack
no flags Details
Only source defaults.vim if none of the documented startup files exist (830 bytes, patch)
2022-08-10 15:30 UTC, Tim Chase
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description mack 2020-11-27 08:01:28 UTC
Created attachment 220026 [details]
vim version info and order of processed config files

The order of processing the config files seems to be odd, as /usr/local/share/vim/vim82/defaults.vim overrides /usr/local/etc/vim/vimrc .

In practice, setting "mouse=r" in /usr/local/etc/vimrc doesn't have any effect, as the mouse option gets overriden in the defaults file.
Comment 1 mack 2020-11-27 08:12:37 UTC
This problem probably lead to the unanswered question at https://forums.freebsd.org/threads/global-configuration-for-vim.60321/

So either the documentation is incorrect or incomplete:

       /usr/local/etc/vim/vimrc                                      
                      System wide Vim initializations.
"... which is overriden by settings in ..."

or the order should be different.
Comment 2 Adam Weinberger freebsd_committer 2021-07-24 19:27:42 UTC
I think this is more of an upstream problem than a FreeBSD problem. We don't alter the load order in any way, so (as you mentioned) either the load order or the documentation about it is wrong. Either way, this seems like a problem that needs to get fixed upstream.

Vim bugs are filed at https://github.com/vim/vim/issues
Can you report there?
Comment 3 Anton Saietskii 2022-06-02 14:07:00 UTC
Long, long existing issue. I don't remeber all the details, but using following as a workaround for several years:
> # cat /usr/local/etc/vim/vimrc
> source $VIMRUNTIME/defaults.vim
> set background=dark
> hi Comment ctermfg=6
> set mouse=
> set scrolloff=1
> set modeline
> set viminfo=""
> let skip_defaults_vim=1
So, 1) source defaults if you need them, 2) set personal options, 3) set defaults to ignore.
Comment 4 Adam Weinberger freebsd_committer 2022-06-02 16:51:27 UTC
(In reply to Anton Saietskii from comment #3)

Ah, that's a good approach.

It's odd that defaults.vim overrides the vimrc, but not if it's sourced first. However, if it works then that's good enough for me.

I can't think of any major downsides to adding `source $VIMRUNTIME/defaults.vim` to the top of files/vimrc. Have you encountered any problems with your approach?
Comment 5 Anton Saietskii 2022-06-02 17:05:21 UTC
(In reply to Adam Weinberger from comment #4)

Nope, works flawlessly (if I'm not blind, lol). Just don't forget to add skip_defaults_vim after your local changes as in example.
Comment 6 commit-hook freebsd_committer 2022-06-02 17:42:12 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=f27ea6b18516fad5be76ba20ce8f53f5ba9c1081

commit f27ea6b18516fad5be76ba20ce8f53f5ba9c1081
Author:     Adam Weinberger <adamw@FreeBSD.org>
AuthorDate: 2022-06-02 17:36:43 +0000
Commit:     Adam Weinberger <adamw@FreeBSD.org>
CommitDate: 2022-06-02 17:41:47 +0000

    editors/vim: Update to 8.2.5052 and fix vimrc clobbering

    Vim runtime's defaults.vim will clobber global vimrc settings. It's
    fine for the settings we provide, but other edits to that file can
    get lost.

    To work around this, defaults.vim is now directly sourced and a flag
    is set to stop that file from getting loaded a second time. Thanks go
    to Anton Saietskii for that bit of magic.

    PR:     251420

 editors/vim/Makefile    | 2 +-
 editors/vim/distinfo    | 6 +++---
 editors/vim/files/vimrc | 5 +++++
 3 files changed, 9 insertions(+), 4 deletions(-)
Comment 7 Adam Weinberger freebsd_committer 2022-06-02 17:46:23 UTC
OK, done!

I appreciate everyone's patience on this bug. Anton, thank you for reaching out with a solution!

I'm closing out this PR. Please reply to this bug if you encounter any further problems with this and I'll reopen it.
Comment 8 kcwu 2022-06-27 17:03:05 UTC
According to vim's doc, defaults.vim won't be sourced if I have $HOME/.vimrc . However, with your recent change, defaults.vim is always sourced. This make me confused why vim suddenly behave differently (scrolloff=5).
Comment 9 Anton Saietskii 2022-06-28 09:07:24 UTC
(In reply to kcwu from comment #8)
Uhm, vim load order is complete mess... BTW, have you tried adding this to ~/.vimrc?
> let skip_defaults_vim=1
> <YOUR OPTIONS HERE>
Comment 10 kcwu 2022-06-28 09:28:09 UTC
(In reply to Anton Saietskii from comment #9)
It doesn't work because personal .vimrc is loaded after system vimrc.
Comment 11 Adam Weinberger freebsd_committer 2022-07-01 14:18:12 UTC
(In reply to kcwu from comment #10)

I agree that this is a problem, but I honestly have no idea how to address this.

The vimrc sourcing order is absurd. I don't know that it's fixable without simply dropping the FreeBSD global vimrc. However, vim defaults are simply dumb and ought to be overridden.
Comment 12 Anton Saietskii 2022-07-05 08:46:32 UTC
(In reply to Adam Weinberger from comment #11)
I've seen similar reports in vim's bugtracker. Not with exactly configs but rather something like syntax modules, etc. Looks like issue is deeper.
Perhaps dropping vimrc would really be a solution. Kinda joke, but vim is such a piece of software where one can see 2 types of users:
1. Those using default settings.
2. Those copying their custom vimrc to different machines for a long decades.
Comment 13 Adam Weinberger freebsd_committer 2022-07-05 13:47:37 UTC
A couple other approaches:

1) Patch $VIMRUNTIME/defaults.vim itself. There's a POLA problem there, but we might be getting deeper into POLA with all our current changes.

2) We could import all of the important settings from defaults.vim into our vimrc and just set skip_defaults_vim=1 to prevent sourcing defaults.vim altogether.

3) We could add a skip_freebsd_vimrc escape to the top of our vimrc:
if exists('skip_freebsd_vimrc')
  finish
endif

4) We could add a note to our vimrc pointing users to $VIMRUNTIME/defaults.vim and noting that they should check there for stuff they want to override.

5) We could get out of the vimrc business altogether.
Comment 14 Tim Chase 2022-08-09 23:36:51 UTC
(In reply to Anton Saietskii from comment #12)

Having hit this today after my upgrade to 13.1 yesterday, I advocate for either

1) dropping the system-wide /usr/local/etc/vim/vimrc file.  Or,

2a) at a minimum, omitting the "source defaults.vim" from the system-wide vimrc

2b) check if a $HOME/.vimrc or $HOME/.vim/vimrc file exists and only execute the "source defaults.vim" line of the system vimrc in the event neither exists.  There might be a few other file-names to test as documented at `:help VIMINIT`, but that help-target documents that default.vim shouldn't run if a user vimrc exists

The default.vim sets some unpleasant new defaults (particularly 'scrolloff' and 'mouse' have broken workflows for me) but as long as I have my mostly-empty ~/.vimrc it hasn't been a problem on various systems.

But now random changes to the defaults.vim mean my .vimrc needs to keep tracking new defaults and manually revert them to non- defaults.vim settings.
Comment 15 Adam Weinberger freebsd_committer 2022-08-10 14:41:45 UTC
(In reply to Tim Chase from comment #14)

The last time upstream's defaults.vim had a substantive change was 2019. The last actual change before that was 2017. I agree that vim's load ordering is a mess but in some respects the best approach is for you to define the defaults you actually want. If you want a particular scrolloff and mouse behaviour, define it in your vimrc so the upstream defaults become irrelevant.

I'm having a hard time balancing the needs of people who want vim to Just Work vs the needs of people who want to control how vim works. The problem with your solution is that it completely prevents upstream's defaults for the people who rely on it. I just don't know how to solve this for everyone... any comprehensive solutions there would be greatly appreciated.

The defaults are designed to be applied before the user vimrc is loaded. As a result, you can't even set something like a g:no_defaults in ~/.vimrc to control it.

See https://github.com/vim/vim/blob/master/src/main.c#L3160 for the load ordering.

What do you think about either of these?

1) What about adding in an environment variable (maybe $VIM_NO_DEFAULTS) to control it? It would preserve the defaults for the default user, and give everyone else a knob to disable them.

2) You could `alias vim='vim -u ~/.vimrc` to prevent /usr/local/etc/vim/vimrc and $VIMRUNTIME/defaults.vim from ever getting loaded.
Comment 16 Tim Chase 2022-08-10 15:30:10 UTC
Created attachment 235824 [details]
Only source defaults.vim if none of the documented startup files exist

(In reply to Adam Weinberger from comment #15)

I'm not keen on the environment-variable work-around.  It does reduce the "track every default you want that defaults.vim might change and set it yourself preemptively" to a single setting, but it's still extra work to fix something that normal-vim (rather than FreeBSDified vim) doesn't require.

I think the right solution is to act like vim does:  don't source the defaults.vim if any of those startup files exist.  Possibly something like this patch?  I'm not 100% certain I've nailed down all the odd edge-cases, but this should cover the vast majority of them, and be fairly easy to modify in the event I missed any.
Comment 17 Tim Chase 2022-08-10 15:31:54 UTC
Comment on attachment 235824 [details]
Only source defaults.vim if none of the documented startup files exist

For speed-up, it might even pay to put a `break` statement after that first `let skip_defaults_vim = 1` statement, since we don't need to continue checking other files if we've already found one of them.
Comment 18 Adam Weinberger freebsd_committer 2022-08-10 17:23:43 UTC
(In reply to Tim Chase from comment #16)
> I think the right solution is to act like vim does:
> don't source the defaults.vim if any of those startup files exist.

I think I'm confused about how the load ordering works.

I'm looking through ':h initialization'... can you confirm that I have the ordering correct here?

1) /usr/local/etc/vim/vimrc is always read
2) vim looks for user vimrc in the places listed in your patch
3) defaults.vim is only loaded if none of (2) is found

Is that correct? If so then your patch is correct, but the load order is absurd. defaults.vim should happen before (1) so that it doesn't override the system vimrc.

If you can confirm that the above ordering is correct then I'll apply your patch.
Comment 19 Tim Chase 2022-08-10 18:29:58 UTC
(In reply to Adam Weinberger from comment #18)

Per Bram over on vim_use@

https://groups.google.com/g/vim_use/c/n2rRNURa8Qw/m/lqVzMihaBAAJ

> Vim does not include a system-wide vimrc file that would go in
/usr/local/etc/vim. Does this file source defaults.vim? Then that's a
problem of FreeBSD.


In your 3-point catalog of understanding where you have "1) /usr/local/etc/vim/vimrc is always read", this was introduced by the FreeBSD port build-process.  The /usr/ports/editors/vim/Makefile installs files/vimrc as a system-wide vimrc atop the default vim distribution leading to this bugzilla ticket.  :-)

But then your #2 & #3 understandings are correct.

However, thinking more about this, I suspect a better solutions is as follows:

1) the editors/vim port Makefile stops modifying the upstream feature.h to include system-wide vimrc #DEFINEs

2) the Makefile continues to *install* a systemwide vimrc (see below), possibly as /usr/share/etc/vim/defaults.vim or freebsd_defaults.vim

3) the Makefile instead patches the distributed defaults.vim to source the systemwide vimrc; whether this happens at the top or bottom of defaults.vim would depend on whose defaults you'd prefer

I'd also be tempted to strip down the installed system-wide vimrc because there are some surprising settings in here like enabling 'autoread' and turning on 'autoindent' by default (rather than relegating that to a filetype plugin or a user's vimrc preferences), as well as some settings that either conflict with (history=100 vs 200) the defaults.vim or are already set there.  But the `augroup FreeBSD` portion and settting `g:is_posix=1` are both valuable additions.

By doing this, vim maintains its expected/documented behavior of only loading defaults.vim if there's no vimrc (or if explicitly sourced from a user's vimrc), while also applying a few FreeBSD-specific tweaks to the default settings.  It also doesn't have to fstat() each of the possible vimrc files twice (once in the stock vim process, and again as done in the attached patch).
Comment 20 Adam Weinberger freebsd_committer 2022-08-10 19:36:48 UTC
(In reply to Tim Chase from comment #19)

Thank you, that is really helpful!

Historically, our vimrc was provided as a way to provide "better defaults," but I'm increasing dissatisfied with that. People who want their own settings should carry around a vimrc... it's not an unreasonable ask.

The downside to patching defaults.vim is that the FreeBSD-specific settings will cease to exist when users have their own vimrc (correct me if I'm wrong on that).

Considering all the options, we could provide ONLY FreeBSD-specific stuff (as you mentioned, the augroup and g:is_posix) and continue to patch it in as a hardcoded load so it can exist even when defaults.vim is skipped. That would solve the skipping-the-defaults.vim problem, but I don't feel great about it.

We could just as easily provide a note in pkg-message and pkg-descr about looking through freebsd_defaults.vim if you use your own vimrc, which feels less abusive than hardcoding stuff (which is what got us into this mess in the first place).

vim-tiny does not include defaults.vim, so I think we will still need to provide a vimrc for it.
Comment 21 Tim Chase 2022-08-10 20:31:16 UTC
(In reply to Adam Weinberger from comment #20)

> Historically, our vimrc was provided as a way to provide "better defaults," but
> I'm increasing dissatisfied with that. People who want their own settings
> should carry around a vimrc... it's not an unreasonable ask.  

The main concern in both this case and in vim's implementation of
defaults.vim handling is that changing long-standing defaults
shouldn't impact people who already have a vimrc configured the way
they want.  So if you don't have a vimrc, then you get the defaults
(whether vim-stock-default or vim-stock-plus-FreeBSD-mojo).  And
if you want original-stock-vim-without-modified-defaults, it's just
a `touch ~/.vimrc` away.

> The downside to patching defaults.vim is that the FreeBSD-specific settings
> will cease to exist when users have their own vimrc (correct me if I'm wrong on
> that).  

You're correct, but I don't see that as a bad thing.  If folks want
FreeBSD-specific augmentations, then sourcing them directly is not
an unreasonable ask (to use your phrasing ;-)

> Considering all the options, we could provide ONLY FreeBSD-specific stuff (as
> you mentioned, the augroup and g:is_posix) and continue to patch it in as a
> hardcoded load so it can exist even when defaults.vim is skipped. That would
> solve the skipping-the-defaults.vim problem, but I don't feel great about it.  

I don't feel great about it from the other direction -- If I have
my vimrc and haven't asked for FreeBSD additions, it's my own dumb
fault if I don't get them.  And that's a good thing.  If I want
FreeBSD goodies in my vimrc, I can source $VIMRUNTIME/defaults.vim
or /usr/local/etc/vim/$FREEBSD_CUSTOM_VIMRC explicitly.

> We could just as easily provide a note in pkg-message and pkg-descr about
> looking through freebsd_defaults.vim if you use your own vimrc, which feels
> less abusive than hardcoding stuff (which is what got us into this mess in the
> first place).  

I don't object to this path of "if you want FreeBSD extras, you
have to source them explicitly" messaging.

> vim-tiny does not include defaults.vim, so I think we will still need to
> provide a vimrc for it.  

I'd be curious if vim-tiny doesn't include defaults.vim in the
build/deploy process, but will source one if provided (in which
case the FreeBSD patching could create a defaults.vim that just
sources the FreeBSD customization file); or if vim-tiny doesn't
even do any defaults.vim logic, in which case I'd leave vim-tiny
to be its own beast.  It omits a lot of functionality, some of which
I think was being used in the current system-wide vimrc (IIRC, it's
not compiled with +eval meaning things like if/endif and try/except
blocks don't work in vim-tiny)

All that to say that that vim-tiny is somewhat its own beast and I
would expect it to behave very differently.
Comment 22 commit-hook freebsd_committer 2022-09-03 18:04:47 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=a058c61ea36c0cb64a61c83563b55a157cf549f4

commit a058c61ea36c0cb64a61c83563b55a157cf549f4
Author:     Adam Weinberger <adamw@FreeBSD.org>
AuthorDate: 2022-09-03 17:56:00 +0000
Commit:     Adam Weinberger <adamw@FreeBSD.org>
CommitDate: 2022-09-03 18:03:52 +0000

    editors/vim: Update to 9.0.0369, and drop the FreeBSD vimrc

    Our FreeBSD vimrc has caused a lot of trouble. It seriously mangles
    config loading order (see below PR for the 21 comments it took before
    I could get my head around it), clobbers defaults, duplicates defaults,
    and simply isn't how Vim does things.

    In this patch, the system vimrc is dropped entirely. Instead,
    $VIMRUNTIME/defaults.vim sources $VIMRUNTIME/defaults_freebsd.vim, which
    contains only FreeBSD-specific settings (today, a convenience augroup
    for port creation, and a flag to let syntax/sh.vim know that /bin/sh
    isn't bash).

    There is no perfect solution here, but by not clobbering anything
    anymore, we at least don't *prevent* other solutions. You now get Vim's
    defaults, and you are free to override them in your ~/.vimrc.

    PR:             251420

 editors/vim/Makefile                         | 21 +++++--------
 editors/vim/distinfo                         |  6 ++--
 editors/vim/files/defaults_freebsd.vim (new) | 10 +++++++
 editors/vim/files/vimrc (gone)               | 45 ----------------------------
 editors/vim/pkg-message (new)                | 11 +++++++
 editors/vim/pkg-plist                        |  1 -
 editors/vim/pkg-plist-tiny                   |  1 -
 7 files changed, 32 insertions(+), 63 deletions(-)
Comment 23 Adam Weinberger freebsd_committer 2022-09-03 18:15:57 UTC
OK hopefully Vim is much more POLA-compatible now. I went with exactly your solution, Tim.

Thanks for staying on the merry-go-round so long, everyone. Just untangling what the port was doing took me an embarrassingly long time.

Tim, can you confirm that the port is doing the right thing now?
Comment 24 Tim Chase 2022-09-03 19:03:44 UTC
(In reply to Adam Weinberger from comment #23)

For new installs, this looks good to me. Thanks for remedying this.

My only concern would be if an existing install that has the old issues (leading to this case) exists, does it get brought in line with these changes on an upgrade?  I don't know the ins and outs of ports/packages to know if this cleanup happens by default, or if any special care is needed to undo the changes that led to this issue when the package is upgraded.

Thanks again!
Comment 25 Adam Weinberger freebsd_committer 2022-09-04 16:34:46 UTC
(In reply to Tim Chase from comment #24)

I added back the system vimrc stuff to feature.h, so people can still use that file if they like. Users will have to deal with loading order stuff on their own, but that's how it should be.

I think this is the best of both worlds. Our FreeBSD-specific stuff is in its own file, and it's sourced by default. If users want their own system vimrc, they can have it, and the decision to source defaults.vim is now up to the user. Hopefully we are as POLA violation-free as possible now.

Thank you immensely for your help on this, Tim. I was having too much trouble untangling this on my own, and the simpler, much more correct solution came because of the clarity you provided.