Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
API documentation in system.api.php
contains @see
references to upload_file_load()
and upload_file_delete()
. Neither of these functions appear to exist.
Comment | File | Size | Author |
---|---|---|---|
#23 | d7-1347812-23.patch | 1.11 KB | xjm |
#20 | 1347812-20.patch | 1.15 KB | xjm |
#17 | documentation-1347812-17.patch | 2.27 KB | nanotube |
#15 | documentation-1347812-15.patch | 1.53 KB | nanotube |
#7 | D8-1347812-4.patch | 736 bytes | nanotube |
Comments
Comment #1
xjmI checked with git blame and these lines date to the introduction of the file in 2008: http://drupalcode.org/project/drupal.git/commit/66df602593230a2483d65389...
Back then there was an
upload.module
. So we can just delete those lines or replace them with references tofile_load()
,file_load_multiple()
,file_delete()
, andfile_file_delete()
as appropriate.Comment #2
xjmComment #3
jhodgdonThat was done in d7, so needs fixing there too.
Comment #4
nanotube CreditAttribution: nanotube commentedSearched all files and references only exist in the
system.api.php
file.Comment #5
nanotube CreditAttribution: nanotube commentedSame patch as above, but rolled for D7.
Comment #7
nanotube CreditAttribution: nanotube commentedrenamed patches and re-upload.
Comment #8
nanotube CreditAttribution: nanotube commented.
Comment #10
drewish CreditAttribution: drewish commentednanotube, you want to name the files 1347812-D7.patch for Drupal 7 and 1347812.patch for Drupal 8. It looks for the -D7 to be a suffix before .patch. And the D8 version shouldn't have a suffix since it's based off the issue's version number.
Comment #11
xjmThanks nanotube. It looks like the
@see
references to the current functions were already there, so just removing these as the patch does should be fine.I grepped with this patch applied and found one other stray reference to
upload.module
:upload_munge_filename()
is nowfile_munge_filename()
. The stray reference is inincludes/file.inc
.Want to add that fix as well? You can name your D7 patch something like
1347812-12-d7.patch
so that testbot skips it. For the D8 patch something like1347812-12.patch
is fine. (Testbot skips anything with a-d#
right before the file extension.)There's also a few config variables that should probably be re-namespaced to
file_
instead ofupload_
, but that's not a documentation fix (nor backportable) so we can create a followup issue for that.Thanks!
Comment #12
jhodgdonRE #10 - Actually, normally we just make a d8 patch and get it through approval, then reroll for d7 at that point. It saves work, since most patches go through several iterations before approval.
RE #11 - I agree, can we include that one other upload_ function in the fix rather than making another issue.
Comment #13
xjmSetting to NW for #11. If someone could add that to the patch, that would be great. Thanks!
Comment #14
nanotube CreditAttribution: nanotube commentedI will get on this in a day or two.
Comment #15
nanotube CreditAttribution: nanotube commentedI didn't see this earlier because I was searching on the files inside the
modules/
directory.Here is the patch to also fix the stray reference in
includes/file.inc
.Comment #16
jhodgdonLooks almost perfect! One thing:
Maybe instead of removing that line, it should be replaced with file_load()?
Comment #17
nanotube CreditAttribution: nanotube commentedAt first, I did not understand why, but a visit to the API website helped. Made the changes and attached a new patch.
Comment #18
xjmI think you added it to the wrong docblock?
Also, I'm actually not sure that we need to add
file_load()
, since it just callsfile_load_multiple()
, which is already referenced. So I actually think #15 is probably good. Let's see what @jhodgdon says, though. :) Thanks!Comment #19
jhodgdonI think referencing file_load() is useful.
That patch is weird... it seems to have system.api.php in it twice?
Comment #20
xjmOh, I see. The patch was created using git format-patch. So actually it's just an illusion of the diff that it was added to the wrong docblock. This is why it's better to use git diff to create patches. :) So here's a reroll of #17 created with git diff.
Comment #21
jhodgdongood deal!
Comment #22
catchCommitted/pushed to 8.x, will need a quick re-roll for 7.x.
Comment #23
xjmA simple reroll. No differences between D7 and D8 here.
Comment #25
catch#23: d7-1347812-23.patch queued for re-testing.
Comment #26
jhodgdonComment #27
webchickGood catch! DIE UPLOAD MODULE, DIE.
Committed and pushed to 7.x. Thanks! :)