Build env setup scripts + initial skeleton of cross-plat scripts for clang-format by maxgolov · Pull Request #11 · open-telemetry/opentelemetry-cpp

maxgolov · 2019-11-04T20:37:07Z

This is a humble attempt to setup some skeleton of a build environment for Windows, Linux and Mac.

Proposed layout:

tools - contains various scripts for setting up the basic build tools

Usage instructions for Windows cmd.exe shell:

call tools\setup-devenv.cmd

Usage instructions for Linux and Mac Bourne shell:

. tools/setup-devenv.sh

In both cases either sudo or elevation is required to install the tools on machine. Unix shell script is trying to cover RedHat, Debian-based distros and Mac, with an assumption that cmake, gcc and/or clang, alongside with clang-format tooling are the common core required dependencies. We can discuss what specific compiler version(s) we require or support in a subsequent commit..

But the first actual functional piece of that setup is a proposal to converge on clang-format and a corresponding .clang-format style in a platform-agnostic way.

Code formatter instructions provided in docs/using-clang-format.md , covering some common "zero installation" options that can be used from various common IDEs, i.e. not requiring the command line tooling.

My personal preference is a custom modified LLVM-based in .clang-format . Please take a look at the output of that. Since we all have different backgrounds - alternatively I am open to adopt the .clang-format.chromium - Chromium style. If we agree on Chromium style, then we'd integrate the one that is .clang-format.chromium , naming that one the .clang-format file in the top folder of the source tree.

reyang · 2019-11-04T21:26:51Z

+apt-get install -qq automake
+apt-get install -qq libtool-bin
+apt-get install -qq cmake
+apt-get install -qq sqlite


Do we need SQLite?

Not at this point.. I'll comment that. We might still need some abstraction layer for the storage. sqlite being a pure C lib has the property of universally available across all OS (windows-10+/downlevel, win10-uwp, Mac, iOS, iPad OS, Linux, Android, Chromium/Chrome OS, etc.).

reyang · 2019-11-04T23:11:20Z

+BraceWrapping:
+#  TODO(lujc) wait for clang-format-9 support in Chromium tools
+#  AfterCaseLabel: true
+  AfterClass: true


Suggested change

AfterClass: true

AfterClass: true

This one was a verbatim copy of the file from Chromium tree.

I kept it as-is in Chromium mainline, renamed this to .clang-format

reyang · 2019-11-04T23:23:18Z

+*.props                 -text diff -merge
+*.pbxproj               -text diff -merge
+
+## Resource files and UI design descriptions


Discussed during the SIG meeting, let's start with a minimum set

g-easy · 2019-11-05T00:55:45Z

+*.bash                  text eol=lf
+
+## Windows batch and PowerShell scripts
+*.bat                   text eol=crlf


Per the meeting and Reiley's comment, I think we should start from an empty file and add things as we find we need them. The EOL stuff looks very useful, but I think git already does a good job of guessing e.g. text vs binary.

Agreed to reduce the scope of it. Unfortunately GitHub doesn't always guess it properly.

I reduced the .gitattributes to very small set, pretty much core essential set.
It's still there for enforcing different lf vs crlf settings for different file types.

g-easy · 2019-11-05T00:56:25Z

+##### Source code #####
+
+## C++ and C source files
+*.c                     text diff=cpp


What does diff=cpp do? e.g. Which tools benefit from this?

diff=cpp allows GitHub to visualize a diff in a 'smarter' way - using language-specific diff parser. From gitattributes man: https://git-scm.com/docs/gitattributes

cpp suitable for source code in the C and C++ languages.

i.e. git diff --word-diff is easier to read with that attribute. There are several projects doing it that way. I'm not sure if it's still relevant as of 2019? But this is one example with some logical reasoning behind this practice: http://lkml.iu.edu/hypermail/linux/kernel/1108.3/01118.html

That's cool! Is there a before/after example? I'd be surprised if GitHub has special handling for diff=cpp but doesn't know to apply it by default to *.cc files (considering they e.g. guess the breakdown of languages for every repo)

