Skip to content

Use tar library instead of tar tool for some operations#6945

Open
rjbou wants to merge 4 commits into
ocaml:masterfrom
rjbou:tar-create
Open

Use tar library instead of tar tool for some operations#6945
rjbou wants to merge 4 commits into
ocaml:masterfrom
rjbou:tar-create

Conversation

@rjbou
Copy link
Copy Markdown
Collaborator

@rjbou rjbou commented May 22, 2026

This PR adds in the API functions to manipulate tar.gz archives and use it to create archives in the code.
This new OpamTar module will be used in #6625 to open and read archives (internal representation of some repositories) without io.
Original work done by @kit-ty-kate

In the story of #6625

@rjbou rjbou added this to the 2.6.0~alpha1 milestone May 22, 2026
@rjbou rjbou requested a review from NathanReb May 22, 2026 16:49
@rjbou rjbou added the PR: QUEUED Pending pull request, waiting for other work to be merged or closed label May 22, 2026
Comment thread tests/reftests/repository.test
Comment thread src/core/opamFilename.ml Outdated
Comment thread src/repository/opamTar.ml
Comment thread src/repository/opamTar.mli Outdated
Comment thread src/repository/opamTar.ml
let tar = Tar_gz.out_gzipped ~level:4 ~mtime:0l Gz.Unix tar in
let buf = Buffer.create 10_485_760 in
to_buffer buf tar;
let str = Buffer.contents buf in
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do we have to go through this buffer to then allocate the whole string to then write to a Unix file descriptor?

Couldn't we write a to_fd function directly instead of the to_buffer one?

Is it so that we don't overwrite the original archive in case an error occurs? If so, then it's probably worth a comment or a bit of documentation.

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.

Opening issue to follow up #6950, if there is performance benefit

Comment thread src/repository/opamTar.ml Outdated
in
let entries =
let dispenser =
Map.fold (fun path content acc ->
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

How about using to_seq followed by Seq.map here?

Was the manual construction of the Seq intentional? Is it a performance issue?

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.

update pushed

Comment thread src/repository/opamTar.ml
Comment thread src/repository/opamTar.ml
Comment thread tests/reftests/repository.test
@rjbou
Copy link
Copy Markdown
Collaborator Author

rjbou commented May 27, 2026

I've updated the PR taking into account some remarks. I'll let @kit-ty-kate answer OpamTar ones.

@rjbou rjbou removed the PR: QUEUED Pending pull request, waiting for other work to be merged or closed label May 27, 2026
@rjbou rjbou force-pushed the tar-create branch 2 times, most recently from e0283a1 to aa32ae4 Compare May 29, 2026 15:50
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.

3 participants