Fix: pop current directory after chdir. (#803)

* Fix: pop current directory after chdir.

Signed-off-by: dblock <dblock@dblock.org>

* Output dir should not be under temp.

Signed-off-by: dblock <dblock@amazon.com>

src/manifests_workflow/input_manifests.py

@@ -40,9 +40,8 @@ def update(self, min_klass, component_klass, keep=False):
         known_versions = self.versions
         logging.info(f"Known versions: {known_versions}")
         main_versions = {}
-        with TemporaryDirectory(keep=keep) as work_dir:
+        with TemporaryDirectory(keep=keep, chdir=True) as work_dir:
             logging.info(f"Checking out components into {work_dir.name}")
-            os.chdir(work_dir.name)
 
             # check out and build #main, 1.x, etc.
             branches = min_klass.branches()

src/run_assemble.py

@@ -41,11 +41,9 @@ def main():
     output_dir = os.path.join(os.getcwd(), "dist")
     os.makedirs(output_dir, exist_ok=True)
 
-    with TemporaryDirectory() as work_dir:
+    with TemporaryDirectory(chdir=True):
         logging.info(f"Bundling {build.name} ({build.architecture}) on {build.platform} into {output_dir} ...")
 
-        os.chdir(work_dir.name)
-
         bundle_recorder = BundleRecorder(build, output_dir, artifacts_dir, args.base_url)
 
         bundle = Bundles.create(build_manifest, artifacts_dir, bundle_recorder)

src/run_build.py

@@ -24,14 +24,11 @@ def main():
     args = BuildArgs()
     console.configure(level=args.logging_level)
     manifest = InputManifest.from_file(args.manifest)
+    output_dir = os.path.join(os.getcwd(), "builds")
 
-    with TemporaryDirectory(keep=args.keep) as work_dir:
-        output_dir = os.path.join(os.getcwd(), "builds")
-
+    with TemporaryDirectory(keep=args.keep, chdir=True) as work_dir:
         logging.info(f"Building in {work_dir.name}")
 
-        os.chdir(work_dir.name)
-
         target = BuildTarget(
             name=manifest.build.name,
             version=manifest.build.version,

src/run_checkout.py

@@ -22,11 +22,9 @@ def main():
     console.configure(level=args.logging_level)
     manifest = InputManifest.from_file(args.manifest)
 
-    with TemporaryDirectory(keep=True) as work_dir:
+    with TemporaryDirectory(keep=True, chdir=True) as work_dir:
         logging.info(f"Checking out into {work_dir.name}")
 
-        os.chdir(work_dir.name)
-
         for component in manifest.components:
 
             logging.info(f"Checking out {component.name}")

src/run_ci.py

@@ -26,11 +26,9 @@ def main():
 
     target = CiTarget(version=manifest.build.version, snapshot=args.snapshot)
 
-    with TemporaryDirectory(keep=args.keep) as work_dir:
+    with TemporaryDirectory(keep=args.keep, chdir=True) as work_dir:
         logging.info(f"Sanity-testing in {work_dir.name}")
 
-        os.chdir(work_dir.name)
-
         logging.info(f"Sanity testing {manifest.build.name}")
 
         for component in manifest.components:

src/run_integ_test.py

@@ -41,10 +41,9 @@ def main():
     for component in test_manifest.components:
         if component.integ_test is not None:
             integ_test_config[component.name] = component
-    with TemporaryDirectory(keep=args.keep) as work_dir:
+    with TemporaryDirectory(keep=args.keep, chdir=True) as work_dir:
         logging.info(f"Switching to temporary work_dir: {work_dir.name}")
         test_recorder = TestRecorder(args.test_run_id, "integ-test", work_dir.name)
-        os.chdir(work_dir.name)
         bundle_manifest = BundleManifest.from_s3(
             args.s3_bucket,
             args.build_id,

src/run_perf_test.py

@@ -41,8 +41,7 @@ def get_infra_repo_url():
 
 
 def main():
-    with TemporaryDirectory(keep=args.keep) as work_dir:
-        os.chdir(work_dir.name)
+    with TemporaryDirectory(keep=args.keep, chdir=True) as work_dir:
         current_workspace = os.path.join(work_dir.name, "infra")
         with GitRepository(get_infra_repo_url(), "main", current_workspace):
             security = False
@@ -55,7 +54,6 @@ def main():
                     test_cluster_endpoint,
                     test_cluster_port,
                 ):
-                    os.chdir(current_workspace)
                     perf_test_suite = PerfTestSuite(manifest, test_cluster_endpoint, security, current_workspace)
                     perf_test_suite.execute()
 

src/system/temporary_directory.py

@@ -22,14 +22,22 @@ def g__handleRemoveReadonly(func, path, exc):
 
 
 class TemporaryDirectory:
-    def __init__(self, keep=False):
+    def __init__(self, keep=False, chdir=False):
         self.keep = keep
         self.name = tempfile.mkdtemp()
+        if chdir:
+            self.curdir = os.getcwd()
+            os.chdir(self.name)
+        else:
+            self.curdir = None
 
     def __enter__(self):
         return self
 
     def __exit__(self, exc_type, exc_value, exc_traceback):
+        if self.curdir:
+            os.chdir(self.curdir)
+
         if self.keep:
             logging.info(f"Keeping {self.name}")
         else:

tests/tests_system/test_temporary_directory.py

@@ -32,3 +32,15 @@ def test_remove_readonly(self):
             self.assertTrue(os.path.exists(filename))
         self.assertFalse(os.path.exists(work_dir.name))
         self.assertFalse(os.path.exists(filename))
+
+    def test_chdir_true(self):
+        before_dir = os.getcwd()
+        with TemporaryDirectory(chdir=True):
+            self.assertNotEqual(before_dir, os.getcwd())
+        self.assertEqual(before_dir, os.getcwd())
+
+    def test_chdir_false(self):
+        before_dir = os.getcwd()
+        with TemporaryDirectory():
+            self.assertEqual(before_dir, os.getcwd())
+        self.assertEqual(before_dir, os.getcwd())