beetbox / beets

music library manager and MusicBrainz tagger
http://beets.io/
MIT License
12.73k stars 1.82k forks source link

permissions: Permit leading zero in YAML for octal notation #2646

Open darnir opened 7 years ago

darnir commented 7 years ago

Problem

I'm using the "permissions" plugin to set the correct file permissions on my Music collection. However, when setting the permissions value in the yaml file to "0644" and "0755", the following stack trace occurs.

If I set the values to "644" and "755", note the missing zeroes, it works just fine. However, permission values in octal are almost always written with a leading 0 to set the sticky bit correctly as well. While, most should not care about the sticky bit in this program, we should be allowed to write the permissions value with the bit stated.

Led to this problem:

Traceback (most recent call last):
  File "/usr/bin/beet", line 11, in <module>
    load_entry_point('beets==1.4.5', 'console_scripts', 'beet')()
  File "/usr/lib/python3.6/site-packages/beets/ui/__init__.py", line 1256, in main
    _raw_main(args)
  File "/usr/lib/python3.6/site-packages/beets/ui/__init__.py", line 1243, in _raw_main
    subcommand.func(lib, suboptions, subargs)
  File "/usr/lib/python3.6/site-packages/beets/ui/commands.py", line 934, in import_func
    import_files(lib, paths, query)
  File "/usr/lib/python3.6/site-packages/beets/ui/commands.py", line 911, in import_files
    session.run()
  File "/usr/lib/python3.6/site-packages/beets/importer.py", line 325, in run
    pl.run_parallel(QUEUE_SIZE)
  File "/usr/lib/python3.6/site-packages/beets/util/pipeline.py", line 445, in run_parallel
    six.reraise(exc_info[0], exc_info[1], exc_info[2])
  File "/usr/lib/python3.6/site-packages/six.py", line 686, in reraise
    raise value
  File "/usr/lib/python3.6/site-packages/beets/util/pipeline.py", line 358, in run
    self.coro.send(msg)
  File "/usr/lib/python3.6/site-packages/beets/util/pipeline.py", line 171, in coro
    task = func(*(args + (task,)))
  File "/usr/lib/python3.6/site-packages/beets/importer.py", line 1463, in manipulate_files
    task.finalize(session)
  File "/usr/lib/python3.6/site-packages/beets/importer.py", line 563, in finalize
    self._emit_imported(session.lib)
  File "/usr/lib/python3.6/site-packages/beets/importer.py", line 589, in _emit_imported
    plugins.send('album_imported', lib=lib, album=self.album)
  File "/usr/lib/python3.6/site-packages/beets/plugins.py", line 452, in send
    result = handler(**arguments)
  File "/usr/lib/python3.6/site-packages/beets/plugins.py", line 124, in wrapper
    return func(*args, **kwargs)
  File "/usr/lib/python3.6/site-packages/beetsplug/permissions.py", line 84, in fix
    dir_perm = convert_perm(dir_perm)
  File "/usr/lib/python3.6/site-packages/beetsplug/permissions.py", line 26, in convert_perm
    return int(perm, 8)
ValueError: invalid literal for int() with base 8: '493'

Here's a link to the music files that trigger the bug (if relevant):

Setup

My configuration (output of beet config) is:

lyrics:
    bing_lang_from: []
    auto: yes
    bing_client_secret: REDACTED
    bing_lang_to:
    google_API_key: REDACTED
    google_engine_ID: REDACTED
    genius_api_key: REDACTED
    fallback:
    force: no
    sources: [google, lyricwiki, musixmatch]
directory: ~/My_Music
threaded: yes

ui:
    color: yes

import:
    move: no
    copy: yes
    resume: yes
    incremental: yes

plugins: web fetchart lyrics lastgenre lastimport duplicates discogs acousticbrainz missing scrub mbsync fromfilename thumbnails badfiles copyartifacts chroma permissions convert

clutter:
- Thumbs.DB
- .DS_Store
- '*.m3u'
- .pls
- '*.jpg'
lastfm:
    user: darnir_redhat
    api_key: REDACTED

paths:
    ext:log: $albumpath/$artist - $album
    ext:cue: $albumpath/$artist - $album
thumbnails:
    dolphin: yes
    auto: yes
    force: no
copyartifacts:
    print_ignored: yes
    extensions: .cue .log .CUE .m3u
permissions:
    file: 0644
    dir: 0755
acoustid:
    apikey: REDACTED
convert:
    copy_album_art: yes
    embed: yes
    dest:
    pretend: no
    threads: 4
    format: mp3
    formats:
        aac:
            command: ffmpeg -i $source -y -vn -acodec aac -aq 1 $dest
            extension: m4a
        alac:
            command: ffmpeg -i $source -y -vn -acodec alac $dest
            extension: m4a
        flac: ffmpeg -i $source -y -vn -acodec flac $dest
        mp3: ffmpeg -i $source -y -vn -aq 2 $dest
        opus: ffmpeg -i $source -y -vn -acodec libopus -ab 96k $dest
        ogg: ffmpeg -i $source -y -vn -acodec libvorbis -aq 3 $dest
        wma: ffmpeg -i $source -y -vn -acodec wmav2 -vn $dest
    max_bitrate: 500
    auto: no
    tmpdir:
    quiet: no

    paths: {}
    never_convert_lossy_files: no
    album_art_maxwidth: 0
chroma:
    auto: yes
lastimport:
    per_page: 500
    retry_limit: 3
duplicates:
    album: no
    checksum: ''
    copy: ''
    count: no
    delete: no
    format: ''
    full: no
    keys: []
    merge: no
    move: ''
    path: no
    tiebreak: {}
    strict: no
    tag: ''
scrub:
    auto: yes
web:
    host: 127.0.0.1
    port: 8337
    cors: ''
    reverse_proxy: no
    include_paths: no
missing:
    count: no
    total: no
    album: no
lastgenre:
    whitelist: yes
    min_weight: 10
    count: 1
    fallback:
    canonical: no
    source: album
    force: yes
    auto: yes
    separator: ', '
    prefer_specific: no
acousticbrainz:
    auto: yes
    force: no
fetchart:
    auto: yes
    minwidth: 0
    maxwidth: 0
    enforce_ratio: no
    cautious: no
    cover_names:
    - cover
    - front
    - art
    - album
    - folder
    sources:
    - filesystem
    - coverart
    - itunes
    - amazon
    - albumart
    google_key: REDACTED
    google_engine: 001442825323518660753:hrh5ch1gjzm
    fanarttv_key: REDACTED
    store_source: no
discogs:
    apikey: REDACTED
    apisecret: REDACTED
    tokenfile: discogs_token.json
    source_weight: 0.5
    user_token: REDACTED
sampsyo commented 7 years ago

That's a good point; it would be nice to be able to write these in octal. As insane as it has always seemed to me, a leading zero is the standard for denoting an octal literal.

We actually have special logic in the plugin to decipher numbers written in octal but parsed as decimal because they don't have a leading zero. It's also possible, I think, to use a YAML string like "0755" to get the effect you want. I'm marking this as a feature request because it would be nice to also omit those quotes. At the very least, we should not throw an exception.

As a caveat, however, this might be a little tricky: because the YAML parser itself decides whether the number is octal or not, we'll have to guess after the fact which way it got parsed. The path of least resistance is probably to try parsing one way and, when that fails, fall back to the other way.

josephbertino commented 1 year ago

It looks like this bug has since been fixed by commit 890b9e8.

Recommending closing this issue.

wisp3rwind commented 1 year ago

It looks like this bug has since been fixed by commit 890b9e8.

Recommending closing this issue.

Pretty sure that this is not the case: The order of events is exactly reversed to what you imply, and a brief look at the code suggests that this specific issues is still present.

josephbertino commented 1 year ago

@wisp3rwind The commit I mentioned succeeds in converting int-type perm values to string, after which it re-converts perm to int with base 8. With this commit (and future ones that improve upon it, like 1ec87a3), users can specify 0-leading octal values in the YAML file by wrapping the values in quotes. How is this not the context of both the issue originator and follow-up by @sampsyo ?

sampsyo commented 1 year ago

Hello! This is what the current status looks like to me: