Thanks for the feedback. I will move it forward into the next patch. I also agree with Laszlo's comment on the first patch: " I'd request that we please hold this patch for now. (Comments in disagreement are welcome of course.) If we agree, I'd suggest marking TianoCore#1821 dependent on TianoCore#1089".
Both due to the soft feature freeze and the OpenSSL update. Thanks, Christian >-----Original Message----- >From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of >Laszlo Ersek >Sent: Wednesday, May 22, 2019 2:50 AM >To: devel@edk2.groups.io; Lu, XiaoyuX <xiaoyux...@intel.com>; Rodriguez, >Christian <christian.rodrig...@intel.com> >Cc: Wang, Jian J <jian.j.w...@intel.com>; Ye, Ting <ting...@intel.com>; Zhu, >Yonghong <yonghong....@intel.com> >Subject: Re: [edk2-devel] [Patch V3] OpensslLib: Missing local header files in >[Sources] section of .INFs > >On 05/22/19 11:40, Xiaoyu Lu wrote: >> Hi Christian, >> >> (1) We use OpenSSL configure script disabled some OpenSSL feature. >> But I saw you include all .h files from OpenSSL(only excluded some). >> Even some header files we don't need (In openssl/crypto/). >> Can you rule them out? >> I found OpenSSl use configdata.pm to store configure result. >> >> use configdata qw/%config/; >> foreach my $enabledcryptomodule (@{config{"%sdirs"}}) >> } >> >> Is it possible? If this is difficult, then I have no problem. >> >> (2) see blow. >> >> (3) I think it's better to separate this patch into two. >> Patch1. process_files.pl >> Patch2. OpensslLib[Crypto].conf >> >>> -----Original Message----- >>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of >>> Christian Rodriguez >>> Sent: Wednesday, May 22, 2019 5:12 AM >>> To: devel@edk2.groups.io >>> Cc: Wang, Jian J <jian.j.w...@intel.com>; Ye, Ting >>> <ting...@intel.com>; Zhu, Yonghong <yonghong....@intel.com> >>> Subject: [edk2-devel] [Patch V3] OpensslLib: Missing local header >>> files in [Sources] section of .INFs >>> >>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1821 >>> >>> In V2: Remove opensslconf.h because it is a script generated header. >>> Update OpenSSL autogeneration script for .INFs because some OpenSSL >>> local headers are missing from [Sources] section of OpensslLib.inf >>> and OpensslLibCrypto.inf. Update OpensslLib.inf and >>> OpensslLibCrypto.inf using the updated script. Enforce compilance of >>> Edk2 INF Spec 3.9, which states, All HII Unicode format files must be >>> listed in [Sources] section. Not functional issue, just compilance. >>> >>> Signed-off-by: Christian Rodriguez <christian.rodrig...@intel.com> >>> Cc: Jian Wang <jian.j.w...@intel.com> >>> Cc: Ting Ye <ting...@intel.com> >>> Cc: Yonghong Zhu <yonghong....@intel.com> >>> --- >>> CryptoPkg/Library/OpensslLib/OpensslLib.inf | 173 >>> >+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>> >+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>> CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf | 167 >>> >+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>> >+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>> +++++++++++++++++++++++++++++++++++++++++++++++++ >>> CryptoPkg/Library/OpensslLib/process_files.pl | 50 >>> ++++++++++++++++++++++++++++++++++++++++++++++++-- >>> 3 files changed, 388 insertions(+), 2 deletions(-) >>> >>> diff --git a/CryptoPkg/Library/OpensslLib/process_files.pl >>> b/CryptoPkg/Library/OpensslLib/process_files.pl >>> index f6e1f43641..9cad6d3ebd 100755 >>> --- a/CryptoPkg/Library/OpensslLib/process_files.pl >>> +++ b/CryptoPkg/Library/OpensslLib/process_files.pl >>> @@ -115,12 +115,18 @@ BEGIN { >>> # Retrieve file lists from OpenSSL configdata # use configdata >>> qw/%unified_info/; >>> +use Cwd qw(cwd getcwd); >>> +use Cwd; >>> >>> my @cryptofilelist = (); >>> my @sslfilelist = (); >>> +my %includedirset = (); >>> foreach my $product ((@{$unified_info{libraries}}, >>> @{$unified_info{engines}})) { >>> foreach my $o (@{$unified_info{sources}->{$product}}) { >>> + foreach my $inc (@{%{$unified_info{includes}}{$o}}) { >>> + $includedirset{$inc} = 1; >>> + } >>> foreach my $s (@{$unified_info{sources}->{$o}}) { >>> next if ($unified_info{generate}->{$s}); >>> next if $s =~ "crypto/bio/b_print.c"; @@ -133,6 +139,46 >>> @@ foreach my $product ((@{$unified_info{libraries}}, >>> } >>> } >>> >>> +my $fullpathcwd = getcwd . '/' . $OPENSSL_PATH . '/'; my $cwdpath = >>> +getcwd . '/'; my @sslincludefilelist = (); my @cryptoincludefilelist >>> += (); # Current working directory header files foreach my $file >>> +(split (/\n/, `find . -maxdepth 1 -name "*.h"`)) { >>> + # Normalize path >>> + my @filearray = split("$cwdpath", Cwd::realpath($file)); >>> + my $pathstring = $filearray[1]; >>> + my $path = ' ' . $pathstring . "\r\n"; >>> + push @cryptoincludefilelist, $path; } # Header files below >>> +$OPENSSL_PATH foreach my $dir (keys %includedirset) { >> >> (2) If run process_files.pl twice, it possible generate different *.inf >> files(*.h files is disordered). "keys %includedirset" is disordered. >> I don't know whether this is a problem. > >Sounds like a good point to me -- in particular if the current file list >generation >for the INF files is "stable", then it should stay so even if we generate more >files for those sources sections. > >Thanks, >Laszlo > >> >>> + foreach my $file (split (/\n/, `find $OPENSSL_PATH/$dir/ -name >>> "*.h"`)) { >>> + # Normalize path >>> + my @filearray = split("$fullpathcwd", Cwd::realpath($file)); >>> + my $pathstring = $filearray[1]; >>> + my $path = ' $(OPENSSL_PATH)/' . $pathstring . "\r\n"; >>> + # Don't reuse duplicates >>> + next if ( $path ~~ @cryptoincludefilelist ); >>> + next if ( $path ~~ @sslincludefilelist ); >>> + # Ignore these types >>> + next if ( $path =~ "test" ); >>> + next if ( $path =~ "apps" ); >>> + next if ( $path =~ "engines" ); >>> + next if ( $path =~ "fuzz" ); >>> + next if ( $path =~ "os-dep" ); >>> + next if ( $path =~ "e_os.h" ); >>> + next if ( $path =~ "opensslconf.h"); >>> + # Seperate Ssl only headers >>> + if ( $path =~ "/ssl/" ) { >>> + push @sslincludefilelist, $path; >>> + next; >>> + } >>> + push @cryptoincludefilelist, $path; >>> + } >>> +} >>> + >>> + >>> # >>> # Update OpensslLib.inf with autogenerated file list # @@ -141,7 >>> +187,7 @@ my $subbing = 0; print "\n--> Updating OpensslLib.inf ... >>> "; foreach (@inf) { >>> if ( $_ =~ "# Autogenerated files list starts here" ) { >>> - push @new_inf, $_, @cryptofilelist, @sslfilelist; >>> + push @new_inf, $_, @cryptofilelist, @sslfilelist, >>> @cryptoincludefilelist, @sslincludefilelist; >>> $subbing = 1; >>> next; >>> } >>> @@ -184,7 +230,7 @@ $subbing = 0; >>> print "\n--> Updating OpensslLibCrypto.inf ... "; foreach (@inf) { >>> if ( $_ =~ "# Autogenerated files list starts here" ) { >>> - push @new_inf, $_, @cryptofilelist; >>> + push @new_inf, $_, @cryptofilelist, @cryptoincludefilelist; >>> $subbing = 1; >>> next; >>> } >>> -- >>> 2.19.1.windows.1 >>> >>> >>> >> >> >> >> > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#41232): https://edk2.groups.io/g/devel/message/41232 Mute This Topic: https://groups.io/mt/31713211/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
