avoid PermissionError on Lustre when copying files (fixes #25354) #27247

BenWibking · 2021-11-05T23:37:17Z

There is a bug in Python versions < 3.7.4 that causes PermissionError to occur when using shutil.copy2 to copy read-only files that have extended attributes set by Lustre (which is used by lib/spack/llnl/util/filesystem.py:504 to copy directory trees). The upstream Python bug is here.

Setting SPACK_PYTHON to point to a python executable >= version 3.7.4 fixes the problem. However, for Python versions below this, we monkeypatch shutil.copystat to set extended attributes before setting permissions. This means that copying read-only files that have extended attributes set by Lustre can succeed, which is necessary to install openjdk (and fix #25354). I have confirmed on my cluster that this patch fixes openjdk installation when spack is installed on a Lustre FS when using Python 3.6.

BenWibking · 2021-11-06T03:27:34Z

It looks like the CI fails due to this function accessing a member variable that only exists in Python 3.3+, but I've guarded against accessing it for earlier verions:

          # use the real function only if it exists
           # *and* it supports follow_symlinks
           def lookup(name):
               fn = getattr(os, name, _nop)
               if sys.version_info >= (3, 3):
                   if fn in os.supports_follow_symlinks:
                       return fn
               return _nop

mwkrentel · 2021-11-09T23:27:05Z

I tried this on theta at ANL. /lus/theta-fs0 is lustre.

$ mount | grep  lustre
172.22.10.92@o2ib:172.22.10.93@o2ib:/snx11214 on /lus/theta-fs0 type lustre (rw,flock,lazystatfs)

On /lus/theta-fs0, current spack develop fails for me as:

$ spack install openjdk 
==> Installing openjdk-11.0.12_7-cw7hhkrfe4x2fk73jlxw22pefvf5n4j5
==> No binary for openjdk-11.0.12_7-cw7hhkrfe4x2fk73jlxw22pefvf5n4j5 found: installing from source
==> Using cached archive: /lus/theta-fs0/projects/CSC250STTO11/krentel/spack/spack-repo/source-cache/_source-cache/archive/87/8770f600fc3b89bf331213c7aa21f8eedd9ca5d96036d1cd48cb2748a3dbefd2.tar.gz
==> No patches needed for openjdk
==> openjdk: Executing phase: 'install'
==> Error: PermissionError: [Errno 13] Permission denied: '/lus/theta-fs0/projects/CSC250STTO11/krentel/spack/install/cray-cnl7-x86_64/gcc-11.2.0/openjdk-11.0.12_7-cw7hhkrfe4x2fk73jlxw22pefvf5n4j5/legal/jdk.localedata/thaidict.md'

/lus/theta-fs0/projects/CSC250STTO11/krentel/spack/spack-repo/var/spack/repos/builtin/packages/openjdk/package.py:144, in install:
        142    def install(self, spec, prefix):
        143        top_dir = 'Contents/Home/' if platform.system() == "Darwin" else '.'
  >>    144        install_tree(top_dir, prefix)

With the patch to filesystem.py it works.

$ spack install openjdk
==> Installing openjdk-11.0.12_7-cw7hhkrfe4x2fk73jlxw22pefvf5n4j5
==> No binary for openjdk-11.0.12_7-cw7hhkrfe4x2fk73jlxw22pefvf5n4j5 found: installing from source
==> Using cached archive: /lus/theta-fs0/projects/CSC250STTO11/krentel/spack/spack-repo/source-cache/_source-cache/archive/87/8770f600fc3b89bf331213c7aa21f8eedd9ca5d96036d1cd48cb2748a3dbefd2.tar.gz
==> No patches needed for openjdk
==> openjdk: Executing phase: 'install'
==> openjdk: Successfully installed openjdk-11.0.12_7-cw7hhkrfe4x2fk73jlxw22pefvf5n4j5
  Fetch: 0.49s.  Build: 6.69s.  Total: 7.18s.
[+] /lus/theta-fs0/projects/CSC250STTO11/krentel/spack/install/cray-cnl7-x86_64/gcc-11.2.0/openjdk-11.0.12_7-cw7hhkrfe4x2fk73jlxw22pefvf5n4j5

So, I can confirm the bug on lustre and the patch works for me.

Btw, it's not just the install_tree and module_roots directories
where it fails, it also fails when build_stage is on lustre.

This is with current spack develop as of today, Nov 9, 2021.

Spack: 0.17.0-48-0322431c5c
Python: 3.6.12
Platform: cray-sles15-haswell
Concretizer: clingo

mwkrentel · 2021-11-17T03:34:15Z

I tested this a week ago, and I was able to reproduce the problem and
confirm the solution works. So, I approve.

@tldahlgren You understand I don't have commit access (and don't really want it), right?

tldahlgren · 2021-11-17T16:12:49Z

I tested this a week ago, and I was able to reproduce the problem and confirm the solution works. So, I approve.

@tldahlgren You understand I don't have commit access (and don't really want it), right?

