Cobrazz improvements #316

yoori · 2023-06-27T15:34:44Z

Improvements:

added function RunEmbedded for run components and stop outside without signal ... (author @anton3332), now stop by mutex.unlock
added ability to build shared libraries variant (for use in rpm build)
ability to build with gdb debug info (USERVER_GEN_GDB_DEBUGINFO - disabled by default) - this allow to see more informative stacks on linux with using debugsource package.
added cmake install of project (for use in rpm build or conan build)
added environment for build rpm on epel8 linux's (script, scripts for build dep fresh packages and spec file)

Fixes:

fix of building inside folder sym linked to other place - in this case "protoc" require that include base is prefix of proto file,
for fix - we resolve paths to real paths before protoc execute

Specific changes:

we decreased protobuf, grpcio python package dependency versions (requirements*.txt) for use standard python packages installed through rpm's.
Otherwise we have strange requirement: should be installed latest version of these packages by pip, independent on versions of c++ packages

All these changes we tested : we link userver preinstalled on host as rpm package, and runned http, grpc components in production

Anton3 · 2023-06-30T14:51:26Z

core/CMakeLists.txt

+  PATTERN "*.h"
+)
+
+install(FILES "${CMAKE_CURRENT_SOURCE_DIR}/src/utils/signal_catcher.hpp"


copy-pasta: we shouldn't need to edit CMake scripts when adding new source files

This part install private file that we need to include after installation (from src/ folder, userver_add_library install only include folder)

Stuff in src/ and impl is private API. We provide no support for code that uses such workarounds to access it anyway. If you still need it, move it to public API (include). Please expand on why you need it, we'll figure out the best way to expose the necessary tools.

Anton3 · 2023-06-30T14:51:52Z

core/src/components/run_embedded.cpp

+  }
+};
+
+void DoRun(std::mutex& stop_mutex,


Why copy-paste components::Run? Please expand on why stop_mutex is needed

Run can be stopped only from components (usually by signal handling), we need to have ability stop it from code (here mutex unlock initiate components destruction and stopping as result)

std::raise(SIGINT)?

Anton3 · 2023-06-30T14:52:40Z

core/src/utils/userver_experiment.cpp

@@ -0,0 +1,54 @@
+#include <utils/userver_experiment.hpp>


An extra file as a rebase artifact?

Anton3 · 2023-06-30T14:52:54Z

core/src/utils/userver_experiment.hpp

@@ -0,0 +1,26 @@
+#pragma once


An extra file as a rebase artifact?

Anton3 · 2023-06-30T14:53:33Z

grpc/CMakeLists.txt

+
+userver_export(TARGETS ${PROJECT_NAME}-deps)
+
+install(FILES "${CMAKE_CURRENT_SOURCE_DIR}/src/ugrpc/client/impl/client_configs.hpp"


Anton3 · 2023-06-30T14:55:36Z

scripts/grpc/requirements.txt

@@ -1 +1 @@
-protobuf>=4.21.12
+protobuf>=3.21.9


Changing these versions will break gRPC tests for someone again :)
We've only just found a good combination that satisfies most users

Protobuf has broken backwards compatibility around version 3.21 (which is, confusingly, also called 4.21): old Python packages must be used with older system protoc, and new Python packages must be used with newer system protoc versions.

We now support two build modes for gRPC. If we detect an old enough system protobuf, we use scripts/grpc/requirements-old.txt instead of scripts/grpc/requirements.txt. Same for testsuite/requirements-grpc.txt and testsuite/requirements-grpc-old.txt.

Anton3 · 2023-06-30T14:58:37Z

cmake/SetupEnvironment.cmake

@@ -140,4 +170,8 @@ else ()
  add_definitions(-D_FORTIFY_SOURCE=2)
 endif ()

+if (UNIX AND BUILD_SHARED_LIBS)


We do not support building userver as a dynamic library. Doing so will result in performance drops (we rely on LTO heavily) and bugs around non-constexpr inline variables and things like AnyMovable comparing std::type_info by pointer value

Anton3 · 2023-06-30T15:13:06Z

core/src/fs/temp_file.cpp

@@ -6,6 +6,16 @@ USERVER_NAMESPACE_BEGIN

 namespace fs {

+// Please, review this TempFile::TempFile implementation
+// I simple added it as workaround for linkage error (if linked as shared library)


Simple: don't use shared linkage with userver

Why ? We already debugged shared library variant installation in this request and it works, here much reasons why 99% of libraries on unix systems install in shared variant. And here we try to make this variant available for userver ….

There are multiple reasons why we would like to avoid building shared libraries:

we guarantee no ABI stability even on minor changes. This is usually a deal breaker for shared libraries distributions

supporting shared libraries would double the testing efforts, double the support efforts, double the build scripts boilerplate and double the development efforts, as it would require -fvisibility=hidden and visibility attributes in code

LTO is not quite nicely supported with shared libraries, we would not like to provoke users for inefficient services

apolukhin · 2023-07-10T07:40:52Z

cmake/GrpcTargets.cmake

@@ -147,6 +147,9 @@ function(generate_grpc_files)

    set(did_generate_proto_sources FALSE)
    if("${newest_proto_dependency}" IS_NEWER_THAN "${GENERATED_PROTO_DIR}/${path_base}.pb.cc")
+      # resolve root_path, proto_file to real path's - protoc check that root_path is prefix of proto_file (this can be non true if project inside folder sym linked to other dir)
+      get_filename_component(real_root_path ${root_path} REALPATH)


A few lines above we already do this, so the root_path should be already a real path. Are we missing something?

github-actions · 2024-01-18T15:55:03Z

CLA Assistant Lite bot:
Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/

1 out of 2 committers have signed the CLA.
✅ @yoori
❌ @arbogdanov
_{You can retrigger this bot by commenting recheck in this Pull Request}

Cobrazz improvements

a4228b0

yoori requested review from itrofimow and segoon as code owners June 27, 2023 15:34

Anton3 requested changes Jun 30, 2023

View reviewed changes

Anton3 reviewed Jun 30, 2023

View reviewed changes

apolukhin added a commit that referenced this pull request Jul 1, 2023

adopt changes from #316

2883a3b

yoori added 5 commits July 4, 2023 12:49

adapt rpm spec for build static libraries

b39fa25

adapt rpm spec for build static libraries

a1f5b25

adapt rpm spec for build static libraries

c3d3308

adapt rpm spec for build static libraries

2b5ae61

adapt rpm spec for build static libraries

b08f4ff

apolukhin requested changes Jul 10, 2023

View reviewed changes

Http integration with cobrazz

24f3ed6

yoori added 2 commits January 22, 2024 14:02

added install of required files

9d0fa86

fix double include error for cmake

220cf6a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cobrazz improvements #316

Cobrazz improvements #316

yoori commented Jun 27, 2023

Anton3 Jun 30, 2023

yoori Jul 1, 2023

Anton3 Jul 3, 2023

Anton3 Jun 30, 2023 •

edited

yoori Jul 1, 2023

Anton3 Jul 3, 2023

Anton3 Jun 30, 2023

Anton3 Jun 30, 2023

Anton3 Jun 30, 2023

Anton3 Jun 30, 2023 •

edited

Anton3 Jun 30, 2023

Anton3 Jun 30, 2023

yoori Jul 1, 2023

apolukhin Jul 2, 2023

apolukhin Jul 10, 2023

github-actions bot commented Jan 18, 2024


		userver_export(TARGETS ${PROJECT_NAME}-deps)

		install(FILES "${CMAKE_CURRENT_SOURCE_DIR}/src/ugrpc/client/impl/client_configs.hpp"

		@@ -1 +1 @@
		protobuf>=4.21.12
		protobuf>=3.21.9

Cobrazz improvements #316

Are you sure you want to change the base?

Cobrazz improvements #316

Conversation

yoori commented Jun 27, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Anton3 Jun 30, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Anton3 Jun 30, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 18, 2024

Anton3 Jun 30, 2023 •

edited

Anton3 Jun 30, 2023 •

edited