On Wed, Jun 8, 2011 at 17:05, Paul Davis <paul.joseph.da...@gmail.com> wrote: > Conform to what? > > Also, I don't mind but we do generally have a policy of separating > behavior changes from formatting changes so that its easier to sift > through commits and mentally ignore anything that is listed as > "whitespace cleanup" or similar.
The commit in question (http://svn.apache.org/viewvc?rev=1133287&view=rev) [not the one in this subject] is to use the variables from AC_CHECK_ICU to fill in the compile flags for the erlang icu driver. As it says in the message, on my machine, the CFLAGS include "-ansi". It is conforming to this flag which prompted me to make the comment style change. And yes, you're right and I'm aware, separate commits for whitespace and cleanup. In this case I actually had them as separate commits locally and squashed them because I wanted to put the justification alongside the cleanup itself and since it was a result of the change to icu driver compilation/linkage I bundled it together and wrote what (I thought) was a sufficiently explanatory commit. Backfired, it seems :). > > On Wed, Jun 8, 2011 at 2:16 PM, Randall Leeds <randall.le...@gmail.com> wrote: >> Read the commit log for http://svn.apache.org/viewvc?rev=1133287&view=rev >> On Jun 8, 2011 10:33 AM, "Paul Davis" <paul.joseph.da...@gmail.com> wrote: >>> What are all these hunks that are changing the comment syntax? >>> >>> >> https://github.com/davisp/couchdb/commit/6ceefeb6d142b995d9a0fe578aac88105d2a0917#L2R16 >>> >>> On Wed, Jun 8, 2011 at 12:49 PM, Paul Davis <paul.joseph.da...@gmail.com> >> wrote: >>>> I'm getting on a train in a few minutes. I'll go through and review >>>> what you did and see if I can't figure out what's going on. May or may >>>> not have internet while traveling but I should be back online in about >>>> 1.5 hours. >>>> >>>> On Wed, Jun 8, 2011 at 12:46 PM, Randall Leeds <randall.le...@gmail.com> >> wrote: >>>>> Yeah, this commit was meant to fix it when bob dionne noticed it >> breaking >>>>> his build, but rnewon did not. Sorry I went to sleep with a broken build >>>>> there. >>>>> >>>>> Can you try CXX or CPP flags for that line instead and re-bootstrap? >>>>> On Jun 8, 2011 8:40 AM, "Filipe David Manana" <fdman...@apache.org> >> wrote: >>>>>> It happens even with a fresh checkout from git:// >>>>> git.apache.org/couchdb.git >>>>>> >>>>>> Reverting the addition of ERLANG_FLAGS to snappy's Makefile.am doesn't >>>>>> help at all, so it must be one of the previous commits that touches >>>>>> the autotools config (ejson builds fine however) >>>>>> >>>>>> On Wed, Jun 8, 2011 at 4:28 PM, Paul Davis <paul.joseph.da...@gmail.com >>> >>>>> wrote: >>>>>>> On Wed, Jun 8, 2011 at 11:26 AM, Randall Leeds < >> randall.le...@gmail.com> >>>>> wrote: >>>>>>>> Strange. >>>>>>>> On my machine that command includes -I for erlang includes. >>>>>>>> >>>>>>>> Paul, that's ERLANG_FLAGS as set by configure, not ERL_FLAGS. >>>>>>>> Does it help if you switch it to CXX or CPP? Maybe your systems are >>>>> stricter >>>>>>>> about using those variables for the .cc based stuff. >>>>>>>> >>>>>>>> Look in that folder's generated Makefile. Does ERLANG_FLAGS have info >>>>> for >>>>>>>> finding erl_nif.h? What's that make target have for variables and >> does >>>>> it >>>>>>>> include the la_CFLAGS automake is supposed to have stuck in there? >> And >>>>> does >>>>>>>> that include ERLANG_FLAGS? >>>>>>>> >>>>>>>> Sorry for breaking this for you. Thanks for your help. >>>>>>> >>>>>>> Huh. Maybe everyone just needs to re-bootstrap? >>>>>>> >>>>>>> I haven't had a chance to get to look at it myself. I was just >>>>>>> confused by ERL_FLAGS vs ERLANG_FLAGS. >>>>>>> >>>>>>>> On Jun 8, 2011 7:24 AM, "Filipe David Manana" <fdman...@apache.org> >>>>> wrote: >>>>>>>>> Breaks my build (make dev) as well: >>>>>>>>> >>>>>>>>> make[3]: Entering directory >> `/home/fdmanana/git/hub/couchdb/src/snappy' >>>>>>>>> /bin/bash ../../libtool --tag=CXX --mode=compile g++ -DHAVE_CONFIG_H >>>>>>>>> -I. -I../.. -I../../src/snappy/google-snappy -D_XOPEN_SOURCE -g >>>>>>>>> -O2 -MT snappy_nif.lo -MD -MP -MF .deps/snappy_nif.Tpo -c -o >>>>>>>>> snappy_nif.lo snappy_nif.cc >>>>>>>>> libtool: compile: g++ -DHAVE_CONFIG_H -I. -I../.. >>>>>>>>> -I../../src/snappy/google-snappy -D_XOPEN_SOURCE -g -O2 -MT >>>>>>>>> snappy_nif.lo -MD -MP -MF .deps/snappy_nif.Tpo -c snappy_nif.cc >> -fPIC >>>>>>>>> -DPIC -o .libs/snappy_nif.o >>>>>>>>> In file included from snappy_nif.cc:21: >>>>>>>>> erl_nif_compat.h:27: fatal error: erl_nif.h: No such file or >> directory >>>>>>>>> compilation terminated. >>>>>>>>> make[3]: *** [snappy_nif.lo] Error 1 >>>>>>>>> >>>>>>>>> >>>>>>>>> On Wed, Jun 8, 2011 at 3:21 PM, Paul Davis < >> paul.joseph.da...@gmail.com >>>>>> >>>>>>>> wrote: >>>>>>>>>> On Wed, Jun 8, 2011 at 10:20 AM, Robert Dionne >>>>>>>>>> <dio...@dionne-associates.com> wrote: >>>>>>>>>>> well it breaks my build :) >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> On Jun 8, 2011, at 10:15 AM, Paul Davis wrote: >>>>>>>>>>> >>>>>>>>>>>> On Wed, Jun 8, 2011 at 5:55 AM, <rand...@apache.org> wrote: >>>>>>>>>>>>> Author: randall >>>>>>>>>>>>> Date: Wed Jun 8 09:55:00 2011 >>>>>>>>>>>>> New Revision: 1133319 >>>>>>>>>>>>> >>>>>>>>>>>>> URL: http://svn.apache.org/viewvc?rev=1133319&view=rev >>>>>>>>>>>>> Log: >>>>>>>>>>>>> include $(ERLANG_FLAGS) when building ejson nif >>>>>>>>>>>>> >>>>>>>>>>>>> Modified: >>>>>>>>>>>>> couchdb/trunk/src/ejson/Makefile.am >>>>>>>>>>>>> >>>>>>>>>>>>> Modified: couchdb/trunk/src/ejson/Makefile.am >>>>>>>>>>>>> URL: >>>>>>>> >>>>> >> http://svn.apache.org/viewvc/couchdb/trunk/src/ejson/Makefile.am?rev=1133319&r1=1133318&r2=1133319&view=diff >>>>>>>>>>>>> >>>>>>>> >>>>> >> ============================================================================== >>>>>>>>>>>>> --- couchdb/trunk/src/ejson/Makefile.am (original) >>>>>>>>>>>>> +++ couchdb/trunk/src/ejson/Makefile.am Wed Jun 8 09:55:00 2011 >>>>>>>>>>>>> @@ -65,6 +65,7 @@ if USE_OTP_NIFS >>>>>>>>>>>>> ejsonpriv_LTLIBRARIES = ejson.la >>>>>>>>>>>>> >>>>>>>>>>>>> ejson_la_SOURCES = $(EJSON_C_SRCS) >>>>>>>>>>>>> +ejson_la_CFLAGS = $(ERLANG_FLAGS) >>>>>>>>>>>>> ejson_la_LDFLAGS = -module -avoid-version >>>>>>>>>>>>> >>>>>>>>>>>>> if WINDOWS >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> Is this right? >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Pretty sure ERL_FLAGS is for flags for erlc which probably aren't >>>>>>>>>> gonna go so hot for gcc. Just saying is all. >>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> -- >>>>>>>>> Filipe David Manana, >>>>>>>>> fdman...@gmail.com, fdman...@apache.org >>>>>>>>> >>>>>>>>> "Reasonable men adapt themselves to the world. >>>>>>>>> Unreasonable men adapt the world to themselves. >>>>>>>>> That's why all progress depends on unreasonable men." >>>>>>>> >>>>>>> >>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> Filipe David Manana, >>>>>> fdman...@gmail.com, fdman...@apache.org >>>>>> >>>>>> "Reasonable men adapt themselves to the world. >>>>>> Unreasonable men adapt the world to themselves. >>>>>> That's why all progress depends on unreasonable men." >>>>> >>>> >> >
