From db140e26843e948d694a6ab086c31523de04154c Mon Sep 17 00:00:00 2001 From: Aidan Heerdegen Date: Mon, 18 May 2020 16:54:48 +1000 Subject: [PATCH 1/4] Fixed overwriting exe manifest in certain situations --- payu/models/model.py | 17 ++++++++++------- test/test_manifest.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/payu/models/model.py b/payu/models/model.py index 117cdd91..926588f5 100644 --- a/payu/models/model.py +++ b/payu/models/model.py @@ -281,13 +281,16 @@ def setup(self): # Make symlink to executable in work directory if self.exec_path: - # Add to exe manifest (this is always done so any change in exe - # path will be picked up) - self.expt.manifest.add_filepath( - 'exe', - self.exec_path_local, - self.exec_path - ) + # If have exe manifest this implies exe reproduce is True. Do not + # want to overwrite exe manifest in this case + if not self.expt.manifest.have_manifest['exe']: + # Add to exe manifest (this is always done so any change in exe + # path will be picked up) + self.expt.manifest.add_filepath( + 'exe', + self.exec_path_local, + self.exec_path + ) timestep = self.config.get('timestep') if timestep: diff --git a/test/test_manifest.py b/test/test_manifest.py index 1c0f7391..a7e95f4f 100644 --- a/test/test_manifest.py +++ b/test/test_manifest.py @@ -213,6 +213,35 @@ def test_exe_reproduce(): # Check manifests have changed as expected assert(not manifests == get_manifests(ctrldir/'manifests')) + # Make an executable in a no-default location + weirdbinpath = bindir / 'weird' / 'model.exe' + weirdbinpath.parent.mkdir(exist_ok=True) + shutil.move(str(bindir/exe), str(weirdbinpath)) + + config['exe'] = str(weirdbinpath) + write_config(config) + + # Run setup with changed exe but reproduce exe set to False + payu_setup(lab_path=str(labdir)) + + manifests = get_manifests(ctrldir/'manifests') + + # Run setup with unchanged exe but reproduce exe set to True. + # Should run without error + payu_setup(lab_path=str(labdir)) + + # Change exe back to exe name with no path + config['exe'] = str(exe) + # Change reproduce exe back to False + config['manifest']['reproduce']['exe'] = True + + write_config(config) + + # Run setup with changed exe but reproduce exe set to True + payu_setup(lab_path=str(labdir)) + + assert(manifests == get_manifests(ctrldir/'manifests')) + def test_input_reproduce(): From e5ac960c6a8501b883623df03f6643058be76830 Mon Sep 17 00:00:00 2001 From: Aidan Heerdegen Date: Tue, 19 May 2020 13:17:15 +1000 Subject: [PATCH 2/4] Altered new test to make it simpler and not effect subsequent tests --- test/test_manifest.py | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/test/test_manifest.py b/test/test_manifest.py index a7e95f4f..aed78088 100644 --- a/test/test_manifest.py +++ b/test/test_manifest.py @@ -213,31 +213,19 @@ def test_exe_reproduce(): # Check manifests have changed as expected assert(not manifests == get_manifests(ctrldir/'manifests')) - # Make an executable in a no-default location - weirdbinpath = bindir / 'weird' / 'model.exe' - weirdbinpath.parent.mkdir(exist_ok=True) - shutil.move(str(bindir/exe), str(weirdbinpath)) - - config['exe'] = str(weirdbinpath) - write_config(config) - - # Run setup with changed exe but reproduce exe set to False - payu_setup(lab_path=str(labdir)) - + # Reset manifests "truth" manifests = get_manifests(ctrldir/'manifests') - # Run setup with unchanged exe but reproduce exe set to True. - # Should run without error - payu_setup(lab_path=str(labdir)) + # Make exe in config.yaml unfindable by giving it a non-existent name + config['exe'] = 'bogus.exe' - # Change exe back to exe name with no path - config['exe'] = str(exe) - # Change reproduce exe back to False + # Change reproduce exe back to True config['manifest']['reproduce']['exe'] = True write_config(config) - # Run setup with changed exe but reproduce exe set to True + # Run setup with changed exe but reproduce exe set to True. Should + # work fine as the exe path is in the manifest payu_setup(lab_path=str(labdir)) assert(manifests == get_manifests(ctrldir/'manifests')) @@ -251,6 +239,7 @@ def test_input_reproduce(): # Set reproduce input to True config['manifest']['reproduce']['exe'] = False config['manifest']['reproduce']['input'] = True + config['exe'] = config_orig['exe'] write_config(config) manifests = get_manifests(ctrldir/'manifests') From c4684fc8c3f84ce28d7aecff4de2bb47688b7aa7 Mon Sep 17 00:00:00 2001 From: Aidan Heerdegen Date: Tue, 19 May 2020 15:04:20 +1000 Subject: [PATCH 3/4] Updated test to have same executable name with different path --- test/test_manifest.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/test_manifest.py b/test/test_manifest.py index aed78088..139b8ac9 100644 --- a/test/test_manifest.py +++ b/test/test_manifest.py @@ -216,8 +216,9 @@ def test_exe_reproduce(): # Reset manifests "truth" manifests = get_manifests(ctrldir/'manifests') - # Make exe in config.yaml unfindable by giving it a non-existent name - config['exe'] = 'bogus.exe' + # Make exe in config.yaml unfindable by giving it a non-existent + # path but crucially the same name as the proper executable + config['exe'] = '/bogus/test.exe' # Change reproduce exe back to True config['manifest']['reproduce']['exe'] = True From 5822a60079415b784a0a4562d37ca8381b8d37c5 Mon Sep 17 00:00:00 2001 From: Aidan Heerdegen Date: Tue, 19 May 2020 15:08:55 +1000 Subject: [PATCH 4/4] PEP8 fixes --- payu/models/model.py | 2 +- test/test_manifest.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/payu/models/model.py b/payu/models/model.py index 926588f5..e62611c1 100644 --- a/payu/models/model.py +++ b/payu/models/model.py @@ -281,7 +281,7 @@ def setup(self): # Make symlink to executable in work directory if self.exec_path: - # If have exe manifest this implies exe reproduce is True. Do not + # If have exe manifest this implies exe reproduce is True. Do not # want to overwrite exe manifest in this case if not self.expt.manifest.have_manifest['exe']: # Add to exe manifest (this is always done so any change in exe diff --git a/test/test_manifest.py b/test/test_manifest.py index 139b8ac9..208c9aae 100644 --- a/test/test_manifest.py +++ b/test/test_manifest.py @@ -216,7 +216,7 @@ def test_exe_reproduce(): # Reset manifests "truth" manifests = get_manifests(ctrldir/'manifests') - # Make exe in config.yaml unfindable by giving it a non-existent + # Make exe in config.yaml unfindable by giving it a non-existent # path but crucially the same name as the proper executable config['exe'] = '/bogus/test.exe'