Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Emit a public constructor for each generated message #136

Merged
merged 2 commits into from
May 19, 2014
Merged

Emit a public constructor for each generated message #136

merged 2 commits into from
May 19, 2014

Conversation

danrice67
Copy link
Collaborator

Fixes #135.

JakeWharton
JakeWharton reviewed May 19, 2014
View reviewed changes
wire-compiler/src/main/java/com/squareup/wire/MessageWriter.java
// private SimpleMessage(Builder builder) {
// super(builder);
// this.optional_int32 = builder.optional_int32;
// }
//
private void emitMessageConstructor(MessageType messageType) throws IOException {
private void emitMessageBuilderConstructor(MessageType messageType) throws IOException {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we eliminate this altogether? It looks like both need to perform the necessary sanitization of inputs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only tricky part is passing the Builder's unknownFieldMap into the Message's constructor.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right. I always forget about that guy. I think it's fine as-is then.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally don't think that's worth it. I'm looking forward to initializing my one or two-field messages with new Foo(bar, baz) and the overhead of one method per-type is worth it to me.

For extremists we could even offer a --no-builders option on the compiler.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think two constructors is the best choice, but would like to find a way to eliminate the duplication of the sanitization work.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. Is that just calling this(builder.foo, builder.bar); and then doing the unknown field map?

@JakeWharton
Copy link
Collaborator

LGTM! One nit and one question about killing the private ctor.

@codefromthecrypt
Copy link

LGTM!

danrice67 pushed a commit that referenced this pull request May 19, 2014
Merge pull request #136 from square/rice/public_message_constructors
4721f75 
Emit a public constructor for each generated message
@danrice67 danrice67 merged commit 4721f75 into master May 19, 2014
@danrice67 danrice67 deleted the rice/public_message_constructors branch May 19, 2014 18:11
@ocean7
Copy link

ocean7 commented May 20, 2014

Not sure what LGTM means but this is pretty sweet. Can I pick this up in a snapshot version?

@danrice67
Copy link
Collaborator Author

LGTM = looks good to me

You can clone the repo and do 'mvn install' locally. Then your upstream project can depend on Wire 1.5.1-SNAPSHOT.

@ocean7
Copy link

ocean7 commented May 28, 2014

Wanted to note that 1.5.1-SNAPSHOT has issues. Attempted to build it and some tests fail. Built with -DskipTests and it built but the generated code has a bunch of compile errors. Anybody else seeing this?

@JakeWharton
Copy link
Collaborator

Nope: https://travis-ci.org/square/wire

Are you using Java 7 or Java 8?

@ocean7
Copy link

ocean7 commented May 28, 2014

Java 1.7.0_45

@danrice67
Copy link
Collaborator Author

@ocean7 would you be willing to paste the test failure output here?

@ocean7
Copy link

ocean7 commented May 28, 2014

C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\BuilderAdapter.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\EnumAdapter.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\ExtendableMessage.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\Extension.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\ExtensionMap.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\ExtensionRegistry.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\Message.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\MessageAdapter.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\package-info.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\Preconditions.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\ProtoEnum.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\ProtoField.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\Redactor.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\TagMap.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\UnknownFieldMap.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\Wire.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\WireInput.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\WireOutput.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\WireType.java:0: File does not end with a newline.

Perhaps some kind of encoding issue?

@danrice67
Copy link
Collaborator Author

Looks like it could be related to Windows encoding. Can you provide more context as to what phase of the build is producing this output? If you like you can paste the entire output into a Gist.

@ocean7
Copy link

ocean7 commented May 28, 2014

Not sure how to post a gist...

$ ~/data/applications/apache-maven-3.0.4/bin/mvn install
[INFO] Scanning for projects...
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Build Order:
[INFO]
[INFO] Wire Protocol Buffers
[INFO] Wire Protocol Buffer Runtime
[INFO] Wire Protocol Buffer Compiler
[INFO] Wire Protocol Buffer Gson Support
[INFO] Wire Protocol Buffer Maven Plugin
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] Building Wire Protocol Buffers 1.5.1-SNAPSHOT
[INFO] ------------------------------------------------------------------------
[INFO]
[INFO] --- maven-enforcer-plugin:1.0:enforce (enforce-maven) @ wire ---
[INFO]
[INFO] --- maven-checkstyle-plugin:2.10:checkstyle (default) @ wire ---
[INFO]
[INFO] --- maven-install-plugin:2.3.1:install (default-install) @ wire ---
[INFO] Installing C:\data\projects\wire\pom.xml to C:\Users\ocean\.m2\repository\com\squareup\wire\wire\1.5.1-SNAPSHOT\wire-1.5.1-SNAPSHOT.pom
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] Building Wire Protocol Buffer Runtime 1.5.1-SNAPSHOT
[INFO] ------------------------------------------------------------------------
[INFO]
[INFO] --- maven-enforcer-plugin:1.0:enforce (enforce-maven) @ wire-runtime ---
[INFO]
[INFO] --- maven-resources-plugin:2.5:resources (default-resources) @ wire-runtime ---
[debug] execute contextualize
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] skip non existing resourceDirectory C:\data\projects\wire\wire-runtime\src\main\resources
[INFO]
[INFO] --- maven-compiler-plugin:3.1:compile (default-compile) @ wire-runtime ---
[INFO] Changes detected - recompiling the module!
[INFO] Compiling 19 source files to C:\data\projects\wire\wire-runtime\target\classes
[WARNING] /C:/data/projects/wire/wire-runtime/src/main/java/com/squareup/wire/Wire.java: Some input files use unchecked or unsafe operations.
[WARNING] /C:/data/projects/wire/wire-runtime/src/main/java/com/squareup/wire/Wire.java: Recompile with -Xlint:unchecked for details.
[INFO]
[INFO] --- maven-resources-plugin:2.5:testResources (default-testResources) @ wire-runtime ---
[debug] execute contextualize
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] skip non existing resourceDirectory C:\data\projects\wire\wire-runtime\src\test\resources
[INFO]
[INFO] --- maven-compiler-plugin:3.1:testCompile (default-testCompile) @ wire-runtime ---
[INFO] Nothing to compile - all classes are up to date
[INFO]
[INFO] --- maven-surefire-plugin:2.10:test (default-test) @ wire-runtime ---
[INFO] Surefire report directory: C:\data\projects\wire\wire-runtime\target\surefire-reports

