reinplace should warn if nothing got replaced

[ryandesign@…]

What's always bugged me about reinplace is that you get no notification if nothing got replaced, which will probably bite you later. If you use a patchfile to modify a file, and the patch is out of date, the patch fails to apply and the port command exits with an error. This is good; it alerts the port author to the situation and they can fix it by changing or removing the patch. But with reinplace, if the underlying file you're modifying has changed such that the reinplace no longer causes anything to change, you get no warning and the port command proceeds.

I think the correct behavior might ultimately be to error out entirely if a reinplace doesn't change a file, just like we do with failed patches. But as a first step, to get port authors used to this change, it might be friendlier to just print a warning for now.

The attached patch adds a warning if a reinplace doesn't change the underlying file.

[jkh@…]

This behavior should be made conditional. There are legitimate usage cases where a replacement might "fail", and before deciding whether this new behavior should be the default or not it should also be tested out in the ports tree to see how many ports actually break with the new behavior.

[ryandesign@…]

I agree it should be tested. I've been meaning to reinstall MacPorts in a different prefix, so I may take this opportunity to do that and reinstall all the ports I use and see if my new warning appears anywhere.

[afb@…]

Issuing a warning shouldn't break the port build, even if it could be annoying if wrong.

[ryandesign@…]

Replying to afb@macports.org:

Issuing a warning shouldn't break the port build, even if it could be annoying if wrong.

Exactly.

So far in my limited tests the warning has been triggered in aquaterm, gd2, libsdl, perl5.8 and perl5.10.

[raimue@…]

Not sure if it is faster or better than diff -q, but cmp -s $file $tmpfile returns as exit value if two files differ, without any output on stdout (getting rid of >/dev/null).

[ryandesign@…]

Replying to jkh@apple.com:

This behavior should be made conditional. There are legitimate usage cases where a replacement might "fail", and before deciding whether this new behavior should be the default or not it should also be tested out in the ports tree to see how many ports actually break with the new behavior.

I've found one such possibly legitimate use case in the mysql5 port, which does a reinplace on every manual page and configuration file, not all of which contain the string to be replaced. This causes many warnings to appear.

    # Fix paths in manpages and sample configuration files
    foreach manpage [glob -type f ${destroot}${prefix}/share/man/man\[1-9\]/*] {
        reinplace "s|/etc/my.cnf|${sysconfdir}/my.cnf|g" ${manpage}
    }
    foreach samp_conffile [glob -type f ${destroot}${prefix}/share/${mysql}/mysql/my-*.cnf] {
        reinplace "s|/etc/my.cnf|${sysconfdir}/my.cnf|g" ${samp_conffile}
    }

Replying to raimue@macports.org:

Not sure if it is faster or better than diff -q, but cmp -s $file $tmpfile returns as exit value if two files differ, without any output on stdout (getting rid of >/dev/null).

Thank you, Rainer, I didn't know cmp. That's surely better. I'll attach a new patch.

[ryandesign@…]

This patch helped me discover that eclipse-ecj32 was broken; see r38136.

[raimue@…]

Added a -f option

[raimue@…]

I added reinplace-warning3.diff which includes a -f option to force the replacement and ignore the warning in cases where this is needed.

Now we need to decide if we want an option to ignore the warning or print it always.

[ryandesign@…]

change the option to -q for "quiet"

[ryandesign@…]

Maybe there should still be some indication in debug mode that a reinplace didn't do anything, even if the quiet switch is given. What do you think?

[ryandesign@…]

show warning in debug mode even if quiet switch is used

[ryandesign@…]

When this ticket is resolved, fix #16384 too.

[ryandesign@…]

refreshed patch

[takeshi@…]

Cc Me!

[takeshi@…]

reinplace-warning7.diff is the difference from base_2_1_2.