On Sun, Jul 28, 2013 at 8:35 PM, Jonathan Nieder <jrnie...@gmail.com> wrote:
> 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
>> -#include "cache.h"
>> -#include "exec_cmd.h"
>> -#include "run-command.h"
>> -#include "prompt.h"
>> #ifdef NO_OPENSSL
>> typedef void *SSL;
>> #ifdef APPLE_COMMON_CRYPTO
>> +/* 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
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