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

DAOS-623 build: Use AddMethod for adding methods to environments. #10654

Merged
merged 8 commits into from
Nov 3, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 9 additions & 13 deletions SConstruct
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,6 @@ wrap scons-3.""")

SCons.Warnings.warningAsException()

try:
input = raw_input # pylint: disable=redefined-builtin
except NameError:
pass


def get_version(env):
""" Read version from VERSION file """
Expand All @@ -50,6 +45,8 @@ API_VERSION = f'{API_VERSION_MAJOR}.{API_VERSION_MINOR}.{API_VERSION_FIX}'

def update_rpm_version(version, tag):
""" Update the version (and release) in the RPM spec file """

# pylint: disable=consider-using-f-string
daltonbohning marked this conversation as resolved.
Show resolved Hide resolved
spec = open("utils/rpms/daos.spec", "r").readlines() # pylint: disable=consider-using-with
current_version = 0
release = 0
Expand Down Expand Up @@ -151,6 +148,7 @@ def build_misc(build_prefix):
def scons(): # pylint: disable=too-many-locals,too-many-branches
"""Execute build"""
if COMMAND_LINE_TARGETS == ['release']:
# pylint: disable=consider-using-f-string
try:
# pylint: disable=import-outside-toplevel
import pygit2
Expand Down Expand Up @@ -376,15 +374,16 @@ def scons(): # pylint: disable=too-many-locals,too-many-branches

set_defaults(env, daos_version)

daos_build.setup(env)

base_env = env.Clone()

base_env_mpi = env.Clone()

compiler_setup.base_setup(env)

if not GetOption('help') and not GetOption('clean'):
mpi = daos_build.configure_mpi(base_env_mpi)
if not mpi:
if not base_env_mpi.d_configure_mpi():
print("\nSkipping compilation for tests that need MPI")
print("Install and load mpich or openmpi\n")
base_env_mpi = None
Expand All @@ -407,12 +406,9 @@ def scons(): # pylint: disable=too-many-locals,too-many-branches
buildinfo.save('.build_vars.json')
# also install to $PREFIX/lib to work with existing avocado test code
if prereqs.test_requested():
daos_build.install(env, "lib/daos/",
['.build_vars.sh', '.build_vars.json'])
env.Install('$PREFIX/lib/daos/TESTING/ftest/util',
['site_scons/env_modules.py'])
env.Install('$PREFIX/lib/daos/TESTING/ftest/',
['ftest.sh'])
env.Install('$PREFIX/lib/daos', ['.build_vars.sh', '.build_vars.json'])
env.Install('$PREFIX/lib/daos/TESTING/ftest/util', ['site_scons/env_modules.py'])
env.Install('$PREFIX/lib/daos/TESTING/ftest/', ['ftest.sh'])

env.Install("$PREFIX/lib64/daos", "VERSION")

Expand Down
53 changes: 27 additions & 26 deletions site_scons/daos_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,11 @@ def __hash__(self):
return hash(self.lstr)


def add_rpaths(env, install_off, set_cgo_ld, is_bin):
def _add_rpaths(env, install_off, set_cgo_ld, is_bin):
"""Add relative rpath entries"""
if GetOption('no_rpath'):
if set_cgo_ld:
env.AppendENVPath("CGO_LDFLAGS", env.subst("$_LIBDIRFLAGS "),
sep=" ")
env.AppendENVPath("CGO_LDFLAGS", env.subst("$_LIBDIRFLAGS "), sep=" ")
return
env.AppendUnique(RPATH_FULL=['$PREFIX/lib64'])
rpaths = env.subst("$RPATH_FULL").split()
Expand All @@ -45,18 +44,17 @@ def add_rpaths(env, install_off, set_cgo_ld, is_bin):
continue
relpath = os.path.relpath(rpath, prefix)
if relpath != rpath:
joined = os.path.normpath(os.path.join(install_off, relpath))
path = r'\$$ORIGIN/%s' % (joined)
if set_cgo_ld:
env.AppendENVPath("CGO_LDFLAGS", "-Wl,-rpath=$ORIGIN/%s/%s" %
(install_off, relpath), sep=" ")
env.AppendENVPath("CGO_LDFLAGS", f'-Wl,-rpath=$ORIGIN/{install_off}/{relpath}%s',
sep=" ")
else:
env.AppendUnique(RPATH=[DaosLiteral(path)])
joined = os.path.normpath(os.path.join(install_off, relpath))
env.AppendUnique(RPATH=[DaosLiteral(fr'\$$ORIGIN/{joined}')])
for rpath in rpaths:
path = os.path.join(prefix, rpath)
if is_bin:
# NB: Also use full path so intermediate linking works
env.AppendUnique(LINKFLAGS=["-Wl,-rpath-link=%s" % path])
env.AppendUnique(LINKFLAGS=[f'-Wl,-rpath-link={path}'])
else:
# NB: Also use full path so intermediate linking works
env.AppendUnique(RPATH=[path])
Expand All @@ -65,11 +63,13 @@ def add_rpaths(env, install_off, set_cgo_ld, is_bin):
env.AppendENVPath("CGO_LDFLAGS", env.subst("$_LIBDIRFLAGS $_RPATH"), sep=" ")


def add_build_rpath(env, pathin="."):
def _add_build_rpath(self, pathin="."):
"""Add a build directory to rpath"""

env = self
path = Dir(pathin).path
env.AppendUnique(LINKFLAGS=["-Wl,-rpath-link=%s" % path])
env.AppendENVPath("CGO_LDFLAGS", "-Wl,-rpath-link=%s" % path, sep=" ")
env.AppendUnique(LINKFLAGS=[f'-Wl,-rpath-link={path}'])
env.AppendENVPath('CGO_LDFLAGS', f'-Wl,-rpath-link={path}', sep=' ')
# We actually run installed binaries from the build area to generate
# man pages. In such cases, we need LD_LIBRARY_PATH set to pick up
# the dependencies
Expand Down Expand Up @@ -117,7 +117,7 @@ def _add_lib(libtype, libname, target):
libraries[libname][libtype] = target


def run_command(env, target, sources, daos_libs, command):
def _run_command(env, target, sources, daos_libs, command):
"""Run Command builder"""
static_deps, shared_deps = _known_deps(env, LIBS=daos_libs)
result = env.Command(target, sources + static_deps + shared_deps, command)
Expand All @@ -141,7 +141,7 @@ def library(env, *args, **kwargs):
"""build SharedLibrary with relative RPATH"""
denv = env.Clone()
denv.Replace(RPATH=[])
add_rpaths(denv, kwargs.get('install_off', '..'), False, False)
_add_rpaths(denv, kwargs.get('install_off', '..'), False, False)
lib = denv.SharedLibrary(*args, **kwargs)
libname = _get_libname(*args, **kwargs)
_add_lib('shared', libname, lib)
Expand All @@ -156,7 +156,7 @@ def program(env, *args, **kwargs):
denv = env.Clone()
denv.AppendUnique(LINKFLAGS=['-pie'])
denv.Replace(RPATH=[])
add_rpaths(denv, kwargs.get('install_off', '..'), False, True)
_add_rpaths(denv, kwargs.get('install_off', '..'), False, True)
prog = denv.Program(*args, **kwargs)
static_deps, shared_deps = _known_deps(env, **kwargs)
Depends(prog, static_deps)
Expand All @@ -169,7 +169,7 @@ def test(env, *args, **kwargs):
denv = env.Clone()
denv.AppendUnique(LINKFLAGS=['-pie'])
denv.Replace(RPATH=[])
add_rpaths(denv, kwargs.get("install_off", None), False, True)
_add_rpaths(denv, kwargs.get("install_off", None), False, True)
testbuild = denv.Program(*args, **kwargs)
static_deps, shared_deps = _known_deps(env, **kwargs)
Depends(testbuild, static_deps)
Expand All @@ -182,13 +182,6 @@ def add_static_library(name, target):
_add_lib('static', name, target)


def install(env, subdir, files):
"""install file to the subdir"""
denv = env.Clone()
path = "$PREFIX/%s" % subdir
denv.Install(path, files)


def _find_mpicc(env):
"""find mpicc"""

Expand Down Expand Up @@ -220,7 +213,7 @@ def _configure_mpi_pkg(env):
return True


def configure_mpi(env):
def _configure_mpi(env):
"""Check if mpi exists and configure environment"""

if GetOption('help'):
Expand All @@ -235,8 +228,16 @@ def configure_mpi(env):
if not load_mpi(mpi):
continue
if _find_mpicc(env):
print("%s is installed" % mpi)
print(f'{mpi} is installed')
return True
print("No %s installed and/or loaded" % mpi)
print('No {mpi} installed and/or loaded')
print("No MPI installed")
return False


def setup(env):
"""Add daos specific methods to environment"""
env.AddMethod(_add_build_rpath, 'd_add_build_rpath')
env.AddMethod(_configure_mpi, 'd_configure_mpi')
env.AddMethod(_run_command, 'd_run_command')
env.AddMethod(_add_rpaths, 'd_add_rpaths')
Comment on lines +238 to +243
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the real change in this PR, the rest is just fluff.

  1. Is this worthwhile?
  2. do we want a common 'd_' prefix for all our methods to make it clear they're non-standard.
  3. Should we copy the CamelCase naming of existing methods?
  4. Assuming this is worthwhile should we do it for all daos_build functions (which will touch more code than this PR).

Copy link
Contributor

@jolivier23 jolivier23 Oct 24, 2022

Choose a reason for hiding this comment

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

For point 2, I think we already have other precedent where we add stuff and don't name it specially. For instance, Configure CheckFlag and CheckFlagCC. I think it would be fine to do something like AddBuildRPATH. For point 4, possibly. But I wouldn't do that in a single PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would mean that we'd replace daos_build.library() with env.Library(), yet there is already a method with that name, the one that we're wrapping/

Copy link
Contributor

Choose a reason for hiding this comment

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

You could do something like DAOSLibrary, I suppose

2 changes: 1 addition & 1 deletion src/cart/SConscript
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def scons():

Import('env', 'prereqs', 'swim_targets', 'CART_VERSION')

daos_build.add_build_rpath(env)
env.d_add_build_rpath()

env.Alias('install', '$PREFIX')

Expand Down
4 changes: 1 addition & 3 deletions src/cart/utils/SConscript
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
#
"""Build crt_utils component"""

import daos_build

LIB_SRC = ['crt_utils.c']


Expand All @@ -25,7 +23,7 @@ def scons():

prereqs.require(env, 'protobufc')

daos_build.add_build_rpath(env)
env.d_add_build_rpath()

# Generate cart utility shared objects
build_utility_shared_obj(env)
Expand Down
2 changes: 1 addition & 1 deletion src/client/api/SConscript
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ def scons():
"""Execute build"""
Import('env', 'API_VERSION', 'prereqs', 'libdaos_tgts')

daos_build.add_build_rpath(env)
env.d_add_build_rpath()
env.AppendUnique(LIBPATH=[Dir('.')])
denv = env.Clone()
prereqs.require(denv, 'protobufc')
Expand Down
2 changes: 1 addition & 1 deletion src/client/dfs/SConscript
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def scons():
"""Execute build"""
Import('env')

daos_build.add_build_rpath(env)
env.d_add_build_rpath()

denv = env.Clone()

Expand Down
2 changes: 1 addition & 1 deletion src/client/pydaos/SConscript
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ def build_shim_module():
if GetOption('help'):
return

version = "{}.{}".format(sys.version_info.major, sys.version_info.minor)
version = f'{sys.version_info.major}.{sys.version_info.minor}'

Import('base_env')

Expand Down
4 changes: 2 additions & 2 deletions src/common/SConscript
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ def scons():

env.AppendUnique(LIBPATH=[Dir('.')])
base_env.AppendUnique(LIBPATH=[Dir('.')])
daos_build.add_build_rpath(base_env)
daos_build.add_build_rpath(env)
base_env.d_add_build_rpath()
env.d_add_build_rpath()

# Hack alert, the argobots headers are required but the shared
# library isn't so add the dependency so the include path
Expand Down
4 changes: 2 additions & 2 deletions src/common/tests/SConscript
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ def scons():
unit_env = tenv.Clone()
unit_env.AppendUnique(OBJPREFIX='utest_')

common_mock_ld_script = "%s/common-mock-ld-opts" % Dir('.').srcnode()
unit_env.AppendUnique(LINKFLAGS=['-Wl,@%s' % common_mock_ld_script])
common_mock_ld_script = f"{Dir('.').srcnode()}/common-mock-ld-opts"
unit_env.AppendUnique(LINKFLAGS=[f'-Wl,@{common_mock_ld_script}'])

mock_test_utils = unit_env.SharedObject(['test_mocks.c', 'test_utils.c'])
drpc_test_utils = unit_env.SharedObject(['../drpc.c']) + mock_test_utils
Expand Down
37 changes: 16 additions & 21 deletions src/control/SConscript
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#!/bin/env python
"""Build DAOS Control Plane"""
# pylint: disable=too-many-locals
import sys
Expand All @@ -7,7 +6,6 @@ import subprocess
from os.path import join, isdir
from os import urandom
from binascii import b2a_hex
import daos_build

Import('env', 'prereqs', 'daos_version', 'conf_dir')

Expand Down Expand Up @@ -111,20 +109,19 @@ def install_go_bin(denv, gosrc, libs, name, install_name):
def go_ldflags():
"Create the ldflags option for the Go build."
path = 'github.com/daos-stack/daos/src/control/build'
return ' '.join(['-ldflags',
'"-X {}.DaosVersion={}'.format(path, daos_version),
'-X {}.ConfigDir={}'.format(path, conf_dir),
'-B %s"' % gen_build_id()])
return ' '.join([f'-X {path}.DaosVersion={daos_version}',
f'-X {path}.ConfigDir={conf_dir}',
f'-B {gen_build_id()}'])
# Must be run from the top of the source dir in order to
# pick up the vendored modules.
# Propagate useful GO environment variables from the caller
if 'GOCACHE' in os.environ:
denv['ENV']['GOCACHE'] = os.environ['GOCACHE']
target = daos_build.run_command(denv, build_bin, src, libs,
f'cd {gosrc}; {GO_BIN} build -mod vendor {go_ldflags()} '
+ f'{get_build_flags(denv)} '
+ f'{get_build_tags(denv)} '
+ f'-o {build_bin} {install_src}')
target = denv.d_run_command(build_bin, src, libs,
f'cd {gosrc}; {GO_BIN} build -mod vendor -ldflags "{go_ldflags()}" '
+ f'{get_build_flags(denv)} '
+ f'{get_build_tags(denv)} '
+ f'-o {build_bin} {install_src}')
# Use the intermediate build location in order to play nicely
# with --install-sandbox.
AlwaysBuild(target)
Expand All @@ -147,22 +144,21 @@ def configure_go(denv):
context.Result(0)
return 0

context.Display('Checking %s version... ' % GO_BIN)
context.Display(f'Checking {GO_BIN} version... ')
cmd_rc = subprocess.run([GO_BIN, 'version'], check=True, stdout=subprocess.PIPE)
out = cmd_rc.stdout.decode('utf-8')
if len(out.split(' ')) < 3:
context.Result('failed to get version from "%s"' % out)
context.Result(f'failed to get version from "{out}"')
return 0

# go version go1.2.3 Linux/amd64
go_version = out.split(' ')[2].replace('go', '')
if len([x for x, y in
zip(go_version.split('.'), MIN_GO_VERSION.split('.'))
if int(x) < int(y)]) > 0:
context.Result('%s is too old (min supported: %s) '
% (go_version, MIN_GO_VERSION))
context.Result(f'{go_version} is too old (min supported: {MIN_GO_VERSION}) ')
return 0
context.Result('%s' % go_version)
context.Result(str(go_version))
return 1

conf = Configure(denv, custom_tests={'CheckGoVersion': check_go_version})
Expand Down Expand Up @@ -210,7 +206,7 @@ def scons():
prereqs.require(denv, 'ofi', 'hwloc')
prereqs.require(denv, 'ucx')
# Sets CGO_LDFLAGS for rpath options
daos_build.add_rpaths(denv, "..", True, True)
denv.d_add_rpaths("..", True, True)
denv.AppendENVPath("CGO_CFLAGS", denv.subst("$_CPPINCFLAGS"), sep=" ")
if prereqs.client_requested():
install_go_bin(denv, gosrc, None, "daos_agent", "daos_agent")
Expand Down Expand Up @@ -241,7 +237,7 @@ def scons():

SConscript('lib/spdk/SConscript', exports='denv')

daos_build.add_rpaths(denv, "..", True, True)
denv.d_add_rpaths("..", True, True)

# Copy setup_spdk.sh script to be executed at daos_server runtime.
senv.Install(join('$PREFIX', 'share/daos/control'),
Expand All @@ -268,12 +264,11 @@ def scons():
aenv.AppendENVPath("CGO_LDFLAGS", ldopts, sep=" ")

aenv.AppendENVPath("CGO_CFLAGS",
senv.subst("-I$SPDK_PREFIX/include "
"-I$OFI_PREFIX/include"),
senv.subst("-I$SPDK_PREFIX/include -I$OFI_PREFIX/include"),
sep=" ")

# Sets CGO_LDFLAGS for rpath
daos_build.add_rpaths(aenv, None, True, True)
aenv.d_add_rpaths(None, True, True)
install_go_bin(aenv, gosrc, ['nvme_control'], "daos_server_helper", "daos_server_helper")

if is_firmware_mgmt_build(denv):
Expand Down
4 changes: 2 additions & 2 deletions src/engine/tests/SConscript
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ def scons():
unit_env = denv.Clone()
unit_env.AppendUnique(OBJPREFIX='utest_')

common_mock_ld_script = "%s/../../common/tests/common-mock-ld-opts" % Dir('.').srcnode()
unit_env.AppendUnique(LINKFLAGS=['-Wl,@%s' % common_mock_ld_script])
common_mock_ld_script = f"{Dir('.').srcnode()}/../../common/tests/common-mock-ld-opts"
unit_env.AppendUnique(LINKFLAGS=[f'-Wl,@{common_mock_ld_script}'])

Depends('drpc_progress_tests', common_mock_ld_script)
daos_build.test(unit_env, 'drpc_progress_tests',
Expand Down
Loading