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.

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

Project does not allow case-sensitive filenames for RST/MD files #53

Closed
garvinhicking opened this issue Nov 8, 2023 · 4 comments · Fixed by phpDocumentor/guides#677
Closed

Comments

@garvinhicking
Copy link
Contributor

Running the command does not find the entrypoint if a filename is called "Index.rst" instead of "index.rst".

Ideally the entrypoint should accept both variants, at least "Index.rst" should be canonical filename, right?

@garvinhicking garvinhicking changed the title Running make pre-commit-test on macOS does not find entrypoint Project does not allow case-sensitive filenames for RST/MD files Nov 14, 2023
@garvinhicking
Copy link
Contributor Author

I'm losing my mind. I just can't debug how phpdocumentor-cli parses file names. It only works if I only use "index.rst" lowercase filenames, never if it's "Index.rst". But this whole flysytem/filesystem API implementation is one level of abstraction too much for me, I cannot find out where the filenames get wrongly lowercased.

I think vendor/phpdocumentor/guides-cli/src/Command/Run.php at line 195 is a possible culprit:

            $documents = $this->commandBus->handle(
                new ParseDirectoryCommand(
                    $sourceFileSystem,
                    '',
                    $settings->getInputFormat(),
                    $projectNode,
                ),
            );

but I don't know how to follow this allong easily. Just throwing this in here so I find it again.

@jaapio
Copy link
Contributor

jaapio commented Nov 14, 2023

did some trouble shooting together with Garvin, it turns out to be a real OS issue...

<?php

if (file_exists('Documentation/Index.rst')) {
echo "Index.rst exists.\n";
}

if (file_exists('Documentation/index.rst')) {
echo "index.rst exists.\n";
}
$ php test.php
Index.rst exists.
index.rst exists.

we can fix this possibly in guides by handling the index detection in a different way... using listContents on the filesystem... which is not very nice... but the only way to cover this.

@garvinhicking
Copy link
Contributor Author

Your help was very productive :)

Do you want me to make a PR using the listContents method we derived, or are you on it yourself?

@jaapio
Copy link
Contributor

jaapio commented Nov 15, 2023

Feel free to create a pr, if you need any help let me know.

garvinhicking added a commit to garvinhicking/guides that referenced this issue Nov 15, 2023
…macOS

On macOS filesystems, a file-check against "index.rst"
using $filesystem->has() would return TRUE, when in fact
a file might be stored as "Index.rst". Thus, at this point
we fetch the whole directory list and compare the contents
with if the INDEX_FILE_NAMES entry matches. This ensures
that we get the file with exactly the casing that is returned
from the filesystem.

Background:

Given a file called "Index.rst" exists on macOS, executing this script:

```
<?php
echo (file_exists('Index.rst') ? 'Index.rst exists!' : 'Index.rst missing.') . "\n";
echo (file_exists('index.rst') ? 'index.rst exists!' : 'index.rst missing') . "\n";
```

will yield:

```
Index.rst exists!
index.rst exists!
```

but in fact, "index.rst" does not exist. macOS does this by default to
prevent case sensitivity issues on the Application-level. In macOS,
if a "index.rst" exists, another file called "Index.rst" may NOT exist.

Sadly, also PHP's realpath() does not yield the "true" filename of a saved
file, so there is no way to deduce the real casing of a file, unless
the directory contents are read.

I made a performance benchmark to test the implications of this change.

Funnily, it does not affect speed in a negative way - in fact it sped
up the process for me. I created 27.000 directories with one file
in them, one directory with 16.000 files and one with 6.000 files.

Average runtime on 5 executions on a macBook M1, no other tasks running:

* Without patch (baseline): 419.67s user 10.27s system 99% cpu 7:13.64 total
* With path:                419.38s user 11.07s system 98% cpu 7:14.98 total

So the impact seems to be within usual fault tolerance.

Resolves: TYPO3-Documentation/render-guides#53
garvinhicking added a commit to garvinhicking/guides that referenced this issue Nov 16, 2023
…macOS

On macOS filesystems, a file-check against "index.rst"
using $filesystem->has() would return TRUE, when in fact
a file might be stored as "Index.rst". Thus, at this point
we fetch the whole directory list and compare the contents
with if the INDEX_FILE_NAMES entry matches. This ensures
that we get the file with exactly the casing that is returned
from the filesystem.

Background:

Given a file called "Index.rst" exists on macOS, executing this script:

```
<?php
echo (file_exists('Index.rst') ? 'Index.rst exists!' : 'Index.rst missing.') . "\n";
echo (file_exists('index.rst') ? 'index.rst exists!' : 'index.rst missing') . "\n";
```

will yield:

