[PATCH] tests: ignore $__CF_USER_TEXT_ENCODING in env during commandtest

Laine Stump posted 1 patch 5 months, 3 weeks ago
Failed in applying to current master (apply log)
tests/commandhelper.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
[PATCH] tests: ignore $__CF_USER_TEXT_ENCODING in env during commandtest
Posted by Laine Stump 5 months, 3 weeks ago
This environment variable is supposedly set according to the contents
of ~/.CFUserTextEncoding, and certainly on MacOS 14 (Sonoma) it shows
up in the environment, causing commandtest to fail. However, the value
that is shown in $__CF_USER_TEXT_ENCODING during the test 1) is not in
the environment of the shell the test is run from, and 2) doesn't
match the contents of ~/.CFUserTextEncoding.

It is true, though, that filtering out this environment setting from
the test results permits commandtest to pass on MacOS 14.

Signed-off-by: Laine Stump <laine@redhat.com>
---

[*] There may be a better way to suppress this environment setting
    (maybe something done to prevent it from ever being added to the
    environment in the first place?), and that would be fine too. This
    patch does work though.

[*] Andrea's patches to force rpcgen to generate ANSI C code are also
    required for the test suite to pass on MacOS 14.

 tests/commandhelper.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tests/commandhelper.c b/tests/commandhelper.c
index 9b56feb120..08ee48a3a8 100644
--- a/tests/commandhelper.c
+++ b/tests/commandhelper.c
@@ -169,9 +169,12 @@ static int printEnvironment(FILE *log)
 
     for (i = 0; i < length; i++) {
         /* Ignore the variables used to instruct the loader into
-         * behaving differently, as they could throw the tests off. */
-        if (!STRPREFIX(newenv[i], "LD_"))
+         * behaving differently, as they could throw the tests off.
+         * Also ignore __CF_USER_TEXT_ENCODING, which is set by MacOS. */
+        if (!STRPREFIX(newenv[i], "LD_") &&
+            !STRPREFIX(newenv[i], "__CF_USER_TEXT_ENCODING=")) {
             fprintf(log, "ENV:%s\n", newenv[i]);
+        }
     }
 
     return 0;
-- 
2.41.0
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH] tests: ignore $__CF_USER_TEXT_ENCODING in env during commandtest
Posted by Daniel P. Berrangé 5 months, 3 weeks ago
On Fri, Nov 03, 2023 at 12:00:30AM -0400, Laine Stump wrote:
> This environment variable is supposedly set according to the contents
> of ~/.CFUserTextEncoding, and certainly on MacOS 14 (Sonoma) it shows
> up in the environment, causing commandtest to fail. However, the value
> that is shown in $__CF_USER_TEXT_ENCODING during the test 1) is not in
> the environment of the shell the test is run from, and 2) doesn't
> match the contents of ~/.CFUserTextEncoding.
> 
> It is true, though, that filtering out this environment setting from
> the test results permits commandtest to pass on MacOS 14.
> 
> Signed-off-by: Laine Stump <laine@redhat.com>

I guess this is being set by macOS libc runtime somewhere.

> ---
> 
> [*] There may be a better way to suppress this environment setting
>     (maybe something done to prevent it from ever being added to the
>     environment in the first place?), and that would be fine too. This
>     patch does work though.
> 
> [*] Andrea's patches to force rpcgen to generate ANSI C code are also
>     required for the test suite to pass on MacOS 14.
> 
>  tests/commandhelper.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/commandhelper.c b/tests/commandhelper.c
> index 9b56feb120..08ee48a3a8 100644
> --- a/tests/commandhelper.c
> +++ b/tests/commandhelper.c
> @@ -169,9 +169,12 @@ static int printEnvironment(FILE *log)
>  
>      for (i = 0; i < length; i++) {
>          /* Ignore the variables used to instruct the loader into
> -         * behaving differently, as they could throw the tests off. */
> -        if (!STRPREFIX(newenv[i], "LD_"))
> +         * behaving differently, as they could throw the tests off.
> +         * Also ignore __CF_USER_TEXT_ENCODING, which is set by MacOS. */
> +        if (!STRPREFIX(newenv[i], "LD_") &&
> +            !STRPREFIX(newenv[i], "__CF_USER_TEXT_ENCODING=")) {
>              fprintf(log, "ENV:%s\n", newenv[i]);
> +        }

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH] tests: ignore $__CF_USER_TEXT_ENCODING in env during commandtest
Posted by Andrea Bolognani 5 months, 3 weeks ago
On Fri, Nov 03, 2023 at 12:00:30AM -0400, Laine Stump wrote:
> [*] There may be a better way to suppress this environment setting
>     (maybe something done to prevent it from ever being added to the
>     environment in the first place?), and that would be fine too. This
>     patch does work though.

