Bug 241077 - Include <sys/sysinfo.h> only on FreeBSD and Linux
Summary: Include <sys/sysinfo.h> only on FreeBSD and Linux
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: bmalloc (show other bugs)
Version: WebKit Nightly Build
Hardware: All Other
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-05-29 09:49 PDT by Leonardo Taccari
Modified: 2022-05-30 15:00 PDT (History)
4 users (show)

See Also:


Attachments
Include <sys/sysinfo.h> only on FreeBSD and Linux (1.02 KB, patch)
2022-05-29 09:53 PDT, Leonardo Taccari
no flags Details | Formatted Diff | Diff
Include <sys/sysinfo.h> only on FreeBSD and Linux (1.14 KB, patch)
2022-05-29 13:27 PDT, Leonardo Taccari
no flags Details | Formatted Diff | Diff
Include <sys/sysinfo.h> only on FreeBSD and Linux (1009 bytes, patch)
2022-05-30 04:12 PDT, Leonardo Taccari
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Leonardo Taccari 2022-05-29 09:49:40 PDT
Include <sys/sysinfo.h> only on FreeBSD and Linux
Comment 1 Leonardo Taccari 2022-05-29 09:52:10 PDT
Patch coming... sorry, that was my first time using Tools/Scripts/webkit-patch and it seems that it didn't well well!
Comment 2 Leonardo Taccari 2022-05-29 09:53:33 PDT
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.
Comment 3 Leonardo Taccari 2022-05-29 09:55:08 PDT
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 4 Fujii Hironori 2022-05-29 12:55:58 PDT
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 5 Leonardo Taccari 2022-05-29 13:26:22 PDT
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!)
Comment 6 Leonardo Taccari 2022-05-29 13:27:14 PDT
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.
Comment 7 Leonardo Taccari 2022-05-29 13:28:13 PDT
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 8 Fujii Hironori 2022-05-29 16:44:36 PDT
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.
Comment 9 Fujii Hironori 2022-05-29 16:45:47 PDT
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
Comment 10 Leonardo Taccari 2022-05-30 03:57:51 PDT
(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).
Comment 11 Leonardo Taccari 2022-05-30 04:12:04 PDT
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.
Comment 12 Leonardo Taccari 2022-05-30 04:12:58 PDT
(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!
Comment 13 Leonardo Taccari 2022-05-30 04:41:45 PDT
(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.
Comment 14 EWS 2022-05-30 07:19:13 PDT
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
Comment 15 EWS 2022-05-30 15:00:00 PDT
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].
Comment 16 Radar WebKit Bug Importer 2022-05-30 15:00:14 PDT
<rdar://problem/94123044>