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

Dependent rules issues #11

Open
devclinton opened this issue Sep 22, 2020 · 4 comments
Open

Dependent rules issues #11

devclinton opened this issue Sep 22, 2020 · 4 comments
Assignees

Comments

@devclinton
Copy link

I have encountered two issues with rule dependencies

The first is with comment after a dependency

depa:
	@python -c "print('A')"

all: depa # comment after dependency
	@python -c "print('TEST-PASS')"

The above will fail when running pymake all with

(venv) C:\Dev\idmtools\test_makefile>pymake all
depa # comment after dependency
Traceback (most recent call last):
  File "C:\Python\Python38\lib\runpy.py", line 194, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "C:\Python\Python38\lib\runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "C:\Dev\idmtools\venv\Scripts\pymake.exe\__main__.py", line 7, in <module>
  File "c:\dev\idmtools\venv\lib\site-packages\pymake\_main.py", line 60, in main
    execute_makefile_commands(commands, target,
  File "c:\dev\idmtools\venv\lib\site-packages\pymake\_pymake.py", line 143, in execute_makefile_commands
    check_call(parsed_cmd)
  File "C:\Python\Python38\lib\subprocess.py", line 359, in check_call
    retcode = call(*popenargs, **kwargs)
  File "C:\Python\Python38\lib\subprocess.py", line 340, in call
    with Popen(*popenargs, **kwargs) as p:
  File "C:\Python\Python38\lib\subprocess.py", line 854, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "C:\Python\Python38\lib\subprocess.py", line 1307, in _execute_child
    hp, ht, pid, tid = _winapi.CreateProcess(executable, args,
FileNotFoundError: [WinError 2] The system cannot find the file specified

You can observe it is attempting to run the command depa # comment after dependency

The other related issue(I think) it multiple dependencies

depb:
	@python -c "print('A')"

depa:
	@python -c "print('A')"

all: depa depb
	@python -c "print('TEST-PASS')"

which fails with

(venv) C:\Dev\idmtools\test_makefile>pymake all
depa depb
Traceback (most recent call last):
  File "C:\Python\Python38\lib\runpy.py", line 194, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "C:\Python\Python38\lib\runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "C:\Dev\idmtools\venv\Scripts\pymake.exe\__main__.py", line 7, in <module>
  File "c:\dev\idmtools\venv\lib\site-packages\pymake\_main.py", line 60, in main
    execute_makefile_commands(commands, target,
  File "c:\dev\idmtools\venv\lib\site-packages\pymake\_pymake.py", line 143, in execute_makefile_commands
    check_call(parsed_cmd)
  File "C:\Python\Python38\lib\subprocess.py", line 359, in check_call
    retcode = call(*popenargs, **kwargs)
  File "C:\Python\Python38\lib\subprocess.py", line 340, in call
    with Popen(*popenargs, **kwargs) as p:
  File "C:\Python\Python38\lib\subprocess.py", line 854, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "C:\Python\Python38\lib\subprocess.py", line 1307, in _execute_child
    hp, ht, pid, tid = _winapi.CreateProcess(executable, args,
FileNotFoundError: [WinError 2] The system cannot find the file specified

I suspect there needs to be some parsing happening on the dependencies portion of rules before execution

@devclinton
Copy link
Author

devclinton commented Sep 22, 2020

Note I can make the second example work by changing it to

depb:
	@python -c "print('A')"

depa:
	@python -c "print('A')"

all: pymake depa depb
	@python -c "print('TEST-PASS')"

I think to make things more compatible with Make would be to parse the other rules as dependencies and not require calling pymake

The comment(at least with make dependencies) and the dependency issue could be resolved using the following(possibly hackey) fix

def execute_makefile_commands(
        commands, alias, silent=False, just_print=False, ignore_errors=False):
    """
    Execution Handler

    Parameters
    ----------
    commands  : dict
        Maps each alias to a list of commands.
    alias  : str

    Bool Options (default-false)
    ----------------------------
    silent, just_print, ignore_errors
    """
    cmds = commands[alias]

    if just_print:
        print('\n'.join(cmds))
        return

    for cmd in cmds:
        # Parse string in a shell-like fashion
        # (incl quoted strings and comments)
        parsed_cmd = shlex.split(cmd, comments=True)
        # Execute command if not empty/comment
        if parsed_cmd:
            if parsed_cmd[0] in commands:
                for part in parsed_cmd:
                    if part in commands:
                        execute_makefile_commands(commands, part, silent, just_print, ignore_errors)
            else:
                if not silent:
                    print(cmd)
                # Launch the command and wait to finish (synchronized call)
                try:
                    check_call(parsed_cmd)
                except:
                    if not ignore_errors:
                        raise

@casperdcl casperdcl self-assigned this Sep 23, 2020
@casperdcl
Copy link
Member

casperdcl commented Sep 23, 2020

Actually same-line dependencies aren't currently supported. From the README:

A maximum of one @make alias or command per line

So this is a feature request rather than a bug

@devclinton
Copy link
Author

@casperdcl thanks. I actually have a more robust resolution in the parse of the make file rules. I will open a PR this week for review. I need to verify that the order is preserved in my fix with my test makefiles. Is this a feature that has requested by others beside myself?

I currently use this in a development environment for a project where the development team is split across Windows, Linux, and Mac. I have many rules I have that have chains of dependencies that would be useful for my project. I assume others have the same concern.

@casperdcl
Copy link
Member

casperdcl commented Sep 24, 2020

I think this would be a useful addition, certainly would be included in tqdm's Makefile too, so happy for a PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants