easybuild-easyblocks icon indicating copy to clipboard operation
easybuild-easyblocks copied to clipboard

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

Open casparvl opened this issue 7 years ago • 5 comments

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.

casparvl avatar Jan 04 '18 09:01 casparvl

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

cmfield avatar Jan 12 '21 09:01 cmfield

@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 avatar Jan 12 '21 11:01 ocaisa

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 avatar Jan 12 '21 11:01 ocaisa

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

ocaisa avatar Jan 12 '21 12:01 ocaisa

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

akesandgren avatar Mar 29 '21 06:03 akesandgren