[Tom Lane <tgl@sss.pgh.pa.us>]

Robert Haas <robertmhaas@gmail.com> writes:
> On Thu, Apr 26, 2018 at 11:29 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Personally, I use ccache which doesn't seem to care too much, but I agree
>> than in some usages, extra touches of headers would be costly.  Perhaps
>> it's worth adding logic to avoid overwriting an existing output file
>> unless it changed?  I'm not sure; that would cost something too.

> It seems like a good idea to me.

I took a quick look into this.  It's very easy to do so far as the Perl
code is concerned: just teach Catalog::RenameTempFile that if the target
file already exists, run File::Compare::compare to see if its contents are
identical to the temp file, and if so unlink the temp file rather than
renaming.  I've tried this, it works, and I can't measure any difference
in the runtime of genbki.pl (at least on my usual dev machine).  So it
seems like there's little downside.

However, RenameTempFile is also used by Gen_fmgrtab.pl, and having the
same sort of no-touch semantics for fmgroids.h and friends would have some
additional fallout.  The makefiles would think they have to keep
re-running Gen_fmgrtab.pl if fmgroids.h is older than the mod time on any
input file, and that's certainly no good.  We can fix that by inventing a
stamp file for the Gen_fmgrtab.pl run, analogous to bki-stamp for the
genbki.pl run, but that means changes in the makefiles that go a bit
beyond the realm of triviality.

A compromise answer is to arrange things so that genbki.pl has no-touch
semantics but Gen_fmgrtab.pl doesn't, requiring either two support
functions in Catalog.pm or an extra argument to RenameTempFile.  But
that's really ugly.  Besides, if we're trying to avoid excess recompiles
due to useless touches of common header files, then failing to have
no-touch behavior for fmgroids.h is leaving a lot of money on the table.
So I don't like that answer at all.

Anyway, I'm happy to go make this happen; it looks like only another hour
or so's worth of work to fix the makefiles.  But I wonder if anyone will
say this is too much churn for post-feature-freeze and we should shelve
it till v12.

			regards, tom lane

Attachments:

wip-dont-overwrite-unchanged-headers.patch (text/x-diff, 1.8K) download
diff --git a/src/backend/catalog/Catalog.pm b/src/backend/catalog/Catalog.pm
index eee7cb3..1b31cdd 100644
*** a/src/backend/catalog/Catalog.pm
--- b/src/backend/catalog/Catalog.pm
*************** package Catalog;
*** 16,21 ****
--- 16,24 ----
  use strict;
  use warnings;
  
+ use File::Compare;
+ 
+ 
  # Parses a catalog header file into a data structure describing the schema
  # of the catalog.
  sub ParseHeader
*************** sub AddDefaultValues
*** 336,350 ****
  }
  
  # Rename temporary files to final names.
! # Call this function with the final file name and the .tmp extension
  # Note: recommended extension is ".tmp$$", so that parallel make steps
! # can't use the same temp files
  sub RenameTempFile
  {
  	my $final_name = shift;
  	my $extension  = shift;
  	my $temp_name  = $final_name . $extension;
! 	rename($temp_name, $final_name) || die "rename: $temp_name: $!";
  }
  
  # Find a symbol defined in a particular header file and extract the value.
--- 339,367 ----
  }
  
  # Rename temporary files to final names.
! # Call this function with the final file name and the .tmp extension.
! #
! # If the final file already exists and has identical contents, don't
! # overwrite it; this behavior avoids unnecessary recompiles due to updating
! # the mod date on header files.
! #
  # Note: recommended extension is ".tmp$$", so that parallel make steps
! # can't use the same temp files.
  sub RenameTempFile
  {
  	my $final_name = shift;
  	my $extension  = shift;
  	my $temp_name  = $final_name . $extension;
! 
! 	if (-f $final_name &&
! 		compare($temp_name, $final_name) == 0)
! 	{
! 		unlink $temp_name || die "unlink: $temp_name: $!";
! 	}
! 	else
! 	{
! 		rename($temp_name, $final_name) || die "rename: $temp_name: $!";
! 	}
  }
  
  # Find a symbol defined in a particular header file and extract the value.