Bug 241077

Patch coming... sorry, that was my first time using Tools/Scripts/webkit-patch and it seems that it didn't well well!

Created attachment 459836 [details] Include <sys/sysinfo.h> only on FreeBSD and Linux sysinfo(2)/sysinfo(3) is used only on FreeBSD and Linux and could be not available in other Unix-like operating systems.

Only for the record here the complete transcript when trying to build JSC via `Tools/Scripts/build-jsc --jsc-only`: ``` [...] -- Configuring done -- Generating done -- Build files have been written to: ...WebKit/WebKitBuild/Release [363/1074] Building CXX object Source/bmalloc/CMakeFiles/bmalloc.dir/bmalloc/AvailableMemory.cpp.o FAILED: Source/bmalloc/CMakeFiles/bmalloc.dir/bmalloc/AvailableMemory.cpp.o /usr/bin/c++ -DBUILDING_JSCONLY__ -DBUILDING_WITH_CMAKE=1 -DBUILDING_bmalloc -DHAVE_CONFIG_H=1 -DPAS_BMALLOC=1 -I...WebKit/Source/bmalloc -I...WebKit/Source/bmalloc/bmalloc -I...WebKit/Source/bmalloc/libpas/src/libpas -fdiagnostics-color=always -Wextra -Wall -pipe -Wno-odr -Wno-stringop-overflow -Wno-nonnull -Wno-array-bounds -Wno-expansion-to-defined -Wno-noexcept-type -Wno-psabi -Wno-misleading-indentation -Wno-maybe-uninitialized -Wwrite-strings -Wundef -Wpointer-arith -Wmissing-format-attribute -Wcast-align -Wno-tautological-compare -fno-strict-aliasing -fno-exceptions -fno-rtti -O3 -DNDEBUG -fPIC -fvisibility=hidden -fvisibility-inlines-hidden -Wno-missing-field-initializers -Wno-cast-align -std=c++2a -MD -MT Source/bmalloc/CMakeFiles/bmalloc.dir/bmalloc/AvailableMemory.cpp.o -MF Source/bmalloc/CMakeFiles/bmalloc.dir/bmalloc/AvailableMemory.cpp.o.d -o Source/bmalloc/CMakeFiles/bmalloc.dir/bmalloc/AvailableMemory.cpp.o -c ...WebKit/Source/bmalloc/bmalloc/AvailableMemory.cpp ...WebKit/Source/bmalloc/bmalloc/AvailableMemory.cpp:47:10: fatal error: sys/sysinfo.h: No such file or directory 47 | #include <sys/sysinfo.h> | ^~~~~~~~~~~~~~~ compilation terminated. [370/1074] Building CXX object Source/WTF/wtf/CMakeFiles/WTF.dir/RunLoop.cpp.o ninja: build stopped: subcommand failed. ```

Comment on attachment 459836 [details] Include <sys/sysinfo.h> only on FreeBSD and Linux View in context: https://bugs.webkit.org/attachment.cgi?id=459836&action=review > COMMIT_MESSAGE:4 > +available in other Unix-like operating systems. This commit message doesn't comply with WebKit format. WebKit has own prepare-commit-msg hook. "./Tools/Scripts/git-webkit setup" will setup for you. Could you redo this patch with it?