Yeah, I think this is pretty safe but it's kinda weird that this
specific environment variable would make it through despite our
filtering mechanisms in virCommand. So if some good soul with access
to a macOS 14 installation (*cough cough* ;) could try to figure out
why this started showing up all of a sudden, when it was not there
for any previous version of macOS, that would be absolutely awesome.

> +++ b/tests/commandhelper.c
> @@ -169,9 +169,12 @@ static int printEnvironment(FILE *log)
>
>      for (i = 0; i < length; i++) {
>          /* Ignore the variables used to instruct the loader into
> -         * behaving differently, as they could throw the tests off. */
> -        if (!STRPREFIX(newenv[i], "LD_"))
> +         * behaving differently, as they could throw the tests off.
> +         * Also ignore __CF_USER_TEXT_ENCODING, which is set by MacOS. */

s/MacOS/macOS/ (in the commit message too)

Reviewed-by: Andrea Bolognani <abologna@redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH] tests: ignore $__CF_USER_TEXT_ENCODING in env during commandtest
Posted by Andrea Bolognani 5 months, 3 weeks ago
On Fri, Nov 03, 2023 at 03:11:07AM -0700, Andrea Bolognani wrote:
> On Fri, Nov 03, 2023 at 12:00:30AM -0400, Laine Stump wrote:
> > [*] There may be a better way to suppress this environment setting
> >     (maybe something done to prevent it from ever being added to the
> >     environment in the first place?), and that would be fine too. This
> >     patch does work though.
>
> Yeah, I think this is pretty safe but it's kinda weird that this
> specific environment variable would make it through despite our
> filtering mechanisms in virCommand. So if some good soul with access
> to a macOS 14 installation (*cough cough* ;) could try to figure out
> why this started showing up all of a sudden, when it was not there
> for any previous version of macOS, that would be absolutely awesome.

Oh, and of course the same person could also look at all the various

  ld: warning: -undefined error is deprecated
  ld: warning: ignoring duplicate libraries: '-lxml2'

that are printed during build O:-)

-- 
Andrea Bolognani / Red Hat / Virtualization
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH] tests: ignore $__CF_USER_TEXT_ENCODING in env during commandtest
Posted by Daniel P. Berrangé 5 months, 3 weeks ago
On Fri, Nov 03, 2023 at 03:19:17AM -0700, Andrea Bolognani wrote:
> On Fri, Nov 03, 2023 at 03:11:07AM -0700, Andrea Bolognani wrote:
> > On Fri, Nov 03, 2023 at 12:00:30AM -0400, Laine Stump wrote:
> > > [*] There may be a better way to suppress this environment setting
> > >     (maybe something done to prevent it from ever being added to the
> > >     environment in the first place?), and that would be fine too. This
> > >     patch does work though.
> >
> > Yeah, I think this is pretty safe but it's kinda weird that this
> > specific environment variable would make it through despite our
> > filtering mechanisms in virCommand. So if some good soul with access
> > to a macOS 14 installation (*cough cough* ;) could try to figure out
> > why this started showing up all of a sudden, when it was not there
> > for any previous version of macOS, that would be absolutely awesome.
> 
> Oh, and of course the same person could also look at all the various
> 
>   ld: warning: -undefined error is deprecated

Meson add "-undefined,error" and macOS no longer likes that:

  https://github.com/mesonbuild/meson/issues/12450

>   ld: warning: ignoring duplicate libraries: '-lxml2'

