mesonbuild / meson

The Meson Build System
http://mesonbuild.com
Apache License 2.0
5.6k stars 1.63k forks source link

File objects should be able to represent directories #4717

Open Akaricchi opened 5 years ago

Akaricchi commented 5 years ago

Since 0.49.0 my build rules started producing the following warning:

WARNING: Custom target input '['bgm', 'gfx', 'models', 'sfx', 'shader', 'fonts']' can't be converted to File object(s).
This will become a hard error in the future.

The only reason for that is because those elements are in fact directories that are fed into an archive-generating script. Here's the code snippet, slightly simplified:

pack = custom_target('packed data files',
    command : [
        pack_exe,
        '@OUTPUT@',
        meson.current_source_dir(),
        '@DEPFILE@',
        '@INPUT@'
    ],
    input : ['bgm', 'gfx', 'models', 'sfx', 'shader', 'fonts'],
    output : '00-taisei.zip',
    depfile : 'pack.d',
    install : true,
    install_dir : data_path,
)
jpakkane commented 5 years ago

In this particular case you don't need to have input at all. This should work:

pack = custom_target('packed data files',
    command : [
        pack_exe,
        '@OUTPUT@',
        meson.current_source_dir(),
        '@DEPFILE@',
        'bgm', 'gfx', 'models', 'sfx', 'shader', 'fonts'
    ],
    output : '00-taisei.zip',
    depfile : 'pack.d',
    install : true,
    install_dir : data_path,
)

If the inputs were File objects, they would be expanded so they would become something like ../path/to/dir/models. This may not be what you want given that you already pass current_source_dir.

As for the feature itself, a File object should not point to a directory but we could consider adding a Directory object that would behave similar to a file object but can only point to a directory.

Akaricchi commented 5 years ago

This may not be what you want given that you already pass current_source_dir.

This may or may not be what I want. In the future I want to refactor this code, so that instead of passing current_source_dir and a bunch of hardcoded directory names, it would just give the script a directory to make a package from. Conceptually, that would be an input, and I see no reason to arbitrarily decide that files can be inputs but directories can't.

I'm not against solving this by adding a Directory object, in principle. I just think this dichotomy may be complicating things slightly more than is necessary. Wherever you special-case for File objects, you have to keep in mind Directory objects as well now. I personally prefer the pathlib approach, where there is just a general Path object with is_file/is_dir methods.

Also, what happens to include_directories() if either of this gets implemented? Can we just deprecate it in favor of the more general directory/path objects?

jpakkane commented 5 years ago

A directory and include_directory are different things, the latter consists of both the source and corresponding build directory.

bluca commented 5 years ago

@jpakkane one of the issues is that without an input, meson/ninja won't watch the directory content for changes. This is a problem when using Sphinx to build documentation, and there are a gazillion directories to watch but only one top-level command to run.

For example in DPDK we are now getting this warning because of the following:

    html_guides_build = custom_target('html_guides_build',
        input: meson.current_source_dir(),
        output: 'guides',
        command: [sphinx, '-b', 'html',
            '-d', meson.current_build_dir() + '/.doctrees',
            '@INPUT@', meson.current_build_dir() + '/guides'],
        build_by_default: false,
        install: get_option('enable_docs'),
        install_dir: htmldir)

https://github.com/DPDK/dpdk/blob/master/doc/guides/meson.build

Any advice on how to deal with it if file objects can't be changed to represent directories?

dcbaker commented 5 years ago

We really should have a module for sphinx like we do for hotdoc

bluca commented 5 years ago

That would be great! However there are more identical use cases, like doxygen:

https://github.com/DPDK/dpdk/blob/master/doc/api/meson.build

Would it be possible to avoid changing this warning into an error until a solution that can work in these cases is found? Thanks!

Akaricchi commented 5 years ago

meson/ninja won't watch the directory content for changes

I'm not sure either meson or ninja have the ability to watch directories; do they?

bluca commented 5 years ago

It works though, touching a file in one of the directories makes ninja rebuild the docs, otherwise it correctly does nothing if it's called again.

gdmoore commented 4 years ago

To add another use case, I need to lasso files into my build directory in order to build a Docker image from the build.

I need to pass both files and folders to an executable, so naming those sources as a list in my meson.build and passing it as @INPUT@ works perfectly as meson adjusts the paths to be relative. If I don't use input, it's a bit more convoluted to pass the correct source path.

Overall the current behaviour actually does exactly what I need and makes for a very simple meson.build but when this warning becomes an error it will be more complex to work around.

ShanShuiXiLiu commented 2 years ago

但是它可以工作,触摸其中一个目录中的文件会使忍者重建文档,否则如果再次调用它,它不会正确执行任何操作。 yes , it can produce the files. but the produced file cannot be used by the other command .