-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running com.squareup.wire.ByteStringTest
Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.056 sec
Running com.squareup.wire.CustomOptionsTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.015 sec
Running com.squareup.wire.protobuf.TestAllTypes
Tests run: 19, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.302 sec
Running com.squareup.wire.RedactedTest
Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.015 sec
Running com.squareup.wire.TagMapTest
Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.001 sec
Running com.squareup.wire.UnknownFieldsTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.001 sec
Running com.squareup.wire.WireTest
Tests run: 8, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.032 sec

Results :

Tests run: 45, Failures: 0, Errors: 0, Skipped: 0

[INFO]
[INFO] --- maven-jar-plugin:2.3.2:jar (default-jar) @ wire-runtime ---
[INFO] Building jar: C:\data\projects\wire\wire-runtime\target\wire-runtime-1.5.1-SNAPSHOT.jar
[INFO]
[INFO] --- maven-checkstyle-plugin:2.10:checkstyle (default) @ wire-runtime ---
[INFO] Starting audit...
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\BuilderAdapter.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\EnumAdapter.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\ExtendableMessage.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\Extension.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\ExtensionMap.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\ExtensionRegistry.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\Message.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\MessageAdapter.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\package-info.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\Preconditions.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\ProtoEnum.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\ProtoField.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\Redactor.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\TagMap.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\UnknownFieldMap.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\Wire.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\WireInput.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\WireOutput.java:0: File does not end with a newline.
C:\data\projects\wire\wire-runtime\src\main\java\com\squareup\wire\WireType.java:0: File does not end with a newline.
Audit done.

[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary:
[INFO]
[INFO] Wire Protocol Buffers ............................. SUCCESS [2.001s]
[INFO] Wire Protocol Buffer Runtime ...................... FAILURE [4.638s]
[INFO] Wire Protocol Buffer Compiler ..................... SKIPPED
[INFO] Wire Protocol Buffer Gson Support ................. SKIPPED
[INFO] Wire Protocol Buffer Maven Plugin ................. SKIPPED
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 6.850s
[INFO] Finished at: Wed May 28 09:12:21 EDT 2014
[INFO] Final Memory: 23M/218M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:2.10:checkstyle (default) on project wire-runtime: An error has occurred in Checkstyle report generation. Failed during checkstyle execution: There are 19 checkstyle errors. -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
[ERROR]
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR]   mvn <goals> -rf :wire-runtime

@ocean7
Copy link

ocean7 commented May 28, 2014

If it is a Windows encoding issue I'll look into how to specify a specific encoding for the build. Seems like maven builds are supposed to define their project encoding anyways. Something to do with this common error perhaps: http://maven.apache.org/general.html#encoding-warning ?

@danrice67
Copy link
Collaborator Author

@ocean7 please make the following change to the checkstyle.xml file and re-run. Let me know if the build succeeds. Basically you will be removing the line <module name="NewlineAtEndOfFile"/> and replacing it with

<module name="NewlineAtEndOfFile">
  <property name="lineSeparator" value="lf"/>
</module>

diff --git a/checkstyle.xml b/checkstyle.xml
index ab2b00b..9b1f97b 100644
--- a/checkstyle.xml
+++ b/checkstyle.xml
@@ -16,7 +16,10 @@
     <property name="checkFormat" value="$1"/>
   </module>

-  <module name="NewlineAtEndOfFile"/>
+  <module name="NewlineAtEndOfFile">
+    <property name="lineSeparator" value="lf"/>
+  </module>
+
   <module name="FileLength"/>
   <module name="FileTabCharacter"/>

@ocean7
Copy link

ocean7 commented May 28, 2014

Fixed one problem, but alas another.

$ ~/data/applications/apache-maven-3.0.4/bin/mvn install
[INFO] Scanning for projects...
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Build Order:
[INFO]
[INFO] Wire Protocol Buffers
[INFO] Wire Protocol Buffer Runtime
[INFO] Wire Protocol Buffer Compiler
[INFO] Wire Protocol Buffer Gson Support
[INFO] Wire Protocol Buffer Maven Plugin
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] Building Wire Protocol Buffers 1.5.1-SNAPSHOT
[INFO] ------------------------------------------------------------------------
[INFO]
[INFO] --- maven-enforcer-plugin:1.0:enforce (enforce-maven) @ wire ---
[INFO]
[INFO] --- maven-checkstyle-plugin:2.10:checkstyle (default) @ wire ---
[INFO]
[INFO] --- maven-install-plugin:2.3.1:install (default-install) @ wire ---
[INFO] Installing C:\data\projects\wire\pom.xml to C:\Users\ocean\.m2\repository\com\squareup\wire\wire\1.5.1-SNAPSHOT\wire-1.5.1-SNAPSHOT.pom
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] Building Wire Protocol Buffer Runtime 1.5.1-SNAPSHOT
[INFO] ------------------------------------------------------------------------
[INFO]
[INFO] --- maven-enforcer-plugin:1.0:enforce (enforce-maven) @ wire-runtime ---
[INFO]
[INFO] --- maven-resources-plugin:2.5:resources (default-resources) @ wire-runtime ---
[debug] execute contextualize
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] skip non existing resourceDirectory C:\data\projects\wire\wire-runtime\src\main\resources
[INFO]
[INFO] --- maven-compiler-plugin:3.1:compile (default-compile) @ wire-runtime ---
[INFO] Changes detected - recompiling the module!
[INFO] Compiling 19 source files to C:\data\projects\wire\wire-runtime\target\classes
[WARNING] /C:/data/projects/wire/wire-runtime/src/main/java/com/squareup/wire/Wire.java: Some input files use unchecked or unsafe operations.
[WARNING] /C:/data/projects/wire/wire-runtime/src/main/java/com/squareup/wire/Wire.java: Recompile with -Xlint:unchecked for details.
[INFO]
[INFO] --- maven-resources-plugin:2.5:testResources (default-testResources) @ wire-runtime ---
[debug] execute contextualize
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] skip non existing resourceDirectory C:\data\projects\wire\wire-runtime\src\test\resources
[INFO]
[INFO] --- maven-compiler-plugin:3.1:testCompile (default-testCompile) @ wire-runtime ---
[INFO] Nothing to compile - all classes are up to date
[INFO]
[INFO] --- maven-surefire-plugin:2.10:test (default-test) @ wire-runtime ---
[INFO] Surefire report directory: C:\data\projects\wire\wire-runtime\target\surefire-reports

