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.

Sign up for GitHub

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

Jump to bottom

Incorrect/confusing include cycle detected error message after a silly mistake #1881

Open
VinGarcia opened this issue Oct 24, 2024 · 4 comments · May be fixed by #1902
Open

Incorrect/confusing include cycle detected error message after a silly mistake #1881

VinGarcia opened this issue Oct 24, 2024 · 4 comments · May be fixed by #1902
Assignees
@semihbkgr
Labels
area: parser Changes related to parsing Taskfiles. good first issue Issues that are good for first-time contributors to pick up.

Comments

@VinGarcia
Copy link

  • Task version: v3.38.0 (h1:O7kgA6BfwktXHPrheByQO46p3teKtRuq1EpGnFxNzbo=)
  • Operating system: MacOS M1 Sonoma

I did a silly mistake on a Taskfile and got a misleading message as a response, so it took me a while to figure out exactly what the issue was.

Minimum Reproducible example

I create two taskfiles in the same directory with the following names:

# Taskfile.yml
version: '3'

includes:
  common: ./TaskfileCommon.yml

tasks:
  test:
    cmds:
      - task: common:test

And

# TaskfileCommon.yml
version: '3'

includes: # This was the silly mistake, this should be `vars` not `includes`
  GOBIN:
    sh: echo $(go env GOPATH)/bin

tasks:
  test:
    cmds:
      - "{{.GOBIN}}/richgo test ./..."

Then I tried running:

task common:test

And the error message I got back was this:

task: include cycle detected between /Users/vingarcia/temp/tfile/Taskfile.yml <--> /Users/vingarcia/temp/tfile/TaskfileCommon.yml

I am not sure how easy it is to fix this issue and I understand this might not be a high priority bug, but I thought it would be worth mentioning anyway.

Great tool, thanks for making it and keeping it open source =]
@task-bot task-bot added the state: needs triage Waiting to be triaged by a maintainer. label Oct 24, 2024
@semihbkgr
Copy link

semihbkgr commented Nov 1, 2024 •
edited
Loading

Hi @VinGarcia , You’re getting a cyclic include error because it implicitly re-points to the default Taskfile.yml file.

In TaskfileCommon.yml, the GOBIN field is missing the path (taskfile) field, so it defaults to an empty string, which points only to the current directory (joining dir and path with path being empty). As a result, when the code tries to locate the actual file using the Exists function, it doesn't find the actual file and then checks for default filenames—in this case, Taskfile.yml.

task/taskfile/taskfile.go

Lines 105 to 126 in fe09c01

func Exists(l *logger.Logger, path string) (string, error) {
fi, err := os.Stat(path)
if err != nil {
return "", err
}
if fi.Mode().IsRegular() ||
fi.Mode()&os.ModeDevice != 0 ||
fi.Mode()&os.ModeSymlink != 0 ||
fi.Mode()&os.ModeNamedPipe != 0 {
return filepath.Abs(path)
}
for _, taskfile := range defaultTaskfiles {
alt := filepathext.SmartJoin(path, taskfile)
if _, err := os.Stat(alt); err == nil {
l.VerboseOutf(logger.Magenta, "task: [%s] Not found - Using alternative (%s)\n", path, taskfile)
return filepath.Abs(alt)
}
}
return "", errors.TaskfileNotFoundError{URI: path, Walk: false}
}

The view of the include cycle is:

graph LR;
    Taskfile.yml-->|common|TaskfileCommon.yml;
    TaskfileCommon.yml-->|GOBIN|Taskfile.yml;
Loading

@semihbkgr
Copy link

I'm not sure if this is an issue that needs to be fixed, but perhaps we could consider preventing empty strings and null values in the path of includes if there’s no use case for them.

@pd93
Copy link
Member

pd93 commented Nov 1, 2024 

includes: # This was the silly mistake, this should be `vars` not `includes`
  GOBIN:
    sh: echo $(go env GOPATH)/bin

This should absolutely be generating an error when no taskfile: or directory: is specified on an include.

@pd93 pd93 added type: bug Something not working as intended. area: parser Changes related to parsing Taskfiles. good first issue Issues that are good for first-time contributors to pick up. and removed state: needs triage Waiting to be triaged by a maintainer. labels Nov 1, 2024
@semihbkgr
Copy link

@pd93 So, I think I can work on this.

@semihbkgr semihbkgr linked a pull request Nov 2, 2024 that will close this issue
Open
@pd93 pd93 removed the type: bug Something not working as intended. label Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@semihbkgr semihbkgr

Labels
area: parser Changes related to parsing Taskfiles. good first issue Issues that are good for first-time contributors to pick up.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Make the taskfile in include required semihbkgr/task
4 participants
@VinGarcia @pd93 @semihbkgr @task-bot