Get warp compiling on FreeBSD#9362
Conversation
wayland-client, cctk, fontconfig, x11rb, native-dialog, notify-rust, the WindowingSystem enum and platform/linux all build and run on FreeBSD with the same code we already use for Linux. Replace `target_os = "linux"` with `any(target_os = "linux", target_os = "freebsd")` across app/ and crates/ so FreeBSD hits the Linux branches instead of the cross-platform no-op fallbacks. InputFlags::IUTF8 in local_tty/unix.rs stays Linux/macOS only because nix does not expose that termios flag on FreeBSD. Linux, macOS, and Windows builds are unaffected: every cfg already evaluated true on Linux and continues to.
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @rudrabhoj on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
|
I'm starting a first review of this pull request. I requested changes on this pull request and posted feedback. Comment I requested changes on this pull request and posted feedback. Comment I requested changes on this pull request and posted feedback. Comment I requested changes on this pull request and posted feedback. Comment I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I requested changes on this pull request and posted feedback. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR broadly widens Linux cfg gates so the existing desktop/Linux implementation is compiled on FreeBSD. The mechanical cfg changes are mostly low-risk, but two changed runtime paths now treat FreeBSD as Linux for downloadable artifacts/update mechanisms.
Concerns
- FreeBSD is routed through the Linux autoupdate implementation, which uses Linux AppImage/package-manager release assets and can offer an incompatible update path.
- FreeBSD selects Linux GNU rust-analyzer assets, which are not native FreeBSD binaries and will fail when Warp installs/executes them.
- Security pass: no direct security vulnerabilities found in the changed lines.
Verdict
Found: 0 critical, 2 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| if #[cfg(target_os = "macos")] { | ||
| mac::download_update_and_cleanup(&version_info, &update_id, last_successful_update_id.as_deref(), server_api.http_client()).await | ||
| } else if #[cfg(target_os = "linux")] { | ||
| } else if #[cfg(any(target_os = "linux", target_os = "freebsd"))] { |
There was a problem hiding this comment.
| "rust-analyzer-x86_64-apple-darwin.gz" | ||
| } | ||
| #[cfg(all(target_os = "linux", target_arch = "x86_64"))] | ||
| #[cfg(all(any(target_os = "linux", target_os = "freebsd"), target_arch = "x86_64"))] |
There was a problem hiding this comment.
There was a problem hiding this comment.
Overview
This PR broadly widens Linux-only cfg guards so FreeBSD builds reuse the existing Linux/X11/Wayland/platform implementations.
Concerns
- FreeBSD now selects Linux rust-analyzer release assets, which will install binaries that do not run on FreeBSD.
- The FreeBSD memory-footprint path now uses Linux
/proc/self/statusfields, so it will report 0/empty data on normal FreeBSD systems.
Verdict
Found: 0 critical, 1 important, 1 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| #[cfg(all(any(target_os = "linux", target_os = "freebsd"), target_arch = "x86_64"))] | ||
| { | ||
| "rust-analyzer-x86_64-unknown-linux-gnu.gz" | ||
| } | ||
| #[cfg(all(target_os = "linux", target_arch = "aarch64"))] | ||
| #[cfg(all(any(target_os = "linux", target_os = "freebsd"), target_arch = "aarch64"))] |
There was a problem hiding this comment.
rust-analyzer-*-unknown-linux-gnu.gz; those Linux ELF binaries will not execute on FreeBSD, so keep FreeBSD unsupported here or select a FreeBSD rust-analyzer asset.
| // --------------------------------------------------------------------------- | ||
|
|
||
| #[cfg(target_os = "linux")] | ||
| #[cfg(any(target_os = "linux", target_os = "freebsd"))] |
There was a problem hiding this comment.
💡 [SUGGESTION] This enables the Linux /proc/self/status parser on FreeBSD, where those VmRSS/VmSwap fields are not available; keep FreeBSD on a fallback or use a native FreeBSD memory API so telemetry is not always zero/empty.
035a80c to
df15f82
Compare
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @rudrabhoj on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
|
I'm checking this implementation PR for association with a likely matching ready issue. Powered by Oz |
df15f82 to
1e0a6cc
Compare
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @rudrabhoj on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
1e0a6cc to
6ec7128
Compare
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @rudrabhoj on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
|
I have signed the Contributor License Agreement and fixed problems cited by @oz-agent kindly review again. |
Three of the previous commit's widened cfg sites pulled FreeBSD into code paths that ship Linux-only artifacts or rely on Linux-only filesystems. Pull FreeBSD back out of those, with a real implementation where a portable one exists. `app/src/autoupdate/mod.rs`: revert the cfg widening. The Linux autoupdate path downloads AppImage and package-manager release assets that aren't published for FreeBSD; FreeBSD now falls through to the existing 'not implemented' arm. FreeBSD users update via pkg or source. `crates/lsp/src/servers/rust.rs`: keep FreeBSD detection working (`should_suggest_for_repo`, `is_installed_on_path`, `is_installed_in_data_dir` are OS-agnostic), but make `install` and `fetch_latest_server_metadata` early-return Err on FreeBSD with a message pointing the user at `rustup component add rust-analyzer` or `pkg install rust-analyzer`. The reason auto-download cannot work: warp installs rust-analyzer through `install_from_github`, which pulls from the `rust-lang/rust-analyzer` GitHub release page. That page publishes mac/linux/windows tarballs only; there is no FreeBSD asset to download. The rustup distribution channel (`static.rust-lang.org/dist/`) does build a FreeBSD rust-analyzer as part of the toolchain, and the FreeBSD ports tree ships one as `devel/rust-analyzer`, so either install lands the binary on PATH where warp finds it. `app/src/system/memory_footprint.rs`: split FreeBSD into its own `mod platform` that calls `getrusage(RUSAGE_SELF)` and reports `ru_maxrss`, instead of pretending `/proc/self/status` exists. linprocfs is optional on FreeBSD and almost never mounted; the previous widened cfg would have made the function silently return 0.
6ec7128 to
3aceb2c
Compare
|
I analyzed the request but did not produce any changes. Powered by Oz |
|
/oz-review |
There was a problem hiding this comment.
Overview
This PR broadens many Linux-specific cfg gates to also compile the existing Linux paths on FreeBSD, plus adds a FreeBSD memory-footprint fallback and disables rust-analyzer auto-install metadata fetches where upstream has no FreeBSD asset.
Concerns
- FreeBSD builds still appear to fail in
app/src/debug_dump.rs: the new FreeBSD cfg referencescrate::autoupdate::linux, butapp/src/autoupdate/mod.rsstill exports that module only on Linux. - No security-specific findings were identified in the changed hunks.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| } | ||
|
|
||
| #[cfg(target_os = "linux")] | ||
| #[cfg(any(target_os = "linux", target_os = "freebsd"))] |
There was a problem hiding this comment.
crate::autoupdate::linux::UpdateMethod reference on FreeBSD, but app/src/autoupdate/mod.rs still exports linux only for target_os = "linux", so FreeBSD builds will fail to resolve the module. Either widen that module cfg too or keep this debug dump block Linux-only.
There was a problem hiding this comment.
Overview
This PR broadens many Linux cfg paths to also compile on FreeBSD and adds small FreeBSD-specific handling for memory reporting and rust-analyzer installation metadata.
Concerns
- The debug dump FreeBSD path now references
crate::autoupdate::linux, but that module is still only declared fortarget_os = "linux", so the FreeBSD build will fail when this file is compiled.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| } | ||
|
|
||
| #[cfg(target_os = "linux")] | ||
| #[cfg(any(target_os = "linux", target_os = "freebsd"))] |
There was a problem hiding this comment.
crate::autoupdate::linux::UpdateMethod, but app/src/autoupdate/mod.rs still declares pub mod linux only under #[cfg(target_os = "linux")], so FreeBSD builds fail; either expose the module for FreeBSD or keep this block Linux-only.
`debug_dump.rs` prints `Package type:` from `crate::autoupdate::linux::UpdateMethod::detect()`. The previous commit reverted `autoupdate` back to Linux-only so the `linux` submodule no longer exists on FreeBSD, but the call site here was widened to `any(linux, freebsd)`. That breaks the FreeBSD build. Narrow the cfg back to Linux-only at the call site. FreeBSD has no package-type detection to report; the line is just a debug-dump nicety, so dropping it on FreeBSD is fine.
|
/oz-review |
There was a problem hiding this comment.
Overview
This PR broadens many Linux cfg gates to include FreeBSD and adds a FreeBSD memory-footprint implementation.
Concerns
- The crash-recovery/minidump cfg alias now includes FreeBSD, but the upstream crash-handler/minidumper/minidump-writer stack used by
sentry_minidumpdoes not export FreeBSD implementations, so FreeBSD builds withcrash_reportingenabled will fail.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| fn main() -> Result<()> { | ||
| cfg_aliases! { | ||
| linux_or_windows: { any(target_os = "linux", windows) }, | ||
| linux_or_windows: { any(any(target_os = "linux", target_os = "freebsd"), windows) }, |
There was a problem hiding this comment.
#[cfg(linux_or_windows)] include FreeBSD, so sentry_minidump is compiled there when crash_reporting is enabled; the upstream crash-handler/minidump-writer crates only export Linux/Android, Windows, and macOS implementations, so keep this alias Linux/Windows-only until a FreeBSD minidump backend exists.
There was a problem hiding this comment.
Yes, that's understandable. minidump crate is a project external to warp and it doesn't support FreeBSD yet, it should be fine. I'll soon submit PR to minidump too until then I think this shouldn't be a blocker for a FreeBSD port of warp.
Most
target_os = "linux"cfg sites in this tree gate code that already works fine on FreeBSD. wayland-client, cctk, fontconfig, x11rb, native-dialog, notify-rust, and the WindowingSystem enum all compile and run there using the existing Linux branches. Widening those cfg guards fromtarget_os = "linux"toany(target_os = "linux", target_os = "freebsd")is enough to get a working build.The change is mechanical: 79 files across
app/andcrates/, all of them just adjusting the cfg.One Linux-only carveout:
InputFlags::IUTF8inapp/src/terminal/local_tty/unix.rs. nix does not expose that termios input flag on FreeBSD, and the PTY works without it.Linux, macOS, and Windows are unaffected by construction: every widened cfg already evaluated true on Linux and continues to; neither macOS nor Windows ever matched these guards.
Tested by building
warp-osson FreeBSD 16-CURRENT amd64 with rust 1.92 and launching it under wayland (niri).