[Errno 13] Permission denied on Windows

[victorlin]

from discussion post

Current Behavior

The error happens with at least the following 3 commands:

1. nextstrain update

   >nextstrain update
   ...
   Traceback (most recent call last):
   File "C:\Users\vlin\Miniconda3\envs\nextstrain\Scripts\nextstrain-script.py", line 9, in <module>
       sys.exit(main())
   File "C:\Users\vlin\Miniconda3\envs\nextstrain\lib\site-packages\nextstrain\cli\__main__.py", line 10, in main
       return cli.run( argv[1:] )
   File "C:\Users\vlin\Miniconda3\envs\nextstrain\lib\site-packages\nextstrain\cli\__init__.py", line 31, in run
       return opts.__command__.run(opts)
   File "C:\Users\vlin\Miniconda3\envs\nextstrain\lib\site-packages\nextstrain\cli\command\update.py", line 25, in run
       statuses = [
   File "C:\Users\vlin\Miniconda3\envs\nextstrain\lib\site-packages\nextstrain\cli\command\update.py", line 26, in <listcomp>
       runner.update()
   File "C:\Users\vlin\Miniconda3\envs\nextstrain\lib\site-packages\nextstrain\cli\runner\docker.py", line 234, in update
       config.set("docker", "image", latest_image)
   File "C:\Users\vlin\Miniconda3\envs\nextstrain\lib\site-packages\nextstrain\cli\config.py", line 105, in set
       save(config, path)
   File "C:\Users\vlin\Miniconda3\envs\nextstrain\lib\site-packages\nextstrain\cli\config.py", line 72, in save
       config.write(file)
   PermissionError: [Errno 13] Permission denied
   

2. nextstrain check-setup --set-default with docker, aws-batch supported

   >nextstrain check-setup --set-default
   ...
   Setting default environment to docker.
   PermissionError: [Errno 13] Permission denied
   
   During handling of the above exception, another exception occurred:
   
   Traceback (most recent call last):
   File "C:\Users\vlin\Miniconda3\envs\nextstrain\Scripts\nextstrain-script.py", line 9, in <module>
       sys.exit(main())
   File "C:\Users\vlin\Miniconda3\envs\nextstrain\lib\site-packages\nextstrain\cli\__main__.py", line 10, in main
       return cli.run( argv[1:] )
   File "C:\Users\vlin\Miniconda3\envs\nextstrain\lib\site-packages\nextstrain\cli\__init__.py", line 31, in run
       return opts.__command__.run(opts)
   File "C:\Users\vlin\Miniconda3\envs\nextstrain\lib\site-packages\nextstrain\cli\command\check_setup.py", line 108, in run
       config.set("core", "runner", supported_runners[0])
   File "C:\Users\vlin\Miniconda3\envs\nextstrain\lib\site-packages\nextstrain\cli\config.py", line 105, in set
       save(config, path)
   File "C:\Users\vlin\Miniconda3\envs\nextstrain\lib\site-packages\nextstrain\cli\config.py", line 72, in save
       config.write(file)
   PermissionError: [Errno 13] Permission denied
   

3. nextstrain check-setup --set-default with aws-batch supported

   >nextstrain check-setup --set-default
   ...
   All good!  Supported Nextstrain environments: aws-batch
   
   Setting default environment to aws-batch.
   PermissionError: [Errno 13] Permission denied
   
   During handling of the above exception, another exception occurred:
   
   Traceback (most recent call last):
   File "C:\Users\vlin\Miniconda3\envs\nextstrain\Scripts\nextstrain-script.py", line 9, in <module>
       sys.exit(main())
   File "C:\Users\vlin\Miniconda3\envs\nextstrain\lib\site-packages\nextstrain\cli\__main__.py", line 10, in main
       return cli.run( argv[1:] )
   File "C:\Users\vlin\Miniconda3\envs\nextstrain\lib\site-packages\nextstrain\cli\__init__.py", line 31, in run
       return opts.__command__.run(opts)
   File "C:\Users\vlin\Miniconda3\envs\nextstrain\lib\site-packages\nextstrain\cli\command\check_setup.py", line 108, in run
       config.set("core", "runner", supported_runners[0])
   File "C:\Users\vlin\Miniconda3\envs\nextstrain\lib\site-packages\nextstrain\cli\config.py", line 105, in set
       save(config, path)
   File "C:\Users\vlin\Miniconda3\envs\nextstrain\lib\site-packages\nextstrain\cli\config.py", line 72, in save
       config.write(file)
   PermissionError: [Errno 13] Permission denied
   

Expected behavior

Normal behavior.

How to reproduce
Steps to reproduce the current behavior:

1. Install Nextstrain using the instructions for Windows OS in install docs.
2. Run any one of the commands above.
3. See error.

Possible solution
(optional)

Your environment: if running Nextstrain locally

• Operating system: Windows 10
• Version: nextstrain-cli 3.0.6

Additional context

I checked on 3.0.3, 3.0.4, 3.0.5, and 3.0.6. The last working version was 3.0.3. So the bug was introduced somewhere here: 3.0.3...3.0.4

Workaround: use WSL on Windows to install Nextstrain.

[tsibley]

I don't have a Windows install to test on myself, but I dug into this a bit based on some hunches. I believe the issue here is the locking introduced in 821c08e.

Specifically, Window's LockFileEx doc says that:

If the locking process opens the file a second time, it cannot access the specified region through this second handle until it unlocks the region.

and I believe that's what's happening since we acquire the lock on the same file we then re-open for writing. This works on Unix because the lock isn't mandatory (OS enforced) unlike Window's LockFileEx. This is noted in fastener's readme, but I didn't realize the full implications of this difference when introducing the locking. (My thinking was enforcement was process-scoped not fd/handle-scoped.)

If the above is indeed the issue, then I see a couple potential solutions:

1. Switch from locking the config file itself (e.g. ~/.nextstrain/config or ~/.nextstrain/secrets) to a standalone lock file (e.g. global ~/.nextstrain/lock or per-file ~/.nextstrain/.config.lock). This is a simple change on the face of it but requires a little care in terms of path handling, whether parent directories exist, permissions, etc. Avoiding that extra overhead is why I chose to acquire a lock on the file itself in the first place, but it's very much doable.

2. Re-use the file handle that's actually locked instead of opening the file again. The .lockfile attribute exists with the actually-locked handle, but it's opened in a+ mode by default, which on Linux prevents overwriting even with seek(0) first. We could remove the O_APPEND flag from the fd with an appropriate fcntl() call, but we'd have to figure out how to do the same on Windows potentially. Alternatively and much simpler, we could override the internal _get_handle() method on the lock class to use r or w (depending on the lock type) instead of a+, but then we'd be monkey-patching.

Both seem doable, but 1 seems better to me.

[tsibley]

@victorlin Can you test if the branch trs/wip/config-locking/global-lockfile resolves this issue for you on Windows?

[victorlin]

@tsibley yes, that branch works!