-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running com.squareup.wire.ByteStringTest
Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.066 sec
Running com.squareup.wire.CustomOptionsTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.022 sec
Running com.squareup.wire.protobuf.TestAllTypes
Tests run: 19, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.279 sec
Running com.squareup.wire.RedactedTest
Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.016 sec
Running com.squareup.wire.TagMapTest
Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.001 sec
Running com.squareup.wire.UnknownFieldsTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.002 sec
Running com.squareup.wire.WireTest
Tests run: 8, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.035 sec

Results :

Tests run: 45, Failures: 0, Errors: 0, Skipped: 0

[INFO]
[INFO] --- maven-jar-plugin:2.3.2:jar (default-jar) @ wire-runtime ---
[INFO] Building jar: C:\data\projects\wire\wire-runtime\target\wire-runtime-1.5.1-SNAPSHOT.jar
[INFO]
[INFO] --- maven-checkstyle-plugin:2.10:checkstyle (default) @ wire-runtime ---
[INFO] Starting audit...
Audit done.

[WARNING] Unable to locate Source XRef to link to - DISABLED
[INFO]
[INFO] --- maven-install-plugin:2.3.1:install (default-install) @ wire-runtime ---
[INFO] Installing C:\data\projects\wire\wire-runtime\target\wire-runtime-1.5.1-SNAPSHOT.jar to C:\Users\ocean\.m2\repository\com\squareup\wire\wire-runtime\1.5.1-SNAPSHOT\wire-runtime-1.5.1-SNAPSHOT.jar
[INFO] Installing C:\data\projects\wire\wire-runtime\pom.xml to C:\Users\ocean\.m2\repository\com\squareup\wire\wire-runtime\1.5.1-SNAPSHOT\wire-runtime-1.5.1-SNAPSHOT.pom
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] Building Wire Protocol Buffer Compiler 1.5.1-SNAPSHOT
[INFO] ------------------------------------------------------------------------
[INFO]
[INFO] --- maven-enforcer-plugin:1.0:enforce (enforce-maven) @ wire-compiler ---
[INFO]
[INFO] --- maven-resources-plugin:2.5:resources (default-resources) @ wire-compiler ---
[debug] execute contextualize
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] skip non existing resourceDirectory C:\data\projects\wire\wire-compiler\src\main\resources
[INFO]
[INFO] --- maven-compiler-plugin:3.1:compile (default-compile) @ wire-compiler ---
[INFO] Nothing to compile - all classes are up to date
[INFO]
[INFO] --- maven-resources-plugin:2.5:testResources (default-testResources) @ wire-compiler ---
[debug] execute contextualize
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] skip non existing resourceDirectory C:\data\projects\wire\wire-compiler\src\test\resources
[INFO]
[INFO] --- maven-compiler-plugin:3.1:testCompile (default-testCompile) @ wire-compiler ---
[INFO] Nothing to compile - all classes are up to date
[INFO]
[INFO] --- maven-surefire-plugin:2.10:test (default-test) @ wire-compiler ---
[INFO] Surefire report directory: C:\data\projects\wire\wire-compiler\target\surefire-reports

