Restore pre-Path::Tiny HasLocation upward traversal semantics

[xsawyerx]

A regression in app root detection was introduced by the Path::Tiny migration in Dancer2::Core::Role::HasLocation.

When walking up parent directories to find an app root (lib+bin or .dancer), switching to $subdir->parent changed behavior for relative paths (notably .), which could stop effective upward traversal and lead to incorrect root selection on some systems.

Fix by restoring pre-migration traversal behavior:

• advance with Path::Tiny::path($subdir, '..') each iteration
• stop at filesystem root via absolute-path comparison

This keeps existing caller semantics intact while fixing template/public/config resolution failures caused by misdetected app location.

[Grinnz]

On my machine this fails a lot of tests still, seems to be the same failures as reported in the CI

[xsawyerx]

Hmm... Okay. I need to figure out how to reproduce it.

[toddr-bot]

PR Review — Restore pre-Path::Tiny HasLocation upward traversal semantics

The diagnosis is correct: Path::Tiny::path('.') is a fixed point of ->parent, so the original upward traversal stalled on relative . paths. The fix to use path($subdir, '..') addresses this. However, two issues need resolution before merge: (1) the unexplained switch from realpath to absolute in the termination condition, and (2) the unresolved CI/test failures reported by @Grinnz, which may stem from downstream code (like the blib regex) reacting differently to the new relative-path string representation. The fix approach is sound in principle but needs the test failures investigated and resolved.

---

🔴 Blocking

1. CI test failures reported and unresolved (lib/Dancer2/Core/Role/HasLocation.pm, L79-81)
@Grinnz reports that this patch causes widespread test failures (matching CI). The core traversal fix (path($subdir, '..')) looks logically correct, but the PR should not merge until the test failures are investigated and resolved. The accumulated relative .. segments (e.g., ../../..) change the string representation of $subdir throughout the rest of the method — any downstream code or test that compares or pattern-matches against $subdir stringification (like the blib regex on line 72: $subdir !~ m![\\/]blib[\\/]?$!) may behave differently.

Specifically, the blib guard $subdir !~ m![\\/]blib[\\/]?$! matches against the stringified path. With the old ->parent traversal on absolute-ish paths, $subdir might stringify to /path/to/blib; with the new approach, it stringifies to relative paths like ../.. which would never match the blib pattern at all, potentially changing which directories are accepted as app roots.

Recommend: run the full test suite, capture the failures, and verify whether the blib regex or other string-based checks on $subdir are the cause.

$subdir !~ m![\\\x2f]blib[\\\x2f]?$!

🟡 Important

1. Unexplained switch from realpath to absolute (lib/Dancer2/Core/Role/HasLocation.pm, L79-81)
The change from $subdir->parent to Path::Tiny::path($subdir, '..') is well-motivated: path('.') is a fixed point of ->parent, so the upward walk stalls. However, the termination condition also changed from ->realpath to ->absolute, and the rationale for this secondary change isn't documented.

realpath resolves symlinks and requires the path to exist on disk; absolute prepends cwd and collapses .. components but does NOT resolve symlinks. In practice both should eventually yield / when walking upward, but they diverge when symlinks are involved in the directory chain. If the application root lives under a symlink, realpath and absolute produce different strings, which could cause the root check to misfire in one direction or the other.

If the intent is to avoid a potential realpath failure on a non-existent path, please document that. If there is no specific reason, consider keeping realpath for consistency with the rest of the method (line 87 still uses realpath).

last if $subdir->absolute->stringify eq Path::Tiny->rootdir->stringify;

---

Checklist

• No hardcoded secrets
• No path traversal from user input
• Test suite passes — critical incompatible splat #2

---

Summary

The diagnosis is correct: Path::Tiny::path('.') is a fixed point of ->parent, so the original upward traversal stalled on relative . paths. The fix to use path($subdir, '..') addresses this. However, two issues need resolution before merge: (1) the unexplained switch from realpath to absolute in the termination condition, and (2) the unresolved CI/test failures reported by @Grinnz, which may stem from downstream code (like the blib regex) reacting differently to the new relative-path string representation. The fix approach is sound in principle but needs the test failures investigated and resolved.

---

Automated review by Kōan

[toddr-bot]

@Grinnz: On my machine this fails a lot of tests still, seems to be the same failures as reported in the CI

The test failures may be related to how $subdir is now stringified. With the old ->parent traversal, $subdir carried forward Path::Tiny objects that often resolved to clean absolute-ish paths. With the new path($subdir, '..') approach, the stringified form is a chain of relative .. segments (e.g., ../.., ../../..).

This matters because the blib guard on line 72 ($subdir !~ m![\\/]blib[\\/]?$!) pattern-matches against the stringified path. A relative path like ../.. will never match the blib pattern, which changes which directories pass the guard.

A potential fix: resolve $subdir to an absolute or real path at the point of assignment, e.g.:

$subdir = Path::Tiny::path( $subdir, '..' )->realpath;

This preserves the upward-walking fix while keeping $subdir in a canonical absolute form that the rest of the loop expects.

[toddr-bot]

@xsawyerx: Hmm... Okay. I need to figure out how to reproduce it.

To reproduce, try running the test suite with the patch applied — @Grinnz confirms the failures match CI. A likely culprit is the blib regex on line 72 and possibly the realpath call on line 87. The new relative-path form of $subdir (../.. etc.) changes string matching behavior throughout the loop. Consider resolving to an absolute path each iteration:

$subdir = Path::Tiny::path( $subdir, '..' )->realpath;

This would fix the ->parent stall on . while keeping $subdir in canonical absolute form.