Skip to content

generic sanity-checking of libraries#1424

Closed
torbjoernk wants to merge 8 commits intoeasybuilders:developfrom
torbjoernk:feature/sanity-check-libs
Closed

generic sanity-checking of libraries#1424
torbjoernk wants to merge 8 commits intoeasybuilders:developfrom
torbjoernk:feature/sanity-check-libs

Conversation

@torbjoernk
Copy link

A new key ('libs') is introduced potentially holding a list of maps specifying libraries to be checked in a generic way. Each library map must have a 'name' and a 'kind' while the latter must either be 'shared' or 'static'. The name is only the library name without path and suffix.

Example:
To test for a library libmy.so, which might either be in lib/ or in lib64/:

sanity_check_paths = {
  'libs': [{'name': 'libmy', 'kind': 'shared'}]
}

This will look for the following files:

lib/libmy.so
lib/libmy.dyn
lib64/libmy.so
lib64/libmy.dyn

This is done by generating a tuple of these, which is appended to the sanity_check_paths['files'] list.

Todo:

  • make the library directory and library suffixes configurable
  • think of better way to specify the libraries in the easyconfig file

BTW: The first commit is a potential bug fix. The lambdas specified in path_keys_and_check where never used.

Signed-off-by: Torbjörn Klatt <opensource@torbjoern-klatt.de>
A new key (`'libs'`) is introduced potentially holding a list of maps
specifying libraries to be checked in a generic way.
Each library map must have a `'name'` and a `'kind'` while the latter
must either be `'shared'` or `'static'`. The name is only the library
name without path and suffix.

Example:
To test for a library `libmy.so`, which might either be in `lib/`
or in `lib64/`:

    sanity_check_paths = {
      'libs': [{'name': 'libmy', 'kind': 'shared'}]
    }

This will look for the following files:

    lib/libmy.so
    lib/libmy.dyn
    lib64/libmy.so
    lib64/libmy.dyn

This is done by generating a tuple of these, which is appended to the
`sanity_check_paths['files']` list.

Signed-off-by: Torbjörn Klatt <opensource@torbjoern-klatt.de>
@hpcugentbot
Copy link

Automatic reply from Jenkins: Can I test this?

Copy link
Member

Choose a reason for hiding this comment

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

constant should be defined at the top of this file, below imports

rather than hardcoding .so or .dyn, use get_shared_lib_ext() from easybuild.tools.systemtools

Copy link
Author

Choose a reason for hiding this comment

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

Awesome. I was looking for such a method but couldn't find it. Thanks.

Edit: There is no such method for static libraries, right?

@boegel
Copy link
Member

boegel commented Oct 12, 2015

@torbjoernk: currently, this will be a bit verbose in the easyconfig due to requiring the name and kind keys; two alternatives I have in mind are using a tuple rather than a dictionary for the libs entries, or using shared_libs and static_libs keys.

Also, doesn't it make sense to omit the lib part too, since that will always be there?

Example comparison:

sanity_check_paths = {
    'libs': [{'name': 'foo', 'kind': 'shared'}, {'name': 'foo', 'kind': 'static'},
             {'name': 'bar', 'kind': 'shared'}, {'name': 'bar', 'kind': 'static'}],
}

vs

sanity_check_paths = {
    'libs': [('foo', 'shared'), ('foo', 'static'), ('bar', 'shared'), ('bar', 'static')],
}

vs

sanity_check_paths = {
    'shared_libs': ['foo', 'bar'],
    'static_libs': ['foo', 'bar'],
}

@boegel
Copy link
Member

boegel commented Oct 12, 2015

Jenkins: ok to test

@hpcugentbot
Copy link

EasyBuild framework unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2181/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

@torbjoernk
Copy link
Author

I personally like the last suggested version with separate fields for shared and static libraries. That seems the least verbose, most expressive and least typo-prone version.
I'll try to implement that one.

Implement @boegel's suggestion of specifying shared and static libraries
separately:

  sanity_check_paths = {
    'shared_libs': ['libone', 'libtwo'],
    'static_libs': ['libone']
  }

Signed-off-by: Torbjörn Klatt <t.klatt@fz-juelich.de>
@hpcugentbot
Copy link

EasyBuild framework unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2186/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

