Skip to content

enforce some commit prerequisites#24

Merged
dustinpho merged 7 commits into
mainfrom
dustin/no-open-fds
Nov 25, 2025
Merged

enforce some commit prerequisites#24
dustinpho merged 7 commits into
mainfrom
dustin/no-open-fds

Conversation

@dustinpho
Copy link
Copy Markdown
Contributor

enforce that there should be no file descriptors open before trying to commit

@dustinpho dustinpho requested a review from blinsay November 21, 2025 18:57
@dustinpho
Copy link
Copy Markdown
Contributor Author

hmm, the README.md didn't seem like a good place to write about commit semantics since its more of a crash course to using Pond. I'll probably hold off on writing about this until after the locking changes.

Comment thread pond/src/volume.rs Outdated
Comment thread pond/src/volume.rs Outdated
) && self.fds.len() == 1;
if !self.fds.is_empty() && !is_commit_only_fd {
return Err(Error::new(
ErrorKind::InvalidData,
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.

EINVAL feels weird. EBUSY? EINPROGRESS?

Comment thread pond/src/volume.rs Outdated
return Err(Error::new(
ErrorKind::InvalidData,
format!(
"all open files must be closed before committing, with the exception of the commit file. num open files: {}",
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.

I wouldn't mention the commit file or the number of open fds here. I think either you don't care and will be confused, or you'll be frustrated that you don't know which fds are still open.

Do we need an api to see which files are open?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Do we need an api to see which files are open?

maybe? it kinda sounds like it would be something we'd do if we decide to make this more git-like with versions -> commits, then we'd want to see a lot more state (like staged files, revert staged files if they were previously committed files, etc.)

@dustinpho
Copy link
Copy Markdown
Contributor Author

fyi the tests were failing because apparently the kernel wasn't sending out the release fast enough -- i thought the release was synchronous in the File's drop, but apparently not.

@dustinpho dustinpho requested a review from blinsay November 25, 2025 16:28
@dustinpho dustinpho merged commit 012ed74 into main Nov 25, 2025
4 checks passed
@dustinpho dustinpho deleted the dustin/no-open-fds branch November 25, 2025 19:52
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