Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add build target keyword parameter 'build_subdir' [v3] #14002

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

keith-packard
Copy link

@keith-packard keith-packard commented Dec 11, 2024

This is a replacement for #4037 as I want to switch the source branch. This would also replace my need for #13960.

Place the build products in a directory of the specified name somewhere within the build directory. This allows use of the target that includes a specific directory name:

    #include <subdir/configure.h>

This also allows creating targets with the same basename by using different subdirectory names.

v2:
Move build_subdir to Target class.
Error if path separator in build_dir

v3:
Rename to 'build_subdir' to make it clear that the name is
appended to a meson-specific build directory, and does not
provide the user with a way to define the overall meson build
heirarchy.
Allow build_subdir to include path separators.
Support 'build_subdir' for configure_file.

@keith-packard keith-packard force-pushed the keithp-build-dir branch 4 times, most recently from d8c8f04 to d5c8541 Compare December 11, 2024 20:10
@jpakkane
Copy link
Member

This has the unfortunate knock-on effect that it breaks the established layout. Specifically whenever you ask "where did this file in my build dir get defined?" the answer is "from the corresponding source dir's build file". Now with this the answer basically becomes "anywhere at all".

I think there was a different MR with something similar to custom targets or something and the result we ended up with there was that while you could define a separate path it would need to be both A) below the current dir (so no ../ shenanigans) and B) there must not be a directory of that name in the source directory. In this way there can be no name collisions.

@keith-packard
Copy link
Author

This has the unfortunate knock-on effect that it breaks the established layout. Specifically whenever you ask "where did this file in my build dir get defined?" the answer is "from the corresponding source dir's build file". Now with this the answer basically becomes "anywhere at all".

Not quite -- the subdir in question is always created beneath the directory corresponding to the original source dir. That ensures the same general relation between source dir and build dir. That's why I called it 'build_subdir' -- it's a subdirectory of the build directory, not some arbitrary directory within the build heirarchy.

I think there was a different MR with something similar to custom targets or something and the result we ended up with there was that while you could define a separate path it would need to be both A) below the current dir (so no ../ shenanigans) and B) there must not be a directory of that name in the source directory. In this way there can be no name collisions.

Those both sound like good restrictions; I'll add validation for them.

@keith-packard keith-packard force-pushed the keithp-build-dir branch 3 times, most recently from 71ea71c to ae3175f Compare December 11, 2024 23:04
Place the build products in a directory of the specified name
somewhere within the build directory. This allows use of the target
that includes a specific directory name:

        #include <subdir/configure.h>

This also allows creating targets with the same basename by using
different subdirectory names.

v2:
    Move build_subdir to Target class.
    Error if path separator in build_dir

v3:
    Rename to 'build_subdir' to make it clear that the name is
    appended to a meson-specific build directory, and does not
    provide the user with a way to define the overall meson build
    heirarchy.

    Allow build_subdir to include path separators.

    Support 'build_subdir' for configure_file.

    build_subdir must not exist in the source directory and
    must not contain '..'

    Add documentation and tests

Signed-off-by: Keith Packard <keithp@keithp.com>
@keith-packard
Copy link
Author

keith-packard commented Dec 14, 2024

I've evaluated the difference for my project between using this patch and #13960.

Here's using #13960:

picolibc/picolibc@be3b851

$ git diff main..rename | diffstat
 dummyhost/meson.build |    5 ++++-
 newlib/meson.build    |   24 ++++++++++++++++++++----
 picocrt/meson.build   |   55 +++++++++++++++++++++++++++++++++++++++++++++----------
 semihost/meson.build  |    5 ++++-
 4 files changed, 73 insertions(+), 16 deletions(-)

And here's using build_subdir:

picolibc/picolibc@a6c8334

$ git diff main..build-subdir | diffstat
 dummyhost/meson.build |    8 +++-----
 newlib/meson.build    |   21 ++++++++++-----------
 picocrt/meson.build   |   43 ++++++++++++++++++++-----------------------
 semihost/meson.build  |    8 +++-----
 4 files changed, 36 insertions(+), 44 deletions(-)

Diffstat shows that this version results in shorter code for this case. Let's look at the changes to meson itself. Here's the diffstat for rename:

$ git diff master..target-rename | diffstat
 docs/yaml/functions/_build_target_base.yaml             |   10 ++++++++++
 docs/yaml/functions/configure_file.yaml                 |    9 +++++++++
 docs/yaml/functions/custom_target.yaml                  |   10 ++++++++++
 docs/yaml/functions/executable.yaml                     |   16 ++++++++++++++++
 docs/yaml/functions/shared_library.yaml                 |   16 ++++++++++++++++
 docs/yaml/functions/shared_module.yaml                  |   16 ++++++++++++++++
 mesonbuild/backend/backends.py                          |   24 ++++++++++++++----------
 mesonbuild/build.py                                     |   68 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 mesonbuild/interpreter/interpreter.py                   |   21 +++++++++++++++++++--
 mesonbuild/interpreter/kwargs.py                        |    1 +
 mesonbuild/interpreter/type_checking.py                 |   16 +++++++++++++++-
 mesonbuild/minstall.py                                  |   23 ++++++++++++++---------
 mesonbuild/mintro.py                                    |    4 ++--
 test cases/common/109 custom target capture/meson.build |   12 ++++++++++++
 test cases/common/109 custom target capture/test.json   |    3 ++-
 test cases/common/117 shared module/meson.build         |   23 +++++++++++++++++++++++
 test cases/common/117 shared module/test.json           |    5 ++++-
 test cases/common/14 configure file/meson.build         |    7 +++++++
 test cases/common/14 configure file/test.json           |    3 ++-
 19 files changed, 257 insertions(+), 30 deletions(-)

and here's build_subdir:

$ git diff master..keithp-build-dir | diffstat
 docs/yaml/functions/_build_target_base.yaml             |   17 +++++++++++++++++
 docs/yaml/functions/configure_file.yaml                 |   17 +++++++++++++++++
 mesonbuild/backend/backends.py                          |   13 ++++++++-----
 mesonbuild/build.py                                     |   24 +++++++++++++++++++++---
 mesonbuild/interpreter/interpreter.py                   |   31 +++++++++++++++++++++++++++----
 mesonbuild/interpreter/type_checking.py                 |    1 +
 test cases/common/109 custom target capture/meson.build |   10 ++++++++++
 test cases/common/109 custom target capture/test.json   |    3 ++-
 test cases/common/117 shared module/meson.build         |    8 ++++++++
 test cases/common/117 shared module/test.json           |    5 ++++-
 test cases/common/14 configure file/meson.build         |    7 +++++++
 test cases/common/14 configure file/test.json           |    3 ++-
 12 files changed, 124 insertions(+), 15 deletions(-)

The build_subdir version is also less fragile for users -- the rename version requires that the meson.build file include complete filenames for generated files, including the 'lib' prefix and '.a' suffix for libraries. The build-subdir version inherits all of that from the existing code. Similarly, import libraries and debuginfo files would be automatically generated and named correctly with the build-subdir version, while the rename version requires that the user provide correct names for each of those.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants