Subject: Re: bug#70914: 29.3; Crashes often on Windows

From: Eli Zaretskii <[email-masked]>
Date: Thu, 23 May 2024 19:02:50 +0300

> From: Ihor Radchenko <[email-masked]>
> Cc: [email-masked], [email-masked], [email-masked], [email-masked]
> Date: Thu, 23 May 2024 14:23:21 +0000
> 
> Eli Zaretskii <[email-masked]> writes:
> 
> >> > My point is that if the URI is "file://", the corresponding :path on
> >> > Windows is not "//".
> >> 
> >> Sure. But what is passed to :activate-func is not a file path. It is
> >> "Org mode link path". They are not the same in this specific case, but
> >> there is nothing wrong on Org side.
> >
> > Then on whose side is something wrong in this case?  Which code calls
> > file-exists-p with the string "//" in the case you described?
> 
> https://github.com/doomemacs/doomemacs/blob/master/modules/lang/org/config.el#L500
> 
> >> >> IMHO, the bug is in file-exists-p:
> >> >
> >> > I don't think so.  "//" is an invalid file name on Windows, so the
> >> > only requirement from file-exists-p in that case is not to crash.
> >> > Which it does, after the recent fix.
> >> 
> >> IMHO, "invalid file name on Windows" leads to "trouble determining
> >> whether the file exists".
> >
> > Since the file name "//" is invalid, you get undefined behavior.
> 
> > Examples:
> >
> >   (file-exists-p "//)
> >     => nil
> 
> As I tried to explain, I do not see the behaviour as "undefined" when the
> file name is really invalid. Not for `file-exists-p'. AFAIU,
> `file-attributes' and `file-directory-p' do not give such promises in
> their docstrings.
> 
> > but
> >
> >   (file-attributes "//")
> >     => (t 1 1001 513 (0 0 0 0) (0 0 0 0) (0 0 0 0) 0 "drwxrwxrwx" t 0 0)
> 
> This is somewhat surprising, but also somewhat expected -
> `file-exists-p' does not always mean that file does not exist: "This
> returns nil for a symlink to a nonexistent file." Yet, symlink to a
> nonexistent file can have attributes. So, I can see how
> `file-attributes' can return something strange for non-existing files.
> 
> > and
> >
> >   (file-directory-p "//")
> >     => nil
> 
> This also follows from the docstring of `file-directory-p':
> 
>     Return t if FILENAME names an existing directory.
>     
>     Return nil if FILENAME does not name a directory, or if there
>     was trouble determining whether FILENAME is a directory.
> 
> >> AFAIU, this is what happens after the fix anyway, so there is nothing
> >> more that should be done on Emacs side.
> >
> > Generating such invalid file names is wrong, so it should be prevented
> > if possible.  But I won't argue more if you are still unconvinced.
> 
> Org does not care about "file" part. There is a link. The link has a
> path. User can put anything in that path. Org does not generate file
> names - Org parses Org link syntax and leaves its interpretation up to
> the more specialized code.
> 
> Of course, if you see problems with Org mode code that interprets file
> links specifically and are _also_ designed to handle file links, I will
> be happy to fix edge cases like you described. The fontification code in
> Org mode has nothing to do with interpreting the meaning of the links.

Got it.  I'm closing this bug, since there doesn't seem to be anything
left to do here.