[Env] add archlinux envs by tdavidcl · Pull Request #1174 · Shamrock-code/Shamrock

tdavidcl · 2025-08-12T14:39:59Z

No description provided.

gemini-code-assist

Summary of Changes

Hello @tdavidcl, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces new environment configurations tailored for Arch Linux, specifically enabling the setup and compilation of projects using either the AdaptiveCpp (ACPP) or Intel LLVM SYCL compilers. These additions aim to simplify the development workflow for Arch Linux users by automating dependency checks, compiler setup, and project configuration within the Shamrock framework.

Highlights

Arch Linux Environment Support: Added new environment setup scripts (.sh and .py files) specifically for Arch Linux.
AdaptiveCpp (ACPP) Integration: Introduced configuration and build scripts to support AdaptiveCpp as a SYCL backend on Arch Linux.
Intel LLVM SYCL Integration: Added configuration and build scripts to support the Intel LLVM SYCL compiler on Arch Linux.
Automated Dependency Checks: The new shell scripts include pacman checks to ensure all necessary build dependencies are installed on Arch Linux.
Streamlined Compiler Setup: Functions are provided to automatically clone, configure, and install both AdaptiveCpp and Intel LLVM compilers if they are not already present.
Shamrock Project Configuration: The scripts include functions to configure the Shamrock project with the newly set up SYCL compilers (ACPPDirect or IntelLLVM).

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request adds environment setup scripts for Arch Linux with AdaptiveCpp and Intel LLVM. The changes are well-structured, but there are a few areas for improvement.

In the shell scripts, several variables representing file paths are not quoted, which could lead to issues if the paths contain spaces. I've suggested quoting them for robustness. I also suggested a more robust way to set LD_LIBRARY_PATH.

In the Python script for AdaptiveCpp setup, there's a critical issue where the script could crash with a TypeError if a target is not specified. I've provided a fix for this. Additionally, I've suggested using os.path.join for constructing paths in both Python scripts for better portability, and using is None for identity checks which is more Pythonic.

Overall, these are good additions and with these changes, the scripts will be more robust and reliable.

gemini-code-assist · 2025-08-12T14:42:16Z

env/machine/archlinux/acpp/setup-env.py

+    acpp_target = utils.acpp.get_acpp_target_env(args)
+    if acpp_target == None:
+        print("-- target not specified using acpp default")
+    else:
+        print("-- setting acpp target to :", acpp_target)


If utils.acpp.get_acpp_target_env(args) returns None, acpp_target will be None. Later, on line 53, concatenating it with a string (... + acpp_target + ...) will raise a TypeError, crashing the script. You should handle the None case by providing a default value, for example an empty string.

Also, it's idiomatic in Python to use is None for identity comparison with None instead of == None.

Suggested change

acpp_target = utils.acpp.get_acpp_target_env(args)

if acpp_target == None:

print("-- target not specified using acpp default")

else:

print("-- setting acpp target to :", acpp_target)

acpp_target = utils.acpp.get_acpp_target_env(args)

if acpp_target is None:

print("-- target not specified using acpp default")

acpp_target = ""

else:

print("-- setting acpp target to :", acpp_target)

gemini-code-assist · 2025-08-12T14:42:16Z

env/machine/archlinux/acpp/env_built_acpp.sh

+    cmake -S ${ACPP_GIT_DIR} -B ${ACPP_BUILD_DIR} -DCMAKE_INSTALL_PREFIX=${ACPP_INSTALL_DIR} || return
+    (cd ${ACPP_BUILD_DIR} && $MAKE_EXEC "${MAKE_OPT[@]}" && $MAKE_EXEC install) || return


It's a good practice in shell scripting to quote variables that represent paths to prevent issues with word splitting and globbing if the paths contain spaces or special characters. Variables like ${ACPP_GIT_DIR}, ${ACPP_BUILD_DIR}, and ${ACPP_INSTALL_DIR} should be quoted.

Suggested change

cmake -S ${ACPP_GIT_DIR} -B ${ACPP_BUILD_DIR} -DCMAKE_INSTALL_PREFIX=${ACPP_INSTALL_DIR} || return

(cd ${ACPP_BUILD_DIR} && $MAKE_EXEC "${MAKE_OPT[@]}" && $MAKE_EXEC install) || return

cmake -S "${ACPP_GIT_DIR}" -B "${ACPP_BUILD_DIR}" -DCMAKE_INSTALL_PREFIX="${ACPP_INSTALL_DIR}" || return

(cd "${ACPP_BUILD_DIR}" && $MAKE_EXEC "${MAKE_OPT[@]}" && $MAKE_EXEC install) || return

