CommentFileSizeAuthor
#16 2367745-16.patch871 bytesPalashvijay4O
#4 2367745-4.patch3.18 KBPalashvijay4O

Comments

gumanist’s picture

gumanist commented
Issue summary: View changes
Parent issue: #2367743: Remove usages of drupal_form_submit() and update documentation » #2205673: [META] Remove all @deprecated functions marked "remove before 8.0"

$ git grep drupal_var_export
core/includes/utility.inc:function drupal_var_export($var, $prefix = '') {
core/scripts/dump-database-d6.sh:// Include the utility drupal_var_export() function.
core/scripts/dump-database-d6.sh: $output .= "db_create_table('" . $table . "', " . drupal_var_export($data) . ");\n";
core/scripts/dump-database-d6.sh: $insert .= '->values('. drupal_var_export($record) .")\n";
core/scripts/dump-database-d6.sh: $output .= "db_insert('". $table . "')->fields(". drupal_var_export(array_keys($data['fields'])) .")\n";
core/scripts/dump-database-d7.sh:// Include the utility drupal_var_export() function.
core/scripts/dump-database-d7.sh: $output .= "db_create_table('" . $table . "', " . drupal_var_export($data) . ");\n";
core/scripts/dump-database-d7.sh: $insert .= '->values('. drupal_var_export($record) .")\n";
core/scripts/dump-database-d7.sh: $output .= "db_insert('". $table . "')->fields(". drupal_var_export(array_keys($data['fields'])) .")\n";

gumanist’s picture

gumanist commented

$ git grep drupal_var_export
core/includes/utility.inc:function drupal_var_export($var, $prefix = '') {
core/scripts/dump-database-d6.sh:// Include the utility drupal_var_export() function.
core/scripts/dump-database-d6.sh: $output .= "db_create_table('" . $table . "', " . drupal_var_export($data) . ");\n";
core/scripts/dump-database-d6.sh: $insert .= '->values('. drupal_var_export($record) .")\n";
core/scripts/dump-database-d6.sh: $output .= "db_insert('". $table . "')->fields(". drupal_var_export(array_keys($data['fields'])) .")\n";
core/scripts/dump-database-d7.sh:// Include the utility drupal_var_export() function.
core/scripts/dump-database-d7.sh: $output .= "db_create_table('" . $table . "', " . drupal_var_export($data) . ");\n";
core/scripts/dump-database-d7.sh: $insert .= '->values('. drupal_var_export($record) .")\n";
core/scripts/dump-database-d7.sh: $output .= "db_insert('". $table . "')->fields(". drupal_var_export(array_keys($data['fields'])) .")\n";

Palashvijay4O’s picture

Palashvijay4O commented
Assigned: Unassigned » Palashvijay4O
Palashvijay4O’s picture

Palashvijay4O commented
Status: Active » Needs review
StatusFileSize
new 2367745-4.patch3.18 KB

Test patch .

dawehner’s picture

dawehner
Primary language German
commented 
+++ b/core/scripts/dump-database-d6.sh
@@ -62,7 +61,7 @@
+  $output .= "db_create_table('" . $table . "', " . Variable::export($data) . ");\n";

I always thought that these scripts run under d6/d7 itself, can you clarify whether they still work?

dawehner’s picture

dawehner
Primary language German
commented

These usages are actually valid as they are executed in the context of Drupal 6 / 7 if I understand it correctly.

chx’s picture

chx commented

Those scripts might be needed to create tests for D8 -> D8 migrations when we get there.

chx’s picture

chx commented
Component: forms system » other
Status: Needs review » Reviewed & tested by the community

And this looks great, thanks for the patch.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work

I can't find a change record for the creation of the Variable class and the deprecation of drupal_var_export(). #2022931: Move drupal_var_export() to \Drupal\Component\Utility does not reference one and searches reveal nothing. Can some on add one (https://www.drupal.org/node/add/changenotice) and link to this issue and that one.

chx’s picture

chx commented
Status: Needs work » Reviewed & tested by the community

Change notice has been written, thanks Palashvijay4O

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Reviewed & tested by the community » Needs work

Hehe, these scripts are always fun. :-)

I'm pretty sure @dawehner is correct in #6 that these scripts are not to be run from a D8 install.

That is verified by the D6 one containing db_fetch_array() which doesn't exist in D8. Sadly, both scripts already contain \Drupal::moduleHandler()->getModuleList().

So, not really sure what to do here.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Title: Remove usage of drupal_var_export() » Remove drupal_var_export() 
index ace93d9..048ed24 100644
--- a/core/scripts/dump-database-d6.sh

--- a/core/scripts/dump-database-d6.sh
+++ b/core/scripts/dump-database-d6.sh

index 4689672..b722fe4 100644
--- a/core/scripts/dump-database-d7.sh

--- a/core/scripts/dump-database-d7.sh
+++ b/core/scripts/dump-database-d7.sh

Yep do not change these files - these are d6 and d7 code. So could be "closed won't fixed". But now we can get on with the removal of the drupal_var_export function. So let's repurpose this issue. Since @Palashvijay4O already made the nice CR and attached it to this issue.

chx’s picture

chx commented

> Yep do not change these files - these are d6 and d7 code.

D7. D6 didnt have drupal_var_export AFAIK.

jamesdixon’s picture

jamesdixon commented

In that case the only file left we may need to change is:

core/includes/utility.inc:function drupal_var_export($var, $prefix = '') {

Should we remove the function from utility.inc?

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Yes

Palashvijay4O’s picture

Palashvijay4O commented
Status: Needs work » Needs review
StatusFileSize
new 2367745-16.patch871 bytes

Ok . Removed the function.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Needs review » Reviewed & tested by the community

Yup, awesome.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

This issue is a prioritized change (deprecated function removal) as per https://www.drupal.org/core/beta-changes and it's benefits outweigh any disruption. Committed 40b2f16 and pushed to 8.0.x. Thanks!

  • alexpott committed 40b2f16 on 8.0.x 
    Issue #2367745 by Palashvijay4O | gumanist: Remove drupal_var_export().

Status: Fixed » Closed (fixed)

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