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 khmer #49276

Closed
wants to merge 69 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
69 commits
Select commit Hold shift + click to select a range
d235628
python <3.12
lskatz Jul 15, 2024
3c89534
shoot forgot to add a new build number
lskatz Jul 15, 2024
500a02a
added name variable; added new pin_subpackage command
lskatz Jul 15, 2024
2df58db
Update python pinnings
rpetit3 Jul 17, 2024
78168e0
Merge branch 'master' into khmer
rpetit3 Jul 17, 2024
fe89993
attempt to fix missing `longintrepr.h` problem
dr-joe-wirth Jul 17, 2024
7c2dcb4
this should prevent skipping versions of 3.11 that are >3.11.0
dr-joe-wirth Jul 17, 2024
3a77668
is this a problem with the skip step? let's try commenting it out
dr-joe-wirth Jul 17, 2024
1225f0e
Sometimes chatgpt is smart, but it is not this day
dr-joe-wirth Jul 17, 2024
f90f96f
Python added to build
dr-joe-wirth Jul 17, 2024
5eb074e
update python pinnings
mencian Jul 18, 2024
9101adf
add c compiler back
mencian Jul 18, 2024
d668c95
seeing if a lower version of `khmer` will build.
dr-joe-wirth Jul 18, 2024
6396c5a
Revert "seeing if a lower version of `khmer` will build."
dr-joe-wirth Jul 18, 2024
d26c86e
Merge branch 'master' into khmer
mencian Jul 24, 2024
bb054f3
Merge branch 'master' into khmer
mencian Jul 24, 2024
7378ed0
Merge branch 'master' into khmer
dr-joe-wirth Jul 29, 2024
b5c5aab
trying to see if v2.1.1 will build
dr-joe-wirth Aug 6, 2024
c61e228
Fix build number to appease linter
dr-joe-wirth Aug 6, 2024
a710fc5
added `gcc` as a build requirement
dr-joe-wirth Aug 7, 2024
f006f5e
can't include `gcc`; it offends the linter
dr-joe-wirth Aug 7, 2024
3f132a4
seeing if adding compiler to `host` will fix missing `gcc` issue
dr-joe-wirth Aug 7, 2024
5ad90fc
appeasing the linter
dr-joe-wirth Aug 7, 2024
aa58b72
add `libgcc-ng` explicitly to give the linter the ole runaround
lskatz Aug 7, 2024
57cc445
remove patch and remove compiler in host step
lskatz Aug 7, 2024
4905cab
revert
lskatz Aug 7, 2024
faf097f
add the programs from the third-party subdirectory
lskatz Aug 7, 2024
7b35d1a
patch setup.py to not install dependencies
lskatz Aug 7, 2024
bfa0bf2
capitalize False
lskatz Aug 7, 2024
d292d9e
Revert "capitalize False"
lskatz Aug 7, 2024
997edb6
capitalize False
lskatz Aug 7, 2024
cf41d92
I think `zlib` is needed at runtime
dr-joe-wirth Aug 8, 2024
4a6ede9
might as well add the other new dependencies to 'run'
lskatz Aug 8, 2024
46c4be0
remove zlibdir and bzip2dir definitions
lskatz Aug 8, 2024
a4b06cc
new patch to remove lib third party dependencies
lskatz Aug 8, 2024
b91403e
python-dev
lskatz Aug 8, 2024
f2ab584
remove python-dev which isn't a real thing and put python into build
lskatz Aug 8, 2024
0e490a2
smhasher to run:
lskatz Aug 8, 2024
9e81815
include_dirs; mmh3 libraries
lskatz Aug 8, 2024
23a16fd
adding back `smhasher` third-party directory to `include_dirs`
dr-joe-wirth Aug 8, 2024
c95958f
removed `mmh3` from run
dr-joe-wirth Aug 8, 2024
6deaf33
removed `smhasher` from build and run
dr-joe-wirth Aug 8, 2024
6d6e998
hopefully including the directory where `seqan` header files live
dr-joe-wirth Aug 8, 2024
31748a9
does chat-gpt know where header files live?
dr-joe-wirth Aug 8, 2024
b0dfca9
more paths for the libs
lskatz Aug 8, 2024
c258382
add some debugging steps
lskatz Aug 8, 2024
0307736
need to include `lib` to find `khmer.hh`
dr-joe-wirth Aug 8, 2024
c82c09e
add build.sh to help us import libraries and have compiler flags
lskatz Aug 8, 2024
7a896ea
move build to build.sh
lskatz Aug 8, 2024
a0d808b
adding third party seqan libs
lskatz Aug 8, 2024
ce5620d
remove external seqan
lskatz Aug 8, 2024
b97ec2d
BUILD_PREFIX->PWD in some paths
lskatz Aug 8, 2024
485df9f
cp in seqan libraries to include
lskatz Aug 8, 2024
e9a955f
debugging statements
lskatz Aug 9, 2024
e696b4c
fix include/seqan bug
lskatz Aug 9, 2024
0840ed6
fix include/seqan bug
lskatz Aug 9, 2024
3af0eec
fix cp statement and add mkdir
lskatz Aug 9, 2024
fdb31cc
use $PYTHON instead of 'python3' in build
lskatz Aug 9, 2024
730f476
cute debug to find `khmer` path
dr-joe-wirth Aug 9, 2024
4864b18
setting `zlib` version to match the version in `third-party`
dr-joe-wirth Aug 9, 2024
b596c92
patching out dependencies from
dr-joe-wirth Aug 9, 2024
bdffee9
revising `zlib` version requirement
dr-joe-wirth Aug 9, 2024
97fa387
forgot to apply the patch to remove dependencies in manifest
dr-joe-wirth Aug 9, 2024
43de852
fixed patch
dr-joe-wirth Aug 9, 2024
8019cf3
trying to statically link conda-installed `zlib` (chat-gpt helped)
dr-joe-wirth Aug 9, 2024
bfd9477
fixed typo
dr-joe-wirth Aug 9, 2024
0cc6dd8
Revert "fixed typo"
dr-joe-wirth Aug 9, 2024
31ceb83
Revert "trying to statically link conda-installed `zlib` (chat-gpt he…
dr-joe-wirth Aug 9, 2024
8205a3d
Merge branch 'master' into khmer
mencian Nov 21, 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/khmer/0002-do-not-install-dependencies-on-our-own.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
--- khmer-2.1.1/setup.py 2017-05-25 20:10:21.000000000 -0400
+++ khmer-2.1.1/setup.py 2024-08-07 21:54:04.775775473 -0400
@@ -123,8 +123,8 @@
# change setup.cfg or use the `--libraries z,bz2` parameter which will make our
# custom build_ext command strip out the bundled versions.

