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.

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

Add tttrlib #51927

Merged
merged 20 commits into from
Nov 13, 2024
Merged

Add tttrlib #51927

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
20519ea
Add tttrlib recipe
tpeulen Nov 5, 2024
aaaf15a
Remove bld.bat
tpeulen Nov 5, 2024
828daf7
Fix version issues
tpeulen Nov 5, 2024
def59b2
Update meta.yaml
mencian Nov 5, 2024
a3e7d40
Update build.sh
mencian Nov 5, 2024
f5611f3
Delete recipes/tttrlib/LICENSE.txt
mencian Nov 5, 2024
db042c3
Update meta.yaml
mencian Nov 5, 2024
78a75c8
Update build.sh
mencian Nov 5, 2024
2f0d38c
Build environment uses python/numpy
tpeulen Nov 5, 2024
e23517a
Remove Windows app wrapper
tpeulen Nov 5, 2024
9bf368a
Update meta.yaml
mencian Nov 5, 2024
9e53bd7
Merge branch 'master' into add_tttrlib
mencian Nov 11, 2024
c239340
try swig in build section
mencian Nov 12, 2024
5441df0
Merge branch 'master' into add_tttrlib
mencian Nov 12, 2024
2158694
pin swig <4.3.0
mencian Nov 12, 2024
e35d14b
Update meta.yaml
mencian Nov 12, 2024
6f8c593
Update build.sh
mencian Nov 12, 2024
4a0ae5f
Update build.sh
mencian Nov 12, 2024
ec34809
Update build.sh
mencian Nov 13, 2024
f000548
Update meta.yaml
mencian Nov 13, 2024
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
31 changes: 31 additions & 0 deletions recipes/tttrlib/build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#!/bin/bash

mkdir b2 && cd b2

Comment on lines +3 to +4
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add shebang and improve directory handling robustness.

The script should handle potential failures in directory operations and be more explicit about the shell interpreter.

+#!/bin/bash
+set -euo pipefail
+
-mkdir b2 && cd b2
+build_dir="build"
+mkdir -p "${build_dir}" || exit 1
+cd "${build_dir}" || exit 1
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
mkdir b2 && cd b2
#!/bin/bash
set -euo pipefail
build_dir="build"
mkdir -p "${build_dir}" || exit 1
cd "${build_dir}" || exit 1
🧰 Tools
🪛 Shellcheck

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)

[warning] 1-1: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.

(SC2164)

Comment on lines +1 to +4
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve build directory handling and error detection.

The build directory creation needs better error handling and cleanup.

 #!/bin/bash
-
-mkdir b2 && cd b2
+set -euo pipefail
+
+BUILD_DIR="b2"
+trap 'rm -rf "${BUILD_DIR}"' EXIT
+rm -rf "${BUILD_DIR}"
+mkdir -p "${BUILD_DIR}" || exit 1
+cd "${BUILD_DIR}" || exit 1
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#!/bin/bash
mkdir b2 && cd b2
#!/bin/bash
set -euo pipefail
BUILD_DIR="b2"
trap 'rm -rf "${BUILD_DIR}"' EXIT
rm -rf "${BUILD_DIR}"
mkdir -p "${BUILD_DIR}" || exit 1
cd "${BUILD_DIR}" || exit 1
🧰 Tools
🪛 Shellcheck

