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

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

Open
casparvl opened this issue Jan 4, 2018 · 5 comments · May be fixed by #2306
Open

Comments

@casparvl
Copy link
Contributor

casparvl commented Jan 4, 2018

PackedBinary contains some logic separating two cases: whether only files are extracted from a tar, or whether folders are extracted.

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)

When folders are extracted (i.e. the 'if' is true), Binary.install_step is called, which executes install_cmd. However, when only files are extracted (the 'elif' scenario), it only copies the files, and the install_cmd is not executed.

For this scenario, PackedBinary should either execute the install_cmd, or return an error (or at least a warning in the log) that the install_cmd is being ignored.

@cmfield
Copy link

cmfield commented Jan 12, 2021

I have just come across this error and I can't believe it's still open after 3 years.. really frustrating!

@ocaisa
Copy link
Member

ocaisa commented Jan 12, 2021

@boegel Think I need a bit of advice here. If I understand the relevant easyblocks correctly, I should be able to switch over the use of https://github.com/easybuilders/easybuild-easyblocks/blob/develop/easybuild/easyblocks/generic/binary.py#L110-L111 to copy() in https://github.com/easybuilders/easybuild-framework/blob/develop/easybuild/tools/filetools.py#L2202 and in that case there is no longer a need for the if/elif?
This makes me nervous because both of these easyblocks are heavily inherited from.

@ocaisa
Copy link
Member

ocaisa commented Jan 12, 2021

Hmm, I think I will do the easy thing because that is not quite right, I need to provide the specific path to the source.

if install_cmd is None:
    try:
        copy(srcpath , self.installdir)
    except OSError as err:
        raise EasyBuildError("Failed to copy %s to %s: %s", self.cfg['start_dir'], self.installdir, err)
else:
    cmd = ' '.join([self.cfg['preinstallopts'], install_cmd, self.cfg['installopts']])
    self.log.info("Installing %s using command '%s'..." % (self.name, cmd))
    run_cmd(cmd, log_all=True, simple=True)

I'll open a PR and we can discuss the merits

@ocaisa
Copy link
Member

ocaisa commented Jan 12, 2021

@cmfield Can you try --include-easyblocks-from-pr=2306 in your command line and see if that solves your problem?

@akesandgren
Copy link
Contributor

@cmfield please do what @ocaisa asked, the PR looks good but need some real-world-testing too.

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 a pull request may close this issue.

4 participants