Remove unused code (#558)

* remove copy_dir

* make utils pub(crate) where possible

* remove dead code in rustc version

* make more things pub(crate)

* only compile ApplyMode::Temporary in test mode

* remove unused argument

* remove unused `handle_html` argument

* don't export metadata

* remove `pub` where possible

* remove unused import

* make create_pool pub(crate)

* fix bad merge

* pub(crate) doesn't make sense in lib.rs

Co-Authored-By: Koenraad Verheyden <mail@koenraadverheyden.com>

* address review comments re: copy test

* remote system dependencies from metadata

this was causing confusion, see #164

* re-comment failing test

* fix failing test

Co-authored-by: Koenraad Verheyden <mail@koenraadverheyden.com>

Author: jyn514

src/bin/cratesfyi.rs

@@ -239,7 +239,7 @@ pub fn main() {
             let mut count = 1;
             let mut total = 0;
             while count != 0 {
-                count = db::file::move_to_s3(&conn, 5_000).expect("Failed to upload batch to S3");
+                count = db::move_to_s3(&conn, 5_000).expect("Failed to upload batch to S3");
                 total += count;
                 eprintln!(
                     "moved {} rows to s3 in this batch, total moved so far: {}",

src/db/file.rs

@@ -42,7 +42,7 @@ fn get_file_list_from_dir<P: AsRef<Path>>(path: P,
 }
 
 
-pub fn get_file_list<P: AsRef<Path>>(path: P) -> Result<Vec<PathBuf>> {
+fn get_file_list<P: AsRef<Path>>(path: P) -> Result<Vec<PathBuf>> {
     let path = path.as_ref();
     let mut files = Vec::new();
 

src/db/migrate.rs

@@ -10,6 +10,7 @@ use std::borrow::Cow;
 #[derive(Copy, Clone)]
 enum ApplyMode {
     Permanent,
+    #[cfg(test)]
     Temporary,
 }
 
@@ -22,6 +23,7 @@ impl MigrationContext {
     fn format_query<'a>(&self, query: &'a str) -> Cow<'a, str> {
         match self.apply_mode {
             ApplyMode::Permanent => Cow::Borrowed(query),
+            #[cfg(test)]
             ApplyMode::Temporary => {
                 Cow::Owned(query.replace("CREATE TABLE", "CREATE TEMPORARY TABLE"))
             }
@@ -73,6 +75,7 @@ pub fn migrate(version: Option<Version>, conn: &Connection) -> CratesfyiResult<(
     migrate_inner(version, conn, ApplyMode::Permanent)
 }
 
+#[cfg(test)]
 pub fn migrate_temporary(version: Option<Version>, conn: &Connection) -> CratesfyiResult<()> {
     migrate_inner(version, conn, ApplyMode::Temporary)
 }

src/db/mod.rs

@@ -3,8 +3,10 @@
 pub(crate) use self::add_package::add_package_into_database;
 pub(crate) use self::add_package::add_build_into_database;
 pub(crate) use self::add_package::CratesIoData;
-pub use self::file::add_path_into_database;
-pub use self::migrate::{migrate, migrate_temporary};
+pub use self::file::{add_path_into_database, move_to_s3};
+pub use self::migrate::migrate;
+#[cfg(test)]
+pub(crate) use self::migrate::migrate_temporary;
 pub use self::delete_crate::delete_crate;
 
 use postgres::{Connection, TlsMode};
@@ -14,7 +16,7 @@ use r2d2;
 use r2d2_postgres;
 
 mod add_package;
-pub mod file;
+pub(crate) mod file;
 mod migrate;
 mod delete_crate;
 pub mod blacklist;
@@ -29,7 +31,7 @@ pub fn connect_db() -> Result<Connection, Error> {
 }
 
 
-pub fn create_pool() -> r2d2::Pool<r2d2_postgres::PostgresConnectionManager> {
+pub(crate) fn create_pool() -> r2d2::Pool<r2d2_postgres::PostgresConnectionManager> {
     let db_url = env::var("CRATESFYI_DATABASE_URL")
         .expect("CRATESFYI_DATABASE_URL environment variable is not exists");
     let manager = r2d2_postgres::PostgresConnectionManager::new(&db_url[..],

src/docbuilder/metadata.rs

@@ -22,7 +22,6 @@ use failure::err_msg;
 /// default-target = "x86_64-unknown-linux-gnu"
 /// rustc-args = [ "--example-rustc-arg" ]
 /// rustdoc-args = [ "--example-rustdoc-arg" ]
-/// dependencies = [ "example-system-dependency" ]
 /// ```
 ///
 /// You can define one or more fields in your `Cargo.toml`.
@@ -49,11 +48,6 @@ pub struct Metadata {
 
     /// List of command line arguments for `rustdoc`.
     pub rustdoc_args: Option<Vec<String>>,
-
-    /// System dependencies.
-    ///
-    /// Docs.rs is running on a Debian jessie.
-    pub dependencies: Option<Vec<String>>,
 }
 
 
@@ -69,7 +63,7 @@ impl Metadata {
         Err(err_msg("Manifest not found"))
     }
 
-    pub fn from_manifest<P: AsRef<Path>>(path: P) -> Metadata {
+    fn from_manifest<P: AsRef<Path>>(path: P) -> Metadata {
         use std::fs::File;
         use std::io::Read;
         let mut f = match File::open(path) {
@@ -93,7 +87,6 @@ impl Metadata {
             default_target: None,
             rustc_args: None,
             rustdoc_args: None,
-            dependencies: None,
         }
     }
 
@@ -122,8 +115,6 @@ impl Metadata {
                         .and_then(|f| f.iter().map(|v| v.as_str().map(|v| v.to_owned())).collect());
                     metadata.rustdoc_args = table.get("rustdoc-args").and_then(|f| f.as_array())
                         .and_then(|f| f.iter().map(|v| v.as_str().map(|v| v.to_owned())).collect());
-                    metadata.dependencies = table.get("dependencies").and_then(|f| f.as_array())
-                        .and_then(|f| f.iter().map(|v| v.as_str().map(|v| v.to_owned())).collect());
                 }
 
         metadata
@@ -151,7 +142,6 @@ mod test {
             default-target = "x86_64-unknown-linux-gnu"
             rustc-args = [ "--example-rustc-arg" ]
             rustdoc-args = [ "--example-rustdoc-arg" ]
-            dependencies = [ "example-system-dependency" ]
         "#;
 
         let metadata = Metadata::from_str(manifest);
@@ -176,9 +166,5 @@ mod test {
         let rustdoc_args = metadata.rustdoc_args.unwrap();
         assert_eq!(rustdoc_args.len(), 1);
         assert_eq!(rustdoc_args[0], "--example-rustdoc-arg".to_owned());
-
-        let dependencies = metadata.dependencies.unwrap();
-        assert_eq!(dependencies.len(), 1);
-        assert_eq!(dependencies[0], "example-system-dependency".to_owned());
     }
 }

src/docbuilder/mod.rs

@@ -1,6 +1,6 @@
 
-pub mod options;
-pub mod metadata;
+pub(crate) mod options;
+mod metadata;
 mod limits;
 mod rustwide_builder;
 mod crates;
@@ -9,6 +9,7 @@ mod queue;
 pub use self::rustwide_builder::RustwideBuilder;
 pub(crate) use self::rustwide_builder::BuildResult;
 pub(crate) use self::limits::Limits;
+pub(self) use self::metadata::Metadata;
 
 
 use std::fs;

src/docbuilder/rustwide_builder.rs

@@ -16,7 +16,7 @@ use std::borrow::Cow;
 use std::collections::HashSet;
 use std::path::Path;
 use utils::{copy_doc_dir, parse_rustc_version, CargoMetadata};
-use Metadata;
+use super::Metadata;
 
 const USER_AGENT: &str = "docs.rs builder (https://github.com/rust-lang/docs.rs)";
 const DEFAULT_RUSTWIDE_WORKSPACE: &str = ".rustwide";
@@ -522,8 +522,7 @@ impl RustwideBuilder {
         }
 
         info!("{} {}", source.display(), dest.display());
-        copy_doc_dir(source, dest, self.rustc_version.trim())?;
-        Ok(())
+        copy_doc_dir(source, dest)
     }
 
     fn upload_docs(

src/error.rs

@@ -2,6 +2,6 @@
 
 use std::result::Result as StdResult;
 
-pub use failure::{Error, ResultExt};
+pub(crate) use failure::Error;
 
 pub type Result<T> = StdResult<T, Error>;

src/lib.rs

@@ -47,10 +47,9 @@ extern crate once_cell;
 pub use self::docbuilder::RustwideBuilder;
 pub use self::docbuilder::DocBuilder;
 pub use self::docbuilder::options::DocBuilderOptions;
-pub use self::docbuilder::metadata::Metadata;
 pub use self::web::Server;
 
-pub mod error;
+mod error;
 pub mod db;
 pub mod utils;
 mod docbuilder;

src/utils/copy.rs

@@ -8,44 +8,16 @@ use error::Result;
 
 use regex::Regex;
 
-
-/// Copies files from source directory to destination directory.
-pub fn copy_dir<P: AsRef<Path>>(source: P, destination: P) -> Result<()> {
-    copy_files_and_handle_html(source.as_ref().to_path_buf(),
-                               destination.as_ref().to_path_buf(),
-                               false,
-                               "")
-}
-
-
 /// Copies documentation from a crate's target directory to destination.
 ///
 /// Target directory must have doc directory.
 ///
-/// This function is designed to avoid file duplications. It is using rustc version string
-/// to rename common files (css files, jquery.js, playpen.js, main.js etc.) in a standard rustdoc.
-pub fn copy_doc_dir<P: AsRef<Path>>(target: P,
-                                    destination: P,
-                                    rustc_version: &str)
-                                    -> Result<()> {
+/// This function is designed to avoid file duplications.
+pub fn copy_doc_dir<P: AsRef<Path>>(target: P, destination: P) -> Result<()> {
     let source = PathBuf::from(target.as_ref()).join("doc");
-    copy_files_and_handle_html(source,
-                               destination.as_ref().to_path_buf(),
-                               true,
-                               rustc_version)
-}
+    let destination = destination.as_ref().to_path_buf();
 
-
-fn copy_files_and_handle_html(source: PathBuf,
-                              destination: PathBuf,
-                              handle_html: bool,
-                              rustc_version: &str)
-                              -> Result<()> {
-
-    // FIXME: handle_html is useless since we started using --resource-suffix
-    //        argument with rustdoc
-
-    // Make sure destination directory is exists
+    // Make sure destination directory exists
     if !destination.exists() {
         fs::create_dir_all(&destination)?;
     }
@@ -65,11 +37,8 @@ fn copy_files_and_handle_html(source: PathBuf,
 
         if metadata.is_dir() {
             fs::create_dir_all(&destination_full_path)?;
-            copy_files_and_handle_html(file.path(),
-                                            destination_full_path,
-                                            handle_html,
-                                            &rustc_version)?
-        } else if handle_html && dup_regex.is_match(&file.file_name().into_string().unwrap()[..]) {
+            copy_doc_dir(file.path(), destination_full_path)?
+        } else if dup_regex.is_match(&file.file_name().into_string().unwrap()[..]) {
             continue;
         } else {
             fs::copy(&file.path(), &destination_full_path)?;
@@ -85,19 +54,21 @@ fn copy_files_and_handle_html(source: PathBuf,
 mod test {
     extern crate env_logger;
     use std::fs;
-    use std::path::Path;
     use super::*;
 
     #[test]
-    #[ignore]
-    fn test_copy_dir() {
-        let destination = tempdir::TempDir::new("cratesfyi").unwrap();
+    fn test_copy_doc_dir() {
+        let source = tempdir::TempDir::new("cratesfyi-src").unwrap();
+        let destination = tempdir::TempDir::new("cratesfyi-dst").unwrap();
+        let doc = source.path().join("doc");
+        fs::create_dir(&doc).unwrap();
 
-        // lets try to copy a src directory to tempdir
-        let res = copy_dir(Path::new("src"), destination.path());
-        // remove temp dir
-        fs::remove_dir_all(destination.path()).unwrap();
+        fs::write(doc.join("index.html"), "<html>spooky</html>").unwrap();
+        fs::write(doc.join("index.txt"), "spooky").unwrap();
 
-        assert!(res.is_ok());
+        // lets try to copy a src directory to tempdir
+        copy_doc_dir(source.path(), destination.path()).unwrap();
+        assert!(destination.path().join("index.html").exists());
+        assert!(!destination.path().join("index.txt").exists());
     }
 }

src/utils/github_updater.rs

@@ -9,11 +9,11 @@ use failure::err_msg;
 /// Fields we need use in cratesfyi
 #[derive(Debug)]
 struct GitHubFields {
-    pub description: String,
-    pub stars: i64,
-    pub forks: i64,
-    pub issues: i64,
-    pub last_commit: time::Timespec,
+    description: String,
+    stars: i64,
+    forks: i64,
+    issues: i64,
+    last_commit: time::Timespec,
 }
 
 

src/utils/mod.rs

@@ -1,12 +1,12 @@
 //! Various utilities for cratesfyi
 
 
-pub use self::copy::{copy_dir, copy_doc_dir};
+pub(crate) use self::copy::copy_doc_dir;
 pub use self::github_updater::github_updater;
 pub use self::release_activity_updater::update_release_activity;
 pub use self::daemon::start_daemon;
-pub use self::rustc_version::{parse_rustc_version, get_current_versions, command_result};
-pub use self::html::extract_head_and_body;
+pub(crate) use self::rustc_version::parse_rustc_version;
+pub(crate) use self::html::extract_head_and_body;
 pub use self::queue::add_crate_to_queue;
 pub(crate) use self::cargo_metadata::{CargoMetadata, Package as MetadataPackage};
 

src/utils/rustc_version.rs

@@ -1,5 +1,4 @@
 
-use std::process::{Command, Output};
 use regex::Regex;
 use error::Result;
 use failure::err_msg;
@@ -18,26 +17,6 @@ pub fn parse_rustc_version<S: AsRef<str>>(version: S) -> Result<String> {
             captures.get(2).unwrap().as_str()))
 }
 
-
-/// Returns current version of rustc and cratesfyi
-pub fn get_current_versions() -> Result<(String, String)> {
-    let rustc_version = command_result(Command::new("rustc").arg("--version").output()?)?;
-    let cratesfyi_version = command_result(Command::new("rustc").arg("--version").output()?)?;
-
-    Ok((rustc_version, cratesfyi_version))
-}
-
-
-pub fn command_result(output: Output) -> Result<String> {
-    let mut command_out = String::from_utf8_lossy(&output.stdout).into_owned();
-    command_out.push_str(&String::from_utf8_lossy(&output.stderr).into_owned()[..]);
-    match output.status.success() {
-        true => Ok(command_out),
-        false => Err(err_msg(command_out)),
-    }
-}
-
-
 #[test]
 fn test_parse_rustc_version() {
     assert_eq!(parse_rustc_version("rustc 1.10.0-nightly (57ef01513 2016-05-23)").unwrap(),

src/web/crate_details.rs

@@ -43,7 +43,7 @@ pub struct CrateDetails {
     github_stars: Option<i32>,
     github_forks: Option<i32>,
     github_issues: Option<i32>,
-    pub metadata: MetaData,
+    pub(crate) metadata: MetaData,
     is_library: bool,
     doc_targets: Option<Json>,
     license: Option<String>,