Conversation
📝 WalkthroughWalkthroughReworked Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Script as plugin_repackaging.sh
participant Python as Python/pip
participant UV as uv (optional)
participant FS as Filesystem
User->>Script: invoke script (with -p / -R as needed)
Script->>FS: cd $CURR_DIR || exit 1
Script->>Python: detect pip (python3 -m pip / pip / pip3)
alt pip not found
Script->>User: exit with error
else pip found
Script->>FS: check for `requirements.txt` or `pyproject.toml`
alt pyproject.toml present and requirements absent
Script->>UV: run `uv lock`/`uv export` (with --prerelease when -R) to generate requirements.txt
UV->>Script: success/fail
alt fail
Script->>User: exit with error
end
end
Script->>Python: `pip download --prefer-binary -r requirements.txt` -> `./wheels`
Python->>Script: wheels downloaded (file sizes/count)
Script->>FS: update requirements for offline, remove `^wheels/` from ignore, cleanup .bak
Script->>FS: package plugin to `OUTPUT_PACKAGE` (report success/failure and final size)
Script->>User: exit with status
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugin_repackaging.sh`:
- Around line 155-159: The script currently runs cd ${CURR_DIR} then continues
unconditionally; add an immediate failure check after attempting to change
directory so the script exits (with an error message) if cd into CURR_DIR fails,
before running chmod or invoking ${CMD_NAME} to package
${PACKAGE_NAME}-${PACKAGE_SUFFIX}; ensure you test the cd result (and quote
variables) and abort early to avoid running the package command from the wrong
working directory.
- Around line 126-132: Change the logic so we only attempt to run "uv pip
compile pyproject.toml -o requirements.txt --index-url ${PIP_MIRROR_URL}" when
requirements.txt does not already exist: if requirements.txt is missing and
pyproject.toml exists, check for command -v uv and run the compile; if uv is not
available print a clear error like "pyproject.toml present but 'uv' not
installed; please provide requirements.txt or install uv" and exit non-zero.
This prevents overwriting an existing requirements.txt and fails gracefully when
uv is required but unavailable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cab7acdd-ccc6-4f6f-9727-d9b296642076
📒 Files selected for processing (2)
dify-plugin-darwin-arm64plugin_repackaging.sh
|
@fatelei I haven't been able to test it thoroughly yet, but when there is a [tool.uv]
no-index = true
find-links = ["./wheels"]If there is no With this change, I think we can fully revert the |
ok |
|
@fatelei Anyway, I’ve made a PR to revert |
|
Published simplified alternative with pyproject.toml support: https://github.com/kurokobo/dify-plugin-offline-packager |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
plugin_repackaging.sh (3)
181-191: Remove redundant2>&1redirections.The
&>operator already redirects both stdout and stderr, making the trailing2>&1redundant.♻️ Proposed fix
- if python3 -m pip --version &> /dev/null 2>&1; then + if python3 -m pip --version &> /dev/null; then PIP_CMD="python3 -m pip" - elif command -v pip &> /dev/null && pip --version &> /dev/null 2>&1; then + elif command -v pip &> /dev/null && pip --version &> /dev/null; then PIP_CMD=pip - elif command -v pip3 &> /dev/null && pip3 --version &> /dev/null 2>&1; then + elif command -v pip3 &> /dev/null && pip3 --version &> /dev/null; then PIP_CMD=pip3🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin_repackaging.sh` around lines 181 - 191, The shell snippet setting PIP_CMD contains redundant redirections like "&> /dev/null 2>&1"; edit the if/elif conditions that check python3 -m pip --version, pip --version, and pip3 --version to remove the trailing "2>&1" so each redirect uses only "&> /dev/null", keeping the same checks and assignments for PIP_CMD and leaving the echo/exit behavior unchanged; update the three occurrences around the commands referenced (the python3 -m pip check and the pip and pip3 checks) to eliminate the duplicate stderr redirection.
238-255: Redundant pattern*manylinux*in case statement.The pattern
*linux*already matches strings containing "manylinux", making*manylinux*redundant in the alternation.♻️ Proposed fix
case "$RAW_PLATFORM" in - *linux*|*manylinux* ) + *linux* ) UV_PLATFORM="linux" echo "Target platform: Linux (cross-compilation from $OS_TYPE)" ;;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin_repackaging.sh` around lines 238 - 255, The case branch matching RAW_PLATFORM contains a redundant pattern "*manylinux*" because "*linux*" already covers it; update the case in plugin_repackaging.sh to remove "*manylinux*" from the linux alternative so the branch reads only "*linux*" (keep setting UV_PLATFORM="linux" and the echo as-is) and ensure no other branches rely on the removed pattern.
155-179: Consider checking awk/mv exit status before confirming success.The success message on line 178 prints regardless of whether the awk or mv commands actually succeeded. If either fails, the message is misleading.
♻️ Proposed fix
- ' "$PYFILE" > "$PYFILE.tmp" && mv "$PYFILE.tmp" "$PYFILE" - echo "Injected [tool.uv] into $PYFILE" + ' "$PYFILE" > "$PYFILE.tmp" && mv "$PYFILE.tmp" "$PYFILE" || { + echo "✗ Warning: Failed to inject [tool.uv] into $PYFILE" + return 1 + } + echo "✓ Injected [tool.uv] into $PYFILE"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin_repackaging.sh` around lines 155 - 179, The success message is printed unconditionally in inject_uv_into_pyproject even if the awk or mv commands fail; update the function to detect and handle failures: run the awk pipeline writing to "$PYFILE.tmp" and capture its exit status, only attempt mv when awk succeeded, check mv's exit status too, remove the temporary file on failure (or use a safe mktemp name) and only echo "Injected [tool.uv] into $PYFILE" when both commands succeeded; reference the inject_uv_into_pyproject function, the awk invocation that writes to "$PYFILE.tmp", and the mv "$PYFILE.tmp" "$PYFILE" operation when implementing these checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugin_repackaging.sh`:
- Around line 367-369: Update the step header string to reflect the correct
sequence: change the echo that prints "Step 4: Packaging plugin" to "Step 5:
Packaging plugin" so the header matches the actual workflow order; locate the
echo block containing the exact string "Step 4: Packaging plugin" (the
three-line echo block) and update that string only, leaving the surrounding
separators and other echo statements unchanged.
---
Nitpick comments:
In `@plugin_repackaging.sh`:
- Around line 181-191: The shell snippet setting PIP_CMD contains redundant
redirections like "&> /dev/null 2>&1"; edit the if/elif conditions that check
python3 -m pip --version, pip --version, and pip3 --version to remove the
trailing "2>&1" so each redirect uses only "&> /dev/null", keeping the same
checks and assignments for PIP_CMD and leaving the echo/exit behavior unchanged;
update the three occurrences around the commands referenced (the python3 -m pip
check and the pip and pip3 checks) to eliminate the duplicate stderr
redirection.
- Around line 238-255: The case branch matching RAW_PLATFORM contains a
redundant pattern "*manylinux*" because "*linux*" already covers it; update the
case in plugin_repackaging.sh to remove "*manylinux*" from the linux alternative
so the branch reads only "*linux*" (keep setting UV_PLATFORM="linux" and the
echo as-is) and ensure no other branches rely on the removed pattern.
- Around line 155-179: The success message is printed unconditionally in
inject_uv_into_pyproject even if the awk or mv commands fail; update the
function to detect and handle failures: run the awk pipeline writing to
"$PYFILE.tmp" and capture its exit status, only attempt mv when awk succeeded,
check mv's exit status too, remove the temporary file on failure (or use a safe
mktemp name) and only echo "Injected [tool.uv] into $PYFILE" when both commands
succeeded; reference the inject_uv_into_pyproject function, the awk invocation
that writes to "$PYFILE.tmp", and the mv "$PYFILE.tmp" "$PYFILE" operation when
implementing these checks.
2 participants
fix #61
Summary by CodeRabbit
Bug Fixes
New Features