* develop:
  skip yeb tests that require PyYAML if it's not  available
  only register tag handler if 'import yaml' worked
  fix broken test
  fix .load() statements in toolchain.py, no named argument 'silent'
  ensure clear error message for failing test in test__list_toolchains
  make sure extended_dry_run build option is always defined
  provide default fallback value for defining Toolchain.dry_run + minor cleanups
  remove unused named argument 'silent' in ModulesTool.load() method
  avoid inline return in filetools.py functions
  ensure clear error message when quering undefined build option, add support for specifying a default fallback value
  don't provide default fallback value in build_option method, quering an unknown build option *should* fail
  fix typo
  rename 'forced' named argument of run_cmd to 'force_in_dry_run' following suggesting by @rjeschmi
  avoid double quotes when ignoring errors
  us %(version)s in source_urls in Python .yeb easyconfig
  fixed templating in exts_list for Python test file
  add self.dry_run_msg method in EasyBlock class
  fixed remarks + failing test
  added sqlite and python test files
  move printing list of loaded modules to toolchain.prepare
  Avoid negations in logic to make code more readable
  Fix HMNS to always use full version number
  add test for adjust_permissions
  Fix @boegel's remarks
  Skip symlinked files when fixing permissions
  fix @geimer's remarks
  fix dry run for checksums
  dry run output for checksums
  guess likely (parent path of) start dir in dry run mode
  check default behaviour w.r.t. converting mismatching type for easyconfig parameter values
  fix broken test
  enable auto-converting type of easyconfig parameter values
  Delete goolf-1.4.10.eb
  fixed spacing on goolf test files
  fixed remarks
  delete intel-2015b.eb test file
  filename param specification
  better parser docstring
  parser docstring
  spacing
  fixed remarks
  Add auto_convert_value_types parameter to parsers
  delete intel-2015b.yeb from tests
  force auto conversion
  small test for yaml_join
  make sure that files are not directories
  fix check for existing files/dirs in sanity check
  fix remarks
  enhance dry run output for fetching files, add 'no ignored errors' check in test for -x
  use dry_run_warning in intelfftw.py
  specify path to apply patch in via 'path' named argument of run_cmd
  fix regex in unit tests
  fix remarks
  extension: use empty list as default value for src/patches
  no (more) need for quotes in version definition in gzip.yeb
  Add join functionality
  Add test files with dependencies
  add extra test cases suggested by @wpoely86
  add support for auto-converting to expected type in check_type_of_param_value
  implement convert_value_type function
  docstrings
  add a comma
  exclude broken easyconfigs from tests that don't expect them
  move call to check_value_types in parser, add easyconfig with wrong value types as test case
  small style changes
  add unit test for check_type_of_param_value, in new test module
  implement basic support for type checking of easyconfig parameters
  fix remarks
  added gzip test case
  enhance unit test for module_generator.load_module
  fix long lines, us recursive_unload for all module_generator.load_module statements
  Fixed typo, changed setting name
  Fix missing easconfig param and add default
  Add support for setting recursive unload in easyconfig file
  fix remarks
  avoid duplicate check_ec_type call
  rename to modaltsoftname
  add test cases for weld_paths with absolute paths and fix the implementation to make them pass ^_^
  fix broken tests
  revert change in test imkl module
  put abspath back
  extend test for patch_step to also check patch-by-copy
  implement weld_paths function + use it to determine path to copy patch files to
  pick MPICH MPI compiler wrappers based on version, fix broken tests
  fix broken checks
  fix typo
  set MPI wrapper comments via _set_mpi_compiler_vars in openmpi.py toolchain support for OpenMPI
  set MPI wrapper comments via _set_compiler_vars in openmpi.py toolchain support for OpenMPI
  fix moar issues
  fix another typo
  fix typo
  also define
  fix remarks + remove hack for fixed issue with license constants
  enhance test for guess_start_dir, extra log statement
  Make both parts consistent
  Remove logger from test
  New test to check the guess_start_dir
  fix remarks to make basic .yeb support ready to merge
  fix guess_start_dir in case we're already in start_dir
  also exclude .yeb test easyconfigs from test_module_naming_scheme
  YAML parsing of basic easyconfig files
  small fixes + is_yeb unit test
  require --experimental when using .yeb easyconfigs
  make PyYAML dependency optional
  'fix' broken scripts test
  fix description match in .yeb unit test
  add first unit test for .yeb
  fix remarks
  add missing(?) extend_path in easybuild/tools/toolchain/__init__.py
  consider python2 before python
  only define $pyver_min when it's required in 'eb'
  add unit test module for testing functionality in environment.py
  add unit test for verifying behavior of run commands in dry run mode
  add support for disabling ignoring of errors that occur during dry run
  fix style issues
  move up self.dry_run definition
  use self.dry_run in easyblock.py/toolchain.py + provide/use dry_run_msg function for printing
  fix pattern in tests that verify missing deps
  fix broken robot_find_easyconfigs
  make eb wrapper use python2 if 'python' is not python2
  make print in bootstrap script compatible with python==python3
  remove requirement of having 'robot_path' build option defined as not None in robot_find_easyconfig
  auto-enable --ignore-osdeps under -x
  add unit test for --extended-dry-run/-x + make toy easyblock compatible with it
  pass self.silent to toolchain.prepare + fix printing of sanity check paths (can be a tuple)
  simulate 'module load' for missing toolchain/dependency modules + break up toolchain.prepare into (private) submethods
  don't print msg for every 'module load' in dry run mode
  don't raise error when in dry run mode for missing Intel FFTW libs
  nicer dry run output if no modules were loaded by toolchain.prepare()
  add pyyaml as install requirement
  start parsing of yeb files
  removing files from version control
  also define $FC in build environment
  force download/write for --from-pr
  add missing check on fake_mod_data
  indent dry run of run_cmd*, improve dry run of apply_patch
  fix issue with fake_mod_data not being defined
  dry run of apply_patch rather than patch_step
  only print (none) for no patches under -x
  significantly improve output in extensions_step
  add 'verbose' option to run_cmd
  avoid excessive output in extensions_step
  fix remark
  avoid long line in dry run msg for run_cmd*
  tweak fake build/install dir path to make it include the actual build/install dir as a suffix
  improve dry run for fetch_step
  include contents of generated module in output of --extended-dry-run
  fix typo
  add [DRY RUN] tag for every step
  print msg to warn about possible quirks in dry run output
  issue warning at the end in case of ignored errors
  implement more informative dry run version of sanity_check_step
  add support for specifying alternate name to be part of generated module name
  don't skip available modules with -x
  make preparing of build environment copy-paste'able in -x output
  catch all exceptions under --extended-dry-run
  catch errors and continue under --extended-dry-run
  dry run for apply_regex_substitutions
  add apply_regex_substitutions function
  dry run for patch_perl_script_autoflush
  don't use dry run version of extract_step + tweak extract_file function
  fix infinite recursion in extract_step
  dry run for patch_step
  implement support for extended dry run
  add support for specifying default value for (missing) build option
  defined --extended-dry-run configure/build option
  move set_tmpdir from tools/config.py to tools/options.py
  add unit test for HgRepository
  fix typo
  replace log.error with 'raise EasyBuildError' in hgrepo.py
  expand_toolchain_load still required
  Update toolchain_mns.py
  Update toolchain_mns.py
  Update toolchain_mns.py
  Update toolchain_mns.py
  Add initial support for a toolchain based module naming schemei, where software is installed in a directory specific to each toolchain, and each toolchain module extends the path. This won't work unless toolchains properly inherit (iccifort -> iimpi -> intel) and dependencies are listed appropriately in toolchains.
  Catch OSError exception for cloning step.
  Apply remarks to modules in easybuild.tools.repository
  Add mercurial repository support.