gemini-code-assist · 2025-08-12T14:42:16Z

env/machine/archlinux/acpp/env_built_acpp.sh

+    cmake \
+        -S $SHAMROCK_DIR \
+        -B $BUILD_DIR \
+        -DSHAMROCK_ENABLE_BACKEND=SYCL \
+        -DSYCL_IMPLEMENTATION=ACPPDirect \
+        -DCMAKE_CXX_COMPILER="${ACPP_INSTALL_DIR}/bin/acpp" \
+        -DCMAKE_CXX_FLAGS="${SHAMROCK_CXX_FLAGS}" \
+        -DACPP_PATH="${ACPP_INSTALL_DIR}" \
+        -DCMAKE_BUILD_TYPE="${SHAMROCK_BUILD_TYPE}" \
+        -DBUILD_TEST=Yes \
+        "${CMAKE_OPT[@]}" || return


The variables $SHAMROCK_DIR and $BUILD_DIR should be quoted to handle potential spaces or special characters in the paths. This improves the script's robustness.

Suggested change

cmake \

-S $SHAMROCK_DIR \

-B $BUILD_DIR \

-DSHAMROCK_ENABLE_BACKEND=SYCL \

-DSYCL_IMPLEMENTATION=ACPPDirect \

-DCMAKE_CXX_COMPILER="${ACPP_INSTALL_DIR}/bin/acpp" \

-DCMAKE_CXX_FLAGS="${SHAMROCK_CXX_FLAGS}" \

-DACPP_PATH="${ACPP_INSTALL_DIR}" \

-DCMAKE_BUILD_TYPE="${SHAMROCK_BUILD_TYPE}" \

-DBUILD_TEST=Yes \

"${CMAKE_OPT[@]}" || return

cmake \

-S "$SHAMROCK_DIR" \

-B "$BUILD_DIR" \

-DSHAMROCK_ENABLE_BACKEND=SYCL \

-DSYCL_IMPLEMENTATION=ACPPDirect \

-DCMAKE_CXX_COMPILER="${ACPP_INSTALL_DIR}/bin/acpp" \

-DCMAKE_CXX_FLAGS="${SHAMROCK_CXX_FLAGS}" \

-DACPP_PATH="${ACPP_INSTALL_DIR}" \

-DCMAKE_BUILD_TYPE="${SHAMROCK_BUILD_TYPE}" \

-DBUILD_TEST=Yes \

"${CMAKE_OPT[@]}" || return

gemini-code-assist · 2025-08-12T14:42:16Z

env/machine/archlinux/acpp/env_built_acpp.sh

