Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression: Piping to "task" command results in task: Missing schema version in Taskfile "__stdin__" #1593

Closed
kasadaamos opened this issue Apr 16, 2024 · 11 comments · Fixed by #1623
Labels
area: reader Changes related to the Taskfile reader. type: regression

Comments

@kasadaamos
Copy link

  • Task version:
❯ task --version
Task version: 3.36.0
  • Operating system:
❯ sw_vers 
ProductName:            macOS
ProductVersion:         14.4.1
BuildVersion:           23E224
  • Experiments enabled:
❯ task --experiments
* GENTLE_FORCE:         off
* REMOTE_TASKFILES: off
* ANY_VARIABLES:    off

The problem: If Task is executed with a pipeline as input, it tries to parse the input as a Taskfile.yaml format and fails.

Here is a minimal Taskfile.yaml to demonstrate the issue:

# https://taskfile.dev/
version: '3'

tasks:
  test:
    cmds:
      - echo hi

When executed normally, it does the expected and prints hi:

❯ task test
task: [test] echo hi
hi

When executed with any input via a pipeline, it prints an error and aborts:

❯ echo | task test
task: Missing schema version in Taskfile "__stdin__"

When executed with some other input, it prints an error which could indicate that it expects a Taskfile.yaml format in its standard input?

❯ echo world | task -t Taskfile.yaml test
task: Failed to parse __stdin__:
yaml: line 1: cannot unmarshal !!str into taskfile

I tried to force it to read the Taskfile.yaml even if it's redundant but it does the same thing - it seems to expect a Taskfile.yaml file format in its standard input:

❯ echo world | task -t Taskfile.yaml test
task: Failed to parse __stdin__:
yaml: line 1: cannot unmarshal !!str into taskfile

❯ echo | task -t Taskfile.yaml test
task: Missing schema version in Taskfile "__stdin__"
@task-bot task-bot added the state: needs triage Waiting to be triaged by a maintainer. label Apr 16, 2024
@kasadaamos
Copy link
Author

Looking at the code, it might be related to https://github.com/go-task/task/blob/main/taskfile/node.go#L33:

	// Check if there is something to read on STDIN
	stat, _ := os.Stdin.Stat()
	if (stat.Mode()&os.ModeCharDevice) == 0 && stat.Size() > 0 {
		return NewStdinNode(dir)
	}
	return NewNode(l, entrypoint, dir, insecure, timeout)

i.e. Task will try to read from stdin if it's not a characater device (e.g. a terminal) and has something to read from it (e.g. a pipeline with some data in it).

@kasadaamos
Copy link
Author

Now I see that this is considered a feature (🤯) and there is no way to turn this off - it was introduced in #1483

I'd like to ask for a flag to turn this feature off. It prevents sub-commands executed from inside the Taskfile from being able to read input from stdin.

@kasadaamos kasadaamos changed the title Piping to "task" command results in task: Missing schema version in Taskfile "__stdin__" Regression: Piping to "task" command results in task: Missing schema version in Taskfile "__stdin__" Apr 16, 2024
@armenr
Copy link

armenr commented Apr 17, 2024

Our temporary workaround for this (for tasks not expecting anything other than just to be invoked):

task my:task:name < /dev/null

@pd93
Copy link
Member

pd93 commented Apr 17, 2024

There's a couple of things here. Firstly, if a user specifies -t, I think this should stop Task from reading stdin. Secondly, I agree that a TASK_NO_STDIN variable and/or a --no-stdin flag or similar are good ideas. I think we can use this issue to track this as a feature request.

@pd93 pd93 added type: feature A new feature or functionality. area: reader Changes related to the Taskfile reader. and removed state: needs triage Waiting to be triaged by a maintainer. labels Apr 17, 2024
@amosshapira
Copy link

amosshapira commented Apr 17, 2024

There's a couple of things here. Firstly, if a user specifies -t, I think this should stop Task from reading stdin. Secondly, I agree that a TASK_NO_STDIN variable and/or a --no-stdin flag or similar are good ideas. I think we can use this issue to track this as a feature request.

  1. I tried -t Taskfile.yaml and it didn't help (see the original comment showing this).
  2. As far as I followed the code, there is no condition to avoid reading from stdin.
  3. I think it should be the other way around - require a -t - or --stdin-taskfile to enable the existing feature since this is the out-of-line configuration as opposed to the "UNIX Way (tm)"

@amosshapira
Copy link

Our temporary workaround for this (for tasks not expecting anything other than just to be invoked):

task my:task:name < /dev/null

That won't solve the original problem, we want to be able to do:

echo 'some input here' | task name-of-task-which-executes-a-command-which-reads-from-stdin`

Telling task to read from /dev/null won't help here.

@aucampia
Copy link

Not being able to pipe into task commands is a serious problem, would be great to see this fixed. We relied on this behaviour in several places, so this is a regression from our point of view.

@pd93
Copy link
Member

pd93 commented Apr 26, 2024

  1. I tried -t Taskfile.yaml and it didn't help (see the original comment showing this).

@amosshapira Apologies, I don't think my response was clear. I wasn't saying this exists now - I was saying this is probably how it should work.

  1. As far as I followed the code, there is no condition to avoid reading from stdin.

Agreed. We should fix this.

  1. I think it should be the other way around - require a -t - or --stdin-taskfile to enable the existing feature since this is the out-of-line configuration as opposed to the "UNIX Way (tm)"

This is an interesting idea. I quite like this. It would be interesting to gather opinions from those who originally requested the stdin functionality in #655. CC @brianbraunstein, @dAnjou, @mgbowman.

For example, those users requested the ability to do task < <(cat ~/shared/Taskfile.yaml). This is possible today, but not if we changed to -t -. Instead, they would have to do something like task -t <(cat ./shared/Taskfile.yaml). Does this satisfy both sides?

Not being able to pipe into task commands is a serious problem, would be great to see this fixed. We relied on this behaviour in several places, so this is a regression from our point of view.

@aucampia Since this is causing issues with existing users, I'll reclassify this as a regression and we'll get a fix out as soon as we agree how to proceed.


Draft PR with -t - for those who wish to try it: #1623

@pd93 pd93 added type: regression and removed type: feature A new feature or functionality. labels Apr 26, 2024
@barryrobison
Copy link

task -f - < ~/shared/Taskfile.yaml

Should still work as intended. The bash redirect operator gets the Taskfile to STDIN all the same.

@amosshapira
Copy link

For example, those users requested the ability to do task < <(cat ~/shared/Taskfile.yaml). This is possible today, but not if we changed to -t -. Instead, they would have to do something like task -t <(cat ./shared/Taskfile.yaml). Does this satisfy both sides?

It would satisfy my requirements.

And as other already pointed out, it's a simple task -t - < file or command which prints taskfile.yaml | task -t - for those who need to read Taskfile.yaml from stdin.

@pd93 pd93 closed this as completed in #1623 May 8, 2024
@pd93
Copy link
Member

pd93 commented May 9, 2024

This should be fixed in the latest release (v3.37.0). Thanks to everyone for the feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: reader Changes related to the Taskfile reader. type: regression
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants