Re: [PATCH 1/2] ci: also define CXX environment variable

[Josh Steadmon <steadmon@google.com>]

On 2024.03.05 20:00, Jeff King wrote:
> On Tue, Mar 05, 2024 at 07:50:58PM -0500, Jeff King wrote:
> 
> > On Tue, Mar 05, 2024 at 01:11:59PM -0800, Josh Steadmon wrote:
> > 
> > > In a future commit, we will build the fuzzer executables as part of the
> > > default 'make all' target, which requires a C++ compiler. If we do not
> > > explicitly set CXX, it defaults to g++ on GitHub CI. However, this can
> > > lead to incorrect feature detection when CC=clang, since the
> > > 'detect-compiler' script only looks at CC. Fix the issue by always
> > > setting CXX to match CC in our CI config.
> > > 
> > > We only plan on building fuzzers on Linux, so none of the other CI
> > > configs need a similar adjustment.
> > 
> > Does this mean that after your patch 2, running:
> > 
> >   make CC=clang
> > 
> > may have problems on Linux, because it will now try to link fuzzers
> > using g++, even though everything else is built with clang (and ditto
> > the detect-compiler used it)?
> 
> Also, if the answer is "yes": do we really need a c++ linker here? My
> understanding from reading "git log -SCXX Makefile" is that when using
> oss-fuzz, you'd sometimes want to pass c++ specific things in
> FUZZ_CXXFLAGS. But we're not using that here, and are just making sure
> that things can be linked. Can we just use $(CC) by default here, then?
> 
> Something like:
> 
> diff --git a/Makefile b/Makefile
> index f74e96d7c2..3f09d75f46 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -3861,17 +3861,18 @@ cover_db_html: cover_db
>  #
>  # An example command to build against libFuzzer from LLVM 11.0.0:
>  #
> -# make CC=clang CXX=clang++ \
> +# make CC=clang FUZZ_CXX=clang++ \
>  #      CFLAGS="-fsanitize=fuzzer-no-link,address" \
>  #      LIB_FUZZING_ENGINE="-fsanitize=fuzzer,address" \
>  #      fuzz-all
>  #
> +FUZZ_CXX ?= $(CC)
>  FUZZ_CXXFLAGS ?= $(ALL_CFLAGS)
>  
>  .PHONY: fuzz-all
>  
>  $(FUZZ_PROGRAMS): %: %.o oss-fuzz/dummy-cmd-main.o $(GITLIBS) GIT-LDFLAGS
> -	$(QUIET_LINK)$(CXX) $(FUZZ_CXXFLAGS) -o $@ $(ALL_LDFLAGS) \
> +	$(QUIET_LINK)$(FUZZ_CXX) $(FUZZ_CXXFLAGS) -o $@ $(ALL_LDFLAGS) \
>  		-Wl,--allow-multiple-definition \
>  		$(filter %.o,$^) $(filter %.a,$^) $(LIBS) $(LIB_FUZZING_ENGINE)
>  
> 
> -Peff

Indeed, it does break, and this is a good fix. Thanks for the catch!