From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.120]) by mx.groups.io with SMTP id smtpd.web12.952.1574441254984692727 for ; Fri, 22 Nov 2019 08:47:35 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=FygMEwUe; spf=pass (domain: redhat.com, ip: 205.139.110.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1574441254; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=kJdruTAk95IiSfRWbaaoLtF1A/4JqvwWhw5q/YCXNDE=; b=FygMEwUeu9z+TWFpGDtVX8KbOpWL/wnqPjoWMq1JKQpBWV7Y4vIoYMvfYdi2UqOXFO4Y4d BeI4y6mLF0dAmRg2brU2cRdQCjcx0Xox/ODg4vuMct29pMKv8BIS7+ZImHZl9ofSwbs8C2 r9FGwawD/zCaCxkM3oyeZi/kJS+BTP0= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-238-wStwb_kkOcaHwVtLyoVELg-1; Fri, 22 Nov 2019 11:47:29 -0500 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 4A065DB61; Fri, 22 Nov 2019 16:47:28 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (unknown [10.36.118.23]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1B22F6E71B; Fri, 22 Nov 2019 16:47:26 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH] BaseTools:fixed Build failed issue for Non-English OS To: "Feng, Bob C" , "devel@edk2.groups.io" , "Fan, ZhijuX" Cc: "Gao, Liming" , Leif Lindholm References: <95a79c71-2eb4-af97-a738-170a2668599d@redhat.com> <08650203BA1BD64D8AD9B6D5D74A85D16156A292@SHSMSX104.ccr.corp.intel.com> From: "Laszlo Ersek" Message-ID: <58388e00-7bcd-7a58-cc74-6a32d5e9589c@redhat.com> Date: Fri, 22 Nov 2019 17:47:26 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <08650203BA1BD64D8AD9B6D5D74A85D16156A292@SHSMSX104.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-MC-Unique: wStwb_kkOcaHwVtLyoVELg-1 X-Mimecast-Spam-Score: 0 Content-Language: en-US Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Hi Bob, On 11/22/19 07:39, Feng, Bob C wrote: > [Bob] The build failure was found on a platform which uses Structure Pcds in dsc and the build machine is a Win10 with Korean language. > The present patch descripts the build failure case. > [Bob] I agree. "Non-English OS" is not precise. For this case, the build failure is because the visual studio c compiler output message includes localized string. > [Bob] I think for this case build tool does not need to handle the non-ascii characters so 'Ignore' will be enough. > [Bob] I agree to mention the present patch is to revert part of commit 8ddec24dea74, drop "non-English OS" language, but keep "structure PCD". thanks for following up. I'm happy if you agree to mention commit 8ddec24dea74. Furthermore, I understand that Visual Studio can print localized strings. But, there are two things that remain unclear: - Why are such messages tied to structure PCDs? Can Visual Studio *not* print the same kind of localized message in other cases? Like, what if you have a (VOID*) PCD and assign an L"..." string to it, in the platform DSC? I mean I always encourage patch authors to include as many details about the failure scenario in the commit message as they feel comfortable about. But the current commit message suggests the issue is specific to structure PCDs *only*. That's the confusing part. - You mention "I think for this case build tool does not need to handle the non-ascii characters so 'Ignore' will be enough." -- Sure, I guess for this particular case, "Ignore" could be OK -- but the method that's being modified bears the generic name "ExecuteCommand". What *else* is ExecuteCommand() used for? I'm not convinced that ignoring decoding errors in the standard output / standard error of the subprocess is acceptable in *all* cases. If there is no other use case for ExecuteCommand()'s standard output / standard error than to print them to the edk2 build log, then I agree "ignore" is tolerable. But I don't know if that's the only use case. If it is, it should be stated in the commit message. Thanks Laszlo