Delete buggy stat(::Integer) method

[LilithHafner]

"Where did someone get a RawFD as an integer anyway?" -@StefanKarpinski

See also #51711

Fixes #51710

[jakobnissen]

This is specifically tested for, hence the tests fail. What did triage agree to yesterday? I can modify #51711 accordingly.
Would a deprecation for a (couple of) releases make sense?

[LilithHafner]

I opened this PR while screen sharing during triage, but it wasn't explicitly a triage decision. I just want to see what nanosoldier has to say about this.

@nanosoldier runtests()

I think the problem with ispath(6) is that stat(6) works, not that ispath is duck-typed.

[nanosoldier]

The package evaluation job you requested has completed - possible new issues were detected.
The full report is available.

[LilithHafner]

Where did someone get a RawFD as an integer anyway?

julia> fd(open("tmp", "w"))
23

[LilithHafner]

The docs don't indicate that stat should accept an integer:

  stat(file)

Return a structure whose fields contain information about the file. The fields of the structure are:

...

[LilithHafner]

Of the 19 packages that failed nano-soldier only on this version, 14 were false positives, 3 were due to stat(::IOStream) failing (fixed in 6b29410) and only 2 require trivial downstream fixes (halleysfifthinc/C3D.jl#19, emmt/InterProcessCommunication.jl#10).

There is perhaps still hope for this in 1.x

[halleysfifthinc]

The docs don't indicate that stat should accept an integer:

Actually, the docs don't indicate at all what stat accepts. I'm happy to accept @LilithHafner 's PR, but that doesn't feel like the complete solution IMO.

• Could we update the fd function to return a RawFD? IIRC, changing return types isn't considered a breaking change. Has that been considered and/or is there a reason not to?
• The stat docstring should be updated (as part of this PR imo), because the lack of clarity around the proper/desired arguments for stat might be why code like mine (i.e. stat(fd(io))) exists. A docstring should be clear enough that users don't need to call methods to understand what argument types it accepts.
  • A quick methods(stat) and blame shows that a stat(::IOStream) = stat(fd(io)) method has existed for much longer than my code, which makes me suspect that the lack of clarity in the stat docstring lead me to the unnecessary indirection through fd instead of a direct stat(io) call.
  • Additional explanation for the fd function and RawFD type docstrings would also be helpful. As it stands currently, they appear to be completely redundant, and its not clear why and when you should use one or the other.

[IanButterworth]

Just came across this.

julia> isdir(false)
false

julia> isfile(false)
false

[LilithHafner]

What did you expect? false is neither a file nor a directory. 😄

[vtjnash]

False is a file for me lol

$ echo "isfile(false)" > isfilefalse.jl
$ <isfilefalse.jl ./julia
true

[fingolfin]

Looks good to me esp. once #55080 is merged?

[LilithHafner]

@nanosoldier runtests()

[mbauman]

Why not a deprecation?

[LilithHafner]

A deprecation would not fix #51710 and I'm optimistic that, with #55080, this is practically non-breaking which makes it plausible to remove outright.

[mbauman]

I suppose I'm just more conservative here — to me this isn't worth an immediate minor change when the deprecation is so straightforward. It's great that you even already tackled the PkgEval failures, but I see that as rescuing two canaries from a particularly deep and obscure branch of our coal mine. I don't know who else is down there, and we might as well sound the alarm for anyone that's listening.

[LilithHafner]

Two of the original PkgEval failures are fixed by #55080. But you are right that there was one that needed to be fixed downstream. We can deprecate.

[LilithHafner]

This is still worth removing eventually because of #51710. I tagged 1.14 to be conservative, but would be open to landing it in 1.13, too.

[nanosoldier]

The package evaluation job you requested has completed - possible new issues were detected.
The full report is available.

[giordano]

Failure in InterProcessCommunication looks real (CC: @emmt), others look noise at a quick glance.

[LilithHafner]

I agree on both counts. That failure is fixed in emmt/InterProcessCommunication.jl#10.

[LilithHafner]

From triage: Keno and Oscar and Lilith are fine with removing this in 1.12 (and postponing the removal if anyone notices)

[LilithHafner]

@nanosoldier runtests()

[nanosoldier]

The package evaluation job you requested has completed - possible new issues were detected.
The full report is available.