+}
+
+function shammake {
+    (cd $BUILD_DIR && $MAKE_EXEC "${MAKE_OPT[@]}" "${@}") || return


The $BUILD_DIR variable should be quoted to prevent issues if the path contains spaces.

Suggested change

(cd $BUILD_DIR && $MAKE_EXEC "${MAKE_OPT[@]}" "${@}") || return

(cd "$BUILD_DIR" && $MAKE_EXEC "${MAKE_OPT[@]}" "${@}") || return

gemini-code-assist · 2025-08-12T14:42:16Z

env/machine/archlinux/acpp/setup-env.py

+        shamrockdir + "/env/helpers/clone-acpp.sh",
+        shamrockdir + "/env/helpers/pull_reffiles.sh",


Using os.path.join() to construct paths is more portable and safer than string concatenation, as it handles path separators correctly across different operating systems.

Suggested change

shamrockdir + "/env/helpers/clone-acpp.sh",

shamrockdir + "/env/helpers/pull_reffiles.sh",

os.path.join(shamrockdir, "env", "helpers", "clone-acpp.sh"),

os.path.join(shamrockdir, "env", "helpers", "pull_reffiles.sh"),

gemini-code-assist · 2025-08-12T14:42:17Z

env/machine/archlinux/intel-llvm/env_built_intel-llvm.sh

+export INTEL_LLVM_INSTALL_DIR=$BUILD_DIR/.env/intelllvm-install
+clone_intel_llvm || return
+
+export LD_LIBRARY_PATH=$INTEL_LLVM_INSTALL_DIR/lib:$LD_LIBRARY_PATH


To avoid issues with an empty $LD_LIBRARY_PATH creating a leading or trailing colon, it's more robust to conditionally add the colon separator.

Suggested change

export LD_LIBRARY_PATH=$INTEL_LLVM_INSTALL_DIR/lib:$LD_LIBRARY_PATH

export LD_LIBRARY_PATH="$INTEL_LLVM_INSTALL_DIR/lib${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}"

gemini-code-assist · 2025-08-12T14:42:17Z

env/machine/archlinux/intel-llvm/env_built_intel-llvm.sh

+    python3 ${INTEL_LLVM_GIT_DIR}/buildbot/configure.py \
+        "${INTEL_LLVM_CONFIGURE_ARGS[@]}" \
+        --cmake-gen "${CMAKE_GENERATOR}" \
+        --cmake-opt="-DCMAKE_INSTALL_PREFIX=${INTEL_LLVM_INSTALL_DIR}" || return
+
+    (cd ${INTEL_LLVM_GIT_DIR}/build && $MAKE_EXEC "${MAKE_OPT[@]}" all libsycldevice) || return
+    (cd ${INTEL_LLVM_GIT_DIR}/build && $MAKE_EXEC install) || return


Variables representing file paths should be quoted to prevent issues with spaces or special characters. This applies to ${INTEL_LLVM_GIT_DIR}. This advice applies to other parts of the script as well, such as in shamconfigure and shammake.

Suggested change

python3 ${INTEL_LLVM_GIT_DIR}/buildbot/configure.py \

"${INTEL_LLVM_CONFIGURE_ARGS[@]}" \

--cmake-gen "${CMAKE_GENERATOR}" \

--cmake-opt="-DCMAKE_INSTALL_PREFIX=${INTEL_LLVM_INSTALL_DIR}" || return

(cd ${INTEL_LLVM_GIT_DIR}/build && $MAKE_EXEC "${MAKE_OPT[@]}" all libsycldevice) || return

(cd ${INTEL_LLVM_GIT_DIR}/build && $MAKE_EXEC install) || return

python3 "${INTEL_LLVM_GIT_DIR}/buildbot/configure.py" \

"${INTEL_LLVM_CONFIGURE_ARGS[@]}" \

--cmake-gen "${CMAKE_GENERATOR}" \

--cmake-opt="-DCMAKE_INSTALL_PREFIX=${INTEL_LLVM_INSTALL_DIR}" || return

(cd "${INTEL_LLVM_GIT_DIR}/build" && $MAKE_EXEC "${MAKE_OPT[@]}" all libsycldevice) || return

(cd "${INTEL_LLVM_GIT_DIR}/build" && $MAKE_EXEC install) || return

gemini-code-assist · 2025-08-12T14:42:17Z

env/machine/archlinux/intel-llvm/env_built_intel-llvm.sh

+    cmake \
+        -S $SHAMROCK_DIR \
+        -B $BUILD_DIR \
+        -DSHAMROCK_ENABLE_BACKEND=SYCL \
+        -DSYCL_IMPLEMENTATION=IntelLLVM \
+        -DINTEL_LLVM_PATH="${INTEL_LLVM_INSTALL_DIR}" \
+        -DCMAKE_CXX_COMPILER="${INTEL_LLVM_INSTALL_DIR}/bin/clang++" \
+        -DCMAKE_CXX_FLAGS="${SHAMROCK_CXX_FLAGS}" \
+        -DCMAKE_BUILD_TYPE="${SHAMROCK_BUILD_TYPE}" \
+        -DBUILD_TEST=Yes \
+        "${CMAKE_OPT[@]}" || return


The variables $SHAMROCK_DIR and $BUILD_DIR should be quoted to handle potential spaces or special characters in the paths. This improves the script's robustness.

Suggested change

cmake \

-S $SHAMROCK_DIR \

-B $BUILD_DIR \

-DSHAMROCK_ENABLE_BACKEND=SYCL \

-DSYCL_IMPLEMENTATION=IntelLLVM \

-DINTEL_LLVM_PATH="${INTEL_LLVM_INSTALL_DIR}" \

-DCMAKE_CXX_COMPILER="${INTEL_LLVM_INSTALL_DIR}/bin/clang++" \

-DCMAKE_CXX_FLAGS="${SHAMROCK_CXX_FLAGS}" \

-DCMAKE_BUILD_TYPE="${SHAMROCK_BUILD_TYPE}" \

-DBUILD_TEST=Yes \

"${CMAKE_OPT[@]}" || return

cmake \

-S "$SHAMROCK_DIR" \

-B "$BUILD_DIR" \

-DSHAMROCK_ENABLE_BACKEND=SYCL \

-DSYCL_IMPLEMENTATION=IntelLLVM \

-DINTEL_LLVM_PATH="${INTEL_LLVM_INSTALL_DIR}" \

-DCMAKE_CXX_COMPILER="${INTEL_LLVM_INSTALL_DIR}/bin/clang++" \

-DCMAKE_CXX_FLAGS="${SHAMROCK_CXX_FLAGS}" \

-DCMAKE_BUILD_TYPE="${SHAMROCK_BUILD_TYPE}" \

-DBUILD_TEST=Yes \

"${CMAKE_OPT[@]}" || return

gemini-code-assist · 2025-08-12T14:42:17Z

env/machine/archlinux/intel-llvm/env_built_intel-llvm.sh

+}
+
+function shammake {
+    (cd $BUILD_DIR && $MAKE_EXEC "${MAKE_OPT[@]}" "${@}") || return


The $BUILD_DIR variable should be quoted to prevent issues if the path contains spaces.

Suggested change

(cd $BUILD_DIR && $MAKE_EXEC "${MAKE_OPT[@]}" "${@}") || return

(cd "$BUILD_DIR" && $MAKE_EXEC "${MAKE_OPT[@]}" "${@}") || return

gemini-code-assist · 2025-08-12T14:42:17Z

env/machine/archlinux/intel-llvm/setup-env.py

+        shamrockdir + "/env/helpers/clone-intel-llvm.sh",
+        shamrockdir + "/env/helpers/pull_reffiles.sh",


Using os.path.join() to construct paths is more portable and safer than string concatenation, as it handles path separators correctly across different operating systems.

Suggested change

shamrockdir + "/env/helpers/clone-intel-llvm.sh",

shamrockdir + "/env/helpers/pull_reffiles.sh",

os.path.join(shamrockdir, "env", "helpers", "clone-intel-llvm.sh"),

os.path.join(shamrockdir, "env", "helpers", "pull_reffiles.sh"),

github-actions · 2025-08-12T16:42:16Z

Workflow report

workflow report corresponding to commit 45fba21
Commiter email is timothee.davidcleris@proton.me
GitHub page artifact URL GitHub page artifact link (can expire)

Pre-commit check report

Pre-commit check: ✅

trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check for merge conflicts................................................Passed
check that executables have shebangs.....................................Passed
check that scripts with shebangs are executable..........................Passed
check for added large files..............................................Passed
check for case conflicts.................................................Passed
check for broken symlinks................................................Passed
check yaml...............................................................Passed
detect private key.......................................................Passed
No-tabs checker..........................................................Passed
Tabs remover.............................................................Passed
Validate GitHub Workflows................................................Passed
clang-format.............................................................Passed
black....................................................................Passed
ruff check...............................................................Passed
Check doxygen headers....................................................Passed
Check license headers....................................................Passed
Check #pragma once.......................................................Passed
Check SYCL #include......................................................Passed
No ssh in git submodules remote..........................................Passed

Test pipeline can run.

Clang-tidy diff report

No relevant changes found.
Well done!

You should now go back to your normal life and enjoy a hopefully sunny day while waiting for the review.

Doxygen diff with `main`

Removed warnings : 0
New warnings : 0
Warnings count : 7021 → 7021 (0.0%)

Detailed changes :

[Env] add archlinux envs

45fba21

gemini-code-assist bot reviewed Aug 12, 2025

View reviewed changes

tdavidcl added the in-review label Aug 12, 2025

tdavidcl merged commit 24c7e68 into Shamrock-code:main Aug 12, 2025
45 checks passed

tdavidcl deleted the arch-envs branch September 12, 2025 08:14

		cmake -S ${ACPP_GIT_DIR} -B ${ACPP_BUILD_DIR} -DCMAKE_INSTALL_PREFIX=${ACPP_INSTALL_DIR} \|\| return
		(cd ${ACPP_BUILD_DIR} && $MAKE_EXEC "${MAKE_OPT[@]}" && $MAKE_EXEC install) \|\| return

	(cd $BUILD_DIR && $MAKE_EXEC "${MAKE_OPT[@]}" "${@}") \|\| return
	(cd "$BUILD_DIR" && $MAKE_EXEC "${MAKE_OPT[@]}" "${@}") \|\| return

		shamrockdir + "/env/helpers/clone-acpp.sh",
		shamrockdir + "/env/helpers/pull_reffiles.sh",

	export LD_LIBRARY_PATH=$INTEL_LLVM_INSTALL_DIR/lib:$LD_LIBRARY_PATH
	export LD_LIBRARY_PATH="$INTEL_LLVM_INSTALL_DIR/lib${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}"

		shamrockdir + "/env/helpers/clone-intel-llvm.sh",
		shamrockdir + "/env/helpers/pull_reffiles.sh",

Conversation

tdavidcl commented Aug 12, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Aug 12, 2025

Workflow report

Pre-commit check report

Clang-tidy diff report

Doxygen diff with main

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Doxygen diff with `main`