There's a similarish warning reported against homebew

  https://github.com/orgs/Homebrew/discussions/4794

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH] tests: ignore $__CF_USER_TEXT_ENCODING in env during commandtest
Posted by Daniel P. Berrangé 5 months, 3 weeks ago
On Fri, Nov 03, 2023 at 10:41:51AM +0000, Daniel P. Berrangé wrote:
> On Fri, Nov 03, 2023 at 03:19:17AM -0700, Andrea Bolognani wrote:
> > On Fri, Nov 03, 2023 at 03:11:07AM -0700, Andrea Bolognani wrote:
> > > On Fri, Nov 03, 2023 at 12:00:30AM -0400, Laine Stump wrote:
> > > > [*] There may be a better way to suppress this environment setting
> > > >     (maybe something done to prevent it from ever being added to the
> > > >     environment in the first place?), and that would be fine too. This
> > > >     patch does work though.
> > >
> > > Yeah, I think this is pretty safe but it's kinda weird that this
> > > specific environment variable would make it through despite our
> > > filtering mechanisms in virCommand. So if some good soul with access
> > > to a macOS 14 installation (*cough cough* ;) could try to figure out
> > > why this started showing up all of a sudden, when it was not there
> > > for any previous version of macOS, that would be absolutely awesome.
> > 
> > Oh, and of course the same person could also look at all the various
> > 
> >   ld: warning: -undefined error is deprecated
> 
> Meson add "-undefined,error" and macOS no longer likes that:
> 
>   https://github.com/mesonbuild/meson/issues/12450
> 
> >   ld: warning: ignoring duplicate libraries: '-lxml2'
> 
> There's a similarish warning reported against homebew
> 
>   https://github.com/orgs/Homebrew/discussions/4794

Actually I think it is more mundane and a stupid warning:

% gcc c.c -lxml2

% gcc c.c -lxml2 -lxml2
ld: warning: ignoring duplicate libraries: '-lxml2'


