Repository: railwayapp/nixpacks
Pull request #685 (2022-11-20 21:36:20)

Support copy files to a subdirectory in app directory
<!-- Thanks for sending a pull request! -->
<!-- Please add a useful description explaining what this PR does -->

This PR gives a solution for issue #630. Now it's possible to use syntax like:

```toml
[start]
  onlyIncludeFiles = [
    '...',
    { from = './templates/index.html.hbs', to = './templates/index.html.hbs' }
  ]
```

so `index.html.hbs` is copied inside a `templates` subdirectory inside the app directory. That's an example that currently fails when using [Rocket](https://rocket.rs/).

I have an [example of a Rocket service](https://github.com/jhg/simple-parking) that doesn't work on Railway.app because templates' files are copied just in the app directory instead of the template folder.

<!-- PR Checklist  -->
- [X] Tests are added/updated
- [X] Docs are updated


flybayer: I'm definitely in favor of just making `onlyIncludeFiles` work with directories like a normal person would expect :) 
jhernandez-at-wiris: @flybayer Files are copied by Docker, using a `--from=` then I can not know if `./something` is a file without extension or a directory to append to destination or not. I'm not sure if doing `COPY --from=0 ./something {app}/something` will work for files and for directories. If that could work, I can add that change in few minutes. 😄 
jhernandez-at-wiris: I see, I was trying to keep compatibility with current behaviour thinking that maybe it's a problem a breaking change. Then I'll do some breaking changes and that will simplify the changes also. Thx for feedback.
coffee-cup: In the past we have just never included files from anywhere but the root of the app. I do agree with @flybayer in that the current behavior is unintuitive. 
jhernandez-at-wiris: Maybe also almost uses in the past include the full app directory, that case was not affected by behavior of Docker's `COPY`.

I have the changes done and I'm reviewing tests. In a little I'll update my branch.
coffee-cup: The tests are also failing because the rust musl example is looking for the compiled binary at the incorrect location. That should be a fairly straightforward fix, but lmk if you would like some help updating it!
github-actions[bot]: This pull request is stale because it has been open 10 days with no activity. Remove stale label or comment or this will be closed in 5 days

Patch:
diff --git a/src/nixpacks/builder/docker/dockerfile_generation.rs b/src/nixpacks/builder/docker/dockerfile_generation.rs
index f7537a5..502a212 100644
--- a/src/nixpacks/builder/docker/dockerfile_generation.rs
+++ b/src/nixpacks/builder/docker/dockerfile_generation.rs
@@ -296,7 +296,7 @@ impl DockerfileGenerator for StartPhase {
 
         let dockerfile: String = match &self.run_image {
             Some(run_image) => {
-                let copy_cmd = utils::get_copy_from_command(
+                let copy_cmds = utils::get_copy_from_commands(
                     "0",
                     &self.only_include_files.clone().unwrap_or_default(),
                     APP_DIR,
@@ -309,12 +309,12 @@ impl DockerfileGenerator for StartPhase {
                   WORKDIR {APP_DIR}
                   COPY --from=0 /etc/ssl/certs /etc/ssl/certs
                   RUN true
-                  {copy_cmd}
+                  {copy_cmds}
                   {start_cmd}
                 ",
                 run_image=run_image,
                 APP_DIR=APP_DIR,
-                copy_cmd=copy_cmd,
+                copy_cmds=copy_cmds.join("\n"),
                 start_cmd=start_cmd,}
             }
             None => {
@@ -370,7 +370,7 @@ impl DockerfileGenerator for Phase {
             (_, Some(files)) => files.clone(),
             _ => vec![".".to_string()],
         };
-        let phase_copy_cmd = utils::get_copy_command(&phase_files, APP_DIR);
+        let phase_copy_cmds = utils::get_copy_commands(&phase_files, APP_DIR);
 
         let cache_mount = utils::get_cache_mount(&cache_key, &phase.cache_directories);
         let cmds_str = if options.incremental_cache_image.is_some() {
@@ -409,7 +409,7 @@ impl DockerfileGenerator for Phase {
                 .join("\n")
         };
 
-        let dockerfile_stmts = vec![build_path, run_path, phase_copy_cmd, cmds_str]
+        let dockerfile_stmts = vec![build_path, run_path, phase_copy_cmds.join("\n"), cmds_str]
             .into_iter()
             .filter(|stmt| !stmt.is_empty())
             .collect::<Vec<_>>()
diff --git a/src/nixpacks/builder/docker/utils.rs b/src/nixpacks/builder/docker/utils.rs
index d19e059..b41c1f1 100644
--- a/src/nixpacks/builder/docker/utils.rs
+++ b/src/nixpacks/builder/docker/utils.rs
@@ -1,3 +1,5 @@
+use std::path::Path;
+
 use super::cache::sanitize_cache_key;
 
 pub fn get_cache_mount(
@@ -24,28 +26,39 @@ pub fn get_cache_mount(
     }
 }
 
-pub fn get_copy_command(files: &[String], app_dir: &str) -> String {
+pub fn get_copy_commands(files: &[String], app_dir: &str) -> Vec<String> {
     if files.is_empty() {
-        String::new()
+        Vec::new()
     } else {
-        format!("COPY {} {}", files.join(" "), app_dir)
+        files
+            .iter()
+            .map(|file| {
+                let file_in_app_dir = Path::new(app_dir)
+                    .join(file.trim_start_matches("./"))
+                    .display()
+                    .to_string();
+
+                format!("COPY {file} {file_in_app_dir}")
+            })
+            .collect()
     }
 }
 
-pub fn get_copy_from_command(from: &str, files: &[String], app_dir: &str) -> String {
+pub fn get_copy_from_commands(from: &str, files: &[String], app_dir: &str) -> Vec<String> {
     if files.is_empty() {
-        format!("COPY --from=0 {} {}", app_dir, app_dir)
+        vec![format!("COPY --from=0 {} {}", app_dir, app_dir)]
     } else {
-        format!(
-            "COPY --from={} {} {}",
-            from,
-            files
-                .iter()
-                .map(|f| f.replace("./", app_dir))
-                .collect::<Vec<_>>()
-                .join(" "),
-            app_dir
-        )
+        files
+            .iter()
+            .map(|file| {
+                let file_in_app_dir = Path::new(app_dir)
+                    .join(file.trim_start_matches("./"))
+                    .display()
+                    .to_string();
+
+                format!("COPY --from={from} {file_in_app_dir} {file_in_app_dir}")
+            })
+            .collect()
     }
 }
 
@@ -81,31 +94,54 @@ mod tests {
         assert_eq!(expected, actual);
     }
 
+    #[cfg(not(target_os = "windows"))]
     #[test]
-    fn test_get_copy_command() {
-        let files = vec!["file1".to_string(), "file2".to_string()];
+    fn test_get_copy_commands() {
         let app_dir = "app";
 
-        assert_eq!(String::new(), get_copy_command(&[], app_dir));
+        assert_eq!(0, get_copy_commands(&[], app_dir).len());
         assert_eq!(
-            format!("COPY {} {}", files.join(" "), app_dir),
-            get_copy_command(&files, app_dir)
+            vec![
+                "COPY file1 app/file1",
+                "COPY ./nested/file app/nested/file",
+                "COPY /from/root /from/root"
+            ],
+            get_copy_commands(
+                &[
+                    "file1".to_string(),
+                    "./nested/file".to_string(),
+                    "/from/root".to_string()
+                ],
+                app_dir
+            ),
         );
     }
 
+    #[cfg(not(target_os = "windows"))]
     #[test]
     fn test_get_copy_from_command() {
         let from = "0";
-        let files = vec!["file1".to_string(), "file2".to_string()];
         let app_dir = "app";
 
         assert_eq!(
-            format!("COPY --from=0 {} {}", app_dir, app_dir),
-            get_copy_from_command(from, &[], app_dir)
+            format!("COPY --from={from} {app_dir} {app_dir}"),
+            get_copy_from_commands(from, &[], app_dir)[0]
         );
         assert_eq!(
-            format!("COPY --from={} {} {}", from, files.join(" "), app_dir),
-            get_copy_from_command(from, &files, app_dir)
+            vec![
+                "COPY --from=0 app/file1 app/file1",
+                "COPY --from=0 app/nested/file app/nested/file",
+                "COPY --from=0 /from/root /from/root"
+            ],
+            get_copy_from_commands(
+                from,
+                &[
+                    "file1".to_string(),
+                    "./nested/file".to_string(),
+                    "/from/root".to_string()
+                ],
+                app_dir
+            ),
         );
     }
 
diff --git a/src/providers/ruby.rs b/src/providers/ruby.rs
index 78c6207..08f44bd 100644
--- a/src/providers/ruby.rs
+++ b/src/providers/ruby.rs
@@ -102,7 +102,8 @@ impl RubyProvider {
 
         if !self.uses_gem_dep(app, "local") {
             // Only run install if Gemfile or Gemfile.lock has changed
-            install.only_include_files = Some(vec!["Gemfile*".to_string()]);
+            install.only_include_files =
+                Some(vec!["Gemfile".to_string(), "Gemfile.lock".to_string()]);
         }
 
         install.add_cmd("bundle install".to_string());
diff --git a/src/providers/rust.rs b/src/providers/rust.rs
index 6e04bbb..747e484 100644
--- a/src/providers/rust.rs
+++ b/src/providers/rust.rs
@@ -134,13 +134,13 @@ impl RustProvider {
     fn get_start(app: &App, env: &Environment) -> Result<Option<StartPhase>> {
         if (RustProvider::get_target(app, env)?).is_some() {
             if let Some(workspace) = RustProvider::resolve_cargo_workspace(app, env)? {
-                let mut start = StartPhase::new(format!("./{}", workspace));
+                let mut start = StartPhase::new(format!("./bin/{}", workspace));
                 start.run_in_slim_image();
                 start.add_file_dependency(format!("./bin/{}", workspace));
 
                 Ok(Some(start))
             } else if let Some(name) = RustProvider::get_app_name(app)? {
-                let mut start = StartPhase::new(format!("./{}", name));
+                let mut start = StartPhase::new(format!("./bin/{}", name));
                 start.run_in_slim_image();
                 start.add_file_dependency(format!("./bin/{}", name));
 


Test patch:
diff --git a/tests/generate_plan_tests.rs b/tests/generate_plan_tests.rs
index ea8f444..931508d 100644
--- a/tests/generate_plan_tests.rs
+++ b/tests/generate_plan_tests.rs
@@ -61,7 +61,7 @@ fn test_rust_rocket() {
         ])
     );
     assert!(start.cmd.is_some());
-    assert_eq!(start.clone().cmd.unwrap(), "./rocket".to_string());
+    assert_eq!(start.clone().cmd.unwrap(), "./bin/rocket".to_string());
     assert!(start.run_image.is_some());
 }
 
diff --git a/tests/snapshots/generate_plan_tests__ruby.snap b/tests/snapshots/generate_plan_tests__ruby.snap
index 0645b23..06e95ba 100644
--- a/tests/snapshots/generate_plan_tests__ruby.snap
+++ b/tests/snapshots/generate_plan_tests__ruby.snap
@@ -28,7 +28,8 @@ expression: plan
         "bundle install"
       ],
       "onlyIncludeFiles": [
-        "Gemfile*"
+        "Gemfile",
+        "Gemfile.lock"
       ],
       "cacheDirectories": [
         "/root/.bundle/cache"
diff --git a/tests/snapshots/generate_plan_tests__ruby_execjs.snap b/tests/snapshots/generate_plan_tests__ruby_execjs.snap
index b4b1371..d12994b 100644
--- a/tests/snapshots/generate_plan_tests__ruby_execjs.snap
+++ b/tests/snapshots/generate_plan_tests__ruby_execjs.snap
@@ -29,7 +29,8 @@ expression: plan
         "bundle install"
       ],
       "onlyIncludeFiles": [
-        "Gemfile*"
+        "Gemfile",
+        "Gemfile.lock"
       ],
       "cacheDirectories": [
         "/root/.bundle/cache"
diff --git a/tests/snapshots/generate_plan_tests__ruby_rails_postgres.snap b/tests/snapshots/generate_plan_tests__ruby_rails_postgres.snap
index feb6029..a19c0ce 100644
--- a/tests/snapshots/generate_plan_tests__ruby_rails_postgres.snap
+++ b/tests/snapshots/generate_plan_tests__ruby_rails_postgres.snap
@@ -33,7 +33,8 @@ expression: plan
         "bundle install"
       ],
       "onlyIncludeFiles": [
-        "Gemfile*"
+        "Gemfile",
+        "Gemfile.lock"
       ],
       "cacheDirectories": [
         "/root/.bundle/cache"
diff --git a/tests/snapshots/generate_plan_tests__ruby_sinatra.snap b/tests/snapshots/generate_plan_tests__ruby_sinatra.snap
index e4a272b..7357ce8 100644
--- a/tests/snapshots/generate_plan_tests__ruby_sinatra.snap
+++ b/tests/snapshots/generate_plan_tests__ruby_sinatra.snap
@@ -28,7 +28,8 @@ expression: plan
         "bundle install"
       ],
       "onlyIncludeFiles": [
-        "Gemfile*"
+        "Gemfile",
+        "Gemfile.lock"
       ],
       "cacheDirectories": [
         "/root/.bundle/cache"
diff --git a/tests/snapshots/generate_plan_tests__ruby_with_node.snap b/tests/snapshots/generate_plan_tests__ruby_with_node.snap
index 7e64737..42ca797 100644
--- a/tests/snapshots/generate_plan_tests__ruby_with_node.snap
+++ b/tests/snapshots/generate_plan_tests__ruby_with_node.snap
@@ -29,7 +29,8 @@ expression: plan
         "bundle install"
       ],
       "onlyIncludeFiles": [
-        "Gemfile*"
+        "Gemfile",
+        "Gemfile.lock"
       ],
       "cacheDirectories": [
         "/root/.bundle/cache"