[85928] trunk/dports/net/nmap/Portfile

[Ryan Schmidt]

On Oct 17, 2011, at 14:45, Daniel J. Luke wrote:
> On Oct 17, 2011, at 3:21 PM, Ryan Schmidt wrote:
>> On Oct 17, 2011, at 12:53, dluke at macports.org wrote:
>>> +pre-configure {
>>> +		reinplace "s|^void main(void)|int main(void)|g" ${worksrcpath}/nbase/configure
>>> +	}
>> 
>> It would be better to use a patchfile for this instead of a reinplace.
> 
> I don't see why. Note that it's an anchored regex, so it will only match instances that we know need to be changed (besides the fact that I know it only matches the two cases that are included in your patch).
> 
> With the reinplace in the portfile, I'm more likely to double-check it if/when upstream fixes the configure test (maybe that's a problem for other maintainers, since AFAIK we didn't integrate your patch that warns when reinplace doesn't match anything?)
> 
> Not that it matters, but the reinplace command in the portfile takes up significantly less HD space than the additional file (minimum file size being 4KB)


Sorry for my delay in responding.

My experience is that maintainers are *less* likely to notice a reinplace that needs changing, which is as you said because we have not integrated fixes from #15514, so when a reinplace doesn't change anything, there is no notification. On the other hand, when a patch needs to be changed, it's usually immediately apparent because applying the patch fails which causes port(1) to exit with an error.

Above and beyond implementing #15514 is the matter that even once you realize a reinplace needs to be changed, if the upstream source has changed in the lines you're trying to patch, you may not know how to update the reinplace. Unified diff files are great because they provide contextual lines above and below the lines being patched, show the lines being patched before and after the change, and even show the line numbers. When the upstream sources change, I've often found this information invaluable to figuring out how the patch needs to be rewritten.

You may know that your reinplace only changes those instances you intend, but I've more than once pointed other port authors to the fact that their reinplaces changed more than they thought they did. What if you decide to no longer maintain the port? Or what if somebody else submits an update to the port and isn't so careful to check that the reinplace still works right, especially since, without the context shown in a diff, they may not understand how the reinplace is intended to function? I've often found portfiles containing reinplaces that do nothing, and it takes a lot of effort to then go back and figure out what it was originally doing years ago when it was added, and whether that's still needed now.

Yes a patch takes up more space, but I feel that's worth it for the above advantages. My opinion on that changes if the diff becomes huge, i.e. if the same straightforward replacement is being made in hundreds of instances.

reinplace is great when you need to substitute a variable into the source, but even then in order to get the above advantages I usually recommend a patchfile that inserts a placeholder (like @PREFIX@) into the source, then a reinplace that changes the placeholder to the desired value (e.g. reinplace "s|@PREFIX@|${prefix}|g").