Re: [PATCH] [edk2/BaseTools] edksetup.bat stuck on unicode locale Windows

["Bob Feng" <bob.c.feng@intel.com>]

Hi Aiden,

Thanks for clarifying this issue. Since I have no Unicode locales windows environment, I can't help to do the verification. 
For the following 2# message = stdout.decode(encoding='utf-8', errors='ignore').encode('utf-8'), after encode(), on python3, the message will not a string, it's bytes type data.

BTW, would you help check whether "message = stdout.decode(errors='ignore')" works?

Thanks,
Bob

-----Original Message-----
From: Park, Aiden 
Sent: Friday, December 20, 2019 3:03 PM
To: Feng, Bob C <bob.c.feng@intel.com>; devel@edk2.groups.io
Subject: RE: [PATCH] [edk2/BaseTools] edksetup.bat stuck on unicode locale Windows

Hi Bob,

> -----Original Message-----
> From: Feng, Bob C <bob.c.feng@intel.com>
> Sent: Thursday, December 19, 2019 10:12 PM
> To: Park, Aiden <aiden.park@intel.com>; devel@edk2.groups.io
> Subject: RE: [PATCH] [edk2/BaseTools] edksetup.bat stuck on unicode 
> locale Windows
> 
> Hi Aiden,
> 
> I'd like to know why need to call 'edksetup.bat forcerebuild' with 
> python subprocess.call().
> Is there other way to execute edksetup.bat?
> 
I forgot to mention one more thing in reproduce steps. This is also reproducible by just calling 'edksetup.bat forcerebuild' in Windows command prompt w/o python subprocess.call().
I have tested on 'Chinese - Traditional, Taiwan' and 'Korean' locale Windows and it's reproducible on both Unicode locales. And below change works around the issue on both locales.

Some experiment. Not sure if this is python2 bug.
message = "" <= type 'str'
temp = stdout.decode(encoding='utf-8', errors='ignore') <= type 'unicode' which has unicode string message = temp.encode('utf-8') <= pass message = temp <= deadlock

> Thanks,
> Bob
> -----Original Message-----
> From: Park, Aiden
> Sent: Friday, December 20, 2019 8:23 AM
> To: Feng, Bob C <bob.c.feng@intel.com>; devel@edk2.groups.io
> Subject: RE: [PATCH] [edk2/BaseTools] edksetup.bat stuck on unicode 
> locale Windows
> 
> Hi Bob,
> 
> > -----Original Message-----
> > From: Feng, Bob C <bob.c.feng@intel.com>
> > Sent: Tuesday, December 17, 2019 9:23 PM
> > To: devel@edk2.groups.io; Park, Aiden <aiden.park@intel.com>
> > Cc: Feng, Bob C <bob.c.feng@intel.com>
> > Subject: RE: [PATCH] [edk2/BaseTools] edksetup.bat stuck on unicode 
> > locale Windows
> >
> > Hi Aiden,
> >
> > If set kwargs["stdout"] = sys.stdout, then stdout and stderr 
> > messages from sub process will directly write to sys.stdout.
> > I think this patch works because sys.stdout.encoding is the correct
> encoding.
> > So what do you think if we fix this Non-Ascii issue by Changing the 
> > line of message = stdout.decode(encoding='utf-8', errors='ignore') 
> > #for compatibility in python 2 and 3 to message = 
> > stdout.decode(encoding=sys.stdout.encoding , errors='ignore') #for 
> > compatibility in python 2 and 3
> >
> > Otherwise, I think this patch may need to take care of the code 
> > after this line kwargs["stdout"] = sys.stdout, I mean
> > p.communicate() will return empty string after your change, and the 
> > following code would be useless.
> >
> Thanks for your comment. You are right. This change makes the 
> following code useless. It should use PIPE.
> 
> I did double-check the environment to make sure reproduce steps, and 
> it turns out that it's reproducible on Unicode locale Windows + Python 2.
> Python 3 does not have the issue.
> 
> There seems to be 2 locations which make deadlock.
> 1. subprocess.Popen()
> It looks there is a race condition when creating a child process with PIPE.
> Therefore, a lock is put for Popen().
> +    popen_lock.acquire(True)
>      p = subprocess.Popen(Args, cwd=WorkDir, stderr=kwargs["stderr"],
> stdout=kwargs["stdout"])
> +    popen_lock.release()
> 
> 2. stdout.decode()
> stdout variable is decoded to 'unicode' type message variable. But, 
> it's stuck if stdout has Unicode string even if errors='ignore'.
> Converting The 'unicode' type message to 'str' type message resolves 
> the issue.
> -        message = stdout.decode(encoding='utf-8', errors='ignore') #for
> compatibility in python 2 and 3
> +        message = stdout.decode(encoding='utf-8',
> + errors='ignore').encode('utf-8') #for compatibility in python 2 and 
> + 3
> 
> FYI, your recommendation encoding=sys.stdout.encoding does not help to 
> resolve this issue.
> 
> I look forward to your feedback. Thanks.
> 
> > Thanks,
> > Bob
> >
> > -----Original Message-----
> > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf 
> > Of Park, Aiden
> > Sent: Thursday, December 12, 2019 6:43 AM
> > To: devel@edk2.groups.io
> > Subject: [edk2-devel] [PATCH] [edk2/BaseTools] edksetup.bat stuck on 
> > unicode locale Windows
> >
> > This issue happens under two conditions.
> >   1. Unicode language environment in Windows
> >   2. Call 'edksetup.bat forcerebuild' with python subprocess.call()
> >
> > Steps to reproduce
> >   C:\edk2>python
> >   Python 2.7.12 (v2.7.12:d33e0cf91556, Jun 27 2016, 15:24:40)
> >   Type help, copyright, credits or license for more information.
> >   >>> import subprocess
> >   >>> subprocess.call(['edksetup.bat', 'forcerebuild'])
> >
> >   The edksetup.bat stuck at 'nmake cleanall'.
> >
> > One of multi-threads is on deadlock when python handles stdout and 
> > stderr in a subprocess pipe only if the outputs include unicode chars.
> > Only stderr will be handled in the pipe same as a single thread call.
> >
> > Reported in Slim Bootloader.
> >   https://github.com/slimbootloader/slimbootloader/issues/478
> > Local fix has been made in Slim Bootloader.
> >   https://github.com/slimbootloader/slimbootloader/pull/490
> >
> > Signed-off-by: Aiden Park <aiden.park@intel.com>
> > ---
> >  BaseTools/Source/C/Makefiles/NmakeSubdirs.py | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/BaseTools/Source/C/Makefiles/NmakeSubdirs.py
> > b/BaseTools/Source/C/Makefiles/NmakeSubdirs.py
> > index 356f5ac..c77bfb0 100644
> > --- a/BaseTools/Source/C/Makefiles/NmakeSubdirs.py
> > +++ b/BaseTools/Source/C/Makefiles/NmakeSubdirs.py
> > @@ -33,7 +33,7 @@ def RunCommand(WorkDir=None, *Args, **kwargs):
> >      if "stderr" not in kwargs:
> >          kwargs["stderr"] = subprocess.STDOUT
> >      if "stdout" not in kwargs:
> > -        kwargs["stdout"] = subprocess.PIPE
> > +        kwargs["stdout"] = sys.stdout
> >      p = subprocess.Popen(Args, cwd=WorkDir, 
> > stderr=kwargs["stderr"],
> > stdout=kwargs["stdout"])
> >      stdout, stderr = p.communicate()
> >      message = ""
> > --
> > 2.10.2.windows.1
> >
> > 
> >
> 
> Best Regards,
> Aiden
> 

Best Regards,
Aiden