Skip to content

Conversation

@ktf
Copy link
Member

@ktf ktf commented Nov 7, 2025

Avoid picking up incompatible protobuf from the system. Needs a new release of
monitoring to be effective.

@ktf ktf requested a review from a team as a code owner November 7, 2025 08:19
sy-c
sy-c previously approved these changes Nov 7, 2025
@sy-c
Copy link
Contributor

sy-c commented Nov 7, 2025

New monitoring version v3.19.9 tagged and included here,
provides the needed CMake options AliceO2Group/Monitoring#360

${GRPC_REVISION:+-DGRPC_ROOT="${GRPC_ROOT}"} \
-DCMAKE_INSTALL_PREFIX=$INSTALLROOT \
${BOOST_REVISION:+-DBOOST_ROOT=$BOOST_ROOT} \
${LIBGRPC_REVISION:--DO2_MONITORING_CONTROL_ENABLE=0} \
Copy link
Member Author

@ktf ktf Nov 7, 2025

Choose a reason for hiding this comment

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

@singiamtel Can we disable SC2086 or make sure that it does not complain for certain kind of variables? ${LIBGRPC_REVISION:--DO2_MONITORING_CONTROL_ENABLE=0} will never expand to something with a space in it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

It can only be disabled globally in the alidistlint config, or case by case with # shellcheck disable=SC2086 . What do you prefer?

If the problem is that it's annoying to quote them when unneeded, maybe the autofix thing helps.

Copy link
Member Author

Choose a reason for hiding this comment

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

No, the issue is that we should not add quotes when not needed. I would vote for disabling SC2086 until it's possible to do whitelist "safe" variables.

set MONITORING_ROOT \$::env(BASEDIR)/$PKGNAME/\$version
prepend-path ROOT_INCLUDE_PATH \$PKG_ROOT/include
EoF
alibuild-generate-module --bin --lib --root > etc/modulefiles/$PKGNAME
Copy link
Member Author

Choose a reason for hiding this comment

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

Same here $PKGNAME is guaranteed not to expand.

Barthelemy
Barthelemy previously approved these changes Nov 10, 2025
ktf added 2 commits November 10, 2025 11:52
Avoid picking up incompatible protobuf from the system. Needs a new release of
monitoring to be effective.
@ktf ktf dismissed stale reviews from Barthelemy and sy-c via a805b29 November 10, 2025 10:54
@ktf ktf requested a review from a team as a code owner November 10, 2025 10:54
@ktf ktf dismissed stale reviews from Barthelemy and sy-c via a805b29 November 10, 2025 10:54
singiamtel added a commit to alisw/alidistlint that referenced this pull request Nov 11, 2025
@ktf ktf merged commit 30f91fa into alisw:master Nov 15, 2025
19 of 20 checks passed
vascobarroso pushed a commit that referenced this pull request Nov 17, 2025
* Move to latest alibuild-recipe-tools
* Cleanup monitoring recipe
Avoid picking up incompatible protobuf from the system. Needs a new release of
monitoring to be effective.
ktf pushed a commit to alisw/alidistlint that referenced this pull request Nov 26, 2025
mrtineide pushed a commit to mrtineide/alidist that referenced this pull request Nov 26, 2025
* Move to latest alibuild-recipe-tools
* Cleanup monitoring recipe
Avoid picking up incompatible protobuf from the system. Needs a new release of
monitoring to be effective.
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.

4 participants