Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Blocked by #103|#76] Make ztoc creation (toc and zinfo) to accept io.Reader instead of filename #366

Open
Tracked by #925
djdongjin opened this issue Jan 27, 2023 · 3 comments
Labels
blocked Cannot proceed until some external condition is met feature New feature or request

Comments

@djdongjin
Copy link
Contributor

djdongjin commented Jan 27, 2023

Is your feature request related to a problem? Please describe.

Currently when soci index creates ztoc (toc and zinfo), it passes a filename to accepts a filename to ztoc.BuildZtoc instead of io.Reader.

It might be good to let index creation create a io.Reader and pass to ztoc creation, which will use to create both toc and zinfo.

func BuildZtoc(gzipFile string, span int64, buildToolIdentifier string) (*Ztoc, error) {

It also avoids creating a tmp file only for ztoc creation (specifically, required by gzip zinfo implmented in c)

Describe the solution you'd like

Change filename usage in index/ztoc creation to using io.Reader.

There is a potential blocker that compression/gzip_zinfo.go (the underlying c code) cannot handle io.Reader in c. See more discussion in #76 (specifically this relevant comment #72 (review))

Describe alternatives you've considered

Keep filename usage unchanged.

Additional context

It's not high priority, since we're also refactoring/decoupling ztoc/compression (#103). I think this change won't impact #103 and has lower priority than #103.

@djdongjin djdongjin added the feature New feature or request label Jan 27, 2023
@Kern--
Copy link
Contributor

Kern-- commented Jan 28, 2023

I think it's worth highlighting as an acceptance criteria that after this change TocBuilder should receive an uncompressed tarball and have no knowledge of compression.

@djdongjin djdongjin changed the title Make index/ztoc/toc/zinfo creation to accept io.Reader instead of filename Make ztoc creation (toc and zinfo) to accept io.Reader instead of filename Feb 1, 2023
@djdongjin
Copy link
Contributor Author

updated the description.

@djdongjin
Copy link
Contributor Author

This one is blocked on:

#103: after ztoc/compression decouple, we'll have a better unstanding of the decoupled structure.
#76: we use filename string in zinfo c code. If we have c code, it seems unavoidable to use filename, since c/cgo doesn't support io.Reader.

@djdongjin djdongjin changed the title Make ztoc creation (toc and zinfo) to accept io.Reader instead of filename [Blocked by #103|#76] Make ztoc creation (toc and zinfo) to accept io.Reader instead of filename Feb 27, 2023
@Kern-- Kern-- added the blocked Cannot proceed until some external condition is met label Feb 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Cannot proceed until some external condition is met feature New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants