public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Steven Shi" <steven.shi@intel.com>
To: devel@edk2.groups.io
Cc: liming.gao@intel.com, bob.c.feng@intel.com,
	christian.rodriguez@intel.com, michael.johnson@intel.com, "Shi,
	Steven" <steven.shi@intel.com>
Subject: [PATCH v3 5/5] BaseTools: Improve the file saving and copying reliability
Date: Tue, 13 Aug 2019 16:50:55 +0800	[thread overview]
Message-ID: <20190813085055.23208-6-steven.shi@intel.com> (raw)
In-Reply-To: <20190813085055.23208-1-steven.shi@intel.com>

From: "Shi, Steven" <steven.shi@intel.com>

BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=2079

The Basetool CopyFileOnChange() and SaveFileOnChange()
functions might raise the IOError occasionally when build
in Windows with multi-process and build cache enabled.
The CopyFileOnChange() and SaveFileOnChange() might be invoked
in multiple sub-processes simultaneously, and this patch adds
a global lock to sync these functions invoking which can
harden their reliability.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Bob Feng <bob.c.feng@intel.com>
Signed-off-by: Steven Shi <steven.shi@intel.com>
---
 BaseTools/Source/Python/AutoGen/GenC.py          |  2 +-
 BaseTools/Source/Python/AutoGen/GenMake.py       |  2 +-
 BaseTools/Source/Python/AutoGen/ModuleAutoGen.py |  2 +-
 BaseTools/Source/Python/Common/Misc.py           | 18 +++++++++++++-----
 4 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/GenC.py b/BaseTools/Source/Python/AutoGen/GenC.py
old mode 100644
new mode 100755
index 910c8fe370..1bb3f42b46
--- a/BaseTools/Source/Python/AutoGen/GenC.py
+++ b/BaseTools/Source/Python/AutoGen/GenC.py
@@ -2103,5 +2103,5 @@ def CreateCode(Info, AutoGenC, AutoGenH, StringH, UniGenCFlag, UniGenBinBuffer,
 #   @retval     False       If the file exists and the content is not changed
 #
 def Generate(FilePath, Content, IsBinaryFile):
-    return SaveFileOnChange(FilePath, Content, IsBinaryFile)
+    return SaveFileOnChange(FilePath, Content, IsBinaryFile, GlobalData.file_lock)
 
diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py b/BaseTools/Source/Python/AutoGen/GenMake.py
index de820eeb2f..e159782306 100755
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -185,7 +185,7 @@ class BuildFile(object):
         self._FileType = FileType
         FileContent = self._TEMPLATE_.Replace(self._TemplateDict)
         FileName = self._FILE_NAME_[FileType]
-        return SaveFileOnChange(os.path.join(self._AutoGenObject.MakeFileDir, FileName), FileContent, False)
+        return SaveFileOnChange(os.path.join(self._AutoGenObject.MakeFileDir, FileName), FileContent, False, GlobalData.file_lock)
 
     ## Return a list of directory creation command string
     #
diff --git a/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py b/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
index ee8518e19c..0d16d8ded7 100755
--- a/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
@@ -1612,7 +1612,7 @@ class ModuleAutoGen(AutoGen):
         destination_dir = os.path.dirname(destination_file)
         CreateDirectory(destination_dir)
         try:
-            CopyFileOnChange(File, destination_dir)
+            CopyFileOnChange(File, destination_dir, GlobalData.file_lock)
         except:
             EdkLogger.quiet("[cache warning]: fail to copy file:%s to folder:%s" % (File, destination_dir))
             return
diff --git a/BaseTools/Source/Python/Common/Misc.py b/BaseTools/Source/Python/Common/Misc.py
old mode 100644
new mode 100755
index 26d149c270..db2c3c9589
--- a/BaseTools/Source/Python/Common/Misc.py
+++ b/BaseTools/Source/Python/Common/Misc.py
@@ -448,7 +448,7 @@ def RemoveDirectory(Directory, Recursively=False):
 #   @retval     True            If the file content is changed and the file is renewed
 #   @retval     False           If the file content is the same
 #
-def SaveFileOnChange(File, Content, IsBinaryFile=True):
+def SaveFileOnChange(File, Content, IsBinaryFile=True, FileLock=None):
 
     if os.path.exists(File):
         if IsBinaryFile:
@@ -486,8 +486,12 @@ def SaveFileOnChange(File, Content, IsBinaryFile=True):
             tf.write(Content)
             tempname = tf.name
         try:
-            os.rename(tempname, File)
-        except:
+            if FileLock:
+                with FileLock:
+                    os.rename(tempname, File)
+            else:
+                os.rename(tempname, File)
+        except IOError as X:
             EdkLogger.error(None, FILE_CREATE_FAILURE, ExtraData='IOError %s' % X)
     else:
         try:
@@ -510,7 +514,7 @@ def SaveFileOnChange(File, Content, IsBinaryFile=True):
 #   @retval     True      The two files content are different and the file is copied
 #   @retval     False     No copy really happen
 #
-def CopyFileOnChange(SrcFile, Dst):
+def CopyFileOnChange(SrcFile, Dst, FileLock=None):
     if not os.path.exists(SrcFile):
         return False
 
@@ -545,7 +549,11 @@ def CopyFileOnChange(SrcFile, Dst):
             # os.rename reqire to remove the dst on Windows, otherwise OSError will be raised.
             if GlobalData.gIsWindows and os.path.exists(DstFile):
                 os.remove(DstFile)
-            os.rename(tempname, DstFile)
+            if FileLock:
+                with FileLock:
+                    os.rename(tempname, DstFile)
+            else:
+                os.rename(tempname, DstFile)
 
     except IOError as X:
         EdkLogger.error(None, FILE_COPY_FAILURE, ExtraData='IOError %s' % X)
-- 
2.17.1


      parent reply	other threads:[~2019-08-13  8:51 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-13  8:50 [PATCH v3 0/5] Build cache enhancement Steven Shi
2019-08-13  8:50 ` [PATCH v3 1/5] BaseTools: Improve the cache hit in the edk2 build cache Steven Shi
2019-08-13  8:50 ` [PATCH v3 2/5] BaseTools: Print first cache missing file for build cachle Steven Shi
2019-08-13  8:50 ` [PATCH v3 3/5] BaseTools: Change the [Arch][Name] module key in Build cache Steven Shi
2019-08-13  8:50 ` [PATCH v3 4/5] BaseTools: Add GenFds multi-thread support in build cache Steven Shi
2019-08-13  8:50 ` Steven Shi [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190813085055.23208-6-steven.shi@intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox