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

Fix unintended directory stripping during cylc install #4931

Merged
merged 7 commits into from
Jun 29, 2022

Conversation

datamel
Copy link
Contributor

@datamel datamel commented Jun 23, 2022

These changes close #4930

Fix bug where hierarchical source workflows (e.g ~/cylc-src/dogs/fido/) would lose the multi-level name on installation if global.cylc[install]source dirs contained ~ or env vars.

Requirements check-list

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • Applied any dependency changes to both setup.cfg and conda-environment.yml.
  • Adapted existing tests.
  • Appropriate change log entry included.
  • No documentation update required.

@datamel datamel self-assigned this Jun 23, 2022
@datamel datamel added this to the cylc-8.0rc4 milestone Jun 23, 2022
@datamel datamel requested review from wxtim and MetRonnie June 23, 2022 13:58
@oliver-sanders
Copy link
Member

I think we should have at least one functional test for this, especially given that we accidentally implemented, then un-implemented this before 🤣.

tests/unit/test_workflow_files.py Outdated Show resolved Hide resolved
cylc/flow/workflow_files.py Outdated Show resolved Hide resolved
cylc/flow/workflow_files.py Outdated Show resolved Hide resolved
tests/unit/test_workflow_files.py Outdated Show resolved Hide resolved
datamel and others added 3 commits June 24, 2022 13:55
Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>
Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>
Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>
@datamel datamel requested a review from MetRonnie June 27, 2022 22:49
@oliver-sanders
Copy link
Member

Works fine for workflows installed by "name" or "path", however, we get a flat installation when installing from "$PWD".

$ cd ~/cylc-src
$ mkdir a/b/c -p
$ touch a/b/c/flow.cylc

$ cylc install a/b/c
INSTALLED a/b/c/run1 from ~/cylc-src/a/b/c

$ cylc install ~/cylc-src/a/b/c
INSTALLED a/b/c/run2 from ~/cylc-src/a/b/c

$ cd a/b/c
$ cylc install
INSTALLED c/run1 from ~/cylc-src/a/b/c

@MetRonnie
Copy link
Member

MetRonnie commented Jun 29, 2022

Agh, /home/ vs /net/home/ strikes again! Tested out the fix ✔️

Copy link
Member

@wxtim wxtim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it's approved by Hilary and Ronnie I've done a quick functional check.

@wxtim wxtim merged commit e117e3b into cylc:master Jun 29, 2022
wxtim added a commit to wxtim/cylc that referenced this pull request Jul 4, 2022
* master: (30 commits)
  Log dir tidy (cylc#4836)
  Cut down on back-compat warnings, plus other tidying (cylc#4943)
  Validate platform settings (background job runner) (cylc#4938)
  clean: push error message to stderr
  Update cylc/flow/id_match.py
  Fix unintended directory stripping during cylc install (cylc#4931)
  stop cylc validate swallowing useful errors (cylc#4936)
  Improve config reference docs (cylc#4916)
  glblcfg: increase default rolling archive length
  Fix job state with pre-submitted failure
  Update tests/functional/cylc-reinstall/02-failures.t
  reinstall: require workflow ID argument
  play: upgrade --start-task's specified in legacy format (cylc#4925)
  expand schema docstring internally (cylc#4926)
  Added a new task filtering test.
  Add comment [skip ci]
  Fix optparse Options class for std options (cylc#4919)
  uid: warn if the run number is provided as a cycle
  Tweak prev.
  Fix task filtering.
  ...
@hjoliver hjoliver modified the milestones: cylc-8.0rc4, cylc-8.0.0 Jul 5, 2022
@datamel datamel deleted the 4930-cylc-install-bug branch September 1, 2022 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cylc install strips directories
5 participants