On Thu, Aug 06, 2020 at 04:47:34PM -0700, Junio C Hamano wrote: > I had seen an interesting compilation breakage today. A topic adds > many more uses of argv-array API so I resolved semantic conflict > patch until "make builtins/submodule-helper.o" passed. I failed to > spot one remaining breakage until I saw > > https://travis-ci.org/github/git/git/jobs/715668996 > > The problematic line was > > precompose_argv(diff_args.argc, diff_args.argv); > > where diff_args used to be an argv_array and is now a strvec. > > Why didn't I catch this in my local test? > > $ git grep -n -e precompose_argv -- \*.h > compat/precompose_utf8.h:31:void precompose_argv(int argc, const char **argv); > git-compat-util.h:256:#define precompose_argv(c,v) > > The problematic part is this one used on all platforms other than macOS: > > /* used on Mac OS X */ > #ifdef PRECOMPOSE_UNICODE > #include "compat/precompose_utf8.h" > #else > #define precompose_str(in,i_nfd2nfc) > #define precompose_argv(c,v) > #define probe_utf8_pathname_composition() > #endif > > I am wondering if it is a good idea to use something like > > static inline void precompose_argv(int argc, const char **argv) > { > ; /* nothing */ > } > > instead. As long as the compiler is reasonable enough, this should > not result in any code change in the result, except that it would > still catch wrong arguments, even if these two parameters are unused > and optimized out. > In my every-day-live these kind of "late detection of breakage" is not uncommon. In other words: this patch allows early detection and is much better. Thanks for cleaning up my mess.
