Display the correct migration order in diesel migration list

The old code was incorrectly assuming that the migration version was
equal to the file name, which caused issues on projects that have
migrations using the old timestamp format. This continues to display the
actual file name, but sorts on migration version instead.

Fixes #1500

CHANGELOG.md

@@ -31,6 +31,12 @@ for Rust libraries in [RFC #1105](https://github.com/rust-lang/rfcs/blob/master/
 
 * `ne_any` has been renamed to `ne_all`.
 
+### Fixed
+
+* `diesel migration list` shows the proper migration order when mixing
+  old and new timestamp formats. (The migrations were always run in the correct
+  order, this only affects the display logic of `migration list`)
+
 ## [1.1.1] - 2018-01-16
 
 ### Added

diesel_cli/src/main.rs

@@ -77,33 +77,22 @@ fn run_migration_command(matches: &ArgMatches) {
             call_with_conn!(database_url, redo_latest_migration(&dir));
         }
         ("list", Some(_)) => {
-            use std::ffi::OsStr;
-
             let database_url = database::database_url(matches);
             let dir = migrations_dir(matches);
-            let migrations =
+            let mut migrations =
                 call_with_conn!(database_url, migrations::mark_migrations_in_directory(&dir))
                     .unwrap_or_else(handle_error);
 
-            // Since our migrations came from `mark_migrations_in_directory` they should all have valid file names.
-            let mut sorted = migrations
-                .into_iter()
-                .map(|(path_buf, applied)| {
-                    let path_buf = path_buf.expect("Found migration with invalid file name");
-                    let file_path = path_buf.as_path();
-                    let file_name = file_path
-                        .file_name()
-                        .and_then(OsStr::to_str)
-                        .map(str::to_string)
-                        .expect(&format!("Error getting file name from {:?}", path_buf));
-                    (file_name, applied)
-                })
-                .collect::<Vec<_>>();
-
-            sorted.sort();
+            migrations.sort_by_key(|&(ref m, _)| m.version().to_string());
 
             println!("Migrations:");
-            for (name, applied) in sorted {
+            for (migration, applied) in migrations {
+                let name = migration
+                    .file_path()
+                    .unwrap()
+                    .file_name()
+                    .unwrap()
+                    .to_string_lossy();
                 let x = if applied { 'X' } else { ' ' };
                 println!("  [{}] {}", x, name);
             }

diesel_cli/tests/migration_list.rs

@@ -146,3 +146,23 @@ fn migration_list_orders_nontimestamp_versions_alphabetically() {
     let output = result.stdout();
     assert_tags_in_order(output, &[&tag1, &tag2, &tag3, &tag4, &tag5, &tag6]);
 }
+
+#[test]
+fn migration_list_orders_old_and_new_timestamp_forms_mixed_correctly() {
+    let p = project("migration_list_mixed_timestamps")
+        .folder("migrations")
+        .build();
+
+    p.command("setup").run();
+
+    let tag1 = "20170505070309_migration";
+    p.create_migration(&tag1, "", "");
+
+    let tag2 = "2017-11-23-064836_migration";
+    p.create_migration(&tag2, "", "");
+
+    let result = p.command("migration").arg("list").run();
+    assert!(result.is_success(), "Result was unsuccessful {:?}", result);
+    let output = result.stdout();
+    assert_tags_in_order(output, &[&tag1, &tag2]);
+}

diesel_migrations/migrations_internals/src/lib.rs

@@ -141,7 +141,7 @@ where
 pub fn mark_migrations_in_directory<Conn>(
     conn: &Conn,
     migrations_dir: &Path,
-) -> Result<Vec<(Option<PathBuf>, bool)>, RunMigrationsError>
+) -> Result<Vec<(Box<Migration>, bool)>, RunMigrationsError>
 where
     Conn: MigrationConnection,
 {
@@ -152,7 +152,7 @@ where
         .into_iter()
         .map(|m| {
             let applied = already_run.contains(&m.version().to_string());
-            (m.file_path().map(|p| p.to_path_buf()), applied)
+            (m, applied)
         })
         .collect();
     Ok(migrations)