textproc/iso8879: Fix build failure by using USES=zip - #610
Conversation
Modernize the port by using USES=zip which utilizes the native unzip command. This fixes a build failure where /usr/local/bin/unzip was missing during the fake stage because it was only listed in BUILD_DEPENDS and the port used NO_BUILD=yes. Also simplify the do-install target by using standard INSTALL_DATA and removing redundant pre-install target. AI-Assisted-by: Gemini CLI <[email protected]> Signed-off-by: Lucas Holt <[email protected]>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideModernizes the iso8879 port to use the standard USES=zip framework instead of a manual unzip dependency and extraction, and simplifies installation by using standard INSTALL_DATA semantics and cleaning up redundant targets and plist substitutions. Flow diagram for iso8879 port build with USES=zipflowchart TD
A["ports framework: fetch distfile isoENTS.zip"] --> B["USES=zip: run native unzip via UNZIP_CMD"]
B --> C["extract to WRKSRC (NO_WRKSUBDIR=yes)"]
C --> D["do-install: MKDIR INSTDIR"]
D --> E["do-install: INSTALL_DATA ISO* to INSTDIR"]
E --> F["do-install: INSTALL_DATA catalog to INSTDIR/catalog"]
F --> G["package plist uses PLIST_SUB with DTD_NAME and CATALOG_PORTS_SGML"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
Code Review
This pull request refactors the textproc/iso8879 Makefile by adopting the USES= zip framework, simplifying dependencies, and cleaning up the installation process. However, the do-install target installs files directly to ${INSTDIR} instead of utilizing the staging directory ${STAGEDIR}. This violates staging requirements and will lead to failures during the staging or packaging phase.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| ${MKDIR} ${INSTDIR} | ||
| ${INSTALL_DATA} ${WRKSRC}/ISO* ${INSTDIR} | ||
| ${INSTALL_DATA} ${FILESDIR}/catalog ${INSTDIR}/catalog |
There was a problem hiding this comment.
In MidnightBSD mports, all files must be installed into the stage directory using ${STAGEDIR} during the do-install target. Installing directly to ${INSTDIR} (which points to ${PREFIX}) will fail during the fake/stage phase or attempt to write directly to the host system.
${MKDIR} ${STAGEDIR}${INSTDIR}
${INSTALL_DATA} ${WRKSRC}/ISO* ${STAGEDIR}${INSTDIR}
${INSTALL_DATA} ${FILESDIR}/catalog ${STAGEDIR}${INSTDIR}/catalog
There was a problem hiding this comment.
In mports, FAKE_DESTDIR is used instead of STAGEDIR and it's not needed in do-install target on mports as PREFIX includes FAKE_DESTDIR
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In the do-install target, consider replacing the
${WRKSRC}/ISO*wildcard with an explicit list of files to install to avoid unintentionally picking up unexpected files if the distfile contents change.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the do-install target, consider replacing the `${WRKSRC}/ISO*` wildcard with an explicit list of files to install to avoid unintentionally picking up unexpected files if the distfile contents change.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Modernize the port by using USES=zip which utilizes the native unzip command. This fixes a build failure where /usr/local/bin/unzip was missing during the fake stage because it was only listed in BUILD_DEPENDS and the port used NO_BUILD=yes.
Also simplify the do-install target by using standard INSTALL_DATA and removing redundant pre-install target.
AI-Assisted-by: Gemini CLI [email protected]
Summary by Sourcery
Modernize the iso8879 port to use the standard zip framework and simplify installation while fixing a missing unzip build-time dependency.
Bug Fixes:
Enhancements: