On api.drupal.org, the branches are set up with "files to parse" like this:
../src/drupal-6
rather than an absolute path like
/var/www/api.staging.drupal.org/src/drupal-6
or
/var/www/api.dev.drupal.org/src/drupal-6

This is apparently not working for parsing. The parser seems to have decided that there aren't any files to parse, or at least not very many.

CommentFileSizeAuthor
#5 api_dotdot.test.txt3.87 KBjhodgdon
#5 dotdotfails.png58.11 KBjhodgdon
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhodgdon’s picture

And as a note, the Drupal root for the site would be in
/var/www/api.staging.drupal.org/htdocs
(or something like that)

jhodgdon’s picture

I did two things to test this on my test site:
a) Set up a new branch with a ../whatever directory path.
b) Edited an existing branch and changed it from an absolute path to a ../whatever path.

For (a) I could see that the parsing jobs were set up correctly in the Drupal Queue. They were pointing to path
../apitoindex/sample/sample.php
which is correct. And everything got parsed correctly, and is displaying correctly on the site.
[EDIT: Actually only the top-level directory was parsed and displayed, noticed later]

For (b) there were a bunch of "Removing ..." entries in the log, as all the documentation from the branch was removed during the scan directory step. And then only 3 entries were added to the queue, to parse the 3 files in the top-level directory. Sub-directories were not scanned.

This is the same result we're seeing on the API staging site right now. So I've reproduced it. There are really two issues:

1) When using .. for a path, only the top-level directory is scanned. [This is the really serious one]

2) When changing from an absolute to a relative path, the existing documentation is dropped. [This would not be so serious because after some cron runs the documentation would be re-created, but it's still a problem. It should have figured out that *relative to the base directory* the files were still the same, but it didn't.]

So... It should be possible to make tests for both (1) and (2), and then fix this problem.

jhodgdon’s picture

Title: Looks like directories starting with .. don't work for parsing » Directories starting with .. don't work for parsing

Definitely confirmed now.

jhodgdon’s picture

Title: Directories starting with .. don't work for parsing » Directories starting with .. -- only the top-level directory is scanned
Assigned: Unassigned » jhodgdon

RE #2 - Oh actually, problem (2) is a symptom of problem (1). All those files were dropped because the scanning stopped after the top-level directory, and the parser thought there were only 3 files in the project instead of all the ones there used to be, so it dropped all the old documentation.

Anyway, I'll take this one on, sometime in the next couple of days.

jhodgdon’s picture

FileSize
58.11 KB
3.87 KB

OK, here's a test for this. I verified that if I set

$prefix = ''; 

just before creating the branch (i.e., not using .. in the path), the tests pass. But with the test as it is in this file, only the top-level files are parsed, and several of the test assertions fail (see screenshot).

So that's good. Now all that remains is to fix the problem until the tests pass. :)

jhodgdon’s picture

(note typo on line 74 of the test -- should be assertTrue rather than assertEqual)

jhodgdon’s picture

OK, here's the problem. In api_scan_directories():

        // See if we want to scan this path, and if so, add to To Do list.
        // Don't keep any directory that starts with ".", or anything in the
        // excluded list.
        if ((strpos($file->filename, '.') !== 0) &&
          !in_array($path, $excluded_array)) {
          $todo[] = $path;
        }

The problem is that $file->filename here is coming from the Drupal function file_scan_directories and it is set up like this:

         $filename = "$dir/$file";
          $basename = basename($file);
          $name = substr($basename, 0, strrpos($basename, '.'));
          $files[$$key] = new stdClass();
          $files[$$key]->filename = $filename;
          $files[$$key]->basename = $basename;
          $files[$$key]->name = $name;

Which is to say that $file->filename is $dir/$file, contains the base directory, and that starts with "..". So we're excluding all the subdirectories no matter what if our directory path starts with "..".

So we need to strip off the $dir when we check if we want to exclude it. I think I should also add a new .hidden directory in the sub-directory so we can test that this is also excluded, just in case.

jhodgdon’s picture

Status: Active » Fixed

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.