On Fri, Dec 17, 2021 at 11:21:13AM -0600, Justin Pryzby wrote:
> I put this together in the simplest way, prefixing all the filenames with the
> configured path..

Well, why not.

> Another options is to chdir() into the given path.  But, pg_upgrade takes (and
> requires) a bunch of other paths, like -d -D -b -B, and those are 
> traditionally
> interpretted relative to CWD.  I could getcwd() and prefix all the -[dDbB] 
> with
> that, but prefixing a handful of binary/data paths is hardly better than
> prefixing a handful of dump/logfile paths.  I suppose that openat() isn't
> portable.  I don't think this it's worth prohibiting relative paths, so I 
> can't
> think of any less-naive way to do this.

If we add a new file, .gitignore would find about it quickly and
inform about a not-so-clean tree.  I would tend to prefer your
approach, here.  Relative paths can be useful.

> I didn't move the delete-old-cluster.sh, since that's intended to stay around
> even after a successful upgrade, as opposed to the other logs, which are
> typically removed at that point.

Makes sense to me.

+       log_opts.basedir = getenv("PG_UPGRADE_LOGDIR");
+       if (log_opts.basedir != NULL)
+               log_opts.basedir = strdup(log_opts.basedir);
+       else
+               log_opts.basedir = "pg_upgrade_log.d";
Why is this controlled with an environment variable?  It seems to me
that an option switch would be much better, no?  While tuning things,
we could choose something simpler for the default, like
"pg_upgrade_log".  I don't have a good history in naming new things,
though :)

.gitignore should be updated, I guess?  Besides, this patch has no
documentation.
--
Michael

Attachment: signature.asc
Description: PGP signature

Reply via email to