Bug 241077 – Include <sys/sysinfo.h> only on FreeBSD and Linux

• Status: RESOLVED
• Resolution: FIXED
• Component: bmalloc

[Leonardo Taccari]

Include <sys/sysinfo.h> only on FreeBSD and Linux

[Leonardo Taccari]

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

[Leonardo Taccari]

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.

[Leonardo Taccari]

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.
```

[Fujii Hironori]

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?

[Leonardo Taccari]

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!)

[Leonardo Taccari]

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.

[Leonardo Taccari]

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!

[Fujii Hironori]

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.

[Fujii Hironori]

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

[Leonardo Taccari]

(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).

[Leonardo Taccari]

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.

[Leonardo Taccari]

(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!

[Leonardo Taccari]

(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.

[EWS]

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

[EWS]

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].

[Radar WebKit Bug Importer]

<rdar://problem/94123044>