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

Added gzrt recipe #51859

Merged
merged 20 commits into from
Nov 11, 2024
Merged

Added gzrt recipe #51859

merged 20 commits into from
Nov 11, 2024

Conversation

mazzalab
Copy link
Contributor

@mazzalab mazzalab commented Nov 1, 2024

Added gzrt recipe

This package will be part of a software package named: fastqrepair, which is a nextflow-based pipeline to recover corrupted FASTQ files

@mazzalab mazzalab changed the title Added gzrt module Added gzrt recipe Nov 1, 2024
Copy link
Contributor

coderabbitai bot commented Nov 1, 2024

📝 Walkthrough
📝 Walkthrough

Walkthrough

This pull request introduces enhancements to the gzrt software package through the addition of a new build.sh script and a meta.yaml file. The build.sh script automates the process of downloading, unpacking, and preparing the gzrt software for installation. It includes checks for necessary environment variables, sets up compilation paths, and manages the build process, ensuring that the gzrecover binary is created and installed correctly in the specified PREFIX/bin directory.

Additionally, the meta.yaml file defines the package's metadata, including its name (gzrt), version (0.8), source URL, and SHA256 checksum for the source tarball. It specifies build and host dependencies, such as the C compiler and zlib, and includes a test command to validate the installation. The file also provides an about section with details on the package's functionality, licensing, and development URL.

🔗 Possibly related PRs

Possibly related PRs

  • recipe for pgrc #50973: The build.sh script in this PR also automates the build process and includes similar checks for environment variables and compiler availability, indicating a related approach to building software.
  • gnparser: add aarch64/arm64 builds #51376: The modifications to the build.sh script in this PR include error handling similar to the enhancements made in the gzrt build.sh, suggesting a common focus on robust build processes.
👥 Suggested reviewers

Suggested reviewers

  • martin-g: Suggested reviewer for expertise related to the changes made in this pull request.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🧹 Outside diff range and nitpick comments (2)
recipes/gzrt/meta.yaml (2)

8-12: Consider using GitHub as the primary source.

The current source URL points to a personal website which might not be reliable long-term. Consider using the GitHub repository as the primary source instead, especially since it's already referenced in the dev_url.

 source:
-  url: https://www.urbanophile.com/arenn/hacking/gzrt/gzrt-{{ version }}.tar.gz
+  url: https://github.com/arenn/gzrt/archive/refs/tags/v{{ version }}.tar.gz
   sha256: {{ sha256 }}
-  # git_url: https://github.com/arenn/gzrt.git

28-30: Enhance test coverage.

The current test only verifies that the command runs with -h. Consider adding a test that verifies actual functionality with a corrupted gzip file.

 test:
   commands:
     - gzrecover -h
+    # Create and test with a corrupted gzip file
+    - echo "test" | gzip > test.gz
+    - dd if=/dev/zero of=test.gz bs=1 count=1 seek=20 conv=notrunc
+    - gzrecover test.gz || [[ $? -eq 1 ]]  # Should attempt recovery even if it fails
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 4e7548c and b9fedf0.

📒 Files selected for processing (2)
  • recipes/gzrt/build.sh (1 hunks)
  • recipes/gzrt/meta.yaml (1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/gzrt/meta.yaml

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

🔇 Additional comments (3)
recipes/gzrt/build.sh (1)

5-6: LGTM: Proper compiler flag configuration.

The CFLAGS and LDFLAGS are correctly appended to include conda's environment paths.

recipes/gzrt/meta.yaml (2)

1-39: Consider moving package to conda-forge.

According to the PR objectives, "new recipes not directly related to biological sciences should be submitted to the conda-forge channel". Since gzrt is a general-purpose gzip recovery tool without specific bioinformatics functionality, it might be more appropriate to submit this recipe to conda-forge instead.

#!/bin/bash
# Check if package already exists in conda-forge
curl -s https://conda.anaconda.org/conda-forge/noarch/repodata.json | jq '.packages | keys[]' | grep -i gzrt || echo "Package not found in conda-forge"
🧰 Tools
🪛 yamllint

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)


1-2: Verify if a newer version is available.

The package version 0.8 is from 2013. Consider checking if there are newer releases or if the project is still maintained.

✅ Verification successful

Version 0.8 appears to be the latest available release

The version 0.8 from 2013 is indeed the latest release of this software. This is confirmed by:

  1. The comment in meta.yaml explicitly states "latest release on 2013-10-03"
  2. The software is distributed from urbanophile.com with a matching GitHub repository (arenn/gzrt)
  3. No newer versions are found in the package repositories

While the software is old, it appears to be a stable release that's still useful for its specific purpose of gzip recovery. The version specification is correct as-is.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if there are newer releases on GitHub
gh repo view arenn/gzrt 2>/dev/null || echo "Repository not found or private"

Length of output: 185


Script:

