globwalk does not work with several relative paths

[FibreFoX]

While migrating from a NodeJS-based project to a Rust-based project I found Tera as a template-engine, it was ment to be my introduction project to learn Rust.

As I separate my program and the used templates, I wanted to pass a relative path of some upper folder, but Tera did not find them. After some investigation (and local experiments to change globwalk with glob, which did not work due to unsupported syntax having curly-braces "examples/basic/templates/**/*.{html, xml}"), I found out that it only is an issue for path that point to the current or upper-level folders.

Globwalk does not work for paths starting with./ or ../, there is already an issue open at the other project: Gilnaa/globwalk#28

[Keats]

So it should be fixed upstream then

[FibreFoX]

At least there, but until then I think it's worth to get mentioned somewhere, that's why I opened this issue here. Others might wonder too.

[eguiraud]

Hi, due to Gilnaa/globwalk#28 mentioned above passing a path starting with ./ or ../ to Tera::new is completely broken. It does not look like Gilnaa/globwalk#28 is going to be fixed any time soon (it's been 2 years already and there is no consensus on the direction to take).

Could Tera maybe side-step the issue by switching to glob or globset?

In my project I'm working around this by taking care to normalize the path I pass to Tera::new, but if there is a clear direction for a fix I could maybe contribute it.

[Keats]

Could Tera maybe side-step the issue by switching to glob or globset?

If we can make it work without breaking any existing code, sure.

[eguiraud]

without breaking any existing code

I am not sure how to guarantee that (I am not sure globwalk and globset behave identically for all inputs).

Something simpler to check would be that the change doesn't break any existing test 😅

[caemor]

Alternatively we could check if the input starts with ./ or ../ and return an error instead of an empty list and document this for the Tera::new command.

This wouldn't really fix the issue, but at least make it more visible why tera doesn't work as expected for this usecase.

[Keats]

That would still be a breaking change though.
If someone can do a PR we can test things.

[eguiraud]

we could check if the input starts with ./ or ../ and return an error

That would still be a breaking change though.

Only for users that expect that adding a ./ at the beginning of the path always results in an empty list. I'm not sure why you would want to keep those weirdos happy... 😄

If someone can do a PR we can test things.

It should be easy enough to have Tera detect this case and error out, I'll try to propose a PR soon. Too bad there is no clear path to an actual resolution.

[Keats]

I'm not sure why you would want to keep those weirdos happy... 😄

https://xkcd.com/1172/

[eguiraud]

I'm perfectly sure those weirdos exist (hyrum's law and all that) 😄 but you don't necessarily have to keep them happy :P

[Keats]

Yeah it's probably fine if it always returned an empty list of files tbh

[eguiraud]

A PR that makes Tera::new error out is up. Locally it produced no test failures.

For the record I still think that doing what 99.9% of users would expect out of the box (i.e. treating ./some/dir/*.html the same as /path/to/some/dir/*.html) would be better, but I guess that's a bigger change in behavior.

[FibreFoX]

Not sure that I am happy with Tera reporting errors with relative paths like in that PR. It does not fix the problem, it just makes more noise.
In my opinion it would be a better fix to use something different than globwalk (and yes, breaking changes are okay IMHO if they introduce fixes).

[Keats]

Can we do a PR with a fix as well?

[eguiraud]

I agree with you @FibreFoX , I just thought there was no consensus on what an actual fix should/would look like, so in the meanwhile I'd rather have Tera report the issue rather than silently having a surprising behavior.

@Keats a PR with what I would propose as an actual fix is at #799 .