Skip to content

Cleanup monitoring recipe #6034

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.

Sign up for GitHub

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
ktf merged 2 commits into from
Nov 15, 2025
Merged

Cleanup monitoring recipe #6034

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
76a1e31
Move to latest alibuild-recipe-tools
ktf Nov 10, 2025
a805b29
Cleanup monitoring recipe
ktf Nov 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions alibuild-recipe-tools.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package: alibuild-recipe-tools
version: "0.2.5"
tag: "v0.2.5"
version: "0.2.6"
tag: "v0.2.6"
source: https://github.com/alisw/alibuild-recipe-tools
---
mkdir -p $INSTALLROOT/bin
Expand All @@ -21,6 +21,5 @@ module-whatis "ALICE Modulefile for $PKGNAME $PKGVERSION-@@PKGREVISION@$PKGHASH@
module load BASE/1.0
# Our environment
set ALIBUILD_RECIPE_TOOLS_ROOT \$::env(BASEDIR)/$PKGNAME/\$version
prepend-path LD_LIBRARY_PATH \$ALIBUILD_RECIPE_TOOLS_ROOT/lib
prepend-path PATH \$ALIBUILD_RECIPE_TOOLS_ROOT/bin
EoF
22 changes: 10 additions & 12 deletions monitoring.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,44 +9,42 @@
build_requires:
- CMake
- alibuild-recipe-tools
- protobuf
- ninja
source: https://github.com/AliceO2Group/Monitoring
incremental_recipe: |
make ${JOBS:+-j$JOBS} install
cmake --build . -- ${JOBS:+-j$JOBS} install
mkdir -p $INSTALLROOT/etc/modulefiles && rsync -a --delete etc/modulefiles/ $INSTALLROOT/etc/modulefiles
---
#!/bin/bash -ex

case $ARCHITECTURE in
osx*) [[ ! $BOOST_ROOT ]] && BOOST_ROOT=$(brew --prefix boost);;
osx*) [[ ! $BOOST_ROOT ]] && BOOST_ROOT=$(brew --prefix boost) ;;

Check notice on line 20 in monitoring.sh

View workflow job for this annotation

GitHub Actions / alidistlint 

Prefer explicit -n to check non-empty string (or use =/-ne to check boolean/integer). [SC2244]

Check notice on line 20 in monitoring.sh

View workflow job for this annotation

GitHub Actions / alidistlint 

Prefer explicit -n to check non-empty string (or use =/-ne to check boolean/integer). [SC2244]

Check notice on line 20 in monitoring.sh

View workflow job for this annotation

GitHub Actions / alidistlint 

Prefer explicit -n to check non-empty string (or use =/-ne to check boolean/integer). [SC2244]
esac


if [[ $ALIBUILD_O2_TESTS ]]; then
CXXFLAGS="${CXXFLAGS} -Werror -Wno-error=deprecated-declarations"
fi

cmake $SOURCEDIR \
-G Ninja \
${LIBRDKAFKA_REVISION:+-DRDKAFKA_ROOT="${LIBRDKAFKA_ROOT}"} \
${GRPC_REVISION:+-DGRPC_ROOT="${GRPC_ROOT}"} \
-DCMAKE_INSTALL_PREFIX=$INSTALLROOT \
${BOOST_REVISION:+-DBOOST_ROOT=$BOOST_ROOT} \
${LIBGRPC_REVISION:--DO2_MONITORING_CONTROL_ENABLE=0} \

Check notice on line 34 in monitoring.sh

View workflow job for this annotation

GitHub Actions / alidistlint 

Double quote to prevent globbing and word splitting. [SC2086]

Check notice on line 34 in monitoring.sh

View workflow job for this annotation

GitHub Actions / alidistlint 

Double quote to prevent globbing and word splitting. [SC2086]

Check notice on line 34 in monitoring.sh

View workflow job for this annotation

GitHub Actions / alidistlint 

Double quote to prevent globbing and word splitting. [SC2086]

@ktf ktf Nov 7, 2025 •
edited
Loading

Copy link
Copy Markdown
Member Author

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.

@ktf ktf Nov 7, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@singiamtel

@singiamtel singiamtel Nov 7, 2025

Copy link
Copy Markdown
Contributor

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.

@ktf ktf Nov 7, 2025

Copy link
Copy Markdown
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.

${LIBRDKAFKA_REVISION:--DO2_MONITORING_KAFKA_ENABLE=0} \

Check notice on line 35 in monitoring.sh

View workflow job for this annotation

GitHub Actions / alidistlint 

Double quote to prevent globbing and word splitting. [SC2086]

Check notice on line 35 in monitoring.sh

View workflow job for this annotation

GitHub Actions / alidistlint 

Double quote to prevent globbing and word splitting. [SC2086]

Check notice on line 35 in monitoring.sh

View workflow job for this annotation

GitHub Actions / alidistlint 

Double quote to prevent globbing and word splitting. [SC2086]
${PROTOBUF_ROOT:+-DPROTOBUF_ROOT=$PROTOBUF_ROOT} \
-DCMAKE_EXPORT_COMPILE_COMMANDS=ON

cp ${BUILDDIR}/compile_commands.json ${INSTALLROOT}

make ${JOBS+-j $JOBS} install
cmake --build . -- ${JOBS+-j $JOBS} install

if [[ $ALIBUILD_O2_TESTS ]]; then
ctest --output-on-failure
fi


#ModuleFile
mkdir -p etc/modulefiles
alibuild-generate-module --bin --lib > etc/modulefiles/$PKGNAME
cat >> etc/modulefiles/$PKGNAME <<EoF
# Our environment
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

Check notice on line 49 in monitoring.sh

View workflow job for this annotation

GitHub Actions / alidistlint 

Double quote to prevent globbing and word splitting. [SC2086]

Check notice on line 49 in monitoring.sh

View workflow job for this annotation

GitHub Actions / alidistlint 

Double quote to prevent globbing and word splitting. [SC2086]

Check notice on line 49 in monitoring.sh

View workflow job for this annotation

GitHub Actions / alidistlint 

Double quote to prevent globbing and word splitting. [SC2086]

@ktf ktf Nov 7, 2025

Copy link
Copy Markdown
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.

mkdir -p $INSTALLROOT/etc/modulefiles && rsync -a --delete etc/modulefiles/ $INSTALLROOT/etc/modulefiles