ZOOKEEPER-3423:use the maven-like way to ignore the generated version java files and doc the cmd:'./zkServer.sh version' by maoling · Pull Request #980 · apache/zookeeper

maoling · 2019-06-11T06:06:54Z

more details in the ZOOKEEPER-3423

eolivelli · 2019-06-14T07:13:30Z

I will wait for @nkalmar feedback

Another maven-like approach would be to create such files into ${project.build.directory}/generated-sources/versioninfo

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

phunt · 2019-06-15T19:53:22Z

.gitignore

 zookeeper-client/zookeeper-client-c/ltmain.sh
 zookeeper-client/zookeeper-client-c/missing
 zookeeper-server/src/main/java/org/apache/zookeeper/version/Info.java
+zookeeper-server/src/main/java/org/apache/zookeeper/version/VersionInfoMain.java


I just noticed this issue during a build, good to fix. I agree with Norbert - this should be part of "generate-sources" phase/directory.

maoling · 2019-06-17T01:39:47Z

@eolivelli Could you plz offer some help? :D

maoling · 2019-06-18T03:33:35Z

Ant PreCommit has failed which will be discarded? this PR works well in my local.

eolivelli

Sorry for late reply.

Your change is perfect!
I left a just a comment. Please take a look.

In theory I have disabled the ant precommit but in practice it is still kicking in.
Do not consider it

eolivelli · 2019-06-18T06:44:36Z

zookeeper-server/src/main/java/org/apache/zookeeper/version/util/VerGen.java

                rev = rev.trim();
            }
-            generateFile(new File("."), version, rev, args[2]);
+            generateFile(new File("../../../target/generated-sources/java"), version, rev, args[2]);


It is better to have this path passed as a parameter, this way it will be easier to drive this code from the pom.xml

szepet · 2019-06-18T12:02:21Z

Neat, thanks for the documentation! :)

phunt · 2019-06-21T02:51:38Z

I'm afraid this causes the ant build to fail. "ant clean compile-test" failed with compilation errors.

…:'./zkServer.sh version'

maoling · 2019-06-21T03:12:27Z

Running ant clean compile-test in my local has got BUILD SUCCESSFUL. not failed?

anmolnar · 2019-06-22T21:08:16Z

retest this please

anmolnar · 2019-06-22T21:14:34Z

I also have compile errors with Ant:

    [javac] /Users/andormolnar/git/my-zookeeper/zookeeper-server/src/main/java/org/apache/zookeeper/Version.java:24: error: package org.apache.zookeeper.version does not exist
    [javac] public class Version implements org.apache.zookeeper.version.Info {
    [javac]                                                             ^
    [javac] /Users/andormolnar/git/my-zookeeper/zookeeper-server/src/main/java/org/apache/zookeeper/Version.java:34: error: cannot find symbol
    [javac]         return REVISION;
...

Though I've just disabled the Ant precommit job on master, so we won't see it running again.

eolivelli · 2019-06-22T22:50:12Z

@maoling please fix it, it should be in the configuration of javac in build.xml you have to add the new directory

anmolnar · 2019-06-23T16:04:12Z

retest ant build

anmolnar · 2019-06-23T18:05:48Z

retest ant build

nkalmar

Sorry for the late entry.
This is looking good now, +1 from my side.

nkalmar · 2019-06-24T08:47:12Z

pinging @eolivelli because of his -1 on this. (I believe all concerns are fixed now).

eolivelli

Sure #shipit

nkalmar · 2019-06-26T08:36:09Z

Committed to master, thanks @maoling

…n java files and doc the cmd:'./zkServer.sh version' - more details in the[ ZOOKEEPER-3423](https://issues.apache.org/jira/browse/ZOOKEEPER-3423) Author: maoling <maoling199210191@sina.com> Reviewers: Enrico Olivelli <eolivelli@apache.org>, Norbert Kalmar <nkalmar@apache.org> Closes apache#980 from maoling/ZOOKEEPER-3423

maoling force-pushed the ZOOKEEPER-3423 branch from 9b3b108 to ba1e594 Compare June 14, 2019 02:26

eolivelli approved these changes Jun 14, 2019

View reviewed changes

eolivelli requested a review from nkalmar June 14, 2019 07:11

phunt reviewed Jun 15, 2019

View reviewed changes

maoling changed the title ~~ZOOKEEPER-3423:add VersionInfoMain.java to .gitignore and doc the cmd:'./zkServer.sh version'~~ ZOOKEEPER-3423:use the maven-like way to ignore the generated version java files and doc the cmd:'./zkServer.sh version' Jun 16, 2019

maoling force-pushed the ZOOKEEPER-3423 branch 2 times, most recently from c2f9a2f to caed68e Compare June 16, 2019 10:20

maoling force-pushed the ZOOKEEPER-3423 branch from cb17b7e to caed68e Compare June 18, 2019 03:34

eolivelli requested changes Jun 18, 2019

View reviewed changes

maoling added 2 commits June 21, 2019 11:10

ZOOKEEPER-3423:add VersionInfoMain.java to .gitignore and doc the cmd…

101064a

…:'./zkServer.sh version'

use the maven-like way to ignore the generated version java files

c5301c0

make the version's outputDirectory parameterized

4e11fc0

maoling force-pushed the ZOOKEEPER-3423 branch from 47ce0c4 to 4e11fc0 Compare June 23, 2019 10:32

make the ant generate the version files in the newOutputDir

4cecaf9

nkalmar approved these changes Jun 24, 2019

View reviewed changes

eolivelli approved these changes Jun 24, 2019

View reviewed changes

asfgit closed this in 4c65069 Jun 26, 2019

maoling deleted the ZOOKEEPER-3423 branch June 26, 2019 08:44

maoling mentioned this pull request Jan 28, 2022

Backport ZOOKEEPER-3423 to Branch 3.5 #1803

Closed

maoling restored the ZOOKEEPER-3423 branch September 2, 2022 11:55

Conversation

maoling commented Jun 11, 2019

Uh oh!

eolivelli commented Jun 14, 2019

Uh oh!

phunt Jun 15, 2019

Choose a reason for hiding this comment

Uh oh!

maoling commented Jun 17, 2019

Uh oh!

maoling commented Jun 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eolivelli left a comment

Choose a reason for hiding this comment

Uh oh!

eolivelli Jun 18, 2019

Choose a reason for hiding this comment

Uh oh!

szepet commented Jun 18, 2019

Uh oh!

phunt commented Jun 21, 2019

Uh oh!

maoling commented Jun 21, 2019

Uh oh!

anmolnar commented Jun 22, 2019

Uh oh!

anmolnar commented Jun 22, 2019

Uh oh!

eolivelli commented Jun 22, 2019

Uh oh!

anmolnar commented Jun 23, 2019

Uh oh!

anmolnar commented Jun 23, 2019

Uh oh!

nkalmar left a comment

Choose a reason for hiding this comment

Uh oh!

nkalmar commented Jun 24, 2019

Uh oh!

eolivelli left a comment

Choose a reason for hiding this comment

Uh oh!

nkalmar commented Jun 26, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

maoling commented Jun 18, 2019 •

edited

Loading