Page requisite behaviour inconsistent between Windows and others

[davidwuehrer]

Describe the bug

The method relfn2path of the class BuildEnvironment in the file sphinx/environment/__init__.py supposedly treats absolute paths as relative to the source dir. This is not the case in Windows. Absolute paths to page requisites provided by conf.py in the source dir remain absolute paths in Windows, but become relative paths in every other OS.

The reason is that os.path.join in Windows applied to two (or more) absolute paths returns the right-most absolute path, throwing away everything to the left of it. This is different in other OSes, in which an absolute path is made relative to the preceding path.

The effect in relfn2path is that absolute paths to page requisites declared by conf.py in the source dir remain those same absolute in Windows, but not anywhere else, causing inconsistent behaviour.

I would suggest the following patch:

@@ -390,8 +390,8 @@ class BuildEnvironment:
         containing document.
         """
         filename = os_path(filename)
-        if filename.startswith('/') or filename.startswith(os.sep):
-            rel_fn = filename[1:]
+        if os.path.isabs(filename):
+            rel_fn = filename[filename.index(os.path.sep)+1:]
         else:
             docdir = path.dirname(self.doc2path(docname or self.docname,

This will make the behaviour consistent.

It will break Windows-only projects that rely on absolute paths being absolute rather than relative to the source dir.

How to Reproduce

from sphinx.environment import BuildEnvironment
env = BuildEnvironment()
env.srcdir = 'srcdir'
env.relfn2path('C:\\project\\srcdir\\img.png', 'document')

Expected behavior

Paths starting with a slash or os.path.sep are understood as relative to the source dir by relfn2path; in most operating systems this is the same as an absolute path, meaning that actually absolute paths are not possible, and probably not desired.

In Windows, actually absolute paths are possible, which causes sphinx to behave differently between operating systems if conf.py declares actually absolute paths, for example using os.path.abspath.

This makes the behaviour of sphinx inconsistent across platforms. It should be consistent. Either absolute paths remaining absolute is an intended feature, then it should be available in operating systems other than Windows as well; or it is not, then absolute Windows paths should be treated the same way as absolute paths everywhere else.

[AA-Turner]

@davidwuehrer thank you for this comprehensive report.

As an alternate patch, what do you think about:

@@ -412,6 +412,7 @@ class BuildEnvironment:
         containing document.
         """
         filename = os_path(filename)
+        _, filename = os.path.splitdrive(filename)
         if filename.startswith(('/', os.sep)):
             rel_fn = filename[1:]
         else:

[12rambau]

I tried both solutions in this PR (#12405) and tested it in my own repository: sphinx-contrib/icon#34

None of them changed anything from my side, all was working in other OSs and wasn't working in Windows.

[picnixz]

None of them changed anything from my side, all was working in other OSs and wasn't working in Windows.

I'm pretty sure it has something to do with the drive that can be changed. But then, I'm wondering how to properly fix it.

[electric-coder]

I'm pretty sure it has something to do with the drive that can be changed. But then, I'm wondering how to properly fix it.

Given that support for Python 3.8 was dropped in Sphinx 7.2.0 it should be possible to remove use of the os.path module altogether in favor using only pathlib (it would simplify the codebase overall to use one less module). However, I'm assuming that was tried and removed in Sphinx 7.2.3 for causing bugs with the note: "Restore support string methods on path objects. This is deprecated and will be removed in Sphinx 8."

[picnixz]

I don't think it has anything to do with the pathlib and the os.path implementation. Those are compatible and are interchangable (os.path works with Path inputs).