-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running com.squareup.wire.WireCompilerErrorTest
Tests run: 5, Failures: 5, Errors: 0, Skipped: 0, Time elapsed: 0.111 sec <<< FAILURE!
Running com.squareup.wire.WireCompilerTest
cwd = C:\data\projects\wire\wire-compiler\.
Compiling proto source file ../wire-runtime/src/test/proto\request_response.proto
Created output directory C:\data\projects\wire\wire-compiler\WireCompilerTest\com\squareup\services\anotherpackage
Writing generated code to C:\data\projects\wire\wire-compiler\WireCompilerTest\com\squareup\services\anotherpackage\SendDataRequest.java
Writing generated code to C:\data\projects\wire\wire-compiler\WireCompilerTest\com\squareup\services\anotherpackage\SendDataResponse.java
Compiling proto source file ../wire-runtime/src/test/proto\service.proto
Writing generated code to C:\data\projects\wire\wire-compiler\WireCompilerTest\com\squareup\services\ExampleService.java
Couldn't delete C:\data\projects\wire\wire-compiler\WireCompilerTest
cwd = C:\data\projects\wire\wire-compiler\.
cwd = C:\data\projects\wire\wire-compiler\.
cwd = C:\data\projects\wire\wire-compiler\.
cwd = C:\data\projects\wire\wire-compiler\.
cwd = C:\data\projects\wire\wire-compiler\.
cwd = C:\data\projects\wire\wire-compiler\.
cwd = C:\data\projects\wire\wire-compiler\.
cwd = C:\data\projects\wire\wire-compiler\.
cwd = C:\data\projects\wire\wire-compiler\.
cwd = C:\data\projects\wire\wire-compiler\.
cwd = C:\data\projects\wire\wire-compiler\.
cwd = C:\data\projects\wire\wire-compiler\.
cwd = C:\data\projects\wire\wire-compiler\.
cwd = C:\data\projects\wire\wire-compiler\.
cwd = C:\data\projects\wire\wire-compiler\.
cwd = C:\data\projects\wire\wire-compiler\.
cwd = C:\data\projects\wire\wire-compiler\.
cwd = C:\data\projects\wire\wire-compiler\.
cwd = C:\data\projects\wire\wire-compiler\.
cwd = C:\data\projects\wire\wire-compiler\.
cwd = C:\data\projects\wire\wire-compiler\.
cwd = C:\data\projects\wire\wire-compiler\.
cwd = C:\data\projects\wire\wire-compiler\.
cwd = C:\data\projects\wire\wire-compiler\.
cwd = C:\data\projects\wire\wire-compiler\.
cwd = C:\data\projects\wire\wire-compiler\.
Tests run: 53, Failures: 53, Errors: 0, Skipped: 0, Time elapsed: 0.248 sec <<< FAILURE!

Results :

