You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucy.apache.org by Nick Wellnhofer <we...@aevum.de> on 2016/03/04 15:37:34 UTC

[lucy-dev] [VOTE] Apache Clownfish 0.5.0 RC 1

Hello,

Release candidate 1 for Apache Clownfish version 0.5.0 can be
found at:

 
https://dist.apache.org/repos/dist/dev/lucy/clownfish/apache-clownfish-0.5.0-rc1/

See the CHANGES file at the top level of the archive for information
about the content of this release.

This candidate was assembled according to the process documented at:

     http://wiki.apache.org/lucy/ReleaseGuide

It was cut using "git archive" from the tag at:

 
https://git-wip-us.apache.org/repos/asf?p=lucy-clownfish.git;a=tag;h=refs/tags/apache-clownfish-0.5.0-rc1

Please vote on releasing this candidate as Apache Clownfish version
0.5.0.  The vote will be held open for at least the next 72
hours.

All interested parties are welcome to inspect the release candidate
and express approval or disapproval.  Votes from members of the Lucy
PMC are binding; the vote passes if there are at least three binding
+1 votes and more +1 votes than -1 votes.

For suggestions as to how to evaluate Apache Clownfish release candidates,
and for information on ASF voting procedures, see:

     http://wiki.apache.org/lucy/ReleaseVerification
     http://wiki.apache.org/lucy/ReleasePrep
     http://www.apache.org/foundation/voting.html

I tested the release candidate under:

Ubuntu 15.04, x86, 32-bit
- Go bindings with stock Go 1.3.3
- Perl bindings with stock Perl 5.20.2
- C bindings with stock GCC 4.9.2

Windows 10, x64
- Perl bindings with Strawberry Perl 5.22.1.2, 32-bit
- C bindings with MSVC 10, 64-bit

[ ] +1 Release RC 1 as Apache Clownfish 0.5.0.
[ ] +0
[ ] -1 Do not release RC 1 as Apache Clownfish 0.5.0 because...

Here's my +1.

Thanks!


Re: [lucy-dev] [VOTE][RESULT] Apache Clownfish 0.5.0 RC 1

Posted by Nick Wellnhofer <we...@aevum.de>.
On 04/03/2016 15:37, Nick Wellnhofer wrote:
> All interested parties are welcome to inspect the release candidate
> and express approval or disapproval.  Votes from members of the Lucy
> PMC are binding; the vote passes if there are at least three binding
> +1 votes and more +1 votes than -1 votes.

> [ ] +1 Release RC 1 as Apache Clownfish 0.5.0.
> [ ] +0
> [ ] -1 Do not release RC 1 as Apache Clownfish 0.5.0 because...

The vote fails with +1 votes from the following PMC members:

- Nick Wellnhofer
- Marvin Humphrey

I'll create another RC addressing the issues that were mentioned.

Nick


Re: [lucy-dev] [VOTE] Apache Clownfish 0.5.0 RC 1

Posted by Nick Wellnhofer <we...@aevum.de>.
On 22/03/2016 15:31, Peter Karman wrote:
> Is this VOTE thread still a VOTE thread? Should we expect another RC?

I think it's best if I assemble another RC.

Nick


Re: [lucy-dev] [VOTE] Apache Clownfish 0.5.0 RC 1

Posted by Peter Karman <pe...@peknet.com>.
Is this VOTE thread still a VOTE thread? Should we expect another RC?

-- 
Peter Karman  .  http://peknet.com/  .  peter@peknet.com

Re: [lucy-dev] [VOTE] Apache Clownfish 0.5.0 RC 1

Posted by Marvin Humphrey <ma...@rectangular.com>.
On Sat, Mar 19, 2016 at 4:17 AM, Nick Wellnhofer <we...@aevum.de> wrote:

> I'm a big fan of -Wconversion and I'd encourage everyone starting with a new
> C project to use this flag from the beginning. Adding this flag to existing
> projects takes a lot of effort, though.

I'm expecting to land a branch that deals with warnings today.  For
Clownfish at least and possibly also for Lucy.

It would not be unreasonable for us to add `-Wconversion` going
forward.  With previous releases of Clownfish and Lucy, I kept us up
to date with MSVC's conversion warnings -- so we are actually pretty
close.

In my branch right now, `-Wconversion` triggers warnings for...

* Charmonizer
* lemon
* the Flex-generated CFCLexHeader.c
* CommonMark

... but not for CFC or the Clownish runtime core under the C or Python
bindings.  I just need to work through Perl and Go and do some branch
cleanup for Clownfish. Then I need to see where we're at with
`-Wconversion` for Lucy.

Marvin Humphrey

Re: [lucy-dev] [VOTE] Apache Clownfish 0.5.0 RC 1

Posted by Nathan Kurz <na...@gmail.com>.
On Sat, Mar 19, 2016 at 4:17 AM, Nick Wellnhofer <we...@aevum.de> wrote:
>> ICC does offer some nice optimization diagnostics.  Assuming most
>> people aren't using it, I've attached a sample here for
>> "-qopt-report=4 -qopt-report-phase cg,ipo,loop,par,vec" which might be
>> worth glancing at.
>
>
> I could only find an attachment with the leak sanitizer output.

Hmm, it's attached in my sent folder.  Either it was stripped by the
mailing list software or at your end.   I've renamed it from
CFCParseHeader.optrpt to CFCParseHeader.txt to see if that helps.

>> The various -fsanitize options are really useful, and surprisingly
>> underutilized.   it probably would be good practice to start testing
>> with them.   Adding them to CFLAGS is a bit difficult in this project,
>> but using "CC='clang -fsanitize=address' configure" seems to work.
>
> We regularly test with Valgrind and from my experience it catches more
> errors than -fsanitize=address and -fsanitize=memory. An -fsanitize build is
> a lot faster than testing with Valgrind, though, and -fsanitize=undefined
> adds some additional diagnostics, so we should provide an easy way to test
> with these flags.

I agree --- for leaks and uninitialized memory, Valgrind is more
complete.  But the 'undefined' checker is unique, and all of them are
fast enough that one might consider enabling one of them by default
for test suite, or at least making each of them a predefined targets.

As I was testing this, I realized that the support for the sanitize
libraries has been broken in the standard Ubuntu and Debian Linux
packages for clang for the last few versions.   It's a known bug, and
has since been fixed, but the packages still haven't been updated.  If
you are affected, I give a workaround here:
http://askubuntu.com/questions/740289/which-package-do-i-need-to-use-clang-with-asan-for-32-bit/745320

On Sat, Mar 19, 2016 at 5:14 AM, Marvin Humphrey <ma...@rectangular.com> wrote:
> I'm expecting to land a branch that deals with warnings today.  For
> Clownfish at least and possibly also for Lucy.

Wonderful!

--nate

Re: [lucy-dev] [VOTE] Apache Clownfish 0.5.0 RC 1

Posted by Nick Wellnhofer <we...@aevum.de>.
On 13/03/2016 01:59, Nathan Kurz wrote:
> Clang: ../src/CFCPython.c:141:9: warning: string literal of length
> 8477 exceeds maximum length 4095 that ISO C99 compilers are required
> to support [-Woverlength-strings]        "static PyObject*\n"
>
> GCC: ../src/CFCPython.c: In function ‘S_gen_callbacks’:
> ../src/CFCPython.c:352:9: warning: string length ‘8477’ is greater
> than the length ‘4095’ ISO C99 compilers are required to support
> [-Woverlength-strings]

This seems to be a harmless warning resulting from the `-pedantic` switch. I 
decided to add `-Wno-overlength-strings` to our compile flags.

> ICC does offer some nice optimization diagnostics.  Assuming most
> people aren't using it, I've attached a sample here for
> "-qopt-report=4 -qopt-report-phase cg,ipo,loop,par,vec" which might be
> worth glancing at.

I could only find an attachment with the leak sanitizer output.

> As Nick hints, there are a _lot_ of warnings that appear for all the
> compilers if you add -Wconversion to CFLAGS.  Note that these warnings
> are in addition to -Wextra.  For library code like this, it's probably
> worth adding this flag, and fixing the warnings.  While most are false
> alarms, there are likely are some real bugs hiding in there.

I'm a big fan of -Wconversion and I'd encourage everyone starting with a new C 
project to use this flag from the beginning. Adding this flag to existing 
projects takes a lot of effort, though.

> I also tested with "-fsanitize=address".   This reported a couple
> dozen potential leaks, although I don't know if they are real or just
> testing artifacts.   I've attached the report in case someone with
> more knowledge wants to check.

There are a couple of known leaks in the test suite. You should compile with 
LUCY_VALGRIND=1 to ignore them.

> I tjhen tested to see if adding "-fsanitize=undefined" (GCC and
> Clang), and "-fsanitize=memory" (Clang only) reported anything, but
> these came up clean.   I only tested for 'compiler/c', though, might
> be worth trying the other directories.
>
> The various -fsanitize options are really useful, and surprisingly
> underutilized.   it probably would be good practice to start testing
> with them.   Adding them to CFLAGS is a bit difficult in this project,
> but using "CC='clang -fsanitize=address' configure" seems to work.
We regularly test with Valgrind and from my experience it catches more errors 
than -fsanitize=address and -fsanitize=memory. An -fsanitize build is a lot 
faster than testing with Valgrind, though, and -fsanitize=undefined adds some 
additional diagnostics, so we should provide an easy way to test with these flags.

Nick


Re: [lucy-dev] [VOTE] Apache Clownfish 0.5.0 RC 1

Posted by Nathan Kurz <na...@gmail.com>.
I haven't been keeping up with the code, but after Marvin's comments
about excess warnings I did try building "compiler/c" with GCC 5.3,
Clang 3.8, and ICC 16.0.1.

Particularly with Clang, the noise from the warnings was overwhelming.
But after a little more inspection, it turned out that most of these
were an interaction between Clang and ccache
(https://ccache.samba.org/).

If you happen to be using ccache (generally highly recommended) you
should check out the instructions here:
http://petereisentraut.blogspot.com/search/label/ccache

In case the compiler versions I'm using are different, I'll point out
a couple warnings besides the "unused variables" and "inline" that
might be worth paying attention to:

Clang: ../src/CFCPython.c:141:9: warning: string literal of length
8477 exceeds maximum length 4095 that ISO C99 compilers are required
to support [-Woverlength-strings]        "static PyObject*\n"

GCC: ../src/CFCPython.c: In function ‘S_gen_callbacks’:
../src/CFCPython.c:352:9: warning: string length ‘8477’ is greater
than the length ‘4095’ ISO C99 compilers are required to support
[-Woverlength-strings]

ICC also reports a number of "Inlining inhibited by limit max-size" and
"Inlining inhibited by limit max-total-size" warnings.  I don't think
there's much that can or should be done about these.

ICC does offer some nice optimization diagnostics.  Assuming most
people aren't using it, I've attached a sample here for
"-qopt-report=4 -qopt-report-phase cg,ipo,loop,par,vec" which might be
worth glancing at.

As Nick hints, there are a _lot_ of warnings that appear for all the
compilers if you add -Wconversion to CFLAGS.  Note that these warnings
are in addition to -Wextra.  For library code like this, it's probably
worth adding this flag, and fixing the warnings.  While most are false
alarms, there are likely are some real bugs hiding in there.

I also tested with "-fsanitize=address".   This reported a couple
dozen potential leaks, although I don't know if they are real or just
testing artifacts.   I've attached the report in case someone with
more knowledge wants to check.

I tjhen tested to see if adding "-fsanitize=undefined" (GCC and
Clang), and "-fsanitize=memory" (Clang only) reported anything, but
these came up clean.   I only tested for 'compiler/c', though, might
be worth trying the other directories.

The various -fsanitize options are really useful, and surprisingly
underutilized.   it probably would be good practice to start testing
with them.   Adding them to CFLAGS is a bit difficult in this project,
but using "CC='clang -fsanitize=address' configure" seems to work.

Hope this is helpful, and sorry I'm sending just suggestions rather
than actual patches.

--nate







On Sat, Mar 12, 2016 at 6:28 AM, Nick Wellnhofer <we...@aevum.de> wrote:
> On 12/03/2016 00:11, Marvin Humphrey wrote:
>>
>> There are more compiler
>> warnings than I'd prefer, but I don't think those should block the release
>> at
>> this point.
>
>
> On my Linux/i386 setup, the Clownfish build only complains about a couple of
> unused variables in the CFCPython code and an unused function in the
> generated callbacks. I don't get any warnings for Lucy.
>
> But I haven't tested on OS X and on 64-bit for a while. So I just built
> everything on OS X and there are a couple of warnings that I didn't see
> before, mainly because of redefining "inline" for MSVC in the cmark code.
> That's my fault and I'll fix that in the 0.5 branch.
>
> I'm also aware that there are many unchecked 64-to-32-bit conversions in the
> Lucy code after we switched to size_t indices in Vector and Hash. Some
> compilers complain loudly about that, but it's not trivial to fix. I think
> it would be best to use size_t indices in the Lucy codebase as well.
>
>> LICENSE, NOTICE, CONTRIBUTING.md, INSTALL.md, and README.md all looked
>> good in
>> a spot check -- except for one thing.  The copyright notice in NOTICE
>> still
>> reads 2014 and should have been updated to 2016.  This isn't the end of
>> the
>> world though, and I'm still willing to vote +1 despite it.
>
>
> That's a stupid oversight on my part. It's not a big deal to prepare another
> release candidate, but I'm on vacation the next week. If anyone wants to
> build and upload an RC2 in the next days, I'd be grateful. But I'm also OK
> with releasing RC1 as is.
>
> Nick

Re: [lucy-dev] [VOTE] Apache Clownfish 0.5.0 RC 1

Posted by Nick Wellnhofer <we...@aevum.de>.
On 12/03/2016 00:11, Marvin Humphrey wrote:
> There are more compiler
> warnings than I'd prefer, but I don't think those should block the release at
> this point.

On my Linux/i386 setup, the Clownfish build only complains about a couple of 
unused variables in the CFCPython code and an unused function in the generated 
callbacks. I don't get any warnings for Lucy.

But I haven't tested on OS X and on 64-bit for a while. So I just built 
everything on OS X and there are a couple of warnings that I didn't see 
before, mainly because of redefining "inline" for MSVC in the cmark code. 
That's my fault and I'll fix that in the 0.5 branch.

I'm also aware that there are many unchecked 64-to-32-bit conversions in the 
Lucy code after we switched to size_t indices in Vector and Hash. Some 
compilers complain loudly about that, but it's not trivial to fix. I think it 
would be best to use size_t indices in the Lucy codebase as well.

> LICENSE, NOTICE, CONTRIBUTING.md, INSTALL.md, and README.md all looked good in
> a spot check -- except for one thing.  The copyright notice in NOTICE still
> reads 2014 and should have been updated to 2016.  This isn't the end of the
> world though, and I'm still willing to vote +1 despite it.

That's a stupid oversight on my part. It's not a big deal to prepare another 
release candidate, but I'm on vacation the next week. If anyone wants to build 
and upload an RC2 in the next days, I'd be grateful. But I'm also OK with 
releasing RC1 as is.

Nick

Re: [lucy-dev] [VOTE] Apache Clownfish 0.5.0 RC 1

Posted by Marvin Humphrey <ma...@rectangular.com>.
On Fri, Mar 4, 2016 at 6:37 AM, Nick Wellnhofer <we...@aevum.de> wrote:

> [ ] +1 Release RC 1 as Apache Clownfish 0.5.0.
> [ ] +0
> [ ] -1 Do not release RC 1 as Apache Clownfish 0.5.0 because...

+1

I tested the release candidate on Amazon Linux and Mac OS X El Capitan, using
Lucy's test_all.sh: Go, C and Perl are all working.  There are more compiler
warnings than I'd prefer, but I don't think those should block the release at
this point.

I also tested Python on both OS X and Amazon Linux.  The build is quite
fragile, which is a known issue since the bindings are nascent.  Amazon Linux
required the addition of `extra_compile_args = ["-std=gnu99"],` to the
`Extension` constructor in setup.py -- after that, it built and passed what
tests we have.

The `test_valgrind` build target passed on Amazon Linux for both CFC and the
Clownfish runtime under the Perl bindings.

LICENSE, NOTICE, CONTRIBUTING.md, INSTALL.md, and README.md all looked good in
a spot check -- except for one thing.  The copyright notice in NOTICE still
reads 2014 and should have been updated to 2016.  This isn't the end of the
world though, and I'm still willing to vote +1 despite it.

I ran the tarball through http://compliance.rocks and everything looked good.

Sums and sigs check out.

Let's get this show on the road!

PS: Apologies for the delayed vote -- a relative in my extended family passed
    away recently.

Marvin Humphrey