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, [email protected] 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").
_______________________________________________
macports-dev mailing list
[email protected]
http://lists.macosforge.org/mailman/listinfo.cgi/macports-dev
