dunst icon indicating copy to clipboard operation
dunst copied to clipboard

initial meson move

Open apprehensions opened this issue 1 year ago • 67 comments

Currently, this has no support for the following features that already existed within Makefile:

  • ~~Valgrind~~ https://mesonbuild.com/Unit-tests.html#other-test-options
  • ~~Test program~~ meson setup -Dtest=true
  • ~~Documentation (#1225)~~
  • Doxygen

Fixes #1224

apprehensions avatar Nov 02 '23 15:11 apprehensions

I had underestimated how big this project is, which is why i initially thought this would be quite easy, as i had figured dunst is this simple notification daemon program.

apprehensions avatar Nov 02 '23 15:11 apprehensions

What is difficult about the test binary? It just needs to be compiled and run with valgrind. Meson probably has some way to run binaries

fwsmit avatar Nov 04 '23 20:11 fwsmit

Thankfully, Meson comes with coverage reports, so i hope we can drop the coverage reporting all together and only have valgrind tests and run the test program.

https://mesonbuild.com/howtox.html#producing-a-coverage-report

sound good to you?

(this is still somewhat very difficult due to the complexity of Dunst, its website, documentation, testing suite and such... i still really expected dunst to be a simple project)

apprehensions avatar Nov 06 '23 18:11 apprehensions

Yeah, seems good to me. Maybe @bebehei has a stronger opinion about this, since he set up most of our testing infrastructure.

fwsmit avatar Nov 07 '23 11:11 fwsmit

Nice!

Meson was on my "fancy things I might implement" list.

Need to try it with some spare time at the evening.

bebehei avatar Nov 07 '23 12:11 bebehei

If you are a bit more experienced with Meson, I'd suggest you give it a shot rather than me :D

apprehensions avatar Nov 07 '23 12:11 apprehensions

Sorry for giving false hints, but I do not have any experience with meson.

bebehei avatar Nov 07 '23 13:11 bebehei

I'll be dropping coverage tests for the time being.

apprehensions avatar Nov 09 '23 16:11 apprehensions

@fwsmit tests program is failing here as well.

apprehensions avatar Nov 09 '23 17:11 apprehensions

Notable changes:

  • GitHub CI workflow is Ubuntu only, due to dependency constraints, and how it shouldn't realistically fail to run on musl or other distributions, the source is their problem typically.

apprehensions avatar Nov 09 '23 17:11 apprehensions

Can main.c be moved into src?

apprehensions avatar Nov 10 '23 08:11 apprehensions

Can main.c be moved into src?

Probably yes.

bebehei avatar Nov 11 '23 10:11 bebehei

I am rather concerned over the fact that 'main.c' isnt simply just the dunst_main function from dunst.c, seems complicated for no good reason that i know of currently..

apprehensions avatar Nov 11 '23 10:11 apprehensions

I am rather concerned over the fact that 'main.c' isnt simply just the dunst_main function from dunst.c, seems complicated for no good reason that i know of currently..

I was also wondering why that is.(some leftover?)

Probably main.c could be removed altogether by putting main in dunst.c?

bynect avatar Nov 11 '23 11:11 bynect

Probably main.c could be removed altogether by putting main in dunst.c?

Other code in the test suites depend on code from dunst.c. I've tried to move dunst_main to main.c myself but unsure what to do about the setup_done variable.

apprehensions avatar Nov 11 '23 11:11 apprehensions

Probably main.c could be removed altogether by putting main in dunst.c?

Other code in the test suites depend on code from dunst.c. I've tried to move dunst_main to main.c myself but unsure what to do about the setup_done variable.

Well, since the main function is heavily tied to everything in dunst.c probably moving main there would be better.

bynect avatar Nov 11 '23 12:11 bynect

Moving main.c to dunst.c, while dunst.c is required by the test suite causes problems, as the test suite depends on dunst's code too.

apprehensions avatar Nov 11 '23 12:11 apprehensions

Moving main.c to dunst.c, while dunst.c is required by the test suite causes problems, as the test suite depends on dunst's code too.

Some simple meta programming can fix that honestly.

#ifndef TESTING

int main(int argc, char **argv) {
    return dunst_main(argc, argv);
}
#endif

bynect avatar Nov 11 '23 12:11 bynect

  • ~~Need to implement debug buildtype~~

apprehensions avatar Nov 11 '23 13:11 apprehensions

anything missing from the makefile so far?

apprehensions avatar Nov 13 '23 08:11 apprehensions

I was wondering, why remove outright the makefile when they can coexist afaik? Shouldn't the option to use meson be added alongside of makefiles?

Then, after some time (to get feedback from the users), removing the latter can be thought of in another pr.

bynect avatar Nov 13 '23 09:11 bynect

It doesn't make sense to keep both build systems, it will be messy.

apprehensions avatar Nov 13 '23 11:11 apprehensions

Yes, but it is still an additional dependency that is really not that required. I mean, it doesn't seem to add any value to what the makefiles can already do. So people would have to install and learn meson instead of just using make (installed by default everywhere) just for the sake of using meson.

Well, that is my idea. @fwsmit or @bebehei should decide a reasonable compromise.

Note: I am not saying that meson is useless, just that removing the makefiles point blank is probably not the best way

bynect avatar Nov 13 '23 11:11 bynect

There were some projects (like mpv, util-linux) which moved to meson and often thery had some gracious time for switching. First deprecating the old system with one release and with another removing it. But this transition period allowed to fix some eventual bugs/quirks with the new buildsystem and allowed package maintainers a smooth transition with a greater time window. It can be frustrating if you want to just make a minor version bump and suddenly the whole thing fails. And often in those cases you're on a time budget and couldn't follow upstream changes :D And then you need to invest some extra time. True, this is a rather uncomplicated project and switching to meson is often fairly easy.. But still. Just for package maintainers sake I would also favour a transition time.

Narrat avatar Nov 13 '23 18:11 Narrat

Yeah thats.. understandable.

apprehensions avatar Nov 13 '23 18:11 apprehensions

uh oh

apprehensions avatar Nov 13 '23 18:11 apprehensions

unfortunately, i'm not really that great at writing this new change in the CHANGELOG or README, sorry

apprehensions avatar Nov 13 '23 19:11 apprehensions

The changes look good overall. I've left one comment. Does @bebehei have any last comments?

unfortunately, i'm not really that great at writing this new change in the CHANGELOG or README, sorry

Okay, I can take a look at that.

fwsmit avatar Dec 31 '23 17:12 fwsmit

It seems the tests don't run. At least in the github tests it says the following

0s
Run ninja -C build test
ninja: Entering directory `build'
[0/1] Running all tests.
No tests defined.

fwsmit avatar Jan 01 '24 16:01 fwsmit

It seems the tests don't run. At least in the github tests it says the following

I had the test program get built only on a build option, but i've dropped that and now only ninja -C build test can run the program - but the test program will always get built so, hope that's alright.

apprehensions avatar Jan 13 '24 14:01 apprehensions