Skip to content

Commit

Permalink
Switch implicit make flags to use GNUMAKEFLAGS var
Browse files Browse the repository at this point in the history
There are a few notable elements addressed by this change:
1. The --load/-l argument is GNU-only and doesn't work on BSD make
2. The GNUMAKEFLAGS environment variable was previously ignored when
   looking for existing make arguments
3. By passing the flags via command line arguments to CMake's build
   invocation, the makeflags were sometimes omitted due to
   incompatibility.
  • Loading branch information
cottsay committed Nov 21, 2024
1 parent 454124f commit 0b46f2a
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 27 deletions.
45 changes: 18 additions & 27 deletions colcon_cmake/task/cmake/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ async def build( # noqa: D102
if environment_callback is not None:
environment_callback(env)

self._set_gnumakeflags(env)

rc = await self._reconfigure(args, env)
if rc:
return rc
Expand Down Expand Up @@ -241,10 +243,6 @@ async def _build(self, args, env, *, additional_targets=None):
cmd += ['--clean-first']
if multi_configuration_generator:
cmd += ['--config', self._get_configuration(args)]
else:
job_args = self._get_make_arguments(env)
if job_args:
cmd += ['--'] + job_args
completed = await run(
self.context, cmd, cwd=args.build_base, env=env)
if completed.returncode:
Expand Down Expand Up @@ -282,42 +280,42 @@ def _get_msbuild_environment(self, args, env):
env['CL'] = ' '.join(cl_split)
return env

def _get_make_arguments(self, env):
def _set_gnumakeflags(self, env):
"""
Get the make arguments to limit the number of simultaneously run jobs.
Set GNUMAKEFLAGS to limit the number of simultaneously run jobs.
The arguments are chosen based on the `cpu_count`, e.g. -j4 -l4.
:param dict env: a dictionary with environment variables
:returns: list of make arguments
:rtype: list of strings
"""
# check MAKEFLAGS for -j/--jobs/-l/--load-average arguments
makeflags = env.get('MAKEFLAGS', '')
regex = (
r'(?:^|\s)'
r'(-?(?:j|l)(?:\s*[0-9]+|\s|$))'
r'|'
r'(?:^|\s)'
r'((?:--)?(?:jobs|load-average)(?:(?:=|\s+)[0-9]+|(?:\s|$)))'
)
matches = re.findall(regex, makeflags) or []
matches = [m[0] or m[1] for m in matches]
if matches:
# do not extend make arguments, let MAKEFLAGS set things
return []
# check MAKEFLAGS for -j/--jobs/-l/--load-average arguments
makeflags = env.get('MAKEFLAGS', '')
if any(m[0] or m[1] for m in re.finditer(regex, makeflags)):
return
# check GNUMAKEFLAGS for -j/--jobs/-l/--load-average arguments
gnumakeflags = env.get('GNUMAKEFLAGS', '')
if any(m[0] or m[1] for m in re.finditer(regex, gnumakeflags)):
return

# Use the number of CPU cores
jobs = os.cpu_count()
with suppress(AttributeError):
# consider restricted set of CPUs if applicable
jobs = min(jobs, len(os.sched_getaffinity(0)))
if jobs is None:
# the number of cores can't be determined
return []
return [
'-j{jobs}'.format_map(locals()),
'-l{jobs}'.format_map(locals()),
]
return

env['GNUMAKEFLAGS'] = f'-j{jobs} -l{jobs}'
if gnumakeflags:
env['GNUMAKEFLAGS'] += ' ' + gnumakeflags

async def _install(self, args, env):
self.progress('install')
Expand All @@ -326,12 +324,9 @@ async def _install(self, args, env):
raise RuntimeError("Could not find 'cmake' executable")
cmd = [CMAKE_EXECUTABLE]
cmake_ver = get_cmake_version()
allow_job_args = True
if cmake_ver and cmake_ver >= Version('3.15.0'):
# CMake 3.15+ supports invoking `cmake --install`
cmd += ['--install', args.build_base]
# Job args not compatible with --install directive
allow_job_args = False
else:
# fallback to the install target which implicitly runs a build
if not cmake_ver:
Expand All @@ -344,9 +339,5 @@ async def _install(self, args, env):
args.build_base, args.cmake_args)
if multi_configuration_generator:
cmd += ['--config', self._get_configuration(args)]
elif allow_job_args:
job_args = self._get_make_arguments(env)
if job_args:
cmd += ['--'] + job_args
return await run(
self.context, cmd, cwd=args.build_base, env=env)
2 changes: 2 additions & 0 deletions test/spell_check.words
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,11 @@ ctest
ctests
dcmake
etree
finditer
findtext
getaffinity
github
gnumakeflags
https
iterdir
kazys
Expand Down

0 comments on commit 0b46f2a

Please sign in to comment.