luvit / lit

Toolkit for developing, sharing, and running luvit/lua programs and libraries.
http://lit.luvit.io/
Apache License 2.0
245 stars 58 forks source link

add a guard for malformed package during lit install #193

Closed james2doyle closed 8 years ago

james2doyle commented 8 years ago

I was trying to setup a project of git today, and lit install was failing but it wasn't clear why. I checked the package.lua and it had a missing comma, causing an error.

Basically, I just added a conditional in the core.installDeps to make sure that the package loaded correctly, and if not, it would spit out a warning letting you know the package file must be malformed.

Before

lit version: 3.4.3
luvi version: v2.7.4
command: install
load config: /Users/james/.litconfig
fail: [string "bundle:libs/core.lua"]:532: attempt to index local 'meta' (a nil value)
stack traceback:
    [string "bundle:libs/core.lua"]:532: in function 'installDeps'
    [string "bundle:commands/install.lua"]:6: in function <[string "bundle:commands/install.lua"]:1>
    [string "bundle:main.lua"]:52: in function <[string "bundle:main.lua"]:39>
    [C]: in function 'xpcall'
    [string "bundle:main.lua"]:39: in function <[string "bundle:main.lua"]:31>

After

lit version: 3.4.3
luvi version: v2.7.4
command: install
load config: /Users/james/.litconfig
fail: [string "bundle:libs/core.lua"]:528: error in package file: /Users/james/Sites/_git/lit/tests/bad-package
stack traceback:
    [C]: in function 'error'
    [string "bundle:libs/core.lua"]:528: in function 'installDeps'
    [string "bundle:commands/install.lua"]:6: in function <[string "bundle:commands/install.lua"]:1>
    [string "bundle:main.lua"]:52: in function <[string "bundle:main.lua"]:39>
    [C]: in function 'xpcall'
    [string "bundle:main.lua"]:39: in function <[string "bundle:main.lua"]:31>
james2doyle commented 8 years ago

Just saw #191, and this may not be necessary

james2doyle commented 8 years ago

It is possible that this could also be fixed with an assert in the pkg.query function. The stolen one from #191 is something like assert(loadfile(path))

squeek502 commented 8 years ago

I'm not really sure how #191 is related, could you clarify what you meant by this?

Just saw #191, and this may not be necessary

james2doyle commented 8 years ago

Oh my bad. I thought it also affected the package loading. Nevermind then. They are unrelated. But I did see that assert line that can validate the file to check for syntax errors

On Thu, Sep 29, 2016, 5:26 PM Ryan Liptak, notifications@github.com wrote:

I'm not really sure how #191 https://github.com/luvit/lit/pull/191 is related, could you clarify what you meant by this?

Just saw #191 https://github.com/luvit/lit/pull/191, and this may not be necessary

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/luvit/lit/pull/193#issuecomment-250629001, or mute the thread https://github.com/notifications/unsubscribe-auth/ABW_mAKQGQQ2hZDQw0mRcSgcrEH9UHWjks5qvFdGgaJpZM4KKmKw .

squeek502 commented 8 years ago

It might make sense to capture the error message from evalModule in pkg.query (which returns both syntax errors and more specific errors like "Missing name in package description in " .. name) and return that from pkg.query here instead of "No meta found". Then you could capture the second return from pkg.query and print it when pkg.query returns nil:

local meta, err = pkg.query(gfs, path)
-- meta will be `nil` if package is malformed
if not meta then
  error(err)
  return
end
james2doyle commented 8 years ago

Yeah that could work. I will take another crack at this. I will submit a new PR with the changes and close this one after