Signed-off-by: Torbjörn Klatt <opensource@torbjoern-klatt.de>
@hpcugentbot
Copy link

EasyBuild framework unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2267/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

@boegel boegel added this to the v2.4.0 milestone Oct 29, 2015
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't one suffix suffice? I don't see it happening soon that we'll have multiple suffixes?

It will also simplify things below a bit.

@boegel boegel modified the milestones: v2.5.0, v2.4.0 Oct 29, 2015
Copy link
Member

Choose a reason for hiding this comment

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

I agree, making this configurable makes a lot of sense.

@boegel
Copy link
Member

boegel commented Oct 29, 2015

@torbjoernk: we also need enhanced unit tests for this... Not sure how easy that would be, shout if you need help.

@Boegle remarked that EB probably doesn't need to deal with more than on
library suffix for either shared or static libs on a single platform.

Signed-off-by: Torbjörn Klatt <t.klatt@fz-juelich.de>
Signed-off-by: Torbjörn Klatt <t.klatt@fz-juelich.de>
@torbjoernk
Copy link
Author

I've implement a simple test case for the new library specifiers. Is that the kind of test you had in mind?

@hpcugentbot
Copy link

EasyBuild framework unit test suite FAILed.

See https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2383/console for more details.

Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do.

@torbjoernk
Copy link
Author

I have no idea why this fails on your Jenkins. 😕

@boegel boegel modified the milestones: v2.5.0, v2.6.0 Dec 14, 2015
@boegel
Copy link
Member

boegel commented Dec 17, 2015

Jenkins: test this please

@hpcugentbot
Copy link

EasyBuild framework unit test suite FAILed.

See https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2464/console for more details.

Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do.

@boegel
Copy link
Member

boegel commented Jan 9, 2016

Jenkins: test this please

@hpcugentbot
Copy link

EasyBuild framework unit test suite FAILed.

See https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2488/console for more details.

Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do.

@boegel
Copy link
Member

boegel commented Jan 10, 2016

@torbjoernk: the problem is that you're changing the toy-0.0.eb test easyconfig, and specify that libtoy.so and libtoy.a must be there

In the unit test you add, you make sure they're there, but the sanity check for that easyconfig fails in other tests where it is used.

You should avoid changing the toy-0.0.eb test easyconfig, and just modify the sanity_check_paths parameter in your test, something like this:

# specify that libtoy.a and libtoy.so should also be there, either in lib or lib64
ec['ec']['sanity_check_paths'].update({
    'shared_libs': ['libtoy'],
    'static_libs': ['libtoy'],
})
eb = EB_toy(ec['ec'])
...

@boegel boegel modified the milestones: v2.6.0, v2.7.0 Jan 22, 2016
Signed-off-by: Torbjörn Klatt <t.klatt@fz-juelich.de>
@hpcugentbot
Copy link

EasyBuild framework unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2591/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

@akesandgren
Copy link
Contributor

@boegel should we move forward with this?

@boegel
Copy link
Member

boegel commented Feb 10, 2020

@akesandgren It may be worth picking this up again, but it should be revived and re-evaluated, it's been too long...

@boegel boegel modified the milestones: 3.x, 4.x Feb 20, 2020
@torbjoernk torbjoernk closed this by deleting the head repository Jan 27, 2025
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 this pull request may close these issues.

4 participants