Yes, I do understand.

I was just flagging PRs with the people who have provided reviews so it's visible on the PR list. IMO it's an easier way to distinguish PRs with real feedback (e.g., something other than spackbot comments).

Perhaps this is a good topic to clarify in the Spack User meeting?

BenWibking · 2021-11-25T09:28:38Z

@tgamblin @becker33 Is there any timeframe on when this can get reviewed? This patch fixes an issue I reported over 3 months ago, and this PR itself is 20 days old...

haampie · 2021-11-25T09:45:24Z

Not sure why this was not reviewed, but one issue is we would add PSF-licensed
code, if I understand correctly.

2. Subject to the terms and conditions of this License Agreement, PSF hereby
   grants Licensee a nonexclusive, royalty-free, world-wide license to reproduce,
   analyze, test, perform and/or display publicly, prepare derivative works,
   distribute, and otherwise use Python 3.10.0 alone or in any derivative
   version, provided, however, that PSF's License Agreement and PSF's notice of
   copyright, i.e., "Copyright © 2001-2021 Python Software Foundation; All Rights
   Reserved" are retained in Python 3.10.0 alone or in any derivative version
   prepared by Licensee.

3. In the event Licensee prepares a derivative work that is based on or
   incorporates Python 3.10.0 or any part thereof, and wants to make the
   derivative work available to others as provided herein, then Licensee hereby
   agrees to include in any such work a brief summary of the changes made to Python
   3.10.0.

Can you rebase the PR?

BenWibking · 2021-11-25T11:19:55Z

Rebased.

I didn't realize the license issue. How does this work for vendored dependencies? Would adding shutil (from 3.7.4 or higher) as a vendored dependency fix this problem?

lib/spack/llnl/util/filesystem.py

BenWibking · 2021-11-25T22:39:53Z

The error message for the unittests (2.7, clingo) check is inscrutable to me. Is this a CI problem, or an actual problem with this patch?

BenWibking · 2021-12-02T21:33:12Z

@haampie @tgamblin @becker33 Just trying to get traction on getting this merged. Is there anything blocking merging this?

BenWibking · 2022-01-04T10:43:17Z

@haampie @tgamblin @becker33 Checking in on this...any possibility of getting this merged?

haampie · 2022-01-04T11:03:11Z

There's still some issue with Python 2.7, see the failing test.

TypeError: stat() takes no keyword arguments

wyphan · 2023-01-17T15:05:15Z

@spackbot fix style

spackbot-app · 2023-01-17T15:05:19Z

Let me see if I can fix that for you!

spackbot-app · 2023-01-17T15:06:32Z

I was able to run spack style --fix for you!

spack style --fix

==> Running style checks on spack
  selected: isort, black, flake8, mypy
==> Modified files
  lib/spack/llnl/util/filesystem.py
==> Running isort checks
  isort checks were clean
==> Running black checks
reformatted lib/spack/llnl/util/filesystem.py
All done! ✨ 🍰 ✨
1 file reformatted.
  black checks were clean
==> Running flake8 checks
  flake8 checks were clean
==> Running mypy checks
Success: no issues found in 555 source files
  mypy checks were clean
==> spack style checks were clean

Keep in mind that I cannot fix your flake8 or mypy errors, so if you have any you'll need to fix them and update the pull request. If I was able to push to your branch, if you make further changes you will need to pull from your updated branch before pushing again.

I've updated the branch with style fixes.

BenWibking · 2023-02-17T14:53:24Z

There's still some issue with Python 2.7, see the failing test.
TypeError: stat() takes no keyword arguments

Since Python 2.7 support has been dropped, is there anything currently blocking this?

wyphan · 2023-02-17T16:33:16Z

