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

Meson fixes and cleanups #256

Closed
wants to merge 10 commits into from
Closed

Conversation

dcbaker
Copy link
Contributor

@dcbaker dcbaker commented Jul 29, 2022

Initially I started on this because I'm using this in a subproject that set's it's C version to c99, and libpkgconf is full of warnings because of undefined functions. I realized that meson isn't setting a c_std unlike autotools. So i fixed that, added some warnings, updated the CI to be more picky, and then cleaned up the meson a bit (I am a meson dev, I might as well do something useful with all that knowledge :)

To avoid warnings about string functions like strdup which are otherwise
undefined, but succeed at linking anyway when the C standard is c99.
This would be triggered without the previous addition of -D_BSD_SOURCE
for strdup, among others
autoconf uses either C99 or Gnu99. Meson does not provide a graceful way
to select gnu99 if possible or c99 (though there are several proposals
currently happening to get there), so I've selected c99 as the
conservative default. Without this, the compiler uses whatever it's
default happens to be, which may or may not work out correctly, and
hides bugs from CI that are present with c99 as the default.
@kaniini
Copy link
Member

kaniini commented Aug 1, 2022

Looks fine, I will merge it into my tree today. (If I click the button here, it will mess up when my personal git instance mirrors the repo back to Github)

@dcbaker
Copy link
Contributor Author

dcbaker commented Aug 1, 2022

I updated this PR to add -Werror to the woodpecker CI as well, but otherwise this latest push is exactly the same as the last (since you mentioned it on the other PR)

This will help catch any new warnings added in CI.
It's more terse, and we don't need the support of a fallback value.
Instead of writing `['HAVE_FOO_H', 'foo.h']`, use meson's string methods
to just write `['foo.h']`, and let meson create `HAVE_FOO_H` for us.
The latter doesn't work correctly when being used as a subproject, as it
returns the *absolute* root. So if pkgconf is being built as part of
muon, then it will return muon's source root. current_source_dir, on the
other hand returns the directory correctly whether being built as a
subproject or superproject.
Instead of attempting to figure out what the paths will be, take the
returned file object and pass that around, meson will then automatically
figure out the correct paths.
Because we really should be checking that it succeeds, and because not
setting it is deprecated.


configure_file(input : 'Kyuafile.in', output : 'Kyuafile', configuration : cdata)
kyuafile = configure_file(input : 'Kyuafile.in', output : 'Kyuafile', configuration : cdata)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May want to match the : style with the rest of the files. Seem to be attached to the key elsewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The style is all over the place here, and I'm inclined to leave it alone since all I did was assigned the output.

@dcbaker
Copy link
Contributor Author

dcbaker commented Aug 2, 2022

Migrated to gitea

@dcbaker dcbaker closed this Aug 2, 2022
@dcbaker dcbaker deleted the submit/meson-cleanups branch August 2, 2022 17:22
@Tachi107
Copy link
Contributor

Tachi107 commented Aug 20, 2022

Migrated to gitea

This Gitea migration is kinda confusing to be honest. The README says to report bugs on GitHub, while the mailing list in on sourcehut, where there's also another pkgconf repo.

I guess that the bug reporting link in the README simply has to be updated to point to Treehouse's Gitea instance (what's Treehouse btw? It's hard to determine by searching the web), but is the mailing list still monitored? What should it be used for?

@dcbaker
Copy link
Contributor Author

dcbaker commented Aug 20, 2022

https://gitea.treehouse.systems/ariadne/pkgconf is the url. We really should update the docs.

@Tachi107
Copy link
Contributor

https://gitea.treehouse.systems/ariadne/pkgconf is the url. We really should update the docs.

Yep, already forked that. My main question is about the sourcehut mailing list really (and also who/what Treehouse is)

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.

None yet

4 participants