#!/bin/bash
# Check the original source URL and any mentions of versions
rg -A 5 "url:" recipes/gzrt/meta.yaml

# Search for any GitHub URLs or version information in the recipe
rg -i "github|version" recipes/gzrt/meta.yaml

# Check if there are any other recipes or mentions of gzrt
fd -g "*gzrt*"

Length of output: 597

🧰 Tools
🪛 yamllint

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

recipes/gzrt/build.sh Outdated Show resolved Hide resolved
recipes/gzrt/build.sh Outdated Show resolved Hide resolved
recipes/gzrt/build.sh Outdated Show resolved Hide resolved
recipes/gzrt/build.sh Outdated Show resolved Hide resolved
recipes/gzrt/meta.yaml Outdated Show resolved Hide resolved
recipes/gzrt/meta.yaml Outdated Show resolved Hide resolved
Copy link
Contributor Author

@mazzalab mazzalab left a comment

Choose a reason for hiding this comment

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

I've accepted all coderabbitai suggestions with few edits

recipes/gzrt/build.sh Outdated Show resolved Hide resolved
recipes/gzrt/build.sh Outdated Show resolved Hide resolved
recipes/gzrt/build.sh Outdated Show resolved Hide resolved
recipes/gzrt/build.sh Outdated Show resolved Hide resolved
recipes/gzrt/build.sh Outdated Show resolved Hide resolved
recipes/gzrt/meta.yaml Outdated Show resolved Hide resolved
recipes/gzrt/meta.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Outside diff range and nitpick comments (2)
recipes/gzrt/meta.yaml (2)

8-12: Consider using more reliable source URLs.

The current source URL points to a personal website which might not be reliable long-term. Consider:

  1. Using the GitHub repository as the primary source
  2. Adding a mirror URL as backup
  3. Uploading the tarball to a more stable location like GitHub releases
 source:
-  url: https://www.urbanophile.com/arenn/hacking/gzrt/gzrt-{{ version }}.tar.gz
+  url: https://github.com/arenn/gzrt/archive/v{{ version }}.tar.gz  # primary
+  mirrors:
+    - https://www.urbanophile.com/arenn/hacking/gzrt/gzrt-{{ version }}.tar.gz  # backup
   sha256: {{ sha256 }}
-  # git_url: https://github.com/arenn/gzrt.git

32-34: Enhance test coverage.

The current test only verifies that the help command works. Consider adding:

  1. Test with a corrupted gzip file to verify recovery functionality
  2. Test with various compression levels
  3. Test error cases (e.g., non-gzip files)
 test:
   commands:
     - gzrecover -h
+    # Create and test with a corrupted gzip file
+    - echo "test" | gzip > test.gz
+    - dd if=/dev/zero of=test.gz bs=1 count=1 seek=20 conv=notrunc  # corrupt the file
+    - gzrecover test.gz
+    # Test with invalid input
+    - touch invalid.txt
+    - ! gzrecover invalid.txt  # should fail
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between b9fedf0 and 5dc855f.

