Comments

mikeytown2’s picture

Status: Active » Needs review
StatusFileSize
new45.09 KB
new42.81 KB

Status: Needs review » Needs work

The last submitted patch, drupal-1345402-1.patch, failed testing.

mikeytown2’s picture

Status: Needs work » Needs review
StatusFileSize
new45.43 KB
new43.15 KB

Round 2

Status: Needs review » Needs work

The last submitted patch, drupal-1345402-2.patch, failed testing.

mikeytown2’s picture

Awesome... looks like simpletest is ran by patching core. Anyway this patch does work, just not with simpletest.

[17:34:07] Applying in /var/lib/drupaltestbot/sites/default/files/checkout file=sites/default/modules/simpletest/D6-core-simpletest.patch
[17:34:07] Command [git apply --check -p0 sites/default/modules/simpletest/D6-core-simpletest.patch 2>&1] failed
  Duration 0 seconds
  Directory [/var/lib/drupaltestbot/sites/default/files/checkout]
  Status [1]
 Output: [error: patch failed: install.php:20
error: install.php: patch does not apply
error: patch failed: includes/bootstrap.inc:1138
error: includes/bootstrap.inc: patch does not apply
error: patch failed: includes/common.inc:2661
error: includes/common.inc: patch does not apply].
[17:34:07] Encountered error on [review], details:
array (
  '@filename' => 'D6-core-simpletest.patch',
)
[17:34:07] Review complete. test_id=194273 result code=8 details=Array
(
    [@filename] => D6-core-simpletest.patch
)

[17:34:07] Static variables reset.
PatchRanger’s picture

mikeytown2’s picture

Status: Needs work » Needs review
StatusFileSize
new49.95 KB

Re-roll of #2. Patch in #2 applied to core with a little bit of fuzz; no fails.

Status: Needs review » Needs work

The last submitted patch, drupal-1345402-7.patch, failed testing.

PatchRanger’s picture

@mikeytown2 Nice try, thanks.
Fresh patch failed by the same reason: D6-core-simpletest.patch failed to apply to the files that were already modified by your patch. I think the reason of D6-core-simpletest.patch failure is in the nature of that patch: it is CVS-style, which (I guess) is rather stupid in merging hunks. The solution is to re-roll D6-core-simpletest.patch (in fact, that is what #1883872: Re-roll the patch D6-core-simpletest.patch to make it to be applied cleanly in #1345402 about).

noslokire’s picture

Mikeytown
Couldnt enable or disable a module with this patch. Reverting this part of patch#7 in the includes/install.inc restored site functionality:

@@ -714,7 +714,7 @@ function drupal_check_module($module) {
// Include install file
$install = drupal_get_install_files(array($module));
if (isset($install[$module])) {
- require_once $install[$module]->filename;
+ require_once DRUPAL_ROOT . '/' . $install[$module]->filename;

mikeytown2’s picture

Thanks for letting me know. I'll investigate it sometime next week.

mdupont’s picture

What would DRUPAL_ROOT add to Drupal 6?

mikeytown2’s picture

Better performance. It saves php from having to look up the real path when given a relative path. http://www.php.net/manual/en/function.realpath-cache-size.php#101840

Every time you perform any of the various file functions or include/require a file and use a relative path, PHP has to look up where that file really exists.

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 7: drupal-1345402-7.patch, failed testing.

naveenvalecha’s picture

Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new49.95 KB

Rerolled #7 patch which had some hunks.

Status: Needs review » Needs work

The last submitted patch, 16: Backport-DRUPAL_ROOT-1345402-16.patch, failed testing.

Status: Needs work » Closed (outdated)

Automatically closed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.