oooooh soo many projects have the same -lXXX repeated mamy times when
building, as build systems don't make it easy to merge duplicates.
Warning about this is madness :-(

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH] tests: ignore $__CF_USER_TEXT_ENCODING in env during commandtest
Posted by Andrea Bolognani 5 months, 3 weeks ago
On Fri, Nov 03, 2023 at 10:47:11AM +0000, Daniel P. Berrangé wrote:
> On Fri, Nov 03, 2023 at 10:41:51AM +0000, Daniel P. Berrangé wrote:
> > On Fri, Nov 03, 2023 at 03:19:17AM -0700, Andrea Bolognani wrote:
> > >   ld: warning: ignoring duplicate libraries: '-lxml2'
> >
> > There's a similarish warning reported against homebew
> >
> >   https://github.com/orgs/Homebrew/discussions/4794
>
> Actually I think it is more mundane and a stupid warning:
>
> % gcc c.c -lxml2
>
> % gcc c.c -lxml2 -lxml2
> ld: warning: ignoring duplicate libraries: '-lxml2'
>
> oooooh soo many projects have the same -lXXX repeated mamy times when
> building, as build systems don't make it easy to merge duplicates.
> Warning about this is madness :-(

Yeah I really don't see the point, seems like it's there just to be
annoying :(

-- 
Andrea Bolognani / Red Hat / Virtualization
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH] tests: ignore $__CF_USER_TEXT_ENCODING in env during commandtest
Posted by Daniel P. Berrangé 5 months, 3 weeks ago
On Fri, Nov 03, 2023 at 04:13:12AM -0700, Andrea Bolognani wrote:
> On Fri, Nov 03, 2023 at 10:47:11AM +0000, Daniel P. Berrangé wrote:
> > On Fri, Nov 03, 2023 at 10:41:51AM +0000, Daniel P. Berrangé wrote:
> > > On Fri, Nov 03, 2023 at 03:19:17AM -0700, Andrea Bolognani wrote:
> > > >   ld: warning: ignoring duplicate libraries: '-lxml2'
> > >
> > > There's a similarish warning reported against homebew
> > >
> > >   https://github.com/orgs/Homebrew/discussions/4794
> >
> > Actually I think it is more mundane and a stupid warning:
> >
> > % gcc c.c -lxml2
> >
> > % gcc c.c -lxml2 -lxml2
> > ld: warning: ignoring duplicate libraries: '-lxml2'
> >
> > oooooh soo many projects have the same -lXXX repeated mamy times when
> > building, as build systems don't make it easy to merge duplicates.
> > Warning about this is madness :-(
> 
> Yeah I really don't see the point, seems like it's there just to be
> annoying :(

We can turn it off with:

  -Wl,-no_warn_duplicate_libraries

but need to probe for whether that option exists or not.

https://indiestack.com/2023/10/xcode-15-duplicate-library-linker-warnings/

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH] tests: ignore $__CF_USER_TEXT_ENCODING in env during commandtest
Posted by Andrea Bolognani 5 months, 3 weeks ago
On Fri, Nov 03, 2023 at 11:17:03AM +0000, Daniel P. Berrangé wrote:
> On Fri, Nov 03, 2023 at 04:13:12AM -0700, Andrea Bolognani wrote:
> > On Fri, Nov 03, 2023 at 10:47:11AM +0000, Daniel P. Berrangé wrote:
> > > On Fri, Nov 03, 2023 at 10:41:51AM +0000, Daniel P. Berrangé wrote:
> > > > On Fri, Nov 03, 2023 at 03:19:17AM -0700, Andrea Bolognani wrote:
> > > > >   ld: warning: ignoring duplicate libraries: '-lxml2'
> > > >
> > > > There's a similarish warning reported against homebew
> > > >
> > > >   https://github.com/orgs/Homebrew/discussions/4794
> > >
> > > Actually I think it is more mundane and a stupid warning:
> > >
> > > % gcc c.c -lxml2
> > >
> > > % gcc c.c -lxml2 -lxml2
> > > ld: warning: ignoring duplicate libraries: '-lxml2'
> > >
> > > oooooh soo many projects have the same -lXXX repeated mamy times when
> > > building, as build systems don't make it easy to merge duplicates.
> > > Warning about this is madness :-(
> >
> > Yeah I really don't see the point, seems like it's there just to be
> > annoying :(
>
> We can turn it off with:
>
>   -Wl,-no_warn_duplicate_libraries
>
> but need to probe for whether that option exists or not.
>
> https://indiestack.com/2023/10/xcode-15-duplicate-library-linker-warnings/

Laine, can you please cook up a patch for this?

-- 
Andrea Bolognani / Red Hat / Virtualization
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH] tests: ignore $__CF_USER_TEXT_ENCODING in env during commandtest
Posted by Laine Stump 5 months, 3 weeks ago
On 11/3/23 8:41 AM, Andrea Bolognani wrote:
> On Fri, Nov 03, 2023 at 11:17:03AM +0000, Daniel P. Berrangé wrote:
>> On Fri, Nov 03, 2023 at 04:13:12AM -0700, Andrea Bolognani wrote:
>>> On Fri, Nov 03, 2023 at 10:47:11AM +0000, Daniel P. Berrangé wrote:
>>>> On Fri, Nov 03, 2023 at 10:41:51AM +0000, Daniel P. Berrangé wrote:
>>>>> On Fri, Nov 03, 2023 at 03:19:17AM -0700, Andrea Bolognani wrote:
>>>>>>    ld: warning: ignoring duplicate libraries: '-lxml2'
>>>>>
>>>>> There's a similarish warning reported against homebew
>>>>>
>>>>>    https://github.com/orgs/Homebrew/discussions/4794
>>>>
>>>> Actually I think it is more mundane and a stupid warning:
>>>>
>>>> % gcc c.c -lxml2
>>>>
>>>> % gcc c.c -lxml2 -lxml2
>>>> ld: warning: ignoring duplicate libraries: '-lxml2'
>>>>
>>>> oooooh soo many projects have the same -lXXX repeated mamy times when
>>>> building, as build systems don't make it easy to merge duplicates.
>>>> Warning about this is madness :-(
>>>
>>> Yeah I really don't see the point, seems like it's there just to be
>>> annoying :(
>>
>> We can turn it off with:
>>
>>    -Wl,-no_warn_duplicate_libraries
>>
>> but need to probe for whether that option exists or not.
>>
>> https://indiestack.com/2023/10/xcode-15-duplicate-library-linker-warnings/
> 
> Laine, can you please cook up a patch for this?
> 

Yeah, I think I see how that's done. I need to take the dog for a walk 
first though :-)
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH] tests: ignore $__CF_USER_TEXT_ENCODING in env during commandtest
Posted by Laine Stump 5 months, 3 weeks ago
On 11/3/23 6:11 AM, Andrea Bolognani wrote:
> On Fri, Nov 03, 2023 at 12:00:30AM -0400, Laine Stump wrote:
>> [*] There may be a better way to suppress this environment setting
>>      (maybe something done to prevent it from ever being added to the
>>      environment in the first place?), and that would be fine too. This
>>      patch does work though.
> 
> Yeah, I think this is pretty safe but it's kinda weird that this
> specific environment variable would make it through despite our
> filtering mechanisms in virCommand. So if some good soul with access
> to a macOS 14 installation (*cough cough* ;) could try to figure out
> why this started showing up all of a sudden, when it was not there
> for any previous version of macOS, that would be absolutely awesome.

Well, I have access to macos 14, but not to anything earlier. It seems 
pretty cut/dried that our virCommand gives the child process a very 
limited environment, so it Dan's supposition must be correct. Possibly 
it can be eliminated by setting the C locale or something?
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org