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

Absolute paths provided as the source are changed and error out #35

Closed
rdmurphy opened this issue Nov 28, 2018 · 5 comments · Fixed by #41
Closed

Absolute paths provided as the source are changed and error out #35

rdmurphy opened this issue Nov 28, 2018 · 5 comments · Fixed by #41
Labels
bug Something isn't working

Comments

@rdmurphy
Copy link

rdmurphy commented Nov 28, 2018

Hello!

Big fan of the speed gains here, but hit a snag when I was migrating a project away from fast-glob. Due to how tiny-glob defaults to . as the cwd if you don't pass anything in, it causes the path.join call within walk to accidentally break the absolute path that was provided as the src.

In other words, when I pass in something like:

/hello/absolute/path/here/

It gets turned into this by that join due to that . being prepended to the file path:

hello/absolute/path/here/

Which then throws an error, because that path does not exist.

Not sure how major of a change it'd be, but it may be worth considering hitting a provided source with path.isAbsolute and skip the cwd prepend if it returns true?

I didn't check node-glob, but it does appear to be how fast-glob approaches this situation.

(Somewhat related, but fast-glob also uses process.cwd() instead of defaulting to the string dot . for cwd, which may also be a bit more resilient to any other weird interpretations of that pathing.)

Happy to take a swing at this as well if you're interested. Thank you!

@terkelg
Copy link
Owner

terkelg commented Nov 30, 2018

Hi @rdmurphy

Thank you for the kind words and for the profound issue description - appreciate it!
I wonder if this is related to #28. Can you maybe have a look at this? I would love to have someone to help maintain tiny-glob. I can add you to the repo if you're keen?

Thanks,
Terkel

@terkelg terkelg added the bug Something isn't working label Dec 1, 2018
@ForsakenHarmony
Copy link
Contributor

oh this was ignored for months now 🙃
was debugging for a long time to figure out where it comes from and what's causing it

https://github.com/terkelg/tiny-glob/blob/master/index.js#L13

it's this line

> path.join('.', '/wew')
'wew'

@lukeed
Copy link
Collaborator

lukeed commented Feb 22, 2019

Thanks for the bump & investigation @ForsakenHarmony 👏 path.resolve should be there instead, ya?

@rdmurphy
Copy link
Author

I mentioned it up in the first post of the issue, but I think the funkiness is coming from having an actual dot . as the default cwd value. I think having it default to process.cwd() instead + skipping the joining of cwd if it's not an absolute path would clear it up with the current code.

@lukeed
Copy link
Collaborator

lukeed commented Feb 22, 2019

Ah, sorry. I dont think I ever looked at this issue.

process.cwd() === resolve('.') so that's not the problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants