Page MenuHomeFreeBSD

fix-shebang: add support for install-time fixes (not just patch-time)
AbandonedPublic

Authored by rfyu28uyeg_snkmail.com on Apr 30 2017, 9:45 PM.

Details

Reviewers
None
Group Reviewers
portmgr
Summary

With the recent more strict checking of shebang, it's more obvious that there are a number of ports where fixing shebangs at patch time is not sufficient. One example is print/tex-xetex which installs files directly from a distribution tarball during the 'stage' phase.

Attached is an update to shebangfix.mk that supports fixing shebangs during the 'stage' phase.

The gating knob to control it is to define SHEBANG_FILES_INSTALL (relative to ${STAGEDIR}${PREFIX} by default) instead of (or in addition to) SHEBANG_FILES.

It also now supports SHEBANG_FILES_PATCH to be more explicit. We could deprecate SHEBANG_FILES, but for now it can be a separate task to replace them.

I wasn't clear whether to name the new variables as *_INSTALL or *_STAGE. Feedback welcome. I opted for _INSTALL since it's generally recommended to use things like post-install rather than post-stage in ports Makefiles.

The premise for the update is pretty simple. Use the same logic at patch time and stage time via .USE. Operate over a different list of files and in a different directory (working directory vs. staging directory).

See also https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=218979

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

Added updates to three ports affected by the prospective changes to shebangfix.mk

miwi added a subscriber: miwi.May 4 2017, 7:39 PM
mat added a comment.May 4 2017, 10:56 PM

This makes the shebangfix.mk code a lot more complicated.

It would be better to simply add three variables, SHEBANG_FILES_INSTALL, SHEBANG_GLOB_INSTALL and SHEBANG_REGEX_INSTALL, create a new target that gets hooked up in the stage phase and enable one or two of the targets depending on which variables are set.

mat added a comment.EditedMay 4 2017, 10:58 PM

As a side note, if you need to "patch" shebangs after the build process has run, you are doing it wrong, you should provide the correct path, or patch the build process so that the correct shebangs are generated.

mat added a comment.May 4 2017, 11:07 PM

And if all this is only to fix the couple of ports that extract their distfiles directly in STAGEDIR during install, I think those should be fixed instead, not the framework.

bapt added a subscriber: bapt.May 4 2017, 11:10 PM

I tend to agree with @mat on this

In D10554#219966, @mat wrote:

As a side note, if you need to "patch" shebangs after the build process has run, you are doing it wrong, you should provide the correct path, or patch the build process so that the correct shebangs are generated.

Patching at patch time is impossible with certain ports. As mentioned in the original description, for instance, print/tex-xetex installs directly to stage/ from a distribution tarball.

I agree patching earlier (in WRKDIR) is preferable.

That said, one could install from dist tarball to WRKDIR, then add an additional step to copy from WRKDIR to STAGEDIR. That adds complication to the port and twice the storage needs (and ensuing extra I/O time). I guess I'd prefer the ability to do some in-place shebang fixes in STAGEDIR.

In D10554#219965, @mat wrote:

This makes the shebangfix.mk code a lot more complicated.

It doesn't change the guts of the shebangfix.mk code at all (hence the .USE) - just facilitates a way to provide a different file list and a different directory.

It would be better to simply add three variables, SHEBANG_FILES_INSTALL, SHEBANG_GLOB_INSTALL and SHEBANG_REGEX_INSTALL, create a new target that gets hooked up in the stage phase and enable one or two of the targets depending on which variables are set.

That (a new target that is hooked up in the stage phase) is exactly what this patch does. Maybe you're saying that you think it would be better to duplicate fix-shebang via copy/paste rather than leverage .USE?

In D10554#219970, @mat wrote:

And if all this is only to fix the couple of ports that extract their distfiles directly in STAGEDIR during install, I think those should be fixed instead, not the framework.

I think the fix for print/tex-xetex would be (as I mentioned a couple comments up) to add an extra step to extract the distribution tarball to WRKDIR, then shebangfix, then copy from WRKDIR to STAGEDIR. The tarball is currently:

du -h /usr/ports/distfiles/TeX/texlive-20150523-texmf.tar.xz
1.8G /usr/ports/distfiles/TeX/texlive-20150523-texmf.tar.xz

Uncompressed that is 3.9G.
Tar extraction takes 3-4 minutes on a moderate machine.
So the extra copy takes another 3.9G on top of that and roughly a similar amount of time as the extract to copy (and a little extra time for the extra removal during clean).

If that is preferred over shebangfix in-place changes in STAGEDIR and a reduced footprint, I'll work up a patch for print/tex-xetex to do that and drop the shebangfix.mk patch.

mat added a comment.May 5 2017, 10:11 AM

If that is preferred over shebangfix in-place changes in STAGEDIR and a reduced footprint, I'll work up a patch for print/tex-xetex to do that and drop the shebangfix.mk patch.

If this patch was to fix hundreds of ports, I would say why not. The only reason a couple of tex ports do nothing but directly extract their distfile in STAGEDIR without going through the normal process is because they are just data files that need to be repackaged. I think the best course of action is to do the patching in place in STAGEDIR after extracting the files.

Abandoning this change (probably not applicable to enough ports).

See https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=218979