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

treat files/directories of unpacked sources equally in PackedBinary #2306

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

ocaisa
Copy link
Member

@ocaisa ocaisa commented Jan 12, 2021

Fixes #1334

@ocaisa
Copy link
Member Author

ocaisa commented Jan 12, 2021

Test report by @ocaisa

Overview of tested easyconfigs (in order)

  • SUCCESS PLINK-2.00-alpha1-x86_64.eb

Build succeeded for 1 out of 1 (1 easyconfigs in total)
generoso - Linux centos linux 8.2.2004, x86_64, Intel(R) Xeon(R) CPU E5-2667 v3 @ 3.20GHz, Python 3.6.8
See https://gist.github.com/8c7fe6bb0fd0b3171460a6ee49e65141 for a full test report.

@ocaisa
Copy link
Member Author

ocaisa commented Jan 12, 2021

Test report by @ocaisa

Overview of tested easyconfigs (in order)

  • SUCCESS Maven-3.6.3.eb

Build succeeded for 1 out of 1 (1 easyconfigs in total)
generoso - Linux centos linux 8.2.2004, x86_64, Intel(R) Xeon(R) CPU E5-2667 v3 @ 3.20GHz, Python 3.6.8
See https://gist.github.com/914b7c1446435f909350b191499af50a for a full test report.

@ocaisa
Copy link
Member Author

ocaisa commented Jan 12, 2021

Test report by @ocaisa

Overview of tested easyconfigs (in order)

  • SUCCESS Maven-3.6.3.eb
  • SUCCESS PLINK-2.00-alpha1-x86_64.eb
  • SUCCESS Tika-1.16.eb

Build succeeded for 3 out of 3 (3 easyconfigs in total)
generoso - Linux centos linux 8.2.2004, x86_64, Intel(R) Xeon(R) CPU E5-2667 v3 @ 3.20GHz, Python 3.6.8
See https://gist.github.com/86843b11ae92366e93f8eb9646de1101 for a full test report.

@ocaisa
Copy link
Member Author

ocaisa commented Jan 12, 2021

Test report by @ocaisa

Overview of tested easyconfigs (in order)

  • SUCCESS Maven-3.6.3.eb
  • SUCCESS PLINK-2.00-alpha1-x86_64.eb
  • SUCCESS Tika-1.16.eb

Build succeeded for 3 out of 3 (3 easyconfigs in total)
generoso - Linux centos linux 8.2.2004, x86_64, Intel(R) Xeon(R) CPU E5-2667 v3 @ 3.20GHz, Python 3.6.8
See https://gist.github.com/1191fe0191f9545d2d05d9923faf4245 for a full test report.

@ocaisa ocaisa added the bug fix label Jan 13, 2021
akesandgren
akesandgren previously approved these changes Mar 29, 2021
Copy link
Contributor

@akesandgren akesandgren left a comment

Choose a reason for hiding this comment

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

LGTM

@akesandgren akesandgren added this to the next release (4.3.4?) milestone Mar 29, 2021
@boegel boegel changed the title Treat files/directories of unpacked sources equally in packedbinary.py treat files/directories of unpacked sources equally in PackedBinary Mar 31, 2021
easybuild/easyblocks/generic/packedbinary.py Outdated Show resolved Hide resolved
easybuild/easyblocks/generic/packedbinary.py Outdated Show resolved Hide resolved
easybuild/easyblocks/generic/packedbinary.py Outdated Show resolved Hide resolved
@ocaisa
Copy link
Member Author

ocaisa commented Apr 7, 2021

Test report by @ocaisa

Overview of tested easyconfigs (in order)

  • SUCCESS Maven-3.6.3.eb
  • SUCCESS PLINK-2.00-alpha1-x86_64.eb
  • SUCCESS Tika-1.16.eb

Build succeeded for 3 out of 3 (3 easyconfigs in total)
generoso - Linux centos linux 8.2.2004, x86_64, Intel(R) Xeon(R) CPU E5-2667 v3 @ 3.20GHz, Python 3.6.8
See https://gist.github.com/105da1493c4331f320f2b31dd9288700 for a full test report.

Comment on lines +52 to +62
# we only handle the case of a single file and no install_cmd here
if os.path.isfile(srcpath) and self.cfg.get('install_cmd', None) is None:
copy(srcpath, self.installdir)
else:
if os.path.isdir(srcpath):
# copy files to install dir via Binary
self.cfg['start_dir'] = src
Binary.install_step(self)
elif os.path.isfile(srcpath):
shutil.copy2(srcpath, self.installdir)
self.cfg['start_dir'] = self.builddir
else:
raise EasyBuildError("Path %s is not a file nor a directory?", srcpath)
except OSError as err:
raise EasyBuildError("Failed to copy unpacked sources to install directory: %s", err)
Binary.install_step(self)
Copy link
Contributor

Choose a reason for hiding this comment

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

@ocaisa this is a pre-existing thing, but the install_step in the Binary easyblock removes the installation directory before copying, shouldn't that be done here too? why not just let the Binary install_step handle this case too, since it already has the logic to handle if install_cmd is None?

Suggested change
# we only handle the case of a single file and no install_cmd here
if os.path.isfile(srcpath) and self.cfg.get('install_cmd', None) is None:
copy(srcpath, self.installdir)
else:
if os.path.isdir(srcpath):
# copy files to install dir via Binary
self.cfg['start_dir'] = src
Binary.install_step(self)
elif os.path.isfile(srcpath):
shutil.copy2(srcpath, self.installdir)
self.cfg['start_dir'] = self.builddir
else:
raise EasyBuildError("Path %s is not a file nor a directory?", srcpath)
except OSError as err:
raise EasyBuildError("Failed to copy unpacked sources to install directory: %s", err)
Binary.install_step(self)
if os.path.isdir(srcpath):
self.cfg['start_dir'] = src
elif os.path.isfile(srcpath):
self.cfg['start_dir'] = self.builddir
else:
raise EasyBuildError("Path %s is not a file nor a directory?", srcpath)
Binary.install_step(self)

@boegel boegel modified the milestones: 4.4.0, release after 4.4.0 May 27, 2021
@boegel boegel modified the milestones: 4.4.2, release after 4.4.2 Sep 1, 2021
@boegel boegel removed this from the 4.5.1 milestone Dec 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PackedBinary doesn't execute install_cmd when only files (no folders) are extracted from tar
4 participants