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.

Sign up for GitHub

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

Jump to bottom

fmt: add header-only option to make it truly header-only #1811

Merged
merged 1 commit into from
Jan 4, 2025
Merged

fmt: add header-only option to make it truly header-only #1811

merged 1 commit into from
Jan 4, 2025

Conversation

trim21
Copy link
Contributor

@trim21 trim21 commented Dec 6, 2024 •
edited
Loading

close #1803

fmt has a header only mode which allow to be used without need to compile cc files.

@trim21 trim21 changed the title fmt: disable install when it's subproject used as static library fmt: disable install when it's subproject or used as static library Dec 6, 2024
@trim21 trim21 marked this pull request as ready for review December 6, 2024 19:53
@trim21 trim21 marked this pull request as draft December 6, 2024 20:04
@trim21 trim21 marked this pull request as ready for review December 6, 2024 20:18
@trim21 trim21 changed the title fmt: disable install when it's subproject or used as static library fmt: disable install when it's subproject or static library Dec 6, 2024
@trim21 trim21 closed this Dec 6, 2024
@trim21 trim21 reopened this Jan 4, 2025
@trim21 trim21 changed the title fmt: disable install when it's subproject or static library fmt: do not build libfmt unless it's need Jan 4, 2025
@trim21 trim21 marked this pull request as draft January 4, 2025 07:14
@trim21 trim21 marked this pull request as ready for review January 4, 2025 07:20
@trim21 trim21 force-pushed the fmt-skip-install branch 3 times, most recently from 857af33 to c4ee85f Compare January 4, 2025 07:24
@trim21 trim21 changed the title fmt: do not build libfmt unless it's need fmt: do not build libfmt unless it's needed Jan 4, 2025
jpakkane
jpakkane reviewed Jan 4, 2025
View reviewed changes
subprojects/packagefiles/fmt/meson.build Outdated
install: true,
build_by_default: false,
# so we do not build libfmt when using fmt_header_only
install: not meson.is_subproject(),
Copy link
Member

Choose a reason for hiding this comment

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

This breaks things for people who use this wrap and do want to build and use fmt as a shared library.

The common solution in these cases is to have a project option to select whether the dep should be built as a lib or is used as a header only lib.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks.

@trim21 trim21 force-pushed the fmt-skip-install branch 3 times, most recently from 25f1e3e to 7959593 Compare January 4, 2025 18:09
@trim21 trim21 changed the title fmt: do not build libfmt unless it's needed fmt: add enable-header-only option to make it truly header-only Jan 4, 2025
@trim21 trim21 requested a review from jpakkane January 4, 2025 18:13
jpakkane
jpakkane reviewed Jan 4, 2025
View reviewed changes
subprojects/packagefiles/fmt/meson_options.txt Outdated
@@ -0,0 +1,6 @@
option(
'enable-header-only',
Copy link
Member

Choose a reason for hiding this comment

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

We intentionally do not use 'enable-prefix in option names as it is an autoconfism.header-only` says the same thing more concisely.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@trim21
Copy link
Contributor Author

trim21 commented Jan 4, 2025

a naive question, is this the correct way to use this new option? fmt = subproject('fmt', default_options: ['-Dheader-only=true'])

@trim21 trim21 changed the title fmt: add enable-header-only option to make it truly header-only fmt: add header-only option to make it truly header-only Jan 4, 2025
@jpakkane
Copy link
Member

jpakkane commented Jan 4, 2025

Yes.

@trim21
Copy link
Contributor Author

trim21 commented Jan 4, 2025

Thanks, does this mean there is now way force this option and make it not overridable from cli?

@trim21 trim21 mentioned this pull request Jan 4, 2025
Merged
@jpakkane
Copy link
Member

jpakkane commented Jan 4, 2025

What is specified on the CLI always overrides everything written in build files. This is mandatory so people can configure their deps to their liking without having to edit their subprojects' source code.

@jpakkane jpakkane merged commit 56ccc01 into mesonbuild:master Jan 4, 2025
12 checks passed
@trim21 trim21 deleted the fmt-skip-install branch January 4, 2025 18:26
@trim21
Copy link
Contributor Author

trim21 commented Jan 4, 2025 •
edited
Loading

Ah, I know. Unless I have 'fmt:header-only=...' in project's defaul_options, no one can override it from cli.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jpakkane jpakkane jpakkane left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ninja still try to compile fmt.lib when only fmt_header_only_dep is used
2 participants
@trim21 @jpakkane