> 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? > > Leslin (Hong Xiang Lin) wrote: > 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
I didn't get any dependency error. I just ran mvn clean install after applying the patch. Don't worry about the permissions - I will manually set the permissions before committing. 4. I guess it should point to the actual repo, not the local folder. Maybe you can print both? Something like this: Repository: <SVN-REPO> (I don't know if you should hard code it, Hbase and Hadoop do show the Apache repo, though I am not sure if it is hard coded - even though it is built locally). Built from: <local-repo> Either way, when using Svn it does not show the local or remote repo. - Hari ----------------------------------------------------------- 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) > >
