On Sun, Jul 28, 2013 at 8:35 PM, Jonathan Nieder <jrnieder@xxxxxxxxx> 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 >> #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 > 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, though. Thoughts? -- David -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html