cylc · oliver-sanders · May 18, 2021 · Apr 21, 2021 · Apr 21, 2021 · Apr 23, 2021
diff --git a/CHANGES.md b/CHANGES.md
@@ -69,6 +69,11 @@ version control information on installation of a workflow.
 
 ### Fixes
 
+[#4193](https://github.com/cylc/cylc-flow/pull/4193) - Standard `cylc install`
+now correctly installs from directories with a `.` in the name. Symlink dirs
+now correctly expands environment variables on the remote. Fixes minor cosmetic
+bugs.
+
 [#4199](https://github.com/cylc/cylc-flow/pull/4199) -
 `cylc validate` and `cylc run` now check task/family names in the `[runtime]`
 section for validity.
@@ -78,7 +83,6 @@ a workflow that uses the deprecated `suite.rc` filename would symlink `flow.cylc
 to the `suite.rc` in the source dir instead of the run dir. Also fixes a couple
 of other, small bugs.
 
-
 -------------------------------------------------------------------------------
 ## __cylc-8.0b1 (<span actions:bind='release-date'>Released 2021-04-21</span>)__
 

diff --git a/cylc/flow/pathutil.py b/cylc/flow/pathutil.py
@@ -153,30 +153,31 @@ def make_localhost_symlinks(rund, named_sub_dir):
                 f'Unable to create symlink to {src}.'
                 f' \'{value}\' contains an invalid environment variable.'
                 ' Please check configuration.')
-        make_symlink(src, dst)
+        symlink_success = make_symlink(src, dst)
         # symlink info returned for logging purposes, symlinks created
         # before logs as this dir may be a symlink.
-        symlinks_created[src] = dst
+        if symlink_success:
+            symlinks_created[src] = dst
     return symlinks_created
 
 
 def get_dirs_to_symlink(install_target, flow_name):
-    """Returns dictionary of directories to symlink from glbcfg."""
+    """Returns dictionary of directories to symlink from glbcfg.
+       Note the paths should remain unexpanded, to be expanded on the remote.
+    """
     dirs_to_symlink = {}
     symlink_conf = glbl_cfg().get(['symlink dirs'])
 
     if install_target not in symlink_conf.keys():
         return dirs_to_symlink
     base_dir = symlink_conf[install_target]['run']
     if base_dir is not None:
-        dirs_to_symlink['run'] = os.path.join(
-            expand_path(base_dir), 'cylc-run', flow_name)
+        dirs_to_symlink['run'] = os.path.join(base_dir, 'cylc-run', flow_name)
     for dir_ in ['log', 'share', 'share/cycle', 'work']:
         link = symlink_conf[install_target][dir_]
         if link is None or link == base_dir:
             continue
-        dirs_to_symlink[dir_] = os.path.join(
-            expand_path(link), 'cylc-run', flow_name, dir_)
+        dirs_to_symlink[dir_] = os.path.join(link, 'cylc-run', flow_name, dir_)
     return dirs_to_symlink
 
 
@@ -189,10 +190,12 @@ def make_symlink(src, dst):
     if os.path.exists(dst):
         if os.path.islink(dst) and os.path.samefile(dst, src):
             # correct symlink already exists
-            return
+            return False
         # symlink name is in use by a physical file or directory
-        raise WorkflowFilesError(
-            f"Error when symlinking. The path {dst} already exists.")
+        # log and return
+        LOG.debug(
+            f"Unable to create {src} symlink. The path {dst} already exists.")
+        return False
     elif os.path.islink(dst):
         # remove a bad symlink.
         try:
@@ -204,6 +207,7 @@ def make_symlink(src, dst):
     os.makedirs(os.path.dirname(dst), exist_ok=True)
     try:
         os.symlink(src, dst, target_is_directory=True)
+        return True
     except Exception as exc:
         raise WorkflowFilesError(f"Error when symlinking\n{exc}")
 

diff --git a/cylc/flow/scripts/install.py b/cylc/flow/scripts/install.py
@@ -23,8 +23,8 @@
 The workflow can then be started, stopped, and targeted by name.
 
 Normal installation creates a directory "~/cylc-run/REG/", with a run
-directory "~/cylc-run/REG/run1" containing a "_cylc-install/source" symlink to
-the source directory.
+directory "~/cylc-run/REG/run1". A "_cylc-install/source" symlink to the source
+directory will be created in the REG directory.
 Any files or directories (excluding .git, .svn) from the source directory are
 copied to the new run directory.
 A ".service" directory will also be created and used for server authentication

diff --git a/cylc/flow/workflow_files.py b/cylc/flow/workflow_files.py
@@ -523,19 +523,20 @@ def parse_workflow_arg(options, arg):
 
 
 def register(
-    flow_name: Optional[str] = None, source: Optional[str] = None
+    flow_name: str, source: Optional[str] = None
 ) -> str:
     """Set up workflow.
     This completes some of the set up completed by cylc install.
-    Called only if running workflow that has not been installed.
+    Called only if running a workflow that has not been installed.
 
     Validates workflow name.
     Validates run directory structure.
+    Creates symlinks for localhost symlink dirs.
     Symlinks flow.cylc -> suite.rc.
     Creates the .service directory.
 
     Args:
-        flow_name: workflow name, default basename($PWD).
+        flow_name: workflow name.
         source: directory location of flow.cylc file, default $PWD.
 
     Return:
@@ -547,8 +548,6 @@ def register(
            - Illegal name (can look like a relative path, but not absolute).
            - Nested workflow run directories.
     """
-    if flow_name is None:
-        flow_name = Path.cwd().stem
     validate_flow_name(flow_name)
     if source is not None:
         if os.path.basename(source) == WorkflowFiles.FLOW_FILE:
@@ -1080,8 +1079,8 @@ def reinstall_workflow(named_run, rundir, source, dry_run=False):
             f"An error occurred when copying files from {source} to {rundir}")
         reinstall_log.warning(f" Error: {stderr}")
     check_flow_file(rundir, symlink_suiterc=True, logger=reinstall_log)
-    reinstall_log.info(f'REINSTALLED {named_run} from {source} -> {rundir}')
-    print(f'REINSTALLED {named_run} from {source} -> {rundir}')
+    reinstall_log.info(f'REINSTALLED {named_run} from {source}')
+    print(f'REINSTALLED {named_run} from {source}')
     _close_install_log(reinstall_log)
     return
 
@@ -1121,14 +1120,13 @@ def install_workflow(
             Another workflow already has this name (unless --redirect).
             Trying to install a workflow that is nested inside of another.
     """
-
     if not source:
         source = Path.cwd()
     elif Path(source).name == WorkflowFiles.FLOW_FILE:
         source = Path(source).parent
     source = Path(expand_path(source))
     if not flow_name:
-        flow_name = source.stem
+        flow_name = source.name
     validate_flow_name(flow_name)
     if run_name in WorkflowFiles.RESERVED_NAMES:
         raise WorkflowFilesError(f'Run name cannot be "{run_name}".')
@@ -1143,11 +1141,13 @@ def install_workflow(
             " name, using the --run-name option.")
     check_nested_run_dirs(rundir, flow_name)
     symlinks_created = {}
+    named_run = flow_name
+    if run_name:
+        named_run = os.path.join(named_run, run_name)
+    elif run_num:
+        named_run = os.path.join(named_run, f'run{run_num}')
     if not no_symlinks:
-        sub_dir = flow_name
-        if run_num:
-            sub_dir = os.path.join(sub_dir, f'run{run_num}')
-        symlinks_created = make_localhost_symlinks(rundir, sub_dir)
+        symlinks_created = make_localhost_symlinks(rundir, named_run)
     install_log = _get_logger(rundir, 'cylc-install')
     if not no_symlinks and bool(symlinks_created) is True:
         for src, dst in symlinks_created.items():
@@ -1175,19 +1175,21 @@ def install_workflow(
     if no_run_name:
         cylc_install = Path(rundir, WorkflowFiles.Install.DIRNAME)
     source_link = cylc_install.joinpath(WorkflowFiles.Install.SOURCE)
+    # check source link matches the source symlink from workflow dir.
     cylc_install.mkdir(parents=True, exist_ok=True)
     if not source_link.exists():
         install_log.info(f"Creating symlink from {source_link}")
         source_link.symlink_to(source)
-    elif source_link.exists() and (os.readlink(source_link) == str(source)):
+    elif source_link.exists() and (
+        source_link.resolve() == source.resolve()
+    ):
         install_log.info(
             f"Symlink from \"{source_link}\" to \"{source}\" in place.")
     else:
         raise WorkflowFilesError(
             "Source directory between runs are not consistent.")
-    # check source link matches the source symlink from workflow dir.
-    install_log.info(f'INSTALLED {flow_name} from {source} -> {rundir}')
-    print(f'INSTALLED {flow_name} from {source} -> {rundir}')
+    install_log.info(f'INSTALLED {named_run} from {source}')
+    print(f'INSTALLED {named_run} from {source}')
     _close_install_log(install_log)
     return source, rundir, flow_name
 
@@ -1296,7 +1298,7 @@ def check_flow_file(
         if flow_file_path.is_symlink():
             # Symlink broken or points elsewhere - replace
             flow_file_path.unlink()
-        flow_file_path.symlink_to(suite_rc_path)
+        flow_file_path.symlink_to(WorkflowFiles.SUITE_RC)
         if logger:
             logger.warning(f'{depr_msg}. Symlink created.')
         return flow_file_path
@@ -1352,10 +1354,10 @@ def unlink_runN(path: Union[Path, str]) -> bool:
 
 def link_runN(latest_run: Union[Path, str]):
     """Create symlink runN, pointing at the latest run"""
-    latest_run = Path(latest_run).expanduser()
+    latest_run = Path(latest_run)
     run_n = Path(latest_run.parent, WorkflowFiles.RUN_N)
     try:
-        run_n.symlink_to(latest_run)
+        run_n.symlink_to(latest_run.name)
     except OSError:
         pass
 

diff --git a/tests/functional/cylc-install/00-simple.t b/tests/functional/cylc-install/00-simple.t
@@ -34,7 +34,7 @@ pushd "${RND_WORKFLOW_SOURCE}" || exit 1
 run_ok "${TEST_NAME}" cylc install
 
 contains_ok "${TEST_NAME}.stdout" <<__OUT__
-INSTALLED $RND_WORKFLOW_NAME from ${RND_WORKFLOW_SOURCE} -> ${RND_WORKFLOW_RUNDIR}/run1
+INSTALLED $RND_WORKFLOW_NAME/run1 from ${RND_WORKFLOW_SOURCE}
 __OUT__
 popd || exit 1
 purge_rnd_workflow
@@ -59,7 +59,7 @@ __ERR__
 TEST_NAME="${TEST_NAME_BASE}-REG-install-ok"
 run_ok "${TEST_NAME}" cylc install "${RND_WORKFLOW_NAME}"
 contains_ok "${TEST_NAME}.stdout" <<__OUT__
-INSTALLED $RND_WORKFLOW_NAME from ${RND_WORKFLOW_SOURCE} -> ${RND_WORKFLOW_RUNDIR}/run1
+INSTALLED $RND_WORKFLOW_NAME/run1 from ${RND_WORKFLOW_SOURCE}
 __OUT__
 popd || exit 1
 purge_rnd_workflow
@@ -74,7 +74,7 @@ touch "${RND_WORKFLOW_SOURCE}/suite.rc"
 run_ok "${TEST_NAME}" cylc install --flow-name="${RND_WORKFLOW_NAME}" -C "${RND_WORKFLOW_SOURCE}"
 
 contains_ok "${TEST_NAME}.stdout" <<__OUT__
-INSTALLED $RND_WORKFLOW_NAME from ${RND_WORKFLOW_SOURCE} -> ${RND_WORKFLOW_RUNDIR}/run1
+INSTALLED $RND_WORKFLOW_NAME/run1 from ${RND_WORKFLOW_SOURCE}
 __OUT__
 # test symlink not made in source dir
 exists_fail "flow.cylc"
@@ -84,7 +84,7 @@ exists_ok "flow.cylc"
 
 TEST_NAME="${TEST_NAME_BASE}-suite.rc-flow.cylc-readlink"
 readlink "flow.cylc" > "${TEST_NAME}.out"
-cmp_ok "${TEST_NAME}.out" <<< "${RND_WORKFLOW_RUNDIR}/run1/suite.rc"
+cmp_ok "${TEST_NAME}.out" <<< "suite.rc"
 
 INSTALL_LOG="$(find "${RND_WORKFLOW_RUNDIR}/run1/log/install" -type f -name '*.log')"
 grep_ok "The filename \"suite.rc\" is deprecated in favour of \"flow.cylc\". Symlink created." "${INSTALL_LOG}"
@@ -99,7 +99,7 @@ make_rnd_workflow
 pushd "${RND_WORKFLOW_SOURCE}" || exit 1
 run_ok "${TEST_NAME}" cylc install --no-run-name
 contains_ok "${TEST_NAME}.stdout" <<__OUT__
-INSTALLED $RND_WORKFLOW_NAME from ${RND_WORKFLOW_SOURCE} -> ${RND_WORKFLOW_RUNDIR}
+INSTALLED $RND_WORKFLOW_NAME from ${RND_WORKFLOW_SOURCE}
 __OUT__
 popd || exit 1
 purge_rnd_workflow
@@ -111,7 +111,7 @@ make_rnd_workflow
 pushd "${RND_WORKFLOW_SOURCE}" || exit 1
 run_ok "${TEST_NAME}" cylc install --flow-name="${RND_WORKFLOW_NAME}-olaf"
 contains_ok "${TEST_NAME}.stdout" <<__OUT__
-INSTALLED ${RND_WORKFLOW_NAME}-olaf from ${RND_WORKFLOW_SOURCE} -> ${RUN_DIR}/${RND_WORKFLOW_NAME}-olaf/run1
+INSTALLED ${RND_WORKFLOW_NAME}-olaf/run1 from ${RND_WORKFLOW_SOURCE}
 __OUT__
 popd || exit 1
 rm -rf "${RUN_DIR}/${RND_WORKFLOW_NAME}-olaf"
@@ -124,7 +124,7 @@ make_rnd_workflow
 pushd "${RND_WORKFLOW_SOURCE}" || exit 1
 run_ok "${TEST_NAME}" cylc install --flow-name="${RND_WORKFLOW_NAME}-olaf" --no-run-name
 contains_ok "${TEST_NAME}.stdout" <<__OUT__
-INSTALLED ${RND_WORKFLOW_NAME}-olaf from ${RND_WORKFLOW_SOURCE} -> ${RUN_DIR}/${RND_WORKFLOW_NAME}-olaf
+INSTALLED ${RND_WORKFLOW_NAME}-olaf from ${RND_WORKFLOW_SOURCE}
 __OUT__
 popd || exit 1
 rm -rf "${RUN_DIR}/${RND_WORKFLOW_NAME}-olaf"
@@ -136,7 +136,7 @@ TEST_NAME="${TEST_NAME_BASE}-option--directory"
 make_rnd_workflow
 run_ok "${TEST_NAME}" cylc install --flow-name="${RND_WORKFLOW_NAME}" --directory="${RND_WORKFLOW_SOURCE}"
 contains_ok "${TEST_NAME}.stdout" <<__OUT__
-INSTALLED $RND_WORKFLOW_NAME from ${RND_WORKFLOW_SOURCE} -> ${RND_WORKFLOW_RUNDIR}/run1
+INSTALLED $RND_WORKFLOW_NAME/run1 from ${RND_WORKFLOW_SOURCE}
 __OUT__
 purge_rnd_workflow
 
@@ -147,12 +147,12 @@ make_rnd_workflow
 pushd "${RND_WORKFLOW_SOURCE}" || exit 1
 run_ok "${TEST_NAME}" cylc install
 contains_ok "${TEST_NAME}.stdout" <<__OUT__
-INSTALLED $RND_WORKFLOW_NAME from ${RND_WORKFLOW_SOURCE} -> ${RND_WORKFLOW_RUNDIR}/run1
+INSTALLED $RND_WORKFLOW_NAME/run1 from ${RND_WORKFLOW_SOURCE}
 __OUT__
 TEST_NAME="${TEST_NAME_BASE}-install-twice-2"
 run_ok "${TEST_NAME}" cylc install
 contains_ok "${TEST_NAME}.stdout" <<__OUT__
-INSTALLED $RND_WORKFLOW_NAME from ${RND_WORKFLOW_SOURCE} -> ${RND_WORKFLOW_RUNDIR}/run2
+INSTALLED $RND_WORKFLOW_NAME/run2 from ${RND_WORKFLOW_SOURCE}
 __OUT__
 popd || exit 1
 purge_rnd_workflow

diff --git a/tests/functional/cylc-install/01-symlinks.t b/tests/functional/cylc-install/01-symlinks.t
@@ -41,7 +41,7 @@ TEST_NAME="${TEST_NAME_BASE}-symlinks-created"
 make_rnd_workflow
 run_ok "${TEST_NAME}" cylc install --flow-name="${RND_WORKFLOW_NAME}" --directory="${RND_WORKFLOW_SOURCE}"
 contains_ok "${TEST_NAME}.stdout" <<__OUT__
-INSTALLED $RND_WORKFLOW_NAME from ${RND_WORKFLOW_SOURCE} -> ${RND_WORKFLOW_RUNDIR}/run1
+INSTALLED $RND_WORKFLOW_NAME/run1 from ${RND_WORKFLOW_SOURCE}
 __OUT__
 
 TEST_SYM="${TEST_NAME_BASE}-run-symlink-exists-ok"
@@ -82,7 +82,7 @@ TEST_NAME="${TEST_NAME_BASE}-no-symlinks-created"
 make_rnd_workflow
 run_ok "${TEST_NAME}" cylc install --flow-name="${RND_WORKFLOW_NAME}" --no-symlink-dirs --directory="${RND_WORKFLOW_SOURCE}"
 contains_ok "${TEST_NAME}.stdout" <<__OUT__
-INSTALLED $RND_WORKFLOW_NAME from ${RND_WORKFLOW_SOURCE} -> ${RND_WORKFLOW_RUNDIR}/run1
+INSTALLED $RND_WORKFLOW_NAME/run1 from ${RND_WORKFLOW_SOURCE}
 __OUT__
 
 

diff --git a/tests/functional/cylc-install/02-failures.t b/tests/functional/cylc-install/02-failures.t
@@ -131,7 +131,7 @@ make_rnd_workflow
 pushd "${RND_WORKFLOW_SOURCE}" || exit 1
 run_ok "${TEST_NAME}" cylc install --run-name=olaf
 contains_ok "${TEST_NAME}.stdout" <<__OUT__
-INSTALLED ${RND_WORKFLOW_NAME} from ${RND_WORKFLOW_SOURCE} -> ${RND_WORKFLOW_RUNDIR}/olaf
+INSTALLED ${RND_WORKFLOW_NAME}/olaf from ${RND_WORKFLOW_SOURCE}
 __OUT__
 TEST_NAME="${TEST_NAME_BASE}-install-twice-mix-options-1-2nd-install"
 run_fail "${TEST_NAME}" cylc install
@@ -149,7 +149,7 @@ make_rnd_workflow
 pushd "${RND_WORKFLOW_SOURCE}" || exit 1
 run_ok "${TEST_NAME}" cylc install
 contains_ok "${TEST_NAME}.stdout" <<__OUT__
-INSTALLED ${RND_WORKFLOW_NAME} from ${RND_WORKFLOW_SOURCE} -> ${RND_WORKFLOW_RUNDIR}/run1
+INSTALLED ${RND_WORKFLOW_NAME}/run1 from ${RND_WORKFLOW_SOURCE}
 __OUT__
 TEST_NAME="${TEST_NAME_BASE}-install-twice-mix-options-2-2nd-install"
 run_fail "${TEST_NAME}" cylc install --run-name=olaf
@@ -167,7 +167,7 @@ make_rnd_workflow
 pushd "${RND_WORKFLOW_SOURCE}" || exit 1
 run_ok "${TEST_NAME}" cylc install --run-name=olaf
 contains_ok "${TEST_NAME}.stdout" <<__OUT__
-INSTALLED ${RND_WORKFLOW_NAME} from ${RND_WORKFLOW_SOURCE} -> ${RND_WORKFLOW_RUNDIR}/olaf
+INSTALLED ${RND_WORKFLOW_NAME}/olaf from ${RND_WORKFLOW_SOURCE}
 __OUT__
 TEST_NAME="${TEST_NAME_BASE}-install-twice-same-run-name-2nd-install"
 run_fail "${TEST_NAME}" cylc install --run-name=olaf

diff --git a/tests/functional/cylc-install/03-file-transfer.t b/tests/functional/cylc-install/03-file-transfer.t
@@ -53,7 +53,7 @@ ${RND_WORKFLOW_RUNDIR}/
 __OUT__
 
 contains_ok "${TEST_NAME}.stdout" <<__OUT__
-INSTALLED $RND_WORKFLOW_NAME from ${RND_WORKFLOW_SOURCE} -> ${RND_WORKFLOW_RUNDIR}
+INSTALLED $RND_WORKFLOW_NAME from ${RND_WORKFLOW_SOURCE}
 __OUT__
 popd || exit 1
 purge_rnd_workflow
@@ -87,7 +87,7 @@ ${RND_WORKFLOW_RUNDIR}/
 __OUT__
 
 contains_ok "${TEST_NAME}.stdout" <<__OUT__
-INSTALLED $RND_WORKFLOW_NAME from ${RND_WORKFLOW_SOURCE} -> ${RND_WORKFLOW_RUNDIR}
+INSTALLED $RND_WORKFLOW_NAME from ${RND_WORKFLOW_SOURCE}
 __OUT__
 popd || exit 1
 purge_rnd_workflow