Here's the mail as received back by one of the reviewers (I cut out the personal email addresses so they don't show up in public searches).
- Forwarded message ----------
From: Mail Delivery System <MAILER-DAEMON@llvm-reviews.chandlerc.com>
Date: 14 November 2013 08:40
Subject: Undelivered Mail Returned to Sender
To: rafael....
This is the mail system at host llvm-reviews.chandlerc.com.
I'm sorry to have to inform you that your message could not
be delivered to one or more recipients. It's attached below.
For further assistance, please send mail to postmaster.
If you do so, please include this problem report. You can
delete your own text from the attached returned message.
The mail system
<reviews+D2165+public+acc007354ff2d405@llvm-reviews.chandlerc.com>: Command
died with status 255: " /srv/http/phabricator/scripts/mail/mail_handler.php
custom/myconfig". Command output: PHP Fatal error: Uncaught exception
'Exception' with message 'Process exited with an open transaction! The
transaction will be implicitly rolled back. Calls to openTransaction() must
always be paired with a call to saveTransaction() or killTransaction().' in
/srv/http/libphutil/src/aphront/storage/connection/AphrontDatabaseTransactionState.php:71
Stack trace: #0 [internal function]:
AphrontDatabaseTransactionState->__destruct() #1 {main} thrown in
/srv/http/libphutil/src/aphront/storage/connection/AphrontDatabaseTransactionState.php
on line 71 Fatal error: Uncaught exception 'Exception' with message
'Process exited with an open transaction! The transaction will be
implicitly rolled back. Calls to openTransaction() must always be paired
with a call to saveTransaction() or killTransaction().' in
/srv/http/libphutil/src/aphront/storage/connection/AphrontDatabaseTransactionState.php:71
Stack trace: #0 [internal function]:
AphrontDatabaseTransactionState->__destruct() #1 {main} thrown in
/srv/http/libphutil/src/aphront/storage/connection/AphrontDatabaseTransactionState.php
on line 71Final-Recipient: rfc822;
reviews+D2165+public+acc007354ff2d405@llvm-reviews.chandlerc.com
Original-Recipient:
rfc822;reviews+D2165+public+acc007354ff2d405@llvm-reviews.chandlerc.com
Action: failed
Status: 5.3.0
Diagnostic-Code: x-unix; PHP Fatal error: Uncaught exception 'Exception' with
message 'Process exited with an open transaction! The transaction will be
implicitly rolled back. Calls to openTransaction() must always be paired
with a call to saveTransaction() or killTransaction().' in
/srv/http/libphutil/src/aphront/storage/connection/AphrontDatabaseTransactionState.php:71
Stack trace: #0 [internal function]:
AphrontDatabaseTransactionState->__destruct() #1 {main} thrown in
/srv/http/libphutil/src/aphront/storage/connection/AphrontDatabaseTransactionState.php
on line 71 Fatal error: Uncaught exception 'Exception' with message
'Process exited with an open transaction! The transaction will be
implicitly rolled back. Calls to openTransaction() must always be paired
with a call to saveTransaction() or killTransaction().' in
/srv/http/libphutil/src/aphront/storage/connection/AphrontDatabaseTransactionState.php:71
Stack trace: #0 [internal function]:
AphrontDatabaseTransactionState->__destruct() #1 {main} thrown in
/srv/http/libphutil/src/aphront/storage/connection/AphrontDatabaseTransactionState.php
on line 71- Forwarded message ----------
From: "Rafael Espíndola" <rafael....>
To: Rui Ueyama <ruiu....>
Cc: reviews+D2165+public+acc007354ff2d405@llvm-reviews.chandlerc.com,
Michael Spencer <bigch....>, llvm-commits
<llvm-commits@cs.uiuc.edu>
Date: Thu, 14 Nov 2013 08:39:43 -0500
Subject: Re: [PATCH] Path: Recognize COFF import library file magic.
As to llvm-readobj, I think we don't have to write a test to run
llvm-readobj with an actual file reside on the file system. My test should
be sufficient as a unit test for the function that's being modified in the
patch. Actually I think this unit test would be better than tests using
llvm-readobj because it's more focused on identity_magic's functionality.
It's easy to write, understand, and debug.
But fails to show why we need a feature and cannot be compared with
native tools like dumpbin. At llvm we normally prefer smallish
integration tests than gtest style unit tests.
The case where unit tests are clearly better is when there is no easy
way to isolate what we want to test. For example, fixing a bug in
densemap. Anything but a unit test would be brittle in that case.
For file type identification, all that the unit test shows is that you
wrote the same number in two files. A test using llvm-readobj lets us
check the file with a native tool. Given that llvm-readobject must
identify the file, it is not brittle as the DenseMap example would be.
So, please, add a test running llvm-readobject.
Cheers,
Rafael