| Message ID | 20250129-b4-pks-meson-improvements-v1-8-ab709f0be12c@pks.im (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | meson: cleanups, improvements, smallish fixes | expand |
On 25/01/29 08:12AM, Patrick Steinhardt wrote: > We're setting up the PATH environment variable such that a set of > necessary build tools can be found at build time. Make this step a bit > less repetitive by only looping through the set of found programs once. > > Signed-off-by: Patrick Steinhardt <ps@pks.im> > --- > meson.build | 17 ++++++++--------- > 1 file changed, 8 insertions(+), 9 deletions(-) > > diff --git a/meson.build b/meson.build > index 48eb068fd8..e3829f2365 100644 > --- a/meson.build > +++ b/meson.build > @@ -181,22 +181,21 @@ if host_machine.system() == 'windows' > program_path += [ 'C:/Program Files/Git/bin', 'C:/Program Files/Git/usr/bin' ] > endif > > +cat = find_program('cat', dirs: program_path) > cygpath = find_program('cygpath', dirs: program_path, required: false) > diff = find_program('diff', dirs: program_path) > +git = find_program('git', dirs: program_path, required: false) > +grep = find_program('grep', dirs: program_path) > +sed = find_program('sed', dirs: program_path) > shell = find_program('sh', dirs: program_path) > tar = find_program('tar', dirs: program_path) At first I was curious to why we wouldn't just invoke `find_program()` during the loop as well, but some of these programs are not required and marked as such here. > script_environment = environment() > -foreach tool : ['cat', 'grep', 'sed'] > - program = find_program(tool, dirs: program_path) > - script_environment.prepend('PATH', fs.parent(program.full_path())) > +foreach program : [cat, cygpath, diff, git, grep, sed, shell, tar] > + if program.found() > + script_environment.prepend('PATH', fs.parent(program.full_path())) > + endif It looks like cygpath, diff, shell, and tar were previously not being appended to the path environment. With this change now they are. > endforeach > - > -git = find_program('git', dirs: program_path, required: false) > -if git.found() > - script_environment.prepend('PATH', fs.parent(git.full_path())) > -endif > - > if get_option('sane_tool_path') != '' > script_environment.prepend('PATH', get_option('sane_tool_path')) > endif > > -- > 2.48.1.362.g079036d154.dirty > >
On Wed, Jan 29, 2025 at 02:42:07PM -0600, Justin Tobler wrote: > On 25/01/29 08:12AM, Patrick Steinhardt wrote: > > script_environment = environment() > > -foreach tool : ['cat', 'grep', 'sed'] > > - program = find_program(tool, dirs: program_path) > > - script_environment.prepend('PATH', fs.parent(program.full_path())) > > +foreach program : [cat, cygpath, diff, git, grep, sed, shell, tar] > > + if program.found() > > + script_environment.prepend('PATH', fs.parent(program.full_path())) > > + endif > > It looks like cygpath, diff, shell, and tar were previously not being > appended to the path environment. With this change now they are. You know, I think I've been approaching this from the wrong angle. It's not like we need to add these tools to PATH in case they have been found via the usual PATH lookup: Meson knows to remember PATH just fine, so the scripts would be able to find them anyway. The actual issue is that we sometimes end up looking up programs via something else but PATH, namely on Windows, where we may instead look up programs via the Git for Windows installation. So the proper way to handle this is to add these system-specific paths to PATH, not every single binary's parent directory. Will adapt, thanks for making me rethink. Patrick
diff --git a/meson.build b/meson.build index 48eb068fd8..e3829f2365 100644 --- a/meson.build +++ b/meson.build @@ -181,22 +181,21 @@ if host_machine.system() == 'windows' program_path += [ 'C:/Program Files/Git/bin', 'C:/Program Files/Git/usr/bin' ] endif +cat = find_program('cat', dirs: program_path) cygpath = find_program('cygpath', dirs: program_path, required: false) diff = find_program('diff', dirs: program_path) +git = find_program('git', dirs: program_path, required: false) +grep = find_program('grep', dirs: program_path) +sed = find_program('sed', dirs: program_path) shell = find_program('sh', dirs: program_path) tar = find_program('tar', dirs: program_path) script_environment = environment() -foreach tool : ['cat', 'grep', 'sed'] - program = find_program(tool, dirs: program_path) - script_environment.prepend('PATH', fs.parent(program.full_path())) +foreach program : [cat, cygpath, diff, git, grep, sed, shell, tar] + if program.found() + script_environment.prepend('PATH', fs.parent(program.full_path())) + endif endforeach - -git = find_program('git', dirs: program_path, required: false) -if git.found() - script_environment.prepend('PATH', fs.parent(git.full_path())) -endif - if get_option('sane_tool_path') != '' script_environment.prepend('PATH', get_option('sane_tool_path')) endif
We're setting up the PATH environment variable such that a set of necessary build tools can be found at build time. Make this step a bit less repetitive by only looping through the set of found programs once. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- meson.build | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-)