protobuf 3 warnings

[yoe]

Hi,

Someone was "nice" enough to upload protobuf 3 into Debian unstable last night. The result is that ola now produces warnings related to protobuf:

libtool: compile:  g++ -DHAVE_CONFIG_H -I. -Wdate-time -D_FORTIFY_SOURCE=2 -I./include -I./include -Wall -Wformat -W -fvisibility-inlines-hidden -pthread -Werror -DPID_DATA_DIR=\"/usr/share/ola/pids\" -Wno-error=deprecated-declarations -Wno-error=unused-parameter -pthread -c common/rdm/Pids.pb.cc  -fPIC -DPIC -o common/rdm/.libs/common_libolacommon_la-Pids.pb.o
common/rdm/Pids.pb.cc: In member function 'virtual google::protobuf::uint8* ola::rdm::pid::LabeledValue::InternalSerializeWithCachedSizesToArray(bool, google::protobuf::uint8*) const':
common/rdm/Pids.pb.cc:517:10: warning: unused parameter 'deterministic' [-Wunused-parameter]
     bool deterministic, ::google::protobuf::uint8* target) const {
          ^~~~~~~~~~~~~
common/rdm/Pids.pb.cc: In member function 'virtual google::protobuf::uint8* ola::rdm::pid::Range::InternalSerializeWithCachedSizesToArray(bool, google::protobuf::uint8*) const':
common/rdm/Pids.pb.cc:928:10: warning: unused parameter 'deterministic' [-Wunused-parameter]
     bool deterministic, ::google::protobuf::uint8* target) const {
          ^~~~~~~~~~~~~
[...]

etc, it repeats a few times.

A workaround is to use -Wno-error=unused-parameter.

See also the Debian bugreport

[peternewman]

Thanks for passing on @yoe . The other half of that, the python-numpy dependency was already fixed as far back as 0.10.0, so I'm not sure how it's managed to avoid Debian:
https://github.com/OpenLightingProject/ola/blob/0.10.0/debian/control

I've done a fix for the warning error here to match our normal style (and avoid overriding the error everywhere), would you mind testing it please:
#1111

[yoe]

Hi Peter,

On Fri, Aug 26, 2016 at 02:32:40AM -0700, Peter Newman wrote:

Thanks for passing on @yoe . The other half of that, the python-numpy
dependency was already fixed as far back as 0.10.0, so I'm not sure how it's
managed to avoid Debian:
https://github.com/OpenLightingProject/ola/blob/0.10.0/debian/control

Debian's tooling nukes upstream debian/ directories. I'm not sure why
that's considered a good thing by the dpkg maintainers, but ah well.

I've already uploaded a new version which adds python-numpy.

I've done a fix for the warning error here to match our normal style (and avoid
overriding the error everywhere), would you mind testing it please:
#1111

Will do.

< ron> I mean, the main practical problem with C++, is there's like a dozen
people in the world who think they really understand all of its rules,
and pretty much all of them are just lying to themselves too.
-- #debian-devel, OFTC, 2016-02-12

[peternewman]

That's not very helpful, how do you do it then, do you have another copy of the debian directory which you pull the source in from? Could you just git external our directory?

[yoe]

On Fri, Aug 26, 2016 at 03:30:45AM -0700, Peter Newman wrote:

That's not very helpful, how do you do it then, do you have another copy of the
debian directory which you pull the source in from? Could you just git external
our directory?

Doesn't involve git at that level.

dpkg-source sees an ola_.orig.tar.gz (which is supposed to be
the source as you release it), and an ola_.debian.tar.gz. When
called with -x, it extracts the orig.tar.gz, nukes any debian/ directory
it may see, and then extracts the debian.tar.gz (which contains the
debian/ directory as well as any patches that I might have added -- no,
there are none currently, but sometimes I may cherry-pick things and
then those get added to debian.tar.gz)

Extracting the source package is done as part of the official build of a
Debian package, so changes that I don't include into the upload into
Debian never make it there.

Having said that, I did originally base the debian/ directory on what's
in the ola project's git repository, and in my own git repository where
I maintain the package I do look at what's in your debian/ directory; I
just don't merge it blindly.

< ron> I mean, the main practical problem with C++, is there's like a dozen
people in the world who think they really understand all of its rules,
and pretty much all of them are just lying to themselves too.
-- #debian-devel, OFTC, 2016-02-12

[peternewman]

Gotcha, so https://github.com/yoe/ola/tree/master/debian is the source of the Debian OLA packages, and where we should backport changes from.

I'll try and look at getting some more of these into 0.10 or 0.11 and reducing the diff:
0.10...yoe:master

[yoe]

On Tue, Aug 30, 2016 at 06:24:13AM -0700, Peter Newman wrote:

Gotcha, so https://github.com/yoe/ola/tree/master/debian is the source of the
Debian OLA packages,

Right.

and where we should backport changes from.

If you want to, but I'm not sure if that's necessary :-)

I'll try and look at getting some more of these into 0.10 or 0.11 and reducing
the diff:
0.10...yoe:master

You could do that. However, do note that many of the commits there were
already merged by @nomis52 with commit
0e32168, as a squash commit. That
leaves them in my git history, without merging them "properly" into your
source tree. As such, master..yoe:master is always going to be slightly
longish. Not much we can (or should?) do about that.

< ron> I mean, the main practical problem with C++, is there's like a dozen
people in the world who think they really understand all of its rules,
and pretty much all of them are just lying to themselves too.
-- #debian-devel, OFTC, 2016-02-12

[peternewman]

Well we ship a debian folder, and get debs built on lanchpad, so ought to have the "best" debian folder we can.

Thanks for the note about the squash, doing a compare seems to hide the changes (at least from the changed files perspective). I think if we cherry-picked the unsquashed commits it might resolve that problem, but it's probably not worth bothering as you say.

[peternewman]

I'm going to close this as the protobuf issue is solved via #1111 . We can continue discussion here if you want, or I'll open an issue to backport more debian folder changes.