Opened 10 years ago

Last modified 7 years ago

#4054 assigned Bug

Retain Finder Labels when using "Move Data File to..."

Reported by: livings124 Owned by: jordan
Priority: Normal Milestone: Sometime
Component: Mac Client Version: 2.20
Severity: Normal Keywords:
Cc:

Description

https://forum.transmissionbt.com/viewtopic.php?f=5&t=11340


I use finder labels to mark files after they have been downloaded, but before they are finished seeding. I use "Move Data File to..." to move the files off of my laptop to an external drive while they are still being seeded. Unfortunately, when Transmission moves those files within the application, the finder labels are set to blank.


Attachments (2)

xattrs.diff (2.3 KB) - added by dim-an 8 years ago.
xattrs.2.diff (5.1 KB) - added by dim-an 8 years ago.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 9 years ago by dim-an

This problem arises because Transmission doesn't copy extended attributes of a file. I can see two ways of fixing the bug:

  • use getxattr / setxattr which are also presented in other unix-like operating system and copy all extended attributes that are connected to file (and modify libtransmission/)

http://developer.apple.com/library/mac/#documentation/Darwin/Reference/ManPages/man2/setxattr.2.html

Which way do you find appropriate?

comment:2 Changed 9 years ago by livings124

The second option seems like the better choice, since it will be compatible with other attributes on other operating systems (and the code to do the move is in libtransmission).

Changed 8 years ago by dim-an

comment:3 Changed 8 years ago by dim-an

Almost a year later I'm sending you a patch proposal. :)

It turned out that xattr-family functions have little bit different signatures on Linux and on MacOS. My patch solves problem described in the ticket for MacOS. The behaviour in other OSes isn't changed. I don't think it would be hard to make support for Linux, but I don't feel confident about such change.

comment:4 Changed 8 years ago by livings124

The patch doesn't seem to be working for me. I have downloaded contents to the desktop, set a label on the folder (the transfer was of a directory), and then do move from the context menu, but the label's lost after the move.

Changed 8 years ago by dim-an

comment:5 Changed 8 years ago by dim-an

Oops, my error. Previous patch copies xattrs of ordinary files only. I completely forgot about folders.

New patch should solve problem for folders as well.

comment:6 Changed 8 years ago by livings124

Great, seems to be working! Thanks!

comment:7 Changed 8 years ago by jordan

This is a pretty good patch!

What would you think about moving the /* copy extended attributes */ block into tr_moveFile()? Is there any use case where it would be wrong for tr_moveFile() to copy the extended attributes?

comment:8 Changed 8 years ago by dim-an

I'm sorry, I'm not sure I got your point. :(

We need to copy extended attributes for files and for folders. Right now extended attributes for files are copied in tr_moveFile (utils.c) and extended attributes for folders are copied in setLocation (torrent.c).

Is your suggestion about moving all extended attributes related code to tr_moveFile?

If yes: such scheme is possible, but in this case determining base directory (and for which folders we should update extended attributes) looks little bit tricky for me. Currently, I see one way to do this: if destination folder was created during the tr_moveFile, extended attributes from corresponding source folder should be copied to this folder.

comment:9 Changed 8 years ago by dim-an

jordan, ping?

comment:10 Changed 8 years ago by jordan

Right, that's it exactly. My initial thought was that tr_moveFile() would be a more useful generic function it it were able to copy the source folders' properties to the folders that it mkdirp()ed. We wouldn't need the copiedxattrs array anymore, since it would be handled by tr_moveFile().

However... looking through the code, I see (1) there are no other callers to the tr_moveFile() function so the "more useful generic function" goal isn't very concrete and (2) the generic "source, target" signature to tr_moveFile() doesn't guarantee any kind of symmetry between the source and target, so figuring out which folder's attributes to copy, and where, would be problematic.

So maybe we're better off using the patch as-is. It is a really good patch.

Part of this is that setLocation() (and its sibling, deleteLocalData()) are a dangerous mixture of critical and convoluted, so I resist adding more code to them even when that code is useful...

comment:11 Changed 8 years ago by jordan

  • Milestone changed from None Set to 2.80
  • Owner changed from livings124 to jordan
  • Status changed from new to assigned

comment:12 Changed 7 years ago by livings124

  • Milestone changed from 2.80 to Sometime
Note: See TracTickets for help on using tickets.