Skip to content
This repository has been archived by the owner on Aug 30, 2022. It is now read-only.

Make Jaeger Client Cpp build and run on Windows #112

Closed
mdouaihy opened this issue Jun 2, 2018 · 3 comments · Fixed by #115
Closed

Make Jaeger Client Cpp build and run on Windows #112

mdouaihy opened this issue Jun 2, 2018 · 3 comments · Fixed by #115

Comments

@mdouaihy
Copy link
Contributor

mdouaihy commented Jun 2, 2018

Make the project available on windows.

Currently, some header files, some cmake commands and a small part of the networking code make the build fail on Windows and MSVC 2017
@mdouaihy
Copy link
Contributor Author

Hello,

I have a working build on https://github.com/mdouaihy/jaeger-client-cpp.

The main painpoints

  • Difference of Network code between Windows/Linux
  • Difference of API between Windows/Linux
  • libraries link between Windows/Linux. Jaeger now links OpenTracing statically. On windows, this leads to multiple copy of opentracing in the final binary. This leads to UB especially when using the Global Tracer, which is a static variable in OT. To handle this, I changed the link to dynamic on Windows
  • Different compilers. I used MSVC 15.7 which made way more easy the support of MSVC. Despite that, there is still some minor differences between the compilers
  • Some tests are random by nature because they start a thread then sleep then expect some action to be done.

I need your feedback on it before I send PR.

@hypnoce
Copy link

hypnoce commented Jun 29, 2018

@mdouaihy do you think you can do the PR since the opentracing-cpp version update has been merged in hunter ?

