On 9/4/07, Chris Frey <[EMAIL PROTECTED]> wrote: > Thanks for the patch! I've taken a look at it, and there are a couple > things I'm not keen on. If you send me a patch that addresses these > issues, I'll probably apply it. :-) You've fixed a number of configure > problems and made it more robust, which is great. > > First, in the debian/rules makefile, you changed: > > - $(MAKE) distclean > - (cd gui && $(MAKE) distclean) > + -$(MAKE) distclean > + -(cd gui && $(MAKE) distclean) > > I'd rather fix the cause of the make errors than ignore them. > Why did you need the - prefixes?
This is actually standard practice with command make distclean, because it only works when the makefiles are present. The clean target shouldn't fail because configure hasn't been run yet, or because the target was run twice. > Second, the --enable-opensync switch doesn't work for me. I had > to use --enable-opensync-plugin which was different from the --help > output. > Third, if you're adding a build dependency to the debian scripts > for libopensync-dev, I'd like it to match version 0.22 or greater. > What system are you testing this on, btw? oops, I thought I took that out - I added it while adding the gtkmm and glademm deps, even though it shouldn't be there until the debian package actually builds an opensync plugin. I'm testing this on Ubuntu 7.04, but I can test that builds work on other versions of debian using pbuilder. > > There are two things I'm not happy with: I had to put a set of > > variable exports in configure.ac when a subdirectory is enabled, so > > that they build and link against the source tree - I couldn't find a > > more elegant way to get that behaviour. > > I'm not sure either, but I suspect the answer might be in other > projects that have a library portion to them. I couldn't find any other way to pass information to the nested configure scripts. The only other thing I can think of would be to split them up into separate source trees, but I don't see the point of doing that. It may be hard to find projects with a similar layout and dependency structure. > > > The other thing is that when > > the subdirectories aren't enabled (and they aren't by default), they > > won't be included in make dist. I figure this is ok since you don't > > use make dist to generate your release tarballs anyway, but I added a > > check that will warn prominently when the conditionally enabled > > subdirectories are missing from the dist. According to the > > documentation, the only other workaround is to manually include them > > in the dist instead of echoing a warning. > > A hard failure would be better, along with the warning. Something > that stops the build process, so that it never accidentally gets > run in some automated build process. I considered that, but I figured that since make dist will only ever get used to generate release tarballs, (and right now, you seem to be using a script to do that instead) a warning would be ok as long as it's very visible. I'll change it though. I'll send a revised patch sometime in the next day. ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ Barry-devel mailing list Barry-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/barry-devel
