Correct stdc++ language and size issues for MSVC #8572

achasveachas · 2019-10-10T19:20:53Z

Description:
This patch makes consistent the type for ssize_t as used on Windows.
We will favor describing it as a ptrdiff_t, which is standard C.
Patches to tclap and nghttp2 are needed for this exercise.

Linux is a 64ILP architecture, so there's no difference between int
and [s]size_t and pointer widths. Windows is a 64P architecture, where
only long long maps to the width of the size_t and C99 standard ptrdiff_t
types.

This leads to a number of places where the Envoy API has presented
an explicit int or int32_t which is smaller than the ptrdiff_t on
Windows. In some places we must cast to or from the API defined width.
Several other misassumptions on the width of int are also addressed.

Windows does not support alternately-named VA_ARGS which is not a
standard C++ language feature in our baseline c++ 17 expectation. Other
minor adjustments reflect other quirks of the MSVC compilier (some more
correct and some simply buggy behavior).

Windows MSVC needs various guards on unrecognized #pragmas, and will not
support any preprocessor operations within the arguments to a macro;
this is not stdc or stdc++.

This patch largely ignores changes which are required for windows, and
do not impact the linux/os-x compilation path. This patch is also
missing the IoHandle abstraction of 'fd' arguments, storage and return
codes, win32 error handling and win32 specific #include's handling.

Signed-off-by: William A Rowe Jr [email protected]
Signed-off-by: Yechiel Kalmenson [email protected]

Risk Level: Moderate
Testing: Passed locally on Windows (with additional patches) and Linux
Docs Changes: None
Release Notes: None

This patch makes consistent the type for ssize_t as used on Windows. We will favor describing it as a ptrdiff_t, which is standard C. Patches to tclap and nghttp2 are needed for this exercise. Linux is a 64ILP architecture, so there's no difference between int and [s]size_t and pointer widths. Windows is a 64P architecture, where only long long maps to the width of the size_t and C99 standard ptrdiff_t types. This leads to a number of places where the Envoy API has presented an explicit int or int32_t which is smaller than the ptrdiff_t on Windows. In some places we must cast to or from the API defined width. Several other misassumptions on the width of int are also addressed. Windows does not support alternately-named __VA_ARGS__ which is not a standard C++ language feature in our baseline c++ 17 expectation. Other minor adjustments reflect other quirks of the MSVC compilier (some more correct and some simply buggy behavior). Windows MSVC needs various guards on unrecognized #pragmas, and will not support any preprocessor operations within the arguments to a macro; this is not stdc or stdc++. This patch largely ignores changes which are required for windows, and do not impact the linux/os-x compilation path. This patch is also missing the IoHandle abstraction of 'fd' arguments, storage and return codes, win32 error handling and win32 specific #include's handling. Signed-off-by: William A Rowe Jr <[email protected]> Signed-off-by: Yechiel Kalmenson <[email protected]>

lizan · 2019-10-10T20:01:53Z

source/common/common/version.cc

 #ifdef NDEBUG
-                                                  "RELEASE",
+  const std::string release_type = "RELEASE";


why this change? make them string_view.

This is another example where the macro CONSTRUCT_ON_FIRST_USE cannot have nested #cpp statements such as #ifdef. We simply followed the file's current convention, if a rewrite is in order, doesn't it make sense to convert this all to absl::string_view? Is it possible to accept this patch as-is, to accomplish the stated purpose of this PR (to get it compiling without gcc'isms)?

What's the gcc'ism in this function before your change? You should be able to make L23-L30 to static const absl::string_view, basically no string allocation other than the last statement. Because these are no longer in CONSTRUCT_ON_FIRST_USE, this means every version() call will result std::string allocation, which is hard no.

@lizan we updated the file, is this what you had in mind?

lizan · 2019-10-10T20:02:32Z

source/common/router/header_parser.h

@@ -1,3 +1,11 @@
+// grpcpp/grpcpp.h includes <windows.h>, so undef some interfering symbols


?? this seems a wrong place.

Doesn't it? Unfortunately grpc is a mess when it comes to injecting windows.h symbols, so this is where this one must be accomodated at the moment. Comments on platform.h elsewhere.

I mean this could never go before #pragma once, and grpcpp is not included in this file either.

Agreed. Tested and removing.

wrowe · 2019-10-10T23:00:49Z

test/common/stream_info/test_util.h

@@ -16,7 +16,11 @@ class TestStreamInfo : public StreamInfo::StreamInfo {
    memset(&fake_time, 0, sizeof(fake_time));
    fake_time.tm_year = 99; // tm < 1901-12-13 20:45:52 is not valid on macOS
    fake_time.tm_mday = 1;
-    start_time_ = std::chrono::system_clock::from_time_t(timegm(&fake_time));
+#if !defined(WIN32)
+    start_time_ = std::chrono::system_clock::from_time_t(::timegm(&fake_time));


This is a particular problem... There are likely more portable solutions which don't violate the check_validate logic that if failing on the presence of a timegm() call. We simply used the existing logic and would beg another eager volunteer to take on that particular refactoring using stdc++ or absl api, no win32-internals knowledge required.

@jmarantz I think we've been discussing use absl::Time and move away from chrono, will that ultimately solve this issue as well?

I would be in favor of moving to absl::Time. However it's a ton of work. I count 1716 std::chrono refs in the codebase.

We'd probably have to do it in a few refactors.

Can't we sed this migration? Maybe with some wrappers?

Is this item worth it's own issue, maybe? I'd beg for this bit to be accepted in the short-term.

Yes, it's definitely a distinct issue. Can you open a GH issue and tag this as TODO?

#8717 re-kicks the earlier ticket for this issue.
I'll mark this thread resolved for purpose of this patch.

htuch

Thanks, exciting to see the Windows port progressing.

htuch · 2019-10-11T04:13:02Z

bazel/tclap-win64-ull-sizet.patch

@@ -0,0 +1,16 @@
+diff --git a/include/tclap/StandardTraits.h b/include/tclap/StandardTraits.h


Can you fix this one upstream?

We will submit it, but it is needed locally short term.

It seems small enough that we could kick this into the pipeline in parallel. We really dislike patches, they are only a solution when all else fails when working with an upstream project.

If and when we ever see a tclap 1.4 release, this patch and the defect go away, since the entire issue was refactored away 6 years ago;
https://sourceforge.net/p/tclap/code/ci/5d4ffbf2db794af799b8c5727fb6c65c079195ac/
Please consider this patch acceptable short-term (or permanently given the stable/no-releases state) for this component and we'll treat this as resolved for modern windows 64 compilers.

Ack, please leave a reference to this thread in the patches arg of the _repository_impl .

Added short note + refactor link

htuch · 2019-10-11T04:13:35Z

bazel/foreign_cc/nghttp2.patch

@@ -8,7 +8,7 @@ index 35c77d1d..47bd63f5 100644
   # Set it to "int" to match the behavior of AC_TYPE_SSIZE_T (autotools).
 -  set(ssize_t int)
 +  if(WIN32 AND CMAKE_SIZEOF_VOID_P EQUAL 8)
-+    set(ssize_t int64_t)
+    set(ssize_t ptrdiff_t)


Can you upstream this? Patches aren't sustainable for us long term.

This will not be accepted upstream until a version major bump (note it is our patch - of envoy's present patch.) PR and discussion item here; nghttp2/nghttp2#1395

Ack, please leave a reference to this thread in the patches arg of the _repository_impl .

Added footnote of ABI compat/next major release, discussion pointer to nghttp2/nghttp2#1395

htuch · 2019-10-11T04:14:48Z

source/common/protobuf/protobuf.h

+#undef TRUE
+#undef DELETE
+#undef ERROR
+#undef GetMessage


This seems pretty fragile, is there some best practice for importing Protobuf headers on Windows or other projects we can crib from?

I'll attach the explanation top-post in the review comments, but I agree we should consume our own platform.h here and delegate inclusion of windows.h at that inflection point, trashing MS macros at the time we include windows.h. Individual modules should never be injecting MS SDK API headers anywhere without coordination.

Thanks, I like this solution.

htuch · 2019-10-11T04:15:45Z

source/common/tracing/http_tracer_impl.h

+// <windows.h> defines macros for ERROR and TRUE, so we need to undef them in
+// case it's been included above
+#undef ERROR
+#undef TRUE


Yeah, definitely not a fan of so many seemingly random places that we need to do these #undef. Would be very keen to see some way we can hide this and bound the number.

My project of Monday is to localize the inclusion of all networking/variant headers into platform.h, and perform the adjustment there. The problem will become to get the code style review to accept that platform.h occurs nearly first, or before any other dependency which tries to inject windows.h

Fantastic. In this case, I would suggest you formalize this rule in tools/header_order.py, so that we have a machine enforcement of this logic.

I think the only remaining threads are the WIN32 that are left in rando files.

Please review our submission of yesterday, localizing the decisions on posix / windows API headers and fix-ups of these to the include/envoy/common/platform.h file.

Expanding on that idea, it would also be possible to resolve the problems with Linux <sys/types.h> and #undef the 'major', 'minor' defines which cause significant noise at certain points in the build. It wouldn't be unreasonable to move this (basically portable) inclusion into platform.h as well.

source/extensions/filters/network/http_connection_manager/config.cc

test/common/http/http2/codec_impl_test.cc

wrowe · 2019-10-11T16:52:01Z

As a top-level comment, feel free to push back any uio.h vs. winsock2.h (and undefines.) The next submission this coming week makes a lot of changes, to localize all of those inclusions in platform.h as our single point of contact with the platform-specific networking API.

wrowe · 2019-10-18T22:23:40Z

Simply to unstale, the two action items I'm pursuing are the request to string_view as identified by lizan, and either retract or advance the use of platform.h for <windows.h> (winsock2 etc) inclusion, and finally open a ticket to refactor to absl time classes that wouldn't fall under this PR. If I have missed anything else that prevents this patch from being pushed, please identify such.

Signed-off-by: Garden Windows <[email protected]>

Signed-off-by: Yechiel Kalmenson <[email protected]> Signed-off-by: William A Rowe Jr <[email protected]>

Signed-off-by: William A Rowe Jr <[email protected]> Signed-off-by: Yechiel Kalmenson <[email protected]>

Signed-off-by: Yechiel Kalmenson <[email protected]> Signed-off-by: William A Rowe Jr <[email protected]>

htuch

Looks much better, thanks for your patience working through the gaps here.
/wait

htuch · 2019-10-24T08:25:35Z

bazel/foreign_cc/nghttp2.patch

@@ -8,7 +8,7 @@ index 35c77d1d..47bd63f5 100644
   # Set it to "int" to match the behavior of AC_TYPE_SSIZE_T (autotools).
 -  set(ssize_t int)
 +  if(WIN32 AND CMAKE_SIZEOF_VOID_P EQUAL 8)
-+    set(ssize_t int64_t)
+    set(ssize_t ptrdiff_t)


Ack, please leave a reference to this thread in the patches arg of the _repository_impl .

htuch · 2019-10-24T08:26:09Z

bazel/tclap-win64-ull-sizet.patch

@@ -0,0 +1,16 @@
+diff --git a/include/tclap/StandardTraits.h b/include/tclap/StandardTraits.h


Ack, please leave a reference to this thread in the patches arg of the _repository_impl .

htuch · 2019-10-24T08:27:14Z

source/common/router/retry_state_impl.cc

@@ -92,7 +92,7 @@ RetryStateImpl::RetryStateImpl(const RetryPolicy& route_policy, Http::HeaderMap&

  if (!retriable_request_headers_.empty()) {
    // If this route limits retries by request headers, make sure there is a match.
-    bool request_header_match = false;
+    uint32_t request_header_match = false;


This is used as a bool below, why an integer?

It isn't used as a bool. It's used for boolean math against a uint32_t.

Can we change the request_header_match = true to request_header_match = 1? I think this is clearer if we switch the type here.

Switched these two constants at 95 and 98.

source/extensions/filters/network/http_connection_manager/config.cc

test/common/http/http2/codec_impl_test.cc

htuch · 2019-10-24T08:35:46Z

test/integration/echo_integration_test.cc

@@ -49,15 +49,16 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, EchoIntegrationTest,
 TEST_P(EchoIntegrationTest, Hello) {
  Buffer::OwnedImpl buffer("hello");
  std::string response;
-  RawConnectionDriver connection(
+  std::unique_ptr<RawConnectionDriver> connection;


Can you elaborate on why this change was needed?

I have literally no clue, it was before my time, and not suggesting this is the single best workaround. Reverting the patch on msvc we end up with;

ERROR: C:/workspace/envoy/test/integration/BUILD:565:1: C++ compilation of rule '//test/integration:echo_integration_test_lib_internal_only' failed (Exit 2) test/integration/echo_integration_test.cc(56): error C2065: 'connection': undeclared identifier test/integration/echo_integration_test.cc(106): error C2065: 'connection': undeclared identifier Target //test/integration:echo_integration_test failed to build

We are happy to withdraw changes to this specific file for separate consideration.

We see an additional problem on Windows with this test. We will bypass it short term and are withdrawing the changes to echo_integration_test until they are better understood by all.

htuch · 2019-10-24T08:36:22Z

source/common/tracing/http_tracer_impl.h

+// <windows.h> defines macros for ERROR and TRUE, so we need to undef them in
+// case it's been included above
+#undef ERROR
+#undef TRUE


I think the only remaining threads are the WIN32 that are left in rando files.

…ilties

…/envoy into fix-msvc-incompatibilties

The purpose of platform.h is to localize the inclusion of POSIX-specific, or Windows-specific common #include's from the C api. This is included when such networking or other constructs are needed -or- when 3rd party dependencies are including these definitions. This patch begins removing posix-specific headers into envoy/common/platform.h, which becomes the only origin of including windows.h API header files in the envoy build schema. This allows us to fix up issues where the Windows API is not a good C++ citizen (`#define interface` for many common keywords, etc.) Signed-off-by: Yechiel Kalmenson <[email protected]> Signed-off-by: William A Rowe Jr <[email protected]>

Signed-off-by: William A Rowe Jr <[email protected]> Signed-off-by: Yechiel Kalmenson <[email protected]>

Introduced a comment to allow windows/winsock to occur prior to mswsock (in spite of alpha ordering) Still, the pre-existing gmtime alert persists until this ticket is addressed in some future PR; #8717 Signed-off-by: Yechiel Kalmenson <[email protected]> Signed-off-by: William A Rowe Jr <[email protected]> Signed-off-by: Yechiel Kalmenson <[email protected]>

Added comments for tclap and nghttp2 patches which are less likely to be seen at this project in the near-term. (Feel free to simplify.) Withdrew changes to echo_integration_test until we better understand the appropriate fix. The remaining PR is not blocked on this change. https://github.com/envoyproxy/envoy/pull/8572/files/81724788290b79d88a01965c39f69f9ea8daf47e#r338447233 Signed-off-by: William A Rowe Jr <[email protected]> Signed-off-by: Yechiel Kalmenson <[email protected]> Signed-off-by: William A Rowe Jr <[email protected]>

achasveachas · 2019-10-30T15:13:25Z

Right now the only formatting failures are the ones associated with the gmtime fix, would you rather we pull that out until that refactor (#8717 ) is addressed so the Linux CI can run properly?

htuch

Awesome, I like the new platform.h construct. A few more comments and we can ship. Thanks.
/wait

htuch · 2019-10-31T20:51:08Z

source/common/common/fmt.h

@@ -1,5 +1,7 @@
 #pragma once

+#include "envoy/common/platform.h" // Avert format.h including windows.h


Can we fix fmtlib upstream?

As a practical matter? Any of our 3 dozen dependencies could introduce an #include <window.h> and disturb our source compilation, that header is not meant for general C++ language consumption. By this, we mean that MS heavily leverages #define and C++ heavily leverages multiple legible tokens. The logic in C language might introduce add_interface() but in C++ we are going to introduce interface.add(), which collides with windows.h defines.

We've introduced platform.h to work around this every time we encounter the issue. And if it is fixed upstream, then we could drop our #include, but it isn't critical. The answer at envoy is to eventually stop using Posix or Windows types in our own .h interface definitions, and localize the Posix and Windows implementations to our .cc sources (which will always include platform.h.) Exceptions would include our .h interface stubs to third party libraries such as grpc, nghttp2, etc.

I don't believe an upstream fix is really a priority, since I'm reading on #envoy-dev that fmtlib is being phased out for absl? It certainly shouldn't block this patch.

I feel that any 1st class support for Windows in Envoy doesn't end at Envoy. With my maintainer hat on I'm labeling this technical debt.. It's just a single line, but one day someone will come along and spend a day or so chasing down some obscure issue that is masked by this.

At the same time, we're pragmatic about tech debt and want to see Windows support land.

I would like to see tracking issues filed with the respective communities or in Envoy GH and referenced in these comments. Even if you folks aren't going to own this, we need to recognize these problems and provide the opportunity for other folks to do so.

Also, we will live with fmtlib for a long time. It's deprecated on the data path, but it's a key part of logging which won't be going away.

Also, we will live with fmtlib for a long time. It's deprecated on the data path, but it's a key part of logging which won't be going away.

Fair point, and researching, it turns out they have already responded.

fmtlib/fmt#171

We have the option of disabling fmt.h's use of windows.h, we just need to define FMT_USE_WINDOWS_H=0 as appropriate. This simply deprives us of the optimizations present in bazel-envoy/external/com_github_fmtlib_fmt/include/fmt/format-inl.h

It's an open question whether we want to define this symbol prior to including format.h in our
source/common/common/fmt.h header or include windows.h before they can, these have the same effect. If we want to explore this further, perhaps an issue is in order for further discussions, but we are reasonably happy with the proposed solution.

htuch · 2019-10-31T20:51:57Z

source/common/network/cidr_range.cc

@@ -1,8 +1,10 @@
 #include "common/network/cidr_range.h"

+#ifndef WIN32


Why not platform.h?

An oversight. Corrected, thanks!

htuch · 2019-10-31T20:53:36Z

source/common/router/retry_state_impl.cc

@@ -92,7 +92,7 @@ RetryStateImpl::RetryStateImpl(const RetryPolicy& route_policy, Http::HeaderMap&

  if (!retriable_request_headers_.empty()) {
    // If this route limits retries by request headers, make sure there is a match.
-    bool request_header_match = false;
+    uint32_t request_header_match = false;


Can we change the request_header_match = true to request_header_match = 1? I think this is clearer if we switch the type here.

source/extensions/filters/network/http_connection_manager/config.cc

Catch missing use of platform.h Using int constants for clarity Signed-off-by: Yechiel Kalmenson <[email protected]> Signed-off-by: William A Rowe Jr <[email protected]> Signed-off-by: Yechiel Kalmenson <[email protected]>

Signed-off-by: William A Rowe Jr <[email protected]> Signed-off-by: Yechiel Kalmenson <[email protected]> Signed-off-by: William A Rowe Jr <[email protected]>

Signed-off-by: Yechiel Kalmenson <[email protected]> Signed-off-by: William A Rowe Jr <[email protected]> Signed-off-by: Yechiel Kalmenson <[email protected]>

htuch

LGTM modulo filing requested tracking issues and updating comments. This is epic, thanks for the major cleanup!

lizan

Great, thanks! LGTM modulo a nit.

source/common/common/version.cc

wrowe · 2019-11-02T02:19:47Z

LGTM modulo filing requested tracking issues and updating comments. This is epic, thanks for the major cleanup!

Some confusion, I'm wondering this is modulo what remaining items? All tracking issues except your fmt class suggestion were filed upstream that I'm aware of, all requested comments had been added in this patch that I'm aware of.

So if there is anything to do other than pushing an upstream a suggestion to the fmt project to hide the inclusion of the windows.h header from all general consumers of their headers (a very good suggestion to push), I've missed it, other than @lizan 's final suggestion of 'static const'ness. Please point out any other remaining items, thanks!

Signed-off-by: William A Rowe Jr <[email protected]> Signed-off-by: Yechiel Kalmenson <[email protected]> Signed-off-by: William A Rowe Jr <[email protected]>

wrowe · 2019-11-04T15:11:17Z

[...] modulo filing requested tracking issues and updating comments.

We believe these are all filed, comments updated, code adjusted as requested.

htuch · 2019-11-04T16:31:44Z

Thanks @wrowe, ready to ship when CI clears.

kfaseela · 2021-11-22T14:31:56Z

There is a tclap 1.2.5 version available, will this patch cause any trouble if we try to upgrade to 1.2.5?

lizan reviewed Oct 10, 2019

View reviewed changes

achasveachas marked this pull request as ready for review October 10, 2019 21:02

achasveachas requested review from alyssawilk, htuch and mattklein123 as code owners October 10, 2019 21:02

wrowe reviewed Oct 10, 2019

View reviewed changes

htuch reviewed Oct 11, 2019

View reviewed changes

jmarantz assigned htuch Oct 11, 2019

htuch added the waiting label Oct 22, 2019

wrowe mentioned this pull request Oct 22, 2019

Refactor chrono date/time to absl:Time #8717

Closed

Garden Windows and others added 2 commits October 23, 2019 10:20

Merge branch 'master' into fix-msvc-incompatibilties

5b5312d

Signed-off-by: Garden Windows <[email protected]>

This cleanup was misplaced; relocating to platform.h

7ea3522

Signed-off-by: Yechiel Kalmenson <[email protected]> Signed-off-by: William A Rowe Jr <[email protected]>

repokitteh-read-only bot removed the waiting label Oct 23, 2019

achasveachas and others added 2 commits October 23, 2019 10:34

Note open ticket for format exception

3505e99

Signed-off-by: William A Rowe Jr <[email protected]> Signed-off-by: Yechiel Kalmenson <[email protected]>

Ensure one-time variables per lizan's review

8172478

Signed-off-by: Yechiel Kalmenson <[email protected]> Signed-off-by: William A Rowe Jr <[email protected]>

htuch suggested changes Oct 24, 2019

View reviewed changes

repokitteh-read-only bot added the waiting label Oct 24, 2019

Garden Windows and others added 4 commits October 28, 2019 11:31

Merge remote-tracking branch 'origin/master' into fix-msvc-incompatib…

d62c39b

…ilties

Merge branch 'fix-msvc-incompatibilties' of github.com:greenhouse-org…

01da2c7

…/envoy into fix-msvc-incompatibilties

Use '!' expression syntax, rather than 'not'

bbf2f05

Signed-off-by: William A Rowe Jr <[email protected]> Signed-off-by: Yechiel Kalmenson <[email protected]>

repokitteh-read-only bot removed the waiting label Oct 29, 2019

achasveachas and others added 2 commits October 29, 2019 18:16

htuch suggested changes Oct 31, 2019

View reviewed changes

repokitteh-read-only bot added the waiting label Oct 31, 2019

Minor corrections identified by htuch

5e98c83

Catch missing use of platform.h Using int constants for clarity Signed-off-by: Yechiel Kalmenson <[email protected]> Signed-off-by: William A Rowe Jr <[email protected]> Signed-off-by: Yechiel Kalmenson <[email protected]>

repokitteh-read-only bot removed the waiting label Nov 1, 2019

wrowe and others added 2 commits November 1, 2019 10:26

Withdraw patch for a later fix using absl::Time

ac0ee9e

Signed-off-by: William A Rowe Jr <[email protected]> Signed-off-by: Yechiel Kalmenson <[email protected]> Signed-off-by: William A Rowe Jr <[email protected]>

Pedantic spelling fix

94070c5

Signed-off-by: Yechiel Kalmenson <[email protected]> Signed-off-by: William A Rowe Jr <[email protected]> Signed-off-by: Yechiel Kalmenson <[email protected]>

htuch reviewed Nov 1, 2019

View reviewed changes

lizan reviewed Nov 2, 2019

View reviewed changes

source/common/common/version.cc Outdated Show resolved Hide resolved

static const constants per lizan

74cee40

Signed-off-by: William A Rowe Jr <[email protected]> Signed-off-by: Yechiel Kalmenson <[email protected]> Signed-off-by: William A Rowe Jr <[email protected]>

htuch approved these changes Nov 4, 2019

View reviewed changes

htuch merged commit 85e2038 into envoyproxy:master Nov 4, 2019

wrowe deleted the fix-msvc-incompatibilties branch November 5, 2019 17:57

sunjayBhatia mentioned this pull request Dec 20, 2019

NT support #129

Closed

		@@ -1,3 +1,11 @@
		// grpcpp/grpcpp.h includes <windows.h>, so undef some interfering symbols

		@@ -0,0 +1,16 @@
		diff --git a/include/tclap/StandardTraits.h b/include/tclap/StandardTraits.h

		@@ -1,5 +1,7 @@
		#pragma once

		#include "envoy/common/platform.h" // Avert format.h including windows.h

		@@ -1,8 +1,10 @@
		#include "common/network/cidr_range.h"

		#ifndef WIN32

Correct stdc++ language and size issues for MSVC #8572

Correct stdc++ language and size issues for MSVC #8572

Conversation

achasveachas commented Oct 10, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lizan Oct 23, 2019 • edited Loading

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

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

Choose a reason for hiding this comment

htuch left a 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

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wrowe commented Oct 11, 2019

wrowe commented Oct 18, 2019

htuch left a 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

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

achasveachas commented Oct 30, 2019

htuch left a 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

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

htuch left a comment

Choose a reason for hiding this comment

lizan left a comment

Choose a reason for hiding this comment

wrowe commented Nov 2, 2019

wrowe commented Nov 4, 2019

htuch commented Nov 4, 2019

kfaseela commented Nov 22, 2021

lizan Oct 23, 2019 •

edited

Loading