mdouaihy added a commit to mdouaihy/jaeger-client-cpp that referenced this issue Jun 30, 2018
@mdouaihy
Fix Build Under Windows ( jaegertracing#112)
dcaf74b 
* Move to OpenTracing 1.5.0 that builds under Windows
* Change link of OpenTracing to dynamic. With Static linking of OpenTracing, we end up with multi definition of OT Symbols. This is not acceptable especially for the OT Global Tracer
* Fix OS specificities
** Networking API
** Windows headers
* Fix some tests with random behavior that failed on Windows
* Fix some MSVC specific compilation
* Add appveyor build file.

Signed-off-by: Mehrez Douaihy <mehrez.douaihy@gmail.com>
@mdouaihy mdouaihy mentioned this issue Jun 30, 2018
Merged
mdouaihy added a commit to mdouaihy/jaeger-client-cpp that referenced this issue Jun 30, 2018
@mdouaihy
Fix Build Under Windows ( jaegertracing#112)
939bf8a 
* Move to OpenTracing 1.5.0 that builds under Windows
* Change link of OpenTracing to dynamic. With Static linking of OpenTracing, we end up with multi definition of OT Symbols. This is not acceptable especially for the OT Global Tracer
* Fix OS specificities
** Networking API
** Windows headers
* Fix some tests with random behavior that failed on Windows
* Fix some MSVC specific compilation
* Add appveyor build file.

Signed-off-by: FR-MUREX-COM\mchaikhadouaihy <mehrez.douaihy@gmail.com>
mdouaihy added a commit to mdouaihy/jaeger-client-cpp that referenced this issue Jul 1, 2018
@mdouaihy
Fix Build Under Windows ( jaegertracing#112)
1c7a31e 
* Move to OpenTracing 1.5.0 that builds under Windows
* Change link of OpenTracing to dynamic. With Static linking of OpenTracing, we end up with multi definition of OT Symbols. This is not acceptable especially for the OT Global Tracer
* Fix OS specificities
** Networking API
** Windows headers
* Fix some tests with random behavior that failed on Windows
* Fix some MSVC specific compilation
* Add appveyor build file.

Signed-off-by: FR-MUREX-COM\mchaikhadouaihy <mehrez.douaihy@gmail.com>
mdouaihy added a commit to mdouaihy/jaeger-client-cpp that referenced this issue Jul 1, 2018
@mdouaihy
Fix Build Under Windows ( jaegertracing#112)
9091fc4 
* Move to OpenTracing 1.5.0 that builds under Windows
* Change link of OpenTracing to dynamic. With Static linking of OpenTracing, we end up with multi definition of OT Symbols. This is not acceptable especially for the OT Global Tracer
* Fix OS specificities
** Networking API
** Windows headers
* Fix some tests with random behavior that failed on Windows
* Fix some MSVC specific compilation
* Add appveyor build file.

Signed-off-by: FR-MUREX-COM\mchaikhadouaihy <mehrez.douaihy@gmail.com>
mdouaihy added a commit to mdouaihy/jaeger-client-cpp that referenced this issue Sep 3, 2018
@mdouaihy
Fix Build Under Windows ( jaegertracing#112)
628f504 
* Move to OpenTracing 1.5.0 that builds under Windows
* Change link of OpenTracing to dynamic. With Static linking of OpenTracing, we end up with multi definition of OT Symbols. This is not acceptable especially for the OT Global Tracer
* Fix OS specificities
** Networking API
** Windows headers
* Fix some tests with random behavior that failed on Windows
* Fix some MSVC specific compilation
* Add appveyor build file.

Signed-off-by: FR-MUREX-COM\mchaikhadouaihy <mehrez.douaihy@gmail.com>
mdouaihy added a commit to mdouaihy/jaeger-client-cpp that referenced this issue Sep 3, 2018
@mdouaihy
Fix Build Under Windows ( jaegertracing#112)
2dad6f7 
* Move to OpenTracing 1.5.0 that builds under Windows
* Change link of OpenTracing to dynamic. With Static linking of OpenTracing, we end up with multi definition of OT Symbols. This is not acceptable especially for the OT Global Tracer
* Fix OS specificities
** Networking API
** Windows headers
* Fix some tests with random behavior that failed on Windows
* Fix some MSVC specific compilation
* Add appveyor build file.

Signed-off-by: FR-MUREX-COM\mchaikhadouaihy <mehrez.douaihy@gmail.com>
@david-antiteum
Copy link

@mdouaihy any news about this development?

mdouaihy added a commit to mdouaihy/jaeger-client-cpp that referenced this issue Jul 9, 2019
@mdouaihy
Fix Build Under Windows ( jaegertracing#112)
7808d18 
* Move to OpenTracing 1.5.0 that builds under Windows
* Change link of OpenTracing to dynamic. With Static linking of OpenTracing, we end up with multi definition of OT Symbols. This is not acceptable especially for the OT Global Tracer
* Fix OS specificities
** Networking API
** Windows headers
* Fix some tests with random behavior that failed on Windows
* Fix some MSVC specific compilation
* Add appveyor build file.
* Update Hunter version.

Signed-off-by: FR-MUREX-COM\mchaikhadouaihy <mehrez.douaihy@gmail.com>
mdouaihy added a commit to mdouaihy/jaeger-client-cpp that referenced this issue Aug 31, 2019
@mdouaihy
Fix Build Under Windows ( jaegertracing#112)
58c0216 
* Move to OpenTracing 1.5.0 that builds under Windows
* Change link of OpenTracing to dynamic. With Static linking of OpenTracing, we end up with multi definition of OT Symbols. This is not acceptable especially for the OT Global Tracer
* Fix OS specificities
** Networking API
** Windows headers
* Fix some tests with random behavior that failed on Windows
* Fix some MSVC specific compilation
* Add appveyor build file.
* Update Hunter version.

Signed-off-by: FR-MUREX-COM\mchaikhadouaihy <mehrez.douaihy@gmail.com>
yurishkuro pushed a commit that referenced this issue Sep 4, 2019
@mdouaihy @yurishkuro
Upgrade to OpenTracing 1.5.0; Support build on Windows (#115)
fec4327 
* Fix Build Under Windows ( #112)

* Move to OpenTracing 1.5.0 that builds under Windows
* Change link of OpenTracing to dynamic. With Static linking of OpenTracing, we end up with multi definition of OT Symbols. This is not acceptable especially for the OT Global Tracer
* Fix OS specificities
** Networking API
** Windows headers
* Fix some tests with random behavior that failed on Windows
* Fix some MSVC specific compilation
* Add appveyor build file.
* Update Hunter version.

Signed-off-by: FR-MUREX-COM\mchaikhadouaihy <mehrez.douaihy@gmail.com>

* Enable compilation against Windows 8.1 SDK

The definition of `inet_ntop` for older Windows SDK versions, such as
8.1, does not declare the `pAddr` argument as `const`. Because of this
we need to cast away the constness first.

Despite the argument not being declared as const the function should not
be touching it, hence this should be safe.

Signed-off-by: Fredrik Appelros <fredrik.appelros@hypergene.se>

* Fix nlohmann_json dependency

The `package_deps` list is used to iterate over all dependencies in the
package config file and run `find_package` on each of them. This should
be using the package name (`nlohmann_json`) and not the target name
(`nlohmann_json::nlohmann_json`).

Signed-off-by: Fredrik Appelros <fredrik.appelros@hypergene.se>

* Avoid redefinition of NOMINMAX

Only define `NOMINMAX` if it has not already been defined to avoid
redefinition warnings.

Signed-off-by: Fredrik Appelros <fredrik.appelros@hypergene.se>

* Add Appveyor badge and debug builds.

Signed-off-by: FR-MUREX-COM\mchaikhadouaihy <mehrez.douaihy@gmail.com>

* Add TODO with the related issues for Tracer.testTracer test.

Signed-off-by: FR-MUREX-COM\mchaikhadouaihy <mehrez.douaihy@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Resolve #112 Build Under Windows mdouaihy/jaeger-client-cpp
3 participants
@hypnoce @mdouaihy @david-antiteum