vsenv: fix ValueError parsing multi-line env variables

[ylatuya]

This helps debugging issues like https://gitlab.freedesktop.org/ylatuya/gstreamer/-/jobs/63636384#L259.

Another question is if it should be treated as an error or ignored like it's done when parsing the key-value string a few lines above.

[eli-schwartz]

Did you try it again with this patch to meson? What was the reported "illegal variable name"?

[ylatuya]

I found the root issue... So the CI adds an env variable CI_MERGE_REQUEST_DESCRIPTION with the description of the issue and containing this (edited for format see the original ):

CI_MERGE_REQUEST_DESCRIPTION=The following MR migrates all the existing code/format/message checks to [pre-commit](https://pre-commit.com/):
- gitlint: Commit messages with gitlint
- rustfmt: Rust format checks
- gst-indent: C format checks
- dotnet-format: C# format checks
Caching of dependencies follows the same decisions discussed in https://gitlab.freedesktop.org/gstreamer/cerbero/-/merge_requests/1315 https://gitlab.freedesktop.org/gstreamer/cerbero/-/merge_requests/1322.
## Installation
➜ pre-commit run  --show-diff-on-failure --files subprojects/gstreamer-sharp/sources/custom/Caps.cs subprojects/gstreamer/libs/gst/helpers/ptp/args.rs subprojects/gstreamer/gst/gst.c  
dotnet-format............................................................Passed
gst-indent...............................................................Failed
- hook id: gst-indent
- exit code: 1
--Checking style--
--- .merge_file_YzhXVL  2024-01-14 20:35:33
+++ /tmp/.merge_file_YzhXVL.ut9WE0      2024-01-14 20:35:33
@@ -1291,7 +1335,7 @@ gst_version_string (void)
     return g_strdup_printf ("GStreamer %d.%d.%d (GIT)", major, minor, micro);
   else
     return g_strdup_printf ("GStreamer %d.%d.%d (prerelease)", major, minor,
-        micro);
+                           micro);
 }
 
 /**
=================================================================================================
 Code style error in: subprojects/gstreamer/gst/gst.c                                                                      
=================================================================================================
=================================================================================================
 Code style error in:                                                                            
   subprojects/gstreamer/gst/gst.c
                                                                                                 
 Please fix before committing. Don't forget to run git add before trying to commit again.        
 If the whole file is to be committed, this should work (ru...

And well... that is broken into lines bat_lines = bat_output.split("\n") and split by = with k, v = bat_line.split("=", 1) ending with k being empty and v =====

[ylatuya]

I updated the unit tests with 2 env variables that replicates the 2 possible errors:

• A line with ==== that would result in an empty key
• A line with %=bar that would result in k being % and therefore an invalid name

[eli-schwartz]

I wonder if it's instead possible to filter those out from subprocess.check_output(bat_file.name, ...) by passing a copied+filtered env, on the rationale that those variables won't affect the VS detection process and having the bat script print them out at all is wasteful.

[ylatuya]

I wonder if it's instead possible to filter those out from subprocess.check_output(bat_file.name, ...) by passing a copied+filtered env, on the rationale that those variables won't affect the VS detection process and having the bat script print them out at all is wasteful.

That is an optimization I can implement. I will also change the code to call the script directly with subprocess instead of having to create a temporary file, that will speed up things as well.

[eli-schwartz]

That is an optimization I can implement. I will also change the code to call the script directly with subprocess instead of having to create a temporary file, that will speed up things as well.

I had been thinking to do something like

newenv = {k:v for k, v in os.environ.items() if '\n' not in v}
bat_output = subprocess.check_output(bat_file.name, env=newenv, universal_newlines=True,
                                     encoding=locale.getpreferredencoding(False))

Or if we knew the minimal set of existing variables the vcvars*.bat file needs to be functional, we could make the new environment only contain those.

I suppose iterating over the set difference of lines would work too as long as none of the different lines are themselves part of a multiline variable which I suppose vcvars should not be producing. It just felt a bit weird to be recovering via random sort. But maybe it's fine.

[eli-schwartz]

The test errors appear to be relevant but I do not understand them.

[jpakkane]

The error messages come from trying to use the GCC compiler. So maybe PATH is messed up somehow?

[ylatuya]

The error messages come from trying to use the GCC compiler. So maybe PATH is messed up somehow?

Somehow the environment is not being set correctly after my changes. I will iterate again over the code and reproduce the error locally with the tests.