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 all 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/
unzip -n $TGT/cuppa.jar -d /tmp/cuppa_jar/
${PYTHON} -m pip install --no-build-isolation --no-deps --no-cache-dir -vvv /tmp/cuppa_jar/pycuppa/
rm -r /tmp/cuppa_jar/
Comment on lines +14 to +17
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

Optimize file operations to avoid unnecessary moves

The current implementation moves the JAR file to the target directory and then unzips it from there, which is inefficient.

Apply this diff to optimize the operations:

-mv cuppa*.jar $TGT/cuppa.jar
+TEMP_DIR=$(mktemp -d) || exit 1
+trap 'rm -rf "${TEMP_DIR}"' EXIT
 
-mkdir -p /tmp/cuppa_jar/
-unzip -n $TGT/cuppa.jar -d /tmp/cuppa_jar/
-${PYTHON} -m pip install --no-build-isolation --no-deps --no-cache-dir -vvv /tmp/cuppa_jar/pycuppa/
-rm -r /tmp/cuppa_jar/
+# First unzip and install from the source
+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
+
+# Then move the JAR to final location
+mv cuppa-*.jar $TGT/cuppa.jar || exit 1

Committable suggestion skipped: line range outside the PR's diff.

15 changes: 6 additions & 9 deletions recipes/hmftools-cuppa/meta.yaml
Original file line number Diff line number Diff line change
@@ -1,29 +1,26 @@
{% 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") }}

requirements:
host:
- python ==3.11
- r-base >=4.2,<4.3.0a0
- unzip
run:
- openjdk >=8
- python ==3.11
Expand Down
Loading