Skip to content

gimlet, propolis, agent: fix compilation errors on linux#90

Open
emilyalbini wants to merge 1 commit intomainfrom
ea-kspvtxptpswp
Open

gimlet, propolis, agent: fix compilation errors on linux#90
emilyalbini wants to merge 1 commit intomainfrom
ea-kspvtxptpswp

Conversation

@emilyalbini
Copy link
Copy Markdown
Member

As we are doing in some places already, this fixes compilation errors and warnings when building and running tests on Linux. Having the list of diagnostics polluted in my editor is fairly annoying, and this fixes that.

Most changes are not invasive, except for gating the code handling zones: I can't just cfg out that module outside illumos, as it's relied upon in multiple places. What I ended up doing was creating a ZoneId wrapper that can be either ZoneId(zoneid_t) on illumos or (the stable equivalent of) ZoneId(!) on Linux. This way the rest of the code is not cfg'd out. Another option would be to pub use libc::c_int as zoneid_t on Linux if you'd prefer.

An alternative to this approach would be to compile out the entirety of the propolis and gimlet factories outside of illumos. While that would avoid some of the noise in the code, when looking at the glue between the buildomat server and the factory having LSP working on the OS-agnostic parts is useful.

@emilyalbini emilyalbini requested a review from jclulow March 18, 2026 12:41
@jclulow
Copy link
Copy Markdown
Collaborator

jclulow commented Apr 7, 2026

I'm hesitant to introduce a bunch of ifdef noise into code that's intended to only ever work on one platform. The example you found in the gimlet disk thing is actually an artefact of my having copied and pasted that file out of pilot almost (perhaps entirely?) unchanged as part of standing up the gimlet factory.

I appreciate the value of the LSP stuff. I edit all of the code on illumos under neovim and rust-analyser is certainly helpful there.

Is it possible to tell rust-analyser that you're actually building illumos-only code for the illumos target? I have to imagine we already do something like that with Hubris, as that's presumably not self-hosting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants