Skip to content

Conversation

IanButterworth
Copy link
Member

@IanButterworth IanButterworth commented Jul 23, 2024

  • Expand longpath error info, otherwise you just get
  SystemError: longpath: The system cannot find the path specified. 
  Stacktrace:
    [1] windowserror(p::Symbol, code::UInt32; extrainfo::Nothing)
      @ Base .\error.jl:191
    [2] windowserror
      @ .\error.jl:190 [inlined]
    [3] longpath(path::String)
      @ Base.Filesystem .\path.jl:479
  • Use abspath in longpath because GetLongPathNameW doesn't seem to handle relative paths

Fixes #54799

cc. @kimikage

@IanButterworth IanButterworth changed the title add path to longpath error for context isfile_casesensitive fixes on Windows Jul 23, 2024
@IanButterworth IanButterworth requested a review from stevengj July 23, 2024 14:32
@IanButterworth IanButterworth added the needs tests Unit tests are required for this change label Jul 23, 2024
@IanButterworth
Copy link
Member Author

More info on the Pkg test case we see this on

C:\Users\runneradmin\AppData\Local\Temp\jl_bMlLt2\environments\v1.12\..\..\..\jl_vAJCF9\B
cyclic dependency graph: Error During Test at D:\a\Pkg.jl\Pkg.jl\test\new.jl:2935
  Got exception outside of a @test
  SystemError: longpath: The system cannot find the path specified. 
  Stacktrace:
    [1] windowserror(p::Symbol, code::UInt32; extrainfo::Nothing)
      @ Base .\error.jl:200
    [2] windowserror
      @ .\error.jl:199 [inlined]
    [3] longpath(path::String)
      @ Base.Filesystem .\path.jl:479
    [4] isfile_casesensitive(path::String)
      @ Base .\loading.jl:15

@IanButterworth
Copy link
Member Author

The issue seems to be that longpath didn't like relative paths

@IanButterworth IanButterworth added backport 1.10 Change should be backported to the 1.10 release backport 1.11 Change should be backported to release-1.11 and removed needs tests Unit tests are required for this change labels Jul 23, 2024
@stevengj
Copy link
Member

Might be worth perusing the discussion of the original implementation of isfile_casesensitive #13542 and a subsequent patch on Windows #19291.

@stevengj
Copy link
Member

LGTM. The Win32 docs on GetLongPathNameW aren't clear on this point to me, but other discussions online seem to agree that its argument should be an absolute path.

@IanButterworth

This comment was marked as resolved.

stevengj
stevengj previously approved these changes Jul 23, 2024
@IanButterworth
Copy link
Member Author

Maybe other platforms need abspath too? This is on MacOS

julia> touch("newfile")
"newfile"

julia> Base.isfile_casesensitive("newfile")
true

julia> Base.isfile_casesensitive(joinpath("dummy", "..", "newfile"))
false

@stevengj
Copy link
Member

Looks like on MacOS the culprit may be isaccessiblefile, which is called by isfile_casesensitive before doing anything else:

julia> touch("newfile")
"newfile"

julia> Base.isaccessiblefile("newfile")
true

julia> Base.isaccessiblefile("dummy/../newfile")
false

@IanButterworth
Copy link
Member Author

IanButterworth commented Jul 23, 2024

If my relative path is valid, in that it steps in and out of a real dir, then

julia> Base.isfile_casesensitive("src/../newfile")
true

julia> Base.isfile_casesensitive("dummy/../newfile")
false

@stevengj
Copy link
Member

stevengj commented Jul 23, 2024

Looks like both isaccessiblefile and ccall(:getattrlist, ...) (both are used inside isfile_casesensitive) don't like the relative path with the dummy component.

Given that isfile also fails for a dummy component:

julia> isfile("dummy/../newfile")
false

it's not obvious to me that this behavior is wrong?

@stevengj
Copy link
Member

stevengj commented Jul 23, 2024

On Windows, does the old isfile_casesensitive succeed for relative paths involving existing directories, or is abspath needed there too?

i.e. is this PR only for dummy path components?

@IanButterworth
Copy link
Member Author

Yeah, added tests here and testing master in #55228

@KristofferC KristofferC mentioned this pull request Jul 24, 2024
46 tasks
@KristofferC KristofferC mentioned this pull request Aug 2, 2024
68 tasks
KristofferC added a commit that referenced this pull request Sep 17, 2024
Backported PRs:
- [x] #55480 <!-- Fix push! for OffsetVectors, add tests for push! and
append! on AbstractVector -->
- [x] #55443 <!-- Add test for upper/lower/titlecase and fix call -->
- [x] #55524 <!-- Set `.jl` sources as read-only during installation -->
- [x] #55500 <!-- make jl_thread_suspend_and_get_state safe -->
- [x] #55506 <!-- Fix indexing in _mapreducedim for OffsetArrays -->
- [x] #55564 <!-- Empty out loaded_precompiles dict instead of asserting
it's empty. -->
- [x] #55567 <!-- Initialize threadpools correctly during sysimg build
-->
- [x] #55596 <!-- Fast bounds-check for CartesianIndex ranges -->
- [x] #55605 <!-- Reroute Symmetric/Hermitian + Diagonal through
triangular -->
- [x] #55640 <!-- win: move stack_overflow_warning to the backtrace
fiber -->
- [x] #55715 <!-- Add precompile signatures to Markdown to reduce
latency. -->
- [x] #55593 <!-- Fix invalidations for FileIO -->
- [x] #55555 <!-- Revert "Don't expose guard pages to malloc_stack API
consumers" -->
- [x] #55720 <!-- Fix `pkgdir` for extensions -->
- [x] #55729 <!-- Avoid confounding compilation side effects of
`@time_imports` -->
- [x] #55718 <!-- Fix `@time_imports` extension recognition -->
- [x] #55522 <!-- Fix tr for Symmetric/Hermitian block matrices -->

Contains multiple commits, manual intervention needed:
- [ ] #55509 <!-- Fix cong implementation to be properly random and not
just cycling. -->

Non-merged PRs with backport label:
- [ ] #55641 <!-- fall back to slower stat filesize if optimized
filesize fails -->
- [ ] #55534 <!-- Set stdlib sources as read-only during installation
-->
- [ ] #55499 <!-- propagate the terminal's `displaysize` to the
`IOContext` used by the REPL -->
- [ ] #55458 <!-- Allow for generically extracting unannotated string
-->
- [ ] #55457 <!-- Make AnnotateChar equality consider annotations -->
- [ ] #55453 <!-- Privatise the annotations API, for StyledStrings -->
- [ ] #55355 <!-- relocation: account for trailing path separator in
depot paths -->
- [ ] #55220 <!-- `isfile_casesensitive` fixes on Windows -->
- [ ] #55169 <!-- `propertynames` for SVD respects private argument -->
- [ ] #54457 <!-- Make `String(::Memory)` copy -->
- [ ] #53957 <!-- tweak how filtering is done for what packages should
be precompiled -->
- [ ] #51479 <!-- prevent code loading from lookin in the versioned
environment when building Julia -->
- [ ] #50813 <!-- More doctests for Sockets and capitalization fix -->
- [ ] #50157 <!-- improve docs for `@inbounds` and
`Base.@propagate_inbounds` -->
- [ ] #41244 <!-- Fix shell `cd` error when working dir has been deleted
-->
@KristofferC KristofferC mentioned this pull request Sep 24, 2024
30 tasks
KristofferC added a commit that referenced this pull request Sep 25, 2024
Backported PRs:
- [x] #55773 <!-- Add compat entry for `Base.donotdelete` -->
- [x] #41244 <!-- Fix shell `cd` error when working dir has been deleted
-->
- [x] #55795 <!-- fix #52986, regression in `@doc` of macro without REPL
loaded -->
- [x] #55829 <!-- [Dates] Make test more robust against non-UTC
timezones -->
- [x] #55641 <!-- fall back to slower stat filesize if optimized
filesize fails -->
- [x] #55744 <!-- fix #45494, error in ssa conversion with complex type
decl -->
- [x] #55783 <!-- use `inferencebarrier` instead of `invokelatest` for
1-arg `@assert` -->
- [x] #55739 <!-- Add `invokelatest` barrier to `string(...)` in
`@assert` -->

