Skip to content

Fix tox error#178

Merged
bjk7119 merged 1 commit intomainfrom
fixtox
Mar 11, 2026
Merged

Fix tox error#178
bjk7119 merged 1 commit intomainfrom
fixtox

Conversation

@bjk7119
Copy link
Contributor

@bjk7119 bjk7119 commented Mar 9, 2026

Description

Fix PyInstaller build failure caused by chardet mypyc

  • chardet 5.0.0+ uses mypyc to compile code with dynamic module names
  • PyInstaller cannot detect these dynamic modules during build
  • Pin chardet<5.0.0 to use pure Python implementation
  • Explicitly collect binaryornot package data in PyInstaller hook

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Documentation update
  • Refactoring, Maintenance
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Summary by CodeRabbit

  • Chores
    • Packaging revamped to a spec-driven process for more reliable CLI builds.
    • Packaged artifacts now include additional dependency data, templates and license files for completeness.
    • CI Windows build updated to use the spec-driven packaging approach.
    • Test configuration updated to exclude build/dist and related directories from test discovery.

@bjk7119 bjk7119 requested a review from dd-jy March 9, 2026 06:15
@bjk7119 bjk7119 self-assigned this Mar 9, 2026
@bjk7119 bjk7119 added the chore [PR/Issue] Refactoring, maintenance the code label Mar 9, 2026
@coderabbitai
Copy link

coderabbitai bot commented Mar 9, 2026

📝 Walkthrough

Walkthrough

Added data collection for chardet and binaryornot in the PyInstaller hook, introduced a new cli.spec packaging spec (including reuse/fosslight_util data, templates, and licenses), switched CI Windows PyInstaller steps to use the spec, and excluded common build/cache dirs from pytest discovery in tox.ini.

Changes

Cohort / File(s) Summary
PyInstaller Hook
hooks/hook-fosslight_prechecker.py
Added datas_*, binaries_*, and hiddenimports_* for chardet and binaryornot via collect_all(...) and merged them into the module-level datas, binaries, and hiddenimports.
PyInstaller Spec
cli.spec
New spec file: collects data/binaries/hiddenimports for chardet and binaryornot, includes data from reuse and fosslight_util, Jinja2 metadata, a template src/.../default_template.jinja2, and license files; builds cli EXE.
CI Workflows
.github/workflows/publish-release.yml, .github/workflows/pull-request.yml
Replaced direct PyInstaller CLI invocation with pyinstaller cli.spec for Windows build steps; retained artifact rename/move to fosslight_prechecker_windows.exe.
Test Configuration
tox.ini
Under [pytest], added norecursedirs = build dist .tox .git __pycache__ to exclude build/cache directories from test discovery.
Packaging Metadata
.reuse/dep5
Added cli.spec entry with copyright (2026 LG Electronics) and license GPL-3.0-only.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Fix tox error' is vague and does not clearly describe the actual changes made in the pull request, which include modifications to tox.ini, PyInstaller hooks, spec files, and CI workflows. Consider a more descriptive title that reflects the actual scope of changes, such as 'Add PyInstaller spec file and update CI workflows for Windows builds' or 'Refactor PyInstaller configuration and dependencies handling'.
✅ Passed checks (2 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fixtox

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@bjk7119 bjk7119 force-pushed the fixtox branch 3 times, most recently from 25db87d to 3d536bb Compare March 9, 2026 06:47
Copy link

@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.

🧹 Nitpick comments (1)
cli.spec (1)

11-17: Keep the extra collect_all() packages in one place.

binaryornot and chardet are collected here and again in hooks/hook-fosslight_prechecker.py. That duplication is easy to let drift and can add the same bundle entries twice. I’d keep one source of truth for these extra packages and let the other layer rely on it.

Also applies to: 25-33

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli.spec` around lines 11 - 17, The duplicates of collect_all('chardet') and
collect_all('binaryornot') in cli.spec and hooks/hook-fosslight_prechecker.py
should be collapsed to a single source of truth: remove the collect_all calls
and added datas (datas_chardet, datas_binaryornot) from cli.spec and centralize
them in one module (e.g., keep them only in hooks/hook-fosslight_prechecker.py),
then update the other location to consume the centralized exports (or import a
helper that returns the combined datas) so that only one place performs
collect_all and the other relies on that shared result; ensure references to
datas_chardet, datas_binaryornot, binaries_chardet, binaries_binaryornot, and
hiddenimports_* are updated accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@cli.spec`:
- Around line 11-17: The duplicates of collect_all('chardet') and
collect_all('binaryornot') in cli.spec and hooks/hook-fosslight_prechecker.py
should be collapsed to a single source of truth: remove the collect_all calls
and added datas (datas_chardet, datas_binaryornot) from cli.spec and centralize
them in one module (e.g., keep them only in hooks/hook-fosslight_prechecker.py),
then update the other location to consume the centralized exports (or import a
helper that returns the combined datas) so that only one place performs
collect_all and the other relies on that shared result; ensure references to
datas_chardet, datas_binaryornot, binaries_chardet, binaries_binaryornot, and
hiddenimports_* are updated accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9ef3fb68-5908-4eeb-86f2-95aeb1b8f796

📥 Commits

Reviewing files that changed from the base of the PR and between 5278e6f and 3d536bb.

📒 Files selected for processing (6)
  • .github/workflows/publish-release.yml
  • .github/workflows/pull-request.yml
  • .reuse/dep5
  • cli.spec
  • hooks/hook-fosslight_prechecker.py
  • tox.ini
🚧 Files skipped from review as they are similar to previous changes (1)
  • tox.ini

@bjk7119 bjk7119 merged commit b056754 into main Mar 11, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore [PR/Issue] Refactoring, maintenance the code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants