-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add option to link files instead of moving or copying them. #710
Conversation
Cool; thanks for addressing this! For the record, this fixes #114. Let's get this finished so we can merge it as soon as possible. Here's what we need to do:
Thanks again for tackling this! It will be great to have in beets core. |
Ah, forgot one more loose end:
|
Right, what type of exception should I raise? |
Good question. The utility function should perhaps raise a special-purpose exception? Or FilesystemError? That's up for debate. Then, the UI should catch this and raise a UserError instead to print a nice message. The tricky part is making sure that the UI code is what raises the UserError so the utility and library stay pure. |
The python os library will raise an exception itself [0] if the function is not supported on the platform, perhaps not try-caching but just letting that exception go through would be better? I need pointers as to what documentation and how I would write tests for the new code. |
Looking great! Thanks for adding the config option. It raises an AttributeError according to my VM:
which is unfortunately unspecific. And good question about the tests. The harnesses are fairly well developed for importer tests; all you need to do is follow the template for one of the existing file-handling tests (e.g., test_import_copy_arrives and test_import_with_move_deletes_import_files). As you can see, the tests set configuration options, run an import, and then inspect the filesystem to see what happens. The test here should turn off moving and copying, enable linking, and make sure that the original file exists but the new location now contains a symlink to the existing file. Thanks again for following up on this—looking better and better! |
Nice job, I've tested it to replace my old python scripts and it works nicely with the symbolic links. Only one issue though, I build the repository on a Debian system and share with Samba to all Windows machines in the house, but, as the symbolic links are absolute instead of relative the music files are not found. Would it be possible to have an option to make symbolic links relative? Best regards |
Having the path be relative should absolutely be possible, though I have no idea how. Though it seems python comes with batteries included: https://docs.python.org/2/library/os.path.html#os.path.relpath I've had a break from working on this firstly because I got a heavy work load on me at uni and now because I'm on vacation mostly without internet access, but secondly because writing tests was confusing. But I'll hop onto irc and ask @sampsyo about the test writing at some point in the future. |
Ye, most of the link stuff is easy, I used Unipath (https://pypi.python.org/pypi/Unipath/) in my old scripts which did the job of making the relative link. |
Modifying util.init.py to the following did the trick for me: def link(path, dest, replace=False, relative=True):
"""Create a symbolic link from path to `dest`. Raises an OSError if
`dest` already exists, unless `replace` is True. Does nothing if
`path` == `dest`."""
if (samefile(path, dest)):
return
path = syspath(path)
dest = syspath(dest)
if relative:
path = os.path.relpath(path, os.path.realpath(os.path.dirname(dest)))
if os.path.exists(dest) and not replace:
raise FilesystemError('file exists', 'rename', (path, dest),
traceback.format_exc())
try:
os.symlink(path, dest)
except OSError:
raise FilesystemError('Operating system does not support symbolic '
'links.', 'link', (path, dest),
traceback.format_exc()) |
Cool! Perhaps one of you can update this PR/open a new one? |
Ill have a look, as an old svn but newbie git user Im still feeling my way around lol |
Is there any point at which absolute links are preferable to relative? I can't think of one. About updating or opening a new pull request: Either works. You pull my changes, add yours and then push your changes to github. After that I get and push your new changes to this branch or you open up a new pull request from your branch. Only difference being that the former keeps comments and changes all in one thread. |
Here's one advantage of absolute symlinks to consider: they don't break when they move. If you every change your directory structure and move the symlinks to a different location, relative symlinks would need to be rewritten but absolute ones would not. |
Well relative ones work within a network share, no matter where you mount it (SMB, NFS), absolute wont. If we start moving the directories around chances are the original file location will change as well and then all bets are off... But, it might be nice to have an option to control it as I can't see every method ppl like to use to organize. |
Then we also have hard links which don't break on file system structure changes at all, you can move both the "source" and "destination" file, but require the files to be on the same fs. So then we have three types of links good for three different situations. @frafall Though relative symlinks won't work if the destination isn't shared in the same tree as the link, right? |
Lol so very true, so it all depends on how people prefer to organize their music. That's the main reason I moved to Beets from my own Python scripts, it really is so flexible can accommodate pretty much everyone's needs :) |
2ded210
to
4b11eed
Compare
@sampsyo What wuold you say about just trying to land this, and then we could add the option for relative links later? I'm quite lost on how to write these test cases. D'you think you could lend me a hand with that? |
@Rovanion, I can help you with the tests: Have a look at the
If your feeling adventurous you can also try to test it with the tagger to make sure that tags are updated in the original file the symlink points to. |
The tagger should preferrably be disabled by default when the symlink 2014-09-11 11:10 GMT+02:00 geigerzaehler notifications@github.com:
|
Ah, ok. Nevertheless, if it is possible people will do it and we should test it. |
Yeah that's correct. 2014-09-11 11:44 GMT+02:00 geigerzaehler notifications@github.com:
|
Hmm, I'm a little confused—maybe I'm missing something, but the current diff does not seem to add anything to the import process. It just seems to add the utility functions and extra flags to the model methods. Maybe something got lost in the git munging? |
@sampsyo I probably messed up my rebase on master. |
Okay, cool—if you can sort out the rebase, then it does seem like this is worth merging! |
Add option to link files instead of moving or copying them.
Thanks again! I've finished this up, @Rovanion. Now merged. |
Thank you! |
Hi,
I've made modifications to beets so that it links in the files instead of copying or moving them. Beets now links when both link and copy are set to no in the yaml since I haven't been able to get a link: yes option into the configure['import'] map.
So this works, as far as I can tell. But I would need help with what I should add where to have for command line flags and config file settings. It only works on systems where Python implements link i.e. not windows.