- 
                Notifications
    
You must be signed in to change notification settings  - Fork 1.4k
 
Bootstrap: update logic to support pre-and post python 3.8 #8446
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
Bootstrap: update logic to support pre-and post python 3.8 #8446
Conversation
| 
           @swift-ci please test  | 
    
| 
           @swift-ci test self hosted windows  | 
    
c703722    to
    ccb4634      
    Compare
  
    | 
           @swift-ci please test  | 
    
| 
           @swift-ci please test self hosted windows  | 
    
| 
           @swift-ci test windows  | 
    
| shutil.copytree(src, dest, ignore=shutil.ignore_patterns(*ignored_patterns), dirs_exist_ok=True) | ||
| additional_kwargs = { "dirs_exist_ok": True } if sys.version_info.major >=3 and sys.version_info >= 8 else {} | ||
| shutil.copytree(src, dest, ignore=shutil.ignore_patterns(*ignored_patterns), **additional_kwargs) | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docs for 3.7 state that the destination must not already exist, which means this doesn't fix the problem the original PR was trying to solve here in that case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bnbarham : Right. If the builds fail, what are you thoughts about this ?
diff --git a/Utilities/bootstrap b/Utilities/bootstrap
index e87e21fae..89230385b 100755
--- a/Utilities/bootstrap
+++ b/Utilities/bootstrap
@@ -16,6 +16,7 @@
 from __future__ import print_function
 
 import argparse
+import fnmatch
 import json
 import logging
 import os
@@ -25,6 +26,8 @@ import re
 import shutil
 import subprocess
 import sys
+import typing as t
+
 from helpers import symlink_force, mkdir_p, call, call_output
 
 
@@ -574,7 +577,8 @@ def install_dylib(args, library_name, install_dir, module_names):
 
 # Helper function that installs a single built artifact to a particular directory. The source may be either a file or a directory.
 @log_entry_exit
-def install_binary(args, binary, destination, destination_is_directory=True, ignored_patterns=[], subpath=None):
+def install_binary(args, binary, destination, destination_is_directory=True, ignored_patterns: t.Optional[t.Iterable[str]]=None, subpath=None):
+    ignored_patterns = ignored_patterns or []
     if subpath:
         basepath = os.path.join(args.bin_dir, subpath)
     else:
@@ -592,8 +596,34 @@ def install_file(args, src, destination, destination_is_directory=True, ignored_
 
     logging.info("Installing %s to %s", src, dest)
     if os.path.isdir(src):
-        additional_kwargs = { "dirs_exist_ok": True } if sys.version_info.major >=3 and sys.version_info.minor >= 8 else {}
-        shutil.copytree(src, dest, ignore=shutil.ignore_patterns(*ignored_patterns), **additional_kwargs)
+        if sys.version_info.major >=3 and sys.version_info.minor >= 8:
+            shutil.copytree(src, dest, ignore=shutil.ignore_patterns(*ignored_patterns), dirs_exist_ok=True)
+        else:
+            # need to ensure dest will be created
+            # it's better to find an proper API for Pyth9on <=3.7, but I wasn't able to.
+
+            def copy_tree_ignore(source_dir, dest_dir, *, ignore_patterns):
+                """
+                Copies a directory tree from source to dest, ignoring files/dirs matching patterns.
+                """
+                if not os.path.exists(dest_dir):
+                    os.makedirs(dest_dir)
+
+                for item in os.listdir(source_dir):
+                    source_item_path = os.path.join(source_dir, item)
+                    dest_item_path = os.path.join(dest_dir, item)
+
+                    if any(fnmatch.fnmatch(item, pattern) for pattern in ignore_patterns):
+                        continue
+
+                    if os.path.isfile(source_item_path):
+                        with open(source_item_path, 'rb') as source_file, open(dest_item_path, 'wb') as dest_file:
+                            dest_file.write(source_file.read())
+                    elif os.path.isdir(source_item_path):
+                        copy_tree_ignore(source_item_path, dest_item_path, ignore_patterns)
+
+            copy_tree_ignore(src, dest, ignore_patterns=ignored_patterns)
+
     else:
         shutil.copy2(src, dest)
PR swiftlang#8373 (commit ID: 0193d5f) updated a shutil.copytree(...) command to pass in the `dirs_exists_ok=True` argument. However, Python added support for the `dirs_exists_ok` argument to `shutil.copytree` in 3.8. Update the function call to inspect the python version, and only add the argument if the script is running in a Python version equal, or higher than 3.8. rdar://147297864
ccb4634    to
    40d32cf      
    Compare
  
    | 
           @swift-ci test  | 
    
| 
           @swift-ci test self hosted windows  | 
    
| 
           @swift-ci test windows  | 
    
| 
           @owenv FYI  | 
    
PR #8373 (commit ID: 0193d5f) updated a
shutil.copytree(...)command to pass in thedirs_exists_ok=Trueargument. However, Python added support for thedirs_exists_okargument toshutil.copytreein 3.8.Update the function call to inspect the python version, and only add the argument if the script is running in a Python version equal, or higher than 3.8.
rdar://147297864