Skip to content

chore: add ADA_TOOLS option #413

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

Merged
merged 1 commit into from
May 18, 2023
Merged

Conversation

myd7349
Copy link
Contributor

@myd7349 myd7349 commented May 18, 2023

This patch adds a new ADA_TOOLS option to allow users enable/disable the build of ada cli tools (adaparse) manually.

When trying to add a new port for ada-url in vcpkg, I found it failed to build adaparse on UWP: microsoft/vcpkg#31485 .

adaparse uses fmt::output_file defined in fmt/os.h. However, since UWP doesn't provide _pipe, FMT_USE_FCNTL is defined as 0, which in turn means fmt::output_file does not exist.

https://github.com/fmtlib/fmt/blob/master/include/fmt/os.h#L23

@anonrig anonrig requested a review from lemire May 18, 2023 14:26
@anonrig
Copy link
Member

anonrig commented May 18, 2023

We'll release v2.4.2 when this PR is merged. Thanks for your contribution @myd7349

@myd7349
Copy link
Contributor Author

myd7349 commented May 18, 2023

How about?

if(NOT ADA_COVERAGE AND NOT EMSCRIPTEN)
  add_subdirectory(singleheader)
endif()

if(ADA_TOOLS)
  add_subdirectory(tools)
endif()

@lemire
Copy link
Member

lemire commented May 18, 2023

@myd7349 Yes. That looks good.

@myd7349 myd7349 force-pushed the ada-tools-option branch from def0f08 to 5696742 Compare May 18, 2023 15:09
@anonrig anonrig merged commit 81ffcb7 into ada-url:main May 18, 2023
@myd7349 myd7349 deleted the ada-tools-option branch May 18, 2023 15:41
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.

3 participants