`ignore_error` not working on new built-in `cp` command

[VnUgE]

Description

• The new built-in cp command to copy a file that may or may not exist, with the ignore_error: true property set on the cmd: scope
• Expected cp command errors to be suppressed
• cp errors were not suppressed and tasks fail
• No change if -f is used or not.
• Adding powershell before the cp command works as expected even if the powershell command fails.

This task is just a best-effort file copy for some testing, and some files may not exist for now, and simply suppressing the error is the simplest solution for now.

Example error

task: Failed to run task "dev-plugins-fetch": GetFileAttributesEx C:\tests\config\Essentials.Accounts.json: The system cannot find the file specified.

Version

3.45.4

Operating system

Windows

Experiments Enabled

None

Example Taskfile

version: 3

tasks:

  default:
    cmds:
     - task: dev-build-plugin
       vars:
         ASM_NAME: "TestAssembly"
         OUTPUT_DIR: "{{ .USER_WORKING_DIR }}"

  dev-build-plugin:
    internal: true
    dir: '..'
    requires: { ASM_NAME, OUTPUT_DIR}
    cmds:
     - cmd: mkdir -p '{{ .OUTPUT_DIR }}'

     - cmd: cp -f config/{{ .ASM_NAME }}.json '{{ .OUTPUT_DIR }}/{{ .ASM_NAME }}.json'
       ignore_error: true

[trulede]

Works on Linux, not on Windows/Powershell. Seems likely that errors.As() is the cause, but I don't know why. I don't have a devenv for windows ...

task/task.go

Line 222 in 2161f33

if errors.As(err, &exitCode) {

It is possible that the error returned from mvdan/sh under windows is not the expected type (i.e. not interp.ExitStatus).

https://github.com/mvdan/sh/blob/7ad4e4310897576a02731b31da97dfebe4a2f9cd/interp/handler.go#L161

[trulede]

The error fs.PathError{Op:"CreateFile"...) from os.OpenFile (in u-root function copyRegularFile()) is passed directly back to Task. Task has been implemented to expect an error of type interp.ExitStatus, so it does not handle the error from u-root.

In mvdan/sh/interp/api.go fromHandlerError() windows code will call fatal(error) which then sets e.code to 1, and e.err to the aforementioned error.

Later on, in mvdan/sh/api.go Run() we see that error is now processed:

// Return the first of: a fatal error, a non-fatal handler error, or the exit code.
if err := r.exit.err; err != nil {
	return err
}
if code := r.exit.code; code != 0 {
	return ExitStatus(code)
}
return nil

and you see that err has priority over code, so, in the windows case, err is returned ... which triggers the failing condition observed in Task. The "code", although set, is actually lost, which is what eventually breaks the Task code.

With that in mind, there are a few ways to fix this problem; have exitStatus:fatal() wrap the error in an ExitStatus error, or change Task so that it also handles other error conditions.

[trulede]

@andreynering or @pd93 do either of you know of the top of you head how to deal with this kind of problem:

func (e *exitStatus) fatal(err error) {
	if !e.fatalExit && err != nil {
		e.exiting = true
		e.fatalExit = true
		e.err = err    // **** u-root error is directly used here, and Task does not handle that (it looks for interp.ExitStatus)
		if e.code == 0 {
			e.code = 1
		}
	}
}

func (e *exitStatus) fromHandlerError(err error) {
	if err != nil {
		var exit errBuiltinExitStatus
		var es ExitStatus
		if errors.As(err, &exit) {
			*e = exitStatus(exit)
		} else if errors.As(err, &es) {
			e.err = err    // **** task looks for this condition, esp. ExitStatus, which is the case on linux
			e.code = uint8(es)
		} else {
			// **** windows/u-root takes this path
			e.fatal(err) // handler's custom fatal error
		}
	} else {
		e.code = 0
	}
}

Task seems to be deliberately only enabling the "ignore-error" for interop.ExitStatus errors, which might be sensible (since other errors are probably non-ignorable). So, the problem might need to be solved in the mcdan/sh interp ... specifically in the area identified above, probably by wrapping the err with ExitStatus.

Or, it could be fixed in the coreutils ExecHandler(), by wrapping the error there, before it reaches the above code. I think that could be the better solution.

[andreynering]

So, the problem might need to be solved in the mcdan/sh interp ... specifically in the area identified above, probably by wrapping the err with ExitStatus.

@trulede Yeah, wrapping the error is probably the right solution here! Just not sure if on the lib (mvdan/sh) or on our handler.