Basically git itself in its own .gitattributes file prefers to explicitly specify the proper diff viewer for .c and .h files... I think we might want to keep that similar logic in our repo. Reference:
https://github.com/git/git/blob/master/.gitattributes

Maybe adding whitespace=space is also something we may want to consider, e.g.
*.[ch] whitespace=indent,trail,space diff=cpp
to enforce spaces instead of tabs.

g-easy · 2019-11-05T00:58:14Z

-*.lo
-*.o
-*.obj
+# User-specific files


I think this file should also start from empty (not even the github default) and we should add things as we find we need them.

I don't think that "it's safe to git add everything" should be a goal. Rather, we should gitignore stuff if we find it's annoying for a large set of users, e.g. if msbuild generates lots of files in the source dirs.

I reduced it going with .gitignore default from here:
https://github.com/github/gitignore/blob/master/C%2B%2B.gitignore

g-easy · 2019-11-05T01:00:16Z

@@ -0,0 +1,65 @@
+#!/bin/sh


This script does a lot. I would prefer a README, rather than running this on my workstation.

I'll make it optional. It bootstraps a lot of common C++ goodies, so it's easier to write a docker file using that script.

...although I think for Linux containers - it'd be better to install a lot of these as a docker build image dependencies, so that the build script doesn't have to install again. Perhaps we should have a separate commit with docker configs for Windows and several common Linux distros. These scripts are useful for folks not using docker, i.e. you can schedule these to run in a GitHub action without needing an extra docker image build.

I agree that automating (and thereby documenting) build deps would be very useful wrt Docker.

I'm happy to keep this script here, I'm just wary of running things myself (especially under sudo) ;)

This was essentially the minimum set of libs + tools required to compile our own telemetry SDK stack, to bootstrap on any default image, such as Raspberry Pi / Raspbian, or an older Ubuntu 14.xx, 16.xx, RedHat, etc.. It did accumulate some meat over time. But in essence it's something that can be just scheduled to run inside of a GitHub Action on machine which by default may not have all the needed deps. It doesn't require docker. It runs once and creates .buildtools file, indicating that the build tools have been deployed, so it won't rerun again for incremental builds.

Alternate solution is to create a separate set of docker image configs (quite a bit of duplication). Then assuming that each image is cooked properly with the right set of tools- it'd be then as easy as invoking cmake in that image, not needing the setup of all build tools.

Most big projects in some way would still enforce their own tooling, their own compiler, etc. like chromium/tools/depot_tools and would still be much more bulky than this shell script. I can truncate that to one command running apt-get install -qq all_deps_at_once + share some docker configs for an equivalent set of tools.

I'm happy to keep the script. Let's see what others think.

g-easy · 2019-11-05T04:23:05Z

git fetch origin pull/11/head:pr11
git checkout pr11
cat /path/to/some/file.cc | clang-format-8 -style=file
mv .clang-format.chromium .clang-format
cat /path/to/some/file.cc | clang-format-8 -style=file

My preferences:

-style=Google
.clang-format.chromium
.clang-format

…try-cpp into maxgolov/clang-format

g-easy

I still think this does more than we currently need, but I'd prefer to keep things moving forward. :)

g-easy · 2019-12-09T05:59:35Z

+IndentWidth: 2
+
+# Keep lines under 100 columns long.
+ColumnLimit: 100


I prefer 80.

g-easy · 2019-12-09T06:00:13Z

+# Always break before braces
+BreakBeforeBraces: Custom
+BraceWrapping:
+#  TODO(lujc) wait for clang-format-9 support in Chromium tools


Probably don't want username here.

g-easy · 2019-12-09T06:00:19Z

+# Use 2 space negative offset for access modifiers
+AccessModifierOffset: -2
+
+# TODO(jmadill): Decide if we want this on. Doesn't have an "all or none" mode.


g-easy · 2019-12-09T06:00:40Z

