rice-solar-physics / pydrad

Python tools for setting up HYDRAD runs and parsing output
https://pydrad.readthedocs.io
MIT License
4 stars 3 forks source link

configure.py -- chmod does not work in Windows #139

Closed jwreep closed 3 years ago

jwreep commented 3 years ago

chmod is not a valid command in a Windows shell, so the following snippet fails under Windows:

https://github.com/rice-solar-physics/pydrad/blob/bc1e0e74f02f9d714f36d7412723a1a3c3c4db3d/pydrad/configure/configure.py#L129-L133

C:\ProgramData\Anaconda3\lib\site-packages\pydrad\configure\configure.py in setup_simulation(self, output_path, base_path, run_initial_conditions, **kwargs)
     87             if run_initial_conditions:
     88                 execute = kwargs.get('execute', True)
---> 89                 self.setup_initial_conditions(tmpdir, execute=execute)
     90             self.setup_hydrad(tmpdir)
     91             self.save_config(os.path.join(tmpdir, 'pydrad_config.asdf'))

C:\ProgramData\Anaconda3\lib\site-packages\pydrad\configure\configure.py in setup_initial_conditions(self, root_dir, execute)
    127                 f.write(filestring)
    128         # NOTE: make sure we have needed permissions to run compile script
--> 129         run_shell_command(
    130             ['chmod', 'u+x', 'build_initial_conditions.bat'],
    131             os.path.join(root_dir, 'Initial_Conditions/build_scripts'),

C:\ProgramData\Anaconda3\lib\site-packages\pydrad\configure\util.py in run_shell_command(cmd, cwd, shell)
     22 
     23 def run_shell_command(cmd, cwd, shell=True):
---> 24     cmd = subprocess.run(
     25         cmd,
     26         cwd=cwd,

C:\ProgramData\Anaconda3\lib\subprocess.py in run(input, capture_output, timeout, check, *popenargs, **kwargs)
    487         kwargs['stderr'] = PIPE
    488 
--> 489     with Popen(*popenargs, **kwargs) as process:
    490         try:
    491             stdout, stderr = process.communicate(input, timeout=timeout)

C:\ProgramData\Anaconda3\lib\subprocess.py in __init__(self, args, bufsize, executable, stdin, stdout, stderr, preexec_fn, close_fds, shell, cwd, env, universal_newlines, startupinfo, creationflags, restore_signals, start_new_session, pass_fds, encoding, errors, text)
    852                             encoding=encoding, errors=errors)
    853 
--> 854             self._execute_child(args, executable, preexec_fn, close_fds,
    855                                 pass_fds, cwd, env,
    856                                 startupinfo, creationflags, shell,

C:\ProgramData\Anaconda3\lib\subprocess.py in _execute_child(self, args, executable, preexec_fn, close_fds, pass_fds, cwd, env, startupinfo, creationflags, shell, p2cread, p2cwrite, c2pread, c2pwrite, errread, errwrite, unused_restore_signals, unused_start_new_session)
   1305             # Start the process
   1306             try:
-> 1307                 hp, ht, pid, tid = _winapi.CreateProcess(executable, args,
   1308                                          # no special security
   1309                                          None, None,

FileNotFoundError: [WinError 2] The system cannot find the file specified

Similarly a bit further down: https://github.com/rice-solar-physics/pydrad/blob/bc1e0e74f02f9d714f36d7412723a1a3c3c4db3d/pydrad/configure/configure.py#L203-L207

Likely this needs a check for the operating system.

wtbarnes commented 3 years ago

Fair point. What is the equivalent command for windows? Out of curiosity, does this work in WSL?

sjbradshaw commented 3 years ago

As I recall, it's 'attrib' as in file attributes. However, anything compiled as a .exe under Windows should be automatically executable.Sent via the Samsung Galaxy S7 active, an AT&T 4G LTE smartphone -------- Original message --------From: Will Barnes notifications@github.com Date: 12/18/20 10:42 PM (GMT-06:00) To: rice-solar-physics/pydrad pydrad@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: Re: [rice-solar-physics/pydrad] configure.py -- chmod does not work in Windows (#139) Fair point. What is the equivalent command for windows? Out of curiosity, does this work in WSL?

—You are receiving this because you are subscribed to this thread.Reply to this email directly, view it on GitHub, or unsubscribe.

wtbarnes commented 3 years ago

Ah ok. The chmod is actually for the .bat file, not the .exe.

sjbradshaw commented 3 years ago

Yes, but it shouldn’t be necessary to change the permissions of an .exe file under Windows. It should be executable by default. The attrib command is typically used to make files read only (or undo same), etc.

From: Will Barnes notifications@github.com Sent: Saturday, December 19, 2020 12:53 AM To: rice-solar-physics/pydrad pydrad@noreply.github.com Cc: sjbradshaw stephen.bradshaw@rice.edu; Comment comment@noreply.github.com Subject: Re: [rice-solar-physics/pydrad] configure.py -- chmod does not work in Windows (#139)

Ah ok. The chmod is actually for the .bat file, not the .exe.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rice-solar-physics/pydrad/issues/139#issuecomment-748430305 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ACC6C7UKAFRLRQJAAGFLUQTSVRETDANCNFSM4VBZIOTA . https://github.com/notifications/beacon/ACC6C7V55YP4JCMHF3ZOI2LSVRETDA5CNFSM4VBZIOTKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOFSOCHYI.gif

sjbradshaw commented 3 years ago

Oh, . bat files should be executable by default too. Sorry, just realized what you were referring to. [The other executable is a .com file, just fyi]

From: Stephen Bradshaw stephen.bradshaw@rice.edu Sent: Saturday, December 19, 2020 1:11 AM To: 'rice-solar-physics/pydrad' reply@reply.github.com; 'rice-solar-physics/pydrad' pydrad@noreply.github.com Cc: 'Comment' comment@noreply.github.com Subject: RE: [rice-solar-physics/pydrad] configure.py -- chmod does not work in Windows (#139)

Yes, but it shouldn’t be necessary to change the permissions of an .exe file under Windows. It should be executable by default. The attrib command is typically used to make files read only (or undo same), etc.

From: Will Barnes <notifications@github.com mailto:notifications@github.com > Sent: Saturday, December 19, 2020 12:53 AM To: rice-solar-physics/pydrad <pydrad@noreply.github.com mailto:pydrad@noreply.github.com > Cc: sjbradshaw <stephen.bradshaw@rice.edu mailto:stephen.bradshaw@rice.edu >; Comment <comment@noreply.github.com mailto:comment@noreply.github.com > Subject: Re: [rice-solar-physics/pydrad] configure.py -- chmod does not work in Windows (#139)

Ah ok. The chmod is actually for the .bat file, not the .exe.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rice-solar-physics/pydrad/issues/139#issuecomment-748430305 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ACC6C7UKAFRLRQJAAGFLUQTSVRETDANCNFSM4VBZIOTA . https://github.com/notifications/beacon/ACC6C7V55YP4JCMHF3ZOI2LSVRETDA5CNFSM4VBZIOTKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOFSOCHYI.gif

wtbarnes commented 3 years ago

Maybe it would just be easiest to add a flag to optionally skip the chmod step?

This also definitely makes the case for running tests on multiple operating systems

jwreep commented 3 years ago

A quick check should be sufficient:

import platform
if platform.system() != 'Windows':
     run_shell_command(...)