Comment on attachment 459836 [details] Include <sys/sysinfo.h> only on FreeBSD and Linux Marking it as obsolete because the commit message format is not okay. (Sorry, I didn't known about `Tools/Scripts/git-webkit setup`, thanks to Fujii for pointing it out, patch hopefully adjusted coming soon!)

Created attachment 459840 [details] Include <sys/sysinfo.h> only on FreeBSD and Linux sysinfo(2)/sysinfo(3) is used only on FreeBSD and Linux and could be not available in other Unix-like operating systems.

Hello Fujii, (In reply to Fujii Hironori from comment #4) > Comment on attachment 459836 [details] > Include <sys/sysinfo.h> only on FreeBSD and Linux > > View in context: > https://bugs.webkit.org/attachment.cgi?id=459836&action=review > > > COMMIT_MESSAGE:4 > > +available in other Unix-like operating systems. > > This commit message doesn't comply with WebKit format. WebKit has own > prepare-commit-msg hook. > "./Tools/Scripts/git-webkit setup" will setup for you. Could you redo this > patch with it? Sure! (sorry, I didn't known about it) I have hopefully reformatted it accordingly. If it needs further adjustment please let me know and/or feel free to adjust the commit message as you prefer. Thank you!

Comment on attachment 459840 [details] Include <sys/sysinfo.h> only on FreeBSD and Linux View in context: https://bugs.webkit.org/attachment.cgi?id=459840&action=review > Source/bmalloc/bmalloc/AvailableMemory.cpp:-47 > -#include <sys/sysinfo.h> Unfortunately, sytle checker doesn't like this change. > ERROR: Source/bmalloc/bmalloc/AvailableMemory.cpp:54: "sys/sysinfo.h" already included at Source/bmalloc/bmalloc/AvailableMemory.cpp:50 [build/include] [4] #if BOS(FREEBSD) || BOS(LINUX) #include <sys/sysinfo.h> #endif This condition matches with the implementation of AvailableMemory.cpp. You can style errors with Tools/Scripts/check-webkit-style before uploading. If you use `webkit-patch upload` or `git-webkit pr` scripts to upload your patch, it automatically run check-webkit-style.

If you don't mind, it'd be great if you change another conidtion to `#if OS(LINUX) || OS(FREEBSD)` in this patch to match with the implementation. https://github.com/WebKit/WebKit/blob/main/Source/WTF/wtf/RAMSize.cpp#L34

(In reply to Fujii Hironori from comment #9) > If you don't mind, it'd be great if you change another conidtion to `#if > OS(LINUX) || OS(FREEBSD)` in this patch to match with the implementation. > https://github.com/WebKit/WebKit/blob/main/Source/WTF/wtf/RAMSize.cpp#L34 I would prefer to do that in a separate patch to avoid possible regressions (I think that the components are separate and this bug actually fix build on non-FreeBSD/non-Linux Unix-like operating systems while the change in RAMSize.cpp will possibly impact FreeBSD only).

Created attachment 459854 [details] Include <sys/sysinfo.h> only on FreeBSD and Linux sysinfo(2)/sysinfo(3) is used only on FreeBSD and Linux and could be not available in other Unix-like operating systems.

(In reply to Fujii Hironori from comment #8) > Comment on attachment 459840 [details] > Include <sys/sysinfo.h> only on FreeBSD and Linux > > View in context: > https://bugs.webkit.org/attachment.cgi?id=459840&action=review > > > Source/bmalloc/bmalloc/AvailableMemory.cpp:-47 > > -#include <sys/sysinfo.h> > > Unfortunately, sytle checker doesn't like this change. > > > ERROR: Source/bmalloc/bmalloc/AvailableMemory.cpp:54: "sys/sysinfo.h" already included at Source/bmalloc/bmalloc/AvailableMemory.cpp:50 [build/include] [4] > > #if BOS(FREEBSD) || BOS(LINUX) > #include <sys/sysinfo.h> > #endif > > This condition matches with the implementation of AvailableMemory.cpp. > > You can style errors with Tools/Scripts/check-webkit-style before uploading. > If you use `webkit-patch upload` or `git-webkit pr` scripts to upload your > patch, it automatically run check-webkit-style. Thanks, I have resubmitted it and I hope it's fine this time!

(In reply to Leonardo Taccari from comment #10) > (In reply to Fujii Hironori from comment #9) > > If you don't mind, it'd be great if you change another conidtion to `#if > > OS(LINUX) || OS(FREEBSD)` in this patch to match with the implementation. > > https://github.com/WebKit/WebKit/blob/main/Source/WTF/wtf/RAMSize.cpp#L34 > > I would prefer to do that in a separate patch to avoid possible regressions > (I think that the components are separate and this bug actually fix build on > non-FreeBSD/non-Linux Unix-like operating systems while the change in > RAMSize.cpp will possibly impact FreeBSD only). I have opened https://bugs.webkit.org/show_bug.cgi?id=241099 for that.

Found 2 new test failures: webgl/2.0.0/conformance2/textures/image_bitmap_from_image_bitmap/tex-2d-rgb8ui-rgb_integer-unsigned_byte.html, webgl/2.0.0/conformance2/textures/image_bitmap_from_image_bitmap/tex-3d-srgb8_alpha8-rgba-unsigned_byte.html

Committed r295033 (251128@main): <https://commits.webkit.org/251128@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 459854 [details].

<rdar://problem/94123044>