couchdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Paul J. Davis" <paul.joseph.da...@gmail.com>
Subject Re: svn commit: r1133319 - /couchdb/trunk/src/ejson/Makefile.am
Date Thu, 09 Jun 2011 02:05:03 GMT




On Jun 8, 2011, at 9:37 PM, Randall Leeds <randall.leeds@gmail.com> wrote:

> On Wed, Jun 8, 2011 at 17:05, Paul Davis <paul.joseph.davis@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.
> 

That flag really wants to have /* */ style comments to compile? Huh. I'd never seen such a
thing. I did see the flag. Had I connected the two I wouldn't have batted an eye. Also, Bruins
are up 3-0.

> 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.leeds@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.davis@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.davis@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.leeds@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" <fdmanana@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.davis@gmail.com
>>>> 
>>>>>> wrote:
>>>>>>>> On Wed, Jun 8, 2011 at 11:26 AM, Randall Leeds <
>>> randall.leeds@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" <fdmanana@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.davis@gmail.com
>>>>>>> 
>>>>>>>>> wrote:
>>>>>>>>>>> On Wed, Jun 8, 2011 at 10:20 AM, Robert Dionne
>>>>>>>>>>> <dionne@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,  <randall@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,
>>>>>>>>>> fdmanana@gmail.com, fdmanana@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,
>>>>>>> fdmanana@gmail.com, fdmanana@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."
>>>>>> 
>>>>> 
>>> 
>> 

Mime
View raw message