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

Add auto options for all experimental plugins #10967

Merged
merged 2 commits into from
May 31, 2024

Conversation

JosiahWI
Copy link
Collaborator

@JosiahWI JosiahWI commented Jan 5, 2024

Auto options were added for the experimental plugins with special dependencies in #10741. This adds options for the remaining experimental plugins.

@JosiahWI JosiahWI self-assigned this Jan 5, 2024
@JosiahWI JosiahWI added this to the 10.0.0 milestone Jan 5, 2024
@bneradt
Copy link
Contributor

bneradt commented Jan 5, 2024

In case it's helpful:
https://ci.trafficserver.apache.org/job/Github_Builds/job/ubuntu/3647/console

INFO: YAML_CPP version 0.8.0
CMake Error at cmake/AutoOptionHelpers.cmake:142 (add_subdirectory):
  add_subdirectory given source "plugins/experimental/hook_trace" which is
  not an existing directory.
Call Stack (most recent call first):
  cmake/ExperimentalPlugins.cmake:111 (auto_option)
  CMakeLists.txt:604 (include)

Thanks for working on this.

@JosiahWI
Copy link
Collaborator Author

JosiahWI commented Jan 5, 2024

Blocking on #10968 for the OSX build.

@bryancall
Copy link
Contributor

[approve ci osx]

@JosiahWI
Copy link
Collaborator Author

JosiahWI commented Jan 9, 2024

The OSX failure is due to a CMake issue present in master. I discussed it with @cmcfarlen and am working on a fix. Marking this as a draft until then.

@bryancall
Copy link
Contributor

[approve ci osx]

Copy link

This pull request has been automatically marked as stale because it has not had recent activity. Marking it stale to flag it for further consideration by the community.

@github-actions github-actions bot added the Stale label Apr 22, 2024
@JosiahWI JosiahWI force-pushed the feat/plugin-auto-options branch 3 times, most recently from 7d1e308 to 311858f Compare April 23, 2024 22:06
@JosiahWI JosiahWI marked this pull request as ready for review April 23, 2024 22:08
@JosiahWI JosiahWI removed the Stale label Apr 23, 2024
@JosiahWI
Copy link
Collaborator Author

This is about ready, but I still have to solve the problem of ImageMagick being used both in both the webp_transform and magick plugins.

@bneradt
Copy link
Contributor

bneradt commented Apr 24, 2024

[approve ci]

@JosiahWI
Copy link
Collaborator Author

It was discussed that it is clearer to have auto_options just set the USE_XXX variables, and put the logic to add the subdirectories in experimental/CMakeLists.txt. I will update the PR with this change at some point.

Auto options were added for the experimental plugins with special
dependencies in apache#10741. This adds options for the remaining experimental
plugins.

It was pointed out that WITH_SUBDIRECTORY is giving auto_options
too many responsibilities and it will be removed.
@JosiahWI
Copy link
Collaborator Author

A CMake issue that caused the necessity for a workaround for ImageMagick is being tracked here: https://gitlab.kitware.com/cmake/cmake/-/issues/25970.

@bryancall bryancall removed this from the 10.0.0 milestone May 20, 2024
@bryancall bryancall added this to the 10.1.0 milestone May 20, 2024
@JosiahWI JosiahWI merged commit 1036e47 into apache:master May 31, 2024
15 checks passed
@JosiahWI JosiahWI deleted the feat/plugin-auto-options branch May 31, 2024 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: For v10.0.0
Development

Successfully merging this pull request may close these issues.

None yet

4 participants