Bug 219780

Summary: [PATCH] USE_PACKAGE_DEPENDS broken if PACKAGES' definition contains colons
Product: Ports & Packages Reporter: Harald Schmalzbauer <bugzilla.freebsd>
Component: Ports FrameworkAssignee: Port Management Team <portmgr>
Status: New ---    
Severity: Affects Some People CC: pizzamig, ports-bugs
Priority: --- Keywords: patch
Version: Latest   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
Fix PKGFILE in case PACKAGES was defined with path name containing a colon none

Description Harald Schmalzbauer 2017-06-04 16:24:41 UTC
Like reported in https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=218827 changes made in r438058 (followed by r438901 and r441712) break PACKAGES usage.
Since latest fix in r441712, package creation is restored, but USE_PACKAGE_DEPENDS is still broken (hadn't noticed the old report was closed, so reporting here as new issue).

Example showing the problem with PACKAGES' definition getting redefined (r441712, escaping colons):
make -DUSE_PACKAGE_DEPENDS_ONLY
===>  Staging for OmniPACK-systools-0.10.2
===>   OmniPACK-systools-0.10.2 depends on executable: cpdup - found
===>   OmniPACK-systools-0.10.2 depends on executable: lz4c - found
===>   OmniPACK-systools-0.10.2 depends on executable: sshguard - found
===>   OmniPACK-systools-0.10.2 depends on executable: tmux - found
===>   OmniPACK-systools-0.10.2 depends on executable: vim - not found
===>   OmniPACK-systools-0.10.2 depends on package: /mnt/pkg/ivybridge/FreeBSD\:11\:amd64/All/vim-lite-8.0.0604.txz - not found
Comment 1 Harald Schmalzbauer 2017-08-29 11:47:21 UTC
Just for documentation:

Index: ports/Mk/Scripts/do-depends.sh
===================================================================
--- ports/Mk/Scripts/do-depends.sh      (Revision 448848)
+++ ports/Mk/Scripts/do-depends.sh      (Arbeitskopie)
@@ -31,6 +31,11 @@
            PKGFILE pkgfile \
            PKGBASE pkgbase
 
+       # It's too much effort handling PKGFILE in port_var_fetch(), so work arround
+       # PACKAGES fallout from r438058 by filtering induvidually – error prone since
+       # somebody would have to check scripts... Just fix this one here
+       pkgfile=$(echo "${pkgfile}" | tr -d '\')
+
        if [ -r "${pkgfile}" -a "${target}" = "${dp_DEPENDS_TARGET}" ]; then
                echo "===>   Installing existing package ${pkgfile}"
                if [ "${pkgbase}" = "pkg" ]; then


Hard to believe USE_PACKAGE_DEPENDS is used that rarely.
More astonishing is to see 218827 beeing closed while this stayed unresolved for that time...
This is just quick'n'dirty. Documented here in case anyone needs a quick solution.
I haven't had a look at r438901 and successors at all. I guess someone with more ports infrastructure knowledge needs to review that. And intervene if things get damaged without repair...

-harry
Comment 2 Harald Schmalzbauer 2017-08-29 17:33:37 UTC
While the former does fix USE_PACKAGE_DEPENDS, the real problem with https://svnweb.freebsd.org/ports/head/Mk/bsd.port.mk?view=log&pathrev=441712 persists and breaks several scripts here.

So please don't consider the proposal in the former comment as a possible fix!

Since I'm not planning to rewrite my scripts and in my opinion, it's simply wrong to define PKGFILE etc, with elsewhere unadequate escape characters, I propose this patch:

Index: bsd.port.mk
===================================================================
--- bsd.port.mk (Revision 448848)
+++ bsd.port.mk (Arbeitskopie)
@@ -2518,7 +2518,6 @@
 PKGREPOSITORYSUBDIR?=  All
 PKGREPOSITORY?=                ${PACKAGES}/${PKGREPOSITORYSUBDIR}
 .if exists(${PACKAGES})
-PACKAGES:=     ${PACKAGES:S/:/\:/g}
 _HAVE_PACKAGES=        yes
 PKGFILE?=              ${PKGREPOSITORY}/${PKGNAME}${PKG_SUFX}
 .else
@@ -3302,19 +3301,19 @@
 # Package
 
 .if defined(_HAVE_PACKAGES)
-_EXTRA_PACKAGE_TARGET_DEP+= ${PKGFILE}
-_PORTS_DIRECTORIES+=   ${PKGREPOSITORY}
+_EXTRA_PACKAGE_TARGET_DEP+= ${PKGFILE:S/:/\:/g}
+_PORTS_DIRECTORIES+=   ${PKGREPOSITORY:S/:/\:/g}
 
-${PKGFILE}: ${WRKDIR_PKGFILE} ${PKGREPOSITORY}
+${PKGFILE:S/:/\:/g}: ${WRKDIR_PKGFILE:S/:/\:/g} ${PKGREPOSITORY:S/:/\:/g}
        @${LN} -f ${WRKDIR_PKGFILE} ${PKGFILE} 2>/dev/null \
                        || ${CP} -f ${WRKDIR_PKGFILE} ${PKGFILE}
 
 .  if ${PKGORIGIN} == "ports-mgmt/pkg" || ${PKGORIGIN} == "ports-mgmt/pkg-devel"
-_EXTRA_PACKAGE_TARGET_DEP+=    ${PKGLATESTREPOSITORY}
-_PORTS_DIRECTORIES+=   ${PKGLATESTREPOSITORY}
-_EXTRA_PACKAGE_TARGET_DEP+=    ${PKGLATESTFILE}
+_EXTRA_PACKAGE_TARGET_DEP+=    ${PKGLATESTREPOSITORY:S/:/\:/g}
+_PORTS_DIRECTORIES+=   "${PKGLATESTREPOSITORY:S/:/\:/g}"
+_EXTRA_PACKAGE_TARGET_DEP+=    ${PKGLATESTFILE:S/:/\:/g}
 
-${PKGLATESTFILE}: ${PKGFILE} ${PKGLATESTREPOSITORY}
+${PKGLATESTFILE:S/:/\:/g}: ${PKGFILE:S/:/\:/g} ${PKGLATESTREPOSITORY:S/:/\:/g}
        ${INSTALL} -l rs ${PKGFILE} ${PKGLATESTFILE}
 .  endif

-harry
Comment 3 Harald Schmalzbauer 2018-05-13 09:24:50 UTC
Hello,

I found USE_PACKAGE_DEPENDS is still broken, if one sets PACKAGES to the file:/// repository path.
The last patch still applies and fixes that.

-harry
Comment 4 Harald Schmalzbauer 2021-02-20 13:44:47 UTC
Created attachment 222671 [details]
Fix PKGFILE in case PACKAGES was defined with path name containing a colon

Mk/Scripts/do-depends.sh sources PKGFILE, which results in a wrong filename since I opend this problem report.

Hope it helps providing a diff as attachment; USE_PACKAGE_DEPENDS is broken for 2 and a half years now if you define PACKAGES with a path name containing the colon (:) - which obviously isn't a bad idea to follow pkg(8)'s ABI convention!
Comment 5 Graham Perrin freebsd_committer freebsd_triage 2022-10-17 12:38:01 UTC
Keyword: 

    patch
or  patch-ready

– in lieu of summary line prefix: 

    [patch]

* bulk change for the keyword
* summary lines may be edited manually (not in bulk). 

Keyword descriptions and search interface: 

    <https://bugs.freebsd.org/bugzilla/describekeywords.cgi>
Comment 6 Luca Pizzamiglio freebsd_committer freebsd_triage 2022-10-24 21:51:45 UTC
Hi. I've checked with portmgr and fixing the support for directory names with colon seems not the way to go.

This is a limitation coming with `make` and fixing this behavior, while possible, introduces quite a lot of complexity in the framework to fix a corner case.

Specifically, with PACKAGES, an easy workaraound could be to have a symlink to the folder without colons.