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

Fix handling of .. in jepath() (requires <filesystem> header) #170

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

juntuu
Copy link
Collaborator

@juntuu juntuu commented Feb 20, 2021

Resolves #169

Oh, <filesystem> header is not available...

I'll leave this here for now, until the future of possibly supporting compiler is cleared

After this fix ../ path components are correctly backed off.

The backing off is done lexically, as was before in the C version, so traversing through symlink and then back (to the parent of the symlinked directory) with ".." still fails.

@herwinw
Copy link
Contributor

herwinw commented Feb 21, 2021

The Ci uses G++ 7.5, which is kind of ancient. From the top of my head, you need version 8 to get full support for C++-17, which introduced the filesystem header. C++-20 is only supported since version 10 (although I'm not sure if it's fully supported yet).

We could work around this problem with a bunch of regexes and dealing with a lot of corner cases, but I'm pretty sure we're going to run into similar problems again when we try to convert code to C++-20. So I guess we're up for another 5 hour episode of getting a CI to run ;)

@juntuu
Copy link
Collaborator Author

juntuu commented Feb 21, 2021

We could work around this problem with a bunch of regexes and dealing with a lot of corner cases,

I think in this case it's better to just wait, as c++20 will anyways be the eventual target. Also the issue is not too bad to work with.

@juntuu juntuu changed the title Fix handling of .. in jepath() Fix handling of .. in jepath() (requires <filesystem> header) Feb 27, 2021
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.

jepath won't work with relative paths which include ".."
2 participants