Repository: codalab/codalab-worksheets

Issue:
Get rid of ._ files when uploading from a Mac
Get rid of `._` files  when using the tar library with `COPYFILE_DISABLE=1`.

Discussion:
teetone: > The change looks good to me. However, I wasn't able to reproduce those additional files `._` when I zip files on MacOS. Does this happen to a certain MacOS version?

@candicegjing I'm not sure if this a problem with a specific MacOS version. I'm on Catalina, which version are you using by the way?
candicegjing: > @candicegjing I'm not sure if this a problem with a specific MacOS version. I'm on Catalina, which version are you using by the way?

I am also on Catalina (version 10.15.1). Interestingly, I don't see any of those files created when I zip files.

Patch:
diff --git a/codalab/worker/file_util.py b/codalab/worker/file_util.py
index 1d7bb0c7..3a7703df 100644
--- a/codalab/worker/file_util.py
+++ b/codalab/worker/file_util.py
@@ -11,7 +11,20 @@ import bz2
 from codalab.common import BINARY_PLACEHOLDER
 
 NONE_PLACEHOLDER = '<none>'
-GIT_PATTERN = '.git'
+
+# Patterns to always ignore when zipping up directories
+ALWAYS_IGNORE_PATTERNS = ['.git', '._*', '__MACOSX']
+
+
+def get_tar_version_output():
+    """
+    Gets the current tar library's version information by returning the stdout
+    of running `tar --version`.
+    """
+    try:
+        return subprocess.getoutput('tar --version')
+    except subprocess.CalledProcessError as e:
+        raise IOError(e.output)
 
 
 def tar_gzip_directory(
@@ -28,19 +41,24 @@ def tar_gzip_directory(
                       the directory structure are excluded.
     ignore_file: Name of the file where exclusion patterns are read from.
     """
-    # Always ignore entries specified by the ignore file (e.g. .gitignore)
     args = ['tar', 'czf', '-', '-C', directory_path]
+
+    # If the BSD tar library is being used, append --disable-copy to prevent creating ._* files
+    if 'bsdtar' in get_tar_version_output():
+        args.append('--disable-copyfile')
+
     if ignore_file:
+        # Ignore entries specified by the ignore file (e.g. .gitignore)
         args.append('--exclude-ignore=' + ignore_file)
     if follow_symlinks:
         args.append('-h')
     if not exclude_patterns:
         exclude_patterns = []
 
-    # Always exclude .git
-    exclude_patterns.append(GIT_PATTERN)
+    exclude_patterns.extend(ALWAYS_IGNORE_PATTERNS)
     for pattern in exclude_patterns:
         args.append('--exclude=' + pattern)
+
     if exclude_names:
         for name in exclude_names:
             # Exclude top-level entries provided by exclude_names


Test patch:
diff --git a/tests/files/ignore_test/__MACOSX/ignored.txt b/tests/files/ignore_test/__MACOSX/ignored.txt
new file mode 100644
index 00000000..ea10ec85
--- /dev/null
+++ b/tests/files/ignore_test/__MACOSX/ignored.txt
@@ -0,0 +1,1 @@
+ignored
diff --git a/tests/files/ignore_test/dir/__MACOSX/ignored.txt b/tests/files/ignore_test/dir/__MACOSX/ignored.txt
new file mode 100644
index 00000000..ea10ec85
--- /dev/null
+++ b/tests/files/ignore_test/dir/__MACOSX/ignored.txt
@@ -0,0 +1,1 @@
+ignored
diff --git a/tests/worker/file_util_test.py b/tests/worker/file_util_test.py
index 5b9817e4..8f539ee4 100644
--- a/tests/worker/file_util_test.py
+++ b/tests/worker/file_util_test.py
@@ -80,3 +80,17 @@ class FileUtilTest(unittest.TestCase):
         self.assertNotIn('ignored_dir', output_dir_entries)
         self.assertTrue(os.path.exists(os.path.join(output_dir, 'dir', 'not_ignored2.txt')))
         self.assertFalse(os.path.exists(os.path.join(output_dir, 'dir', 'ignored2.txt')))
+
+    def test_tar_always_ignore(self):
+        dir = os.path.join(os.path.dirname(os.path.dirname(__file__)), 'files/ignore_test')
+        temp_dir = tempfile.mkdtemp()
+        self.addCleanup(lambda: remove_path(temp_dir))
+        output_dir = os.path.join(temp_dir, 'output')
+
+        un_tar_directory(tar_gzip_directory(dir), output_dir, 'gz')
+        output_dir_entries = os.listdir(output_dir)
+        self.assertNotIn('._ignored', output_dir_entries)
+        self.assertIn('dir', output_dir_entries)
+        self.assertNotIn('__MACOSX', output_dir_entries)
+        self.assertFalse(os.path.exists(os.path.join(output_dir, 'dir', '__MACOSX')))
+        self.assertFalse(os.path.exists(os.path.join(output_dir, 'dir', '._ignored2')))