couchdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Randall Leeds <randall.le...@gmail.com>
Subject Re: svn commit: r1133319 - /couchdb/trunk/src/ejson/Makefile.am
Date Thu, 09 Jun 2011 01:37:46 GMT
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.

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