Skip to content
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

Opening config produces Errno 22 #2847

Open
SuperSandro2000 opened this issue Mar 20, 2018 · 15 comments
Open

Opening config produces Errno 22 #2847

SuperSandro2000 opened this issue Mar 20, 2018 · 15 comments
Labels
bug bugs that are confirmed and actionable good first issue https://github.com/beetbox/beets/pull/5479 windows Relates specifically to Windows OS

Comments

@SuperSandro2000
Copy link

Problem

Trying to open the config file on Windows 10 with beet -vv config -e leads to error: Could not edit configuration: [Errno 22] Invalid argument. Checked the file and it exists and would be open with Notepad++.

Setup

  • OS: Windows 10
  • Python version: Python 3.6 (Python 2.7 is installed to put not used by default)
  • beets version: 1.4.6
  • Turning off plugins made problem go away (yes/no): don't have any

My configuration (output of beet config) is:

{}
@sampsyo
Copy link
Member

sampsyo commented Mar 20, 2018

Please include the complete output from the command.

@sampsyo sampsyo added the needinfo We need more details or follow-up from the filer before this can be tagged "bug" or "feature." label Mar 20, 2018
@SuperSandro2000
Copy link
Author

This is the complete command output.

C:\Users\Sandro>beet -vv config -e
error: Could not edit configuration: [Errno 22] Invalid argument

C:\Users\Sandro>

@sampsyo
Copy link
Member

sampsyo commented Mar 20, 2018

That's odd! I would have expected more output from setting up beets before the crash.

Have you tried setting your %EDITOR% environment variable to a specific command? That's what beets tries to use to open the config file, by default. Otherwise, it tries to use the Windows start command. So maybe it's worth trying to run start C:\path\to\file.yaml to see if that works.

@SuperSandro2000
Copy link
Author

%EDITOR% sadly does not exist by default.
start C:\Users\Sandro\AppData\Roaming\beets\config.yaml opens up notepad++ how it should.

@sampsyo
Copy link
Member

sampsyo commented Mar 21, 2018

Strange! I'm not sure what to suggest because I don't have a Windows machine to use to reproduce the problem. Would you be willing to dig into the code to help see where things are going wrong? The editor_command function in beets.util would be a good place to start.

@SuperSandro2000
Copy link
Author

SuperSandro2000 commented Mar 21, 2018

I don't get the reason for that line https://github.com/beetbox/beets/blob/master/beets/util/__init__.py#L911

With some print statements we see that the editor command is duped:

    # Split the command string into its arguments.
    try:
        args = shlex_split(command)
        print(args)
    except ValueError:  # Malformed shell tokens.
        args = [command]
    print(args)
    args.insert(0, args[0])  # for argv[0]
    print(args)
    args += targets

Output:

['start']
['start', 'start']

I would remove it if it is not needed for Linux. args should't contain two starts as it will spawn an unnecessary console on Windows.


To fix the actual issue I switched to os.system() for Windows as a new process needs to be called anyway and invoke the editor command on each file in targets. I have no real idea if that works on Unix systems correctly as I can't easily test it right now. If it needs to be changed I am more than willing to change it.

Did a pull #2851 to fix this issue.

@jackwilsdon
Copy link
Member

jackwilsdon commented Mar 22, 2018

The args.insert(0, args[0]) is needed for *nix to ensure that the process gets it's own name correctly. The correct fix here is to probably to just not add that on Windows.

@SuperSandro2000
Copy link
Author

SuperSandro2000 commented Mar 22, 2018

I did change the PR to reflect that but on windows os.system would be a better choice and most windows editors don't support opening multiple files in one command anyway.
#2851

@sampsyo
Copy link
Member

sampsyo commented Mar 22, 2018

Care to expand on why os.system would be a better choice? I presume you mean because Windows doesn't really support exec-like functionality, so we're getting a subprocess anyway. Is that right?

@SuperSandro2000
Copy link
Author

SuperSandro2000 commented Mar 22, 2018

You're right and why appveyor fails, I have no plan.

@MinchinWeb
Copy link

cf #2717

@sampsyo
Copy link
Member

sampsyo commented Apr 4, 2018

Wow; thanks for the note, @MinchinWeb—it's amazing how quickly I can forget about comments I wrote. 😳

@MinchinWeb
Copy link

@sampsyo Happy to lend a new pair of eyes. I started poking around through the issues because I ran into this myself. Hope it can be fixed soon!

@arcresu arcresu added the windows Relates specifically to Windows OS label Apr 27, 2019
@SuperSandro2000 SuperSandro2000 mentioned this issue Jul 10, 2020
3 tasks
@stale
Copy link

stale bot commented Jul 11, 2020

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jul 11, 2020
@SuperSandro2000
Copy link
Author

/unstale

@stale stale bot removed the stale label Jul 12, 2020
@jtpavlock jtpavlock added bug bugs that are confirmed and actionable good first issue https://github.com/beetbox/beets/pull/5479 and removed needinfo We need more details or follow-up from the filer before this can be tagged "bug" or "feature." labels Jul 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug bugs that are confirmed and actionable good first issue https://github.com/beetbox/beets/pull/5479 windows Relates specifically to Windows OS
Projects
None yet
6 participants