Skip to content

Commit

Permalink
[yugabyte#14366] Improve usability of yb_release with LTO build
Browse files Browse the repository at this point in the history
Summary:
Prior to this diff, yb_release would not work with LTO builds unless a full build had already been done. There was an assumption that all protobufs would be generated by the time dependency graph traversal starts. Relaxing that assumption in case we know the build might be incomplete.

Other changes:
- Fixing the usability of --no-tests mode. Prior to this diff, the dependency graph framework self-test would fail.
- Deduplicating command line flags used to build Postgres.
- Removing a couple of unnecessary protobuf imports.

Test Plan:
Jenkins: compile only

Manual tests (a clean build in each case):
- LTO build through yb_release:
  - `./yb_release --build_args="--clang14 --lto=full" --force`
  - `build-support/dependency_graph --build-root=build/release-clang14-linuxbrew-full-lto-ninja --incomplete-build self-test`
- LTO build through yb_release without use of Linuxbrew:
  - `./yb_release --build_args="--lto=full --no-linuxbrew" --force`
  - `build-support/dependency_graph --build-root=build/release-clang14-full-lto-ninja --incomplete-build self-test`
- A build with no tests:
  - `./yb_build.sh --clang14 debug --no-tests --export-compile-commands`
  - `build-support/dependency_graph --build-root=build/debug-clang14-dynamic-ninja self-test`
- A build with tests:
  - `./yb_build.sh --clang14 fastdebug --export-compile-commands`
  - `build-support/dependency_graph --build-root=build/fastdebug-clang14-dynamic-ninja self-test`

Reviewers: sergei

Reviewed By: sergei

Subscribers: ybase, bogdan

Differential Revision: https://phabricator.dev.yugabyte.com/D20333
  • Loading branch information
mbautin committed Oct 21, 2022
1 parent aec6272 commit 7b0623e
Show file tree
Hide file tree
Showing 12 changed files with 338 additions and 93 deletions.
8 changes: 7 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -747,7 +747,7 @@ string (REPLACE "-Werror=reorder" "" CMAKE_C_FLAGS ${CMAKE_C_FLAGS})
string (REPLACE "-Wnon-virtual-dtor" "" CMAKE_C_FLAGS ${CMAKE_C_FLAGS})
string (REPLACE "-Woverloaded-virtual" "" CMAKE_C_FLAGS ${CMAKE_C_FLAGS})


yb_deduplicate_arguments(CMAKE_C_FLAGS)
message("CMAKE_C_FLAGS ${CMAKE_C_FLAGS}")

set(CMAKE_CXX_STANDARD 20)
Expand All @@ -758,8 +758,10 @@ if (IS_GCC AND
ADD_CXX_FLAGS("-Wno-aligned-new")
endif()

yb_deduplicate_arguments(CMAKE_CXX_FLAGS)
set(CMAKE_CXX_STANDARD_REQUIRED ON)

yb_deduplicate_arguments(CMAKE_EXE_LINKER_FLAGS)
message("Linker flags for executables: ${CMAKE_EXE_LINKER_FLAGS}")

# Define full paths to libpq and libyb_pgbackend shared libraries. These libraries are built as part
Expand Down Expand Up @@ -822,6 +824,10 @@ string(REPLACE "\\n" "\n" YB_ALL_DEPS_FINAL "${YB_ALL_DEPS}")
# The same target can appear on the left hand side of multiple such lines.
file(WRITE "${YB_BUILD_ROOT}/yb_cmake_deps.txt" "${YB_ALL_DEPS_FINAL}")

# ------------------------------------------------------------------------------------------------
# Final reporting
# ------------------------------------------------------------------------------------------------

math(EXPR YB_NUM_EXCLUDED_TESTS "${YB_NUM_TESTS} - ${YB_NUM_INCLUDED_TESTS}")
message("Total tests: ${YB_NUM_TESTS}, "
"included: ${YB_NUM_INCLUDED_TESTS}, "
Expand Down
39 changes: 36 additions & 3 deletions cmake_modules/YugabyteFunctions.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,9 @@ function(YB_INCLUDE_EXTENSIONS)
endfunction()

function(yb_remember_dependency target)
if("${ARGN}" STREQUAL "")
message(FATAL_ERROR "yb_remember_dependency() called with no arguments")
endif()
# We use \\n instead of a real newline as this is stored in the CMake cache, and some versions
# of CMake can't parse their own cache in case some values have newlines.
set(YB_ALL_DEPS "${YB_ALL_DEPS}\\n${target}: ${ARGN}" CACHE INTERNAL "All dependencies" FORCE)
Expand Down Expand Up @@ -577,8 +580,9 @@ function(ADD_YB_LIBRARY LIB_NAME)

add_library(${LIB_NAME} ${ARG_SRCS})

target_link_libraries(${LIB_NAME} ${ARG_DEPS})
yb_remember_dependency(${LIB_NAME} ${ARG_DEPS})
if(ARG_DEPS)
target_link_libraries(${LIB_NAME} ${ARG_DEPS})
endif()
if(ARG_NONLINK_DEPS)
add_dependencies(${LIB_NAME} ${ARG_NONLINK_DEPS})
endif()
Expand Down Expand Up @@ -832,6 +836,35 @@ function(yb_add_lto_target exe_name)
if("${YB_DYNAMICALLY_LINKED_EXE_SUFFIX}" STREQUAL "")
message(FATAL_ERROR "${YB_DYNAMICALLY_LINKED_EXE_SUFFIX} is not set")
endif()
# We need to build the corresponding non-LTO executable, such as yb-master or yb-tserver, first.
# We need to build the corresponding non-LTO executable first, such as yb-master or yb-tserver.
add_dependencies("${exe_name}" "${dynamic_exe_name}")
endfunction()

# Checks for redundant compiler or linker arguments in the given variable. Removes duplicate
# arguments and stores the result back in the same variable. If the
# YB_DEBUG_DUPLICATE_COMPILER_ARGS environment variable is set to 1, prints detailed debug output.
function(yb_deduplicate_arguments args_var_name)
set(debug OFF)
if("$ENV{YB_DEBUG_DUPLICATE_COMPILER_ARGS}" STREQUAL "1")
set(debug ON)
endif()
separate_arguments(args_list UNIX_COMMAND "${${args_var_name}}")
if(debug)
message("Deduplicating ${args_var_name}:")
endif()
set(deduplicated_args "")
foreach(arg IN LISTS args_list)
if(arg IN_LIST deduplicated_args)
if(debug)
message(" DUPLICATE argument : ${arg}")
endif()
else()
if(debug)
message(" Non-duplicate argument : ${arg}")
endif()
list(APPEND deduplicated_args "${arg}")
endif()
endforeach()
list(JOIN "${deduplicated_args}" " " joined_deduplicated_args)
set("${args_var_name}" PARENT_SCOPE "${joined_deduplicated_args}")
endfunction()
5 changes: 3 additions & 2 deletions cmake_modules/YugabyteTesting.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -201,8 +201,9 @@ function(ADD_YB_TEST_LIBRARY LIB_NAME)
set_target_properties(${LIB_NAME}
PROPERTIES COMPILE_FLAGS ${ARG_COMPILE_FLAGS})
endif()
target_link_libraries(${LIB_NAME} ${ARG_DEPS})
yb_remember_dependency(${LIB_NAME} ${ARG_DEPS})
if(ARG_DEPS)
target_link_libraries(${LIB_NAME} ${ARG_DEPS})
endif()
if(ARG_NONLINK_DEPS)
add_dependencies(${LIB_NAME} ${ARG_NONLINK_DEPS})
endif()
Expand Down
8 changes: 8 additions & 0 deletions python/python_vscode.env
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# See https://code.visualstudio.com/docs/python/environments#_environment-variable-definitions-file
# for details on how this file is used

# Refer to this file as follows in your VSCode user settings or workspace settings:
#
# "python.envFile:": "${workspaceFolder}/python/python_vscode.env"

PYTHONPATH=python
43 changes: 43 additions & 0 deletions python/yb/cmake_cache.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# Copyright (c) Yugabyte, Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
# in compliance with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software distributed under the License
# is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
# or implied. See the License for the specific language governing permissions and limitations
# under the License.


import re
from typing import Dict, Tuple, Any


CMAKE_CACHE_VAR_RE = re.compile(r'^([a-zA-Z_0-9-]+):([A-Z]+)=(.*)$')


class CMakeCache:
name_to_val_and_type: Dict[str, Tuple[str, str]] = {}

def __init__(self, path: str) -> None:
self.name_to_val_and_type = {}
with open(path) as input_file:
for line in input_file:
line = line.strip()
if line.startswith(('#', '//')) or not line:
continue
m = CMAKE_CACHE_VAR_RE.match(line)
if not m:
raise ValueError(f'Invalid line in CMake Cache at {path}: {line}')
self.name_to_val_and_type[m.group(1)] = (m.group(3), m.group(2))

def get(self, key: str, default_value: Any = None) -> Any:
if key not in self.name_to_val_and_type:
return default_value
value, type_name = self.name_to_val_and_type[key]
if type_name == 'BOOL':
assert value in ['ON', 'OFF']
return value == 'ON'
return value
62 changes: 62 additions & 0 deletions python/yb/common_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -480,3 +480,65 @@ def append_to_list_in_dict(dest: Dict[K, List[V]], key: K, new_item: V) -> None:
dest[key].append(new_item)
else:
dest[key] = [new_item]


def optional_message_to_prefix(message: str) -> str:
message = message.strip()
if message == '':
return ''
if message.endswith((':', '.')):
return message + ' '
return message + ': '


def set_to_str_one_element_per_line(s: Set[Any]) -> str:
'''
>>> print(set_to_str_one_element_per_line({1}))
{1}
>>> print(set_to_str_one_element_per_line({1, 2}))
{
1,
2
}
>>> print(set_to_str_one_element_per_line({'a', 'b', 'c'}))
{
a,
b,
c
}
'''
if not s:
return '{}'
elements = sorted(s)
if len(elements) == 1:
return '{%s}' % str(elements[0])
return '{\n %s\n}' % (
',\n '.join(
[str(element) for element in elements]
)
)


def assert_sets_equal(a: Set[Any], b: Set[Any], message: str = '') -> None:
a = set(a)
b = set(b)
if a != b:
raise AssertionError(
f"{optional_message_to_prefix(message)}"
f"Sets are not equal: {set_to_str_one_element_per_line(a)} vs. "
f"{set_to_str_one_element_per_line(b)}. "
f"Elements only in the first set: {set_to_str_one_element_per_line(a - b)}. "
f"Elements only in the second set: {set_to_str_one_element_per_line(b - a)}.")


def assert_set_contains_all(a: Set[Any], b: Set[Any], message: str = '') -> None:
a = set(a)
b = set(b)
d = b - a
if d:
raise AssertionError(
f"{optional_message_to_prefix(message)}"
f"First set does not contain the second: {set_to_str_one_element_per_line(a)} vs. "
f"{set_to_str_one_element_per_line(b)}. "
f"Elements in the second set but not in the first: "
f"{set_to_str_one_element_per_line(d)}.")
Loading

0 comments on commit 7b0623e

Please sign in to comment.