Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed debian release #63

Closed
wants to merge 1 commit into from

Conversation

drmargarido
Copy link

Changed chmod call to an execute call because fs.chmod was removed from luarocks.fs in version 3.

@coveralls
Copy link

coveralls commented Feb 2, 2019

Coverage Status

Coverage remained the same at 25.122% when pulling 2de38ef on drmargarido:fix-debian-release into 9bb4192 on MisterDA:master.

@MisterDA
Copy link
Owner

MisterDA commented Feb 2, 2019

Thanks for the bug fix!

A quick grep over the LuaRocks codebase shows that they’ve switched to
LuaPosix functions, I’d rather follow their lead.
http://luaposix.github.io/luaposix/modules/posix.html#chmod

spec/fs_spec.lua
36:         fs.execute("chmod -r " .. fs.Q(path))
44:         fs.execute("chmod -w " .. fs.Q(path))
52:         fs.execute("chmod -x " .. fs.Q(path))

spec/util/test_env.lua
640:            execute_bool("chmod og-wx ~/.ssh/authorized_keys")

src/luarocks/fs/lua.lua
399:      return posix.chmod(dest, fullattrs)
933:   local err = posix.chmod(filename, perms)

I’d escape the whole path, just in case.

Do you want to fix this, or should I do it? If you do, you can rebase
force-push to this branch.

Thanks!

@drmargarido
Copy link
Author

I can fix it, thanks for the feedback.

…uaposix chmod.

Changed the debian script to also use the luaposix chmod.
@@ -109,7 +110,8 @@ function s.script(project)
"love '"..loveFileDeb:gsub("'", "\\'").."'\n",
true
)
assert(fs.chmod(tempDir.."/usr/bin/"..project.package, "+x"))

assert(posix.chmod(tempDir.."/usr/bin/"..project.package, 'u+x'))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it really u+x ? If the package is installed as root, then only root will be able to execute the game, right ?
I think that a better perm would be rwxr-xr-x (0755).

@MisterDA
Copy link
Owner

MisterDA commented Feb 8, 2019

… and I can’t compile LuaPosix.
luaposix/luaposix#320

@drmargarido
Copy link
Author

My installation of luaposix also failed initially with luarocks so I used their luke file to install it -> https://github.com/luaposix/luaposix/tree/master/build-aux

@MisterDA
Copy link
Owner

MisterDA commented Feb 8, 2019

I think I’m gonna go with

diff --git a/src/scripts/debian.lua b/src/scripts/debian.lua
index 24ddf8e..e3b1578 100644
--- a/src/scripts/debian.lua
+++ b/src/scripts/debian.lua
@@ -109,7 +109,8 @@ function s.script(project)
       "love '"..loveFileDeb:gsub("'", "\\'").."'\n",
       true
   )
-  assert(fs.chmod(tempDir.."/usr/bin/"..project.package, "+x"))
+  assert(fs.set_permissions(fs.Q(tempDir.."/usr/bin/"..project.package)),
+                            "exec", "all") -- 755
 
   -- /usr/share/games/${PACKAGE}/${LOVE_FILE}
   copyFile(project.releaseDirectory.."/"..script.loveFile, loveFileDeb, true)

@drmargarido
Copy link
Author

Yes, that's better

@MisterDA
Copy link
Owner

MisterDA commented Feb 8, 2019

I get weird results with chmod and fs.Q. If there is a sample file test, all it does is putting quotes around the parameter.

cfg = require("luarocks.core.cfg")
cfg.init()
fs = require("luarocks.fs")
fs.init()
fs.set_permissions("test", "exec", "all") -- works
fs.set_permissions("'test'", "exec", "all") -- does not work

@MisterDA MisterDA closed this in 49bad4a Feb 9, 2019
MisterDA added a commit that referenced this pull request Apr 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants