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.