Sometimes, it's not a bug, but a workaround.

I found a few ';;' in the Corinthia source and thought -- aha, typo!  So, I fixed and committed, but alas...

http://www.mail-archive.com/dev@corinthia.incubator.apache.org/msg01164.html

...it's a VC++ bug, it won't do C99 without this little hack.

A little perl alias for git log messages

To get a nice log message for your git diff patch which makes it easy for a reviewer to check your stuff, try this little perl proggy:

alias gitdifflogmesg="perl -ne 'BEGIN {print \"[[[\";} /^\\+\\+\\+ b\\/(\\S+)/ && print \"\\n\\n* \$1\";  /^@\@.*[ *]([^ ]+)\(/ && print \"\\n  (\$1).\"; END { print \"\\n\\n]]]\\n\";}'"

Put this into your .bashrc (as one line), and, grab a patch you made with git diff, and use the alias like so:

cat your.patch | gitdifflogmesg

The output is something like this:

* path/to/modifed-file1.c
   (some_function_a).
   (some_function_b).

* path/to/modifed-file2.c
   (some_function_c).
   (some_function_d).

Now all you need to do is some accounting -- a good 50 char intro line and say what you did with all those functions and your log message is ready for sharing.  Of course not all log messages need this level of scrutiny, but, this goes a long way towards helping your reviewers and also, to help yourself to produce a great patch (or mailing list post).

Note that this does not pick up everything --- new #includes and new functions for example.  Then again, whilst that one liner is kinda cute, this service should really be offered by git itself.  But whilst git doesn't have this feature, this goes a long way towards making log messages easy ;-)

I am not a perl wizard though, I just read the book 'Perl One-Liners: 130 Programs That Get Things Done' -- even if you only read the first 3 chapters, you gain a handy little skill, plus, it's just fun to fiddle with.

Another failed patch

Whilst doing the long overdue merge of trunk into my invoke-diff-cmd branch, I spotted a deprecation warning that looked like a minor oversight, and so, I fixed it, svn compiled, the tests passed and ... the patch turned out to be a no-can-do, because APR_ARRAY_PUSH doesn't care what type you stuff in there.

I thought it did, because of this:

#define APR_ARRAY_PUSH(ary,type) (*((type *)apr_array_push(ary))) 

which *looks* like it's type-safe at least if you don't know what you're really looking at (and if you haven't bothered to check the underlying code to see what it actually does).  It of course does no such thing -- so, what does this trick that does seemingly nothing actually do?

Ask on #svn-dev and ye shall receive:

[19:32] <breser> gbb: The reason it takes a type is to avoid warnings.  The apr_array_push() function returns a void *.  Technically there's no issue with putting any pointer in a void pointer, however some  compilers will warn you about that since you may not be doing it intentionally.                                                        
                                                                      
[19:33] <breser> gbb: The array code doesn't care what the pointer is  to, it's up to the user of APR Arrays to keep their types straight.   
                                                                      
[19:34] <breser> gbb: In this case the fact that the array code is  taking a type (and casting) is hiding the errors/warnings you would have otherwise seen.                                                                                                                     
[20:30] <Bert> breser, gbb: Apr arrays can be of different kinds than just pointers. In some cases we have arrays of specific struct kinds  (e.g. for property changes in the diff handling) 

Thank you Ben and Bert, it certainly did clear things up, and made me look up the 'long answer' --- the very short answer of course is that C isn't C++, and so doesn't have type-safe templates.  Caveat coder!

If at first you don't succeed...

I tidied up the "comments" which are actually defines (what was I thinking?) in this file because, well, they looked messy :)

However, in this case, the position of the defines actually reflected the structure of the data -- something I have never seen before, but it's indeed very handy, once you know what you're looking at.

So, the patch then morphed from 'fixing' what was not broken to adding information about what is going on :)

Since we didn't want Doxygen to pick up that comment since it's only useful when you view the code, I used a /* plain comment */ here.

Diff is difficult.

After much staring, stepping and grepping, I finally produced a solution for the partial problem defined in (Issue 2074, "--extensions '' doesn't work.) which is part of (Issue 2044, Fully customizable external diff invocations).

It's not bad(it works for very small values of functional), but also not very good, since it is rather agricultural and I'm not sure how to handle deprecation, but, it's a start.

However it only deals with one part of the problem and not with the issue of giving the user a wormhole to call any diff function they want to use, however they want to.

To be continued... ;-)

New Subversion Web Site Changes Guide

I added a Web Site Changes guide aimed at assisting contributors with submitting patches for corrections and contents for the Subversion Website.

This was my first direct commit to Subversion , and of course I had to make the obligatory mistake, mercifully it was only in the log file and not in the live website, and so, I learned on the spot how to change the log files as well :)

Your left is there where your thumb on the right is

It's perfectly possible to be laboring under the illusion that what you're doing is working and, it also may seem that way.

The patch I posted compiled, but created a new directory (see mistake 1). I have no idea why it compiled, but it did and so I just assumed that will be correct.

This time, the moments when I didn't keep a log came back to bite me.

Mistake 1) getting confuzzled and switching prefixes -- I ended up with libgtest and gtestlib. One of the worked but was wrong; the other was correct, but broken.

Mistake 2) Littering. I left an exploration hack around which added an extra 'lib' to one of the macro expansion and ended up with liblibgtest being sought. Plus I forgot I had done it and had to sleuth for 10 mins.

Mistake 3) Not changing all occurences of gtest properly.

Mistake 4) not having a test for 'correct compile', and I didn't even consider what 'done' means...

Build System Gtest addition, next attempt

Resistance was futile... :)

Without the excellent tip to keep a log of debug activities, that I picked up from the Udacity debugging course, I probably would have given up or would still be mud wresting this thing.  (I will blog about this course next!)

The man who teaches this most excellent course is totally correct: the moment I started keeping notes instead of jamming my short term memory full of  Vogon poetry, my brain was  freed up to do some thinking, and I was able to take a break  to snatch a few moments of real life without losing my spot.

It's amazing how much work a few lines of trivial looking code can make, and mind you, whilst it 'works' currently I had to cheat a little :)

Here is my (partial, since I forgot to keep it up at some points) log, in all it's gory:

AM_M macros do not work in autconf, try autoconf --force and that will
reset (something) and make it work once, but no more.  Removed.

configure: WARNING: unrecognized options: --enable-maintainer-mode, --enable-gtest

1 Identify how many spurious warnings are generated

2 Examine Makefile for gbg_autoconf spoor

*make optional note in configure output to inform user this error
 message is spurious* it is generated in generator/gen-py and almost
 looks like code rot, is there a bug catergory for this, should this
 be queued?

Note: build-outputs.mk still have references to gtest_LDFLAGS etc
gtest/gtest-1.la: $(gtest_DEPS)
        cd gtest && $(LINK_CXX)) $(gtest_LDFLAGS) -o gtest-1.la $(LT_NO_UNDEFINED) 

        $(gtest_OBJECTS) $(LIBS)
WHY

Considering target file `all'.
 File `all' does not exist.
 Finished prerequisites of target file `all'.
Must remake target `all'.
g++ -Igtest_hack/include -Igtest_hack -c gtest_hack/src/gtest-all.cc
Putting child 0x090799a0 (all) PID 24007 on the chain.
Live child 0x090799a0 (all) PID 24007
Reaping winning child 0x090799a0 PID 24007
ar -rv libgtest.a gtest-all.o

Currently, the build system is "improving" our link command in two
ways:

1.  it is stepping into gtest (not necessarily horrible)
2.  it is adding all the objects fromthe compilation to the command line
FIX: Remove the extra compilation targets?
DONE: Change to build.conf

idea: make gtest a "exe"
Result: no change

