Skip to content

fcntl_lock should be supported on Solaris#1226

Merged
sunfishcode merged 1 commit intobytecodealliance:mainfrom
psumbera:solaris-fixes
Dec 8, 2024
Merged

fcntl_lock should be supported on Solaris#1226
sunfishcode merged 1 commit intobytecodealliance:mainfrom
psumbera:solaris-fixes

Conversation

@psumbera
Copy link
Copy Markdown
Contributor

Even when Solaris libc doesn't define LOCK_SH, LOCK_EX, LOCK_NB and LOCK_UN (which are flock() related).

Copy link
Copy Markdown
Member

@sunfishcode sunfishcode left a comment

Choose a reason for hiding this comment

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

Ah, this looks like my mistake. When libc removed LOCK_SH etc., I disabled FlockOperation on Solaris and then disabled fcntl_lock as well to make everything build, forgetting that we still need that on Solaris. The fix looks good; I just suggest adding something like the following comment to explain what's going on:

target_os = "vita",
target_os = "wasi"
)))]
#[cfg(not(any(target_os = "espidf", target_os = "vita", target_os = "wasi")))]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
#[cfg(not(any(target_os = "espidf", target_os = "vita", target_os = "wasi")))]
// Solaris doesn't support `flock` and doesn't define `LOCK_SH` etc., but we
// reuse this `FlockOperation` enum for `fcntl_lock`, so on Solaris we use
// our own made-up integer values.
#[cfg(not(any(target_os = "espidf", target_os = "vita", target_os = "wasi")))]

@sunfishcode
Copy link
Copy Markdown
Member

The CI failure is an unrelated bug now fixed on main.

Even when Solaris libc doesn't define LOCK_SH, LOCK_EX, LOCK_NB and LOCK_UN (which are flock() related).
@sunfishcode sunfishcode merged commit 7355af2 into bytecodealliance:main Dec 8, 2024
@sunfishcode
Copy link
Copy Markdown
Member

Thanks!

sunfishcode pushed a commit that referenced this pull request Dec 8, 2024
Even when Solaris libc doesn't define LOCK_SH, LOCK_EX, LOCK_NB and LOCK_UN (which are flock() related).
sunfishcode pushed a commit that referenced this pull request Dec 8, 2024
Even when Solaris libc doesn't define LOCK_SH, LOCK_EX, LOCK_NB and LOCK_UN (which are flock() related).
@sunfishcode
Copy link
Copy Markdown
Member

This is now released in rustix 0.38.42.

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