Filter non-BuildTarget paths from extra_paths - #16011
Conversation
In Postgres, we saw an issue[0] where translation targets (custom targets) parent directories were being added to PATH on Windows. Generally, this would not be a problem. However, Windows limits the length of environment variables. Given the amount of translation targets in Postgres, this led to PATH being longer than what Windows allows and tests would fail to run. By filtering non-BuildTarget dependencies, we can mitigate the problem. Windows searches for DLLs and executables on PATH, so because we can assume that non-BuildTarget dependencies are not going to be DLLs or executables, we can safely filter them out of the extra_paths list. Link: https://www.postgresql.org/message-id/CAD5tBcJXAX8bz5q9ngE4znvVLiXaPO89D0yTFhJgEPrPyT-Erw@mail.gmail.com [0] Signed-off-by: Tristan Partin <tristan@partin.io>
|
I believe based on discussion in Matrix that Luca is also suggesting that we also include |
|
Does this break the |
I was thinking about this, and I guess it would. I'm not sure the best way to work around that. |
|
In Postgres, the things that are causing us trouble are the |
|
I was thinking something like that: diff --git a/mesonbuild/backend/backends.py b/mesonbuild/backend/backends.py
index 21cc8bb0a..81acf9cbb 100644
--- a/mesonbuild/backend/backends.py
+++ b/mesonbuild/backend/backends.py
@@ -1216,7 +1216,21 @@ class Backend:
extra_bdeps: T.List[build.BuildTargetTypes] = []
if isinstance(exe, build.CustomTarget):
extra_bdeps = list(exe.get_transitive_build_target_deps())
- extra_bdeps.extend(t.depends)
+
+ # Don't include the entire t.depends to avoid hitting limmits
+ # on the environment block size (about 32k characters). See
+ # https://github.com/mesonbuild/meson/issues/16010
+ for d in t.depends:
+ if isinstance(d, (build.BuildTarget, build.Executable)):
+ extra_bdeps.add(d)
+ for l in d.get_all_link_deps():
+ if isinstance(l, build.SharedLibrary):
+ extra_bdeps.add(l)
+ elif isinstance(d, build.LocalProgram):
+ pass # TODO
+ elif isinstance(d, programs.ExternalProgram):
+ extra_bdeps.add(d)
+
extra_bdeps.extend(a for a in t.cmd_args if isinstance(a, build.BuildTarget))
extra_paths = self.determine_windows_extra_paths(exe, extra_bdeps)
else:Maybe we can call |
I'd just go for a middle ground: If a In general you never can tell. For example, a depend may be a Python module generated via a Custom target. In that case you don't want to add to PATH, but to PYTHONPATH. Only the user knows Note: we're already filtering on UNIX: https://github.com/mesonbuild/meson/blob/master/mesonbuild/backend/backends.py#L1254 |
determine_windows_extra_paths() on non-Windows is not used for too much.
It is used in two instances. In those two instances, extra_paths has very
narrow use cases.
- Backend::get_devenv(): we add to extra_paths for every
build.Executable. This also happens on Windows.
- Backend::get_executable_serialisation(): extra_paths is empty on
non-Windows.
It is important to remember that Windows has no equivalent to
{DY,}LD_LIBRARY_PATH. Instead it looks up DLLs on PATH. Performing this
filter will have an adverse effect on adding build outputs from
build.CustomTarget to PATH. However, when setting {DY,}LD_LIBRARY_PATH,
we filter for build.SharedLibrary explicitly, so it was an oversight in
the original patch[0] to include this additional behavior.
Link: mesonbuild#14649 [0]
Fixes: mesonbuild#16010
Supersedes: mesonbuild#16011
Signed-off-by: Tristan Partin <tristan@partin.io>
determine_windows_extra_paths() on non-Windows is not used for too much.
It is used in two instances. In those two instances, extra_paths has very
narrow use cases.
- Backend::get_devenv(): we add to extra_paths for every
build.Executable. This also happens on Windows.
- Backend::get_executable_serialisation(): extra_paths is empty on
non-Windows.
It is important to remember that Windows has no equivalent to
{DY,}LD_LIBRARY_PATH. Instead it looks up DLLs on PATH. Performing this
filter will have an adverse effect on adding build outputs from
build.CustomTarget to PATH. However, when setting {DY,}LD_LIBRARY_PATH,
we filter for build.SharedLibrary explicitly, so it was an oversight in
the original patch[0] to include this additional behavior.
Link: mesonbuild#14649 [0]
Fixes: mesonbuild#16010
Supersedes: mesonbuild#16011
Signed-off-by: Tristan Partin <tristan@partin.io>
In Postgres, we saw an issue[0] where translation targets (custom targets) parent directories were being added to PATH on Windows. Generally, this would not be a problem. However, Windows limits the length of environment variables. Given the amount of translation targets in Postgres, this led to PATH being longer than what Windows allows and tests would fail to run.
By filtering non-BuildTarget dependencies, we can mitigate the problem. Windows searches for DLLs and executables on PATH, so because we can assume that non-BuildTarget dependencies are not going to be DLLs or executables, we can safely filter them out of the extra_paths list.
Link: https://www.postgresql.org/message-id/CAD5tBcJXAX8bz5q9ngE4znvVLiXaPO89D0yTFhJgEPrPyT-Erw@mail.gmail.com [0]