Opened 3 years ago

Last modified 3 years ago

#62748 new enhancement

reinplace -q: Check if replacement is needed before replacing

Reported by: ryandesign (Ryan Carsten Schmidt) Owned by:
Priority: Normal Milestone:
Component: ports Version: 2.6.4
Keywords: Cc:
Port:

Description

Some ports run reinplace over a large set of files, using fs-traverse, when not all of the files need the replacement. They use the reinplace -q flag to silence the warning that would otherwise be printed when no replacement occurs, but it was pointed out in [63bac4a94759958edb1a995907e1ce7cc599c5d8/macports-ports] that performing a no-op replacement still takes some time which can add up when there are many files to process: in that commit, it is claimed that the patch phase runs 15 times faster by just checking first (with grep) whether the file needs to the replacement before replacing it.

I propose enhancing reinplace so that it automatically performs this check (ideally using native Tcl code, not executing grep) when reinplace is called using the -q flag.

The slowness of performing no-op replacements probably also partly relates to the amount of logging that reinplace performs, regardless of whether a replacement happened.

Change History (2)

comment:1 Changed 3 years ago by tsabirgaliev (Tair Sabirgaliev)

@ryandesign, BTW, I see that reinplace doesn't use sed -i, that is "edit in place". Is there a particular reason for that?

comment:2 Changed 3 years ago by ryandesign (Ryan Carsten Schmidt)

The reason I can think of off the top of my head is so that we can determine afterward if the file got changed or not. But I think we were using this method before we were checking whether anything got changed, so there may be another reason as well. I haven't looked through the whole commit history of the reinplace procedure to find out. The commit that added the reinplace procedure doesn't mention why it uses a temp file rather than -i. We could ask Landon but it was 18 years ago so he may not remember.

One possible reason might be that I have never found a portable way to specify that no backup file should be created, and perhaps it was desired for the code to be portable to Linux systems that tend to use GNU sed even if the primary audience is Mac users who have BSD sed. For BSD sed, it appears that the arguments must be specified as -i "" (with a space between the flag and the empty backup filename extension) and for GNU sed, it appears that it must be -i"" (with no space between). Granted this difference is something that we could interrogate at configure time and write the procedure to use the configuration result accordingly.

Or we could specify a non-empty backup file extension in the -i flag, e.g. -i.orig or -i.bak. (Both GNU sed and BSD sed support this notation with no space between when the backup filename extension is not empty.) Then we could compare the .orig or .bak file against the modified file to see if it was modified. But having sed create the backup file would not be significantly different from having MacPorts create the temp file, would it?

Note: See TracTickets for help on using tickets.