+## Python scripts
+*.py                    text eol=lf diff=python
+
+## Perl scripts/libraries/modules


I don't think we're going to use any perl.

g-easy · 2019-12-09T06:02:03Z

@@ -1,3 +1,4 @@
+# Ref. https://github.com/github/gitignore/blob/master/C%2B%2B.gitignore
 # Prerequisites
 *.d


Both CMake and bazel do out-of(-source)-tree builds. We don't need to ignore deps or objs here.

maxgolov · 2019-12-16T22:18:17Z

However has the magic powers to merge, please help to merge this - I can work on incremental updates to address some nit-/minor suggestions in a follow-up PR.

rnburn · 2019-12-16T22:22:25Z

Merged

Reapply "Change OTLP HTTP content_type default to binary (open-telemetry#2558)" (open-telemetry#2…

Build env setup scripts

26c0cd6

maxgolov requested review from g-easy, jmacd, reyang, rnburn and tigrannajaryan as code owners November 4, 2019 20:37

reyang reviewed Nov 4, 2019

View reviewed changes

maxgolov changed the title ~~Build env setup scripts + initial skeleton of cross-plat scripts for clang-format~~ [WIP] Build env setup scripts + initial skeleton of cross-plat scripts for clang-format Nov 4, 2019

Remove sqlite for now

21f1fff

reyang reviewed Nov 4, 2019

View reviewed changes

Comment thread .clang-format.chromium Outdated

reyang reviewed Nov 4, 2019

View reviewed changes

g-easy reviewed Nov 5, 2019

View reviewed changes

maxgolov added 3 commits December 2, 2019 14:52

Merge branch 'master' of https://github.com/open-telemetry/openteleme…

08b17dd

…try-cpp into maxgolov/clang-format

Reduce the scope of .clang-format .gitattributes and .gitignore changes

27f4f60

Fix code comment

c94ba55

g-easy approved these changes Dec 9, 2019

View reviewed changes

jmacd approved these changes Dec 9, 2019

View reviewed changes

rnburn approved these changes Dec 9, 2019

View reviewed changes

rnburn changed the title ~~[WIP] Build env setup scripts + initial skeleton of cross-plat scripts for clang-format~~ Build env setup scripts + initial skeleton of cross-plat scripts for clang-format Dec 16, 2019

rnburn merged commit 34988af into master Dec 16, 2019

reyang deleted the maxgolov/clang-format branch July 21, 2020 19:00

ZDzhao mentioned this pull request Feb 18, 2022

crash in ext::http::client::curl::HttpOperation::Send #1220

Closed

GerHobbelt pushed a commit to GerHobbelt/opentelemetry-cpp that referenced this pull request Jun 21, 2024

Merge pull request open-telemetry#11 from open-telemetry/main

8cd8eac

Reapply "Change OTLP HTTP content_type default to binary (open-telemetry#2558)" (open-telemetry#2…

zhugul mentioned this pull request Mar 27, 2025

"malloc(): unaligned fastbin chunk detected" crash in OpenTelemetry C++ SDK 1.16 #3326

Closed

bharat2002 mentioned this pull request May 8, 2025

std::thread crashing when ending Span and or whereever new std::thread is writtern in library(.so) #3394

Open

Sowmya-Iyer mentioned this pull request Jul 21, 2025

AddCallback() for metrics (gauge) causes a pure virtual method called error #3555

Open

Uh oh!

Conversation

maxgolov commented Nov 4, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maxgolov Nov 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maxgolov Dec 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maxgolov Nov 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

top-5 Nov 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

g-easy commented Nov 5, 2019

Uh oh!

g-easy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maxgolov commented Dec 16, 2019

Uh oh!

rnburn commented Dec 16, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

maxgolov Nov 4, 2019 •

edited

Loading

maxgolov Dec 2, 2019 •

edited

Loading

maxgolov Nov 5, 2019 •

edited

Loading

top-5 Nov 5, 2019 •

edited

Loading