Merge pull request #615 from mlcommons/dev

Merge dev

Author: arjunsuresh

.github/workflows/build_wheel_off.yml

@@ -1,6 +1,5 @@
 name: Build wheel and release into PYPI (off now)
 
-
 on: 
   push:
     branches:

.github/workflows/test-mlperf-inference-tvm-resnet50.yml

@@ -29,7 +29,7 @@ jobs:
         pip install mlcflow
     - name: Pull MLOps repository
       env:
-        REPO: ${{ github.event.pull_request.head.repo.html_url }
+        REPO: ${{ github.event.pull_request.head.repo.html_url }}
         BRANCH: ${{ github.event.pull_request.head.ref }}
       run: |
         mlc pull repo "$REPO" --branch="$BRANCH"

VERSION

@@ -1 +1 @@
-1.0.2
+1.1.0

automation/script/docker.py

@@ -55,6 +55,10 @@ def dockerfile(self_module, input_params):
             'script_variation_tags': variation_tags
         }
         docker_settings = metadata.get('docker', {})
+        docker_settings_default_env = docker_settings.get('default_env', {})
+        for key in docker_settings_default_env:
+            env.setdefault(key, docker_settings_default_env[key])
+
         state_data['docker'] = docker_settings
         add_deps_recursive = input_params.get('add_deps_recursive', {})
 
@@ -93,6 +97,11 @@ def dockerfile(self_module, input_params):
         # Set Docker-specific configurations
         docker_settings = state_data['docker']
 
+        if is_true(docker_settings.get('pass_docker_to_script', False)):
+            input_params['docker'] = True
+            r = self_module.run(input_params)
+            return r
+
         if not docker_settings.get('run', True) and not input_params.get(
                 'docker_run_override', False):
             logger.info("Docker 'run' is set to False in meta.json")
@@ -245,12 +254,6 @@ def docker_run(self_module, i):
     r = prune_input({'input': i, 'extra_keys_starts_with': ['docker_']})
     f_run_cmd = r['new_input']
 
-    # Regenerate Dockerfile if required
-    if regenerate_docker_file:
-        r = dockerfile(self_module, i)
-        if r['return'] > 0:
-            return r
-
     # Save current directory and prepare to search for scripts
     cur_dir = os.getcwd()
     r = self_module.search(i.copy())
@@ -276,11 +279,7 @@ def docker_run(self_module, i):
     image_repo = i.get('docker_image_repo', '')
     add_deps_recursive = i.get('add_deps_recursive', {})
 
-    # Ensure Docker is available
-    r = self_module.action_object.access(
-        {'action': 'run', 'automation': 'script', 'tags': "get,docker"})
-    if r['return'] > 0:
-        return r
+    input_i = copy.deepcopy(i)
 
     # Process each artifact
     for artifact in sorted(lst, key=lambda x: x.meta.get('alias', '')):
@@ -301,6 +300,10 @@ def docker_run(self_module, i):
         folder_path_env_keys = meta.get('folder_path_env_keys', [])
 
         docker_settings = meta.get('docker', {})
