AVRO-1936: C++: adds new include guards

[moriarty]

This change adds additional include guards in a different format. https://issues.apache.org/jira/browse/AVRO-1936

Removes the random numbers from the include guards, instead guarding only by NAMESPACE_TYPE_ where TYPE is the Enum, Record or Union Type.

Adds #ifndef NAMESPACE_TYPE_... checks before each struct declaration.

Adds #define NAMESPACE_TYPE_... to the generate(Enum/Record/Union)Traits funtion.

This allows including multiple of the generated C++ files into one source C++ file, where the avsc files were generated from avdl, and the avdl files contained includes with shared basic types.

The previous way to achieve this, as was done in the AvrogencppTests, was to abuse or take advantage adding additional meaningless namespaces.

This doesn't fail the existing AvrogencppTests, but I would like input from avro-cpp users how they've gotten around this and if this breaks anything.

[moriarty]

~~closing for now - will separate into it's own commit and make optional to not break the existing tests~~

Edit: sorry I meant to close #145

[dkulp]

Can this be re-based on master and determine if any of it is still needed?

[moriarty]

Hi,

Last I checked, was about a year ago and it was still needed. I’m at a different company now, and not using AVRO... but when I left, this patch was still being applied to our environment to make thinks work.

@e-schumann @shehzi001 or @garinkid are AFIAK still with https://www.tba.group where this patch was being used, and might be able to answer the question sooner.

Otherwise I’ll check in the new year

[moriarty]

@e-schumann also FYI: #146 was closed because of #404 adding proper C++11 enums. So it’s probably time someone looked at upgrading AVRO if you’re still using my patches.

[moriarty]

I see that someone added a comment in the JIRA issue: https://issues.apache.org/jira/browse/AVRO-1936?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16730381#comment-16730381

I agree, that the better solution would be AVRO-1370, but I think I specifically wrote the ticket title “(AVRO-1936) avrogencpp, includes should have more gaurds or generate more headers” because generating additional headers didn’t seem like it would be quickly implementable without being familiar with the avrogen codebase, and I just needed it to work.

[bleppard]

Just in case anyone thought this was dead. We patch a local fork with this PR every time there is a new AVRO release.

[mkmkme]

Hey @bleppard @moriarty !

I'm quite new to Avro so sorry if my question won't make sense. Would having a #pragma once at the top of the header also do the trick? It seems to be supported by all compilers for years and at the same time it would make the PR simpler and would remove the need for TODO.

If you want, I can try to craft the version with #pragma once

[bleppard]

@mkmkme Unfortunately no. When avdl files are imported into another avdl, the imported definition is placed directly in the target. When the c++ header file is generated there is no "include" for the imported code. The pragma once would only work if the imported code was "included" rather than directly embedded. I find @moriarty 's solution quite acceptable. The alternate proposed solution of generating .h files for everything would be a disruptive, breaking change in our usage context. Perhaps if @moriarty 's solution could be invoked with a command line switch on avrogencpp we could get this functionality merged in?

[mkmkme]

@bleppard thanks for your detailed response! Let's see if resolving AVRO-1370 would take a lot of effort. I'll take a look at it as soon as I have some free time.

[martin-g]

Hello,

Currently there is no active maintainer of the C/C++ Rust SDK! If you want your PR(s) to be merged then I'd like to ask you for a favour - please review (and test) other open C/C++ PRs and approve/comment on them! I could help with the merge if there are at least 2 approvals by users of the library!

[mkmkme]

Hey @martin-g, Does Rust part of Avro also need attention? I'd be happy to help!

[martin-g]

The Rust SDK has an active maintainer who'd be more than happy to work with you! :-)

[mkmkme]

Hey @bleppard !

Would you be able to provide an example for this:

When avdl files are imported into another avdl, the imported definition is placed directly in the target. When the c++ header file is generated there is no "include" for the imported code. The pragma once would only work if the imported code was "included" rather than directly embedded.

I mean the example avdl files and the command to generate the header.

Sorry I'm quite new to Avro C++ codebase and it seems that examples are quite simple and only use JSON. If you could provide me with an example, that would definitely help me understand the problem better. Such an example could also be put in the codebase itself for clarity.