From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adolf Belka To: development@lists.ipfire.org Subject: Re: [PATCH] perl-Encode-Locale: New module dependency for LWP::UserAgent Date: Sun, 19 Jun 2022 12:37:54 +0200 Message-ID: <23124135-c91c-83e8-96aa-b126712666dc@ipfire.org> In-Reply-To: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============4798279712292049795==" List-Id: --===============4798279712292049795== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi Rob and All, On 19/06/2022 00:48, Rob Brewer wrote: > Hi Adolf ++, > > On Saturday 18 June 2022 10:33 Adolf Belka wrote: > >> Hi Stefan, Rob and all, >> >> On 18/06/2022 10:29, Stefan Schantl wrote: >>> Hello Adolf, >>> >>> thanks for working on this and sending your patch. >>> >>> Everything looks good except the rootfile. When adding new perl modulles >>> they are some kind of special and must not contain any architecture >>> specific directory names. >>> >>> These needs to be replaced by a "xxxMACHINExxx". >> Duuuh. >> >> I know that but still managed to miss it. My only excuse is that I was >> on vacation for 2 weeks and did not work on any patches so forgot about >> that or didn't even see the architecture specific bits in the rootfile >> lines. :-) :-) >> >> >> I do have a question now about this patch. The need for it was triggered >> by an email from Rob. I had thought that he had identified error >> messages about the missing Encode::Locale for LWP::UserAgent in core >> parts of IPFire. >> >> However i see now from this message from Rob >> https://lists.ipfire.org/pipermail/development/2022-June/013691.html >> that it was identified from a personal add-on that he runs to generate >> ADSL graphs. >> >> In IPFire LWP::UserAgent is used for the IDS core package and for the >> Proxy Accounting add-on. I have both activated in my IPFire systems and >> checking the logs there are no entries at all related to Encode, encode, >> Locale or locale. So it looks like for IPFire its use of UserAgent.pm >> does not need the Encode/Locale.pm module. >> >> My feeling would therefore be that Encode::Locale should not be >> installed into the core package set unless my comments about >> UserAgent.pm use in IPFire has a fault in it. >> >> Rob could make an additional personal add-on for Encode::Locale so his >> ADSL data graphing add-on continues to work. >> >> It doesn't seem correct to add another package into the core IPFire set >> that won't be used by nearly all IPFire systems. >> >> Feedback welcome, especially if I have misunderstood or missed anything. >> >> Regards, >> >> Adolf. >> > I understand your concerns, in my application I use LWP::UserAgent to access > the router web page and this produced errors that Encode::Locale was missing > and failed to compile after the new UserAgent.pm was introduced on CU 165. > Checking my backups before CU 165 shows that 'require Encode::Locale' was > introduced into LWP::UserAgent in the version at the update to CU 165. > > It would seem that this latest version of UserAgent assumes that > Encode::Locale is present so I would be surprised that other programs using > UserAgent were not affected by the missing perl module. Valid point, so I had a search through the coding. I have gone through the UserAgent .pm code and also the places in IPFire that= specify the LWP::UserAgent module and found the following interesting result= s. In UserAgent.pm in the env_proxy subroutine both the Encode and Encode::Local= e modules are defined as Required. These two modules are completely separate = modules. The Encode::Locale modules says "The |Encode::Locale| module looks up the charset and encoding and arranges f= or the Encode module to know this encoding under the name "locale". It means = bytes obtained from the environment can be converted to Unicode strings by ca= lling |Encode::encode(locale =3D> $bytes)| and converted back again with |Enc= ode::decode(locale =3D> $string)|" In UserAgent, only the Encode::decode(locale =3D> $string) command is used. F= or something to be done with the output from this command there should be the= perl Encode module also installed but that is not present in IPFire. From wh= at you wrote it looks like you didn't need to install that module for your ad= don to work again. It looks like your addon is using the env_proxy subroutine= from UserAgent and that is what is triggering the need for Encode::Locale. I= nterestingly your addon is calling the env_proxy subroutine in UserAgent, whi= ch then decodes the locale and places the result in a variable called "locale= " for the Encode perl module to use if it was installed. In IPFire LWP::UserAgent is defined as "use" in ids-functions.pl, functions.p= l and accounting.cgi for the Proxy Accounting addon. In both ids-functions.pl and functions.pl the only call to UserAgent is for t= he new command and this is used to request SSL hostname verification and to s= pecify the CA file path. In accounting.cgi there is no subroutine call from UserAgent, so it looks lik= e it has been defined to use LWP::UserAgent but is not actually ever used. So= probably that use command could be removed from that cgi script. So the reason that IPFire does not get any error messages related to Encode::= Locale is that the env_proxy subroutine from UserAgent is not called for use = anywhere in IPFire. Based on this I think we should not install the Encode::Locale module and you= should add this perl module into the build of your addon. Regards, Adolf > Looking at my ADSL graph code there is nothing unusual about the use of > UserAgent and I raised this query expecting that other code such as that > mentioned above would be similarly affected as mine and this would help sort > problems elsewhere. If as you suggest that the other scripts on IPFire > aren't affected I am happy to make changes to my code. > > Regards > > Rob > > > >>> For more details see: >>> > https://git.ipfire.org/?p=3Dipfire-2.x.git;a=3Dblob;f=3Dconfig/rootfiles/co= mmon/perl-HTTP-Date;h=3D75c250b7bad745eb70cec3115e79e24d231d69c1;hb=3DHEAD > >>> Best regards, >>> >>> -Stefan >>> >>> Am 17. Juni 2022 22:50:45 schrieb Adolf Belka : >>> >>>> - UserAgent.pm now has a dependency on Encode/Locale.pm >>>> - lfs and rootfile created >>>> - Module added to make.sh >>>> >>>> Signed-off-by: Adolf Belka >>>> --- >>>> config/rootfiles/common/perl-Encode-Locale | 6 ++ >>>> lfs/perl-Encode-Locale | 79 ++++++++++++++++++++++ >>>> make.sh | 1 + >>>> 3 files changed, 86 insertions(+) >>>> create mode 100644 config/rootfiles/common/perl-Encode-Locale >>>> create mode 100644 lfs/perl-Encode-Locale >>>> >>>> diff --git a/config/rootfiles/common/perl-Encode-Locale >>>> b/config/rootfiles/common/perl-Encode-Locale >>>> new file mode 100644 >>>> index 000000000..b3c4d8fb7 >>>> --- /dev/null >>>> +++ b/config/rootfiles/common/perl-Encode-Locale >>>> @@ -0,0 +1,6 @@ >>>> +#usr/lib/perl5/site_perl/5.32.1/Encode >>>> +usr/lib/perl5/site_perl/5.32.1/Encode/Locale.pm >>>> +#usr/lib/perl5/site_perl/5.32.1/x86_64-linux-thread-multi/auto/Encode >>>> +#usr/lib/perl5/site_perl/5.32.1/x86_64-linux-thread- > multi/auto/Encode/Locale >>>> +#usr/lib/perl5/site_perl/5.32.1/x86_64-linux-thread- > multi/auto/Encode/Locale/.packlist >>>> +#usr/share/man/man3/Encode::Locale.3 >>>> diff --git a/lfs/perl-Encode-Locale b/lfs/perl-Encode-Locale >>>> new file mode 100644 >>>> index 000000000..a51208971 >>>> --- /dev/null >>>> +++ b/lfs/perl-Encode-Locale >>>> @@ -0,0 +1,79 @@ >>>> > +##########################################################################= ##### >>>> +# >>>> # >>>> +# IPFire.org - A linux based firewall >>>> # >>>> +# Copyright (C) 2007-2019 IPFire Team >>>> # >>>> +# >>>> # >>>> +# This program is free software: you can redistribute it and/or >>>> modify # >>>> +# it under the terms of the GNU General Public License as published >>>> by # >>>> +# the Free Software Foundation, either version 3 of the License, or >>>> # >>>> +# (at your option) any later version. >>>> # >>>> +# >>>> # >>>> +# This program is distributed in the hope that it will be useful, >>>> # >>>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of >>>> # >>>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>>> # >>>> +# GNU General Public License for more details. >>>> # >>>> +# >>>> # >>>> +# You should have received a copy of the GNU General Public License >>>> # >>>> +# along with this program. If not, see >>>> . # >>>> +# >>>> # >>>> > +##########################################################################= ##### >>>> + >>>> + >>>> > +##########################################################################= ##### >>>> +# Definitions >>>> > +##########################################################################= ##### >>>> +include Config >>>> +VER =3D 1.05 >>>> + >>>> +THISAPP =3D Encode-Locale-$(VER) >>>> +DL_FILE =3D ${THISAPP}.tar.gz >>>> +DL_FROM =3D $(URL_IPFIRE) >>>> +DIR_APP =3D $(DIR_SRC)/$(THISAPP) >>>> +TARGET =3D $(DIR_INFO)/$(THISAPP) >>>> + >>>> > +##########################################################################= ##### >>>> +# Top-level Rules >>>> > +##########################################################################= ##### >>>> + >>>> +objects =3D $(DL_FILE) >>>> + >>>> +$(DL_FILE) =3D $(DL_FROM)/$(DL_FILE) >>>> + >>>> +$(DL_FILE)_BLAKE2 =3D >>>> > f66bac8ebf012e7673b344b3899bed755558b80833a68b009b6083aeadd9d69748a63bee4e5= e3c20dffaf7f2551fd6c9c778273ae992752c426e081d35715dee >>>> + >>>> +install : $(TARGET) >>>> + >>>> +check : $(patsubst %,$(DIR_CHK)/%,$(objects)) >>>> + >>>> +download :$(patsubst %,$(DIR_DL)/%,$(objects)) >>>> + >>>> +b2 : $(subst %,%_BLAKE2,$(objects)) >>>> + >>>> +dist: >>>> + @$(PAK) >>>> + >>>> > +##########################################################################= ##### >>>> +# Downloading, checking, b2sum >>>> > +##########################################################################= ##### >>>> + >>>> +$(patsubst %,$(DIR_CHK)/%,$(objects)) : >>>> + @$(CHECK) >>>> + >>>> +$(patsubst %,$(DIR_DL)/%,$(objects)) : >>>> + @$(LOAD) >>>> + >>>> +$(subst %,%_BLAKE2,$(objects)) : >>>> + @$(B2SUM) >>>> + >>>> > +##########################################################################= ##### >>>> +# Installation Details >>>> > +##########################################################################= ##### >>>> + >>>> +$(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects)) >>>> + @$(PREBUILD) >>>> + @rm -rf $(DIR_APP) && cd $(DIR_SRC) && tar zxf $(DIR_DL)/$(DL_FILE) >>>> + cd $(DIR_APP) && perl Makefile.PL >>>> + cd $(DIR_APP) && make $(MAKETUNING) $(EXTRA_MAKE) >>>> + cd $(DIR_APP) && make install >>>> + @rm -rf $(DIR_APP) >>>> + @$(POSTBUILD) >>>> diff --git a/make.sh b/make.sh >>>> index 2a4f6d0bd..dd84cdc99 100755 >>>> --- a/make.sh >>>> +++ b/make.sh >>>> @@ -1373,6 +1373,7 @@ buildipfire() { >>>> lfsmake2 perl-Digest >>>> lfsmake2 perl-Digest-SHA1 >>>> lfsmake2 perl-Digest-HMAC >>>> + lfsmake2 perl-Encode-Locale >>>> lfsmake2 perl-libwww >>>> lfsmake2 perl-LWP-Protocol-https >>>> lfsmake2 perl-Net-HTTP >>>> -- >>>> 2.36.1 --===============4798279712292049795==--