Failed tests:   testCorrect(com.squareup.wire.WireCompilerErrorTest)
  testZeroTag(com.squareup.wire.WireCompilerErrorTest)
  testEnumNamespace(com.squareup.wire.WireCompilerErrorTest)
  testNoPackageNameIsLegal(com.squareup.wire.WireCompilerErrorTest)
  testDuplicateTag(com.squareup.wire.WireCompilerErrorTest)
  testSimpleServiceOption(com.squareup.wire.WireCompilerTest): expected:<...ntime/src/test/proto[/]request_response.pro...> but was:<...ntime/src/test/proto[\]request_response.pro...>
  testCustomOptionsNoOptions(com.squareup.wire.WireCompilerTest): expected:<0> but was:<1>
  testCustomOptionsNoOptions(com.squareup.wire.WireCompilerTest)
  testCustomOptions(com.squareup.wire.WireCompilerTest): expected:<0> but was:<1>
  testCustomOptions(com.squareup.wire.WireCompilerTest)
  testSimpleService(com.squareup.wire.WireCompilerTest): expected:<0> but was:<1>
  testSimpleService(com.squareup.wire.WireCompilerTest)
  testChildPackage(com.squareup.wire.WireCompilerTest): expected:<0> but was:<1>
  testChildPackage(com.squareup.wire.WireCompilerTest)
  testServiceRoots2(com.squareup.wire.WireCompilerTest): expected:<0> but was:<1>
  testServiceRoots2(com.squareup.wire.WireCompilerTest)
  testRxJavaService(com.squareup.wire.WireCompilerTest): expected:<0> but was:<1>
  testRxJavaService(com.squareup.wire.WireCompilerTest)
  testSingleLevel(com.squareup.wire.WireCompilerTest): expected:<0> but was:<1>
  testSingleLevel(com.squareup.wire.WireCompilerTest)
  testSameBasename(com.squareup.wire.WireCompilerTest): expected:<0> but was:<1>
  testSameBasename(com.squareup.wire.WireCompilerTest)
  testUnknownFields(com.squareup.wire.WireCompilerTest): expected:<0> but was:<1>
  testUnknownFields(com.squareup.wire.WireCompilerTest)
  sanitizeJavadocWrapsSeeLinks(com.squareup.wire.WireCompilerTest): expected:<0> but was:<1>
  sanitizeJavadocWrapsSeeLinks(com.squareup.wire.WireCompilerTest)
  testRetrofitService(com.squareup.wire.WireCompilerTest): expected:<0> but was:<1>
  testRetrofitService(com.squareup.wire.WireCompilerTest)
  testServiceRoots1(com.squareup.wire.WireCompilerTest): expected:<0> but was:<1>
  testServiceRoots1(com.squareup.wire.WireCompilerTest)
  testAllTypes(com.squareup.wire.WireCompilerTest): expected:<0> but was:<1>
  testAllTypes(com.squareup.wire.WireCompilerTest)
  testRootsB(com.squareup.wire.WireCompilerTest): expected:<0> but was:<1>
  testRootsB(com.squareup.wire.WireCompilerTest)
  testRootsE(com.squareup.wire.WireCompilerTest): expected:<0> but was:<1>
  testRootsE(com.squareup.wire.WireCompilerTest)
  testRegistry(com.squareup.wire.WireCompilerTest): expected:<0> but was:<1>
  testRegistry(com.squareup.wire.WireCompilerTest)
  testEdgeCases(com.squareup.wire.WireCompilerTest): expected:<0> but was:<1>
  testEdgeCases(com.squareup.wire.WireCompilerTest)
  testRedacted(com.squareup.wire.WireCompilerTest): expected:<0> but was:<1>
  testRedacted(com.squareup.wire.WireCompilerTest)
  testSimple(com.squareup.wire.WireCompilerTest): expected:<0> but was:<1>
  testSimple(com.squareup.wire.WireCompilerTest)
  testNoRoots(com.squareup.wire.WireCompilerTest): expected:<0> but was:<1>
  testNoRoots(com.squareup.wire.WireCompilerTest)
  testRootsI(com.squareup.wire.WireCompilerTest): expected:<0> but was:<1>
  testRootsI(com.squareup.wire.WireCompilerTest)
  testRootsH(com.squareup.wire.WireCompilerTest): expected:<0> but was:<1>
  testRootsH(com.squareup.wire.WireCompilerTest)
  testPerson(com.squareup.wire.WireCompilerTest): expected:<0> but was:<1>
  testPerson(com.squareup.wire.WireCompilerTest)
  testRootsA(com.squareup.wire.WireCompilerTest): expected:<0> but was:<1>
  testRootsA(com.squareup.wire.WireCompilerTest)
  sanitizeJavadocGuardsFormatCharacters(com.squareup.wire.WireCompilerTest): expected:<0> but was:<1>
  sanitizeJavadocGuardsFormatCharacters(com.squareup.wire.WireCompilerTest)
  sanitizeJavadocStripsTrailingWhitespace(com.squareup.wire.WireCompilerTest): expected:<0> but was:<1>
  sanitizeJavadocStripsTrailingWhitespace(com.squareup.wire.WireCompilerTest)

Tests run: 58, Failures: 58, Errors: 0, Skipped: 0

[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary:
[INFO]
[INFO] Wire Protocol Buffers ............................. SUCCESS [2.086s]
[INFO] Wire Protocol Buffer Runtime ...................... SUCCESS [5.133s]
[INFO] Wire Protocol Buffer Compiler ..................... FAILURE [0.833s]
[INFO] Wire Protocol Buffer Gson Support ................. SKIPPED
[INFO] Wire Protocol Buffer Maven Plugin ................. SKIPPED
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 8.303s
[INFO] Finished at: Wed May 28 10:11:59 EDT 2014
[INFO] Final Memory: 23M/342M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.10:test (default-test) on project wire-compiler: There are test failures.
[ERROR]
[ERROR] Please refer to C:\data\projects\wire\wire-compiler\target\surefire-reports for the individual test results.
[ERROR] -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
[ERROR]
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR]   mvn <goals> -rf :wire-compiler

@danrice67
Copy link
Collaborator Author

@ocean7 there are many places that we assume a file separator character of /. I don't have access to a windows machine, but I am willing to send you patches if you would be willing to try them out and send back the output.

@ocean7
Copy link

ocean7 commented May 28, 2014

Sure would be happy to try and get this fixed. (Would really like to use ctors for message construction.) Do you want to create a branch that I can clone and build on?

