Hi, When I reached out to the Apache Yetus dev mailing list for some information regarding the addition of precommit check. I got the reply from them which said that AVRO might want to change the approach of which they run the tests. I wanted to know if everyone was okay with what was being suggested before I go around trying to make it work.
What is the opinion of people ? ---------- Forwarded message ---------- > From: Allen Wittenauer <[email protected]> > Date: Tue, Sep 13, 2016 at 8:13 AM > Subject: Re: Avro Pre-commit. > To: [email protected] > > > I've been thinking about this issue. With a bit of work, I think the > approach should probably be to: > a) make a custom build tool that knows which dirs use which other > tools > b) add a new file in each dir that can tell precommit where to > break for modules > > For example, let's say we have a dir structure like this: > > build.sh > c/ > Makefile > java/ > pom.xml > perl/ > Makefile.PL > > Let's add a file called multibuild.sh that precommit can use: > > build.sh > multibuild.sh > c/ > Makefile > multibuild.sh > java/ > pom.xml > multibuild.sh > perl/ > Makefile.PL > multibuild.sh > > > In our "multibuild" build tool plugin, we'd define > > function multibuild_buildfile > { > echo "multibuild.sh" > } > > Now when precommit gets a patch, it will know that a change in c/ > is a different module than a change in java/ or perl/ because of the > presence of the multibuild.sh file. This prevents the "build the world" > problem for a largely diverse project like avro. > > The place where this gets hard is our theoretical > multibuild_executor function because precommit doesn't pass as a param the > module that's currently being processed. (we should probably fix that) But > we can still make it work and reality, it might have led us down a false > path. If we make this function look like this: > > function multibuild_executor > { > echo "multibuild.sh" > } > > and make sure that BUILDTOOLCWD=module (the default), then precommit will > run our file that we placed to actually do the work. This file can then > take whatever parameters we're getting passed and convert it as appropriate > to the build tool for that directory. Since this is a custom build tool, > you can dictate what parameters to pass to which modules via > _modules_workers. For example, an easy route might be: > > function multibuild_modules_workers > { > modules_workers $2 > } > > then multibuild.sh for each language becomes a series of case statements. > For C, it might look like: > > testname=$1 > shift > > case ${testname} in > compile) > make -Dcustomflag=1 > ;; > doc) > make man > unit) > make -Danotherflag=1 test > ;; > *) > ;; > esac > > It's pretty hacky, but theoretically it would work until we get a > chance to make precommit smarter about multiple build tools in a single > project. (which would be a pretty hard undertaking, but something we should > probably do) > > > > On Sep 10, 2016, at 6:32 PM, suraj acharya <[email protected]> wrote: > > > > Yes. > > The ./build.sh test > > <https://github.com/apache/avro/blob/master/build.sh#L40> command helps > run > > all the tests from the top level. > > However, I am not sure if we need to run all the tests when a change is > > made? > > I was thinking of running language specific tests for changes in the > > specific directory. > > For example for changes in python will trigger ant test > > <https://github.com/apache/avro/blob/master/build.sh#L43>. > -Suraj Acharya On Fri, Aug 19, 2016 at 9:54 AM, Sean Busbey <[email protected]> wrote: > If we have files that fail to include the required ASF licensing > headers, you're correct that's important to fix. > > I think either working on it before or after would be fine. Ideally, > Yetus Precommit should properly let us know when a patch is fixing > those kinds of problems, so it would be a good validation that things > are working. > > On Thu, Aug 18, 2016 at 8:18 PM, suraj acharya <[email protected]> > wrote: > > Okay. I will look at the docker command and the image. > > > > One other question I have is that the asflicense is an important aspect > of > > yetus. However, many java files have that missing. And whenever a patch > > touches that file it returns a -1. Do you think I should first fix all > the > > licenses and then continue with this? > > > > S > > > > -Suraj Acharya > > > > On Thu, Aug 18, 2016 at 12:37 PM, Sean Busbey <[email protected]> > wrote: > > > >> is the 3-5 minutes doing the tests across all of the language > >> libraries or just the java ones? > >> > >> docker will definitely be needed, due to the number of different > >> system dependencies needed to build the different languages. > >> I was hoping we could reuse the Docker image that is currently > >> used for the "./build.sh docker" command. > >> > >> > >> On Wed, Aug 17, 2016 at 6:41 PM, suraj acharya <[email protected]> > >> wrote: > >> > Hi, > >> > I am a new guy and I have been working on setting up pre-commit check > for > >> > Avro : AVRO-1887 <https://issues.apache.org/jira/browse/AVRO-1887>. > >> > > >> > I was looking at the unit tests being run currently. It takes > somewhere > >> > around 3-5 minutes depending on the network and the repo's present. > >> > I was thinking that since the time is so low why not run all the tests > >> for > >> > every commit. Also, the tests being lightweight I was thinking that > there > >> > is no need for docker to be needed. > >> > > >> > I would like to gather your opinions on this topic before going > further > >> > down this path. > >> > > >> > Thanks > >> > > >> > -Suraj Acharya > >> > >> > >> > >> -- > >> busbey > >> > > > > -- > busbey >
