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

Update hmftools CHORD and CUPPA #52175

Merged
merged 9 commits into from
Nov 19, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
6 changes: 3 additions & 3 deletions recipes/hmftools-chord/meta.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{% set version = "2.1.0_beta" %}
{% set beta_suffix = ".1" %}
{% set sha256 = "25a4dfd8a0ab436e0f23d94e77cebb59a0830a052a90c823844b798bd409bfc3" %}
{% set beta_suffix = ".2" %}
{% set sha256 = "847e0239b7440b70d67957d38afb3ab42bd3acecb931afc49ab8aef4c248a1da" %}

package:
name: hmftools-chord
Expand All @@ -12,7 +12,7 @@ source:

build:
noarch: generic
number: 2
number: 3
run_exports:
- {{ pin_subpackage("hmftools-chord", max_pin="x.x") }}

Expand Down
11 changes: 8 additions & 3 deletions recipes/hmftools-cuppa/build.sh
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
#!/bin/bash

TGT="$PREFIX/share/$PKG_NAME-$PKG_VERSION-$PKG_BUILDNUM"
[ -d "$TGT" ] || mkdir -p $TGT/{,chart/}
[ -d "$TGT" ] || mkdir -p $TGT
[ -d "${PREFIX}/bin" ] || mkdir -p "${PREFIX}/bin"
Comment on lines +4 to 5
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 proper quoting and error handling for directory creation

The directory creation commands should be properly quoted and include error handling to ensure script reliability.

Apply this diff:

-[ -d "$TGT" ] || mkdir -p $TGT
-[ -d "${PREFIX}/bin" ] || mkdir -p "${PREFIX}/bin"
+[ -d "$TGT" ] || mkdir -p "$TGT" || exit 1
+[ -d "${PREFIX}/bin" ] || mkdir -p "${PREFIX}/bin" || 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
[ -d "$TGT" ] || mkdir -p $TGT
[ -d "${PREFIX}/bin" ] || mkdir -p "${PREFIX}/bin"
[ -d "$TGT" ] || mkdir -p "$TGT" || exit 1
[ -d "${PREFIX}/bin" ] || mkdir -p "${PREFIX}/bin" || exit 1


cd "${SRC_DIR}"
mv jar/cuppa*.jar $TGT/cuppa.jar
${PYTHON} -m pip install --no-build-isolation --no-deps --no-cache-dir -vvv src/cuppa/src/main/python/pycuppa/
mv cuppa*.jar $TGT/cuppa.jar
Comment on lines 7 to +8
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 error handling for directory change operation

The cd command should handle potential failures to prevent script from continuing in wrong directory.

Apply this diff to add error handling:

-cd "${SRC_DIR}"
+cd "${SRC_DIR}" || exit 1

Also, consider making the JAR file pattern more specific to avoid potential conflicts:

-mv cuppa*.jar $TGT/cuppa.jar
+mv cuppa-*.jar $TGT/cuppa.jar
📝 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
cd "${SRC_DIR}"
mv jar/cuppa*.jar $TGT/cuppa.jar
${PYTHON} -m pip install --no-build-isolation --no-deps --no-cache-dir -vvv src/cuppa/src/main/python/pycuppa/
mv cuppa*.jar $TGT/cuppa.jar
cd "${SRC_DIR}" || exit 1
mv cuppa-*.jar $TGT/cuppa.jar
🧰 Tools
🪛 Shellcheck

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

(SC2164)


cp $RECIPE_DIR/cuppa.sh $TGT/cuppa
ln -s $TGT/cuppa ${PREFIX}/bin/
chmod 0755 "${PREFIX}/bin/cuppa"
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 error handling for chmod operation

Critical operations like chmod should include error handling.

Apply this diff:

-chmod 0755 "${PREFIX}/bin/cuppa"
+chmod 0755 "${PREFIX}/bin/cuppa" || 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
chmod 0755 "${PREFIX}/bin/cuppa"
chmod 0755 "${PREFIX}/bin/cuppa" || exit 1


mkdir -p /tmp/cuppa_jar/
RUN unzip -n cuppa*.jar -d /tmp/cuppa_jar/
${PYTHON} -m pip install --no-build-isolation --no-deps --no-cache-dir -vvv /tmp/cuppa_jar/pycuppa/
RUN rm -r /tmp/cuppa_jar/
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 shell script syntax and add error handling

There are several issues in this section:

  1. The RUN prefix is incorrect (appears to be copied from a Dockerfile)
  2. Missing error handling for critical operations
  3. Temporary directory should use mktemp for safety

Apply this diff to fix the issues:

-mkdir -p /tmp/cuppa_jar/
-RUN unzip -n cuppa*.jar -d /tmp/cuppa_jar/
-${PYTHON} -m pip install --no-build-isolation --no-deps --no-cache-dir -vvv /tmp/cuppa_jar/pycuppa/
-RUN rm -r /tmp/cuppa_jar/
+TEMP_DIR=$(mktemp -d)
+trap 'rm -rf "${TEMP_DIR}"' EXIT
+
+unzip -n cuppa*.jar -d "${TEMP_DIR}" || exit 1
+${PYTHON} -m pip install --no-build-isolation --no-deps --no-cache-dir -vvv "${TEMP_DIR}/pycuppa/" || exit 1

The changes:

  • Use mktemp for secure temporary directory creation
  • Add cleanup trap to ensure temporary directory is removed
  • Remove incorrect RUN prefix
  • Add error handling for critical operations
📝 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 -p /tmp/cuppa_jar/
RUN unzip -n cuppa*.jar -d /tmp/cuppa_jar/
${PYTHON} -m pip install --no-build-isolation --no-deps --no-cache-dir -vvv /tmp/cuppa_jar/pycuppa/
RUN rm -r /tmp/cuppa_jar/
TEMP_DIR=$(mktemp -d)
trap 'rm -rf "${TEMP_DIR}"' EXIT
unzip -n cuppa*.jar -d "${TEMP_DIR}" || exit 1
${PYTHON} -m pip install --no-build-isolation --no-deps --no-cache-dir -vvv "${TEMP_DIR}/pycuppa/" || exit 1

14 changes: 5 additions & 9 deletions recipes/hmftools-cuppa/meta.yaml
Original file line number Diff line number Diff line change
@@ -1,22 +1,18 @@
{% set version = "2.3.0_beta" %}
{% set sha256_jar = "69f94a54f232a2a22927dce165120d1f166d4931bfb8f299515edaa577607c7c" %}
{% set sha256_src = "65a2070e78243fed975b8738d2ce79dfead1a5702c2bc91e26488c806e25d158" %}
{% set beta_suffix = ".1" %}
{% set sha256 = "ae3a3d9bc31e45cba52943df01b6f8f190beff904dff392e6413c1396d7db36d" %}

package:
name: hmftools-cuppa
version: '{{ version }}'

source:
- folder: jar
url: https://github.com/hartwigmedical/hmftools/releases/download/cuppa-v{{ version }}/cuppa-{{ version|replace(".0", "") }}.jar
sha256: '{{ sha256_jar }}'
- folder: src
url: https://github.com/hartwigmedical/hmftools/archive/refs/tags/cuppa-v{{ version }}.tar.gz
sha256: '{{ sha256_src }}'
url: https://github.com/hartwigmedical/hmftools/releases/download/cuppa-v{{ version }}/cuppa_v{{ version|replace(".0", "") }}{{ beta_suffix }}.jar
sha256: '{{ sha256 }}'

build:
noarch: generic
number: 0
number: 1
run_exports:
- {{ pin_subpackage("hmftools-cuppa", max_pin="x.x") }}

Expand Down
Loading