Removed the headers from private-includes in build.conf
        gtest/include/gtest/internal/*.h
        gtest/include/gtest/*.h

This does not affect build behaviour.  Also, it can't stay there
because gtest is optional and the script crashes when it cannot find
required headers.

Did this instead in Makefile.in: GTEST_INCLUDES = -Igtest -Igtest/include

Also added a lot of stuff into Makefile.in here is the current selection:

COMPILE_GTEST_CXX = $(COMPILE_CXX) $(GTEST_INCLUDES) -o $@ -c
LT_COMPILE = $(LIBTOOL) $(LTFLAGS) --mode=compile $(COMPILE) $(LT_CFLAGS)
LT_COMPILE_CXX = $(LIBTOOL) $(LTCXXFLAGS) --mode=compile $(COMPILE_CXX) $(LT_CFLAGS)
## I don't understand why, but I need to provide "-o ... -c" in order to get the 

  object file in the correct directory                                               
LT_COMPILE_GTEST_CXX = $(LIBTOOL) $(LTCXXFLAGS) 

  --mode=compile $(COMPILE_CXX) $(LT_FLAGS) $(GTEST_INCLUDES) -o lib$@ -c

and also

LINK_GTEST_CXX = $(LIBTOOL) $(LTCXXFLAGS) --mode=link $(CXX) $(LT_LDFLAGS) 

  $(CXXFLAGS) $(LDFLAGS) -rpath $(libdir)

(forgot to document and prompty got lost and forgot what I did...)


idea: following the thread of the remake stack trace
------ completed all in apr-util

##>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
g++ -std=c++98  -D_REENTRANT -D_GNU_SOURCE -D_LARGEFILE64_SOURCE  -Wall -Wpointer-arith 

-Wwrite-strings -Wshadow -Wunused -Wunreachable-code -Wno-system-headers   \
-g3 -fno-omit-frame-pointer -fno-inline   -DSVN_DEBUG -DAP_DEBUG  -I./subversion/include 

-I./subversion -I/home/g/trunk/apr/include   -I/home/g/trunk/apr-util/incl\
ude -I/home/g/trunk/sqlite-amalgamation  -Igtest -Igtest/include 

-o gtest/src/gtest-all.lo -c  gtest/src/gtest-all.cc
##<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
##>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
cd gtest && /bin/bash /home/g/trunk/libtool --tag=CXX --silent --mode=link g++ 

-g3 -fno-omit-frame-pointer -fno-inline   -DSVN_DEBUG -DAP_DEBUG   -rpath /tmp/foo/\
lib  -o gtest-1.la  src/gtest-all.lo
##<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
libtool: link: `src/gtest-all.lo' is not a valid libtool object
build-outputs.mk:479: *** [gtest/gtest-1.la] Error 1

#0  gtest/gtest-1.la at /home/g/trunk/build-outputs.mk:479
#1  gtest at /home/g/trunk/build-outputs.mk:1472
#2  local-all at /home/g/trunk/Makefile:390
#3  all at /home/g/trunk/Makefile:382
Command-line invocation:

Problem:
by including "-o $@", the output is named ".lo" instead of ".o".  The reason
we added -o ... is to get the object file in the correct directory

TODO
should be building in the gtest directory
Ruby error : already at newest ruby, spurious warning?
get-deps.sh is giving an error message. Need to svn up at some point

Libtool won't link to a library not called libtool
Do we need to use libtool?
No, not necessarily, BUT the rest of SVN does and we would like to follow
their style

Ideas:
1.  Avoid libtool
Pluses: might work
minus: might be harder than libtool

2.  Hack gen-make.py
Pluses: ...
Minuses: serpents  will issue forth from the CPU

3.  Rename tree to libgtest
Pluses: Will work
Minuses: Kitten with ball of angora wool

4.  libtool is actually a svn shell script.  Is the restriction on
name inside the shell system?  Why was it put in the first place?
Hack it if it exists?

Pluses: it might just work
Minuses: it might not

5.  Get libtool compile command to add lib-prefix
Plus: right thing to do
test:
    Yes it could work, although it puts the lib in trunk
        TODO: Work out why/ask why it drops it in trunk
This involves changing the name of the targets all the way through the make
file

Plan
6: Rename target to libgtest
1.  update configure.ac, build.conf

removed getst from aclocl.m4 in orig trunk as a test...
test if svn still compiles w/o gtest.m4 .. yes it does, removed.

PLAN: patch & ship

patched copy, didn't work, failed with:

/bin/bash /home/g/projects/svn/trunk/libtool --tag=CXX --silent --mode=compile g++ -

std=c++98  -D_REENTRANT -D_GNU_SOURCE -D_LARGEFILE64_SOURCE   -g -O2  -I./subve\
rsion/include -I./subversion -I/home/g/projects/svn/trunk/apr/include 

-I/home/g/projects/svn/trunk/apr-util/include 

-I/home/g/projects/svn/trunk/sqlite-amalgamat\
ion   -Igtest -Igtest/include -o libgtest/src/gtest-all.lo -c  gtest/src/gtest-all.cc

cd gtest && /bin/bash /home/g/projects/svn/trunk/libtool --tag=CXX --silent 

--mode=link g++  -g -O2   -rpath /dev/null/lib  -o libgtest-1.la  src/gtest-all.lo
libtool: link: `src/gtest-all.lo' is not a valid libtool object
make: *** [gtest/libgtest-1.la] Error 1
g@musashi:~/projects/svn/trunk$ cd -

patch copy of faulty compile is the same as output of working one.

changed get_dep.sh to move unzipped gtest into libgtest instead

Added in dep-get.sh: warnings about the spurious warning about the
headers in autogen.sh and the repeated spurous warnings in ./configure
--enable-gtest,

*IT COMPILES* :)

cd libgtest && /bin/bash /home/g/projects/svn/trunk/libtool 

 --tag=CXX --silent --mode=link g++  -g -O2 

 -rpath /dev/null/lib  -o libgtest-1.la

compile svn with gtest present but without flags test:
make clean
./configure --enable-maintaner-mode --prefix=/dev/null
make
PASS

compile svn without gtest present and without flags test:
make clean
./configure --enable-maintaner-mode --prefix=/dev/null
make
PASS

compile svn without gtest present and with flag test:
make clean
./configure --enable-maintaner-mode --prefix=/dev/null
make
PASS

roll patch, log & ship it.

[PATCH] OPW 2013: Build System Gtest Addition

Details can be found here:
[PATCH] OPW 2013: Build System Gtest Addition

The original project proposals can be found here:

My First Blog post (Hello World reloaded)

How to Mess up a Patch [Pt. II]

Failures are are a valuable part of the learning process, but no-one likes to repeatedly have failures in public -- the dev mailing list is quite a public place, and being a rookie among so many good coders is quite intimidating.

So it's easy to get stage-fright here and start to feel a little bad about your performance -- it's a totally healthy reaction.

However, don't get bogged down, but always analyze how you arrived at the failure and if in doubt, listen to Zappa:

Finally, remember, your mistakes currently are simple ones.  As you get better, your mistakes will get more sophisticated, and you'll end up thinking fondly of the time when a white space or a simple procedural error was your biggest problem.  So, enjoy this happy time whilst it lasts :)

How to mess up a patch [Part I]

Strictly speaking, this should have been [Part 15] or so. I keep making mistakes, and it is not for want of checking -- my 'spidey sense' just isn't honed and there is quite a bit odd detail to be observed. Lucky for me, the Subversion team are eagle-eyed and patient!


So what went wrong in today's patch?  Let's count the ways:

1.  I didn't take a break before proofreading.
2. My original goal was to do A, then I found that doing B was superior, but then claimed I did A instead of B in the log message.    Always check your entry and exit contract.
3. I assumed something weird and didn't question it. What I should have thought instead was: how odd! and investigated.
4. I didn't consider that two Apache sites would have uniformity in architecture, concept and appearance.

All this looks perfectly obvious in hindsight, but actually, it's not so trivial.  It's not that the individual tasks are difficult, it's just that there are so many of them!

Pre-Internship Patches

Here is my collection of accepted patches so far:

[PATCH] Test for line ending bug in svnrdump (issue 4263)

[PATCH] Fix for "\r" in "svnrdump load" (issue 4263)

[PATCH] Update to out-of-date code comments

[PATCH]: Re: Proposal for OPW project(get-deps.sh I)

[PATCH]: Re: Proposal for OPW project(get-deps.sh II)