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.

Sign up for GitHub

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

Jump to bottom

Improve admin::render_readmes #4095

Open
nipunn1313 opened this issue Oct 28, 2021 · 3 comments
Open

Improve admin::render_readmes #4095

nipunn1313 opened this issue Oct 28, 2021 · 3 comments
Labels
A-backend ⚙️ C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works

Comments

@nipunn1313
Copy link
Contributor

From @Turbo87 in discord:

I would do it in roughly these steps:

  1. Add a vcs_info JSONB column to the versions table and populate it with the file content from new publishes
  2. Create script or temporary crates-admin command to backfill vcs_info and readme columns on versions that don't have them yet
  3. Adjust render-readmes to use readme column instead of downloading the crate file
  4. Adjust render-readmes to use the vcs_info column to render relative paths correctly for crates in subdirectories

In this strategy

Ideally admin::render_readmes does not download the crate at all.

Requires a one-off script to backfill columns to the versions and crates table from the downloaded crate.

If we want to support rerunning admin::render_readmes on older versions of crates - we would need to move the readme column to the versions table (currently it's in crates table - only the latest version of readme is persisted - overwritten every time a new version is uploaded - never used).

Alternate strategy

Opted against this strategy in the discussion so far, but putting it here for completeness. If we considered going with this - which would enable things like #3971 and #3972

Don't use the readme column of crates (I've just confirmed that it's completely unused currently in the codebase afaict)
Download old crates from crates.io to recalculate/find the readme again.
@nipunn1313 nipunn1313 mentioned this issue Oct 28, 2021
Merged
@Turbo87
Copy link
Member

Turbo87 commented Oct 28, 2021

I've just confirmed that it's completely unused currently in the codebase

It might be unused in the Rust portion of the codebase, but I think the search functionality uses the column inside of SQL query. 😉

@Turbo87 Turbo87 added A-backend ⚙️ C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works labels Oct 28, 2021
@nipunn1313
Copy link
Contributor Author

Aha. Makes sense! Search functionality would make fantastic use of that column.

Moving readme column to the versions table would certainly help us regenerate old readmes. Would it be beneficial for the user to be able to search through past versions of readmes as well?

@Turbo87
Copy link
Member

Turbo87 commented Oct 28, 2021

oh... to be honest, I wasn't aware the readme column was only on the crates table 🙈

that certainly complicates things. and I guess it explains why the render-readmes command does not use the readme column... 😅

Moving readme column to the versions table would certainly help us regenerate old readmes

Yeah, adding a readme column to the versions table might be a good idea. I'm not sure how much bigger the database would grow if we did this though 🤔

Would it be beneficial for the user to be able to search through past versions of readmes as well?

It's probably sufficient to only be able to search in the README of the most recent release.

bors added a commit that referenced this issue Jan 3, 2022
@bors
Auto merge of #3973 - nipunn1313:readme2, r=Turbo87
3f67acf 
Support relative URLs from repo-subdirectory packages

Extract `.cargo_vcs_info.json` from the tarball and use it to determine the path in vcs.

Support for this was added to cargo with
rust-lang/cargo#9866

Fixes #3484
(note that support for this in admin::render_readmes not yet exists - would come with #4095)

Depends on #4097 (Because of community/community#4477 - github ends up rendering the diffs together. Go to the commits tab and just look at the most recent commits to review).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-backend ⚙️ C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Turbo87 @nipunn1313