Directory structure prevents successful translation file import

This will need fixed after #483844: Restore code to beta0. I like the directory structure Kiam came up with but if it causes issues with the translations then I can move the xmlsitemap module back to the parent directory.

I will move the xmlsitemap/ directory files back to the main module directory to provide Kiam the ability to do as he says. I agree that it is a bit much to translate the same base strings over and over again. While what you have isn't bad but the current tool set doesn't support it easily and the users aren't that used to not finding an info and a module file in the base directory. Resetting to critical and a bug report to indicate the importance of this issue.

NOTE: Kiam, I don't think it was wrong that you put the module files in a subdirectory and I have seen it elsewhere and have supported that idea. It is just a pain for the translation and easier for this project to change than it is to change all the other projects.

@dave, @hass, I see both of you are agreeing with each other without realizing it.

Current 6.x-1.x design:

xmlsitemap/xmlsitemap/xmlsitemap.module
...

6.x-2.x design:

xmlsitemap/xmlsitemap.module
...

Hass has said that the current 6.x-1.x design causes issues with translation because he can't create one .pot file since there is no module in the main directory. He has stated this as a bug because it isn't The Drupal Way(tm). Instead he proposes the directory structure should be the following.

Proposed 6.x-1.x design:

xmlsitemap/xmlsitemap.module
...

Anyway, could the directory structure be considered a bug?
I could be old school, but a bug is something that causes the code to
not work properly, or to return a wrong result. The code works correctly
whichever directory is used for the module (which still needs to be
placed in sites/all/modules).

I hate to pollute this issue but for Kiam's et alia benefit and to get this off my chest; a bug can be relative to design as well as code. The code itself may be coded to design but that doesn't make the design free of flaws. Analyst's who point to the developer of his design without consideration of a flaw in the design as the one at fault usually do not do well AFAIC. Developers who realize the design flaw and keep silent about it also do not do well AFAIC. Design and code are inseparable and both can be faulty. In this case it is merely the fact that the directory structure does not permit ease of translation due to Drupal as a whole so therefore the module is buggy due to other limitations.

I don't see anything in Drupal.org that says xmlsitemap.module must be in xmlsitemap, rather than xmlsitemap/xmlsitemap

All truths are yet to be written; this truth has been implied. It is how 99% of all modules are written including those in core. Just because one module has set precedent doesn't make it correct.

I'm trying to determine the best way to handle a hook_update_N that will check for the existence of an xmlsitemap directory in the current module directory. I'm thinking that maybe an abort by throwing an exception is the best way to get the users attention. It doesn't look pretty but leaving the directory in place is asking for trouble and the module shouldn't be able to removed the directory on its own.

Maybe a hook_init that checks for the directory should also be created for this same abort.

Ideas please.

@hass: For the reason that people don't do what they need to do. Just check this issue queue for the number of people who don't do what they're told. Posts complaining that they get errors about the nid column is missing in xmlsitemap_node or the tid column is missing in xmlsitemap_taxonomy just make my hair stand on end.

@Dave Reid: Thanks for the suggestion. I'll give it a try but throwing a exception from the hook_update_N is handled extremely well in the update.php script. I had to call the watchdog function before throwing the exception though. The message I created is a little more blunt stating that instructions were not followed and that if the instructions had been followed the situation causing issues wouldn't exist.

In xmlsitemap.module I have:

/**
 * Implementation of hook_init().
 */
function xmlsitemap_init() {
  // Make sure that the xmlsitemap directory is removed.
  $path = drupal_get_path('module', 'xmlsitemap');
  if (is_dir($path . '/xmlsitemap')) {
    $msg = 'You have not correctly followed the installation instructions for this module.  You must make certain that the old xmlsitemap/xmlsitemap diretory is removed.  The xmlsitemap/xmlsitemap/xmlsitemap.* files have been moved back to the modules root directory.  The install instructions were to removed the modules/xmlsitemap directory completely before extracting the update.  If you had followed that step you would have prevented this message.';
    watchdog('xmlsitemap', $msg, array(), WATCHDOG_ERROR);
    drupal_set_message(t($msg), 'error');
  }
}

And in xmlsitemap.install I have:

function xmlsitemap_update_6150() {
  // Used 6150 since beta5 contained 6148.
  // Make sure that the xmlsitemap directory is removed.
  $ret = array();
  $path = drupal_get_path('module', 'xmlsitemap');
  if (is_dir($path . '/xmlsitemap')) {
    $msg = 'You have not correctly followed the installation instructions for this module.  You must make certain that the old xmlsitemap/xmlsitemap diretory is removed.  The xmlsitemap/xmlsitemap/xmlsitemap.* files have been moved back to the modules root directory.  The install instructions were to remove the modules/xmlsitemap directory completely before extracting the update.  If you had followed that step you would have prevented this message.';
    watchdog('xmlsitemap', $msg, array(), WATCHDOG_ERROR);
    $ret['#abort'] = array(
      'success' => FALSE,
      'query' => $msg,
    );
  }
  return $ret;
}

A decision on the message string needs to be made. OASN: Drupal needs to standardize it's use of t(), why does watchdog not require it while drupal_set_message does. Hopefully this is addressed in D7 but I haven't the time to look.

I will look at hook_requirements but I want this to be more obnoxious than an ice cube in Hell. Who better to get a sites' owner attention than the normal users of the system but I can filter the message to those who have administration access.

Hook_requirements isn't obnoxious enough. I want the idiot who installs without removing the previous directory to know immediately that there is a problem and with hook_requirements you have to go to admin/reports before you see the message. A user may have left the module active and installed as well and just overwrite the directory with the contents of the new package regardless of how many times you say otherwise, and thus thwarting the "cannot install" nicety. I've modified the hook_init like the following to address who sees the message issue.

/**
 * Implementation of hook_init().
 */
function xmlsitemap_init() {
  // Make sure that the xmlsitemap directory is removed.
  $path = drupal_get_path('module', 'xmlsitemap');
  $accesses = array(
    'access administration pages',
    'access site reports',
    'administer actions',
    'administer files',
    'administer site configuration',
  );
  if ($user->uid == 1) {
    $admin_user = TRUE;
  }
  else {
    foreach ($accesses as $access) {
      if ($admin_user = user_access($access)) {
        break;
      }
    }
  }
  if (is_dir($path . '/xmlsitemap') && $admin_user) {
    $msg = 'You have not correctly followed the installation instructions for this module.  You must make certain that the old xmlsitemap/xmlsitemap diretory is removed.  The xmlsitemap/xmlsitemap/xmlsitemap.* files have been moved back to the modules root directory.  The install instructions were to removed the modules/xmlsitemap directory completely before extracting the update.  If you had followed that step you would have prevented this message.';
    watchdog('xmlsitemap', $msg, array(), WATCHDOG_ERROR);
    drupal_set_message(t($msg), 'error');
  }
}

The main problem of this edge case is that the user can execute with no seeming issue. I will implement a hook_requirements instead of hook_init. IMO, it is more of a waste of time to spend responding to nonsense issues with RTFM and protecting the issue queue from idiots as much as possible is a good thing; but you're correct, wasting processing time isn't worth it.

Change has been committed. http://drupal.org/cvs?commit=228264