Revert "Change the copy_archives method to use..."

This change is causing logs to not be copied correctly. See IT-19109,
IT-19131.
This reverts commit 3fad9329a6de8384446fb63215311d29e8aab363.

Change-Id: I7b134d2ac5f8c3b93808c1b863f3f01df0cc0a95
Signed-off-by: Eric Ball <eball@linuxfoundation.org>

diff --git a/lftools/deploy.py b/lftools/deploy.py
index a7bcbe5..6537e13 100755 (executable)
--- a/lftools/deploy.py
+++ b/lftools/deploy.py
@@ -50,14 +50,9 @@ def _compress_text(dir):
         paths.extend(glob.glob(search, recursive=True))
 
     for _file in paths:
-        # glob may follow symlink paths that open can't find
-        if os.path.exists(_file):
-            log.debug("Compressing file {}".format(_file))
-            with open(_file, 'rb') as src, gzip.open('{}.gz'.format(_file), 'wb') as dest:
-                shutil.copyfileobj(src, dest)
-                os.remove(_file)
-        else:
-            log.info("Could not open path from glob {}".format(_file))
+        with open(_file, 'rb') as src, gzip.open('{}.gz'.format(_file), 'wb') as dest:
+            shutil.copyfileobj(src, dest)
+            os.remove(_file)
 
     os.chdir(save_dir)
 
