When trying to upload a module, I got the following error message:

  • File Transfer failed, reason: Unable to remove to file /var/www/drupal/modules/webform/.gitignore

It should say 'Unable to remove THE file'.

The offending files are below:

/var/www/drupal/core/lib/Drupal/Core/FileTransfer/FTPExtension.php:
   75    protected function removeFileJailed($destination) {
   76      if (!ftp_delete($this->connection, $destination)) {
   77:       throw new FileTransferException("Unable to remove to file @file", NULL, array('@file' => $destination));
   78      }
   79    }

/var/www/drupal/core/modules/system/src/Tests/FileTransfer/TestFileTransfer.php:
   47    function removeFileJailed($destination) {
   48      if (!ftp_delete($this->connection, $item)) {
   49:       throw new FileTransferException('Unable to remove to file @file.', NULL, array('@file' => $item));
   50      }
   51    }
CommentFileSizeAuthor
#21 grammar-issue-with-ftp-error-message-2859029-21.patch2.36 KBjoe_carvajal
#15 interdiff.txt1.2 KBajmantis
#15 grammar-issue-with-ftp-error-message-2859029-15.patch1.4 KBajmantis
#9 grammar.txt1.06 KBPavan B S
#9 grammar-issue-with-ftp-error-message-2859029-10.patch2.34 KBPavan B S
#6 grammar-issue-with-ftp-error-message-2859029-6.patch1.4 KBmanojbisht_drupal
#5 2859029-grammar-issue-with-ftp-error-message-5.patch682 bytesmayank_kaushik
#3 2859029-grammar-issue-with-ftp-error-message-3.patch1.36 KBmayank_kaushik
#2 2859029-grammar-issue-with-ftp-error-message-2.patch1.36 KBmayank_kaushik

Comments

CatsFromStonehenge created an issue. See original summary.

mayank_kaushik’s picture

mayank_kaushik commented
StatusFileSize
new 2859029-grammar-issue-with-ftp-error-message-2.patch1.36 KB

Here is attached patch for Grammar issue with FTP error message.

mayank_kaushik’s picture

mayank_kaushik commented
Status: Active » Needs review
StatusFileSize
new 2859029-grammar-issue-with-ftp-error-message-3.patch1.36 KB

RE-attaching the patch with status update.

cilefen’s picture

cilefen commented
Version: 8.2.x-dev » 8.3.x-dev
Issue tags: -error message, -ftp

Read the issue tag guidelines: https://www.drupal.org/node/1023102

mayank_kaushik’s picture

mayank_kaushik commented
Issue tags: +Novice
StatusFileSize
new 2859029-grammar-issue-with-ftp-error-message-5.patch682 bytes

Issue tag updated. As version Changed to 8.3.x-dev for issue, Updated has been attached. In 8.3.x-dev version TestFileTransfer.php has been removed.

manojbisht_drupal’s picture

manojbisht_drupal commented
StatusFileSize
new grammar-issue-with-ftp-error-message-2859029-6.patch1.4 KB

Hi,

Please find attached patch, It has been generated with respect to 8.4.x, as new commit will be related to this branch.

Thanks,
Manoj Bisht

prash_98’s picture

prash_98 commented
Version: 8.3.x-dev » 8.4.x-dev
Status: Needs review » Reviewed & tested by the community

The required changes happen and also the patch applies well with 8.4.x.
So changing to RTBC.

alexpott’s picture

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

It looks as if this was intended and not a mistake - ie it is trying to say the "to" file - as in destination. We also have the following exception messages in the same class.

throw new FileTransferException("Unable to change to directory @directory", NULL, ['@directory' => $directory]);

throw new FileTransferException("Unable to remove to directory @directory", NULL, ['@directory' => $directory]);

In all cases it looks like the error would be improved by changing the to to a the because the directory / file is also specified in the message.

Pavan B S’s picture

Pavan B S commented
Status: Needs work » Needs review
StatusFileSize
new grammar-issue-with-ftp-error-message-2859029-10.patch2.34 KB
new grammar.txt1.06 KB

Applying the patch, please review

joe_carvajal’s picture

joe_carvajal
He/him
Primary language Spanish
Location Seville (Spain)
commented
Status: Needs review » Reviewed & tested by the community

It seems to be OK according to the comment #8.

joe_carvajal’s picture

joe_carvajal
He/him
Primary language Spanish
Location Seville (Spain)
commented
Issue tags: +DevDaysSeville

Update issue with the event tag.

cilefen’s picture

cilefen commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/lib/Drupal/Core/FileTransfer/FTPExtension.php
@@ -45,7 +45,7 @@ protected function createDirectoryJailed($directory) {
-      throw new FileTransferException("Unable to change to directory @directory", NULL, ['@directory' => $directory]);
+      throw new FileTransferException("Unable to change the directory @directory", NULL, ['@directory' => $directory]);

Isn't this one actually "change to"?

haza’s picture

haza
Primary language French
commented

I agree with that. I think the "to directory" should stay the same, and only the one (the file related error) might be changed.

ajmantis’s picture

ajmantis commented
Assigned: Unassigned » ajmantis

I will work on this.

ajmantis’s picture

ajmantis commented
Status: Needs work » Needs review
StatusFileSize
new grammar-issue-with-ftp-error-message-2859029-15.patch1.4 KB
new interdiff.txt1.2 KB

I have make changes according to #13

darius.restivan’s picture

darius.restivan commented
Status: Needs review » Reviewed & tested by the community

Fixed

alexpott’s picture

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

Re #12 and #13.... nope it is not "change to" here "to" means destination in same way it is being used in the file example.

I'm wrong if (!ftp_chdir($this->connection, $directory)) {

alexpott’s picture

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

But still we have this later...

    if (!ftp_rmdir($this->connection, $directory)) {
      throw new FileTransferException("Unable to remove to directory @directory", NULL, ['@directory' => $directory]);
    }
alexpott’s picture

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

Also for

throw new FileTransferException("Unable to change to directory @directory", NULL, ['@directory' => $directory]);

To actually be clear we should change it to:

throw new FileTransferException("Unable to change current directory to @directory", NULL, ['@directory' => $directory]);
joe_carvajal’s picture

joe_carvajal
He/him
Primary language Spanish
Location Seville (Spain)
commented

OK! Thank you Alexpott, I'll do it then.

joe_carvajal’s picture

joe_carvajal
He/him
Primary language Spanish
Location Seville (Spain)
commented
StatusFileSize
new grammar-issue-with-ftp-error-message-2859029-21.patch2.36 KB
joe_carvajal’s picture

joe_carvajal
He/him
Primary language Spanish
Location Seville (Spain)
commented
Status: Needs work » Needs review
dinesh18’s picture

dinesh18 commented

Reviewed the patch mentioned in comment #21 and it is working as expected.

joe_carvajal’s picture

joe_carvajal
He/him
Primary language Spanish
Location Seville (Spain)
commented

So, should we change the issue to RTBC?

sahilsharma011’s picture

sahilsharma011 commented

@joe_carvajal I reviewed the patch mentioned in #21 on 8.4.x branch. It works as expected. It can be changed to RTBC as it fixes all the grammar issues for aforementioned error messages.

juanjesustrigo’s picture

juanjesustrigo commented
Status: Needs review » Reviewed & tested by the community

According to comments #23 and #25 change issue to RTBC.

alexpott’s picture

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

Committed a908c19 and pushed to 8.4.x. Thanks!

8.3.x is now in a freeze I'm not sure we should backport to 8.3.1 because something in contrib or custom might be testing against these exception messages and it is not worth breaking that in a patch release.

  • alexpott committed a908c19 on 8.4.x 
    Issue #2859029 by maya96, ajmantis, Pavan B S, joe_carvajal,...

Status: Fixed » Closed (fixed)

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