fix(sandbox): reject existing destinations when copying inputs by Alizter · Pull Request #16632 · ocaml/dune · GitHub
Skip to content

fix(sandbox): reject existing destinations when copying inputs - #16632

Merged
Alizter merged 1 commit into
ocaml:mainfrom
Alizter:push-vmtwyqlvkyup
Oct 21, 2026
Merged

Alizter merged 1 commit into
ocaml:mainfrom
Alizter:push-vmtwyqlvkyup

Conversation

@Alizter

@Alizter Alizter commented Oct 17, 2026 •

Copy link
Copy Markdown
Collaborator

Allow Io.copy_file callers to choose exclusive creation or overwriting. Reject existing destinations during copy and patch-back sandbox population.

This implements Rudi's request in #16596 to stop silent overwriting in copy mode before fixing the overlapping directory dependencies behind #16547. It updates the reproduction from #16626; resolving the overlap itself remains a separate change.

@Alizter
Alizter marked this pull request as ready for review October 17, 2026 18:23
@Alizter
Alizter force-pushed the push-vmtwyqlvkyup branch 2 times, most recently from e268304 to 539c856 Compare October 17, 2026 19:07
@Alizter
Alizter requested a review from rgrinberg October 20, 2026 10:53

@rgrinberg rgrinberg left a comment

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.

Instead of "exclusive", I would prefer the name "overwrite" instead.

Comment thread otherlibs/stdune/src/copyfile_stubs.c Outdated

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 think there's a flag to overwrite with copyfile. So none of this should be necessary.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

On macos, the flag is implemented using remove rather than unlink which can delete directories too. This means it will differ in behaviour from Linux. Do you think that's a big deal?

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 don't know. But given that it's not the same, let's just not touch it.

Allow Io.copy_file to choose if files are able to overwritten.

Signed-off-by: Ali Caglayan <alizter@gmail.com>
@Alizter
Alizter merged commit 7eb159a into ocaml:main Oct 21, 2026
35 of 36 checks passed
@Alizter
Alizter deleted the push-vmtwyqlvkyup branch October 21, 2026 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants