Created attachment 265027 [details] [review]
ftp: Set etag::value

Set etag::value based on the file's modification time.

Why do we need this? I think It should be implemented etag check also into the replace method (as in smb, sftp, dav backends) to make sense.

(In reply to comment #2)
> Why do we need this? I think It should be implemented etag check also into the
> replace method (as in smb, sftp, dav backends) to make sense.

The etag check is used when replacing a file; however, etag::value is used for GFileInfo results.

Compare the output of gvfs-info on a file with sftp and ftp; the sftp backend has a etag::value line whereas the ftp backend doesn't.

The corresponding bit of code which sets etag::value in the sftp backend is in parse_attributes():
  if (flags & SSH_FILEXFER_ATTR_ACMODTIME)
    {
      guint32 v;
      char *etag;
      
      v = g_data_input_stream_read_uint32 (reply, NULL, NULL);
      g_file_info_set_attribute_uint64 (info, G_FILE_ATTRIBUTE_TIME_ACCESS, v);
      v = g_data_input_stream_read_uint32 (reply, NULL, NULL);
      g_file_info_set_attribute_uint64 (info, G_FILE_ATTRIBUTE_TIME_MODIFIED, v);

      etag = g_strdup_printf ("%lu", (long unsigned int)v);
      g_file_info_set_attribute_string (info, G_FILE_ATTRIBUTE_ETAG_VALUE, etag); << etag::value is set here
      g_free (etag);
    }

Of course, the etag should also be checked when replacing a file with the ftp backend but that is a separate issue.

(In reply to comment #3)
> (In reply to comment #2)
> > Why do we need this? I think It should be implemented etag check also into the
> > replace method (as in smb, sftp, dav backends) to make sense.
> 
> The etag check is used when replacing a file; however, etag::value is used for
> GFileInfo results.

You are right, I know about. I've been looking that some editors even use it.

Review of attachment 265027 [details] [review]:

Looks good. However would be nice to add comment/warning into replace function, or file a new bug about.

(In reply to comment #6)
> Review of attachment 265027 [details] [review]:
> 
> Looks good. However would be nice to add comment/warning into replace function,
> or file a new bug about.

Opened as bug 722412.

Pushed to master as 947ca91. Thanks for the review!