API update to 6.x

I'm having trouble instituting the menu system. I'm getting hook_menu(), hook_init() and hook_boot() mixed up.

Fixed drupal_uninstall_schema() in api.install

Fixed no menu display problem. (Void comment 1) in api.module.

Updated to new FormAPI in api.module.

Added $radios = array(); to api_page_admin_form() in api.module to fix undeclared var.

I think this is now ready for review. Awaiting testing.

Marking code needs review.

The info patch fails for some reason, but that doesn't really matter.

Other patches apply. Testing functionality.

API settings menu item does not appear for user 1 or for users with 'administer API reference' permission.

Hmm interesting. The menu setting shows up fine on the original admin accout when tested against HEAD.
I haven't tried other users yet. I plan to do so.

Hm. I'm running CVS HEAD (about three days old) as user 1. Nothing in the menu_* tables, either. Will check again after updating Drupal CVS.

FWIW: When I move this menu item from hook_init() to hook_menu(), it works.

But I haven't studied the new D6 menu system enough to know why.

Hmm Ok. Thanks. As noted earlier...I'm confused about hook_menu(), hook_init(), and hook_boot(). Specifically how to change what the original code was before to initiate correctly in the new menu system. Any enlightenment appreciated. :)

I'm having the same issue with all the menu items, actually, but don't have time to check through the code.

Take a look at http://drupal.org/node/103114 for documentation.

I removed any instance of hook_init() and moved all menu options to hook_menu().