-ZLIBDIR = 'third-party/zlib'
-BZIP2DIR = 'third-party/bzip2'
+# ZLIBDIR = 'third-party/zlib'
+# BZIP2DIR = 'third-party/bzip2'

BUILD_DEPENDS = []
BUILD_DEPENDS.extend(path_join("lib", bn + ".hh") for bn in [
@@ -259,7 +259,7 @@
if sys.platform == 'darwin' and 'gcov' in self.libraries:
self.libraries.remove('gcov')

- if "z" not in self.libraries:
+ if False:
zcmd = ['bash', '-c', 'cd ' + ZLIBDIR + ' && ( test Makefile -nt'
' configure || bash ./configure --static ) && make -f '
'Makefile.pic PIC']
@@ -269,7 +269,7 @@
"adler32", "compress", "crc32", "deflate", "gzclose",
"gzlib", "gzread", "gzwrite", "infback", "inffast",
"inflate", "inftrees", "trees", "uncompr", "zutil"])
- if "bz2" not in self.libraries:
+ if False:
bz2cmd = ['bash', '-c', 'cd ' + BZIP2DIR + ' && make -f '
'Makefile-libbz2_so all']
spawn(cmd=bz2cmd, dry_run=self.dry_run)
13 changes: 13 additions & 0 deletions recipes/khmer/0003-remove-dependencies-from-config.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
--- khmer-2.1.1/setup.cfg 2017-05-25 20:10:21.000000000 -0400
+++ khmer-2.1.1/setup.cfg 2024-08-07 22:04:20.100887810 -0400
@@ -3,8 +3,8 @@
undef = NO_UNIQUE_RC
# libraries = z,bz2
## if using system libraries
-include-dirs = lib:third-party/zlib:third-party/bzip2:third-party/seqan/core/include:third-party/smhasher
-# include-dirs = lib
+# include-dirs = lib:third-party/zlib:third-party/bzip2:third-party/seqan/core/include:third-party/smhasher
+include_dirs = lib:include:include/seqan:third-party/smhasher
## if using system libraries (broken)

