Skip to content

Commit 74f6f7d

Browse files
committed
Remove usages of the now-unused license col on the crates table
This is a big change! `cargo check` says this is fine, but some manual verification and testing is probably a good idea. (`cargo test` does pass locally for me, but that only tells half the story.) As part of (GitHub) #736, this commit not only removes the commitment of license information into the database, but it also removes the handling in the controller. For silent backwards compatibility, it might be better to simply drop the license field, for example if it's being uploaded by outdated clients. I am not sure, however, and will probably need feedback on this. (A commit running `cargo fmt` was also rebased into this one.) Signed-off-by: Kristofer Rye <[email protected]>
1 parent a248372 commit 74f6f7d

File tree

5 files changed

+6
-39
lines changed

5 files changed

+6
-39
lines changed

src/controllers/krate/publish.rs

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -79,17 +79,12 @@ pub fn publish(req: &mut dyn Request) -> CargoResult<Response> {
7979
documentation: new_crate.documentation.as_ref().map(|s| &**s),
8080
readme: new_crate.readme.as_ref().map(|s| &**s),
8181
repository: repo.as_ref().map(String::as_str),
82-
license: new_crate.license.as_ref().map(|s| &**s),
8382
max_upload_size: None,
8483
};
8584

8685
let license_file = new_crate.license_file.as_ref().map(|s| &**s);
87-
let krate = persist.create_or_update(
88-
&conn,
89-
license_file,
90-
user.id,
91-
Some(&app.config.publish_rate_limit),
92-
)?;
86+
let krate =
87+
persist.create_or_update(&conn, user.id, Some(&app.config.publish_rate_limit))?;
9388

9489
let owners = krate.owners(&conn)?;
9590
if user.rights(req.app(), &owners)? < Rights::Publish {

src/models/krate.rs

Lines changed: 2 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ pub struct Crate {
4242
pub description: Option<String>,
4343
pub homepage: Option<String>,
4444
pub documentation: Option<String>,
45-
pub license: Option<String>,
4645
pub repository: Option<String>,
4746
pub max_upload_size: Option<i32>,
4847
}
@@ -58,7 +57,6 @@ type AllColumns = (
5857
crates::description,
5958
crates::homepage,
6059
crates::documentation,
61-
crates::license,
6260
crates::repository,
6361
crates::max_upload_size,
6462
);
@@ -72,7 +70,6 @@ pub const ALL_COLUMNS: AllColumns = (
7270
crates::description,
7371
crates::homepage,
7472
crates::documentation,
75-
crates::license,
7673
crates::repository,
7774
crates::max_upload_size,
7875
);
@@ -97,20 +94,18 @@ pub struct NewCrate<'a> {
9794
pub readme: Option<&'a str>,
9895
pub repository: Option<&'a str>,
9996
pub max_upload_size: Option<i32>,
100-
pub license: Option<&'a str>,
10197
}
10298

10399
impl<'a> NewCrate<'a> {
104100
pub fn create_or_update(
105101
mut self,
106102
conn: &PgConnection,
107-
license_file: Option<&'a str>,
108103
uploader: i32,
109104
rate_limit: Option<&PublishRateLimit>,
110105
) -> CargoResult<Crate> {
111106
use diesel::update;
112107

113-
self.validate(license_file)?;
108+
self.validate()?;
114109
self.ensure_name_not_reserved(conn)?;
115110

116111
conn.transaction(|| {
@@ -132,7 +127,7 @@ impl<'a> NewCrate<'a> {
132127
})
133128
}
134129

135-
fn validate(&mut self, license_file: Option<&'a str>) -> CargoResult<()> {
130+
fn validate(&mut self) -> CargoResult<()> {
136131
fn validate_url(url: Option<&str>, field: &str) -> CargoResult<()> {
137132
let url = match url {
138133
Some(s) => s,
@@ -163,28 +158,6 @@ impl<'a> NewCrate<'a> {
163158
validate_url(self.homepage, "homepage")?;
164159
validate_url(self.documentation, "documentation")?;
165160
validate_url(self.repository, "repository")?;
166-
self.validate_license(license_file)?;
167-
Ok(())
168-
}
169-
170-
fn validate_license(&mut self, license_file: Option<&str>) -> CargoResult<()> {
171-
if let Some(license) = self.license {
172-
for part in license.split('/') {
173-
license_exprs::validate_license_expr(part).map_err(|e| {
174-
human(&format_args!(
175-
"{}; see http://opensource.org/licenses \
176-
for options, and http://spdx.org/licenses/ \
177-
for their identifiers",
178-
e
179-
))
180-
})?;
181-
}
182-
} else if license_file.is_some() {
183-
// If no license is given, but a license file is given, flag this
184-
// crate as having a nonstandard license. Note that we don't
185-
// actually do anything else with license_file currently.
186-
self.license = Some("non-standard");
187-
}
188161
Ok(())
189162
}
190163

src/models/token.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,5 +96,4 @@ mod tests {
9696
.find(r#""last_used_at":"2017-01-06T14:23:12+00:00""#)
9797
.is_some());
9898
}
99-
10099
}

src/tasks/update_downloads.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ mod test {
103103
name: "foo",
104104
..Default::default()
105105
}
106-
.create_or_update(conn, None, user_id, None)
106+
.create_or_update(conn, user_id, None)
107107
.unwrap();
108108
let version = NewVersion::new(
109109
krate.id,

src/tests/builders.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ impl<'a> CrateBuilder<'a> {
234234

235235
let mut krate = self
236236
.krate
237-
.create_or_update(connection, None, self.owner_id, None)?;
237+
.create_or_update(connection, self.owner_id, None)?;
238238

239239
// Since we are using `NewCrate`, we can't set all the
240240
// crate properties in a single DB call.

0 commit comments

Comments
 (0)