```
Index.rst exists!
index.rst exists!
```

but in fact, "index.rst" does not exist. macOS does this by default to
prevent case sensitivity issues on the Application-level. In macOS,
if a "index.rst" exists, another file called "Index.rst" may NOT exist.

Sadly, also PHP's realpath() does not yield the "true" filename of a saved
file, so there is no way to deduce the real casing of a file, unless
the directory contents are read.

I made a performance benchmark to test the implications of this change.

Funnily, it does not affect speed in a negative way - in fact it sped
up the process for me. I created 27.000 directories with one file
in them, one directory with 16.000 files and one with 6.000 files.

Average runtime on 5 executions on a macBook M1, no other tasks running:

* Without patch (baseline): 419.67s user 10.27s system 99% cpu 7:13.64 total
* With path:                419.38s user 11.07s system 98% cpu 7:14.98 total

So the impact seems to be within usual fault tolerance.

Resolves: TYPO3-Documentation/render-guides#53
linawolf pushed a commit to phpDocumentor/guides that referenced this issue Nov 16, 2023
#677)

* [BUGFIX] Check Document entry root by respecting case sensitivity on macOS

On macOS filesystems, a file-check against "index.rst"
using $filesystem->has() would return TRUE, when in fact
a file might be stored as "Index.rst". Thus, at this point
we fetch the whole directory list and compare the contents
with if the INDEX_FILE_NAMES entry matches. This ensures
that we get the file with exactly the casing that is returned
from the filesystem.

Background:

Given a file called "Index.rst" exists on macOS, executing this script:

```
<?php
echo (file_exists('Index.rst') ? 'Index.rst exists!' : 'Index.rst missing.') . "\n";
echo (file_exists('index.rst') ? 'index.rst exists!' : 'index.rst missing') . "\n";
```

will yield:

```
Index.rst exists!
index.rst exists!
```

but in fact, "index.rst" does not exist. macOS does this by default to
prevent case sensitivity issues on the Application-level. In macOS,
if a "index.rst" exists, another file called "Index.rst" may NOT exist.

Sadly, also PHP's realpath() does not yield the "true" filename of a saved
file, so there is no way to deduce the real casing of a file, unless
the directory contents are read.

I made a performance benchmark to test the implications of this change.

Funnily, it does not affect speed in a negative way - in fact it sped
up the process for me. I created 27.000 directories with one file
in them, one directory with 16.000 files and one with 6.000 files.

Average runtime on 5 executions on a macBook M1, no other tasks running:

* Without patch (baseline): 419.67s user 10.27s system 99% cpu 7:13.64 total
* With path:                419.38s user 11.07s system 98% cpu 7:14.98 total

So the impact seems to be within usual fault tolerance.

Resolves: TYPO3-Documentation/render-guides#53

* [CLEANUP] Fix code style
phpdoc-bot pushed a commit to phpDocumentor/guides-core that referenced this issue Nov 16, 2023
…S (#677)

* [BUGFIX] Check Document entry root by respecting case sensitivity on macOS

On macOS filesystems, a file-check against "index.rst"
using $filesystem->has() would return TRUE, when in fact
a file might be stored as "Index.rst". Thus, at this point
we fetch the whole directory list and compare the contents
with if the INDEX_FILE_NAMES entry matches. This ensures
that we get the file with exactly the casing that is returned
from the filesystem.

Background:

Given a file called "Index.rst" exists on macOS, executing this script:

```
<?php
echo (file_exists('Index.rst') ? 'Index.rst exists!' : 'Index.rst missing.') . "\n";
echo (file_exists('index.rst') ? 'index.rst exists!' : 'index.rst missing') . "\n";
```

will yield:

```
Index.rst exists!
index.rst exists!
```

but in fact, "index.rst" does not exist. macOS does this by default to
prevent case sensitivity issues on the Application-level. In macOS,
if a "index.rst" exists, another file called "Index.rst" may NOT exist.

Sadly, also PHP's realpath() does not yield the "true" filename of a saved
file, so there is no way to deduce the real casing of a file, unless
the directory contents are read.

I made a performance benchmark to test the implications of this change.

Funnily, it does not affect speed in a negative way - in fact it sped
up the process for me. I created 27.000 directories with one file
in them, one directory with 16.000 files and one with 6.000 files.

Average runtime on 5 executions on a macBook M1, no other tasks running:

* Without patch (baseline): 419.67s user 10.27s system 99% cpu 7:13.64 total
* With path:                419.38s user 11.07s system 98% cpu 7:14.98 total

So the impact seems to be within usual fault tolerance.

Resolves: TYPO3-Documentation/render-guides#53

* [CLEANUP] Fix code style
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants