Refactor plugin installation logic to support custom plugin URLs and improve error handling during installation

3 files changed, 160 insertions, 215 deletions

diff --git a/tests/plugins/repository/test.lua b/tests/plugins/repository/test.lua
index e0c67058b..c4c6c4c20 100644
--- a/tests/plugins/repository/test.lua
+++ b/tests/plugins/repository/test.lua
@@ -1,3 +1,5 @@
+local global = import("core.base.global")
+
 function _write_plugin(dir, name, text)
     io.writefile(path.join(dir, "xmake.lua"), string.format([[
 task("%s")
@@ -8,8 +10,7 @@ task("%s")
     io.writefile(path.join(dir, "main.lua"), string.format([[function main() print("%s") end]], text))
 end
 
-function main()
-    local global = import("core.base.global")
+function main(t)
     local suffix = path.filename(os.tmpfile()):gsub("[^%w]", "")
     local reponame = "plugin-test-repository-" .. suffix
     local hello_name = "plugin-test-hello-" .. suffix
@@ -20,85 +21,66 @@ function main()
     local localdir = path.join(os.tmpfile() .. ".local-plugin", local_name)
     local plugindir = path.join(global.directory(), "plugins")
     local cachefile = path.join(global.cachedir(), "repository")
-    local cachebackup = os.tmpfile() .. ".repository"
-
-    if os.isfile(cachefile) then
-        os.cp(cachefile, cachebackup)
-    end
 
+    -- reset test state
     local function cleanup()
         for _, name in ipairs({hello_name, formatter_name, available_name, local_name}) do
             os.tryrm(path.join(plugindir, name))
         end
-        if os.isfile(cachebackup) then
-            os.cp(cachebackup, cachefile)
-        else
-            os.tryrm(cachefile)
-        end
-        os.tryrm(cachebackup)
+        local cache = os.isfile(cachefile) and io.load(cachefile) or {}
+        cache.repositories = cache.repositories or {}
+        cache.repositories[reponame] = nil
+        io.save(cachefile, cache)
         os.tryrm(repodir)
         os.tryrm(path.directory(localdir))
     end
+    cleanup()
+
+    -- mock repository with installed and available plugins
+    _write_plugin(path.join(repodir, "plugins", hello_name), hello_name, "repo-ok")
+    _write_plugin(path.join(repodir, "plugins", formatter_name), formatter_name, "format-ok")
+    _write_plugin(path.join(repodir, "plugins", available_name), available_name, "available-ok")
+    os.mkdir(path.directory(cachefile))
+    local cache = os.isfile(cachefile) and io.load(cachefile) or {}
+    cache.repositories = cache.repositories or {}
+    cache.repositories[reponame] = {repodir}
+    io.save(cachefile, cache)
 
+    -- Feature: install by plain name from repository
+    os.runv("xmake", {"plugin", "--install", hello_name})
+    os.runv("xmake", {hello_name})
 
-    try
-    {
-        function ()
-            -- mock repository with installed and available plugins
-            _write_plugin(path.join(repodir, "plugins", hello_name), hello_name, "repo-ok")
-            _write_plugin(path.join(repodir, "plugins", formatter_name), formatter_name, "format-ok")
-            _write_plugin(path.join(repodir, "plugins", available_name), available_name, "available-ok")
-            os.mkdir(path.directory(cachefile))
-            local cache = os.isfile(cachefile) and io.load(cachefile) or {}
-            cache.repositories = cache.repositories or {}
-            cache.repositories[reponame] = {repodir}
-            io.save(cachefile, cache)
+    -- Feature: install by repo@name format
+    os.runv("xmake", {"plugin", "--install", reponame .. "@" .. formatter_name})
+    os.runv("xmake", {formatter_name})
 
-            -- Feature: install by plain name and repo@name in one invocation
-            os.exec("xmake plugin --install " .. hello_name .. " " .. reponame .. "@" .. formatter_name)
-            os.exec("xmake " .. hello_name)
-            os.exec("xmake " .. formatter_name)
+    -- Feature: --list shows installed and available repository plugins
+    local out = os.iorun("xmake plugin --list")
+    t:require(out:find(hello_name, 1, true))
+    t:require(out:find(formatter_name, 1, true))
+    t:require(out:find(available_name, 1, true))
+    t:require(out:find("xmake plugin --install " .. available_name, 1, true))
 
-            -- Feature: --list shows installed and available repository plugins
-            local out = os.iorun("xmake plugin --list")
-            assert(out:find(hello_name, 1, true))
-            assert(out:find(formatter_name, 1, true))
-            assert(out:find(available_name, 1, true))
-            assert(out:find("xmake plugin --install " .. available_name, 1, true))
+    -- Feature: install from local directory
+    _write_plugin(localdir, local_name, "local-ok")
+    os.runv("xmake", {"plugin", "--install", localdir})
+    os.runv("xmake", {local_name})
 
-            -- Feature: install from local directory
-            _write_plugin(localdir, local_name, "local-ok")
-            os.exec("xmake plugin --install " .. os.args(localdir))
-            os.exec("xmake " .. local_name)
+    out = os.iorun("xmake plugin --list")
+    t:require(out:find(local_name, 1, true))
 
-            out = os.iorun("xmake plugin --list")
-            assert(out:find(local_name, 1, true))
+    -- Feature: remove plugin
+    os.runv("xmake", {"plugin", "--remove", local_name})
+    out = os.iorun("xmake plugin --list")
+    t:require_not(out:find(local_name, 1, true))
 
-            -- Feature: remove plugin
-            os.exec("xmake plugin --remove " .. local_name)
-            out = os.iorun("xmake plugin --list")
-            assert(not out:find(local_name, 1, true))
+    -- Feature: install non-existent plugin fails gracefully
+    local ok = try { function () os.runv("xmake", {"plugin", "--install", "plugin-test-missing-" .. suffix}) return true end }
+    t:require_not(ok)
 
-            -- Feature: install non-existent plugin fails gracefully
-            local ok = try { function () os.exec("xmake plugin --install plugin-test-missing-" .. suffix) end }
-            assert(not ok)
-            -- Feature: reject plugin name traversal
-            ok = try { function () os.exec("xmake plugin --install " .. reponame .. "@..") end }
-            assert(not ok)
+    -- Feature: reject plugin name traversal
+    ok = try { function () os.runv("xmake", {"plugin", "--install", reponame .. "@.."}) return true end }
+    t:require_not(ok)
 
-        end,
-        catch
-        {
-            function (errors)
-                cleanup()
-                raise(errors)
-            end
-        },
-        finally
-        {
-            function ()
-                cleanup()
-            end
-        }
-    }
+    cleanup()
 end
diff --git a/xmake/plugins/plugin/main.lua b/xmake/plugins/plugin/main.lua
index db03c7abb..09de02983 100644
--- a/xmake/plugins/plugin/main.lua
+++ b/xmake/plugins/plugin/main.lua
@@ -45,36 +45,22 @@ end
 
 -- get plugin urls for batch install
 function _plugin_urls()
-    local urls = {
-    "https://github.com/xmake-io/xmake-plugins.git",
-    "https://gitlab.com/tboox/xmake-plugins.git",
-    "https://gitee.com/tboox/xmake-plugins.git"}
-    fasturl.add(urls)
-    return fasturl.sort(urls)
-end
-
--- run a function with a temporary directory that is removed on success or failure
-function _with_tmpdir(fn)
-    local tmpdir = os.tmpfile() .. ".dir"
-    return try
-    {
-        function ()
-            return fn(tmpdir)
-        end,
-        catch
-        {
-            function (errors)
-                os.tryrm(tmpdir)
-                raise(errors)
-            end
-        },
-        finally
-        {
-            function ()
-                os.tryrm(tmpdir)
-            end
-        }
-    }
+    local urls = option.get("plugins")
+    if urls then
+        local result = {}
+        for _, url in ipairs(table.wrap(urls)) do
+            table.insert(result, git.asgiturl(url) or url)
+        end
+        urls = result
+    else
+        urls = {
+        "https://github.com/xmake-io/xmake-plugins.git",
+        "https://gitlab.com/tboox/xmake-plugins.git",
+        "https://gitee.com/tboox/xmake-plugins.git"}
+        fasturl.add(urls)
+        urls = fasturl.sort(urls)
+    end
+    return urls
 end
 
 function _manifest_path()
@@ -143,67 +129,34 @@ function _install_from_git(url)
             branch = url:sub(i + 1)
             url = url:sub(1, i - 1)
         end
-        url = "https://github.com/" .. url .. ".git"
+        url = git.asgiturl("github:" .. url)
     end
-    _with_tmpdir(function (tmpdir)
-        local clone_opt = {verbose = option.get("verbose"), outputdir = tmpdir}
-        if branch then
-            clone_opt.branch = branch
-        end
-        git.clone(git.asgiturl(url) or url, clone_opt)
-        local found = false
-        local function install(srcdir, name)
-            local dstdir = _get_plugindir(name)
-            assert(not os.isdir(dstdir), "plugin(%s) already exists!", name)
-            os.vcp(srcdir, dstdir)
-            cprint("  ${color.success}-> ${bright}%s${clear}", name)
-            found = true
-        end
-        if os.isfile(path.join(tmpdir, "xmake.lua")) then
-            install(tmpdir, path.basename(path.filename(url)))
-        else
-            for _, filepath in ipairs(os.files(path.join(tmpdir, "*", "xmake.lua"))) do
-                local srcdir = path.directory(filepath)
-                install(srcdir, path.filename(srcdir))
-            end
-        end
-        if not found then
-            raise("no plugin found in %s", url)
-        end
-    end)
-end
-
--- install a single plugin
-function _install_one(name)
-    -- parse repo@plugin format
-    local i = name:find("@", 1, true)
-    if i and not name:find("[/\\:]") then
-        local reponame = name:sub(1, i - 1)
-        local pluginname = name:sub(i + 1)
-        _install_plugins_from_repo(pluginname, reponame)
-        return
+    local tmpdir = os.tmpfile() .. ".dir"
+    local clone_opt = {verbose = option.get("verbose"), outputdir = tmpdir}
+    if branch then
+        clone_opt.branch = branch
     end
-
-    -- github shortcut: github:user/repo or github:user/repo#branch
-    if name:startswith("github:") then
-        _install_from_git(name)
-        return
+    git.clone(git.asgiturl(url) or url, clone_opt)
+    local found = false
+    local function install(srcdir, name)
+        local dstdir = _get_plugindir(name)
+        assert(not os.isdir(dstdir), "plugin(%s) already exists!", name)
+        os.vcp(srcdir, dstdir)
+        cprint("  ${color.success}-> ${bright}%s${clear}", name)
+        found = true
     end
-
-    -- git url or local path
-    if name:startswith("file://") or git.asgiturl(name) then
-        _install_from_git(name)
-        return
-    elseif os.isdir(name) then
-        _install_from_local(name)
-        return
-    elseif name:find("[/\\:]") then
-        _install_from_git(name)
-        return
+    if os.isfile(path.join(tmpdir, "xmake.lua")) then
+        install(tmpdir, path.basename(path.filename(url)))
+    else
+        for _, filepath in ipairs(os.files(path.join(tmpdir, "*", "xmake.lua"))) do
+            local srcdir = path.directory(filepath)
+            install(srcdir, path.filename(srcdir))
+        end
+    end
+    os.tryrm(tmpdir)
+    if not found then
+        raise("no plugin found in %s", url)
     end
-
-    -- plain name: try to find it in repositories
-    _install_plugins_from_repo(name)
 end
 
 -- install plugins
@@ -212,69 +165,84 @@ function _install()
     -- enter environment
     environment.enter()
 
-    local errors
     try
     {
         function ()
 
-            -- install requested plugins
-            local names = option.get("plugins")
-            if names then
-                for _, name in ipairs(names) do
-                    _install_one(name)
+            -- do install
+            local name = option.get("plugins")
+            if name and #name > 0 then
+                -- parse repo@plugin format
+                local i = name:find("@", 1, true)
+                if i and not name:find("[/\\:]") then
+                    local reponame = name:sub(1, i - 1)
+                    local pluginname = name:sub(i + 1)
+                    _install_plugins_from_repo(pluginname, reponame)
+                    return
                 end
+
+                -- github shortcut: github:user/repo or github:user/repo#branch
+                if name:startswith("github:") then
+                    _install_from_git(name)
+                    return
+                end
+
+                -- git url or local path
+                if name:startswith("file://") or git.asgiturl(name) then
+                    _install_from_git(name)
+                    return
+                elseif os.isdir(name) then
+                    _install_from_local(name)
+                    return
+                elseif name:find("[/\\:]") then
+                    _install_from_git(name)
+                    return
+                end
+
+                -- plain name: try to find it in repositories
+                _install_plugins_from_repo(name)
                 return
             end
 
             -- do batch install from plugin collection urls
             local urls = _plugin_urls()
+            local tmpdir = os.tmpfile() .. ".dir"
             local plugindir = _get_plugindir()
             local installed_url
-            _with_tmpdir(function (tmpdir)
-                for _, url in ipairs(urls) do
-                    cprint("installing plugins from ${bright}%s${clear} ..", url)
-                    local ok = try
-                    {
-                        function ()
-                            git.clone(url, {verbose = option.get("verbose"), outputdir = tmpdir})
-                            return true
-                        end
-                    }
-                    if ok then
-                        installed_url = url
-                        break
-                    end
-                    os.tryrm(tmpdir)
-                end
-                assert(installed_url, "failed to install plugins from all urls!")
-                for _, filepath in ipairs(os.files(path.join(tmpdir, "*", "xmake.lua"))) do
-                    local srcdir = path.directory(filepath)
-                    local name = path.filename(srcdir)
-                    local dstdir = _get_plugindir(name)
-                    assert(not os.isdir(dstdir), "plugin(%s) already exists!", name)
-                    os.vcp(srcdir, dstdir)
-                    cprint("  ${color.success}-> ${bright}%s${clear}", name)
-                end
-            end)
-            local manifest = _load_manifest() or {}
-            manifest.urls = manifest.urls or {}
-            table.join2(manifest.urls, installed_url)
-            _save_manifest(manifest)
+            for _, url in ipairs(urls) do
+                cprint("installing plugins from ${bright}%s${clear} ..", url)
+                git.clone(url, {verbose = option.get("verbose"), outputdir = tmpdir})
+                installed_url = url
+                break
+            end
+            for _, filepath in ipairs(os.files(path.join(tmpdir, "*", "xmake.lua"))) do
+                local srcdir = path.directory(filepath)
+                local name = path.filename(srcdir)
+                local dstdir = _get_plugindir(name)
+                assert(not os.isdir(dstdir), "plugin(%s) already exists!", name)
+                os.vcp(srcdir, dstdir)
+                cprint("  ${color.success}-> ${bright}%s${clear}", name)
+            end
+            os.tryrm(tmpdir)
+
+            if installed_url then
+                local manifest = _load_manifest() or {}
+                manifest.urls = manifest.urls or {}
+                table.join2(manifest.urls, installed_url)
+                _save_manifest(manifest)
+            end
             cprint("${color.success}all plugins have been installed in ${bright}%s${clear}!", plugindir)
         end,
         catch
         {
-            function (_errors)
-                errors = _errors
+            function (errors)
+                raise(errors)
             end
         }
     }
 
     -- leave environment
     environment.leave()
-    if errors then
-        raise(errors)
-    end
 end
 
 -- update plugins
@@ -283,7 +251,6 @@ function _update()
     -- enter environment
     environment.enter()
 
-    local errors
     try
     {
         function ()
@@ -295,40 +262,36 @@ function _update()
             local plugindir = _get_plugindir()
             for _, url in ipairs(urls) do
                 cprint("updating plugins from ${bright}%s${clear} ..", url)
-                _with_tmpdir(function (tmpdir)
-                    git.clone(url, {verbose = option.get("verbose"), outputdir = tmpdir})
-                    for _, filepath in ipairs(os.files(path.join(tmpdir, "*", "xmake.lua"))) do
-                        local srcdir = path.directory(filepath)
-                        local name = path.filename(srcdir)
-                        local dstdir = _get_plugindir(name)
-                        os.tryrm(dstdir)
-                        os.vcp(srcdir, dstdir)
-                        cprint("  ${color.success}-> ${bright}%s${clear}", name)
-                    end
-                end)
+                local tmpdir = os.tmpfile() .. ".dir"
+                git.clone(url, {verbose = option.get("verbose"), outputdir = tmpdir})
+                for _, filepath in ipairs(os.files(path.join(tmpdir, "*", "xmake.lua"))) do
+                    local srcdir = path.directory(filepath)
+                    local name = path.filename(srcdir)
+                    local dstdir = _get_plugindir(name)
+                    os.tryrm(dstdir)
+                    os.vcp(srcdir, dstdir)
+                    cprint("  ${color.success}-> ${bright}%s${clear}", name)
+                end
+                os.tryrm(tmpdir)
             end
             cprint("${color.success}all plugins have been updated in ${bright}%s${clear}!", plugindir)
         end,
         catch
         {
-            function (_errors)
-                errors = _errors
+            function (errors)
+                raise(errors)
             end
         }
     }
 
     -- leave environment
     environment.leave()
-    if errors then
-        raise(errors)
-    end
 end
 
 -- remove the given installed plugin
 function _remove()
-    local names = assert(option.get("plugins"), "please specify the plugin name to be removed!")
-    assert(#names == 1, "please specify only one plugin name to be removed!")
-    local name = _check_plugin_name(names[1])
+    local name = assert(option.get("plugins"), "please specify the plugin name to be removed!")
+    assert(name ~= "" and name ~= "." and not name:find("..", 1, true) and not name:find("[/\\:]"), "invalid plugin name(%s)!", name)
     local dir = _get_plugindir(name)
     assert(os.isdir(dir), "plugin(%s) not found!", name)
     os.rmdir(dir)
diff --git a/xmake/plugins/plugin/xmake.lua b/xmake/plugins/plugin/xmake.lua
index aca6def8d..a1b7f61ba 100644
--- a/xmake/plugins/plugin/xmake.lua
+++ b/xmake/plugins/plugin/xmake.lua
@@ -30,7 +30,7 @@ task("plugin")
             {'r', "remove",  "k", nil, "Remove the given installed plugin."},
             {'l', "list",    "k", nil, "List all installed plugins."},
             {'c', "clear",   "k", nil, "Clear all installed plugins."},
-            {nil, "plugins", "vs", nil, "The plugin paths, urls or names.",
+            {nil, "plugins", "v", nil, "The plugins path, url or package name.",
                                        "e.g.",
                                        "    $ xmake plugin --install https://github.com/xmake-io/xmake-plugins",
                                        "    $ xmake plugin --install github:xmake-io/xmake-plugins",