SCons: Unify tools/target build type configuration#66242
Conversation
There was a problem hiding this comment.
These options are not macOS or clang specific, so probably should be set everywhere (/Oy for MSVC).
With exception to -no_deduplicate (LD64 flag).
There was a problem hiding this comment.
Indeed, currently we seem to set -fomit-frame-pointer -ftree-vectorize only for Clang builds: Android, iOS, macOS.
I don't know why / if there's a problem using those with GCC. I guess we could try them out in a separate PR so that we don't have to deal with changing this optimization in this massive refactor.
/Oy for MSVC seems implied by both /O1 and /O2 so I guess it's not needed: https://learn.microsoft.com/en-us/cpp/build/reference/oy-frame-pointer-omission?view=msvc-170
There was a problem hiding this comment.
-fomit-frame-pointer has been a thing in GCC for a very long time (12+ years). I've seen it used by default in a lot of C++ projects, so I'd say it's battle-tested at this point 🙂
There was a problem hiding this comment.
Not sure why it's here. Isn't it to make executable symbols visible to the libraries?
There was a problem hiding this comment.
Yeah we noticed this discrepancy a while ago in March on Rocket.Chat, discussing it with @raulsntos and @neikeq: https://chat.godotengine.org/channel/devel?msg=2Dkxu3KsP2TZEwRNn
It seems it was added by @marcelofg55 in #11061, presumably for the Linux crash handler. I don't know if it's actually needed though. I can try crashing Godot without it and see what happens.
It also used to be used by the Mono module (on all targets, which would actually prevent removing unused symbols, explaining a part of the size difference between Mono and non-Mono builds in 3.x) but it's not needed anymore with .NET.
There was a problem hiding this comment.
So I gave it a try, and indeed without -rdynamic the crash handler doesn't retrieve the symbols properly.
With -rdynamic:
================================================================
handle_crash: Program crashed with signal 4
Engine version: Godot Engine v4.0.beta.custom_build (8e14f9ba21725a9445f3979742f2fc87c8a7b463)
Dumping the backtrace. Please include this when reporting the bug on: https://github.com/godotengine/godot/issues
[1] /lib64/libc.so.6(+0x36970) [0x7f4f2fa7d970] (??:0)
[2] core_bind::OS::crash(String const&) (/home/akien/Projects/godot/godot.git/core/core_bind.cpp:232)
[3] void call_with_ptr_args_helper<__UnexistingClass, String const&, 0ul>(__UnexistingClass*, void (__UnexistingClass::*)(String const&), void const**, IndexSequence<0ul>) (/home/akien/Projects/godot/godot.git/./core/variant/binder_common.h:283 (discriminator 8))
[4] void call_with_ptr_args<__UnexistingClass, String const&>(__UnexistingClass*, void (__UnexistingClass::*)(String const&), void const**) (/home/akien/Projects/godot/godot.git/./core/variant/binder_common.h:532)
[5] MethodBindT<String const&>::ptrcall(Object*, void const**, void*) (/home/akien/Projects/godot/godot.git/./core/object/method_bind.h:331)
[6] GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Callable::CallError&, GDScriptFunction::CallState*) (/home/akien/Projects/godot/godot.git/modules/gdscript/gdscript_vm.cpp:1965)
[7] GDScript::_create_instance(Variant const**, int, Object*, bool, Callable::CallError&) (/home/akien/Projects/godot/godot.git/modules/gdscript/gdscript.cpp:175)
[8] GDScript::instance_create(Object*) (/home/akien/Projects/godot/godot.git/modules/gdscript/gdscript.cpp:406)
[9] Object::set_script(Variant const&) (/home/akien/Projects/godot/godot.git/core/object/object.cpp:843)
[10] MainLoop::initialize() (/home/akien/Projects/godot/godot.git/core/os/main_loop.cpp:61 (discriminator 4))
[11] OS_LinuxBSD::run() (/home/akien/Projects/godot/godot.git/platform/linuxbsd/os_linuxbsd.cpp:571)
[12] ./bin/godot.linuxbsd.tools.x86_64.rdynamic(main+0x146) [0x2460eec] (/home/akien/Projects/godot/godot.git/platform/linuxbsd/godot_linuxbsd.cpp:74)
[13] /lib64/libc.so.6(+0x23677) [0x7f4f2fa6a677] (??:0)
[14] /lib64/libc.so.6(__libc_start_main+0x85) [0x7f4f2fa6a735] (??:0)
[15] ./bin/godot.linuxbsd.tools.x86_64.rdynamic(_start+0x21) [0x2460ce1] (??:?)
-- END OF BACKTRACE --
================================================================
Without -rdynamic:
================================================================
handle_crash: Program crashed with signal 4
Engine version: Godot Engine v4.0.beta.custom_build (8e14f9ba21725a9445f3979742f2fc87c8a7b463)
Dumping the backtrace. Please include this when reporting the bug on: https://github.com/godotengine/godot/issues
[1] /lib64/libc.so.6(+0x36970) [0x7fb97e2c1970] (??:0)
[2] ./bin/godot.linuxbsd.tools.x86_64.no_rdynamic() [0x43cc3d7] (/home/akien/Projects/godot/godot.git/core/core_bind.cpp:232)
[3] ./bin/godot.linuxbsd.tools.x86_64.no_rdynamic() [0x756b14] (/home/akien/Projects/godot/godot.git/./core/variant/binder_common.h:283 (discriminator 8))
[4] ./bin/godot.linuxbsd.tools.x86_64.no_rdynamic() [0x756737] (/home/akien/Projects/godot/godot.git/./core/variant/binder_common.h:532)
[5] ./bin/godot.linuxbsd.tools.x86_64.no_rdynamic() [0x7563d0] (/home/akien/Projects/godot/godot.git/./core/object/method_bind.h:331)
[6] ./bin/godot.linuxbsd.tools.x86_64.no_rdynamic() [0xc647a4] (/home/akien/Projects/godot/godot.git/modules/gdscript/gdscript_vm.cpp:1965)
[7] ./bin/godot.linuxbsd.tools.x86_64.no_rdynamic() [0xb0ad5f] (/home/akien/Projects/godot/godot.git/modules/gdscript/gdscript.cpp:175)
[8] ./bin/godot.linuxbsd.tools.x86_64.no_rdynamic() [0xb0c41e] (/home/akien/Projects/godot/godot.git/modules/gdscript/gdscript.cpp:406)
[9] ./bin/godot.linuxbsd.tools.x86_64.no_rdynamic() [0x49b029e] (/home/akien/Projects/godot/godot.git/core/object/object.cpp:843)
[10] ./bin/godot.linuxbsd.tools.x86_64.no_rdynamic() [0x4479a86] (/home/akien/Projects/godot/godot.git/core/os/main_loop.cpp:61 (discriminator 4))
[11] ./bin/godot.linuxbsd.tools.x86_64.no_rdynamic() [0x20fd63] (/home/akien/Projects/godot/godot.git/platform/linuxbsd/os_linuxbsd.cpp:571)
[12] ./bin/godot.linuxbsd.tools.x86_64.no_rdynamic() [0x209eec] (/home/akien/Projects/godot/godot.git/platform/linuxbsd/godot_linuxbsd.cpp:74)
[13] /lib64/libc.so.6(+0x23677) [0x7fb97e2ae677] (??:0)
[14] /lib64/libc.so.6(__libc_start_main+0x85) [0x7fb97e2ae735] (??:0)
[15] ./bin/godot.linuxbsd.tools.x86_64.no_rdynamic() [0x209ce1] (??:?)
-- END OF BACKTRACE --
================================================================
Size difference:
-rwxr-xr-x 1 akien akien 697M Sep 22 13:41 godot.linuxbsd.tools.x86_64.no_rdynamic*
-rwxr-xr-x 1 akien akien 731M Sep 22 13:41 godot.linuxbsd.tools.x86_64.rdynamic*
However gdb has no problem getting the stacktrace from the build without -rdynamic so this might be a problem with our crash handler. It does imply that even with debug symbols, our release_debug builds currently wouldn't give a meaningful backtrace.
(gdb) bt
#0 0x00000000043cc3d7 in core_bind::OS::crash (this=0x8eb3ee0, p_message=...) at core/core_bind.cpp:232
#1 0x0000000000756b14 in call_with_ptr_args_helper<__UnexistingClass, String const&, 0ul> (p_instance=0x8eb3ee0,
p_method=(void (__UnexistingClass::*)(__UnexistingClass * const, const String &)) 0x43cc392 <core_bind::OS::crash(String const&)>, p_args=0x7fffffff9a20) at ./core/variant/binder_common.h:283
#2 0x0000000000756737 in call_with_ptr_args<__UnexistingClass, String const&> (p_instance=0x8eb3ee0,
p_method=(void (__UnexistingClass::*)(__UnexistingClass * const, const String &)) 0x43cc392 <core_bind::OS::crash(String const&)>, p_args=0x7fffffff9a20) at ./core/variant/binder_common.h:531
#3 0x00000000007563d0 in MethodBindT<String const&>::ptrcall (this=0x8eb4c30, p_object=0x8eb3ee0, p_args=0x7fffffff9a20, r_ret=0x0) at ./core/object/method_bind.h:329
#4 0x0000000000c647a4 in GDScriptFunction::call (this=0xaabc110, p_instance=0xb7736a0, p_args=0x0, p_argcount=0, r_err=..., p_state=0x0) at modules/gdscript/gdscript_vm.cpp:1962
#5 0x0000000000b0ad5f in GDScript::_create_instance (this=0xb773e20, p_args=0x0, p_argcount=0, p_owner=0x7fffac002d70, p_is_ref_counted=false, r_error=...) at modules/gdscript/gdscript.cpp:175
#6 0x0000000000b0c41e in GDScript::instance_create (this=0xb773e20, p_this=0x7fffac002d70) at modules/gdscript/gdscript.cpp:406
#7 0x00000000049b029e in Object::set_script (this=0x7fffac002d70, p_script=...) at core/object/object.cpp:843
#8 0x0000000004479a86 in MainLoop::initialize (this=0x7fffac002d70) at core/os/main_loop.cpp:61
#9 0x000000000020fd63 in OS_LinuxBSD::run (this=0x7fffffffcff0) at platform/linuxbsd/os_linuxbsd.cpp:563
#10 0x0000000000209eec in main (argc=3, argv=0x7fffffffd548) at platform/linuxbsd/godot_linuxbsd.cpp:72
There was a problem hiding this comment.
For now I'm adding a comment to clarify that, but we might be able to get rid of the flag by reworking the crash handler.
49fff8a to
a313013
Compare
There was a problem hiding this comment.
This changes behavior, tests will now be compiled also for template builds with the dev=yes convenience shortcut.
We already have tests working on non-editor builds so there's no reason to exclude them when full-blown dev mode is requested.
There was a problem hiding this comment.
@Faless I started from https://github.com/godotengine/godot-cpp/blob/master/tools/targets.py but as I started syncing with the platform-specific detect.py I had to change some things which should likely be unified further in godot-cpp:
- Removed
0to3flags as3isn't valid for MSVC, and IMO the use for these is limited. I think users can specify what they need withCCFLAGSif they need to override what we have (we could addoptimize=customfor this if need be, to make sure we don't end up adding-O0fromoptimize=noneafter user-specifiedCCFLAGS). - Removed
autoas the logic is more complex than this, instead we handle it inSConstructwhen handling the targets. - Removed forcing debug symbols on
-dev(exdebug) builds. There's a sane default now, but I think we should still give precedence to thedebug_symbolsoption if specified. - MSVC:
- Removed
/Z7for builds without debug symbols. Copy paste mistake I believe? - Added
/FSfordebug_symbols=yesas this was used inplatform/windows/detect.py. Didn't assess what it is/does. - Removed
_DEBUGas we weren't defining it before. I think it's meant to be set by MSVC when usingMTdorMDdand IMO shouldn't be tied todebug_symbols, which is something we can strip (adebug_symbols=yesbuild stripped should be similar to adebug_symbols=nobuild IMO). sizeoptimization: the proper flag for this is/O1: https://learn.microsoft.com/en-us/cpp/build/reference/o1-o2-minimize-size-maximize-speed?view=msvc-170./Osis a subset of/O1but the equivalent to GCC-Osis/O1as far as I know (this also makes a case for not exposing0-3flags as-O1and/O1are quite different).- Added
env.Append(LINKFLAGS=["/OPT:REF"])which was in the Windowsdetect.py.
- Removed
- GCC/Clang:
- Added
speed-debugfor-O2on what used to berelease_debugbuilds, as that's what we used there (apparently to keep potential backtraces somewhat usable).
- Added
There was a problem hiding this comment.
I think all the changes are good, and I agree having a optmize=custom that requires you to set CCFLAGS seems reasonable.
/FS seems to be a partial workaround to windows having issues with multiprocessing.
They say it helps though it doesn't actually fix all the problems: https://learn.microsoft.com/en-us/cpp/build/reference/fs-force-synchronous-pdb-writes?view=msvc-170
Seems safe to use.
There was a problem hiding this comment.
I added optimize=custom in the latest iteration.
There was a problem hiding this comment.
A couple differences for Android:
optimize=sizeused to be-Oz, now it's-Oslike for all other platforms.-Ozis apparently a more aggressive size optimization flag, but for the Web build we have this info:
# -Os reduces file size by around 5 MiB over -O3. -Oz only saves about
# 100 KiB over -Os, which does not justify the negative impact on
# run-time performance.
target=debugused to force no optimization (-O0), now it uses debug optimization (-Og) like other platforms, which is a subset of-O1which does not hinder debugging.
There was a problem hiding this comment.
Some of these defines seem to be overkill for Android:
-fno-limit-debug-infoisn't defined on any other platform so not sure why it's needed here. Seems like it was added a long time ago by @RandomShaper in Improve Android build (Clang + tidyness) #6958, do you remember why?- Undefining
NDEBUGshouldn't be needed since we don't define it. But most importantly we want to avoid exceptions and AFAIK this controls exception behavior inassertso I'm not sure why we specifically want to re-enable it. Was also added in Improve Android build (Clang + tidyness) #6958. _DEBUGAFAIK is a MSVC specific define used for CRT debugging, so it makes no sense for Android. It's been there since Godot was open sourced, can likely be safely removed as it was likely an oversight.
There was a problem hiding this comment.
In Android I tried to match the flags set by the NDK as close as possible. However, I understand that some may not be technically needed and the NDK is adding them to support Android tooling (i.e., Android Studio). It's hard to tell. Maybe @m4gr3d has some insight on how convenient it'd be to remove it.
There was a problem hiding this comment.
- Does it make a difference for iOS if we use
-gor-gdwarf-2? If not we can remove that first line. - Worth noting that the change will also make iOS
template-devbuild (optimize=debug) use-Oglike other platforms instead of-O0like before. _DEBUGis likely unnecessary as this is a MSVC specific macro.- Not sure what
DEBUG=1does and why this is only defined on iOS.
There was a problem hiding this comment.
Macros are probably just copy-pasted for other platform, I do not think it will do anything.
gdwarf-2 is likely also unnecessary, probably for using debuggers on old macOS versions (IIRC, default for current clang is drawf-4, and Xcode on macOS older than 10.10 do not support it).
There was a problem hiding this comment.
I removed the check since we don't support editor build on iOS for now. Could be readded once it's needed.
There was a problem hiding this comment.
@Faless By unifying the code with other platforms a couple changes were made here:
optimize=speednow uses-O3fortemplate-releasebut-O2fortemplate-debug/editor, like on other platforms. We optimize for size by default anyway so I figured this isn't a big deal?-devbuilds (extarget=debug) default todebugoptimization level i.e.-Og, instead of-O1as was set here. It seemed to me the change is fine asdebugis not something we ship, and-Ogis supposed to be fairly close to-O1, with the exception of those optimizations that hinder debugging.
There was a problem hiding this comment.
Our handling of SSE2 is a bit arbitrary. We seem to forcefully define it for Windows MinGW and macOS x86_64 with target=release specifically, and not on any other platform:
$ rg msse2
platform/windows/detect.py
496: env.Append(CCFLAGS=["-msse2"])
platform/macos/detect.py
95: env.Prepend(CCFLAGS=["-msse2"])
modules/raycast/SCsub
70: env_raycast.Append(CPPFLAGS=["-msse2", "-mxsave"])
There was a problem hiding this comment.
@Faless Worth noting that I changed this too while at it. I've changed all cases of "\.tools" but I'm not sure that covers everything for the Web platform.
a313013 to
7aa7959
Compare
|
I noticed this size difference between a Edit: The difference is that for LinuxBSD we didn't set |
The size difference doesn't sound weird to me. What confuses me is why we weren't using |
I'm referring to this in particular: I wouldn't trust that it's better for debugging, though. There have been problems in the past, like |
aaronfranke
left a comment
There was a problem hiding this comment.
Looks really good overall.
Faless
left a comment
There was a problem hiding this comment.
Really good work.
The only remark I would like to add is about:
This one could possibly be split into a separate dev_build=yes option, which would bring down the target enum to "editor", "template-release", "template-debug", which are effectively what we ship. Thoughts? This idea is growing on me as I type it.
I wholeheartedly support doing this to avoid further confusion among the users.
And I think dev_build=yes should simply add the DEV_ENABLED flag and set default values for debug symbols and optimization levels so they can still be overridden.
By splitting dev_build=yes engine developers could simply set it the custom.py and have those defaults set for all targets (maybe ignore it if target is "template-release"?).
There was a problem hiding this comment.
I think all the changes are good, and I agree having a optmize=custom that requires you to set CCFLAGS seems reasonable.
/FS seems to be a partial workaround to windows having issues with multiprocessing.
They say it helps though it doesn't actually fix all the problems: https://learn.microsoft.com/en-us/cpp/build/reference/fs-force-synchronous-pdb-writes?view=msvc-170
Seems safe to use.
|
Here we go! As a reminder:
|
|
How about putting that in documentation? IIRC there's a whole section on compiling with Scons... |
Sure, but one thing at a time. See #66242 (comment) |
| f'bin\\godot.windows.{config}{dev}.{plat_id}{f".{name}" if name else ""}.exe' | ||
| for config in ModuleConfigs.CONFIGURATIONS | ||
| for plat_id in ModuleConfigs.PLATFORM_IDS | ||
| for dev in ModuleConfig.DEV_SUFFIX |
There was a problem hiding this comment.
Typo? Should be ModuleConfigs. I get an error immediately running scons platform=windows dev_build=yes vsproj=yes -j10. NameError: name 'ModuleConfig' is not defined
There was a problem hiding this comment.
Yes, feel free to make a PR if you can also validate that it actually works fine to build in VS.
|
Also cannot build vsproj: |
Introduced in godotengine/godot#66242 the `tools=yes/no` option was removed and merged into the `target` preset. Replace the `tools=yes/no` calls in the documentation to the new template target presets `editor`, `template_release` and `template_debug`.
Introduced in godotengine/godot#66242 the `tools=yes/no` option was removed and merged into the `target` preset. Replace the `tools=yes/no` calls in the documentation to the new template target presets `editor`, `template_release` and `template_debug`.
Introduced in godotengine/godot#66242 the `tools=yes/no` option was removed and merged into the `target` preset. Replace the `tools=yes/no` calls in the documentation to the new template target presets `editor`, `template_release` and `template_debug`.
) Document the Unification of tools/target build type configuration Introduced in godotengine/godot#66242 the `tools=yes/no` option was removed and merged into the `target` preset. Includes miscellaneous fixes identified during code review as well Co-authored-by: Raul Santos <raulsntos@gmail.com> Co-authored-by: Marius Hanl <66004280+Maran23@users.noreply.github.com> Co-authored-by: Clay John <claynjohn@gmail.com>
Implements godotengine/godot-proposals#3371.
TL;DR:
scons target=editor dev_build=yesdev_mode=yeswhich is an alias forverbose=yes warnings=extra werror=yes tests=yesscons target=editorscons target=editor debug_symbols=yes.o,.a,.obj,.lib) and the Godot binary in bin are changing names, so you might want to clean your Git checkout to remove the old ones to save disk space, and adjust any scripts/config you might have that runs the Godot binary to use the new name.New
targetpresetsThe
toolsoption is removed andtargetchanges to use three new presets,which match the builds users are familiar with. These targets control the
default optimization level and enable editor-specific and debugging code:
editor: Replacestools=yes target=release_debug.TOOLS_ENABLED,DEBUG_ENABLED,-O2//O2template_debug: Replacestools=no target=release_debug.DEBUG_ENABLED,-O2//O2template_release: Replacestools=no target=release.-O3//O2New
dev_buildoptionThe previous
target=debugis now replaced by a separatedev_build=yesoption, which can be used in combination with either of the three targets,
and changes the following:
dev_build: DefinesDEV_ENABLED, disables optimization (-O0//0d),enables generating debug symbols, does not define
NDEBUGsoassert()works in thirdparty libraries, adds a
.devsuffix to the binary name.Note: Unlike previously,
dev_builddefaults to off so that users whocompile Godot from source get an optimized and small build by default.
Engine contributors should now set
dev_build=yesin their build scripts orIDE configuration manually.
Changed binary names
The name of generated binaries and object files are changed too, to follow
this format:
godot.<platform>.<target>[.dev][.double].<arch>[.<extra_suffix>][.<ext>]For example:
godot.linuxbsd.editor.dev.arm64godot.windows.template_release.double.x86_64.mono.exeBe sure to update your links/scripts/IDE config accordingly.
More flexible
optimizeanddebug_symbolsoptionsThe optimization level and whether to generate debug symbols can be further
specified with the
optimizeanddebug_symbolsoptions. So the defaultvalues listed above for the various
targetanddev_buildcombinationsare indicative and can be replaced when compiling, e.g.:
scons p=linuxbsd target=template_debug dev_build=yes optimize=debugwill make a "debug" export template with dev-only code enabled,
-Ogoptimization level for GCC/Clang, and debug symbols. Perfect for debugging
complex crashes at runtime in an exported project.
The above description was updated to match the latest state of this PR.
Original description and comments
There's a lot to discuss there, I'll try to make a list of the main point.
As described in the proposal, this PR unifies the current
toolsandtargetcombinations in a singletargetenum, with the intention of making the valid build targets clearer, and to clarify the confusion around the meaning of the word "debug".The targets implemented here are therefore:
Notes
targetmostly governs three things:-g3 -Og)dev_build=yesoption, which would bring down thetargetenum to"editor", "template-release", "template-debug", which are effectively what we ship. Thoughts? This idea is growing on me as I type it.optimizeanddebug_symbolscan be overridden for any target.target=editor-dev optimize=size debug_symbols=noor something. It doesn't make much sense per se but the flexibility can be useful in other situations liketarget=template-release optimize=speed-debug debug_symbols=yes. This is not exactly the same astemplate-debugas the debug features are still off - so it can be pertinent to debug an issue that only happens in release export templates and may be linked to not haveDEBUG_ENABLEDcode.opt,toolsanddebugto just usetarget, so the build objects and final binary names are changing. E.g.scons p=linuxbsd target=editor-dev arch=x86_64will producebin/godot.linuxbsd.editor-dev.x86_64.sconswithout target is nowtarget=editor, which will implyoptimize=speed-debug debug_symbols=yestools=yes target=debug debug_symbols=yeswhich means a debug-optimized (-Og) (or actually one some platforms it would be unoptimized-O0) build withDEV_ENABLED.target=editor-devto get back the previous behavior.optimizeanddebug_symbolsin the SCons help are not really accurate as they eventually depend on thetarget. I could addautoas a default to clarify this, though this can be a bit tricky to handle properly.optimizeanddebug_symbolslevels are now unified inSConstruct, right afterconfigure(needs to be after forenv.msvcto be valid).Tool.detect.py, which should be assessed. There are also some default values changed which I'll point out in review comments.godot-cppwould need to be ported to support something similar probably. Whether or not it can/should might also inform us on whether the set of target presets proposed here is actually the best design or not.SHLIBSUFFIXDEBUG=1,DEBUG:FULL_DEBUG,NDEBUG... this needs a thorough review and unification to make sure we do the right thing in the right situation. Notably, it used to be tied to either usingdebug_symbols=yesortarget=debugbut I'm not sure it's a good idea for all of these.DEBUG,_DEBUGandNDEBUGdefines #66303.SCsubs via the env, e.g.env.target.editor_build/env.target.template_build/env.target.debug_features, etc.CCFLAGSandLINKFLAGS). I don't know why, but I guess theLINKFLAGSlines can be safely dropped?-ftree-vectorizefor both speed and size optimizations, and-fomit-frame-pointerfor speed optimizations. This is apparently supported by GCC too, and possibly by Emscripten (LLVM based too), so should we define them consistently for all platforms? On MSVC they're somewhat part of/O1//O2already by way of/Oy.-fomit-frame-pointerand-ftree-vectorizeflags for optimized builds #66296 and SCons: Remove redundant-fomit-frame-pointerand-ftree-vectorize#66297.