Need manual backport:
- [ ] #55798 <!-- Broadcast binary ops involving strided triangular -->

Contains multiple commits, manual intervention needed:
- [ ] #55509 <!-- Fix cong implementation to be properly random and not
just cycling. -->
- [ ] #55569 <!-- Add a docs section about loading/precomp/ttfx time
tuning -->
- [ ] #55824 <!-- Replace regex package module checks with actual code
checks -->

Non-merged PRs with backport label:
- [ ] #55845 <!-- privatize annotated string API, take two -->
- [ ] #55828 <!-- Fix some corner cases of `isapprox` with unsigned
integers -->
- [ ] #55813 <!-- Check for conflicting `@ccallable` name before JIT
registration -->
- [ ] #55743 <!-- doc: heap snapshot viewing -->
- [ ] #55741 <!-- Change annotations to use a NamedTuple -->
- [ ] #55534 <!-- Set stdlib sources as read-only during installation
-->
- [ ] #55499 <!-- propagate the terminal's `displaysize` to the
`IOContext` used by the REPL -->
- [ ] #55458 <!-- Allow for generically extracting unannotated string
-->
- [ ] #55457 <!-- Make AnnotateChar equality consider annotations -->
- [ ] #55453 <!-- Privatise the annotations API, for StyledStrings -->
- [ ] #55355 <!-- relocation: account for trailing path separator in
depot paths -->
- [ ] #55220 <!-- `isfile_casesensitive` fixes on Windows -->
- [ ] #55169 <!-- `propertynames` for SVD respects private argument -->
- [ ] #54457 <!-- Make `String(::Memory)` copy -->
- [ ] #53957 <!-- tweak how filtering is done for what packages should
be precompiled -->
- [ ] #51479 <!-- prevent code loading from lookin in the versioned
environment when building Julia -->
- [ ] #50813 <!-- More doctests for Sockets and capitalization fix -->
- [ ] #50157 <!-- improve docs for `@inbounds` and
`Base.@propagate_inbounds` -->
@KristofferC KristofferC mentioned this pull request Sep 30, 2024
39 tasks
KristofferC added a commit that referenced this pull request Oct 1, 2024
Backported PRs:
- [x] #55849 <!-- Mmap: fix grow! for non file IOs -->
- [x] #55863 <!-- Update TaskLocalRNG docstring according to #49110 -->
- [x] #54433 <!-- Root globals in toplevel exprs -->
- [x] #55828 <!-- Fix some corner cases of `isapprox` with unsigned
integers -->
- [x] #55890 <!-- Profile: fix order of fields in heapsnapshot & improve
formatting -->
- [x] #55884 <!-- inference: add missing `TypeVar` handling for
`instanceof_tfunc` -->
- [x] #55881 <!-- Install terminfo data under /usr/share/julia -->
- [x] #55909 <!-- do not intentionally suppress errors in precompile
script from being reported or failing the result -->
- [x] #55355 <!-- relocation: account for trailing path separator in
depot paths -->
- [x] #55917 <!-- fix rawbigints OOB issues -->
- [x] #55892 <!-- TOML: Avoid type-pirating `Base.TOML.Parser` -->
- [x] #55798 <!-- Broadcast binary ops involving strided triangular -->
- [x] #55919 <!-- Limit `@inbounds` to indexing in the dual-iterator
branch in `copyto_unaliased!` -->

Contains multiple commits, manual intervention needed:
- [ ] #54009 <!-- allow extensions to trigger from packages in [deps]
-->
- [ ] #55509 <!-- Fix cong implementation to be properly random and not
just cycling. -->
- [ ] #55569 <!-- Add a docs section about loading/precomp/ttfx time
tuning -->
- [ ] #55824 <!-- Replace regex package module checks with actual code
checks -->

Non-merged PRs with backport label:
- [ ] #55932 <!-- REPL: make UndefVarError aware of imported modules -->
- [ ] #55910 <!-- Prevent extensions from blocking parallel
pre-compilation -->
- [ ] #55908 <!-- add logic to prefer loading modules that are already
loaded -->
- [ ] #55886 <!-- irrationals: restrict assume effects annotations to
known types -->
- [ ] #55871 <!-- lowering: don't reverse handler order in
`(pop-handler-list ...)` -->
- [ ] #55870 <!-- fix infinite recursion in `promote_type` for
`Irrational` -->
- [ ] #55867 <!-- update `hash` doc string: `widen` not required any
more -->
- [ ] #55851 <!-- [REPL] Fix #55850 by using `safe_realpath` instead of
`abspath` in `projname` -->
- [ ] #55813 <!-- Check for conflicting `@ccallable` name before JIT
registration -->
- [ ] #55743 <!-- doc: heap snapshot viewing -->
- [ ] #55741 <!-- Change annotations to use a NamedTuple -->
- [ ] #55534 <!-- Set stdlib sources as read-only during installation
-->
- [ ] #55499 <!-- propagate the terminal's `displaysize` to the
`IOContext` used by the REPL -->
- [ ] #55458 <!-- Allow for generically extracting unannotated string
-->
- [ ] #55457 <!-- Make AnnotateChar equality consider annotations -->
- [ ] #55220 <!-- `isfile_casesensitive` fixes on Windows -->
- [ ] #55169 <!-- `propertynames` for SVD respects private argument -->
- [ ] #54457 <!-- Make `String(::Memory)` copy -->
- [ ] #53957 <!-- tweak how filtering is done for what packages should
be precompiled -->
- [ ] #51479 <!-- prevent code loading from lookin in the versioned
environment when building Julia -->
- [ ] #50813 <!-- More doctests for Sockets and capitalization fix -->
- [ ] #50157 <!-- improve docs for `@inbounds` and
`Base.@propagate_inbounds` -->
@KristofferC KristofferC mentioned this pull request Oct 7, 2024
44 tasks
@KristofferC KristofferC mentioned this pull request Oct 18, 2024
43 tasks
@KristofferC KristofferC mentioned this pull request Oct 29, 2024
47 tasks
@KristofferC KristofferC mentioned this pull request Dec 3, 2024
51 tasks
This was referenced Jan 28, 2025
This was referenced Mar 11, 2025
@KristofferC KristofferC mentioned this pull request Apr 25, 2025
71 tasks
@KristofferC KristofferC mentioned this pull request Aug 19, 2025
65 tasks
DilumAluthge added a commit that referenced this pull request Sep 5, 2025
Backported PRs:
- [x] #54840 <!-- Add boundscheck in speccache_eq to avoid OOB access
due to data race -->
- [x] #42080 <!-- recommend explicit `using Foo: Foo, ...` in package
code (was: "using considered harmful") -->
- [x] #58127 <!-- [DOC] Update installation docs: /downloads/ =>
/install/ -->
- [x] #58202 <!-- [release-1.11] malloc: use jl_get_current_task to fix
null check -->
- [x] #58584 <!-- Make `Ptr` values static-show w/ type-information -->
- [x] #58637 <!-- Make late gc lower handle insertelement of alloca use.
-->
- [x] #58837 <!-- fix null comparisons for non-standard address spaces
-->
- [x] #57826 <!-- Add a `similar` method for `Type{<:CodeUnits}` -->
- [x] #58293 <!-- fix trailing indices stackoverflow in reinterpreted
array -->
- [x] #58887 <!-- Pkg: Allow configuring can_fancyprint(io::IO) using
IOContext -->
- [x] #58937 <!-- Fix nthreadpools size in JLOptions -->
- [x] #58978 <!-- Fix precompilepkgs warn loaded setting -->
- [x] #58998 <!-- Bugfix: Use Base.aligned_sizeof instead of sizeof in
Mmap.mmap -->
- [x] #59120 <!-- Fix memory order typo in "src/julia_atomics.h" -->
- [x] #59170 <!-- Clarify and enhance confusing precompile test -->

Need manual backport:
- [ ] #56329 <!-- loading: clean up more concurrency issues -->
- [ ] #56956 <!-- Add "mea culpa" to foreign module assignment error.
-->
- [ ] #57035 <!-- linux: workaround to avoid deadlock inside
dl_iterate_phdr in glibc -->
- [ ] #57089 <!-- Block thread from receiving profile signal with
stackwalk lock -->
- [ ] #57249 <!-- restore non-freebsd-unix fix for profiling -->
- [ ] #58011 <!-- Remove try-finally scope from `@time_imports`
`@trace_compile` `@trace_dispatch` -->
- [ ] #58062 <!-- remove unnecessary edge from `exp_impl` to `pow` -->
- [ ] #58157 <!-- add showing a string to REPL precompile workload -->
- [ ] #58209 <!-- Specialize `one` for the `SizedArray` test helper -->
- [ ] #58108 <!-- Base.get_extension & Dates.format made public -->
- [ ] #58356 <!-- codegen: remove readonly from abstract type calling
convention -->
- [ ] #58415 <!-- [REPL] more reliable extension loading -->
- [ ] #58510 <!-- Don't filter `Core` methods from newly-inferred list
-->
- [ ] #58110 <!-- relax dispatch for the `IteratorSize` method for
`Generator` -->
- [ ] #58965 <!-- Fix `hygienic-scope`s in inner macro expansions -->
- [ ] #58971 <!-- Fix alignment of failed precompile jobs on CI -->
- [ ] #59066 <!-- build: Also pass -fno-strict-aliasing for C++ -->

Contains multiple commits, manual intervention needed:
- [ ] #55877 <!-- fix FileWatching designs and add workaround for a stat
bug on Apple -->
- [ ] #56755 <!-- docs: fix scope type of a `struct` to hard -->
- [ ] #57809 <!-- Fix fptrunc Float64 -> Float16 rounding through
Float32 -->
- [ ] #57398 <!-- Make remaining float intrinsics require float
arguments -->
- [ ] #56351 <!-- Fix `--project=@script` when outside script directory
-->
- [ ] #57129 <!-- clarify that time_ns is monotonic -->
- [ ] #58134 <!-- Note annotated string API is experimental in Julia
1.11 in HISTORY.md -->
- [ ] #58401 <!-- check that hashing of types does not foreigncall
(`jl_type_hash` is concrete evaluated) -->
- [ ] #58435 <!-- Fix layout flags for types that have oddly sized
primitive type fields -->
- [ ] #58483 <!-- Fix tbaa usage when storing into heap allocated
immutable structs -->
- [ ] #58512 <!-- Make more types jl_static_show readably -->
- [ ] #58012 <!-- Re-enable tab completion of kwargs for large method
tables -->
- [ ] #58683 <!-- Add 0 predecessor to entry basic block and handle it
in inlining -->
- [ ] #59112 <!-- Add builtin function name to add methods error -->

Non-merged PRs with backport label:
- [ ] #59329 <!-- aotcompile: destroy LLVM context after serializing
combined module -->
- [ ] #58848 <!-- Set array size only when safe to do so -->
- [ ] #58535 <!-- gf.c: include const-return methods in
`--trace-compile` -->
- [ ] #58038 <!-- strings/cstring: `transcode`: prevent Windows sysimage
invalidation -->
- [ ] #57604 <!-- `@nospecialize` for `string_index_err` -->
- [ ] #57366 <!-- Use ptrdiff_t sized offsets for gvars_offsets to allow
large sysimages -->
- [ ] #56890 <!-- Enable getting non-boxed LLVM type from Julia Type -->
- [ ] #56823 <!-- Make version of opaque closure constructor in world
-->
- [ ] #55958 <!-- also redirect JL_STDERR etc. when redirecting to
devnull -->
- [ ] #55956 <!-- Make threadcall gc safe -->
- [ ] #55534 <!-- Set stdlib sources as read-only during installation
-->
- [ ] #55499 <!-- propagate the terminal's `displaysize` to the
`IOContext` used by the REPL -->
- [ ] #55458 <!-- Allow for generically extracting unannotated string
-->
- [ ] #55457 <!-- Make AnnotateChar equality consider annotations -->
- [ ] #55220 <!-- `isfile_casesensitive` fixes on Windows -->
- [ ] #53957 <!-- tweak how filtering is done for what packages should
be precompiled -->
- [ ] #51479 <!-- prevent code loading from lookin in the versioned
environment when building Julia -->
- [ ] #50813 <!-- More doctests for Sockets and capitalization fix -->
- [ ] #50157 <!-- improve docs for `@inbounds` and
`Base.@propagate_inbounds` -->

---------

Co-authored-by: Kiran Pamnany <[email protected]>
Co-authored-by: adienes <[email protected]>
Co-authored-by: Gabriel Baraldi <[email protected]>
Co-authored-by: Keno Fischer <[email protected]>
Co-authored-by: Simeon David Schaub <[email protected]>
Co-authored-by: Jameson Nash <[email protected]>
Co-authored-by: Alex Arslan <[email protected]>
Co-authored-by: Fons van der Plas <[email protected]>
Co-authored-by: Ian Butterworth <[email protected]>
Co-authored-by: JonasIsensee <[email protected]>
Co-authored-by: Curtis Vogt <[email protected]>
Co-authored-by: Dilum Aluthge <[email protected]>
Co-authored-by: DilumAluthgeBot <[email protected]>
Co-authored-by: DilumAluthge <[email protected]>
@PallHaraldsson
Copy link
Contributor

Was this forgotten? I think NOT backported despite on it and approved long ago.

@stevengj
Copy link
Member

stevengj commented Sep 9, 2025

cc @KristofferC

@KristofferC
Copy link
Member

I think NOT backported despite on it and approved long ago.

?? It is not merged.

@PallHaraldsson
Copy link
Contributor

PallHaraldsson commented Sep 9, 2025

Yes. I'm simply asking if forgotten, and if actually working (seemingly CI is a false alarm EDIT is ISN'T failing on WIndows; rather on Unix...). Not criticizing you or anyone, saying anyone dropped the ball, maybe @IanButterworth abandoned PR or Julia. It seems important to merge all Windows fixes. Then backport... just regrettable didn't land in rc2.

@DilumAluthge
Copy link
Member

CI failures seem related to the contents of the PR?

@KristofferC
Copy link
Member

I'm simply asking if forgotten,

No, you said

I think NOT backported despite on it and approved long ago.

Even if it was approved long ago it obviously isn't backported because it isn't merged?

@stevengj
Copy link
Member

stevengj commented Sep 9, 2025

As discussed above, the basic question with this PR was what to do with relative paths that contain nonexistent components that "cancel out", like nonexistent/... isfile fails for these, so presumably isfile_casesensitive should fail as well, but with this PR it succeeds on Windows because abspath eliminates all relative paths.

@IanButterworth IanButterworth dismissed stevengj’s stale review September 9, 2025 16:44

Questions raised after approval

@IanButterworth
Copy link
Member Author

I don't recall my personal motivations to work on this, so it just slipped. (others please feel free to take this over/replace it) I've also invalidated the approval to be clearer, given questions were raised after the approval.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 1.10 Change should be backported to the 1.10 release backport 1.11 Change should be backported to release-1.11

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Problem with isfile_casesensitive() on Windows

5 participants