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

admin::render_readmes: Support broader implicit readmes #3972

Closed
wants to merge 5 commits into from

Conversation

nipunn1313
Copy link
Contributor

Depends on #3971

(Because of community/community#4477 - github ends up rendering the diffs together. Go to the commits tab and just look at the most recent commit to review).

Fixes the comment here #3970 (comment)

Achieves this by pulling the entire tar file into
memory in the form of a hashmap. Then we can look for
the files as needed.

Before, we would stream the pkg response from the server
and un-tar it on the fly, but this behavior is incorrect
if the Cargo.toml isn't at the beginning of the archive.

Could imagine this might be problematic if the entire
archive won't fit in memory - but that seems unlikely?
We could mitigate by limiting to certain file types, but
we won't know what type the README will be.

Added a test to for this case - the test fails
before this change and passes with this change
@nipunn1313
Copy link
Contributor Author

Closing based on discussion - not planning to maintain admin::render_readmes in this state.

Ideally admin::render_readmes is not downloading anything from crates-io.

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