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

Add recipe for stxtyper v1.0.25 #51792

Merged
merged 2 commits into from
Oct 29, 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
11 changes: 11 additions & 0 deletions recipes/stxtyper/build.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#!/bin/bash

export INCLUDES="-I${PREFIX}/include"
export LIBPATH="-L${PREFIX}/lib"
export CXXFLAGS="${CXXFLAGS} -O3"
export CPPFLAGS="${CPPFLAGS} -O3 -I${PREFIX}/include"
export LDFLAGS="${LDFLAGS} -L${PREFIX}/lib"
Comment on lines +3 to +7
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

Consider more robust compiler flag handling.

While the environment setup is generally correct, there are a few suggestions for improvement:

  1. The -O3 optimization level might be too aggressive and could cause issues on some platforms
  2. Consider using DESTDIR for staged installations

Here's a more robust approach:

-export INCLUDES="-I${PREFIX}/include"
-export LIBPATH="-L${PREFIX}/lib"
-export CXXFLAGS="${CXXFLAGS} -O3"
-export CPPFLAGS="${CPPFLAGS} -O3 -I${PREFIX}/include"
-export LDFLAGS="${LDFLAGS} -L${PREFIX}/lib"
+# Use -O2 for better stability
+export CXXFLAGS="${CXXFLAGS} -O2"
+export CPPFLAGS="${CPPFLAGS} -O2 -I${PREFIX}/include"
+export LDFLAGS="${LDFLAGS} -L${PREFIX}/lib"
📝 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
export INCLUDES="-I${PREFIX}/include"
export LIBPATH="-L${PREFIX}/lib"
export CXXFLAGS="${CXXFLAGS} -O3"
export CPPFLAGS="${CPPFLAGS} -O3 -I${PREFIX}/include"
export LDFLAGS="${LDFLAGS} -L${PREFIX}/lib"
# Use -O2 for better stability
export CXXFLAGS="${CXXFLAGS} -O2"
export CPPFLAGS="${CPPFLAGS} -O2 -I${PREFIX}/include"
export LDFLAGS="${LDFLAGS} -L${PREFIX}/lib"


make CXX="${CXX}" CXXFLAGS="${CXXFLAGS}" CPPFLAGS="${CPPFLAGS}" -j"${CPU_COUNT}"

make install PREFIX="${PREFIX}"
Comment on lines +9 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.

⚠️ Potential issue

Add error checking and ensure proper installation.

The build and install commands should include error checking and use DESTDIR for staged installations.

Apply these improvements:

-make CXX="${CXX}" CXXFLAGS="${CXXFLAGS}" CPPFLAGS="${CPPFLAGS}" -j"${CPU_COUNT}"
+# Ensure CPU_COUNT is set
+export CPU_COUNT=${CPU_COUNT:-1}
+
+# Build with error checking
+if ! make CXX="${CXX}" CXXFLAGS="${CXXFLAGS}" CPPFLAGS="${CPPFLAGS}" -j"${CPU_COUNT}"; then
+    echo "Build failed"
+    exit 1
+fi
 
-make install PREFIX="${PREFIX}"
+# Install with DESTDIR for proper staging
+if ! make install DESTDIR="${PREFIX}" PREFIX=""; then
+    echo "Installation failed"
+    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
make CXX="${CXX}" CXXFLAGS="${CXXFLAGS}" CPPFLAGS="${CPPFLAGS}" -j"${CPU_COUNT}"
make install PREFIX="${PREFIX}"
# Ensure CPU_COUNT is set
export CPU_COUNT=${CPU_COUNT:-1}
# Build with error checking
if ! make CXX="${CXX}" CXXFLAGS="${CXXFLAGS}" CPPFLAGS="${CPPFLAGS}" -j"${CPU_COUNT}"; then
echo "Build failed"
exit 1
fi
# Install with DESTDIR for proper staging
if ! make install DESTDIR="${PREFIX}" PREFIX=""; then
echo "Installation failed"
exit 1
fi

36 changes: 36 additions & 0 deletions recipes/stxtyper/meta.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
{% set name = "stxtyper" %}
{% set version = "1.0.25" %}
{% set sha256 = "adc734340c6a10f1bfc34ca637197e93e1515d0231bd023b33ce4c07aab9578e" %}

package:
name: {{ name|lower }}
version: {{ version }}

source:
url: https://github.com/ncbi/stxtyper/archive/refs/tags/v{{ version }}.tar.gz
sha256: {{ sha256 }}

build:
number: 0
run_exports:
- {{ pin_subpackage('stxtyper', max_pin='x') }}

requirements:
build:
- make
- {{ compiler('cxx') }}
host:
- zlib
run:
- blast

test:
commands:
- "stxtyper -h"

about:
home: https://github.com/ncbi/stxtyper
license: Public Domain
license_file: LICENSE
summary: 'Accurately type both known and unknown Shiga toxin operons from assembled genomic sequence.'
dev_url: https://github.com/ncbi/stxtyper
Loading