Skip to content

Conversation

@rsdmse
Copy link
Contributor

@rsdmse rsdmse commented May 23, 2020

(created using eb --new-pr)

@rsdmse
Copy link
Contributor Author

rsdmse commented May 23, 2020

The prefix should've been
{chem}[GCCcore/system]

@rsdmse rsdmse changed the title {chem}[dummy/] atat v3.36 {chem}[GCCcore/system] atat v3.36 May 23, 2020
@rsdmse rsdmse changed the title {chem}[GCCcore/system] atat v3.36 {chem}[GCCcore/9.3.0] atat v3.36 May 23, 2020
@boegel
Copy link
Member

boegel commented May 24, 2020

Test report by @boegel
FAILED
Build succeeded for 0 out of 1 (1 easyconfigs in this PR)
generoso - Linux centos linux 7.6.1810, x86_64, Intel(R) Xeon(R) CPU E5-2660 v4 @ 2.00GHz, Python 3.6.8
See https://gist.github.com/22c4bfdc7de2b2b732d329f03b152291 for a full test report.

Copy link
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

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

@rsdmse Thank you very much for your contribution!

I tried installing this easyconfig file, but it doesn't work for me, see the uploading test report.
It seems like we should define $BINDIR to %(installdir)s/bin?

That could be done with:

prebuildopts = "export BINDIR=%(installdir)s/bin && "

Do let us know if you need help with processing the suggested changes.

@easybuilders easybuilders deleted a comment from boegelbot May 24, 2020
@boegel boegel added the new label May 24, 2020
@boegel boegel added this to the 4.x milestone May 24, 2020
@boegel
Copy link
Member

boegel commented May 24, 2020

The BINDIR variable is only invoked during make install but I see that a nonexistent BINDIR is causing the failure in the make stage in your test (even though the installation was successful locally and on Travis). I had to export it separately in prebuildopts and postinstallcmds, which isn't very clean but I couldn't find another way.

Before this commit I tried to put the customized install commands in install_cmd but that was ignored by EasyBuild. (Although eb -a -e MakeCp shows install_cmd is a valid parameter in MakeCp.) So I had to put those under postinstallcmds.

The fact that install_cmd is still listed as a valid easyconfig parameter is indeed confusing, please open an issue in https://github.com/easybuilders/easybuild-easyblocks/issues for this, we should get that cleaned up.

I'm not sure MakeCp is the best choice of easyblock here, since you seem to suggest that there's a proper make install supported by ATAT?

Maybe using ConfigureMake together with skipsteps = ['configure'] is a better fit, since then you can use installopts rather than postinstallcmds:

preinstallopts = "export BINDIR=%(installdir)s/bin && "
installopts = "-C src && make install -C glue/jobctrl && make install -C glue/vasp"

The make clean is then no longer needed anymore, since we're not copying the whole build directory in the installation directory anymore (so files_to_copy is also useless).

@boegel
Copy link
Member

boegel commented May 24, 2020

Test report by @boegel
FAILED
Build succeeded for 0 out of 1 (1 easyconfigs in this PR)
generoso - Linux centos linux 7.6.1810, x86_64, Intel(R) Xeon(R) CPU E5-2660 v4 @ 2.00GHz, Python 3.6.8
See https://gist.github.com/1e4b9ae7dbfa5bbc37ae007aa96bd7cb for a full test report.

@boegel
Copy link
Member

boegel commented May 24, 2020

@rsdmse The BINDIR issue is still there for me, it looks like export BINDIR isn't working, we need to override the BINDIR value by passing an argument to make, so something like:

prebuildopts = "mkdir -p %(installdir)s/bin && "
buildopts = "BINDIR=%(installdir)s/bin"

@boegel boegel changed the title {chem}[GCCcore/9.3.0] atat v3.36 {chem}[GCCcore/9.3.0] ATAT v3.36 May 24, 2020
@rsdmse
Copy link
Contributor Author

rsdmse commented May 24, 2020

Thanks again for your help. I discovered that I can't reference %(installdir)s in the build step, so I had to do mkdir %(builddir)s/bin, use that as BINDIR, and later copy bin to %(installdir)s as a postinstallcmd. It should work this time, hopefully!

@rsdmse
Copy link
Contributor Author

rsdmse commented May 24, 2020

I don't understand what's causing this error?

....E.E./opt/hostedtoolcache/Python/3.7.7/x64/lib/python3.7/site-packages/pep8.py:110: FutureWarning: Possible nested set at position 1
  EXTRANEOUS_WHITESPACE_REGEX = re.compile(r'[[({] | []}),;:]')

@lexming
Copy link
Contributor

lexming commented Jul 4, 2020

Test report by @lexming
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in this PR)
node379.hydra.os - Linux centos linux 7.7.1908, x86_64, Intel(R) Xeon(R) Gold 6148 CPU @ 2.40GHz, Python 2.7.5
See https://gist.github.com/3ba6422a02d517c554027f0d0be9f95c for a full test report.

@lexming
Copy link
Contributor

lexming commented Jul 4, 2020

Test report by @lexming
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in this PR)
node051.manticore.os - Linux centos linux 7.8.2003, x86_64, Intel(R) Xeon(R) CPU E5-2650L v2 @ 1.70GHz, Python 2.7.5
See https://gist.github.com/76ba92a809e6ab56952b8f29b7083ffa for a full test report.

lexming
lexming previously approved these changes Jul 4, 2020
Copy link
Contributor

@lexming lexming left a comment

Choose a reason for hiding this comment

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

LGTM

@lexming
Copy link
Contributor

lexming commented Jul 4, 2020

@boegel what do you think? Could you repeat your failed test?

@rsdmse
Copy link
Contributor Author

rsdmse commented Oct 2, 2020

@boegel The requested changes have been made and all checks have passed

@boegel
Copy link
Member

boegel commented Jun 28, 2023

merged via #18213 (but commit history got messed up somehow, so not automatically detected as merged)

@boegel boegel closed this Jun 28, 2023
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.

3 participants