I tried rebasing to latest develop, and it succeeded. But it turns out I don't have write access to your branch 🥲

…

On Fri, Feb 17, 2023, 09:53 Ben Wibking ***@***.***> wrote: There's still some issue with Python 2.7, see the failing test. TypeError: stat() takes no keyword arguments Since Python 2.7 support has been dropped, is there anything currently blocking this? — Reply to this email directly, view it on GitHub <#27247 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AMERY5F3NBKQQPMFPE3P4NDWX6GG7ANCNFSM5HO4P76A> . You are receiving this because you were assigned.Message ID: ***@***.***>

BenWibking · 2023-02-17T16:59:40Z

I've checked the "Allow edits and access to secrets by maintainers" box. Is there anything else I need to do?

…opying dir trees

spackbot-app bot added the utilities label Nov 5, 2021

tldahlgren assigned mwkrentel Nov 16, 2021

tldahlgren requested review from tgamblin and becker33 November 16, 2021 20:59

haampie reviewed Nov 25, 2021

View reviewed changes

lib/spack/llnl/util/filesystem.py Outdated Show resolved Hide resolved

spackbot-app bot added the core PR affects Spack core functionality label Aug 5, 2022

mwkrentel mentioned this pull request Jan 26, 2023

Installation issue: openjdk #35171

Open

4 tasks

scheibelp assigned haampie and wyphan Feb 8, 2023

BenWibking and others added 4 commits February 17, 2023 11:59

monkeypatch shutil.copystat to avoid PermissionError on Lustre when c…

995bc98

…opying dir trees

fix copystat monkeypatch on python 2

44cef5b

fix whitespace in filesystem.py

ab57b88

Disable vermin on version guarded block

61a89cf

[@spackbot] updating style on behalf of BenWibking

e029cda

BenWibking force-pushed the develop branch from f81d473 to e029cda Compare February 17, 2023 16:59

haampie approved these changes Feb 17, 2023

View reviewed changes

haampie merged commit e8238fe into spack:develop Feb 20, 2023

koysean pushed a commit to koysean/spack that referenced this pull request Feb 20, 2023

Patchel shutil.copystat to avoid PermissionError on Lustre (spack#27247)

535a93a

jmcarcell pushed a commit to key4hep/spack that referenced this pull request Apr 13, 2023

Patchel shutil.copystat to avoid PermissionError on Lustre (spack#27247)

6190f1f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

avoid PermissionError on Lustre when copying files (fixes #25354) #27247

avoid PermissionError on Lustre when copying files (fixes #25354) #27247

BenWibking commented Nov 5, 2021 •

edited

BenWibking commented Nov 6, 2021

mwkrentel commented Nov 9, 2021

mwkrentel commented Nov 17, 2021

tldahlgren commented Nov 17, 2021

BenWibking commented Nov 25, 2021

haampie commented Nov 25, 2021

BenWibking commented Nov 25, 2021

BenWibking commented Nov 25, 2021

BenWibking commented Dec 2, 2021

BenWibking commented Jan 4, 2022

haampie commented Jan 4, 2022

wyphan commented Jan 17, 2023

spackbot-app bot commented Jan 17, 2023

spackbot-app bot commented Jan 17, 2023

BenWibking commented Feb 17, 2023

wyphan commented Feb 17, 2023 via email

BenWibking commented Feb 17, 2023

avoid PermissionError on Lustre when copying files (fixes #25354) #27247

avoid PermissionError on Lustre when copying files (fixes #25354) #27247

Conversation

BenWibking commented Nov 5, 2021 • edited

BenWibking commented Nov 6, 2021

mwkrentel commented Nov 9, 2021

mwkrentel commented Nov 17, 2021

tldahlgren commented Nov 17, 2021

BenWibking commented Nov 25, 2021

haampie commented Nov 25, 2021

BenWibking commented Nov 25, 2021

BenWibking commented Nov 25, 2021

BenWibking commented Dec 2, 2021

BenWibking commented Jan 4, 2022

haampie commented Jan 4, 2022

wyphan commented Jan 17, 2023

spackbot-app bot commented Jan 17, 2023

spackbot-app bot commented Jan 17, 2023

BenWibking commented Feb 17, 2023

wyphan commented Feb 17, 2023 via email

BenWibking commented Feb 17, 2023

BenWibking commented Nov 5, 2021 •

edited