Re: pg_regress: stat correct paths - Mailing list pgsql-patches

From Jorgen Austvik
Subject Re: pg_regress: stat correct paths
Date
Msg-id 474B0201.4040405@sun.com
Whole thread Raw
In response to Re: pg_regress: stat correct paths  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: pg_regress: stat correct paths  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-patches
Tom Lane wrote:
> Jorgen Austvik <Jorgen.Austvik@Sun.COM> writes:
>> Tom Lane wrote:
>>> Actually, it looks to me like the patch is wrong specifically because
>>> it does not do the right thing in the VPATH case.
>
>> Are you thinking about "failing if the folders are missing" as "not the
>> right thing in the VPATH case", or are you thinking about something else?
>
> The point is that in VPATH you are running in a build tree, and should
> copy the source files from the source tree, but *not* modify the source
> tree.  Thus, fetching relative to $srcdir but writing relative to .
> is in fact the correct behavior.

Ah, I understand.

It is this part you don't like:
---8<--------------8<--------------8<--------------8<--------------8<-----------
+     snprintf(destdir, MAXPGPATH, "%s/%s", abs_srcdir, dest);
<snip>
!         snprintf(destfile, MAXPGPATH, "%s/%s.%s", destdir, prefix, suffix);
---8<--------------8<--------------8<--------------8<--------------8<-----------

Thanks for the guidance, I'll try come up with a better alternative.
Should outputdir be used instead of current working directory (cwd) if
it is set?

> There has not previously been any complaint that pg_regress was broken
> in this regard, so maybe you should take two steps back and explain what
> problem you think needs fixing, rather than just dropping a patch on us.

I tried to explain it in the mail, but let me try again, this time
showing some code.

Here we stat <cwd>/input:

---8<--------------8<--------------8<--------------8<--------------8<-----------
static void
convert_sourcefiles(void)
{
    struct stat st;
    int            ret;

    ret = stat("input", &st);
    if (ret == 0 && S_ISDIR(st.st_mode))
        convert_sourcefiles_in("input", "sql", "sql");
---8<--------------8<--------------8<--------------8<--------------8<-----------

But if we have set srcdir, the directory we are stat'ing, is not the
same directory that we are reading the files from:

---8<--------------8<--------------8<--------------8<--------------8<-----------
static void
convert_sourcefiles_in(char *source, char *dest, char *suffix)

         <snip>

    if (srcdir)
        strcpy(abs_srcdir, srcdir);
    else
        strcpy(abs_srcdir, abs_builddir);

    snprintf(indir, MAXPGPATH, "%s/%s", abs_srcdir, source);
    names = pgfnames(indir);
---8<--------------8<--------------8<--------------8<--------------8<-----------

So I wanted to provide a patch that ran stat on the folder that we were
reading the files from.

-J
--
Jørgen Austvik, Software Engineering
Database Technology Group


pgsql-patches by date:

Previous
From: Tom Lane
Date:
Subject: Re: pg_regress: stat correct paths
Next
From: Tom Lane
Date:
Subject: Re: pg_regress: stat correct paths