Skip to content

feat(templater): add absPath template function#2788

Open
mateenali66 wants to merge 3 commits intogo-task:mainfrom
mateenali66:fix/2681-abs-path
Open

feat(templater): add absPath template function#2788
mateenali66 wants to merge 3 commits intogo-task:mainfrom
mateenali66:fix/2681-abs-path

Conversation

@mateenali66
Copy link
Copy Markdown
Contributor

adds absPath as a thin wrapper over filepath.Abs so Taskfiles can resolve paths with .. or . components without shelling out. slots in next to joinPath and relPath in internal/templater/funcs.go, same one-liner style.

originally discussed in #2681. i first tried adding osAbs in slim-sprig (go-task/slim-sprig#23), but per @trulede's suggestion in #2681 (comment) that repo is inactive, so moving the change here instead.

the integration test under testdata/abs_path verifies {{absPath "foo/../bar" | base}} resolves to "bar". also added a one-line example to the Path Functions section of the templating reference docs, and a CHANGELOG entry.

symlink resolution (filepath.EvalSymlinks) was also mentioned in the issue but keeping scope to just absPath for now, happy to follow up with a second PR if wanted.

Fixes #2681

adds absPath as a thin wrapper over filepath.Abs, so Taskfiles can
resolve paths containing .. or . components without shelling out.

fits the same style as the existing joinPath and relPath entries in
internal/templater/funcs.go. added an integration test under
testdata/abs_path and a docs example in the templating reference.

Fixes go-task#2681

Signed-off-by: Mateen Anjum <mateenali66@gmail.com>
sprig's `base` wraps path.Base which only splits on forward slashes,
so on Windows the absolute path came back with backslashes and base
returned the whole string. osBase wraps filepath.Base which is
separator-aware.

Signed-off-by: Mateen Anjum <mateenali66@gmail.com>
Copy link
Copy Markdown
Contributor

@trulede trulede left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I've suggested an improvement to the test.

tasks:
default:
cmds:
- cmd: echo '{{absPath "foo/../bar" | base}}'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if you remove base?

I wonder if this test is really proving that the function is working. It is really checking the last element of the path, and not that the path is absolute.

You should be able to echo the TASKDIR to get the working dir, and then absPath should resolve to TASKDIR+"/bar", then you can programmatically read both lines, and construct the check condition from the first line, and compare to the second.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

switched to comparing against filepath.Join(cwd, "bar") computed at test time in 8c9d68f. the osBase pipe only checked the last element, you were right that it wasn't proving the result is absolute or that .. got resolved. the new assertion fails if either property breaks.

tried the TASKFILE_DIR approach first but absPath runs during template evaluation using the test process cwd (the repo root), not the taskfile dir, so TASKFILE_DIR + /bar wouldn't match the actual output. cwd-based construction was the closest equivalent that still validates both properties.

switched from osBase piping to comparing against filepath.Join(cwd,
"bar") computed at test time. the previous assertion only checked
the last path element, which didn't verify that the result is actually
absolute or that .. was resolved. the new check fails if either
property breaks.

Signed-off-by: Mateen Anjum <mateenali66@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a resolvePath or absPath function

2 participants