Hi Maxime, Maxime Devos <[email protected]> writes:
> Mark H Weaver schreef op vr 05-11-2021 om 15:42 [-0400]: >> + ;; XXX Until we switch back to using GCC, >> + ;; work around <https://bugs.gnu.org/51591>. >> + ,@(if (string=? "i686-linux" (%current-system)) >> + '((substitute* >> "Source/WTF/wtf/CheckedArithmetic.h" >> + (("#define USE_MUL_OVERFLOW 1") >> + "#define USE_MUL_OVERFLOW 0"))) >> + '()) > > For cross-compilation, this should use (%current-target-system) > instead. Also, I'd presume the same issue exists for the Hurd. > I'd suggest: (target-x86-32?). That procedure is not > yet in master (only core-updates{-frozen{,-batched-changes}}), > so it would need to be cherry picked. You're certainly right that the simple test above is not a good example of how best to check if we're building for i686-linux. For that matter, a more proper fix would be the one I proposed in my earlier email: to apply a patch to Source/WTF/wtf/CheckedArithmetic.h that changes "CPU(ARM)" to "(CPU(ARM) || CPU(XXX))", where XXX is the appropriate symbol for 32-bit x86. Such a patch could even be submitted upstream. That's really the kind of approach I'd like to encourage. In this case, I had very little available time to work on it, but wanted to get the problem fixed on i686-linux ASAP. Given this, and the fact that this code is a temporary workaround until the core-updates-frozen merge, and also my assumption that we are quite far from being able to usefully cross-compile WebKitGTK and its dependencies, I don't feel *too* badly about cutting corners here. Having said all of this, I certainly agree that the conditional above should be changed to follow our best practices. Would you like to propose a patch? Thanks very much for your vigilance. Regards, Mark -- Disinformation flourishes because many people care deeply about injustice but very few check the facts. Ask me about <https://stallmansupport.org>.