# define = NDEBUG
21 changes: 21 additions & 0 deletions recipes/khmer/0004-remove-dependencies-from-manifest.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
--- khmer-2.1.1/MANIFEST.in
+++ khmer-2.1.1/MANIFEST.in
@@ -3,12 +3,12 @@
include LICENSE TODO .ycm_extra_conf.py authors.csv
recursive-include lib *.hh *.cc [Mm]akefile* get_version.py
recursive-include khmer *.hh
-recursive-include third-party *.cc *.1 *.xsl README* sample* words* *.sh *.c
-recursive-include third-party manual* [Mm]akefile* *.pl *.dsp CHANGES *.txt *.h
-recursive-include third-party ChangeLog FAQ INDEX configure *.xsl
-recursive-include third-party/bzip2 bzip.css entities.xml libbz2.def LICENSE
-include third-party/zlib/zconf.h.in third-party/zlib/zlib.pc.in
-exclude third-party/zlib/Makefile third-party/zlib/zconf.h
+#recursive-include third-party *.cc *.1 *.xsl README* sample* words* *.sh *.c
+#recursive-include third-party manual* [Mm]akefile* *.pl *.dsp CHANGES *.txt *.h
+#recursive-include third-party ChangeLog FAQ INDEX configure *.xsl
+#recursive-include third-party/bzip2 bzip.css entities.xml libbz2.def LICENSE
+#include third-party/zlib/zconf.h.in third-party/zlib/zlib.pc.in
+#exclude third-party/zlib/Makefile third-party/zlib/zconf.h
recursive-include scripts README.txt
graft tests
include scripts/oxli
32 changes: 32 additions & 0 deletions recipes/khmer/build.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#!/bin/bash

set -x -e

# export variables for compiling
export INCLUDE_PATH="${BUILD_PREFIX}/include:${PWD}/third-party/seqan/core/include"
export LIBRARY_PATH="${BUILD_PREFIX}/lib"
export LD_LIBRARY_PATH="${LD_LIBRARY_PATH}:${PWD}/lib"

export LDFLAGS="-L${BUILD_PREFIX}/lib -L${PWD}/third-party/seqan/core/include"
export CCPFLAGS="-I${BUILD_PREFIX}/include -I${PWD}/third-party/seqan/core/include"
Comment on lines +1 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

Review environment variable handling

The environment variable setup has several potential issues:

  1. INCLUDE_PATH and LIBRARY_PATH completely override system paths instead of appending
  2. Hardcoded paths to third-party/seqan could break if the directory structure changes
  3. Modifying LD_LIBRARY_PATH might affect runtime behavior unexpectedly

Consider these improvements:

-export INCLUDE_PATH="${BUILD_PREFIX}/include:${PWD}/third-party/seqan/core/include"
-export LIBRARY_PATH="${BUILD_PREFIX}/lib"
-export LD_LIBRARY_PATH="${LD_LIBRARY_PATH}:${PWD}/lib"
+export INCLUDE_PATH="${INCLUDE_PATH:+$INCLUDE_PATH:}${BUILD_PREFIX}/include:${PWD}/third-party/seqan/core/include"
+export LIBRARY_PATH="${LIBRARY_PATH:+$LIBRARY_PATH:}${BUILD_PREFIX}/lib"
# Remove LD_LIBRARY_PATH modification as it's typically not needed during build
📝 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
set -x -e
# export variables for compiling
export INCLUDE_PATH="${BUILD_PREFIX}/include:${PWD}/third-party/seqan/core/include"
export LIBRARY_PATH="${BUILD_PREFIX}/lib"
export LD_LIBRARY_PATH="${LD_LIBRARY_PATH}:${PWD}/lib"
export LDFLAGS="-L${BUILD_PREFIX}/lib -L${PWD}/third-party/seqan/core/include"
export CCPFLAGS="-I${BUILD_PREFIX}/include -I${PWD}/third-party/seqan/core/include"
#!/bin/bash
set -x -e
# export variables for compiling
export INCLUDE_PATH="${INCLUDE_PATH:+$INCLUDE_PATH:}${BUILD_PREFIX}/include:${PWD}/third-party/seqan/core/include"
export LIBRARY_PATH="${LIBRARY_PATH:+$LIBRARY_PATH:}${BUILD_PREFIX}/lib"
# Remove LD_LIBRARY_PATH modification as it's typically not needed during build
export LDFLAGS="-L${BUILD_PREFIX}/lib -L${PWD}/third-party/seqan/core/include"
export CCPFLAGS="-I${BUILD_PREFIX}/include -I${PWD}/third-party/seqan/core/include"


# create directories for compiling
mkdir -p $PREFIX/bin
mkdir -p $PREFIX/include
mkdir -p $PREFIX/lib
mkdir -p $PREFIX/scripts
Comment on lines +13 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.

⚠️ Potential issue

