Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Illumos/Solaris updates #17341

Merged
merged 11 commits into from
Oct 3, 2023
Merged

Illumos/Solaris updates #17341

merged 11 commits into from
Oct 3, 2023

Conversation

rzezeski
Copy link
Contributor

I'd like to integrate these changes I've made in service of getting newer Zig (0.11.0 and latest master) running on illumos. I've tested these changes on OmniOS where I'm able to compile & run the latest version of ncdu along with my own personal Zig project. I've also managed to run some of the test suite, albeit with failures that need fixing. I spoke to @andrewrk about some of these failures in the Zig IRC channel, and he agreed it would be okay to get my improvements up as a base to start fixing bugs.

Zig currently has no illumos OS tag, only solaris. While these two systems stem from the same codebase, they broke apart in 2010 when Oracle reclosed the source and illumos was formed as an open-source fork based on the latest code drop. After 13 years they have grown enough apart that it is detrimental to consider them the same platform. I would like to propose adding an illumos tag, probably similar to how it's been done in Go and Rust. This work will need to be done if #7152 is to be accepted. But for this initial support I've worked under the existing solaris tag.

These changes should fix #12213.

I've presented this as a series of smaller changes so it might be easier to digest. But I'm happy to squash this into one if that's preferred.

On illumos (and Solaris) there is, by design, only one libc.
Solaris/illumos is multi-lib, so you can't rely on an arbitrary
executable to give you the correct dynamic linker. Besides, it's
always the same path.
@The-King-of-Toasters
Copy link
Contributor

I've built Zig with these patches on an OmniOS VM and it works great. Of course it requires building LLVM+Clang+LLD from source as OmniOS doesn't package LLD. I also found that I needed to compile stage3 with -fPIC to be able to link against libc.

@@ -1575,7 +1575,7 @@ pub const Target = struct {
.netbsd => return copy(&result, "/libexec/ld.elf_so"),
.openbsd => return copy(&result, "/usr/libexec/ld.so"),
.dragonfly => return copy(&result, "/libexec/ld-elf.so.2"),
.solaris => return copy(&result, "/lib/64/ld.so.1"),
.solaris => return copy(&result, "/usr/lib/amd64/ld.so.1"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would /usr/lib/64/ld.so.1 also work here?

@andrewrk
Copy link
Member

I would like to propose adding an illumos tag

Please proceed. Start here:

--- a/lib/std/target.zig
+++ b/lib/std/target.zig
@@ -58,6 +58,7 @@ pub const Target = struct {
             glsl450,
             vulkan,
             plan9,
+            illumos,
             other,
 
             pub inline fn isDarwin(tag: Tag) bool {

From there, compilation errors should guide you along the way.

@The-King-of-Toasters
Copy link
Contributor

Ryan, I've forked your branch and added support for the .illumos tag here. I've confirmed that stage3 builds and runs the stdlib tests on the x86_64-solaris and x86_64-illumos tuples, which was about the safe functionality before. Can you pull these changes into your tree and confirm that they work on your end?

BTW, this will require rebuilding zig1.wasm to be able to parse the new tag (at least it did for me).

- Adds `illumos` to the `Target.Os.Tag` enum. A new function,
  `isSolarish` has been added that returns true if the tag is either
  Solaris or Illumos. This matches the naming convention found in Rust's
  `libc` crate[1].
- Add the tag wherever `.solaris` is being checked against.
- Check for the C pre-processor macro `__illumos__` in CMake to set the
  proper target tuple. Illumos distros patch their compilers to have
  this in the "built-in" set (verified with `echo | cc -dM -E -`).

  Alternatively you could check the output of `uname -o`.

Right now, both Solaris and Illumos import from `c/solaris.zig`. In the
future it may be worth putting the shared ABI bits in a base file, and
mixing that in with specific `c/solaris.zig`/`c/illumos.zig` files.

[1]: https://github.com/rust-lang/libc/tree/6e02a329a2a27f6887ea86952f389ca11e06448c/src/unix/solarish
The 5.11 in uname is not something that is ever updated. There is no
versioning of the illumos system in general. Illumos prefers to rely
on feature detection.

I can't say what Solaris does these days as I do not work at Oracle;
so I left it alone.
@rzezeski rzezeski requested a review from kprotty as a code owner October 2, 2023 23:19
@rzezeski
Copy link
Contributor Author

rzezeski commented Oct 2, 2023

@The-King-of-Toasters We ended up doing the same work pretty much, so I cherry picked your commits.

Though I removed illumos from semver as it doesn't do any versioning at the system level. I can't remember if Oracle Solaris decided to start making use of the uname version again or not, but I decided to leave it alone.

@andrewrk andrewrk merged commit 7733894 into ziglang:master Oct 3, 2023
10 checks passed
@andrewrk
Copy link
Member

andrewrk commented Oct 3, 2023

Nice work!

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.

Cannot build stage 2 on x86-64-illumos
3 participants