I had a patch for this in bug 1318370, I'm going to split it out to a separate bug because I also need it for bug 1311729.

I forgot to change the bug number in the commit message so the try push got posted in bug 1318370, but it was green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3e6aa35db1a23dac567a861fe3754e0102229179 I made a few changes from the previous iteration of the patch: * Handled clang-cl, added a `msvc_or_clang_cl` function and used it (converted a few other places to use it as well) * Changed `--with-ccache` to be a `js_option` so `depend_cflags` wouldn't be broken in js/src.

Comment on attachment 8892879 [details] bug 1384557 - move _DEPEND_CFLAGS+CL_INCLUDES_PREFIX to toolchain.configure, ignore {CC,CXX}_WRAPPER when using sccache https://reviewboard.mozilla.org/r/163892/#review169262 ::: commit-message-58451:1 (Diff revision 1) > +bug 1384557 - move _DEPEND_CFLAGS+CL_INCLUDES_PREFIX to toolchain.configure, ignore {CC,CXX}_WRAPPER when using sccache r=glandium As expected, this patch caused build failure on my system: $ mach build 0:03.89 d:\mozilla-build\mozmake\mozmake.EXE -f client.mk -s 0:45.67 "f:\m\mozilla-unified\obj-x86_64-pc-mingw32\dist": ???????????????? 0:45.67 0:45.67 Automatically clobbering f:\m\mozilla-unified\obj-x86_64-pc-mingw32 0:45.67 Successfully completed auto clobber. 0:51.87 Adding client.mk options from f:/m/mozilla-unified/.mozconfig: 0:51.87 AUTOCLOBBER=1 0:51.87 CONFIG_GUESS=x86_64-pc-mingw32 0:51.87 MOZ_OBJDIR=f:/m/mozilla-unified/obj-x86_64-pc-mingw32 0:51.87 OBJDIR=f:/m/mozilla-unified/obj-x86_64-pc-mingw32 0:51.87 FOUND_MOZCONFIG=f:/m/mozilla-unified/.mozconfig 0:51.96 Generating f:/m/mozilla-unified/configure 0:52.12 Generating f:/m/mozilla-unified/js/src/configure 0:54.30 cd f:/m/mozilla-unified/obj-x86_64-pc-mingw32 0:54.32 f:/m/mozilla-unified/configure 0:55.17 Creating Python environment 1:13.63 New python executable in f:\m\mozilla-unified\obj-x86_64-pc-mingw32\_virtualenv\Scripts\python2.7.exe 1:13.63 Also creating executable in f:\m\mozilla-unified\obj-x86_64-pc-mingw32\_virtualenv\Scripts\python.exe 1:13.63 Installing setuptools, pip, wheel...done. 1:15.30 Error processing command. Ignoring because optional. (optional:setup.py:third_party/python/psutil:build_ext:--inplace) 1:15.30 f:\m\mozilla-unified\python\mozbuild\mozbuild\virtualenv.py:376: UserWarning: Hacking environment to allow binary Python extensions to build. You can make this warning go away by installing Visual Studio 2008. You can download the Express Edition installer from http://go.microsoft.com/?linkid=7729279 1:15.30 warnings.warn('Hacking environment to allow binary Python ' 1:15.30 Reexecuting in the virtualenv 1:16.02 Adding configure options from f:\m\mozilla-unified\.mozconfig 1:16.02 --enable-application=browser 1:16.02 --enable-crashreporter 1:16.02 --enable-warnings-as-errors 1:16.02 --enable-stylo 1:16.02 --with-libclang-path=c:/Users/kimu/.mozbuild/clang/bin 1:16.02 --with-clang-path=c:/Users/kimu/.mozbuild/clang/bin/clang.exe 1:16.02 --enable-geckodriver 1:16.02 --host=x86_64-pc-mingw32 1:16.02 --target=x86_64-pc-mingw32 1:16.02 checking for vcs source checkout... hg 1:16.31 checking for a shell... D:/mozilla-build/msys/bin/sh.exe 1:16.63 checking for host system type... x86_64-pc-mingw32 1:16.95 checking for target system type... x86_64-pc-mingw32 1:17.01 checking for a shell... D:/mozilla-build/msys/bin/sh.exe 1:17.34 checking for host system type... x86_64-pc-mingw32 1:17.64 checking for target system type... x86_64-pc-mingw32 1:17.70 checking for vcs source checkout... hg 1:17.70 checking whether cross compiling... no 1:17.89 checking for the target C compiler... 'D:/PROGRA~2/MICROS~1/VC/bin/amd64/cl.exe' 1:18.21 checking whether the target C compiler can be used... yes 1:18.23 checking for hg... d:/mozilla-build/python/Scripts/hg.exe 1:19.71 checking for Mercurial version... 4.2.2 1:20.56 checking for pkg_config... not found 1:20.58 checking for yasm... d:/mozilla-build/yasm/yasm.exe 1:20.63 checking yasm version... 1.3.0 1:20.63 checking the target C compiler version... 19.00.24215 1:20.87 checking the target C compiler works... yes 1:20.89 checking for the target C++ compiler... 'D:/PROGRA~2/MICROS~1/VC/bin/amd64/cl.exe' 1:21.13 checking whether the target C++ compiler can be used... yes 1:21.13 checking the target C++ compiler version... 19.00.24215 1:21.23 checking the target C++ compiler works... yes 1:21.25 checking for the host C compiler... 'D:/PROGRA~2/MICROS~1/VC/bin/amd64/cl.exe' 1:21.35 checking whether the host C compiler can be used... yes 1:21.35 checking the host C compiler version... 19.00.24215 1:21.41 checking the host C compiler works... yes 1:21.43 checking for the host C++ compiler... 'D:/PROGRA~2/MICROS~1/VC/bin/amd64/cl.exe' 1:21.52 checking whether the host C++ compiler can be used... yes 1:21.52 checking the host C++ compiler version... 19.00.24215 1:21.59 checking the host C++ compiler works... yes 1:21.67 checking for 64-bit OS... yes 1:21.67 checking bindgen cflags... no 1:21.93 checking for Windows SDK... 0x0a00 in 'D:\Program Files (x86)\Windows Kits\10\' 1:21.93 checking for Universal CRT SDK... 10.0.14393.0 in 'D:\Program Files (x86)\Windows Kits\10\' 1:21.95 checking for the Debug Interface Access SDK... D:/PROGRA~2/MICROS~1/DIA SDK 1:21.99 checking for mt... 'D:/PROGRA~2/WINDOW~1/10/bin/x64/mt.exe' 1:22.09 checking whether MT is really Microsoft Manifest Tool... yes 1:22.09 checking for link... 'D:/PROGRA~2/MICROS~1/VC/bin/amd64/link.exe' 1:22.11 checking for makecab... c:/WINDOWS/System32/makecab.exe 1:22.35 Traceback (most recent call last): 1:22.35 File "f:/m/mozilla-unified/configure.py", line 124, in <module> 1:22.35 sys.exit(main(sys.argv)) 1:22.35 File "f:/m/mozilla-unified/configure.py", line 29, in main 1:22.35 sandbox.run(os.path.join(os.path.dirname(__file__), 'moz.configure')) 1:22.35 File "f:\m\mozilla-unified\python\mozbuild\mozbuild\configure\__init__.py", line 426, in run 1:22.35 func(*args) 1:22.35 File "f:\m\mozilla-unified\python\mozbuild\mozbuild\configure\__init__.py", line 472, in _value_for 1:22.36 return self._value_for_depends(obj, need_help_dependency) 1:22.36 File "f:\m\mozilla-unified\python\mozbuild\mozbuild\util.py", line 925, in method_call 1:22.36 cache[args] = self.func(instance, *args) 1:22.36 File "f:\m\mozilla-unified\python\mozbuild\mozbuild\configure\__init__.py", line 481, in _value_for_depends 1:22.36 return obj.result(need_help_dependency) 1:22.36 File "f:\m\mozilla-unified\python\mozbuild\mozbuild\util.py", line 925, in method_call 1:22.36 cache[args] = self.func(instance, *args) 1:22.36 File "f:\m\mozilla-unified\python\mozbuild\mozbuild\configure\__init__.py", line 122, in result 1:22.36 return self._func(*resolved_args) 1:22.36 File "f:\m\mozilla-unified\python\mozbuild\mozbuild\configure\__init__.py", line 1000, in wrapped 1:22.36 return new_func(*args, **kwargs) 1:22.36 File "f:/m/mozilla-unified/build/moz.configure/windows.configure", line 441, in msvc_showincludes_prefix 1:22.36 if line.endswith('\\stdio.h'): 1:22.36 UnicodeDecodeError: 'ascii' codec can't decode byte 0x83 in position 0: ordinal not in range(128) 1:22.39 *** Fix above errors and then restart with\ 1:22.39 "d:/mozilla-build/mozmake/mozmake.EXE -f client.mk build" 1:22.41 mozmake.EXE[2]: *** [f:/m/mozilla-unified/client.mk:383: configure] Error 1 1:22.41 mozmake.EXE[1]: *** [f:/m/mozilla-unified/client.mk:396: f:/m/mozilla-unified/obj-x86_64-pc-mingw32/Makefile] Error 2 1:22.42 mozmake.EXE: *** [client.mk:170: build] Error 2 1:22.46 363 compiler warnings present. 2

(In reply to Masatoshi Kimura [:emk] from comment #3) > As expected, this patch caused build failure on my system: Thanks for the heads up! I think I have a non-en-US Visual Studio installed in a VM here somewhere, I will try to test with that.

Comment on attachment 8892879 [details] bug 1384557 - move _DEPEND_CFLAGS+CL_INCLUDES_PREFIX to toolchain.configure, ignore {CC,CXX}_WRAPPER when using sccache https://reviewboard.mozilla.org/r/163892/#review171606 ::: build/moz.configure/toolchain.configure:873 (Diff revision 1) > host_cxx_compiler = compiler('C++', host, c_compiler=host_c_compiler, > other_compiler=cxx_compiler, > other_c_compiler=c_compiler) > > # Generic compiler-based conditions. > +msvc_or_clang_cl = depends(c_compiler)(lambda info: info.type in ('clang-cl', 'msvc')) I'm not particularly convinced those generic compiler-based conditions are useful. Especially when all that follows should theoretically be repeated for the host compiler. So I'd rather leave this change out. ::: build/moz.configure/windows.configure (Diff revision 1) > os.environ['INCLUDE'] = includes > return includes > > set_config('INCLUDE', include_path) > > - Please leave this empty line alone. ::: build/moz.configure/windows.configure:436 (Diff revision 1) > > check_prog('MAKECAB', ('makecab.exe',)) > > +@depends(c_compiler, when=msvc_or_clang_cl) > +@imports(_from='re', _import='compile', _as='re_compile') > +def msvc_showincludes_prefix(c_compiler): CL_INCLUDES_PREFIX is only used by cl.py, so we can skip this test when using sccache. ::: build/moz.configure/windows.configure:437 (Diff revision 1) > check_prog('MAKECAB', ('makecab.exe',)) > > +@depends(c_compiler, when=msvc_or_clang_cl) > +@imports(_from='re', _import='compile', _as='re_compile') > +def msvc_showincludes_prefix(c_compiler): > + pattern = re_compile(r'^([^:]*:.*[ :] )(.*\\stdio.h)$') maybe you need a b'' string here. ::: build/moz.configure/windows.configure:444 (Diff revision 1) > + ['-nologo', '-c', '-Fonul', '-showIncludes']) > + for line in output.splitlines(): > + if line.endswith('\\stdio.h'): > + m = pattern.match(line) > + if m: > + if not m.group(2): This condition is a noop. You can have a match and an empty group 2. ::: config/config.mk (Diff revision 1) > > export CL_INCLUDES_PREFIX > -# Make sure that the build system can handle non-ASCII characters > -# in environment variables to prevent it from breking silently on > -# non-English systems. > -export NONASCII Please leave this alone.

Comment on attachment 8892879 [details] bug 1384557 - move _DEPEND_CFLAGS+CL_INCLUDES_PREFIX to toolchain.configure, ignore {CC,CXX}_WRAPPER when using sccache https://reviewboard.mozilla.org/r/163892/#review171606 > Please leave this alone. I removed it at your request the last time you reviewed this patch: https://bugzilla.mozilla.org/show_bug.cgi?id=1318370#c9

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #6) > Comment on attachment 8892879 [details] > bug 1384557 - move _DEPEND_CFLAGS+CL_INCLUDES_PREFIX to toolchain.configure, > ignore {CC,CXX}_WRAPPER when using sccache > > https://reviewboard.mozilla.org/r/163892/#review171606 > > > Please leave this alone. > > I removed it at your request the last time you reviewed this patch: > https://bugzilla.mozilla.org/show_bug.cgi?id=1318370#c9 Erf. Then please remove it separately.

(In reply to Mike Hommey [:glandium] from comment #7) > > I removed it at your request the last time you reviewed this patch: > > https://bugzilla.mozilla.org/show_bug.cgi?id=1318370#c9 > > Erf. Then please remove it separately. OK. Should I leave the setting of `NONASCII` in old-configure.in, or remove it in this patch? If I remove it then the export in config.mk doesn't actually do anything anyway.

Comment on attachment 8892879 [details] bug 1384557 - move _DEPEND_CFLAGS+CL_INCLUDES_PREFIX to toolchain.configure, ignore {CC,CXX}_WRAPPER when using sccache https://reviewboard.mozilla.org/r/163892/#review171606 > This condition is a noop. You can have a match and an empty group 2. I just ported this straight from the existing old-configure logic, but I'm not attached to it.

Working on getting a Japanese MSVC installed and working so I can test that.

"chcp 932" would be required to let MSVC use localized messages. Also, the system locale must be "Japanese" to replicate the "mbcs" encoding behavior of Japanese Windows.

(In reply to Masatoshi Kimura [:emk] from comment #11) > "chcp 932" would be required to let MSVC use localized messages. > Also, the system locale must be "Japanese" to replicate the "mbcs" encoding > behavior of Japanese Windows. I could not get this working for the life of me. Even with the Japanese locale pack installed, the system locale set to "Japanese", and the codepage in the shell I was testing with set to 932, the compiler still doesn't produce Japanese text for the -showIncludes prefix. Masatoshi: can you test the updated patch here? I am at a loss as to how to get MSVC to generate Japanese output, short of installing a full Japanese Windows (which might be beyond my ability as I can't read Japanese). glandium: review ping?

This patch works for me.

Comment on attachment 8892879 [details] bug 1384557 - move _DEPEND_CFLAGS+CL_INCLUDES_PREFIX to toolchain.configure, ignore {CC,CXX}_WRAPPER when using sccache https://reviewboard.mozilla.org/r/163892/#review183578

https://hg.mozilla.org/integration/mozilla-inbound/rev/6555a2a899a0fbe6967846e1d73465fc67ac12ad bug 1384557 - move _DEPEND_CFLAGS+CL_INCLUDES_PREFIX to toolchain.configure, ignore {CC,CXX}_WRAPPER when using sccache r=glandium

https://hg.mozilla.org/integration/mozilla-inbound/rev/b83927b6c8904881f58f2fa9a59be22f54451842 bug 1384557 - fix flake8 lint errors

https://hg.mozilla.org/mozilla-central/rev/6555a2a899a0 https://hg.mozilla.org/mozilla-central/rev/b83927b6c890

Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/bb96540a6525 Keep build files in sync (Port bug 1384557 - ignore {CC,CXX}_WRAPPER when using sccache). r=tomprince(via IRC)

Once again this patch broke my local build. Something was changed during 2 months. Please backout.

Backed out: https://hg.mozilla.org/integration/mozilla-inbound/rev/fb5b3dbdaf6799c8ba321f3993ed19049430676e Please integrate patches from bug 1414060 before relanding.

Backout by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/ab5f127c30be Backed out changeset bb96540a6525: Port of bug 1384557 which got backed out. a=jorgk DONTBUILD

Rebased Ted's patch; had to move a few things around since clang-cl has gotten some snazzy dependency stuff in the interim. glandium, do you want to re-review this? Green on try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=323cc6dc3dfb5a3bba9e246843da362ff5fb542a

Comment on attachment 9008119 [details] [diff] [review] move _DEPEND_CFLAGS+CL_INCLUDES_PREFIX to toolchain.configure, ignore {CC,CXX}_WRAPPER when using sccache Review of attachment 9008119 [details] [diff] [review]: ----------------------------------------------------------------- ::: build/moz.configure/toolchain.configure @@ +1247,5 @@ > > +@depends(c_compiler, using_sccache) > +def depend_cflags(info, using_sccache): > + if info.type not in ('clang-cl', 'msvc'): > + return '-MD -MP -MF $(MDDEPDIR)/$(@F).pp' It would be better to uses lists. I'm also a little bothered that this uses hardcoded make syntax... but I don't have a better idea. ::: old-configure.in @@ -3793,5 @@ > - fi > - dnl Don't override this for MSVC > - if test -z "$_WIN32_MSVC"; then > - _USE_CPP_INCLUDE_FLAG= > - _DEFINES_CFLAGS='$(ACDEFINES) -D_MOZILLA_CONFIG_H_ -DMOZILLA_CLIENT' This thing is puzzling.

(In reply to Mike Hommey [:glandium] from comment #25) > ::: build/moz.configure/toolchain.configure > @@ +1247,5 @@ > > > > +@depends(c_compiler, using_sccache) > > +def depend_cflags(info, using_sccache): > > + if info.type not in ('clang-cl', 'msvc'): > > + return '-MD -MP -MF $(MDDEPDIR)/$(@F).pp' > > It would be better to uses lists. I'm also a little bothered that this uses > hardcoded make syntax... but I don't have a better idea. I believe trying to make it a list would hit this case: https://searchfox.org/mozilla-central/source/python/mozbuild/mozbuild/shellutil.py#110-113 when config.status is executed. So that's no good...

Why would it?

Oh, moz.configure lists are not like old-configure lists (I was running into a problem yesterday where MOZ_SUBST_LIST on a similar make-syntax thing was erroring). OK, so there's no issues here. Will change.

_DEPEND_CFLAGS is now a list!

Comment on attachment 9008410 [details] [diff] [review] move _DEPEND_CFLAGS+CL_INCLUDES_PREFIX to toolchain.configure, ignore {CC,CXX}_WRAPPER when using sccache Review of attachment 9008410 [details] [diff] [review]: ----------------------------------------------------------------- ::: build/moz.configure/toolchain.configure @@ +1247,5 @@ > > +@depends(c_compiler, using_sccache) > +def depend_cflags(info, using_sccache): > + if info.type not in ('clang-cl', 'msvc'): > + return ['-MD', '-MP', '-MF $(MDDEPDIR)/$(@F).pp'] Maybe worth filing a separate bug to track the things we put in the config that are using make syntax like this, because we'll have to figure out something for those at some point.

Pushed by nfroyd@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/98343cbec0c4 move _DEPEND_CFLAGS+CL_INCLUDES_PREFIX to toolchain.configure, ignore {CC,CXX}_WRAPPER when using sccache; r=glandium

Thanks for finishing this off!