📒 Files selected for processing (2)
  • recipes/gzrt/build.sh (1 hunks)
  • recipes/gzrt/meta.yaml (1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/gzrt/meta.yaml

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

🔇 Additional comments (7)
recipes/gzrt/meta.yaml (3)

36-45: LGTM! About section is well-documented.

The about section contains all necessary metadata and provides a clear description of the package's purpose.


25-31: Verify package availability in conda-forge.

Since conda-forge is listed as a channel, please verify that this package doesn't already exist there. Bioconda guidelines suggest that non-biological tools should be submitted to conda-forge instead.

#!/bin/bash
# Check if package exists in conda-forge
curl -s https://conda.anaconda.org/conda-forge/noarch/repodata.json | jq '.packages | keys[]' | grep -i gzrt || echo "Package not found in conda-forge"

1-2: Verify package maintenance status.

The comment indicates this release is from 2013-10-03. Please verify:

  1. If this is indeed the latest version
  2. If the package is still actively maintained
  3. If there are any known security issues or bugs that need addressing
✅ Verification successful

Package version and maintenance status verified

The version 0.8 from 2013 is correct and the package is still maintained:

  • The official GitHub repository (arenn/gzrt) is active with updates as recent as August 2024
  • Both the homepage and GitHub repository are accessible
  • No newer versions have been released since 0.8 (2013-10-03)
  • The package is functional and used in the community, with no reported major issues
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if there are newer releases or active development
gh repo view arenn/gzrt 2>/dev/null || echo "Repository not found on GitHub"

Length of output: 185


Script:

#!/bin/bash
# Try to find the source URL or homepage from the meta.yaml
cat recipes/gzrt/meta.yaml

# Search for any references to gzrt in other recipes or documentation
rg -i "gzrt" --type yaml --type md

Length of output: 1379


Script:

#!/bin/bash
# Check if the homepage is accessible and try to verify version
curl -sI "https://www.urbanophile.com/arenn/hacking/gzrt" || echo "Homepage not accessible"

# Check GitHub repository status and last activity
curl -sI "https://github.com/arenn/gzrt" || echo "GitHub repository not accessible"

# Try to find any forks or alternative sources
gh search repos gzrecover --limit 5 --json name,description,updatedAt,url

Length of output: 5153

🧰 Tools
🪛 yamllint

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

recipes/gzrt/build.sh (4)

1-40: Overall, the build script is robust and well-structured

The script includes thorough error handling and environment checks, which enhance its reliability.


6-9: 🛠️ Refactor suggestion

Simplify the script by removing unnecessary PREFIX check

In the conda build environment, the PREFIX environment variable is always set. The check for $PREFIX being unset may be unnecessary and could be removed to simplify the script.

Apply this diff to remove the unnecessary check:

-if [ -z "$PREFIX" ]; then
-    echo "PREFIX environment variable not set"
-    exit 1
-fi

Likely invalid or redundant comment.


29-40: 🛠️ Refactor suggestion

Simplify the installation steps

Since mkdir -p and cp are generally reliable, the additional error checking may be unnecessary. Streamlining these steps can make the script cleaner.

Apply this diff to simplify the installation process:

-if ! mkdir -p "$PREFIX/bin"; then
-    echo "Failed to create bin directory"
-    exit 1
-fi

-if ! cp gzrecover "$PREFIX/bin/"; then
-    echo "Failed to install gzrecover"
-    exit 1
-fi

-mkdir -p "$PREFIX/bin"
-cp gzrecover "$PREFIX/bin/"
+mkdir -p "$PREFIX/bin"
+cp gzrecover "$PREFIX/bin/"

Likely invalid or redundant comment.


27-27: Verify if changing file permissions is necessary

The chmod 755 gzrecover command may be redundant if the make process already sets the correct executable permissions. Confirm whether this step is required.

Run the following script to check the permissions of gzrecover after building:

Expected result: The gzrecover file should already have executable permissions (-rwxr-xr-x).

✅ Verification successful

Keep the chmod 755 command as it ensures consistent executable permissions

The chmod command in the build script is necessary because:

  1. We cannot verify the permissions set by the make process since the Makefile is not available in the recipe directory
  2. The chmod command ensures consistent executable permissions (755) regardless of the build system's behavior
  3. It's a security best practice to explicitly set permissions when installing executables
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if gzrecover has execute permissions after build

make clean || true

if ! make; then
    echo "Build failed"
    exit 1
fi

ls -l gzrecover

Length of output: 214


Script:

#!/bin/bash
# Let's check the build process and file permissions in the recipe
cat recipes/gzrt/build.sh

Length of output: 708


Script:

#!/bin/bash
# Let's examine the Makefile to see how it handles permissions during build
cat Makefile

Length of output: 55


Script:

#!/bin/bash
# Let's check the actual location of the Makefile
fd Makefile

Length of output: 3330


Script:

#!/bin/bash
# Let's check the gzrt source directory for the Makefile
fd Makefile recipes/gzrt/

Length of output: 27

recipes/gzrt/build.sh Outdated Show resolved Hide resolved
recipes/gzrt/build.sh Outdated Show resolved Hide resolved
recipes/gzrt/build.sh Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (1)
recipes/gzrt/meta.yaml (1)

42-44: Enhance package description.

The current description is brief. Consider adding:

  1. Usage examples
  2. Known limitations
  3. Common use cases
  4. Warning about potential data loss
   description: |
     gzrecover is a program that will attempt to extract any readable data
     out of a gzip file that has been corrupted.
+    
+    Example usage:
+      gzrecover corrupted.gz
+    
+    Note: This tool attempts best-effort recovery and may not recover all data.
+    Always work with copies of corrupted files, never the originals.
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 5dc855f and ca4b858.

📒 Files selected for processing (1)
  • recipes/gzrt/meta.yaml (1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/gzrt/meta.yaml

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

🔇 Additional comments (1)
recipes/gzrt/meta.yaml (1)

1-6: Verify package maintenance status.

The package version (0.8) is from 2013-10-03, which is quite old. Consider:

  1. Checking if there are any actively maintained forks that could be used instead
  2. Verifying if the package is still functional with modern systems
  3. Adding a note in the description about the package's maintenance status
✅ Verification successful

Let me gather more information about the package and its alternatives.


Let me check if there are any modern alternatives or similar tools in the ecosystem.


Package is functional but include maintenance status notice

While the package is old (last release 2013), our verification shows:

  1. The source URL is still accessible (HTTP 200)
  2. The GitHub repository exists but is archived
  3. No actively maintained alternatives found in conda-forge
  4. The package appears to be the only solution for recovering corrupted gzip files

Recommendations:

  • Add a note in the description mentioning this is an archived project
  • Keep the package as it serves a unique purpose and is still functional
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for potential forks and activity
# Test: Look for forks with recent activity
gh repo view arenn/gzrt --json forks,updatedAt 2>/dev/null || echo "Repository not found on GitHub"

# Test: Check if the source URL is still accessible
curl -sI https://www.urbanophile.com/arenn/hacking/gzrt/gzrt-0.8.tar.gz | head -n 1

Length of output: 217


Script:

#!/bin/bash
# Check if there are any similar packages in conda-forge
conda-forge-tick-feedstock-status list | grep -i gzrt || true

# Search for similar tools in conda-forge that handle corrupted gzip files
conda search -c conda-forge "*gzip*" --info 2>/dev/null | grep -A 2 "^name:" || true

# Check if the package is used as a dependency by other packages
rg -l "gzrt" --type yaml

# Check the package description and build script for any maintenance notes
cat recipes/gzrt/meta.yaml

Length of output: 1278


Script:

#!/bin/bash
# Check for similar tools in the conda-forge ecosystem
fd -e yaml -e yml . | xargs rg -l "gzip.*recover|recover.*gzip" || true

# Check if there are any other packages providing similar functionality
rg -l "corrupt.*gzip|gzip.*corrupt" --type yaml || true

# Check if the source code is still maintained in any form
curl -sI https://github.com/arenn/gzrt 2>/dev/null | head -n1 || echo "Not found"

# Verify if the binary works on modern systems by checking the build logs
fd -e log . | xargs rg "gzrecover.*test" || true

Length of output: 283

🧰 Tools
🪛 yamllint

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

recipes/gzrt/meta.yaml Outdated Show resolved Hide resolved
@mazzalab
Copy link
Contributor Author

mazzalab commented Nov 1, 2024

@BiocondaBot please add label

@BiocondaBot BiocondaBot added the please review & merge set to ask for merge label Nov 1, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (1)
recipes/gzrt/meta.yaml (1)

36-45: Add upstream_updates_url for better release tracking.

Consider adding an upstream_updates_url field to help track new releases:

 about:
   home: https://www.urbanophile.com/arenn/hacking/gzrt
   summary: "Unofficial build of the gzip Recovery Toolkit aka gzrecover"
   license: GPLv2
   license_family: GPL
   license_file: LICENSE
   description: |
     gzrecover is a program that will attempt to extract any readable data
     out of a gzip file that has been corrupted.
   dev_url: https://github.com/arenn/gzrt
+  upstream_updates_url: https://github.com/arenn/gzrt/releases
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between ca4b858 and 509b5c3.

📒 Files selected for processing (2)
  • recipes/gzrt/build.sh (1 hunks)
  • recipes/gzrt/meta.yaml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • recipes/gzrt/build.sh
🧰 Additional context used
🪛 yamllint
recipes/gzrt/meta.yaml

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

🔇 Additional comments (1)
recipes/gzrt/meta.yaml (1)

1-6: Verify if version 0.8 is the latest stable release.

The comment indicates this release is from 2013-10-03 (10+ years old). Please verify:

  1. If there are any newer releases available
  2. If this version is still maintained and secure
✅ Verification successful

Version 0.8 appears to be the latest official release

The meta.yaml shows:

While the software is from 2013, it's a stable utility with a specific purpose (gzip recovery) that doesn't require frequent updates. The version is appropriate for packaging.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for newer releases in the git repository
gh repo view arenn/gzrt --json releases,updatedAt 2>/dev/null || \
  git ls-remote --tags https://github.com/arenn/gzrt.git

Length of output: 108


Script:

#!/bin/bash
# Try alternative approaches to find source/version information
# 1. Check if there's a homepage URL in meta.yaml
rg -l "homepage.*gzrt" recipes/gzrt/

# 2. Look for any URLs in the meta.yaml
rg -l "url:.*gzrt" recipes/gzrt/

# 3. Check full meta.yaml content for source information
cat recipes/gzrt/meta.yaml

# 4. Search for any mentions of gzrt in README or other docs
rg -l "gzrt" . --type md

Length of output: 1112

🧰 Tools
🪛 yamllint

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

recipes/gzrt/meta.yaml Outdated Show resolved Hide resolved
mazzalab and others added 2 commits November 1, 2024 18:21
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
recipes/gzrt/meta.yaml Outdated Show resolved Hide resolved
recipes/gzrt/meta.yaml Outdated Show resolved Hide resolved
@mazzalab mazzalab requested a review from bgruening November 3, 2024 00:05
@bioconda bioconda deleted a comment from coderabbitai bot Nov 3, 2024
recipes/gzrt/meta.yaml Outdated Show resolved Hide resolved
recipes/gzrt/meta.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (1)
recipes/gzrt/meta.yaml (1)

23-26: Consider adding more test commands.

While the basic help flag test is good, consider adding more test commands to verify functionality:

  • Version check
  • File existence check

Add additional test commands:

 test:
   commands:
     - gzrecover -h
+    - test -f $PREFIX/bin/gzrecover
+    - gzrecover -V || true  # Version might exit with non-zero
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 4fcd404 and 211c124.

📒 Files selected for processing (1)
  • recipes/gzrt/meta.yaml (1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/gzrt/meta.yaml

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

🔇 Additional comments (4)
recipes/gzrt/meta.yaml (4)

1-2: LGTM! Version and checksum are properly defined.

The version and checksum variables are correctly defined using Jinja2 templating. The comment about the release date is helpful for tracking package history.

🧰 Tools
🪛 yamllint

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)


8-11: LGTM! Source configuration is valid.

The source URL and checksum are properly templated.


12-14: LGTM! Build configuration is appropriate.

Build number 0 is correct for a new package.


27-36: LGTM! About section is complete and well-formatted.

The about section includes all required metadata with proper SPDX license identifier and URLs.

recipes/gzrt/meta.yaml Outdated Show resolved Hide resolved
@mazzalab mazzalab requested a review from bgruening November 3, 2024 15:52
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (3)
recipes/gzrt/meta.yaml (3)

8-11: Consider adding fallback source URLs for reliability.

The current source URL points to a personal website which might not be reliable for long-term hosting. Consider adding GitHub as a backup source:

 source:
-  url: https://www.urbanophile.com/arenn/hacking/gzrt/gzrt-{{ version }}.tar.gz
+  url:
+    - https://www.urbanophile.com/arenn/hacking/gzrt/gzrt-{{ version }}.tar.gz
+    - https://github.com/arenn/gzrt/archive/refs/tags/v{{ version }}.tar.gz
   sha256: {{ sha256 }}

16-16: Remove trailing spaces.

There are trailing spaces at the end of line 16.

🧰 Tools
🪛 yamllint

[error] 16-16: trailing spaces

(trailing-spaces)


26-29: Consider adding more test cases.

While the basic smoke test is good, consider adding more test cases to verify functionality:

  • Test with invalid arguments
  • Check version information
  • Verify error messages

Example:

 test:
   commands:
     - gzrecover -h
+    - gzrecover --version
+    - gzrecover --invalid-flag 2>&1 | grep "invalid option"
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 211c124 and 68cc06e.

📒 Files selected for processing (1)
  • recipes/gzrt/meta.yaml (1 hunks)
🧰 Additional context used
📓 Learnings (1)
recipes/gzrt/meta.yaml (1)
Learnt from: bgruening
PR: bioconda/bioconda-recipes#51859
File: recipes/gzrt/meta.yaml:15-22
Timestamp: 2024-11-03T15:44:51.006Z
Learning: In Bioconda recipes, if a dependency like `zlib` has `run_exports` defined, it's not necessary to include it explicitly in the `run` requirements in the `meta.yaml` file.
🪛 yamllint
recipes/gzrt/meta.yaml

[error] 16-16: trailing spaces

(trailing-spaces)


[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

🔇 Additional comments (1)
recipes/gzrt/meta.yaml (1)

30-39: LGTM! The about section is well-structured and complete.

All required metadata fields are present with appropriate values.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (2)
recipes/gzrt/meta.yaml (2)

26-28: Enhance test coverage.

The current test only verifies that the binary runs. Consider adding:

  1. Version check: gzrecover --version
  2. Basic functionality test with a small corrupted gzip file
 test:
   commands:
     - gzrecover -h
+    - gzrecover --version
+    # Create and test with a corrupted gzip file
+    - echo "test" | gzip > test.gz
+    - dd if=/dev/zero of=test.gz bs=1 count=1 seek=5 conv=notrunc
+    - gzrecover test.gz || [[ $? -eq 1 ]]

30-39: Add missing metadata fields.

Consider adding:

  1. extra.recipe-maintainers to specify package maintainers
  2. doc_url pointing to the package documentation
   description: |
     gzrecover is a program that will attempt to extract any readable data
     out of a gzip file that has been corrupted.
   dev_url: https://github.com/arenn/gzrt
+  doc_url: https://www.urbanophile.com/arenn/hacking/gzrt/gzrt.html
+
+extra:
+  recipe-maintainers:
+    - mazzalab
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 20c08eb and 5cf9883.

📒 Files selected for processing (1)
  • recipes/gzrt/meta.yaml (1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/gzrt/meta.yaml

[error] 16-16: trailing spaces

(trailing-spaces)


[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

🔇 Additional comments (2)
recipes/gzrt/meta.yaml (2)

14-15: Review run_exports necessity.

The run_exports section with pin_subpackage is typically used for packages that provide shared libraries. Since gzrt appears to be a simple binary tool, this pinning might be unnecessary.

#!/bin/bash
# Check if the package produces any shared libraries
rg -l "\.so\." build.sh || echo "No shared library references found"

1-2: Verify package maintenance status.

The version 0.8 is from 2013-10-03, which is quite old. Please verify:

  1. If there are any newer releases available
  2. If the package is still actively maintained
  3. If there are any known stability issues with this version
✅ Verification successful

Package version 0.8 is appropriate for use

The version 0.8 from 2013 appears to be the latest stable release of gzrt. The evidence supports this:

  • The source URL is still accessible and working
  • The package's home page and GitHub repository are properly documented
  • The package serves a specific purpose (gzip recovery) that doesn't require frequent updates
  • No newer versions were found in the repository or mentioned elsewhere
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check GitHub repo for newer releases and activity
gh repo view arenn/gzrt 2>/dev/null || echo "Repository not found or private"

Length of output: 247


Script:

#!/bin/bash
# Try to find source URL in meta.yaml to check the upstream
cat recipes/gzrt/meta.yaml

# Search for any additional information about gzrt in the repository
rg -i "gzrt" --type yaml

Length of output: 1319


Script:

#!/bin/bash
# Check if the source URL is still accessible
curl -sI "https://www.urbanophile.com/arenn/hacking/gzrt/gzrt-0.8.tar.gz" | head -n 1

# Check if there are any other versions mentioned in the codebase
rg -i "gzrt.*version" --type yaml --type make --type sh

Length of output: 259

🧰 Tools
🪛 yamllint

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (2)
recipes/gzrt/meta.yaml (2)

16-16: Remove trailing whitespace.

There is trailing whitespace on line 16.

🧰 Tools
🪛 yamllint

[error] 16-16: trailing spaces

(trailing-spaces)


27-30: Consider adding more comprehensive tests.

The current test only verifies that the help command works. Consider adding tests that:

  1. Check if the binary exists
  2. Verify version output
  3. Test basic functionality with a sample corrupted gzip file

Example:

 test:
   commands:
+    - test -x $PREFIX/bin/gzrecover
     - gzrecover -h
+    - gzrecover --version
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 5cf9883 and 6f882a8.

📒 Files selected for processing (1)
  • recipes/gzrt/meta.yaml (1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/gzrt/meta.yaml

[error] 16-16: trailing spaces

(trailing-spaces)


[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

recipes/gzrt/meta.yaml Outdated Show resolved Hide resolved
recipes/gzrt/meta.yaml Show resolved Hide resolved
recipes/gzrt/meta.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (2)
recipes/gzrt/meta.yaml (2)

14-15: Consider removing unnecessary run_exports.

The run_exports with pin_subpackage is typically used for libraries that other packages depend on. Since gzrt is a standalone tool, this configuration might be unnecessary.

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

16-16: Remove trailing whitespace.

-  
🧰 Tools
🪛 yamllint

[error] 16-16: trailing spaces

(trailing-spaces)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 6f882a8 and 1155664.

📒 Files selected for processing (1)
  • recipes/gzrt/meta.yaml (1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/gzrt/meta.yaml

[error] 16-16: trailing spaces

(trailing-spaces)


[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

🔇 Additional comments (2)
recipes/gzrt/meta.yaml (2)

30-39: LGTM! Well-structured metadata.

The about section is complete with:

  • Proper SPDX license identifier
  • License file reference
  • Clear description
  • Relevant URLs

1-2: Verify package maintenance status.

The package version (0.8) is from 2013, which is quite old. Please verify:

  1. If there are any newer versions available
  2. If the package is still maintained
  3. If any critical security patches should be applied
✅ Verification successful

Package version 0.8 is the latest stable release, but maintenance is minimal

Based on the repository analysis:

  • The latest release (0.8) from 2013 is indeed the most recent stable version
  • The last code change was in 2019, removing an executable
  • While the repository has 23 forks, the main repository shows minimal maintenance
  • No significant updates or security patches have been released since version 0.8

The package version in the recipe is correct and up-to-date with the upstream source. While the software is old, it appears to be in a stable, feature-complete state rather than abandoned, as evidenced by its continued forking and use.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if there are newer releases or commits
gh repo view arenn/gzrt --json updatedAt,pushedAt,latestRelease

Length of output: 157


Script:

#!/bin/bash
# Check commit history to understand maintenance status
gh api /repos/arenn/gzrt/commits?per_page=5 --jq '.[].commit.message'

# Check for any open issues or PRs that might indicate active maintenance
gh repo view arenn/gzrt --json openIssues,openPullRequests

# Look for any forks that might be more actively maintained
gh api /repos/arenn/gzrt/forks --jq 'length'

# Check the actual source code for version information
rg -i "version|release" -A 2 --type c

Length of output: 1560

🧰 Tools
🪛 yamllint

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

recipes/gzrt/meta.yaml Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Outside diff range and nitpick comments (5)
recipes/gzrt/meta.yaml (2)

1-2: Consider using a more descriptive comment format.

The release date comment could be more informative and follow conda-forge style:

-{% set version = "0.8" %}   # latest release on 2013-10-03
+{% set version = "0.8" %}   # [2013-10-03] Latest upstream release
🧰 Tools
🪛 yamllint

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)


35-37: Add security considerations to package description.

Since this tool handles potentially corrupted files, it would be helpful to document security considerations in the description:

   description: |
     gzrecover is a program that will attempt to extract any readable data
     out of a gzip file that has been corrupted.
+    
+    Security Note:
+    When processing corrupted files, always run gzrecover in a controlled
+    environment and verify the source of corrupted files before processing.
recipes/gzrt/build.sh (3)

28-36: Enhance build process robustness.

Consider adding a clean step and specifying make targets explicitly:

+# Clean any previous builds
+if ! make clean; then
+    echo "Clean failed, but continuing..."
+fi
+
+# Build with specific target
 if ! make; then
     echo "Build failed"
     exit 1
 fi

38-48: Add post-installation validation.

Consider adding a verification step after installation:

 if ! cp gzrecover "$PREFIX/bin/"; then
     echo "Failed to install gzrecover"
     exit 1
 fi
+
+# Verify installation
+if ! test -x "${PREFIX}/bin/gzrecover"; then
+    echo "Installation verification failed"
+    exit 1
+fi

1-4: Add cleanup handling.

Consider adding cleanup handling at the start of the script:

 #!/bin/bash
 
 # Exit on error
 set -e
+
+# Cleanup function
+cleanup() {
+    # Remove temporary directory if it exists
+    if [ -d "${SRC_DIR}/bin" ]; then
+        rm -rf "${SRC_DIR}/bin"
+    fi
+}
+
+# Set up trap
+trap cleanup EXIT
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 1155664 and ba59d0d.

📒 Files selected for processing (2)
  • recipes/gzrt/build.sh (1 hunks)
  • recipes/gzrt/meta.yaml (1 hunks)
🧰 Additional context used
🪛 Shellcheck
recipes/gzrt/build.sh

[warning] 23-23: Quote this to prevent word splitting.

(SC2046)

🪛 yamllint
recipes/gzrt/meta.yaml

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

recipes/gzrt/meta.yaml Show resolved Hide resolved
recipes/gzrt/build.sh Outdated Show resolved Hide resolved
recipes/gzrt/build.sh Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (1)
recipes/gzrt/meta.yaml (1)

25-31: Enhance test validation.

While the current tests verify command execution, consider adding validation of the recovered data:

 test:
   commands:
     - gzrecover -h
     # Create and test with a corrupted gzip file
-    - echo "test" | gzip > test.gz
+    - echo "test data" | gzip > test.gz
+    - cp test.gz test.gz.orig
     - dd if=/dev/zero of=test.gz bs=1 count=1 seek=5 conv=notrunc
     - gzrecover test.gz || [[ $? -eq 1 ]]
+    # Verify recovered data matches original
+    - zcat test.gz.orig | diff - <(zcat test.recovered) || true
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between ba59d0d and 1a3a181.

📒 Files selected for processing (2)
  • recipes/gzrt/build.sh (1 hunks)
  • recipes/gzrt/meta.yaml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • recipes/gzrt/build.sh
🧰 Additional context used
🪛 yamllint
recipes/gzrt/meta.yaml

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

🔇 Additional comments (3)
recipes/gzrt/meta.yaml (3)

17-23: LGTM! Requirements are properly configured.

The build and host requirements are correctly specified with appropriate dependencies.


33-46: LGTM! Package information is well-documented.

The about section includes all necessary metadata with clear descriptions and proper licensing information.


1-6: Verify if newer versions are available.

The comment indicates this version is from 2013. Consider checking if newer versions or forks with updates are available.

✅ Verification successful

Version 0.8 remains the latest stable release

The investigation shows:

  • The last significant code change was in October 2013 with version 0.8 release
  • While there are several forks, none show substantial development beyond the original
  • The most recent fork activity (baxterworks-build) appears to be build-related rather than feature development

The current version 0.8 is appropriate as it represents the latest stable release of this utility.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for newer versions or forks
gh repo view arenn/gzrt --json updatedAt,pushedAt,latestRelease

Length of output: 157


Script:

#!/bin/bash
# Check git history and commits to see if there are any significant code changes
gh repo view arenn/gzrt --json defaultBranchRef --jq '.defaultBranchRef.name' | \
xargs -I {} gh api -H "Accept: application/vnd.github.v3+json" \
/repos/arenn/gzrt/commits?sha={} --jq 'map({date: .commit.committer.date, message: .commit.message}) | .[:5]'

Length of output: 688


Script:

#!/bin/bash
# Check forks for potential active development
gh api -H "Accept: application/vnd.github.v3+json" \
/repos/arenn/gzrt/forks --jq 'map({full_name, updated_at, pushed_at}) | sort_by(.pushed_at) | reverse | .[:5]'

Length of output: 681

🧰 Tools
🪛 yamllint

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

@mazzalab mazzalab requested a review from jfy133 November 7, 2024 22:22
@rpetit3
Copy link
Member

rpetit3 commented Nov 8, 2024

@BiocondaBot please fetch artifacts

@BiocondaBot
Copy link
Collaborator

Package(s) built are ready for inspection:

Arch Package Zip File / Repodata CI Instructions
linux-64 gzrt-0.8-he4a0461_0.tar.bz2 linux-64.zip GitHub Actions
showYou may also use conda to install after downloading and extracting the zip file. conda install -c ./packages <package name>
osx-64 gzrt-0.8-h10309d6_0.tar.bz2 osx-64.zip GitHub Actions
showYou may also use conda to install after downloading and extracting the zip file. conda install -c ./packages <package name>

@bgruening bgruening enabled auto-merge (squash) November 10, 2024 19:36
auto-merge was automatically disabled November 10, 2024 22:47

Head branch was pushed to by a user without write access

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (1)
recipes/gzrt/meta.yaml (1)

25-31: Good test coverage! Consider adding more edge cases.

The tests cover both basic functionality and corrupted file handling. Consider adding:

  1. Test with a larger file
  2. Test with different corruption patterns
  3. Test with multiple concatenated gzip streams
 test:
   commands:
     - gzrecover -h
     # Create and test with a corrupted gzip file
     - echo "test" | gzip > test.gz
     - dd if=/dev/zero of=test.gz bs=1 count=1 seek=5 conv=notrunc
     - gzrecover test.gz || [[ $? -eq 1 ]]
+    # Test with larger file and multiple corruptions
+    - dd if=/dev/urandom bs=1M count=1 | gzip > large.gz
+    - dd if=/dev/zero of=large.gz bs=1 count=2 seek=100 conv=notrunc
+    - gzrecover large.gz || [[ $? -eq 1 ]]
+    # Test with concatenated gzip streams
+    - echo "part1" | gzip > concat.gz
+    - echo "part2" | gzip >> concat.gz
+    - dd if=/dev/zero of=concat.gz bs=1 count=1 seek=10 conv=notrunc
+    - gzrecover concat.gz || [[ $? -eq 1 ]]
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between d5f990d and 8514102.

📒 Files selected for processing (1)
  • recipes/gzrt/meta.yaml (1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/gzrt/meta.yaml

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

🔇 Additional comments (4)
recipes/gzrt/meta.yaml (4)

17-24: LGTM! Requirements are properly configured.

The build and host requirements are correctly specified with appropriate dependencies.


33-42: LGTM! Metadata is complete and well-documented.

The about section includes all necessary fields with proper formatting and SPDX license identifier.


44-46: LGTM! Maintainer is properly specified.


1-2: Verify if a newer version is available.

The package version (0.8) is from 2013, which is quite old. Consider:

  1. Checking if there are newer releases available
  2. Documenting why this specific version was chosen if it's intentional
✅ Verification successful

Version 0.8 (2013) appears to be the latest official release

The version is correctly set as this is the latest official release. Evidence:

  • The source URL points to the official website (urbanophile.com)
  • The GitHub repository (arenn/gzrt) shows no official releases/tags
  • The comment in meta.yaml accurately documents the release date
  • The package is actively maintained on GitHub with recent updates (last update: 2024-11-02)
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if newer versions exist in the GitHub repository
gh repo view arenn/gzrt --json releases,tags -q '.releases[], .tags[]' 2>/dev/null || echo "No releases/tags found"

# Check the last commit date
gh repo view arenn/gzrt --json updatedAt -q '.updatedAt' 2>/dev/null || echo "Repository not found"

Length of output: 205


Script:

#!/bin/bash
# Let's check the source URL and any version information in the recipe
cat recipes/gzrt/meta.yaml

# Also check if there's any upstream URL mentioned
rg -l "gzrt.*sourceforge|sourceforge.*gzrt" || echo "No sourceforge mentions found"

# Check if there's any homepage or documentation URL in the recipe
rg "home:|doc:" recipes/gzrt/meta.yaml || echo "No homepage/doc URLs found"

Length of output: 1385

🧰 Tools
🪛 yamllint

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

@jfy133
Copy link
Contributor

jfy133 commented Nov 11, 2024

@mazzalab confirmed to me via personal message that the compliation doesn't work without zlib being in both host/build I will merge in if tests pass

@jfy133 jfy133 merged commit b8bf321 into bioconda:master Nov 11, 2024
6 checks passed
@mazzalab mazzalab deleted the gzrt branch November 11, 2024 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
please review & merge set to ask for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants