vim-maktaba icon indicating copy to clipboard operation
vim-maktaba copied to clipboard

Symlinked plugin with addon-info.json "name" may be added to the runtimepath twice

Open glts opened this issue 11 years ago • 9 comments

This happens on an old MacVim 7.3, but not on newer versions of Vim.

Create a plugin with an addon-info.json file with an explicit "name", for example ~/code/vim-linked/addon-info.json:

{
    "name": "linked.vim",
    "version": "0.0.0",
    "dependencies": {
        "maktaba": {}
    }
}

Create a symlink to the plugin location, then start Vim with a vimrc that #GetOrInstalls the symlink location. (Here I used locations with different leaf dir names, but the same thing happens when the leaf dir names are the same.)

set nocompatible
source ~/.vim/maktaba/maktaba/bootstrap.vim
call maktaba#plugin#GetOrInstall('~/.vim/maktaba/linked')

Result. &rtp:

~/.vim,
~/code/vim-linked/,
~/.vim/maktaba/linked/,
/Applications/MacVim.app/Contents/Resources/vim/vimfiles,
/Applications/MacVim.app/Contents/Resources/vim/runtime,
/Applications/MacVim.app/Contents/Resources/vim/vimfiles/after,
~/.vim/after,
~/.vim/maktaba/maktaba

:echo maktaba#plugin#RegisteredPlugins():

['linked.vim', 'maktaba']

glts avatar Feb 14 '14 14:02 glts

This should be fixed by #60 (almost ready to submit). I'm changing all the maktaba internal stuff to not touch rtp, so it will only be updated once, by your explicit call to maktaba#plugin#GetOrInstall.

That's a bit of a workaround. The detection would theoretically still fail to realize that '~/code/vim-linked/' and '~/.vim/maktaba/linked/' are the same path. Might still be worth investigating this bug independently and figuring out why maktaba doesn't understand these are the same path.

dbarnett avatar Feb 14 '14 15:02 dbarnett

Ah, cool.

FYI, I've tried the 'register' branch, and while the runtimepath problem goes away, my MacVim now starts with uncaught exceptions. Setup is the same as above.

Error detected while processing function maktaba#plugin#Enter..maktaba#plugin#Flag:
line    5:
E605: Exception not caught: ERROR(NotFound): Flag "plugin" not defined in plugin "linked.vim"
Error detected while processing function maktaba#plugin#Enter:
line   22:
E171: Missing :endif

Though this is probably a user error. I'll check later.

glts avatar Feb 14 '14 16:02 glts

Hmm… I can't reproduce with vim 7.3.429. Followed your directions exactly. What version is your MacVim?

You can debug if you do :func /RegisterPlugin and then take that function name (e.g., <SNR>16_RegisterPlugin) and add breakadd func <SNR>NN_RegisterPlugin to your vimrc. I don't see any branch in the code where it would source instant files without first having defined the flags, but maybe it's somehow checking the wrong path for the plugin/ directory at the top of s:InitPlugin not finding it, or l:already_installed is messing up somehow?

dbarnett avatar Feb 15 '14 08:02 dbarnett

This is on a plain Vim 7.3 (no patches), which for a long time had been the latest supported MacVim 7.3 on Mac OS X 10.6.

I had some trouble getting debugging to work, but here is the plugin object as returned by maktaba#plugin#Get after starting up. The setup is exactly as given above.

{
  'AddonInfo': function('maktaba#plugin#AddonInfo'),
  'Flag': function('maktaba#plugin#Flag'),
  'GenerateHelpTags': function('maktaba#plugin#GenerateHelpTags'),
  'HasDir': function('maktaba#plugin#HasDir'),
  'HasFiletypeData': function('maktaba#plugin#HasFiletypeData'),
  'HasFlag': function('maktaba#plugin#HasFlag'),
  'IsLibrary': function('maktaba#plugin#IsLibrary'),
  'Load': function('maktaba#plugin#Load'),
  'MapPrefix': function('maktaba#plugin#MapPrefix'),
  'Source': function('maktaba#plugin#Source'),
  '_addon_info': {
    'author': 'author',
    'dependencies': {'maktaba': {}},
    'name': 'linked.vim',
    'version': '0.0.0',
  },
  '_entered': {
    'autoload': [],
    'ftplugin': {},
    'instant': [],
    'plugin': [],
  },
  'flags': {},
  'globals': {},
  'location': '/Users/glts/code/vim-linked/',
  'logger': { ... },
  'name': 'linked.vim',
}

Flags are indeed not defined, and the location is the dereferenced symlink. On a current Vim the location is /Users/glts/.vim/maktaba/linked/.

glts avatar Feb 15 '14 10:02 glts

Phhhh, this is almost certainly to do with the resolve problem prior to patch 7.3.194, discussed in #18 and #31.

https://github.com/google/maktaba/blob/7b25c31c1769576802ff49baf1ca88392c0d4a1a/autoload/maktaba/plugin.vim#L507-L525

glts avatar Feb 15 '14 12:02 glts

Ah, okay, so this a bug independent of #60 and the register branch. We need working path resolving for all supported vim versions. Let's try to bisect what version of vim is affected and find a workaround (or find an implementation that works in both old and new vim).

dbarnett avatar Feb 16 '14 19:02 dbarnett

I wouldn't call this independent of #60 and the 'register' branch. The bug is in the new code, in fact on line 522 of the part I highlighted.

Changing

" If plugin is symlinked, register resolved path as custom location to avoid
" conflicts.
let l:resolved_location = s:Fullpath(resolve(a:plugin.location))

to

" If plugin is symlinked, register resolved path as custom location to avoid
" conflicts. As above, fnamemodify() is required to strip the trailing slash.
let l:resolved_location = s:Fullpath(resolve(fnamemodify(a:plugin.location, ':p:h')))

fixes the problem. The problem description is exactly the same as that given in the comment 15 lines above. I'm too lazy to open a pull req, can you fix it?

I don't know your reasoning behind requiring the trailing slashes in paths, but it does make life a little harder. Cheers.

glts avatar Feb 16 '14 22:02 glts

Oh, I misunderstood then.

Should be fixed on the resolve branch now if you want to double-check I fixed it properly.

I don't know your reasoning behind requiring the trailing slashes in paths, but it does make life a little harder.

The reason we prefer explicit trailing slashes is that it makes the path unambiguously refer to a directory, which helps in path manipulation code like maktaba#path#Dirname to not get confused. It reduces the need for going to the filesystem to determine whether paths are supposed to refer to a file or directory, and also makes things behave better when referring to dirs that don't exist (yet).

dbarnett avatar Feb 16 '14 23:02 dbarnett

A-ok.

glts avatar Feb 17 '14 08:02 glts