I deleted WireCompilerTest.java and WireCompilerErrorTest.java and was able to build and install. I still get this problem where the generated code is trying to call to a Message constructor that doesn't exist. For example I have a message:

  private DefineTermCommandMessage(Builder builder) {
    super(builder);

The super(builder) call has a compiler error:

The constructor Message(DefineTermCommandMessage.Builder) is undefined

The Message class in 1.5.1-SNAPSHOT has no ctor that takes a constructor. Instead (looking at the bytecode in Eclipse) it seems to have a default ctor and a protected setBuilder(Builder) method. So for some reason (putting aside the test that I can just delete) the maven plugin is still generating uncompilable code.

@ocean7 ocean7 mentioned this pull request May 28, 2014
Closed
@danrice67
Copy link
Collaborator Author

@ocean7 can you make sure all your old generated code has been removed before building? We should not be generating a call to super(builder) anymore.

Try building with the flag -Dmaven.test.skip=true. Then maven will not compile or run the test code.

@ocean7
Copy link

ocean7 commented May 28, 2014

Code generation itself is failing with messages like:

[ERROR] ..\api\DefineTermCommandMessage.java:[34,4] error: constructor Message in class Message cannot be applied to given types;

So it seems to be generating that code. The pom file looks like:

<dependencies>
        <dependency>
            <groupId>com.squareup.wire</groupId>
            <artifactId>wire-compiler</artifactId>
            <version>1.5.1-SNAPSHOT</version>
        </dependency>
        <dependency>
            <groupId>com.squareup.wire</groupId>
            <artifactId>wire-runtime</artifactId>
            <version>1.5.1-SNAPSHOT</version>
        </dependency>
    </dependencies>

    <build>

        <pluginManagement>
            <plugins>
                <plugin>
                    <groupId>org.eclipse.m2e</groupId>
                    <artifactId>lifecycle-mapping</artifactId>
                    <version>1.0.0</version>
                    <configuration>
                        <lifecycleMappingMetadata>
                            <pluginExecutions>
                                <pluginExecution>
                                    <pluginExecutionFilter>
                                        <groupId>com.squareup.wire</groupId>
                                        <artifactId>wire-maven-plugin</artifactId>
                                        <versionRange>1.5.1-SNAPSHOT</versionRange>
                                        <goals>
                                            <goal>generate-sources</goal>
                                        </goals>
                                    </pluginExecutionFilter>
                                    <action>
                                        <execute>
                                            <runOnIncremental>true</runOnIncremental>
                                        </execute>
                                    </action>
                                </pluginExecution>
                            </pluginExecutions>
                        </lifecycleMappingMetadata>
                    </configuration>
                </plugin>
            </plugins>
        </pluginManagement>

        <plugins>

            <plugin>
                <groupId>org.apache.maven.plugins</groupId>
                <artifactId>maven-compiler-plugin</artifactId>
                <configuration>
                    <source>1.7</source>
                    <target>1.7</target>
                    <showDeprecation>true</showDeprecation>
                    <showWarnings>true</showWarnings>
                    <fork>true</fork>
                </configuration>
            </plugin>

            <plugin>
                <groupId>com.squareup.wire</groupId>
                <artifactId>wire-maven-plugin</artifactId>
                <executions>
                    <execution>
                        <phase>generate-sources</phase>
                        <goals>
                            <goal>generate-sources</goal>
                        </goals>
                        <configuration>
                            <protoFiles>
                                <param>dictionary/dictionary.proto</param>
                            </protoFiles>
                        </configuration>
                    </execution>
                </executions>
            </plugin>

            <plugin>
                <groupId>org.codehaus.mojo</groupId>
                <artifactId>build-helper-maven-plugin</artifactId>
                <executions>
                    <execution>
                        <id>add-source</id>
                        <phase>generate-sources</phase>
                        <goals>
                            <goal>add-source</goal>
                        </goals>
                        <configuration>
                            <sources>
                                <source>${project.build.directory}/generated-sources/wire/</source>
                            </sources>
                        </configuration>
                    </execution>
                </executions>
            </plugin>

Nothing terribly special...

@danrice67
Copy link
Collaborator Author

@ocean7 would you mind testing out the rice/windows_compatibility branch to see if the tests run.

I'll try out your pom to see if I can reproduce the error above.

@danrice67
Copy link
Collaborator Author

@ocean7 can you send me the whole pom.xml as well as your dictionary.proto file? You can email to rice@squareup.com.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Expose Constructors For Direct Initialization
4 participants
@danrice67 @JakeWharton @codefromthecrypt @ocean7