Date: Mon, 4 Jan 2021 18:56:31 +0100 From: Michael Gmelin <freebsd@grem.de> To: "freebsd-ports@freebsd.org" <freebsd-ports@freebsd.org> Cc: Steve Wills <swills@FreeBSD.org> Subject: Re: REINPLACE_CMD QA causing pkg-fallout Message-ID: <20210104185631.19742845@bsd64.grem.de> In-Reply-To: <20210102183312.2d39afa1@bsd64.grem.de> References: <20210102183312.2d39afa1@bsd64.grem.de>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, 2 Jan 2021 18:33:12 +0100
Michael Gmelin <freebsd@grem.de> wrote:
> Hi,
>
> I updated the devel/phabricator port yesterday, which included running
> the usual QA steps (`poudriere testport', also running "make
> check-plist" on a local ports tree).
>
> Later, after committing the change and when updating on a production
> machine, I noticed a problem with the package list, which I first
> corrected in-place and then committed the corrections. Today I started
> receiving pkg-fallout messages - seems like timing was quite
> unfortunate. Those messages motivated me to look into the issue
> and I figured out why it failed:
>
> It turns out that if DEVELOPER is set, it worked fine (which is
> also what is set when `poudriere testport' is run). Without DEVELOPER
> set (e.g., in `poudriere bulk') it failed.
>
> The reason is that this port sets REINPLACE_ARGS to override the "-i"
> argument to $REINPLACE_CMD like this:
>
> REINPLACE_ARGS= -i ""
>
> which seems to be the intended use of this variable, according to
> bsd.port.mk:
>
> # Macro for doing in-place file editing using regexps.
> # REINPLACE_ARGS may only be used to set or override the -i
> # argument. Any other use is considered invalid.
>
> Now, since r522484[0], reviewed in [1], REINPLACE_CMD is set to
> sed_checked.sh (which now is ${SCRIPTSDIR}/sed_checked.sh), which
> hardcodes the call to `/usr/bin/sed -i.bak "$@"'.
>
> Therefore, this is what made me create the broken pkg-plist:
> - I had "DEVELOPER=yes" in /etc/make.conf.
> - I used `make makeplist' to create a new pkg-list.
> - The new pkg-plist contained .bak files created by sed_checked.sh
> without me noticing.
> - `make stage-qa' and `poudriere testport' worked just fine, as the
> .bak files are created there as well.
> - Once building without "DEVELOPER" set (production server with ports
> tree, `poudriere bulk', project build servers), it failed, as
> sed_checked.sh isn't involved there, so no .bak files are created,
> as REINPLACE_ARGS is applied.
>
> To keep supporting overriding "-i" and making sure that DEVELOPER
> builds are the same as non-DEVELOPER builds, I would suggest to modify
> sed_checked.sh like this:
>
> - Change the call to sed: /usr/bin/sed -i.sedcheck "$@"
> - Move the backup file back to the original file after checking:
> mv "$@".sedcheck "$@"
> - Run sed again, this time passing $REINPLACE_ARGS
>
> Maybe @swills has time to look into this, if not, I'll try to find the
> time to come up with a patch and open a review.
>
> Cheers,
> Michael
>
> [0] https://svnweb.freebsd.org/ports?view=revision&revision=522484
> [1] https://reviews.freebsd.org/D22174
>
I implemented a simple fix and opened a review:
https://lists.freebsd.org/pipermail/freebsd-ports/2021-January/119978.html
-m
--
Michael Gmelin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20210104185631.19742845>