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.

Sign up for GitHub

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

Jump to bottom

joinPath sometimes has extra . in the beginning of the path #23712

Open
alex65536 opened this issue Jun 13, 2024 · 3 comments
Open

joinPath sometimes has extra . in the beginning of the path #23712

alex65536 opened this issue Jun 13, 2024 · 3 comments

Comments

@alex65536
Copy link
Contributor

alex65536 commented Jun 13, 2024 •
edited
Loading

Description

import std/os

echo joinPath(".", "a")
echo joinPath("b", "..", "a")
echo joinPath(".", "..")
echo joinPath("./", "a")
echo joinPath("./", "..")
echo joinPath(".", "a", "b")
echo joinPath("./a", "b")
echo joinPath("x/..", "/a", "/b", "../../..")
echo joinPath("x/../a/b", "../../..")

Nim Version

2.0.4 (also reproducible on the latest devel)

Current Output

./a
./a
./..
./a
./..
./a/b
a/b
./..
..

Expected Output

a
a
..
a
..
a/b
a/b
..
..

Possible Solution

No response

Additional Information

No response
@alex65536
Copy link
Contributor Author

alex65536 commented Jun 13, 2024 •
edited
Loading

While looking at the code, I have found out that there are two issues basically.

In joinPath(".", "a") and similar cases, the path becomes "." after first addNormalizePath and second addNormalizePath doesn't remove it. In test cases this one is called "controversial" because of how paths to executables behave on Unix systems:

Nim/tests/stdlib/tos.nim

Lines 590 to 863 in 0b5a938

# controversial: inconsistent with `joinPath("zook/.","abc")`
# on linux, `./foo` and `foo` are treated a bit differently for executables
# but not `./foo/bar` and `foo/bar`
doAssert joinPath(".", "/lib") == unixToNativePath"./lib"
doAssert joinPath(".", "abc") == unixToNativePath"./abc"

Retaining this behavior might be good to prevent older programs from breaking, but it starts to look much weirder in case of joinPath("b", "..", "a") or joinPath(".", ".."), so probably worth either fixing or documenting.

The second part is less controversial and more looks like a bug. The problem is in the line

Nim/lib/pure/pathnorm.nim

Line 75 in 0b5a938

it.notFirst = (state shr 1) > 0

This is not the correct way to check if it's the first component of the path, as state == 0 holds also for such paths as . or ../... Thus, when handling joinPath("x/..", "/a", "/b", "../../..") it starts to think at some point that the path is absolute, because it receives "/" from PathIter. Though, by coincidence and only because the . is not removed because of the first issue, it seems that such incorrect state doesn't lead to wrong results (or at least I haven't been able to found the failing case).

@juancarlospaco
Copy link
Collaborator

But ./a is not the same as a. 🤔

@alex65536
Copy link
Contributor Author

alex65536 commented Jun 13, 2024 •
edited
Loading

But ./a is not the same as a. 🤔

Well, but

import std/[os, paths]

echo normalizedPath("./a")
echo "./a".Path == "a".Path

prints

a
true

And the documentation promises that joinPath

Nim/lib/std/private/ospaths2.nim

Lines 96 to 98 in 0b5a938

## returns normalized path concatenation of `head` and `tail`, preserving
## whether or not `tail` has a trailing slash (or, if tail if empty, whether
## head has one).

but ./a is obviously not normalized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@juancarlospaco @alex65536