Then in the admin settings Go ahead and try to index some documentation (drupal Docs: http://cvs.drupal.org/viewvc.py/drupal/contributions/docs/developer/).

Then you might receive this error:

"notice: Undefined variable: first_key in C:\wamp\www\drupal\modules\api\api.module on line 989."

Could anyone help me debug this?

Those are -- and my term may be off here -- E_ALL strict errors.

They simply mean that you must define a variable (in this case $first_key) before you evaluate it.

Well the line where I'm getting the error is:

And $first_key is not pre-defined. I'm guessing I should change this. Just declare it without a value?

Fixed it (comment 15) with this code.

Bumping back to review. Great job on this, linuxlover!! :D

These types of errors are going to be found throughout the code because of the new stricter error reporting.

Bump. :( Student is waiting on us for a review since Dec. 4. :(

Oh, I wasn't informed that this was a GHOP task. Thanks!

I was planning on updating the module myself this weekend, but I will review instead.

My review would be: GHOP done and done well. Let the maintainer(s) fix E_ALL errors.

@agentrickard: Does that mean you tested this and it works properly?

>.< The due date is today (12/10/07). Please help review so this task on GHOP can be finished.

@webchick: Apologies. I still have to test the new menu fixes from #13. I just think E_ALL is minor and could be considered out of scope for GHOP.

Menu fixes worked.

I get a white screen after submitting "Index PHP manual pages" -- but I suspect that behavior is in the D5 version as well.

After 6 tries, I can't get the module to index any content.

Make sure your allowed script execution time is 0 (unlimited). This is because there is a VAST amount of info to index in the database. There are many functions. So the script may need more time to execute than usual.

Though, when I put in a server absolute path to a folder with a correct file with documentation (including code) it adds the info in the database. But I can't seem to get it to display the new (index) page. :\

I don't think script execution time is the issue -- I never had that problem in D5, even on a shared host.

See http://therickards.com/api

I feel I may end up redoing everything except for the Menu System and the database Schema.

I don't think so. I didn't dig into my error logs very deeply, but I was seeing more 'undefined index' errors.

If the module is working in your test environment, then it means we need more testers.

Wow I received a huge error when trying to index the PHP manual.

EDIT: "The most common reason for the MySQL server has gone away error is that the server timed out and closed the connection." Hmm.

Let's start with just indexing local files. I wasn't able to get that working, either.

Well I know the process is. You put the files in a directory. You input which directory the file is in. Then you have to run cron.php. But still I'm having some troubles.

What kind of troubles? Perhaps we can schedule some IRC time for this weekend.

k. This task has been going on for about 2 weeks now, so it's time to wrap it up.

From what I can gather, this for the most part is done, but there remain critical issues which prevent it from working. And worse, these issues are very hard to track down and vary from system to system. Sounds thorny and messy. :(

agentrickard, linuxlover, could we come to a consensus of what we believe to be the minimum amount of functionality in order for us to mark this "Closed" and give linuxlover credit for the task so he can move onto another one?

I've extended the task deadline to 2007-12-18, but let's get this figured out ASAP.

To me, minimum functionality is the ability to index local files properly.

I just started poking at the code and found a major problem:

http://drupal.org/node/114774#db-num-rows

Which explains some of the issues we've run into. So the port needs to be checked against all conditions in the upgrade list.

db_next_id has also been deprecated. http://drupal.org/node/114774#db_last_insert_id

I think it is worth extending this task if we can get a few more reviewers to look at the output, or if we can wait for me to catch up.

Fixing those last two issues helps a lot.

One big gotcha: the ability to split page functions into separate files caused the old "is this a unique did" query to fail. It is now necessary to add "AND file_name = '%s' to that query.

The issue is that parser.inc has not been updated to Drupal 6. There are some minor changes that need checking.

Grr. I forgot to upload my parser.inc patch in the first place. Thats probably why there was so much inconsistency with the errors in testing.

It explains a ton, and I should have caught that sooner. Make sure you run each file through the whole list of changes in the updating from 5.x to 6.x list.

Also watch out for the gotcha noted in #37. The ability to split modules into multiple files may cause errors that did not exist in the D5 version.

Then, I think if we can parse and display API functions, this is done. Only worry about the E_ALL errors if they harm functionality.

Ok, Sorry about the mess. I'll apply your patch and make the necessary changes. I hope to have this done by tonight or tomorrow because I want to move on to the next task. Hopefully, the next one won't be as much of a hassle.

Alright. I applied your patch put in a path for the local documentation. And I got a message that api_parse_file() doesn't exist. This is due to api_cron() in api.module not being able to access 'modules/api/parser.inc'. (That path was generated with drupal_get_path('module', 'api') .'/parser.inc')).

I've made sure the file is named correctly...and that it points to the right place. But it can't seem to open the file and I don't know why.

In api_cron() I changed include_once to require_once and got this message.
Fatal error: require_once() [function.require]: Failed opening required 'modules/api/parser.inc' (include_path='.;C:\php5\pear') in C:\wamp\www\drupal\modules\api\api.module on line 1114

EDIT:
There was something wrong with my file permissions. For some reason there's a difference between "-rwx------" and "-rwx------+". The first permission was put on due to me applying the patch...curious. So now it no longer gives me that error.

Ok, api_cron() adds the correct info into the database. But it won't correctly call that info for viewing. I also get:

EDIT:
In parser.inc api_save_documentation() uses &$docblock as a parameter. For some reason with in that array, there isn't an index named groups. This is probably hindering the processing of the documentation.

Hmm. Well, For some reason the module can not determine the correct ID from the database so that it can display the correct page. I'll see if I can figure this out.

EDIT:
Something is going wrong with

If thats true then something is wrong in api_get_active_branch(). Maybe its the dynamic arguments (i.e. arg(1)).

linuxlover, could you attach a patch of what you're working on so far? that'd make it easier for other people to help test it and see if they can help you track down the errors.

Here is what I am currently working with.

Also, I have a question. I keep patching by comparing the original file to the new one. Am I doing this correctly? Or should I compare to the previous change (version) I made?

Probably easiest for all involved if you roll new patches against the original file(s). That way, people new to the issue can jump in.

Ok Good. I've been doing it correctly then.

These latest additions need review.

@linuxlover: we generally like patches to include changes to all files for the issue, so that only one patch is needed in a case like this. See http://drupal.org/patch/create for more information. It also looks like you're not using the correct options with diff when you create your patches. Normally patches will include the actual diff command used to create the patch.

If you're not using cvs diff to create your patches, I suspect that you will find that doing so will save you a lot of time and frustration.

Also, I noticed that you have made edits to many of your comments on this issue. Generally we only make edits when there is something like a typo or when you forget to upload a patch with a comment. The reason is that many people subscribe to issues via email, and when you edit a comment a new email is not sent. So, if you're adding useful information to a comment when editing it, people who don't actually browse this page itself won't see your edits.

I'm going to extend your due date on the GHOP task tracker by 3 additional days. It would be best if you could fix the patch as I specified above and then try to get people to review the patch for you. Dropping into #drupal or #drupal-ghop on irc.freenode.net and asking for reviewers can be an effective way to get people to review your patch (and also to get to know other community members better).

Ok used cvs diff. This patch should be up to the standards provided. If not, please notify me.

Still getting duplicate key errors from api_save_documentation when running cron.php.

Path 'api/files' shows no values even when data exists in database.

Path 'api/functions' shows no values even when data exists in database.

I went ahead and gave linuxlover credit on this task, based on the work done so far. It'd be awesome if the community could help carry it the rest of the way.

Agreed. Sorry that I have been working on my own stuff.

Rerolled against DRUPAL-5 instead of HEAD that has had DRUPAL-5 tacked onto it. (more signal, less noise, branch merges should be done as branch merges, not patches.)

Claiming task temporarily.

Well, I didn't get it quite figured out, but I did a lot of work tidying up the menu hook...

Still definately "needs work." I'm seeing a LOT of warnings coming from the parser as well... Then again, I run with E_ALL...

This code is producing a message on each drupal page that says:

"

I couldn't pin down the bug, but, I didn't have much time either. Also, files don't appear to be crawled.

Robin

58 comments and no real progress - i think we need to tackle this issue a a little more structured. i posted a suggestion for a 6.x Upgrade Roadmap to the API module developers group at http://groups.drupal.org. please chime in, discuss that roadmap (it's a wiki page) and the implementation, and let the (small) patches (that would be applied to HEAD step by step) flow in. thanks everyone!

subscribing ...

@Robin: The '' comes from drupal_set_message(var_export(implode('/', array_slice($map, $index + 3, $end - $index + 1)), true)); which seems to just be a debug statement.

Line 1149 checked for $form_state['values']['default_branch'] which called a notice if it wasn't defined, so I added a !empty() around it. Apart from this and removing some excessive whitespace (a newline and two trailing spaces), nothing else is changed in the patch, as the parser bails out in a pgsql environment. I'm going to try and use my MySQL and see if it works there, and then possibly work on cross-compatibility. Possibly.