easybuilders / easybuild-easyblocks

Collection of easyblocks that implement support for building and installing software with EasyBuild.
https://easybuild.io
GNU General Public License v2.0
104 stars 284 forks source link

Binary block does not listen to extract_sources #1462

Open paulmelis opened 6 years ago

paulmelis commented 6 years ago

Given this config:

easyblock = 'Binary'

name = 'ExifTool'
version = '11.01'

homepage = 'http://owl.phy.queensu.ca/~phil/exiftool/'
description = """Perl module (Image::ExifTool) and program (exiftool)
to read EXIF information from images"""

toolchain = {'name': 'GCCcore', 'version': '6.4.0'}

sources = ['Image-ExifTool-11.01.tar.gz']
source_urls = ['https://www.sno.phy.queensu.ca/~phil/exiftool/']

extract_sources = True

dependencies = [
    ('Perl', '5.26.0')
]

sanity_check_paths = {
    'files': ['exiftool'],
    'dirs' : [],
}

moduleclass = 'tools'

Building fails as the tarball isn't extracted:

paulm@tcn172 11:03 ~/easybuild/easyconfigs-surfsara-git/e/ExifTool$ eblocalinstall -f ExifTool-11.01-GCCcore-6.4.0.eb 
== temporary log file in case of crash /tmp/paulm/eb-IPhSxu/easybuild-zDkbJU.log
== processing EasyBuild easyconfig /nfs/home4/paulm/easybuild/easyconfigs-surfsara-git/e/ExifTool/ExifTool-11.01-GCCcore-6.4.0.eb
== building and installing ExifTool/11.01-GCCcore-6.4.0...
== fetching files...
== creating build dir, resetting environment...
== unpacking...
== patching...
== preparing...
== configuring...
== building...
== testing...
== installing...
== taking care of extensions...
== postprocessing...
== sanity checking...
== FAILED: Installation ended unsuccessfully (build directory: /tmp/paulm/ExifTool/11.01/GCCcore-6.4.0): build failed (first 300 chars): Sanity check failed: no file of ('exiftool',) in /home/paulm/.local/easybuild/RedHatEnterpriseServer7/software/ExifTool/11.01-GCCcore-6.4.0
== Results of the build can be found in the log file(s) /tmp/paulm/eb-IPhSxu/easybuild-ExifTool-11.01-20180718.110347.WMRdH.log
ERROR: Build of /nfs/home4/paulm/easybuild/easyconfigs-surfsara-git/e/ExifTool/ExifTool-11.01-GCCcore-6.4.0.eb failed (err: "build failed (first 300 chars): Sanity check failed: no file of ('exiftool',) in /home/paulm/.local/easybuild/RedHatEnterpriseServer7/software/ExifTool/11.01-GCCcore-6.4.0")
File does not exist
paulm@tcn172 11:03 ~/easybuild/easyconfigs-surfsara-git/e/ExifTool$ ls /home/paulm/.local/easybuild/RedHatEnterpriseServer7/software/ExifTool/11.01-GCCcore-6.4.0
Image-ExifTool-11.01.tar.gz
paulm@tcn172 11:03 ~/easybuild/easyconfigs-surfsara-git/e/ExifTool$ grep Unpacking /tmp/paulm/eb-IPhSxu/easybuild-ExifTool-11.01-20180718.110347.WMRdH.log
paulm@tcn172 11:04 ~/easybuild/easyconfigs-surfsara-git/e/ExifTool$ 

Changing the config to use PackedBinary works:

paulm@tcn172 11:04 ~/easybuild/easyconfigs-surfsara-git/e/ExifTool$ cat ExifTool-11.01-GCCcore-6.4.0.eb
easyblock = 'PackedBinary'

name = 'ExifTool'
version = '11.01'

homepage = 'http://owl.phy.queensu.ca/~phil/exiftool/'
description = """Perl module (Image::ExifTool) and program (exiftool)
to read EXIF information from images"""

toolchain = {'name': 'GCCcore', 'version': '6.4.0'}

sources = ['Image-ExifTool-11.01.tar.gz']
source_urls = ['https://www.sno.phy.queensu.ca/~phil/exiftool/']

#extract_sources = True

dependencies = [
    ('Perl', '5.26.0')
]

sanity_check_paths = {
    'files': ['exiftool'],
    'dirs' : [],
}

moduleclass = 'tools'

paulm@tcn172 11:04 ~/easybuild/easyconfigs-surfsara-git/e/ExifTool$ eblocalinstall -f ExifTool-11.01-GCCcore-6.4.0.eb 
== temporary log file in case of crash /tmp/paulm/eb-lmG35M/easybuild-vzvyuN.log
== processing EasyBuild easyconfig /nfs/home4/paulm/easybuild/easyconfigs-surfsara-git/e/ExifTool/ExifTool-11.01-GCCcore-6.4.0.eb
== building and installing ExifTool/11.01-GCCcore-6.4.0...
== fetching files...
== creating build dir, resetting environment...
== unpacking...
== patching...
== preparing...
== configuring...
== building...
== testing...
== installing...
== taking care of extensions...
== postprocessing...
== sanity checking...
== cleaning up...
== creating module...
== permissions...
== packaging...
== COMPLETED: Installation ended successfully
== Results of the build can be found in the log file(s) /home/paulm/.local/easybuild/RedHatEnterpriseServer7/software/ExifTool/11.01-GCCcore-6.4.0/easybuild/easybuild-ExifTool-11.01-20180718.110518.log
== Build succeeded for 1 out of 1
== Temporary log file(s) /tmp/paulm/eb-lmG35M/easybuild-vzvyuN.log* have been removed.
== Temporary directory /tmp/paulm/eb-lmG35M has been removed.
grpaulm@tcn172 11:05 ~/easybuild/easyconfigs-surfsara-git/e/ExifTool$ grep Unpacking /home/paulm/.local/easybuild/RedHatEnterpriseServer7/software/ExifTool/11.01-GCCcore-6.4.0/easybuild/easybuild-ExifTool-11.01-20180718.1105.log
== 2018-07-18 11:05:07,910 easyblock.py:1666 INFO Unpacking source Image-ExifTool-11.01.tar.gz

I don't understand why this is happening. The docs seems to suggest setting extract_sources to True should unpack the source tarball, but it doesn't happen. The relevant code seems ok.

ocaisa commented 6 years ago

I used this easyblock today, I think the problem is a misunderstanding. What you probably wanted is:

buildininstalldir = True
extract_sources = True

The Binary easyblock by default unpacks into the temporary build space, if you don't give it an install_cmd then nothing will happen. If you look at the source for PackedBinary you see all it does is copy the unpacked sources from the builddir to the installdir (which is exactly what you wanted in this case).

paulmelis commented 6 years ago

Hmm, I've never used the buildininstalldir option, seems like that's asking for trouble as the default is False.

For me the docs are a bit underspecified/ambiguous:

Binary (derives from EasyBLock) Support for installing software that comes in binary form. Just copy the sources to the install dir, or use the specified install command.

PackedBinary (derives from Binary, EasyBlock) Support for installing packed binary software. Just unpack the sources in the install dir

If PackedBinary is meant for packed binary software then Binary is apparently not meant for packed software. Which I think I've discovered in the original report, because PackedBinary is the block to use. But then the extract_sources option of Binary doesn't make any sense, as it clearly unpacks something: when Binary.extract_sources is True it simply calls EasyBlock.extract_sources which extracts each of the source files to the build directory. In fact, when Binary.extract_sources is True the unpack step is exactly the same as for PackedBinary.

So what's the use case anyway for Binary?

ocaisa commented 6 years ago

Hmm, I just tried your failing 'Binary' example and it worked for me, what version of EB are you using?

Probably the main difference between the two easyblocks is that is that you can run the install_cmd in every directory that get's unpacked in the PackedBinary case, how useful that really is I don't know.

The basic logic is that sometimes you don't want to unpack, hence Binary. There are some decent examples of the Binary use case in the repo, for example,

But most times you do want to unpack, hence many more examples of PackedBinary in the easyconfig repo.

I was using Binary this week for a trial run of installing Go packages (https://github.com/easybuilders/easybuild-easyblocks/issues/1466).

The docs are partly auto-generated and this can lead to things not being entirely clear. If you think the docs could be clearer you can make a PR against the docstring in the easyblocks: https://github.com/easybuilders/easybuild-easyblocks/blob/master/easybuild/easyblocks/generic/binary.py#L51 https://github.com/easybuilders/easybuild-easyblocks/blob/master/easybuild/easyblocks/generic/packedbinary.py#L40

ocaisa commented 6 years ago

Ah, the working version was 3.6.2, I just tried 3.5.3 and it failed so something has been fixed along the way.

ocaisa commented 6 years ago

Looks like the fix was here https://github.com/easybuilders/easybuild-easyblocks/commit/2a88c972a00c129e908c740228cccdc2b779a17b for version 3.6.0

paulmelis commented 6 years ago

Thanks for looking this up. Support for extract_sources was added due to #1401, but why? Why not point people to PackedBinary instead of adding a confusing and overlapping option to Binary? I really do not understand the uses cases for these separate blocks, why not merge them?

ocaisa commented 6 years ago

Thoughts @boegel ?

The thing is, before they didn't quite overlap and now they do. The way it stands, both currently exist and are being used; and to maintain backwards compatibility they will both have to remain. For EB 4.0 we could (now) merge them.