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

Filter by bin/lib #814

Open
carols10cents opened this issue Jun 26, 2017 · 10 comments
Open

Filter by bin/lib #814

carols10cents opened this issue Jun 26, 2017 · 10 comments
Labels
A-search C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works

Comments

@carols10cents
Copy link
Member

Related to, but not quite the same as #483: have a way to filter crates by those that contain binaries or are binary-only, or the opposite, that have a library component.
@felipesere
Copy link

I was looking for interesting rust binaries for a talk and I stumbled across this.
Is this something that people would be keen to see?
With a little support I'd like to try this 😄

@carols10cents carols10cents mentioned this issue Nov 18, 2019
Open
@smokytheangel0
Copy link
Contributor

smokytheangel0 commented Nov 20, 2019 •
edited
Loading

for both this one and 1904, it seems like we could modify the body and the result of uploader.rs/verify_tarball() so that it could check for naming inconsistencies (in Cargo.toml) as well as
return extra information to be stored in the db (such as whether this crate contains main.rs/lib.rs or both). As meantioned in 1904, this would best be accomplished as a background job, and I was wondering what the status quo for paralel work in crates.io, process, thread or async?

@carols10cents
Copy link
Member Author

We use a framework that sgrif created called swirl. Search in this repo for swirl for some example uses.

I'm not sure that verify_tarball is the right place to do this but that's because our publishing logic has gotten messy and tangled in general; not sure if we should use this opportunity as a forcing function to make us clean up or if we should throw another hack on the pile for now and clean up after.

@smokytheangel0
Copy link
Contributor

I would be honoured to help clean up now, but this is not my house so suggestions are neccessary from the team. I have alot of time to work on things, I am attempting to build experience in working with other developers so I don't make alot of n00b mistakes on my portfolio projects for my eventual applications to software companies.

@Turbo87 Turbo87 added C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works and removed C-feature-request labels Feb 11, 2021
@epage epage mentioned this issue Apr 10, 2024
Merged
bors added a commit to rust-lang/cargo that referenced this issue Apr 29, 2024
@bors
Auto merge of #13713 - epage:no-auto, r=weihanglo
ba6fb40 
fix(toml): Warn, rather than fail publish, if a target is excluded

### What does this PR try to resolve?

We have a couple of problems with publishing
- Inconsistent errors: if a target that `package` doesn't verify is missing `path`, it will error, while one with `path` won't, see #13456
- Users may want to exclude targets and their choices are
  - Go ahead and include them.  I originally excluded my examples before doc-scraping was a think.  The problem was if I had to set `required-features`, I then could no longer exclude them
  - Muck with `Cargo.toml` during publish and pass `--allow-dirty`

This fixes both by auto-stripping targets on publish.  We will warn the user that we did so.

This is a mostly-one-way door on behavior because we are turning an error case into a warning.
For the most part, I think this is the right thing to do.
My biggest regret is that the warning is only during `package`/`publish` as it will be too late to act on it and people who want to know will want to know when the problem is introduced.
The error is also very late in the process but at least its before a non-reversible action has been taken.
Dry-run and `yank` help.

Fixes #13456
Fixes #5806

### How should we test and review this PR?

Tests are added in the first commit and you can then follow the commits to see how the test output evolved.

The biggest risk factors for this change are
- If the target-stripping logic mis-identifies a path as excluded because of innocuous path differences (e.g. case)
- Setting a minimum MSRV for published packages: `auto*` were added in 1.27 (#5335) but were insta-stable.  `autobins = false` did nothing until 1.32 (#6329).  I have not checked to see how this behaves pre-1.32  or pre-1.27.  Since my memory of that error is vague, I believe it will either do a redundant discovery *or* it will implicitly skip discovery

Resolved risks
- #13729 ensured our generated target paths don't have `\` in them
- #13729 ensures the paths are normalize so the list of packaged paths

For case-insensitive filesystems, I added tests to show the original behavior (works locally but will fail when depended on from a case-sensitive filesystem) and tracked how that changed with this PR (on publish warn that those targets are stripped).  We could try to normalize the case but it will also follow symlinks and is likely indicative of larger casing problems that the user had.  Weighing how broken things are now , it didn't seem changing behavior on this would be too big of a deal.

We should do a Call for Testing when this hits nightly to have people to `cargo package` and look for targets exclusion warnings that don't make sense.

### Additional information

This builds on #13701 and the work before it.

By enumerating all targets in `Cargo.toml`, it makes it so rust-lang/crates.io#5882 and rust-lang/crates.io#814 can be implemented without any other filesystem interactions.

A follow up PR is need to make much of a difference in performance because we unconditionally walk the file system just in case `autodiscover != Some(false)` or a target is missing a `path`.

We cannot turn off auto-discovery of libs, so that will always be done for bin-only packages.
@Turbo87
Copy link
Member

Turbo87 commented Jul 17, 2024

FWIW we now have has_lib and bin_names columns in the database that would potentially allow us to implement a lib/bin filter for the search, if there is enough interest.

@NobodyXu
Copy link

NobodyXu commented Sep 2, 2024

Would it be possible to support filtering baaed on binary/library crates in crates searching?

i..e. add a new query param like this https://crates.io/api/v1/crates?sort=downloads&has_binaries=true

@NobodyXu
Copy link

NobodyXu commented Sep 2, 2024

Motivation: I'm working on cargo-bins/cargo-quickinstall#268

Without this functionality, I will have to issue one restful API per crate for the most popular crates I got from https://crates.io/api/v1/crates?sort=downloads

@Turbo87
Copy link
Member

Turbo87 commented Sep 2, 2024

it would probably be better to work with our daily database dump instead of regularly querying the API for this information. also, whether a crate version contains a lib or bin does not change overtime, so you should only need to query the information once and then store it on your side.

@NobodyXu
Copy link

NobodyXu commented Sep 2, 2024 •
edited
Loading

Thanks!

I didn't think of that, this sounds much more efficient than using APIs.

Where can I get this db dump?

Edit:

thanks I've found it

@LawnGnome
Copy link
Contributor

Just in case anyone finds their way here in the future from a search result or something, the dumps are linked from https://crates.io/data-access#database-dumps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-search 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
6 participants
@Turbo87 @carols10cents @LawnGnome @felipesere @NobodyXu @smokytheangel0