Don't use pipefail with non-bash exec/custom commands, fixes ddev/dde…

…v-contrib#80 (#2395)

* Don't use pipefail with non-bash exec/custom commands, fixes ddev/ddev-contrib#80
* Add test coverage for exec error cases and non-ddev containers

pkg/ddevapp/ddevapp.go

@@ -1065,7 +1065,8 @@ func (app *DdevApp) Exec(opts *ExecOpts) (string, string, error) {
 	if !nodeps.ArrayContainsString([]string{"web", "db", "dba"}, opts.Service) {
 		shell = "sh"
 	}
-	exec = append(exec, shell, "-c", "set -eu -o pipefail && ( "+opts.Cmd+")")
+	errcheck := "set -eu"
+	exec = append(exec, shell, "-c", errcheck+` && ( `+opts.Cmd+`)`)
 
 	files, err := app.ComposeFiles()
 	if err != nil {

pkg/ddevapp/ddevapp_test.go

@@ -1728,17 +1728,28 @@ func TestDdevImportFilesCustomUploadDir(t *testing.T) {
 func TestDdevExec(t *testing.T) {
 	assert := asrt.New(t)
 	app := &ddevapp.DdevApp{}
+	testDir, _ := os.Getwd()
 
-	for _, site := range TestSites {
+	for index, site := range TestSites {
 		switchDir := site.Chdir()
 		runTime := util.TimeTrack(time.Now(), fmt.Sprintf("%s DdevExec", site.Name))
 
+		if index == 0 {
+
+			err := fileutil.CopyFile(filepath.Join(testDir, "testdata", t.Name(), "docker-compose.busybox.yaml"), filepath.Join(site.Dir, ".ddev", "docker-compose.busybox.yaml"))
+			defer func() {
+				err = os.RemoveAll(filepath.Join(site.Dir, ".ddev", "docker-compose.busybox.yaml"))
+				assert.NoError(err)
+			}()
+			assert.NoError(err)
+		}
 		err := app.Init(site.Dir)
 		assert.NoError(err)
 
 		app.Hooks = map[string][]ddevapp.YAMLTask{"post-exec": {{"exec-host": "touch hello-post-exec-" + app.Name}}, "pre-exec": {{"exec-host": "touch hello-pre-exec-" + app.Name}}}
 		defer func() {
 			app.Hooks = nil
+			_ = app.Stop(true, false)
 			_ = app.WriteConfig()
 		}()
 
@@ -1801,6 +1812,38 @@ func TestDdevExec(t *testing.T) {
 			})
 			assert.NoError(err)
 			assert.Regexp("/etc/php.*/php.ini", out)
+
+			// Make sure error works for unset env vars, etc.
+			_, stderr, err := app.Exec(&ddevapp.ExecOpts{
+				Service: "web",
+				Cmd:     "echo $ENVDOESNOTEXIST",
+			})
+			assert.Error(err)
+			assert.Contains(stderr, "ENVDOESNOTEXIST: unbound variable")
+
+		}
+
+		// Make sure that exec works on non-ddev container like busybox as well
+		if index == 0 {
+			_, _, err = app.Exec(&ddevapp.ExecOpts{
+				Service: "busybox",
+				Cmd:     "ls | grep bin",
+			})
+			assert.NoError(err)
+
+			_, stderr, err := app.Exec(&ddevapp.ExecOpts{
+				Service: "busybox",
+				Cmd:     "echo $ENVDOESNOTEXIST",
+			})
+			assert.Error(err)
+			assert.Contains(stderr, "parameter not set")
+
+			_, stderr, err = app.Exec(&ddevapp.ExecOpts{
+				Service: "busybox",
+				Cmd:     "this is an error;",
+			})
+			assert.Error(err)
+			assert.Contains(stderr, "this: not found")
 		}
 
 		err = app.Stop(true, false)

pkg/ddevapp/testdata/TestDdevExec/docker-compose.busybox.yaml

@@ -0,0 +1,9 @@
+version: '3.6'
+services:
+  busybox:
+    image: busybox
+    command: tail -f /dev/null
+    container_name: ddev-${DDEV_SITENAME}-busybox
+    labels:
+      com.ddev.site-name: ${DDEV_SITENAME}
+      com.ddev.approot: $DDEV_APPROOT