Re: [PATCH] test/ssl: rework the sslfiles Makefile target - Mailing list pgsql-hackers

From Jacob Champion
Subject Re: [PATCH] test/ssl: rework the sslfiles Makefile target
Date
Msg-id 8538b3f9ea1b24fdd41e515caf2593b92c37074e.camel@vmware.com
Whole thread Raw
In response to Re: [PATCH] test/ssl: rework the sslfiles Makefile target  (Michael Paquier <michael@paquier.xyz>)
List pgsql-hackers
On Tue, 2021-08-10 at 16:22 +0900, Michael Paquier wrote:
> Regarding 0002, I am not sure.  Even if this reduces a lot of
> duplication, which is really nice, enforcing .SECONDARY to not trigger
> with a change impacting Makefile.global.in does not sound very
> appealing to me in the long-run, TBH.

De-duplication isn't the primary goal of the .SECONDARY change. It
definitely helps with that, but the major improvement is that Make can
maintain the CA state with less hair-pulling:

1. Developer updates an arbitrary number of certificates and runs
   `make sslfiles`.
2. Make sees that the CA state is missing, and creates it once at the
   start of the run.
3. Make runs multiple `openssl ca` commands, depending on the
   certificates being changed, which modify the CA state as they go.
4. After Make is finished, it removes all the CA files, putting your
   local state back the way it was before you ran Make.

Doing it this way has several advantages:

- The magic state files don't persist to influence a future Make run,
  so there's less chance of "I generated with local changes, then
  pulled in the changes you made, and now everything's busted in weird
  ways because my CA state disagrees with what's in the tree".

- Order-only intermediates do The Right Thing in this case -- create
  once when needed, accumulate state during the run, remove at the end
  -- whether you add a single certificate, or regenerate the entire
  tree, or even Ctrl-C halfway through. That's going to be very hard to
  imitate by sprinkling `rm`s like the current Makefile does, though
  I've been the weeds long enough that maybe I'm missing an obvious
  workaround.

- If, after all that, something still goes wrong (your machine crashes
  so Make can't clean up), `git status` will now help you debug
  dependency problems because it's no longer "normal" to carry the
  intermediate litter in your source tree.

What it doesn't fix is the fact that we're still checking in generated
files that have interdependencies, so the timestamps Make is looking at
are still going to be wrong during initial checkout. That problem
existed before and will persist after this change.

On Wed, 2021-08-11 at 09:47 +0900, Michael Paquier wrote:
> The part I am mainly objecting to is the change in Makefile.global.in,
> but I have to admit after thinking about it that enforcing SECONDARY
> may not be a good idea if other parts of the system rely on that, so
> encouraging the use of clean_intermediates may be dangerous (Tom's
> point from upthread).

I don't really want to encourage the use of clean_intermediates. I just
want Make to have its default, useful behavior for this one Makefile.

> I have not tried so I am not sure, but perhaps we should just focus on
> reducing the number of openssl commands rather than making easier the
> integration of new files?  It could be possible to close the gap with
> the addition of new files with some more documentation for future
> hackers then?

I'd rather fix the dependency/state bugs than document how to work
around them. I know the workarounds; it doesn't make working with this
Makefile any less maddening.

--Jacob

pgsql-hackers by date:

Previous
From: Thomas Munro
Date:
Subject: Re: Worth using personality(ADDR_NO_RANDOMIZE) for EXEC_BACKEND on linux?
Next
From: Jacob Champion
Date:
Subject: Re: [PATCH] test/ssl: rework the sslfiles Makefile target