btc-ag / service-idl

Xtext-based Service IDL (Interface Definition Language) and Code Generators for Protobuf, C++, Java and .NET
Eclipse Public License 2.0
8 stars 8 forks source link

.NET and C++ target assembly names are identical when not specifying a "main module" #217

Closed kamiddel closed 5 years ago

kamiddel commented 5 years ago

If the generated code is compiled for .NET and CPP, for both technologies binary files with identical names are created. A deployment in a directory for both technologies will result in a name conflict.

sigiesec commented 5 years ago

Looks like a regression. In principle, .NET assemblies should always contain a .NET name fragment, controlled by the "main" prefix to the module keyword.

sigiesec commented 5 years ago

I checked the current implementation. The .NET fragment is only added after the modules marked as main, these may be 0, 1 or multiple. However, the nu-get package name will always have .NET at its end, so to get a consistent naming, for a file named Foo.Bar.idl, you should use:

module Foo {
  main module Bar {
    // your interface definition
  }
}

Can you use this approach for the moment? I will discuss with @GerrietReents how to deal with this in future versions. I can imagine removing/deprecating the main keyword completely, and derive this from the package name. Maybe it also makes sense to check consistency between the module name and the file name.

kamiddel commented 5 years ago

I will try that and then feedback here. Thanks.

kamiddel commented 5 years ago

Now I get files like BTC.GA.FieldLink.NET.ServiceAPI.dll. This is not the name structure I had expected (request: BTC.GA.FieldLink.ServiceAPI.NET.dll), but solves the problem for now.

sigiesec commented 5 years ago

@kamiddel As discussed today, the current behaviour when specifying the inner module as the main module will be preserved. Appending .NET at the end of each module name will probably not be supported, as this would create an inconsistency with the package name.

huttenlocher commented 5 years ago

Just a remark: the current behavior is "by design" and was requested in this form by Stefan Baier. Using "main" keyword allows the IDL author to designate the namespace part with ".NET" appendix in a flexible way. Without this hint, there is indeed no difference between resulting namespaces in C++ und .NET

sigiesec commented 5 years ago

After discussion with @GerrietReents and @huttenlocher, which includes some other issues with the naming (where the package name is not consistent with the module name), we came to the following conclusion:

In a future step, the IDL syntax might be changed to remove the possibility for some misuses, by replacing the nested module specification by a single module statement, similar to the package declaration in Java. This is beyond the scope of this issue.

huttenlocher commented 5 years ago

@sigiesec, just to mention: the first check (there may be only one "main" module) already exists since "main" keyword was introduced, see com.btc.serviceidl.validation.IdlValidator::checkMainModule.