Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

illumos support#12410

Closed
LiamSnow wants to merge 6 commits into
fish-shell:masterfish-shell/fish-shell:masterfrom
LiamSnow:illumos-supportLiamSnow/fish-shell:illumos-supportCopy head branch name to clipboard
Closed

illumos support#12410
LiamSnow wants to merge 6 commits into
fish-shell:masterfish-shell/fish-shell:masterfrom
LiamSnow:illumos-supportLiamSnow/fish-shell:illumos-supportCopy head branch name to clipboard

Conversation

@LiamSnow

@LiamSnow LiamSnow commented Feb 3, 2026

Copy link
Copy Markdown
Contributor

Added fixes to support illumos. Closes issue #11099

@LiamSnow

LiamSnow commented Feb 3, 2026

Copy link
Copy Markdown
Contributor Author

Lmk if I should add tests and/or changes to the change log

@LiamSnow

LiamSnow commented Feb 3, 2026

Copy link
Copy Markdown
Contributor Author

@krobelus What would you like me to do for casting flags? I made it more concise but now clippy is failing. I think a clippy ignore is probably a bad solution here. From my understanding, the only real solution is going to be the original version (cfg_if) with some duplication.

@danielrainer

Copy link
Copy Markdown

We use #[allow(clippy::unnecessary_cast)] for several such situations. If we want to avoid unsafe casts, I think we'd need a separate code path doing something like https://doc.rust-lang.org/std/primitive.i32.html#method.try_from-4 for the configurations where a type conversion is needed.

@LiamSnow

LiamSnow commented Feb 6, 2026

Copy link
Copy Markdown
Contributor Author

I think we should just remove the type annotation on flags. Leaves us with just:

let mut flags = 0;
flags |= libc::POSIX_SPAWN_SETSIGDEF;
flags |= libc::POSIX_SPAWN_SETSIGMASK;
if desired_pgid.is_some() {
    flags |= libc::POSIX_SPAWN_SETPGROUP;
}
attr.set_flags(flags.try_into().expect("Flags should fit in c_short"))?;

@krobelus krobelus added this to the fish 4.5 milestone Feb 8, 2026
@krobelus

krobelus commented Feb 8, 2026 via email

Copy link
Copy Markdown
Contributor

@LiamSnow

LiamSnow commented Feb 8, 2026

Copy link
Copy Markdown
Contributor Author

Added. Lmk if you want any changes. Otherwise, should be good to merge.

krobelus pushed a commit that referenced this pull request Feb 8, 2026
krobelus pushed a commit that referenced this pull request Feb 8, 2026
krobelus pushed a commit that referenced this pull request Feb 8, 2026
@krobelus krobelus closed this in bdb8e48 Feb 8, 2026
@krobelus

krobelus commented Feb 8, 2026 via email

Copy link
Copy Markdown
Contributor

@yjhn yjhn mentioned this pull request Mar 5, 2026
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.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.