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

Fix #38491: fix an abspath() edge case on Windows #38981

Merged
merged 4 commits into from
Mar 25, 2021
Merged

Fix #38491: fix an abspath() edge case on Windows #38981

merged 4 commits into from
Mar 25, 2021

Conversation

belamenso
Copy link
Contributor

@belamenso belamenso commented Dec 23, 2020 •
edited
Loading

Fixes #38491.

@musm
Copy link
Contributor

musm commented Dec 24, 2020

LGTM thanks!

@musm musm added backport 1.6 Change should be backported to release-1.6 system:windows Affects only Windows labels Dec 24, 2020
@musm
Copy link
Contributor

musm commented Dec 24, 2020

I'll give it a day or two to let others also look and comment if they have something to say. Otherwise, ping me if I forget to merge.

vtjnash
vtjnash approved these changes Dec 24, 2020
View reviewed changes
Copy link
Sponsor Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I tested this and added some thoughts. PTAL and let me know what you think.

base/path.jl Outdated Show resolved Hide resolved
test/path.jl
@@ -9,6 +9,23 @@
@test isabspath(S(homedir()))
@test !isabspath(S("foo"))
end
if Sys.iswindows()
@testset "issue #38491" begin
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@testset "issue #38491" begin
@testset "issue #38491" begin withenv("=X:" => nothing, "=Y:" => nothing) do

test/path.jl
@test lowercase(abspath("$(pwd_drive)a")) == lowercase(joinpath(pwd(), "a"))
@test lowercase(abspath(lowercase(pwd_drive))) == lowercase("$(pwd())\\")
@test lowercase(abspath(uppercase(pwd_drive))) == lowercase("$(pwd())\\")
end
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
end
end; end

test/path.jl Outdated Show resolved Hide resolved
@KristofferC KristofferC mentioned this pull request Dec 27, 2020
Merged
26 tasks
@KristofferC KristofferC mentioned this pull request Jan 19, 2021
Merged
60 tasks
@KristofferC KristofferC mentioned this pull request Feb 1, 2021
Merged
10 tasks
@KristofferC KristofferC mentioned this pull request Feb 11, 2021
Merged
52 tasks
@KristofferC KristofferC mentioned this pull request Mar 14, 2021
Merged
14 tasks
@KristofferC KristofferC mentioned this pull request Mar 23, 2021
Merged
10 tasks
@musm @vtjnash
Update base/path.jl
d580960 
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
musm
musm reviewed Mar 24, 2021
View reviewed changes
base/path.jl Outdated Show resolved Hide resolved
musm and others added 2 commits March 24, 2021 14:09
@musm
Update base/path.jl
2bca0e9
@musm @vtjnash
Update test/path.jl
b37e290 
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
@musm musm added the merge me PR is reviewed. Merge when all tests are passing label Mar 24, 2021
@musm
Copy link
Contributor

musm commented Mar 25, 2021

unrelated test failures on linux

@musm musm merged commit f023677 into JuliaLang:master Mar 25, 2021
KristofferC pushed a commit that referenced this pull request Mar 26, 2021
@belamenso @KristofferC
Fix #38491: fix an abspath() edge case on Windows (#38981)
94bf33e 
* Fix #38491: fix an abspath() edge case on Windows

* Update base/path.jl

Co-authored-by: Jameson Nash <vtjnash@gmail.com>

* Update base/path.jl

* Update test/path.jl

Co-authored-by: Jameson Nash <vtjnash@gmail.com>

Co-authored-by: Mustafa M <mus-m@outlook.com>
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
(cherry picked from commit f023677)
@KristofferC KristofferC mentioned this pull request Mar 26, 2021
Merged
33 tasks
@simeonschaub simeonschaub removed the merge me PR is reviewed. Merge when all tests are passing label Apr 3, 2021
KristofferC pushed a commit that referenced this pull request Apr 4, 2021
@belamenso @KristofferC
Fix #38491: fix an abspath() edge case on Windows (#38981)
f6787a9 
* Fix #38491: fix an abspath() edge case on Windows

* Update base/path.jl

Co-authored-by: Jameson Nash <vtjnash@gmail.com>

* Update base/path.jl

* Update test/path.jl

Co-authored-by: Jameson Nash <vtjnash@gmail.com>

Co-authored-by: Mustafa M <mus-m@outlook.com>
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
(cherry picked from commit f023677)
KristofferC pushed a commit that referenced this pull request Apr 4, 2021
@belamenso @KristofferC
Fix #38491: fix an abspath() edge case on Windows (#38981)
63ab04c 
* Fix #38491: fix an abspath() edge case on Windows

* Update base/path.jl

Co-authored-by: Jameson Nash <vtjnash@gmail.com>

* Update base/path.jl

* Update test/path.jl

Co-authored-by: Jameson Nash <vtjnash@gmail.com>

Co-authored-by: Mustafa M <mus-m@outlook.com>
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
(cherry picked from commit f023677)
@KristofferC KristofferC removed the backport 1.6 Change should be backported to release-1.6 label May 4, 2021
ElOceanografo pushed a commit to ElOceanografo/julia that referenced this pull request May 4, 2021
@belamenso @musm
@vtjnash @ElOceanografo
Fix JuliaLang#38491: fix an abspath() edge case on Windows (JuliaLang…
3f425db 
…#38981)

* Fix JuliaLang#38491: fix an abspath() edge case on Windows

* Update base/path.jl

Co-authored-by: Jameson Nash <vtjnash@gmail.com>

* Update base/path.jl

* Update test/path.jl

Co-authored-by: Jameson Nash <vtjnash@gmail.com>

Co-authored-by: Mustafa M <mus-m@outlook.com>
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
antoine-levitt pushed a commit to antoine-levitt/julia that referenced this pull request May 9, 2021
@belamenso @musm
@vtjnash @antoine-levitt
Fix JuliaLang#38491: fix an abspath() edge case on Windows (JuliaLang…
741e703 
…#38981)

* Fix JuliaLang#38491: fix an abspath() edge case on Windows

* Update base/path.jl

Co-authored-by: Jameson Nash <vtjnash@gmail.com>

* Update base/path.jl

* Update test/path.jl

Co-authored-by: Jameson Nash <vtjnash@gmail.com>

Co-authored-by: Mustafa M <mus-m@outlook.com>
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
johanmon pushed a commit to johanmon/julia that referenced this pull request Jul 5, 2021
@belamenso @musm
@vtjnash @johanmon
Fix JuliaLang#38491: fix an abspath() edge case on Windows (JuliaLang…
5a6892e 
…#38981)

* Fix JuliaLang#38491: fix an abspath() edge case on Windows

* Update base/path.jl

Co-authored-by: Jameson Nash <vtjnash@gmail.com>

* Update base/path.jl

* Update test/path.jl

Co-authored-by: Jameson Nash <vtjnash@gmail.com>

Co-authored-by: Mustafa M <mus-m@outlook.com>
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
staticfloat pushed a commit that referenced this pull request Dec 23, 2022
@belamenso @staticfloat
Fix #38491: fix an abspath() edge case on Windows (#38981)
fc2a7b4 
* Fix #38491: fix an abspath() edge case on Windows

* Update base/path.jl

Co-authored-by: Jameson Nash <vtjnash@gmail.com>

* Update base/path.jl

* Update test/path.jl

Co-authored-by: Jameson Nash <vtjnash@gmail.com>

Co-authored-by: Mustafa M <mus-m@outlook.com>
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
(cherry picked from commit f023677)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vtjnash vtjnash vtjnash approved these changes

@musm musm musm approved these changes

Assignees
No one assigned
Labels
system:windows Affects only Windows
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

abspath bug for relative directories
5 participants
@belamenso @musm @vtjnash @KristofferC @simeonschaub