rust-lang / crates-io-cargo-teams

the home of the crates io team
5 stars 11 forks source link

Request: Align validations between Cargo and Crates.io #41

Open shepmaster opened 5 years ago

shepmaster commented 5 years ago

After extensive testing locally and in CI, I finally went to publish to crates.io and received the error:

error: api errors (status 200 OK): invalid upload request: invalid value: string "futures-0.1", expected a valid feature name containing only letters, numbers, hyphens, or underscores at line 1 column 966

This is unfortunate as my multi-crate deploy is now in an unfinished state and I have to burn through a version number.

(@carols10cents told me to file this here)

shepmaster commented 5 years ago

my multi-crate deploy is now in an unfinished state

Incidentally, it would be really nice if cargo publish --dry-run could somehow work for multi-crate publishes without publishing the dependencies.

ehuss commented 5 years ago

There are a number of open issues for better validation: rust-lang/cargo#4300 rust-lang/cargo#5554 rust-lang/cargo#4377 rust-lang/crates.io#7250.

I think there has been some previous talk about sharing some validation code. I'm of the opinion that it would be a good idea to create a new library for this purpose, which contains only very basic validation functions. I think it is unlikely Cargo and crates.io could share something like the Manifest definition, so it would probably just need to contain a few free functions and constants. It would also only validate very basic elements. For example, it would not validate category names, or crates.io's list of reserved crate names (which lives in a database).

Perhaps those limitations means it would not have much value. I would like to hear from others what they think.

ehuss commented 5 years ago

I did a survey of some of the validation done between the two. Of course cargo does lots of other validation that isn't applicable here (profiles, dependencies, etc.).

I think it is feasible to create a new package that the two projects share, that would have about 5 to 10 free functions defining some validation. I'm still not sure if it will provide significant maintenance improvements (weighed against the maintenance burden of publishing and coordinating a new package). There are also a lot of judgement calls listed below, as to how Cargo would treat the extra restrictions. Adding a warning to the publish/package step doesn't really help much (just makes --dry-run a little better, and feedback a little sooner). Happy to hear any thoughts here.


cargo

crates.io

Shareability

Package name

  • Not empty.
  • All characters `is_alphanumeric` or `_` or `-`.
  • NOTE: init/new has its own blacklist (rust keywords, reserved names) and cannot start with a digit
  • Not empty.
  • First character `is_alphabetic`.
  • All characters `is_alphanumeric` or `_` or `-`.
  • All characters are ASCII.
  • Max 64 characters.
  • Rejects reserved crate names (stored in database).

Difficult to share the extra crates.io validations.  Not sure why restrictions are in database and not in code. This needs careful consideration.

version

  • Must parse with `semver`
  • Must parse with `semver`

Not likely shareable, part of type definition.

authors

No restrictions.

  • Must have at least one non-empty string.

Possibly shareable as a publish/package warning?

description

  • Warns if not set.
  • Must be set.

Keep as-is? See https://github.com/rust-lang/cargo/issues/4840

homepage, documentation, repository

  • Warns if all 3 not set.
  • Optional, but if set must pass:
  • parse with `url` crate
  • must be `http` or `https`
  • must be a “base” url

Possibly shareable as a publish/package warning or error?

readme

  • If set, must point to an existing file.


N/A

keyword

No restrictions.

  • Not empty
  • First character `is_alphabetic`
  • All characters `is_alphanumeric` or `_` or `-`
  • All characters are ASCII
  • At most 20 characters.

Possibly shareable as a publish/package warning or error?

keywords

No restrictions.

  • At most 5 entries.

Possibly shareable as a publish/package warning or error?

category

No restrictions.

  • Unknown categories result in a warning (and are ignored).

Unlikely shareable. Maybe address with future “dry run” publish API?

categories

No restrictions.

  • At most 5 entries.

Possibly shareable as a publish/package warning or error?

license

  • Publish warns if neither license nor license_file is set.
  • Either license or license_file should be set.
  • If license is set, must validate with `license_exprs` crate.

Possibly shareable as a publish/package warning or error? See also https://github.com/rust-lang/cargo/issues/2039 for discussion on license parsing. Also https://github.com/rust-lang/cargo/issues/3540

Feature name

  • Not empty
  • Not empty
  • All characters `is_alphanumeric` or `_` or `+` or `-`
  • All characters are ASCII

Possibly shareable, as a Cargo.toml hard error?  Depends on if packages must be ASCII?

Feature list entry

  • Entries must be other features or dependencies.
  • Slash parsing, namespaced crates, etc.
  • "feature_name" or "feature_name/feature_name"

Same as above.

Dependencies

  • Path dependencies must also specify `version`.
  • Version requirement must parse with `semver`.
  • `registry` not allowed
  • `*` not allowed
  • Must match an existing package name.
  • Version requirement must parse with `semver`

Unlikely to be shareable.

Badges

  • Each entry must be a table.
  • Invalid badge formats result in a warning (and are ignored).
  • See `Badge` enum definition for complete list.

Maybe shareable?  Warn on publish/package?  Wouldn’t really improve things except for `—dry-run`. See also https://github.com/rust-lang/cargo/issues/3576

MaulingMonkey commented 5 years ago

Sharing some thoughts from rust-lang/cargo#7256 :