+        docker_settings_default_env = docker_settings.get('default_env', {})
+        for key in docker_settings_default_env:
+            env.setdefault(key, docker_settings_default_env[key])
+
         state['docker'] = docker_settings
         run_state = {
             'deps': [], 'fake_deps': [], 'parent': None,
@@ -360,6 +363,24 @@ def docker_run(self_module, i):
             logger.info("docker.run set to False in meta.yaml")
             continue
 
+        if is_true(docker_settings.get('pass_docker_to_script', False)):
+            logger.info("Docker 'run' is passed to the run script")
+            i['docker'] = True
+            r = self_module.run(i)
+            return r
+
+        # Regenerate Dockerfile if required
+        if regenerate_docker_file:
+            r = dockerfile(self_module, input_i)
+            if r['return'] > 0:
+                return r
+
+        # Ensure Docker is available
+        r = self_module.action_object.access(
+            {'action': 'run', 'automation': 'script', 'tags': "get,docker"})
+        if r['return'] > 0:
+            return r
+
         r = self_module._update_env_from_input(env, i)
         if r['return'] > 0:
             return r

automation/script/docker_utils.py

@@ -215,14 +215,14 @@ def update_docker_environment(
     # Add host group ID if specified in the Docker settings and not on Windows
     if not is_false(docker_settings.get('pass_group_id')) and os.name != 'nt':
         env['+ MLC_DOCKER_BUILD_ARGS'].append(
-            f"GID=\\\" $(id -g $USER) \\\""
+            f"GID=\" $(id -g $USER) \""
         )
 
     # Add host user ID if specified in the Docker settings and not on Windows
     if not is_false(docker_settings.get(
             'use_host_user_id')) and os.name != 'nt':
         env['+ MLC_DOCKER_BUILD_ARGS'].append(
-            f"UID=\\\" $(id -u $USER) \\\""
+            f"UID=\" $(id -u $USER) \""
         )
 
     return {'return': 0}
@@ -285,7 +285,8 @@ def regenerate_script_cmd(i):
             # Check if the value is a string containing the specified paths
             if isinstance(value, str) and (
                     os.path.join("local", "cache", "") in value or
-                    os.path.join("MLC", "repos", "") in value
+                    os.path.join("MLC", "repos", "") in value or
+                    "<<<" in value
             ):
                 del env[key]
 
@@ -296,7 +297,8 @@ def regenerate_script_cmd(i):
                     val for val in value
                     if isinstance(val, str) and (
                         os.path.join("local", "cache", "") in val or
-                        os.path.join("MLC", "repos", "") in val
+                        os.path.join("MLC", "repos", "") in val or
+                        "<<<" in value
                     )
                 ]
 
@@ -349,7 +351,7 @@ def rebuild_flags(
                 continue
 
             value = command_dict[key]
-            quote = '\\"' if full_key in quote_keys else ""
+            quote = '"' if full_key in quote_keys else ""
 
             # Recursively process nested dictionaries.
             if isinstance(value, dict):
@@ -363,14 +365,15 @@ def rebuild_flags(
             # Process lists by concatenating values with commas.
             elif isinstance(value, list):
                 list_values = ",".join(
-                    f"{quote}{str(item)}{quote}" for item in value)
+                    quote_if_needed(
+                        item, quote) for item in value)
                 command_line += f" --{full_key},={list_values}"
             # Process scalar values.
             else:
                 if full_key in ['s', 'v']:
                     command_line += f" -{full_key}"
                 else:
-                    command_line += f" --{full_key}={quote}{str(value)}{quote}"
+                    command_line += f" --{full_key}={quote_if_needed(value, quote)}"
 
         return command_line
 

automation/script/module.py

@@ -504,6 +504,9 @@ def _run(self, i):
                 env[key] = os.environ[key]
 
         r = self._update_env_from_input(env, i)
+        if env.get('MLC_OUTDIRNAME', '') != '':
+            if not os.path.isabs(env['MLC_OUTDIRNAME']):
+                env['MLC_OUTDIRNAME'] = os.path.abspath(env['MLC_OUTDIRNAME'])
 
         #######################################################################
         # Check if we want to skip cache (either by skip_cache or by fake_run)
@@ -796,6 +799,7 @@ def _run(self, i):
         run_state['script_repo_git'] = script_item.repo.meta.get(
             'git', False)
         run_state['cache'] = meta.get('cache', False)
+        run_state['cache_expiration'] = meta.get('cache_expiration', False)
 
         if not recursion:
             run_state['script_entry_repo_to_report_errors'] = meta.get(
@@ -1748,12 +1752,7 @@ def _run(self, i):
 
             tmp_curdir = os.getcwd()
             if env.get('MLC_OUTDIRNAME', '') != '':
-                if os.path.isabs(env['MLC_OUTDIRNAME']) or recursion:
-                    c_outdirname = env['MLC_OUTDIRNAME']
-                else:
-                    c_outdirname = os.path.join(
-                        env['MLC_TMP_CURRENT_PATH'], env['MLC_OUTDIRNAME'])
-                    env['MLC_OUTDIRNAME'] = c_outdirname
+                c_outdirname = env['MLC_OUTDIRNAME']
 
                 if not fake_run:  # prevent permission error inside docker runs
                     if not os.path.exists(c_outdirname):
@@ -2041,6 +2040,10 @@ def _run(self, i):
                                 cached_path, dependent_cached_path):
                             cached_meta['dependent_cached_path'] = dependent_cached_path
 
+                if run_state.get('cache_expiration'):  # convert to seconds
+                    cached_meta['cache_expiration'] = parse_expiration(
+                        run_state['cache_expiration'])
+
                 ii = {'action': 'update',
                       'target': 'cache',
                       'uid': cached_uid,
@@ -3151,9 +3154,8 @@ def native_run(self, i):
                 if os.name == 'nt':
                     script.append('set ' + k + '=' + v)
                 else:
-                    if ' ' in v:
-                        v = '"' + v + '"'
-                    script.append('export ' + k + '=' + v)
+                    safe_v = quote_if_needed(v)
+                    script.append('export ' + k + '=' + safe_v)
 
             script.append('')
 
@@ -4836,13 +4838,19 @@ def find_cached_script(i):
 
         for cached_script in found_cached_scripts:
             skip_cached_script = False
+            dependent_paths = []
             dependent_cached_path = cached_script.meta.get(
-                'dependent_cached_path', '')
+                'dependent_cached_path')
             if dependent_cached_path:
+                dependent_paths.append(dependent_cached_path)
+            dependent_cached_paths = cached_script.meta.get(
+                'dependent_cached_paths', '').split(':')
+            dependent_paths += [p for p in dependent_cached_paths if p]
+            for dep in dependent_paths:
                 if not os.path.exists(dependent_cached_path):
                     # TODO Need to restrict the below check to within container
                     # env
-                    i['tmp_dep_cached_path'] = dependent_cached_path
+                    i['tmp_dep_cached_path'] = dep
                     from script import docker_utils
                     r = docker_utils.get_container_path_script(i)
                     if not os.path.exists(r['value_env']):
@@ -4851,7 +4859,10 @@ def find_cached_script(i):
                             recursion_spaces +
                             '  - Skipping cached entry as the dependent path {} is missing!'.format(r['value_env']))
                         skip_cached_script = True
-                        continue
+                        break
+
+            if skip_cached_script:
+                continue
 
             os_info = self_obj.os_info
 
@@ -5047,7 +5058,7 @@ def update_env_with_values(env, fail_on_not_found=False, extra_env=None):
         # No placeholders found
         if not placeholders:
             # Special handling for MLC_GIT_URL
-            if key == 'MLC_GIT_URL' and env.get('MLC_GIT_AUTH', "no") == "yes":
+            if key == 'MLC_GIT_URL' and is_true(env.get('MLC_GIT_AUTH')):
                 if env.get('MLC_GH_TOKEN',
                            '') and '@' not in env['MLC_GIT_URL']:
                     params = {"token": env['MLC_GH_TOKEN']}
@@ -5287,7 +5298,7 @@ def prepare_and_run_script_with_postprocessing(i, postprocess="postprocess"):
         if r['return'] > 0:
             return r
 
-        # Save file to run without CM
+        # Save file to run without MLC
         if debug_script_tags != '' and all(
                 item in found_script_tags for item in debug_script_tags.split(',')):
 
@@ -5593,10 +5604,12 @@ def convert_env_to_script(env, os_info, start_script=None):
                 os_info['env_var'].replace(
                     'env_var', key)}"""
 
+        env_quote = os_info['env_quote']
         # Replace placeholders in the platform-specific environment command
+        # and escapes any quote in the env value
         env_command = os_info['set_env'].replace(
             '${key}', key).replace(
-            '${value}', str(env_value))
+            '${value}', str(env_value).replace(env_quote, f"""\\{env_quote}"""))
         script.append(env_command)
 
     return script
@@ -5880,6 +5893,9 @@ def update_state_from_meta(meta, env, state, const, const_state, deps, post_deps
     if meta.get('cache', '') != '':
         run_state['cache'] = meta['cache']
 
+    if meta.get('cache_expiration', '') != '':
+        run_state['cache_expiration'] = meta['cache_expiration']
+
     default_env = meta.get('default_env', {})
     for key in default_env:
         env.setdefault(key, default_env[key])

automation/utils.py

@@ -45,6 +45,7 @@ def get_host_os_info(i={}):
         info['bat_ext'] = '.bat'
         info['set_env'] = 'set ${key}=${value}'
         info['env_separator'] = ';'
+        info['env_quote'] = '\''
         info['env_var'] = '%env_var%'
         info['bat_rem'] = 'rem ${rem}'
         info['run_local_bat'] = 'call ${bat_file}'
@@ -63,6 +64,7 @@ def get_host_os_info(i={}):
         info['bat_ext'] = '.sh'
         info['set_env'] = 'export ${key}="${value}"'
         info['env_separator'] = ':'
+        info['env_quote'] = '"'
         info['env_var'] = '${env_var}'
         info['set_exec_file'] = 'chmod 755 "${file_name}"'
         info['bat_rem'] = '# ${rem}'
@@ -1101,3 +1103,44 @@ def print_json(i):
     print(json.dumps(meta, indent=2))
 
     return {'return': 0}
+
+
+def parse_expiration(user_input: str) -> float:
+    import time
+    """
+    Parse user input like '10m', '2h', '3d' into a UNIX timestamp.
+    """
+    units = {
+        'm': 60,             # minutes
+        'h': 3600,           # hours
+        'd': 86400,          # days
+    }
+
+    if not user_input:
+        raise ValueError("Expiration time cannot be empty")
+
+    unit = user_input[-1].lower()
+    if unit not in units:
+        raise ValueError(f"Unknown unit '{unit}', use m/h/d")
+
+    try:
+        value = int(user_input[:-1])
+    except ValueError:
+        raise ValueError(f"Invalid number in '{user_input}'")
+
+    seconds = value * units[unit]
+    return time.time() + seconds
+
+
+def quote_if_needed(val: str, quote: str) -> str:
+    """
+    Return the value, quoted with escaped quotes if it contains spaces
+    or quotes.
+    """
+    s = str(val)
+
+    if " " in s or '"' in s or quote == '"':
+        # Escape all existing double quotes
+        s = s.replace('"', r'\"')
+        return f'"{s}"'
+    return s