> On June 25, 2012, 8:27 p.m., Hari Shreedharan wrote: > > Pathc applies cleanly. Hit some issues, but looks ok. Here are the issues I > > faced: > > * mvn gets permission denied for saveVersion.sh. So I had to manually set > > the permissions on this file: > > [ERROR] Failed to execute goal > > org.codehaus.mojo:exec-maven-plugin:1.2.1:exec (generate-version) on > > project flume-ng-core: Command execution failed. Cannot run program > > "scripts/saveVersion.sh" (in directory > > "/Users/hshreedharan/work/flume-latest/flume-svn/flume-ng-core"): error=13, > > Permission denied -> [Help 1] > > > > - Could you please make this executable by default? > > > > * VersionAnnotation.java is missing the license header, so RAT complains. > > > > * md5sum does not exist by default on Mac OS X, it is simply called md5 and > > gives output as: MD5 (bin/flume-ng) = 03a82dc344d478e53d4e011a942ba12b. > > Since Mac OS is a supported OS, we need to fix this or omit md5 sum for now. > > * The version command prints the repo name which is local for the git > > mirror: > > Subversion > > git://Haris-MacBook-Pro.local/Users/hshreedharan/work/flume-latest/flume -r > > 62e135c09e99b94a138e3a67260d48df22fa1c23 - would it be possible to make > > this print the actual Apache repo? > > > > For the subversion repo, it does not print anything at all: > > ./flume-ng version > > Flume 1.2.0-incubating-SNAPSHOT > > Subversion -r 1353431 > > Compiled by hshreedharan on Mon Jun 25 13:22:34 PDT 2012 > > From source with checksum > > > > Most of these seem like minor issues, especially first 2. I am ok with > > ignoring the 3rd one for now. I am not sure about the fourth one, is it > > very hard to fix?
Hi, Hari thanks for your careful help. 1. Before I do patch, I have setup saveVersion.sh script to execute permission like below. You maybe figure out a potential defect here. I need to use Maven Assembly Plugin assembly descriptor to make it execute on different OS. -rwxr-xr-x 1 leslin leslin 2376 2012-06-25 20:52 saveVersion.sh 2. license added 3. md5sum, I will add judge logic for Mac OS X if need -------------- 4. Is it reasonable to point to Apache repository? The version info here is for flume built env. info. IMO, it should reflect the actual image when user built them. If it's built on Apache host, it will reflect Apache repository. For example, if user setup his own flume image and change code or add his own patch, the build is different from his own repository instead of Apache repo? If all point to base repository(Apache here), it will be easy and we just do some hard code for it. What's your opinion? For SVN repository, I will test it again BTW, how can you run flume-ng as there is mvn dependency error here? Thank you Regards Leslin - Leslin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5463/#review8564 ----------------------------------------------------------- On June 25, 2012, 2:20 p.m., Leslin (Hong Xiang Lin) wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5463/ > ----------------------------------------------------------- > > (Updated June 25, 2012, 2:20 p.m.) > > > Review request for Flume. > > > Description > ------- > > Add feature to let flume-ng get version info: > eslin@ubtServer:/usr/lib/flume-ng/bin$ flume-ng version > Apache Flume version: 1.2.0-incubating-SNAPSHOT > Built-By: 1.2.0-incubating-SNAPSHOT > > > This addresses bug FLUME-1240. > https://issues.apache.org/jira/browse/FLUME-1240 > > > Diffs > ----- > > bin/flume-ng 43d1766 > flume-ng-core/pom.xml e4c8104 > flume-ng-core/scripts/saveVersion.sh PRE-CREATION > flume-ng-core/src/main/java/org/apache/flume/VersionAnnotation.java > PRE-CREATION > flume-ng-core/src/main/java/org/apache/flume/tools/VersionInfo.java > PRE-CREATION > flume-ng-core/src/test/java/org/apache/flume/tools/TestVersionInfo.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/5463/diff/ > > > Testing > ------- > > Test with: > flume-ng version > flume-ng agent -n agent -f /usr/lib/flume-ng/conf/flume-conf.properties > > > Thanks, > > Leslin (Hong Xiang Lin) > >
