You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "cmcfarlen (via GitHub)" <gi...@apache.org> on 2024/02/07 18:58:47 UTC
[PR] Add auto_option for ImageMagick and add support for specifying a COMP… [trafficserver]
cmcfarlen opened a new pull request, #11050:
URL: https://github.com/apache/trafficserver/pull/11050
…ONENT to auto_option
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add auto_option for ImageMagick and add support for specifying a COMP… [trafficserver]
Posted by "cmcfarlen (via GitHub)" <gi...@apache.org>.
cmcfarlen merged PR #11050:
URL: https://github.com/apache/trafficserver/pull/11050
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add auto_option for ImageMagick and add support for specifying a COMP… [trafficserver]
Posted by "cmcfarlen (via GitHub)" <gi...@apache.org>.
cmcfarlen commented on code in PR #11050:
URL: https://github.com/apache/trafficserver/pull/11050#discussion_r1482173596
##########
cmake/AutoOptionHelpers.cmake:
##########
@@ -37,18 +37,26 @@ function(_REGISTER_AUTO_OPTION _NAME _FEATURE_VAR _DESCRIPTION _DEFAULT)
)
endfunction()
-macro(_CHECK_PACKAGE_DEPENDS _OPTION_VAR _PACKAGE_DEPENDS _FEATURE_VAR)
+macro(_CHECK_PACKAGE_DEPENDS _OPTION_VAR _PACKAGE_DEPENDS _FEATURE_VAR _COMPONENT)
Review Comment:
agreed, but I couldn't get it to work providing two lists
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add auto_option for ImageMagick and add support for specifying a COMP… [trafficserver]
Posted by "cmcfarlen (via GitHub)" <gi...@apache.org>.
cmcfarlen commented on code in PR #11050:
URL: https://github.com/apache/trafficserver/pull/11050#discussion_r1482173318
##########
cmake/AutoOptionHelpers.cmake:
##########
@@ -37,18 +37,26 @@ function(_REGISTER_AUTO_OPTION _NAME _FEATURE_VAR _DESCRIPTION _DEFAULT)
)
endfunction()
-macro(_CHECK_PACKAGE_DEPENDS _OPTION_VAR _PACKAGE_DEPENDS _FEATURE_VAR)
+macro(_CHECK_PACKAGE_DEPENDS _OPTION_VAR _PACKAGE_DEPENDS _FEATURE_VAR _COMPONENT)
if(${${_OPTION_VAR}} STREQUAL AUTO)
set(STRICTNESS)
else()
set(STRICTNESS REQUIRED)
endif()
foreach(PACKAGE_NAME ${_PACKAGE_DEPENDS})
- find_package(${PACKAGE_NAME} ${STRICTNESS})
- if(NOT ${PACKAGE_NAME}_FOUND)
- set(${_FEATURE_VAR} FALSE)
- break()
+ if(NOT ${_COMPONENT} STREQUAL OFF)
Review Comment:
I'm pretty sure I tried this and couldn't get it to work.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add auto_option for ImageMagick and add support for specifying a COMP… [trafficserver]
Posted by "JosiahWI (via GitHub)" <gi...@apache.org>.
JosiahWI commented on code in PR #11050:
URL: https://github.com/apache/trafficserver/pull/11050#discussion_r1482155329
##########
cmake/AutoOptionHelpers.cmake:
##########
@@ -37,18 +37,26 @@ function(_REGISTER_AUTO_OPTION _NAME _FEATURE_VAR _DESCRIPTION _DEFAULT)
)
endfunction()
-macro(_CHECK_PACKAGE_DEPENDS _OPTION_VAR _PACKAGE_DEPENDS _FEATURE_VAR)
+macro(_CHECK_PACKAGE_DEPENDS _OPTION_VAR _PACKAGE_DEPENDS _FEATURE_VAR _COMPONENT)
Review Comment:
This would be more flexible as a list of components. Maybe we should think about that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add auto_option for ImageMagick and add support for specifying a COMP… [trafficserver]
Posted by "cmcfarlen (via GitHub)" <gi...@apache.org>.
cmcfarlen commented on PR #11050:
URL: https://github.com/apache/trafficserver/pull/11050#issuecomment-1933073350
[approve ci]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add auto_option for ImageMagick and add support for specifying a COMP… [trafficserver]
Posted by "cmcfarlen (via GitHub)" <gi...@apache.org>.
cmcfarlen commented on code in PR #11050:
URL: https://github.com/apache/trafficserver/pull/11050#discussion_r1482174089
##########
cmake/AutoOptionHelpers.cmake:
##########
@@ -128,11 +140,17 @@ macro(auto_option _FEATURE_NAME)
message(FATAL_ERROR "Invalid auto_option default ${ARG_DEFAULT}")
endif()
+ if(ARG_COMPONENT)
+ set(COMPONENT ${ARG_COMPONENT})
+ else()
+ set(COMPONENT OFF)
Review Comment:
If I don't set it to off, the condition picking the COMPONENTS path would not work
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add auto_option for ImageMagick and add support for specifying a COMP… [trafficserver]
Posted by "JosiahWI (via GitHub)" <gi...@apache.org>.
JosiahWI commented on code in PR #11050:
URL: https://github.com/apache/trafficserver/pull/11050#discussion_r1482153923
##########
cmake/AutoOptionHelpers.cmake:
##########
@@ -37,18 +37,26 @@ function(_REGISTER_AUTO_OPTION _NAME _FEATURE_VAR _DESCRIPTION _DEFAULT)
)
endfunction()
-macro(_CHECK_PACKAGE_DEPENDS _OPTION_VAR _PACKAGE_DEPENDS _FEATURE_VAR)
+macro(_CHECK_PACKAGE_DEPENDS _OPTION_VAR _PACKAGE_DEPENDS _FEATURE_VAR _COMPONENT)
if(${${_OPTION_VAR}} STREQUAL AUTO)
set(STRICTNESS)
else()
set(STRICTNESS REQUIRED)
endif()
foreach(PACKAGE_NAME ${_PACKAGE_DEPENDS})
- find_package(${PACKAGE_NAME} ${STRICTNESS})
- if(NOT ${PACKAGE_NAME}_FOUND)
- set(${_FEATURE_VAR} FALSE)
- break()
+ if(NOT ${_COMPONENT} STREQUAL OFF)
Review Comment:
What about:
```suggestion
if(${_COMPONENT})
```
`"OFF"`, `"0"`, `"FALSE"` and `""` should all have falsey values, I think.
##########
cmake/AutoOptionHelpers.cmake:
##########
@@ -37,18 +37,26 @@ function(_REGISTER_AUTO_OPTION _NAME _FEATURE_VAR _DESCRIPTION _DEFAULT)
)
endfunction()
-macro(_CHECK_PACKAGE_DEPENDS _OPTION_VAR _PACKAGE_DEPENDS _FEATURE_VAR)
+macro(_CHECK_PACKAGE_DEPENDS _OPTION_VAR _PACKAGE_DEPENDS _FEATURE_VAR _COMPONENT)
Review Comment:
This would be more flexible as a list of components.
##########
cmake/AutoOptionHelpers.cmake:
##########
@@ -128,11 +140,17 @@ macro(auto_option _FEATURE_NAME)
message(FATAL_ERROR "Invalid auto_option default ${ARG_DEFAULT}")
endif()
+ if(ARG_COMPONENT)
+ set(COMPONENT ${ARG_COMPONENT})
+ else()
+ set(COMPONENT OFF)
Review Comment:
I'm not sure about setting it to `OFF` here; it doesn't represent an option.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org