Skip to content

bug fix: copy permissions when staging#10285

Merged
scheibelp merged 3 commits intospack:developfrom
odoublewen:fix_stage_permissions
Jan 9, 2019
Merged

bug fix: copy permissions when staging#10285
scheibelp merged 3 commits intospack:developfrom
odoublewen:fix_stage_permissions

Conversation

@odoublewen
Copy link
Copy Markdown
Contributor

@odoublewen odoublewen commented Jan 9, 2019

This is proposed as a fix for #10284 which I recently reported.

PR #10152 replaced shutil.move with llnl's copy and copy_tree
https://github.com/spack/spack/pull/10152/files#diff-b96a1a55f58dffde150cbe13f13ae2acL548

I think this is causing the build error with lua that I'm seeing today. See below for details. I don't know if this is an acceptable way to fix the issue, but I offer this PR as both a bug report and suggestion of possible fix.

$ spack install lua                                                                                                           

[snip]
                                  
==> Building lua [Package]                                                                                                                                                      
==> Executing phase: 'install'                                                                                                                                                  
==> Error: ProcessError: ./configure: Permission denied                                                                                                                         
    Command: './configure' '--prefix=/bifx/apps/spack/spack-20190108/opt/spack/linux-centos7-x86_64/gcc-8.2.0/lua-5.3.4-diuptpgekoebrs4bg6eprp6c4sizzaxu' '--with-lua=/bifx/apps
/spack/spack-20190108/opt/spack/linux-centos7-x86_64/gcc-8.2.0/lua-5.3.4-diuptpgekoebrs4bg6eprp6c4sizzaxu'

The configure in luarocks is executable when the tarball is unpacked, but when the tree is copied into lua, it is no longer executable:

$ ls -l /bifx/apps/spack/spack-20190108/var/spack/stage/resource-luarocks-diuptpgekoebrs4bg6eprp6c4sizzaxu/luarocks-2.3.0/configure
-rwxr-xr-x 1 osolberg domain users 13021 Dec 29  2015 configure

$ ls -l /bifx/apps/spack/spack-20190108/var/spack/stage/lua-5.3.4-diuptpgekoebrs4bg6eprp6c4sizzaxu/lua-5.3.4/luarocks/luarocks/configure
-rw-r--r-- 1 osolberg domain users 13021 Jan  8 13:50 configure

@hartzell FYI

adamjstewart
adamjstewart reviewed Jan 9, 2019
View reviewed changes
@adamjstewart
Copy link
Copy Markdown
Member

adamjstewart commented Jan 9, 2019

Pinging @michaelkuhn, @healther, and @scheibelp who were involved in the conversation in #10152.

@scheibelp scheibelp merged commit c48b0a1 into spack:develop Jan 9, 2019
@scheibelp
Copy link
Copy Markdown
Member

scheibelp commented Jan 9, 2019

Thanks!

@odoublewen odoublewen deleted the fix_stage_permissions branch January 9, 2019 05:57
@healther
Copy link
Copy Markdown
Contributor

healther commented Jan 9, 2019

A sorry, I only triggered the discussion copy vs copy_tree and completely overlooked the permissions thing... Maybe we should have a clearer defined scope of when to use copy vs install, because for staging it kind of feels wrong to "install" something doesn't it? I know it's just semantics but ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adamjstewart adamjstewart adamjstewart left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@odoublewen @adamjstewart @scheibelp @healther