@@ -222,7 +217,7 @@ def copy_archives(workspace, pattern=None):
         :arg str pattern: Space-separated list of Unix style glob patterns.
             (default: None)
     """
-    archives_dir = os.path.join(workspace, "archives")
+    archives_dir = os.path.join(workspace, 'archives')
     dest_dir = os.getcwd()
 
     log.debug('Copying files from {} with pattern \'{}\' to {}.'.format(
@@ -238,14 +233,11 @@ def copy_archives(workspace, pattern=None):
             for file_or_dir in os.listdir(archives_dir):
                 f = os.path.join(archives_dir, file_or_dir)
                 try:
-                    log.debug('Copying {}'.format(f))
-                    shutil.copy2(f, dest_dir, follow_symlinks=False)
-                except IsADirectoryError:
-                    os.makedirs(os.path.join(dest_dir, file_or_dir),
-                                exist_ok=True)
+                    log.debug('Moving {}'.format(f))
+                    shutil.move(f, dest_dir)
                 except shutil.Error as e:
                     log.error(e)
-                    raise OSError(errno.EPERM, 'Could not copy to', archives_dir)
+                    raise OSError(errno.EPERM, 'Could not move to', archives_dir)
     else:
         log.error('Archives dir {} does not exist.'.format(archives_dir))
         raise OSError(errno.ENOENT, 'Missing directory', archives_dir)
@@ -270,7 +262,6 @@ def copy_archives(workspace, pattern=None):
         if len(os.path.basename(src)) > 255:
             log.warn('Filename {} is over 255 characters. Skipping...'.format(
                 os.path.basename(src)))
-            continue
 
         dest = os.path.join(dest_dir, src[len(workspace)+1:])
         log.debug('{} -> {}'.format(src, dest))
@@ -278,9 +269,9 @@ def copy_archives(workspace, pattern=None):
         if os.path.isfile(src):
             try:
                 shutil.move(src, dest)
-            except FileNotFoundError as e:
+            except IOError as e:  # Switch to FileNotFoundError when Python 2 support is dropped.
                 log.debug("Missing path, will create it {}.\n{}".format(os.path.dirname(dest), e))
-                os.makedirs(os.path.dirname(dest), exist_ok=True)
+                os.makedirs(os.path.dirname(dest))
                 shutil.move(src, dest)
         else:
             log.info('Not copying directories: {}.'.format(src))

diff --git a/releasenotes/notes/fix-archive-symlink-loops-0e754059d8db33ba.yaml b/releasenotes/notes/fix-archive-symlink-loops-0e754059d8db33ba.yaml
deleted file mode 100644 (file)
index ae78b11..0000000
--- a/releasenotes/notes/fix-archive-symlink-loops-0e754059d8db33ba.yaml
+++ /dev/null
@@ -1,8 +0,0 @@
----
-fixes:
-  - |
-    Change the copy_archives method used by deploy_archives to use copy2 with
-    follow_symlinks set to False, which will copy symlinks as symlinks, rather
-    than following their path. Since glob does not have an option to not follow
-    symlinks, we instead just check for file existence to ignore issues with
-    globbing symlinked paths that "open" fails to find.

diff --git a/tests/fixtures/deploy/workspace-symlinks/archives/child/parent b/tests/fixtures/deploy/workspace-symlinks/archives/child/parent
deleted file mode 120000 (symlink)
index a96aa0e..0000000
--- a/tests/fixtures/deploy/workspace-symlinks/archives/child/parent
+++ /dev/null
@@ -1 +0,0 @@
-..
\ No newline at end of file

diff --git a/tests/fixtures/deploy/workspace-symlinks/archives/non-existent-file b/tests/fixtures/deploy/workspace-symlinks/archives/non-existent-file
deleted file mode 120000 (symlink)
index ef70665..0000000
--- a/tests/fixtures/deploy/workspace-symlinks/archives/non-existent-file
+++ /dev/null
@@ -1 +0,0 @@
-non-existent-file
\ No newline at end of file

diff --git a/tests/fixtures/deploy/workspace-symlinks/archives/test.log b/tests/fixtures/deploy/workspace-symlinks/archives/test.log
deleted file mode 100644 (file)
index b3589bf..0000000
--- a/tests/fixtures/deploy/workspace-symlinks/archives/test.log
+++ /dev/null
@@ -1 +0,0 @@
-This is a test log.

diff --git a/tests/test_deploy.py b/tests/test_deploy.py
index e33ac12..77d0a1c 100644 (file)
--- a/tests/test_deploy.py
+++ b/tests/test_deploy.py
@@ -11,7 +11,6 @@
 
 import os
 import sys
-import tempfile
 
 import pytest
 import requests
@@ -75,26 +74,6 @@ def test_copy_archive_dir(cli_runner, datafiles):
     assert os.path.exists(os.path.join(stage_dir, 'test.log'))
 
 
-@pytest.mark.datafiles(
-    os.path.join(FIXTURE_DIR, 'deploy'),
-    )
-def test_copy_archive_dir_symlinks(cli_runner, datafiles):
-    """Test copy_archives() command with symlink loop and a broken symlink, in
-    order to test that these conditions are handled properly."""
-    os.chdir(str(datafiles))
-    workspace_dir = os.path.join(str(datafiles), 'workspace-symlinks')
-    stage_dir = str(datafiles.mkdir("stage_archive"))
-
-    os.chdir(stage_dir)
-    result = cli_runner.invoke(
-        cli.cli,
-        ['--debug', 'deploy', 'copy-archives', workspace_dir],
-        obj={})
-    assert result.exit_code == 0
-
-    assert os.path.exists(os.path.join(stage_dir, 'test.log'))
-
-
 @pytest.mark.datafiles(
     os.path.join(FIXTURE_DIR, 'deploy'),
     )
@@ -153,7 +132,7 @@ def test_deploy_archive(cli_runner, datafiles, responses):
     )
 def test_deploy_archive2(datafiles):
     """Test deploy_archives() command when archives dir is missing."""
-    os.chdir(tempfile.mkdtemp(prefix='lftools-test.'))
+    os.chdir(str(datafiles))
     workspace_dir = os.path.join(str(datafiles), 'workspace-noarchives')
 
     with pytest.raises(OSError) as excinfo:
@@ -166,7 +145,7 @@ def test_deploy_archive2(datafiles):
     )
 def test_deploy_archive3(datafiles):
     """Test deploy_archives() command when archives dir is a file instead of a dir."""
-    os.chdir(tempfile.mkdtemp(prefix='lftools-test.'))
+    os.chdir(str(datafiles))
     workspace_dir = os.path.join(str(datafiles), 'workspace-archivesfile')
 
     with pytest.raises(OSError) as excinfo:
@@ -179,7 +158,7 @@ def test_deploy_archive3(datafiles):
     )
 def test_deploy_archive4(cli_runner, datafiles, responses):
     """Test deploy_archives() command when using duplicated patterns."""
-    os.chdir(tempfile.mkdtemp(prefix='lftools-test.'))
+    os.chdir(str(datafiles))
     workspace_dir = os.path.join(str(datafiles), 'workspace-patternfile')
     pattern=["**/*.log", "**/hs_err_*.log", "**/target/**/feature.xml", "**/target/failsafe-reports/failsafe-summary.xml", "**/target/surefire-reports/*-output.txt", "**/target/surefire-reports/*-output.txt", "**/target/failsafe-reports/failsafe-summary.xml", "**/*"]
     result = deploy_sys.copy_archives(workspace_dir, pattern)