tzachar / compe-tabnine

TabNine source for hrsh7th/nvim-compe
BSD 3-Clause "New" or "Revised" License
52 stars 3 forks source link

fix binary path which create by TabNine binary #33

Closed YuseiUeno closed 3 years ago

YuseiUeno commented 3 years ago

My Environment

Feature

fix binary path which create by TabNine binary

Fixed problem

Sometime execute binaries/TabNine_Darwin make tabnine binary upside directory, like this.

['/Users/user/.cache/dein/repos/github.com/tzachar/0.0.1', '/Users/user/.cache/dein/repos/github.com/tzachar/3.5.35']

I don't wanna pollute directory.

Apparently TabNine binary make directory it-self at tow higher level. Example for 0.0.1 and 3.5.35 and so on. So, I want change Tabnine_xx alis state.

tzachar commented 3 years ago

Im not sure I understand the problem. As the install script works right now, the binaries are created in the binaries folder, so I dont see the case for using the latest subfolder.

Can you further explain the issue?

YuseiUeno commented 3 years ago

Yes, install script works with nothing problems. But sometime makes directory for tabnine binary when executing compe-tabine extension. I executed rm command, but directory is still there. It remove with finish compe-tabine extension. It happen to me few times. I don't know why happen. But, I guess that it make by binaries/TabNine_Darwin. Do TabNine make directory for binary like 0.0.1?

I'm sorry to let you spend your time.

YuseiUeno commented 3 years ago

It's maybe this problem.

https://github.com/codota/TabNine/blob/master/HowToWriteAClient.md#setting-up-tabnine-within-an-editor-plugin

You must preserve the directory structure created by dl_binaries.sh, or else TabNine's automatic updating will not work. When TabNine updates, it downloads the new version in the same location as the current binary but with a different version directory. For example, if the current binary is at bin/1.0.5/x86_64-apple-darwin/TabNine, and TabNine downloads version 1.0.7, it will be installed at bin/1.0.7/x86_64-apple-darwin/TabNine.

YuseiUeno commented 3 years ago

I correct it. It occur when TabNine binary is old version.

$ mkdir tmp && cd tmp
$ git clone --depth 1 https://github.com/tzachar/compe-tabnine.git

## get old version tabnine binary

$ git clone --depth 1 https://github.com/codota/tabnine-vim.git
$ mkdir -p compe-tabnine/binaries
$ mv tabnine-vim/binaries/3.3.33 compe-tabnine/binaries/
$ rm -rf tabnine-vim
$ tree -L 3
.
└── compe-tabnine
    ├── LICENSE
    ├── README.md
    ├── after
    │   └── plugin
    ├── binaries
    │   └── 3.3.33
    ├── install.sh
    └── lua
        └── compe_tabnine

7 directories, 3 files

$ cd compe-tabnine/binaries/
$ ln -s 3.3.33/x86_64-apple-darwin/TabNine TabNine_Darwin
$ cd ../../ 
$ tree -L 3
.
└── compe-tabnine
    ├── LICENSE
    ├── README.md
    ├── after
    │   └── plugin
    ├── binaries
    │   ├── 3.3.33
    │   └── TabNine_Darwin -> 3.3.33/x86_64-apple-darwin/TabNine
    ├── install.sh
    └── lua
        └── compe_tabnine

7 directories, 4 files
$ ./compe-tabnine/binaries/TabNine_Darwin
$ tree -L 3
.
├── 0.0.1
│   └── binaries
├── 3.5.37
│   └── binaries
│       ├── TabNine
│       ├── TabNine-deep-cloud
│       ├── TabNine-deep-local
│       ├── WD-TabNine
│       └── bundle.lock
└── compe-tabnine
    ├── LICENSE
    ├── README.md
    ├── after
    │   └── plugin
    ├── binaries
    │   ├── 3.3.33
    │   └── TabNine_Darwin -> 3.3.33/x86_64-apple-darwin/TabNine
    ├── install.sh
    └── lua
        └── compe_tabnine

11 directories, 9 files
YuseiUeno commented 3 years ago

I think it don't use alias is better. Like below. https://github.com/tbodt/deoplete-tabnine/blob/181dc9e615e39fa95a722ec21b5604ef3b40c6f3/rplugin/python3/deoplete/sources/tabnine.py#L226-L240

Can I challenge it?

tzachar commented 3 years ago

I think the best solution would be, as you say, to use latest version explicitly. Can you try this patch?


diff --git a/install.sh b/install.sh
index 11e483d..163866d 100755
--- a/install.sh
+++ b/install.sh
@@ -29,7 +29,3 @@ curl https://update.tabnine.com/bundles/${path}/TabNine.zip --create-dirs -o bin
 unzip -o binaries/${path}/TabNine.zip -d binaries/${path}
 rm -rf binaries/${path}/TabNine.zip
 chmod +x binaries/$path/*
-
-target="binaries/TabNine_$(uname -s)"
-rm $target || true # remove old link
-ln -sf $path/TabNine $target
diff --git a/lua/compe_tabnine/init.lua b/lua/compe_tabnine/init.lua
index b3f161d..e5589c0 100644
--- a/lua/compe_tabnine/init.lua
+++ b/lua/compe_tabnine/init.lua
@@ -33,13 +33,30 @@ end

 -- locate the binary here, as expand is relative to the calling script name
-local binary = nil
-if fn.has("mac") == 1 then
-   binary = fn.expand("<sfile>:p:h:h:h") .. "/binaries/TabNine_Darwin"
-elseif fn.has('unix') == 1 then
-   binary = fn.expand("<sfile>:p:h:h:h") .. "/binaries/TabNine_Linux"
-else
-   binary = fn.expand("<sfile>:p:h:h:h") .. "/binaries/TabNine_Windows"
+local function binary()
+   local binaries_folder = fn.expand("<sfile>:p:h:h:h") .. "/binaries"
+   local versions_folders = fn.globpath(binaries_folder, '*', false, true)
+   local versions = {}
+   for _, path in ipairs(versions_folders) do
+       for i in string.gmatch(path, '/[^/]*$') do
+           local version, _ = string.gsub(i, '/', '')
+           table.insert(versions, {path=path, version=version})
+       end
+   end
+   table.sort(versions, function (a, b) return a.version < b.version end)
+   local latest = versions[#versions]
+
+   local platform = nil
+   if fn.has('win32') == 1 then
+       platform = 'i686-pc-windows-gnu'
+   elseif fn.has('win64') == 1 then
+       platform = 'x86_64-pc-windows-gnu'
+   elseif fn.has("mac") == 1 then
+       platform = 'x86_64-apple-darwin'
+   elseif fn.has('unix') == 1 then
+       platform = 'x86_64-unknown-linux-musl'
+   end
+   return latest.path .. '/' .. platform .. '/' .. 'TabNine'
 end

 local function is_enabled()
@@ -185,7 +202,7 @@ Source._on_exit = function(_, code)
        return
    end

-   Source.job = fn.jobstart({binary}, {
+   Source.job = fn.jobstart({binary()}, {
        on_stderr = Source._on_stderr;
        on_exit = Source._on_exit;
        on_stdout = Source._on_stdout;
YuseiUeno commented 3 years ago

I got a error, when binaries directory includes only old version tabnine.

...hub.com/tzachar/compe-tabnine/lua/compe_tabnine/init.lua:59: attempt to index local 'latest' (a nil value)

binaries_folder to be a /binary, so we have to fix it. ~It will be fix replace<sfile> to %.~ Sorry, it not fixed. And why don't you support 'arm64'? dd2e1e5

binaries_folder maybe fix as below. Do you know another better way?

diff --git a/lua/compe_tabnine/init.lua b/lua/compe_tabnine/init.lua
index 98b47bc..240eb6f 100644
--- a/lua/compe_tabnine/init.lua
+++ b/lua/compe_tabnine/init.lua
@@ -1,6 +1,7 @@
 local compe = require'compe'
 local api = vim.api
 local fn = vim.fn
+local binaries_folder = fn.expand("<sfile>:p:h:h:h") .. "/binaries"
 local compe_config = require'compe.config'
 --

@@ -34,7 +35,6 @@ end

 -- locate the binary here, as expand is relative to the calling script name
 local function binary()
-   local binaries_folder = fn.expand("%:p:h:h:h") .. "/binaries"
    local versions_folders = fn.globpath(binaries_folder, '*', false, true)
    local versions = {}
    for _, path in ipairs(versions_folders) do
tzachar commented 3 years ago

The fix is to cache the binary folder on startup, as expand does not work correctly when we reload TabNine. something like this:


-- do this once on init, otherwise on restart this dows not work                                
local binaries_folder = fn.expand("<sfile>:p:h:h:h") .. "/binaries"                             

-- locate the binary here, as expand is relative to the calling script name                     
local function binary()                                                                         
        local versions_folders = fn.globpath(binaries_folder, '*', false, true)                 
        local versions = {}                                                                     
        for _, path in ipairs(versions_folders) do                                              
                for i in string.gmatch(path, '/[^/]*$') do                                      
                        local version, _ = string.gsub(i, '/', '')                              
                        table.insert(versions, {path=path, version=version})                    
                end                                                                             
        end                                                                                     
        table.sort(versions, function (a, b) return a.version < b.version end)                  
        local latest = versions[#versions]                                                      

        local platform = nil                                                                    
        if fn.has('win32') == 1 then                                                            
                platform = 'i686-pc-windows-gnu'                                                
        elseif fn.has('win64') == 1 then                                                        
                platform = 'x86_64-pc-windows-gnu'                                              
        elseif fn.has("mac") == 1 then                                                          
                platform = 'x86_64-apple-darwin'                                                
        elseif fn.has('unix') == 1 then                                                         
                platform = 'x86_64-unknown-linux-musl'                                          
        end                                                                                     
        return latest.path .. '/' .. platform .. '/' .. 'TabNine'                               
end                                                                                             

Also, feel free to add any missing archs.

p.s. we also need to add an rm -rf binaries/TabNine_* to the install script. Otherwise, the logic above does not work. A different approach is to check that the first chararcter in version is a number.

YuseiUeno commented 3 years ago

Thank you for your support! 😄

tzachar commented 3 years ago

Thank you for you effort :)