Add safety check for $PREFIX environment variable

The script assumes $PREFIX is set but doesn't verify it. This could lead to unexpected behavior if the variable is unset.

Add this check at the beginning of the script:

 set -x -e

+# Verify required environment variables
+if [ -z "${PREFIX}" ]; then
+    echo "Error: PREFIX environment variable is not set"
+    exit 1
+fi

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


# populate compiling directories
cp -v lib/* $PREFIX/include
cp -v scripts/* $PREFIX/bin

# copies over third-party/seqan/core/include/seqan
mkdir -pv $PREFIX/include/seqan
cp -vr third-party/seqan/core/include/seqan/* $PREFIX/include/seqan/
Comment on lines +19 to +25
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 incorrect file copying operations and add error handling

There are several issues with the file copying operations:

  1. Copying lib/* to include directory seems incorrect - library files should go to lib
  2. No verification that source files exist before copying
  3. Missing error handling for failed copies

Consider this improved implementation:

-cp -v lib/* $PREFIX/include
-cp -v scripts/* $PREFIX/bin
+# Verify source directories exist
+for dir in lib scripts third-party/seqan/core/include/seqan; do
+    if [ ! -d "$dir" ]; then
+        echo "Error: Required directory $dir not found"
+        exit 1
+    fi
+done
+
+# Copy files to correct locations
+cp -v lib/* $PREFIX/lib || { echo "Error copying library files"; exit 1; }
+cp -v scripts/* $PREFIX/bin || { echo "Error copying scripts"; exit 1; }

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


# install khmer
$PYTHON -m pip install . --no-deps --no-build-isolation --no-cache-dir --use-pep517 -vvv

# debugging line to find where khmer was installed
echo "here i am! ⊂◉‿◉つ "
ls -lhR
45 changes: 26 additions & 19 deletions recipes/khmer/meta.yaml
Original file line number Diff line number Diff line change
@@ -1,43 +1,49 @@
{% set version = "3.0.0a3" %}
{% set sha256 = "9850baec8b905c0896f00d0e427d4564f4c87dd7f1abae64734ad0361678e620" %}
{% set version = "2.1.1" %}
{% set sha256 = "39981730c2e08ee183c5ce6ce80cfe2bd6d813cfa37b9ae1f7be0dd1a01dae85" %}
{% set name = "khmer" %}

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

source:
url: https://github.com/dib-lab/khmer/archive/v{{ version }}.tar.gz
sha256: {{ sha256 }}
patches:
- 0001-Inject-compiler-and-flags-for-third-party-compilation.patch
- 0002-do-not-install-dependencies-on-our-own.patch
- 0003-remove-dependencies-from-config.patch
- 0004-remove-dependencies-from-manifest.patch

build:
number: 4
skip: True # [py<30 or py>=312 or osx]
script: {{ PYTHON }} -m pip install . --no-deps -vv
number: 0
skip: True # [py >= 312]
run_exports:
- {{ pin_subpackage('khmer', max_pin="x.x") }}
- {{ pin_subpackage(name|lower, max_pin="x.x") }}

Comment on lines +18 to +21
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

Inconsistency with PR objectives regarding Python 3.12 compatibility.

The PR objectives mention addressing Python 3.12 compatibility issues, but the recipe explicitly skips Python >= 3.12. This seems contradictory.

Please clarify:

  1. If the package should support Python 3.12, remove the skip condition
  2. If Python 3.12 is not supported, update the PR objectives to reflect this limitation

requirements:
build:
- {{ compiler("c") }}
- {{ compiler("cxx") }}
- llvm-openmp # [osx]
- libgomp # [linux]
- bzip2
- make
- python
- zlib
host:
- python >=3.6,<3.12
- python
- pip
- setuptools >=18.0
- Cython >=0.25.2
- cython >=0.25.2
- pytest-runner
- llvm-openmp # [osx]
- libgomp # [linux]
- bzip2
- zlib
run:
- python >=3.6,<3.12
- screed >=1.0
- bz2file
- bzip2
- bz2file
- python
- screed >=1.0
- zlib
- smhasher

test:
imports:
Expand All @@ -46,11 +52,12 @@ test:
- oxli

about:
home: https://khmer.readthedocs.io/
license: BSD License
home: "https://github.com/dib-lab/khmer"
license: "BSD-3-Clause"
license_family: BSD
summary: khmer k-mer counting library
dev_url: https://github.com/dib-lab/khmer
dev_url: "https://github.com/dib-lab/khmer"
doc_url: "https://khmer.readthedocs.io"

extra:
identifiers:
Expand Down
Loading