REINPLACE_CMD QA causing pkg-fallout

[Michael Gmelin]

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

-- 
Michael Gmelin