On Sun, Jul 28, 2013 at 8:35 PM, Jonathan Nieder <jrnie...@gmail.com> wrote:
> Hi,
> David Aguilar wrote:
>> --- a/imap-send.c
>> +++ b/imap-send.c
>> @@ -22,14 +22,11 @@
>>   *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  
>> USA
>>   */
>> -#include "cache.h"
>> -#include "exec_cmd.h"
>> -#include "run-command.h"
>> -#include "prompt.h"
>>  #ifdef NO_OPENSSL
>>  typedef void *SSL;
>>  #else
>> +/* git-compat-util.h overwrites ctype.h; this must be included first */
>>  #include <CommonCrypto/CommonHMAC.h>
> Thanks for your work on this.
> Currently each translation unit of git includes git-compat-util.h or a
> header like cache.h that includes git-compat-util.h before doing
> anything else, since otherwise feature test macros are not set before
> the first system header is included.
> The above (CommonCrypto needing to be included before some of the
> definitions from git-compat-util.h) suggests to me that CommonCrypto
> should just be included directly from git-compat-util.h in some
> appropriate place.  That way any other header that needs CommonCrypto
> routines only has to include git-compat-util.h first as usual and
> doesn't have to worry about the order of other #includes.  Could that
> work?

imap-send is currently the only user of this stuff; I believe this
would pull in these library dependencies for all builtins.

If we don't mind a new file, something like git-compat-crypto.h could
be a nice place to tuck all these #ifdefs away.  If we had another
command that needed these then it'd be a easier to justify a new file.

A test-crypto command in the test suite could be written to verify
that the implementations work as advertised.  That counts as "another
command", albeit an internal one.  I don't believe imap-send has any
test coverage so pulling this stuff out would help make it more
testable, which is a net win.  Libifying might be a little premature,

To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to