Release: v2.51.0 #1933
Release: v2.51.0 #1933
Conversation
Currently states it's a M2 but according to https://support.apple.com/en-us/102852 and https://everymac.com/ultimate-mac-lookup/?search_keywords=Mac14%2C12 it's a M2 Pro.
…message truncation Fixes #1919
Fixes twin detection
…and related fields ... by accurately handling process names with spaces or special characters.
Add GNU/Hurd support
Tested in VM
Support Android
There was a problem hiding this comment.
Pull Request Overview
This is a release PR for version 2.51.0 that makes comprehensive changes to the module system and adds several new features.
- Standardizes module function return types from void to bool to indicate success/failure
- Adds new string buffer functions for integer and double formatting using yyjson
- Extends platform support to include GNU Hurd
Reviewed Changes
Copilot reviewed 223 out of 226 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/strbuf.c | Adds comprehensive tests for new integer and double string buffer functions |
| src/util/smbiosHelper.c | Adds GNU Hurd platform support alongside existing Unix-like systems |
| src/util/platform/FFPlatform_unix.c | Extends executable path detection to support GNU Hurd |
| src/util/arrayUtils.h | New utility header providing safe array size macros with compile-time type checking |
| src/util/FFstrbuf.h | Adds function declarations for new integer and double string buffer functions |
| src/util/FFstrbuf.c | Implements new string buffer functions for formatting integers and doubles |
| src/options/general.h | Extends platform-specific conditional compilation to include GNU Hurd |
| src/options/general.c | Updates platform checks throughout to include GNU Hurd support |
| src/options/display.c | Improves debug flag handling in release builds with proper error messaging |
| src/modules//.h | Updates all module print function signatures to return bool instead of void |
| src/modules//.c | Updates all module implementations to return success/failure status |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if (__builtin_expect(value > 1e21 || value < -1e21, false)) | ||
| { | ||
| // If the value is too large, yyjson_write_number will write it in scientific notation | ||
| return; | ||
| } |
There was a problem hiding this comment.
Magic number 1e21 should be defined as a named constant to improve code readability and maintainability. Consider defining it as something like FF_DOUBLE_SCIENTIFIC_NOTATION_THRESHOLD.
Copilot uses AI. Check for mistakes.
| @@ -176,7 +180,7 @@ void ffInitOSOptions(FFOSOptions* options) | |||
| #elif __Haiku__ | |||
| "" | |||
| #else | |||
| "?" | |||
| "" | |||
There was a problem hiding this comment.
The hardcoded Unicode character should be defined as a named constant or documented with a comment explaining what symbol this represents and why it was chosen as the fallback.
Copilot uses AI. Check for mistakes.
| ffStrbufSetF(&key, "%s (%s)", FF_DISK_MODULE_NAME, disk->mountpoint.chars); | ||
| } | ||
| else |
There was a problem hiding this comment.
The logic for setting the key has become complex with nested conditionals. Consider extracting the key formatting logic into a separate helper function to improve readability and maintainability.
Copilot uses AI. Check for mistakes.
| ffStrbufSetStatic(&ds->wmProcessName, "WindowManager"); | ||
| ffStrbufSetStatic(&ds->wmPrettyName, "Window Manager"); | ||
| ffStrbufSetStatic(&ds->wmProcessName, "surfaceflinger"); | ||
| ffStrbufSetStatic(&ds->wmPrettyName, "SurfaceFlinger"); |
There was a problem hiding this comment.
@CarterLi Are you sure?
I thought that (quoting you)
"It's fastfetch for android. We are talking about android system. The display server API is Display and the window manager API is WindowManager"
Did you change your mind?
termux/termux-packages#25266 (comment)
Note
The reason I am interested in this topic and your decision-making process behind this is actually not about the XFCE thing, it is really because historically, I had a conversation with someone else who said this:
so actually, from this full context, you can see that, despite what one might assume, you have actually taken my side in my argument with twaik by choosing that same name for the Android display server which I argued and they argued against,
so really I am wondering if there is a source that you used for this change, which I could also copy and send to people in order to back up my own similar claim that "the name of the default display server of Android is 'SurfaceFlinger'"
(also I will still make a PR I was talking about for detecting XFCE in Mac, Windows and Android but I did not finish yet)
There was a problem hiding this comment.
When I initially wrote the code, I found both SurfaceFlinger and WindowManager. I didn't quite understand the differences between them. I decided to use WindowManger because, well, WM in *nix world actually means window manager.
Some time ago I happened to find that there is actually a process in Android system called surfaceflinger in ADB shell.
PD2408:/ $ ps -ef | grep surface
system 1935 1 6 07:51:28 ? 00:21:38 surfaceflinger
shell 25096 22911 1 13:53:07 pts/0 00:00:00 grep surface
I just read https://source.android.com/docs/core/graphics/surfaceflinger-windowmanager carefully. I think SurfaceFlinger acts like a display server, and WindowManger is actually a window manager. There is just no process called windowmanager in Android system. Instead, WindowManager is a system service managed by system_server.
There was a problem hiding this comment.
Some time ago I happened to find that there is actually a process in Android system called surfaceflinger in ADB shell.
Yes I agree, and I will add additional information that I know about this process from the perspective that I find important, in case it is interesting.
this process, which is launched from the file stored at /system/bin/surfaceflinger,
is the binary which in Android provably serves the same function that the process launched from /usr/bin/Xorg (traditional X11) or /usr/bin/mutter (typical Wayland example) does on Desktop Linux.
In any modern OS that uses a Linux kernel and typical graphics hardware, the lowest level kernel-dependent API that userspace can interact with to control the screen is currently libdrm.
You know about this since there are some references to and use of libdrm in the code of fastfetch, but libdrm is actually used almost always in newly-shipping Android, Desktop Linux, Mobile Linux, and all other Linux-kernel-based operating systems, regardless of the exact userspace. That is because the entire GUI of userspace, no matter what it is, must interact with libdrm at some point in order to control the screen efficiently. (for simple explanation I am ignoring older solutions that were used before the creation of libdrm because in the 2020s they are usually considered obsolete and are not frequently used in new products)
Here is the location inside the code of Xorg where that initially happens (can sometimes also be located inside modular drivers specific to hardware, but this is the unified implementation maintained within the main tree of Xorg):
Here is the location inside mutter where that initially happens (every different Wayland compositor must individually do this on its own, which is why each Wayland compositor is usually considered to be an independent display server implementation, but they are supposed to be compatible with the same Application Programs, by making the servers all use libwayland-server.so.0 and making the Application Programs all use libwayland-client.so.0):
And finally, here is the location inside the code of surfaceflinger where that initially happens in MakeDrmModeResUnique():
which is called from DrmDevice::DrmDevice()
which is called from DrmDevice::CreateInstance()
which is called from ResourceManager::Init()
There is then some heavy abstraction involving build-time-generated C++ code files, but in some way that is called from ComposerClient::init()
which is called from ComposerClientImpl::create()
which is called from ComposerImpl::createClient()
which is called from HidlComposer::HidlComposer()
which is called from Composer::create()
which is called from HWComposer::HWComposer()
which is called from DefaultFactory::createHWComposer()
which is called from SurfaceFlinger::init()
at this point up the call stack is literally inside SurfaceFlinger.cpp, which is compiled and installed into /system/bin/surfaceflinger by platform/frameworks/native/services/surfaceflinger/Android.bp
As you can see, from the perspective of libdrm and the kernel, all "display server" userspace programs must use at least some of the same APIs to set up and launch, so if the use of those APIs is tracked through each userspace codebase, the identity of each program that the kernel and libdrm "perceive" as the system's display server is revealed.
There was a problem hiding this comment.
Here is a fork of Android that I am trying to help develop in which /system/bin/surfaceflinger is not present (making it a truly headless by default Android ROM), and its only currently practically available display server (optionally) is /data/data/com.termux/files/usr/bin/Xvnc which is a kind of X11,
https://github.com/termux/termux-docker
I don't talk about that distro (termux-docker) much because it currently has some build-reproducibility issues (the source code is not available and I have rewritten it from scratch) but when those issues are fixed, I will feel more comfortable promoting it.
(Also it's totally OK, in my opinion, if fastfetch in termux-docker still reports "WindowManager" or "SurfaceFlinger" or whatever you prefer, for now, even if neither of them are technically present there, since it is such a niche and abnormal Android fork. If I want to change that I will try making a PR in fastfetch in the future at some point)
There was a problem hiding this comment.
In any modern OS that uses a Linux kernel and typical graphics hardware, the lowest level kernel-dependent API that userspace can interact with to control the screen is currently libdrm.
You know about this since there are some references to and use of libdrm in the code of fastfetch,
Yeah.
but libdrm is actually used almost always in newly-shipping Android, Desktop Linux, Mobile Linux, and all other Linux-kernel-based operating systems, regardless of the exact userspace. That is because the entire GUI of userspace, no matter what it is, must interact with libdrm at some point in order to control the screen efficiently.
Yes and no. libdrm itself doesn't really matter; DRM matters. libdrm is just a wrapper of some ioctl syscalls.
DRM (Direct Rendering Manager) is the GPU driver subsystem of Linux. For example: radeon/amdgpu, i915/xe, nvidia-drm. On Android, it's MSM for Qualcomm Adreno and mediatek for MediaTek. You can talk to them by opening the DRI (Direct Rendering Infrastructure, the userspace interface of DRM) device file `/dev/dri/{cardN,renderN}. However, opening the device file is restricted on Android, you must root your phone to make libdrm work. That's why fastfetch don't use libdrm to detect display info.
BTW, DRM is not a must have for X11 server and Wayland composers to work. They can still write frame buffer directly.
There was a problem hiding this comment.
I am aware that libdrm is a userspace wrapper of the kernel interface but most software, including all the software I linked (in the larger message), uses libdrm to interact with it.
BTW, DRM is not a must have for X11 server and Wayland composers to work. They can still write frame buffer directly.
it varies, but on most products shipping in the 2020s the other methods are inefficient because drivers are no longer made to enable acceleration on present-day hardware without direct rendering manager so they would use software rendering. That is why I referred to the old solutions as "obsolete". but of course they exist, so you do work with them and support them which makes sense.
That's why fastfetch don't use libdrm to detect display info [on Android].
in Termux generally for other packages, our practice is to check if the current process has permission to access a file by attempting to open it, which it might not, but it also might if the root user on a rooted device was used to run the program, then only perform the action if the file is accessible, but if you prefer to use different criteria to decide whether to check libdrm for info in fastfetch that is completely fine.
No description provided.