search

mesonbuild / wrapdb

New wrap requests
https://mesonbuild.com/Adding-new-projects-to-wrapdb.html
MIT License
69 stars 174 forks source link

[uchardet] Add ability to disable building tests #1559

Open kasper93 opened 1 week ago

kasper93 commented 1 week ago

As title says, would be nice to disable building tests, if not required. 

option('tests', type: 'feature', value: 'enabled', yield: true, description: 'Enable or disable tests')

or

option('tests', type: 'boolean', value: false, description: 'Enable or disable tests')

Thanks

eli-schwartz commented 1 week ago

The entire testsuite is a single C file with zero dependencies. What would an option even do?

kasper93 commented 1 week ago

The entire testsuite is a single C file with zero dependencies. What would an option even do?

This option would skip building this single C file with zero dependencies.

Some people don't want to remember complex commands that exclude subprojects tests and may want to do meson tests without them.

eli-schwartz commented 1 week ago

So instead they need to remember complex commands that exclude subprojects test options and may want to do meson setup without them.

What's the difference?

kasper93 commented 1 week ago

It is common practice to disable tests with -Dtests=false or -Dtests=disabled. I see no reason why uchardet should ignore that, just because its test file is "simple". I don't understand why we are even arguing about it. Feel free to ignore if you really think following standard convention of disabling tests is not a good idea.

Also if you run meson test current uchardet adds over 100 of them and they all fail, because they cannot load test file if they are run as a subproject.

EDIT:

As for the usecase, I disable unneeded things from build in OSS-Fuzz. I can disable tests from mpv, libplacebo, harfbuzz, fontconfig, fribidi, they all support this argumentand only uchardet left.

eli-schwartz commented 1 week ago

I don't understand why we are even arguing about it. Feel free to ignore if you really think following standard convention of disabling tests is not a good idea.

It's a standard convention to have a tests option for disabling the lookup of test dependencies, as it prevents setting up a build at all, if you don't need to run tests and don't have the test dependencies.

It's also often used as a workaround for the fact that meson currently builds tests as part of ninja all, which can be wasteful if you don't plan to run meson test and the tests take a long time to compile. It's on my list of things to fix, that they should only be built as part of meson test...

My personal opinion is that the cost/benefit ratio here isn't favorable.

Also if you run meson test current uchardet adds over 100 of them and they all fail, because they cannot load test file if they are run as a subproject.

That sounds quite bad since tests are supposed to pass when testing the wrap itself as part of the WrapDB CI.