Make fails with 'GOOGLE_CHECK' not declared in this scope

[Da-Boom]

I was compiling for arch linux, when i got the following python failure during make:

protoc/StrUtil.cpp: In function ‘char* ola::FastHexToBuffer(int, char*)’:
protoc/StrUtil.cpp:256:3: error: ‘GOOGLE_CHECK’ was not declared in this scope
   GOOGLE_CHECK(i >= 0)
   ^~~~~~~~~~~~
protoc/StrUtil.cpp: In function ‘char* ola::FastUInt64ToBufferLeft(uint64_t, char*)’:
protoc/StrUtil.cpp:436:3: error: ‘GOOGLE_DCHECK_LT’ was not declared in this scope
   GOOGLE_DCHECK_LT(digits, 100);
   ^~~~~~~~~~~~~~~~
protoc/StrUtil.cpp:436:3: note: suggested alternative: ‘GOOGLE_THREAD_LOCAL’
   GOOGLE_DCHECK_LT(digits, 100);
   ^~~~~~~~~~~~~~~~
   GOOGLE_THREAD_LOCAL

i am running the latest protobuf currently in the arch repos, and this error appears regardless of running in python2.7 or python3 (i tried 2.7 in a virtualenv) but it seems to have failed...

now i am not sure what the difference is between GOOGLE_DCHECK_LT and GOOGLE_THREAD_LOCAL, but it seems to be suggesting that we use that instead.

Now i have no idea if this is an arch only issue, but i would rather test on my PC, before moving to my raspberry pi with a cap touch screen. (I'm gonna be messing with the APIS and combining it with PYGAME (and if i can work it out, a custom web server)

I have also noticed a few other items that the compiler seems to have skipped right over, like a deprecated function:

protoc/CppGenerator.cpp:57:3: warning: ‘template<class> class std::auto_ptr’ is deprecated [-Wdeprecated-declarations]

also something to do with random during ./configure

configure: WARNING: random: present but cannot be compiled
configure: WARNING: random:     check for missing prerequisite headers?
configure: WARNING: random: see the Autoconf documentation
configure: WARNING: random:     section "Present But Cannot Be Compiled"
configure: WARNING: random: proceeding with the compiler's result
configure: WARNING:     ## --------------------------------------------- ##
configure: WARNING:     ## Report this to open-lighting@googlegroups.com ##
configure: WARNING:     ## --------------------------------------------- ##
checking for random... no

Now i dont pretend to know anything about these APIs I just thought I would point them out

[peternewman]

I was compiling for arch linux, when i got the following python failure during make:

protoc/StrUtil.cpp: In function ‘char* ola::FastHexToBuffer(int, char*)’:
protoc/StrUtil.cpp:256:3: error: ‘GOOGLE_CHECK’ was not declared in this scope
   GOOGLE_CHECK(i >= 0)
   ^~~~~~~~~~~~
protoc/StrUtil.cpp: In function ‘char* ola::FastUInt64ToBufferLeft(uint64_t, char*)’:
protoc/StrUtil.cpp:436:3: error: ‘GOOGLE_DCHECK_LT’ was not declared in this scope
   GOOGLE_DCHECK_LT(digits, 100);
   ^~~~~~~~~~~~~~~~
protoc/StrUtil.cpp:436:3: note: suggested alternative: ‘GOOGLE_THREAD_LOCAL’
   GOOGLE_DCHECK_LT(digits, 100);
   ^~~~~~~~~~~~~~~~
   GOOGLE_THREAD_LOCAL

i am running the latest protobuf currently in the arch repos, and this error appears regardless of running in python2.7 or python3 (i tried 2.7 in a virtualenv) but it seems to have failed...

now i am not sure what the difference is between GOOGLE_DCHECK_LT and GOOGLE_THREAD_LOCAL, but it seems to be suggesting that we use that instead.

Damn, I thought we might escape this for a bit longer. This is a bug/feature due to moving to Protobuf 3.7 and above (it worked with 3.6). I'd spotted it already as Homebrew have dropped Protobuf 3.6 support. FWIW, this is a C++ issue, rather than anything really to do with Protobuf. There are some improvements to make it better here:
https://github.com/OpenLightingProject/ola/pull/1535/files

But they don't quite fix it unfortunately:
https://travis-ci.org/OpenLightingProject/ola/jobs/504183634#L3648-L3666

How's your C++? Given you can easily recreate it, you might have more luck at fixing it than me trying to round trip via Travis to test code changes.

I have also noticed a few other items that the compiler seems to have skipped right over, like a deprecated function:

protoc/CppGenerator.cpp:57:3: warning: ‘template<class> class std::auto_ptr’ is deprecated [-Wdeprecated-declarations]

Thanks for the heads up, that's expected, hence why we've turned off the warning. They've simultaneously switched auto_ptr to unique_ptr, so if we want to support new and old compilers, we have to stick with the old one and mute the message.

also something to do with random during ./configure

configure: WARNING: random: present but cannot be compiled
configure: WARNING: random:     check for missing prerequisite headers?
configure: WARNING: random: see the Autoconf documentation
configure: WARNING: random:     section "Present But Cannot Be Compiled"
configure: WARNING: random: proceeding with the compiler's result
configure: WARNING:     ## --------------------------------------------- ##
configure: WARNING:     ## Report this to open-lighting@googlegroups.com ##
configure: WARNING:     ## --------------------------------------------- ##
checking for random... no

I thought we'd fixed this, which version of OLA are you compiling against?

In terms of more generally, if you can install an older Protobuf, it should all compile fine and you can do your Python work as you need to.

[Da-Boom]

I have a small amount of C++ expirience, but it's minimal at best - I'm more comfortable with java (although at uni, i will be learning C++ properly next semester)

I only noticed this, when compiling folowing the git tutorial on https://www.openlighting.org/ola/linuxinstall/#Git - I also tried installing with ola-git on the AUR, both yielded the same result - this was yesterday 28th, at about 9pm ACDT (Australian Central Daylight TIme) And i am running the x86_64 Linux 4.19.30-1-MANJARO kernel

I can, however follow instructions, so if you wish to give me some I can follow them (however i am doing this as a side project, and University assignments are picking up

[peternewman]

Okay Da-Boom, well first thing, can you install an older version of Protobuf easily or is that not possible in Arch? That should get you going at least.

[Da-Boom]

Ok.. so i have managed to downgrade(now i have to be careful when running pacman to update though lol, because it immediately realized that i'm running older versions and wanted to update)

I am now running protobuf-3.6.1.3-1 and python-protobuf-3.6.1.3-1

I will go ahead and compile..

Still getting the same random errors during ./configure
Still getting the deprecated stuff during make - in fact there is a lot more stuff that is deprecated
Make success!

I have attached the entire output of make - it is just a standard log file, you can view it in a text editor - it should have recorded all the deprecated warnings, and maybe you can make sense of them, considering i am an amateur when it comes to C++ and i don't know much about the internals and codebase of this software...

OLA_MAKELOG.log

I am going to continue with make check and install the software now!

But, i would like to say that in my opinion this should only be regarded as a workaround and should not be considered as a solution in any right

[stewiem2000]

I'm not entirely familiar with the iner workings of OLA, C++ standards, nor protobuf, but Googling about seems to suggest that this functionality is now a core part of C++11? In which case, this hack (on top of #1535) seems to get OLA building for me on FreeBSD 12.0 with protobuf 3.7.1. However, I can't help but think it might already be handled with protobuf's FieldDescriptor::InternalTypeOnceInit() ??

patch-1535suppliment.txt

[dj-foxxy]

Hi all,

Compiling on Arch Linux. Master compiled after applying @stewiem2000 patch and deleting the GOOGLE_CHECK and GOOGLE_DCHECK_LT checks.

diff --git a/protoc/CppFileGenerator.cpp b/protoc/CppFileGenerator.cpp
index e7dd95ccb..7ce9d022c 100644
--- a/protoc/CppFileGenerator.cpp
+++ b/protoc/CppFileGenerator.cpp
@@ -223,9 +223,9 @@ void FileGenerator::GenerateBuildDescriptors(Printer* printer) {
     printer->Print(
       "namespace {\n"
       "\n"
-      "GOOGLE_PROTOBUF_DECLARE_ONCE(protobuf_AssignDescriptors_once_);\n"
+      "::google::protobuf::internal::once_flag protobuf_AssignDescriptors_once_;\n"
       "inline void protobuf_AssignDescriptorsOnce() {\n"
-      "  ::google::protobuf::GoogleOnceInit(&protobuf_AssignDescriptors_once_,"
+      "  ::google::protobuf::internal::call_once(protobuf_AssignDescriptors_once_,"
       "\n"
       "                 &$assigndescriptorsname$);\n"
       "}\n"
diff --git a/protoc/StrUtil.cpp b/protoc/StrUtil.cpp
index 85891a6a4..76579f632 100644
--- a/protoc/StrUtil.cpp
+++ b/protoc/StrUtil.cpp
@@ -253,9 +253,6 @@ char *FastInt32ToBuffer(int32_t i, char* buffer) {
 }
 
 char *FastHexToBuffer(int i, char* buffer) {
-  GOOGLE_CHECK(i >= 0)
-      << "FastHexToBuffer() wants non-negative integers, not " << i;
-
   static const char *hexdigits = "0123456789abcdef";
   char *p = buffer + 21;
   *p-- = '\0';
@@ -433,7 +430,6 @@ char* FastUInt64ToBufferLeft(uint64_t u64, char* buffer) {
   u = u64 - (top_11_digits * 1000000000);
 
   digits = u / 10000000;  // 10,000,000
-  GOOGLE_DCHECK_LT(digits, 100);
   ASCII_digits = two_ASCII_digits[digits];
   buffer[0] = ASCII_digits[0];
   buffer[1] = ASCII_digits[1];

[peternewman]

Now see #1558 not #1535 .

[kripton]

#1558 fixes that specific error for me. However, it fails later in the compilation with:

common/protocol/OlaService.pb.cpp:37:29: error: expected constructor, destructor, or type conversion before '(' token
   37 | GOOGLE_PROTOBUF_DECLARE_ONCE(protobuf_AssignDescriptors_once_);
      |                             ^
common/protocol/OlaService.pb.cpp: In function 'void ola::proto::{anonymous}::protobuf_AssignDescriptorsOnce()':
common/protocol/OlaService.pb.cpp:39:23: error: 'GoogleOnceInit' is not a member of 'google::protobuf'
   39 |   ::google::protobuf::GoogleOnceInit(&protobuf_AssignDescriptors_once_,
      |                       ^~~~~~~~~~~~~~
common/protocol/OlaService.pb.cpp:39:39: error: 'protobuf_AssignDescriptors_once_' was not declared in this scope; did you mean 'protobuf_AssignDescriptorsOnce'?
   39 |   ::google::protobuf::GoogleOnceInit(&protobuf_AssignDescriptors_once_,
      |                                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                       protobuf_AssignDescriptorsOnce

which is a bit strange since that is generated code ....
Using protbuf 3.11.4 here

[kripton]

Found the piece of code that the compiler now throws up upon: https://github.com/OpenLightingProject/ola/blob/master/protoc/CppFileGenerator.cpp#L226

[kripton]

This branch works for me: https://github.com/OpenLightingProject/ola/compare/master...kripton:protobufFixes?expand=1
I've modified values on the web page and tested with the included "ola_dmxmonitor". Is that enough to know that the protobuf changes are okay?

[peternewman]

Thanks for the heads up and further work @kripton !

which is a bit strange since that is generated code ....
Using protbuf 3.11.4 here

Yeah unfortunately although it's generated code, it's generated with an ancient version of the protoc generator which we modified, but I never managed to find out which version our modifications were based on, so easily modifying a new version is pretty tough, hence these continued issues.

This branch works for me: https://github.com/OpenLightingProject/ola/compare/master...kripton:protobufFixes?expand=1

Thanks for that, care to open a pull request so we can look at getting them merged in?

I've modified values on the web page and tested with the included "ola_dmxmonitor". Is that enough to know that the protobuf changes are okay?

Does make check pass too? It would be good to do it the other way round too, i.e. modify via ola_dmxconsole and confirm the web page reflects. It would also be excellent if you wouldn't mind doing equivalent tests with the python versions in python/examples just to confirm they all still play ball.

Then I can test your PR in an older version for backwards compatibility.

[kripton]

Yeah unfortunately although it's generated code, it's generated with an ancient version of the protoc generator which we modified, but I never managed to find out which version our modifications were based on, so easily modifying a new version is pretty tough, hence these continued issues.

I also investigated this yesterday. Actually, the binary protoc installed on the system is used. The code shipped with OLA is a plugin to that protobuf compiler. However, I also was unable to find good resources about protoc plugins.
The most relevant comments are in https://github.com/OpenLightingProject/ola/blob/master/protoc/ola-protoc-generator-plugin.cpp#L23. Back then, it was recommended on https://developers.google.com/protocol-buffers/docs/proto#services to provide custom code to generate the protobuf Services. Today, the seem to recommend gRPC for that. So for the long run (and an incompatible change), this might be investigated.

Thanks for that, care to open a pull request so we can look at getting them merged in?

Sure, I'll create a proper PR later today. Is it okay to base on your code as I did + add my changes? I didn't know if your changes were proper or rather some quick hack to get it working.

Does make check pass too? It would be good to do it the other way round too, i.e. modify via ola_dmxconsole and confirm the web page reflects. It would also be excellent if you wouldn't mind doing equivalent tests with the python versions in python/examples just to confirm they all still play ball.

dmxconsole works both ways, so I can also write the values and the web interface reflects the changes.
I've also run make check today and the results look good:

============================================================================
Testsuite summary for OLA 0.10.7
============================================================================
# TOTAL: 88
# PASS:  88

Currently working on the Python part but since I'm not used to writing Python code, it might take a bit.

Then I can test your PR in an older version for backwards compatibility.

That would be great. I don't have access to protobuf older than 3.10.1.
Do you think running cross-versions checks are required (OLAd built against protobuf 3.2 and a client built against protobuf 3.11.4)?

[kripton]

Good news, I tested ola_universe_info.py, ola_recv_dmx.py and ola_send_dmx.py using Python 3.6 and protobuf-python 3.11.4. It all works as expected.
Creating the PR now

[peternewman]

Yeah unfortunately although it's generated code, it's generated with an ancient version of the protoc generator which we modified, but I never managed to find out which version our modifications were based on, so easily modifying a new version is pretty tough, hence these continued issues.

I also investigated this yesterday. Actually, the binary protoc installed on the system is used. The code shipped with OLA is a plugin to that protobuf compiler. However, I also was unable to find good resources about protoc plugins.

Ah yes sorry, my mistake on terminology.

There is some stuff here:
https://developers.google.com/protocol-buffers/docs/reference/cpp/google.protobuf.compiler.plugin
https://developers.google.com/protocol-buffers/docs/reference/cpp/google.protobuf.compiler.code_generator#CodeGenerator

The most relevant comments are in https://github.com/OpenLightingProject/ola/blob/master/protoc/ola-protoc-generator-plugin.cpp#L23. Back then, it was recommended on https://developers.google.com/protocol-buffers/docs/proto#services to provide custom code to generate the protobuf Services. Today, the seem to recommend gRPC for that. So for the long run (and an incompatible change), this might be investigated.

Thanks for that, care to open a pull request so we can look at getting them merged in?

Sure, I'll create a proper PR later today. Is it okay to base on your code as I did + add my changes? I didn't know if your changes were proper or rather some quick hack to get it working.

They weren't particularly thorough, just based on trying to get it to compile initially, but only via Travis so not particularly well tested anyway.

Does make check pass too? It would be good to do it the other way round too, i.e. modify via ola_dmxconsole and confirm the web page reflects. It would also be excellent if you wouldn't mind doing equivalent tests with the python versions in python/examples just to confirm they all still play ball.

dmxconsole works both ways, so I can also write the values and the web interface reflects the changes.
I've also run make check today and the results look good:

============================================================================
Testsuite summary for OLA 0.10.7
============================================================================
# TOTAL: 88
# PASS:  88

Excellent, that all looks very promising.

Currently working on the Python part but since I'm not used to writing Python code, it might take a bit.

Then I can test your PR in an older version for backwards compatibility.

That would be great. I don't have access to protobuf older than 3.10.1.

I'm sure mine is older than that.

Do you think running cross-versions checks are required (OLAd built against protobuf 3.2 and a client built against protobuf 3.11.4)?

Given it should all only be running on the same machine (see the localhost comments here):
http://docs.openlighting.org/ola/doc/latest/rpc_system.html#rpc_OLA_RPC

I'd suggest we could require them to use the same protobuf version on both client and server (worst case scenario is going to be say running Python talking to the C++ OLAd, and I think getting them to match isn't a huge ask).

Good news, I tested ola_universe_info.py, ola_recv_dmx.py and ola_send_dmx.py using Python 3.6 and protobuf-python 3.11.4. It all works as expected.
Creating the PR now

That's all great news!

[peternewman]

Reopening just until we get this into 0.10 for 0.10.8.

[kripton]

Can be closed, fixed by #1630 :)

[peternewman]

Can be closed, fixed by #1630 :)

Indeed, closing!

[dj-foxxy]

Thanks for fixing this issue. Just installed directly from Arch Linux's AUR (ola-git package) with no issues!

[peternewman]

That's great to hear @dj-foxxy . Hopefully it works too! 😆

We've discovered a small issue ( #1630 (comment) ) around a minor difference with how we've fixed it in the 0.10 branch, so if you fancy retesting in a few days when we've got that sorted and merged back into master would be great thanks.

[dj-foxxy]

@peternewman, I update about once a week so will at least test building. However, given the current pandemic, we're not getting many live bookings. Next gig tentatively rescheduled for April next year.