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

[CMake] Make switches for server-only options dependent on XRCL_ONLY=FALSE #1805

Merged
merged 1 commit into from Nov 7, 2022

Conversation

amadio
Copy link
Member

@amadio amadio commented Oct 20, 2022

Fixes: ff67c2a, Closes: #1804

@amadio amadio self-assigned this Oct 20, 2022
@amadio
Copy link
Member Author

amadio commented Oct 20, 2022

@simonmichal I guess at least ENABLE_VOMS could be turned into a dependent option as well. Which other options are client-only?

@simonmichal
Copy link
Contributor

@amadio : thanks a lot for the PR :-)

I think, it is worth investigating first why did we run into the problem described in #1804 in the first place, if I read the code correctly, if the XRDCL_ONLY is set to true Macaroons and SciTokens plug-ins should have been excluded from the build:

if( NOT XRDCL_ONLY )
include( XrdServer )
include( XrdDaemons )
include( XrdFrm )
include( XrdFfs )
include( XrdPlugins )
include( XrdSsi )
include( XrdPfc )
if( CMAKE_COMPILER_IS_GNUCXX )
include( XrdOssCsi )
endif()
if( BUILD_HTTP )
include( XrdHttp )
include( XrdTpc )
endif()
if( BUILD_MACAROONS )
include( XrdMacaroons )
endif()
if( BUILD_VOMS )
include( XrdVoms )
endif()
if( XRDCEPH_SUBMODULE )
add_subdirectory( XrdCeph )
endif()
if( BUILD_SCITOKENS )
include( XrdSciTokens )
include( XrdSecztn )
endif()
endif()

or do I miss something?

@amadio
Copy link
Member Author

amadio commented Oct 20, 2022

The lookup for SciTokens and Macaroons happens in cmake/XRootDFindLibs.cmake and that's not protected with if(NOT XRDCL_ONLY). That's why I said maybe moving the calls to find_package(...) into the subdirectories where they are needed could make sense. It's an alternative solution to what I'm proposing here, but may not be as nice if multiple subdirectories depend on the same packages, as those would be needed at top level anyway.

@simonmichal
Copy link
Contributor

@amadio : thanks for the explanation, it all makes sense now!

smuzaffar added a commit to cms-sw/cmsdist that referenced this pull request Oct 20, 2022
@simonmichal simonmichal merged commit c267103 into xrootd:stable-5.5.x Nov 7, 2022
@amadio amadio deleted the issue-1804 branch November 7, 2022 13:03
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

2 participants