[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.

(SC2164)

if [[ "${target_platform}" == osx-* ]]; then
# See https://conda-forge.org/docs/maintainer/knowledge_base.html#newer-c-features-with-old-sdk
CXXFLAGS="${CXXFLAGS} -D_LIBCPP_DISABLE_AVAILABILITY"
export CONFIG_ARGS="-DCMAKE_FIND_FRAMEWORK=NEVER -DCMAKE_FIND_APPBUNDLE=NEVER"
else
export CONFIG_ARGS=""
fi
Comment on lines +5 to +11
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add validation for target_platform variable.

The script assumes target_platform is set but should validate its existence to prevent potential errors.

+if [[ -z "${target_platform:-}" ]]; then
+    echo "Error: target_platform is not set" >&2
+    exit 1
+fi
+
 if [[ "${target_platform}" == osx-* ]]; then
   # See https://conda-forge.org/docs/maintainer/knowledge_base.html#newer-c-features-with-old-sdk
   CXXFLAGS="${CXXFLAGS} -D_LIBCPP_DISABLE_AVAILABILITY"
   export CONFIG_ARGS="-DCMAKE_FIND_FRAMEWORK=NEVER -DCMAKE_FIND_APPBUNDLE=NEVER"
 else
   export CONFIG_ARGS=""
 fi
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if [[ "${target_platform}" == osx-* ]]; then
# See https://conda-forge.org/docs/maintainer/knowledge_base.html#newer-c-features-with-old-sdk
CXXFLAGS="${CXXFLAGS} -D_LIBCPP_DISABLE_AVAILABILITY"
export CONFIG_ARGS="-DCMAKE_FIND_FRAMEWORK=NEVER -DCMAKE_FIND_APPBUNDLE=NEVER"
else
export CONFIG_ARGS=""
fi
if [[ -z "${target_platform:-}" ]]; then
echo "Error: target_platform is not set" >&2
exit 1
fi
if [[ "${target_platform}" == osx-* ]]; then
# See https://conda-forge.org/docs/maintainer/knowledge_base.html#newer-c-features-with-old-sdk
CXXFLAGS="${CXXFLAGS} -D_LIBCPP_DISABLE_AVAILABILITY"
export CONFIG_ARGS="-DCMAKE_FIND_FRAMEWORK=NEVER -DCMAKE_FIND_APPBUNDLE=NEVER"
else
export CONFIG_ARGS=""
fi
🧰 Tools
🪛 Shellcheck

[warning] 5-5: target_platform is referenced but not assigned.

(SC2154)


cmake -S .. -B . \
-DCMAKE_CXX_COMPILER="${CXX}" \
-DCMAKE_INSTALL_PREFIX="${PREFIX}" \
-DBUILD_PYTHON_INTERFACE=ON \
-DCMAKE_BUILD_TYPE=Release \
-DBUILD_LIBRARY=ON \
-DPYTHON_VERSION=$(python -c 'import platform; print(platform.python_version())') \
-DPython_ROOT_DIR="${PREFIX}/bin" \
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix Python root directory path.

The Python root directory should point to PREFIX, not PREFIX/bin. The current path is incorrect and may cause Python component discovery issues.

- -DPython_ROOT_DIR="${PREFIX}/bin" \
+ -DPython_ROOT_DIR="${PREFIX}" \
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
-DPython_ROOT_DIR="${PREFIX}/bin" \
-DPython_ROOT_DIR="${PREFIX}" \

-G Ninja \
"${CONFIG_ARGS}"

# On some platforms (notably aarch64 with Drone) builds can fail due to
# running out of memory. If this happens, try the build again; if it
# still fails, restrict to one core.
ninja install -k 0 || ninja install -k 0 || ninja install -j ${CPU_COUNT}

# Copy programs to bin
chmod 0755 $SRC_DIR/bin/*
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add directory existence check before chmod.

The script attempts to modify permissions without verifying the existence of the directory or files, which could mask errors.

-chmod 0755 $SRC_DIR/bin/*
+if [[ -d "${SRC_DIR}/bin" ]] && compgen -G "${SRC_DIR}/bin/*" > /dev/null; then
+    chmod 0755 "${SRC_DIR}"/bin/*
+else
+    echo "Error: No files found in ${SRC_DIR}/bin" >&2
+    exit 1
+fi
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
chmod 0755 $SRC_DIR/bin/*
if [[ -d "${SRC_DIR}/bin" ]] && compgen -G "${SRC_DIR}/bin/*" > /dev/null; then
chmod 0755 "${SRC_DIR}"/bin/*
else
echo "Error: No files found in ${SRC_DIR}/bin" >&2
exit 1
fi

cp -f $SRC_DIR/bin/* $PREFIX/bin
Comment on lines +29 to +31
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Improve security and robustness of binary installation.

The current binary installation process needs better security measures and validation.

-# Copy programs to bin
-chmod 0755 $SRC_DIR/bin/*
-cp -f $SRC_DIR/bin/* $PREFIX/bin
+# Install binaries with proper permissions
+BINDIR="${SRC_DIR}/bin"
+if [[ ! -d "${BINDIR}" ]]; then
+    echo "Error: Binary directory ${BINDIR} not found" >&2
+    exit 1
+fi
+
+# Ensure destination exists
+mkdir -p "${PREFIX}/bin"
+
+# Process each file individually with proper permission checks
+find "${BINDIR}" -type f -print0 | while IFS= read -r -d '' file; do
+    if [[ -x "${file}" ]]; then
+        install -v -m 755 "${file}" "${PREFIX}/bin/"
+    else
+        echo "Warning: Skipping non-executable file: ${file}" >&2
+    fi
+done
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Copy programs to bin
chmod 0755 $SRC_DIR/bin/*
cp -f $SRC_DIR/bin/* $PREFIX/bin
# Install binaries with proper permissions
BINDIR="${SRC_DIR}/bin"
if [[ ! -d "${BINDIR}" ]]; then
echo "Error: Binary directory ${BINDIR} not found" >&2
exit 1
fi
# Ensure destination exists
mkdir -p "${PREFIX}/bin"
# Process each file individually with proper permission checks
find "${BINDIR}" -type f -print0 | while IFS= read -r -d '' file; do
if [[ -x "${file}" ]]; then
install -v -m 755 "${file}" "${PREFIX}/bin/"
else
echo "Warning: Skipping non-executable file: ${file}" >&2
fi
done

70 changes: 70 additions & 0 deletions recipes/tttrlib/meta.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
{% set name = "tttrlib" %}
{% set version = "0.25.0" %}

package:
name: {{ name }}
version: {{ version }}

source:
url: https://github.com/Fluorescence-Tools/tttrlib/archive/refs/tags/v{{ version }}.tar.gz
sha256: e15604ef063257cd87d3bdf266c1d5a448189508f51cdc4156a93cb2b46fe807

build:
number: 0
skip: True # [osx and py > 310]
run_exports: '{{ pin_subpackage("tttrlib", max_pin="x.x") }}'

requirements:
build:
- {{ compiler('c') }}
- {{ compiler('cxx') }}
- pkg-config
- cmake
- ninja
- make
host:
- swig <4.3.0
- doxygen
- llvm-openmp # [osx]
- libgomp # [linux]
- boost-cpp
- hdf5
- python
- pip
- numpy
run:
- python
- llvm-openmp # [osx]
- libgomp # [linux]
- boost-cpp
- tqdm
- click
- click-didyoumean
- numpy
- scikit-image
- matplotlib-base
- hdf5

test:
imports:
- tttrlib
commands:
- tttrlib --help
Comment on lines +48 to +52
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add click packages to test requirements.

The test command uses the CLI (tttrlib --help) which depends on click packages, but they're not explicitly listed in test requirements.

 test:
   imports:
     - tttrlib
+  requires:
+    - click
+    - click-didyoumean
   commands:
     - tttrlib --help
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
test:
imports:
- tttrlib
commands:
- tttrlib --help
test:
imports:
- tttrlib
requires:
- click
- click-didyoumean
commands:
- tttrlib --help


about:
home: https://github.com/fluorescence-tools/tttrlib
summary: 'A file format agnostic library for time-resolved imaging and spectroscopic data.'
description: |
tttrlib is a simple, fast, libray to read, write and process
time-resolved imaging and spectroscopic data. For speed, it
is written in C++ and wrapped for Python via SWIG.
license: BSD-3-Clause
license_family: BSD
license_file: LICENSE.txt
doc_url: https://tttrlib.readthedocs.io
dev_url: https://github.com/fluorescence-tools/tttrlib

extra:
recipe-maintainers:
- tpeulen
- khemmen