Merge branch 'master' into dorfire/bpcmds

microsoft · Apr 22, 2019 · 810ff34 · 810ff34
2 parents c1a8628 + 3ed44e5
commit 810ff34
Show file tree

Hide file tree

Showing 6 changed files with 54 additions and 3 deletions.
diff --git a/src/BuildScriptGenerator/Node/NodeConstants.cs b/src/BuildScriptGenerator/Node/NodeConstants.cs
@@ -23,6 +23,7 @@ internal static class NodeConstants
         internal const string AllNodeModulesDirName = "__oryx_all_node_modules";
         internal const string ProdNodeModulesDirName = "__oryx_prod_node_modules";
         internal const string NodeModulesDirName = "node_modules";
+        internal const string NodeModulesToBeDeletedName = "_del_node_modules";
         internal const string NodeModulesZippedFileName = "node_modules.zip";
         internal const string NodeModulesTarGzFileName = "node_modules.tar.gz";
         internal const string NodeModulesFileBuildProperty = "compressedNodeModulesFile";

diff --git a/src/BuildScriptGenerator/Node/NodePlatform.cs b/src/BuildScriptGenerator/Node/NodePlatform.cs
@@ -303,6 +303,7 @@ public IEnumerable<string> GetDirectoriesToExcludeFromCopyToIntermediateDir(
                 NodeConstants.AllNodeModulesDirName,
                 NodeConstants.ProdNodeModulesDirName,
                 NodeConstants.NodeModulesDirName,
+                NodeConstants.NodeModulesToBeDeletedName,
                 NodeConstants.NodeModulesZippedFileName,
                 NodeConstants.NodeModulesTarGzFileName
             };
@@ -364,7 +365,7 @@ private static bool GetNodeModulesPackOptions(
                 else if (string.Equals(compressNodeModulesOption, ZipNodeModulesOption, StringComparison.InvariantCultureIgnoreCase))
                 {
                     compressedNodeModulesFileName = NodeConstants.NodeModulesZippedFileName;
-                    compressNodeModulesCommand = $"zip -q -r";
+                    compressNodeModulesCommand = $"zip -y -q -r";
                     isNodeModulesPackaged = true;
                 }
             }

diff --git a/src/startupscriptgenerator/node/scriptgenerator.go b/src/startupscriptgenerator/node/scriptgenerator.go
@@ -79,6 +79,16 @@ func (gen *NodeStartupScriptGenerator) GenerateEntrypointScript() string {
 		// Some versions of node, in particular Node 4.8 and 6.2 according to our tests, do not find the node_modules
 		// folder at the root. To handle these versions, we also add /node_modules to the NODE_PATH directory.
 		scriptBuilder.WriteString("        export NODE_PATH=/node_modules:$NODE_PATH\n")
+		// NPM adds the current directory's node_modules/.bin folder to PATH before it runs, so commands in
+		// "npm start" can files there. Since we move node_modules, we have to add it to the path ourselves.
+		scriptBuilder.WriteString("        export PATH=/node_modules/.bin:$PATH\n")
+		// To avoid having older versions of packages available, we delete existing node_modules folder.
+		// We do so in the background to not block the app's startup.
+		scriptBuilder.WriteString("        if [ -d node_modules ]; then\n")
+		// We move the directory first to prevent node from start using it
+		scriptBuilder.WriteString("            mv -f node_modules _del_node_modules || true\n")
+		scriptBuilder.WriteString("            nohup rm -fr _del_node_modules &> /dev/null &\n")
+		scriptBuilder.WriteString("        fi\n")
 		scriptBuilder.WriteString("    fi\n")
 		scriptBuilder.WriteString("    echo \"Done.\"\n")
 		scriptBuilder.WriteString("fi\n\n")

diff --git a/src/startupscriptgenerator/node/wrapper/node b/src/startupscriptgenerator/node/wrapper/node
@@ -23,7 +23,6 @@ case $targetScript in
     # If the path is /usr/local, we asumme we're running an intermal library or tool, e.g. npm,
     # and not the app. In this case we run the node binary unchanged.
     "/usr/local/"*)
-        echo "target1: $targetScript"
         $originalNodePath $@
         ;;
     *)

diff --git a/tests/BuildScriptGenerator.Tests/Node/NodeBuildScriptGenerationTest.cs b/tests/BuildScriptGenerator.Tests/Node/NodeBuildScriptGenerationTest.cs
@@ -407,7 +407,7 @@ public void GeneratedScript_ZipsNodeModules_IfZipNodeProperty_IsZip()
                     NodeConstants.ProductionOnlyPackageInstallCommandTemplate,
                     NpmInstallCommand),
                 compressedNodeModulesFileName: "node_modules.zip",
-                compressNodeModulesCommand: "zip -q -r");
+                compressNodeModulesCommand: "zip -y -q -r");
 
             // Act
             var snippet = scriptGenerator.GenerateBashBuildScriptSnippet(context);

diff --git a/tests/Oryx.Integration.Tests/LocalDockerTests/NodeEndToEndTests.cs b/tests/Oryx.Integration.Tests/LocalDockerTests/NodeEndToEndTests.cs
@@ -607,6 +607,46 @@ await EndToEndTestHelper.BuildRunAndAssertAppAsync(
                 });
         }
 
+        [Theory]
+        // TODO - renable it with 851972, removing the inline data
+        //[MemberData(nameof(TestValueGenerator.GetNodeVersions), MemberType = typeof(TestValueGenerator))]
+        [InlineData("10.10")]
+        [InlineData("10.14")]
+        public async Task Node_CreateReactAppSample_zippedNodeModules(string nodeVersion)
+        {
+            // Arrange
+            var appName = "create-react-app-sample";
+            var hostDir = Path.Combine(_hostSamplesDir, "nodejs", appName);
+            var volume = DockerVolume.Create(hostDir);
+            var appDir = volume.ContainerDir;
+            var portMapping = $"{HostPort}:{ContainerPort}";
+            var runAppScript = new ShellScriptBuilder()
+                .AddCommand($"cd {appDir}")
+                .AddCommand($"oryx -appPath {appDir} -bindPort {ContainerPort}")
+                .AddCommand(DefaultStartupFilePath)
+                .ToString();
+
+            await EndToEndTestHelper.BuildRunAndAssertAppAsync(
+                appName,
+                _output,
+                volume,
+                "oryx",
+                new[] { "build", appDir, "-l", "nodejs", "--language-version", nodeVersion, "-i", "/tmp", "-p", "compress_node_modules=zip" },
+                $"oryxdevms/node-{nodeVersion}",
+                portMapping,
+                "/bin/sh",
+                new[]
+                {
+                    "-c",
+                    runAppScript
+                },
+                async () =>
+                {
+                    var data = await _httpClient.GetStringAsync($"http://localhost:{HostPort}/");
+                    Assert.Contains("<title>React App</title>", data);
+                });
+        }
+
         [Fact]
         public async Task Node_CreateReactAppSample_singleImage()
         {